Skip to content

Commit

Permalink
resolves #2716 optimize SCP BVP execution (#2717)
Browse files Browse the repository at this point in the history 
* resolves #2716 optimize SCP BVP execution

Co-authored-by: david g <[email protected]>
  • Loading branch information
@adamcin @davidjgonzalez
adamcin and davidjgonzalez authored Oct 20, 2021
1 parent 5d0de7b commit bc2e231
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 23 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
### Fixed

- #2704 - Fixed issue with MCP report generation throwing an exception, and fixed some minor UI issues on AEM SDK (added BG color)
- #2716 - Fixed issue with Shared Component Properties Bindings Values Provider facing lock contention
- #2718 - Fixes CM Code Quality Pipeline failure caused by TestMarketoInterfaces and Jacoco instrumentation
- #2713 - Marketo form/cloud config root missing 
- #2714 - Implemented shared and global component properties to work in experience fragments.
Expand Down
60 changes: 45 additions & 15 deletions ...s/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@

import com.adobe.acs.commons.wcm.PageRootProvider;
import com.adobe.acs.commons.wcm.properties.shared.SharedComponentProperties;
import com.day.cq.wcm.api.Page;
import com.day.cq.wcm.api.components.Component;
import com.day.cq.wcm.commons.WCMUtils;
import org.apache.felix.scr.annotations.Component;

