diff --git a/CHANGELOG.md b/CHANGELOG.md index 06dff02d51..b250cf5bf8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java index e8d0143606..6f45af0b6c 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProvider.java @@ -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; @@ -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); @@ -68,10 +66,9 @@ 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"); } @@ -79,23 +76,56 @@ public void addBindings(Bindings bindings) { } } - 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()); diff --git a/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java b/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java index 3719364c8c..84095f46e4 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/wcm/properties/shared/impl/SharedComponentPropertiesBindingsValuesProviderTest.java @@ -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; @@ -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; @@ -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; @@ -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 + "/" @@ -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()); globalProps = new ValueMapDecorator(new HashMap()); sharedProps.put("shared", "value"); @@ -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> asFunction = + (resourceType, searchPaths) -> SharedComponentPropertiesBindingsValuesProvider + .getCanonicalResourceTypeRelativePath(resourceType, + Optional.ofNullable(searchPaths) + .map(list -> list.toArray(new String[0])).orElse(null)); + + final List emptySearchPaths = Collections.emptyList(); + final List 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 @@ -112,4 +150,4 @@ public void addBindings() { assertEquals(sharedProps, bindings.get(SharedComponentProperties.SHARED_PROPERTIES)); assertEquals(globalProps, bindings.get(SharedComponentProperties.GLOBAL_PROPERTIES)); } -} \ No newline at end of file +}