import org.apache.commons.lang3.StringUtils;
import org.apache.felix.scr.annotations.Reference;
Expand Down Expand Up @@ -54,7 +52,7 @@
* to instance-level values, then shared values, and finally global
* values when properties exist at multiple levels with the same name.
*/
@org.apache.felix.scr.annotations.Component
@Component
@Service
public class SharedComponentPropertiesBindingsValuesProvider implements BindingsValuesProvider {
private static final Logger log = LoggerFactory.getLogger(SharedComponentPropertiesBindingsValuesProvider.class);
Expand All @@ -68,34 +66,66 @@ public class SharedComponentPropertiesBindingsValuesProvider implements Bindings
@Override
public void addBindings(Bindings bindings) {
Resource resource = (Resource) bindings.get("resource");
Component component = WCMUtils.getComponent(resource);
if (component != null) {
if (resource != null) {
if (pageRootProvider != null) {
setSharedProperties(bindings, resource, component);
setSharedProperties(bindings, resource);
} else {
log.debug("Page Root Provider must be configured for shared component properties to be supported");
}
setMergedProperties(bindings, resource);
}
}

private void setSharedProperties(Bindings bindings, Resource resource, Component component) {
/**
* Construct a canonical resource type relative path for the provided resource type,
* or null if the result is not acceptable.
* Step 1: discard empty and JCR node types / sling:nonexisting (contains ":")
* Step 2: return result if already relative (does not start with /)
* Step 3: relativize an absolute path using elements of {@code searchPaths} and return the first match found.
*
* @param resourceType the request resource resourceType
* @param searchPaths {@link org.apache.sling.api.resource.ResourceResolver#getSearchPath()}
* @return the canonical resource type or null
*/
static String getCanonicalResourceTypeRelativePath(final String resourceType, final String[] searchPaths) {
if (StringUtils.isEmpty(resourceType) || resourceType.contains(":")) {
return null;
}

if (resourceType.charAt(0) != '/') {
return resourceType;
} else if (searchPaths != null) {
for (final String searchPath : searchPaths) {
if (resourceType.startsWith(searchPath)) {
return resourceType.substring(searchPath.length());
}
}
}
return null;
}

private void setSharedProperties(Bindings bindings, Resource resource) {
String rootPagePath = pageRootProvider.getRootPagePath(resource.getPath());
if (StringUtils.isNotBlank(rootPagePath)) {
String rootPageContentPath = rootPagePath + "/jcr:content/";
String globalPropsPath = rootPageContentPath + SharedComponentProperties.NN_GLOBAL_COMPONENT_PROPERTIES;

Resource globalPropsResource = resource.getResourceResolver().getResource(globalPropsPath);
if (globalPropsResource != null) {
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES, globalPropsResource.getValueMap());
bindings.put(SharedComponentProperties.GLOBAL_PROPERTIES_RESOURCE, globalPropsResource);
}

String sharedPropsPath = rootPageContentPath + SharedComponentProperties.NN_SHARED_COMPONENT_PROPERTIES + "/"
+ component.getResourceType();
Resource sharedPropsResource = resource.getResourceResolver().getResource(sharedPropsPath);
if (sharedPropsResource != null) {
bindings.put(SharedComponentProperties.SHARED_PROPERTIES, sharedPropsResource.getValueMap());
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE, sharedPropsResource);

final String resourceTypeRelativePath = getCanonicalResourceTypeRelativePath(resource.getResourceType(),
resource.getResourceResolver().getSearchPath());
if (resourceTypeRelativePath != null) {
String sharedPropsPath = rootPageContentPath + SharedComponentProperties.NN_SHARED_COMPONENT_PROPERTIES + "/"
+ resourceTypeRelativePath;
Resource sharedPropsResource = resource.getResourceResolver().getResource(sharedPropsPath);
if (sharedPropsResource != null) {
bindings.put(SharedComponentProperties.SHARED_PROPERTIES, sharedPropsResource.getValueMap());
bindings.put(SharedComponentProperties.SHARED_PROPERTIES_RESOURCE, sharedPropsResource);
}
}
} else {
log.debug("Could not determine shared properties root for resource {}", resource.getPath());
Expand Down
54 changes: 46 additions & 8 deletions ...mmons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package com.adobe.acs.commons.wcm.properties.shared.impl;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
Expand All @@ -39,7 +40,13 @@
import org.mockito.junit.MockitoJUnitRunner;
import org.powermock.reflect.Whitebox;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import java.util.function.BiFunction;
import java.util.function.Function;
import javax.script.Bindings;
import javax.script.SimpleBindings;

Expand All @@ -55,9 +62,7 @@ public class SharedComponentPropertiesBindingsValuesProviderTest {
private Resource globalPropsResource;
private Page page;
private Bindings bindings;
private Component component;
private ResourceResolver resourceResolver;
private ComponentManager componentManager;
private ValueMap sharedProps;
private ValueMap globalProps;

Expand All @@ -67,11 +72,9 @@ public void setUp() throws Exception {
pageRootProvider = mock(PageRootProvider.class);
page = mock(Page.class);
bindings = new SimpleBindings();
component = mock(Component.class);
sharedPropsResource = mock(Resource.class);
globalPropsResource = mock(Resource.class);
resourceResolver = mock(ResourceResolver.class);
componentManager = mock(ComponentManager.class);

final String globalPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_GLOBAL_COMPONENT_PROPERTIES;
final String sharedPropsPath = SITE_ROOT + "/jcr:content/" + SharedComponentProperties.NN_SHARED_COMPONENT_PROPERTIES + "/"
Expand All @@ -80,15 +83,15 @@ public void setUp() throws Exception {
bindings.put("resource", resource);

when(resource.getResourceResolver()).thenReturn(resourceResolver);
when(resource.getResourceType()).thenReturn(RESOURCE_TYPE);
when(resourceResolver.getSearchPath()).thenReturn(new String[]{"/apps/", "/libs/"});
when(resourceResolver.getResource(sharedPropsPath)).thenReturn(sharedPropsResource);
when(resourceResolver.getResource(globalPropsPath)).thenReturn(globalPropsResource);
when(resourceResolver.adaptTo(ComponentManager.class)).thenReturn(componentManager);
when(componentManager.getComponentOfResource(resource)).thenReturn(component);

when(resource.getPath()).thenReturn(SITE_ROOT);
when(pageRootProvider.getRootPagePath(anyString())).thenReturn(SITE_ROOT);
when(component.getResourceType()).thenReturn(RESOURCE_TYPE);


sharedProps = new ValueMapDecorator(new HashMap<String, Object>());
globalProps = new ValueMapDecorator(new HashMap<String, Object>());
sharedProps.put("shared", "value");
Expand All @@ -98,6 +101,41 @@ public void setUp() throws Exception {
when(sharedPropsResource.getValueMap()).thenReturn(sharedProps);
}

@Test
public void testGetCanonicalResourceTypeRelativePath() {
// make this test readable by wrapping the long method name with a function
final BiFunction<String, List<String>, String> asFunction =
(resourceType, searchPaths) -> SharedComponentPropertiesBindingsValuesProvider
.getCanonicalResourceTypeRelativePath(resourceType,
Optional.ofNullable(searchPaths)
.map(list -> list.toArray(new String[0])).orElse(null));

final List<String> emptySearchPaths = Collections.emptyList();
final List<String> realSearchPaths = Arrays.asList("/apps/", "/libs/");
assertNull("expect null for null rt", asFunction.apply(null, emptySearchPaths));
assertNull("expect null for empty rt", asFunction.apply("", emptySearchPaths));
assertNull("expect null for absolute rt and null search paths",
asFunction.apply("/fail/" + RESOURCE_TYPE, null));
assertNull("expect null for cq:Page",
asFunction.apply("cq:Page", realSearchPaths));
assertNull("expect null for nt:unstructured",
asFunction.apply("nt:unstructured", realSearchPaths));
assertNull("expect null for absolute rt and empty search paths",
asFunction.apply("/fail/" + RESOURCE_TYPE, emptySearchPaths));
assertNull("expect null for sling nonexisting rt",
asFunction.apply(Resource.RESOURCE_TYPE_NON_EXISTING, emptySearchPaths));
assertEquals("expect same for relative rt", RESOURCE_TYPE,
asFunction.apply(RESOURCE_TYPE, emptySearchPaths));
assertEquals("expect same for relative rt and real search paths", RESOURCE_TYPE,
asFunction.apply(RESOURCE_TYPE, realSearchPaths));
assertEquals("expect relative for /apps/ + relative and real search paths", RESOURCE_TYPE,
asFunction.apply("/apps/" + RESOURCE_TYPE, realSearchPaths));
assertEquals("expect relative for /libs/ + relative and real search paths", RESOURCE_TYPE,
asFunction.apply("/libs/" + RESOURCE_TYPE, realSearchPaths));
assertNull("expect null for /fail/ + relative and real search paths",
asFunction.apply("/fail/" + RESOURCE_TYPE, realSearchPaths));
}

@Test
public void addBindings() {
final SharedComponentPropertiesBindingsValuesProvider sharedComponentPropertiesBindingsValuesProvider
Expand All @@ -112,4 +150,4 @@ public void addBindings() {
assertEquals(sharedProps, bindings.get(SharedComponentProperties.SHARED_PROPERTIES));
assertEquals(globalProps, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES));
}
}
}

0 comments on commit bc2e231

Please sign in to comment.