Merge branch 'master' into contentsync

CHANGELOG.md

@@ -11,10 +11,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com)
 ## Unreleased ([details][unreleased changes details])
 - #3443 - Content Sync: don't drill down into content tree if recursion is off
 
-### Changed
+### Fixed
+- #3459 - Top level properties in parameterized include are now respected.
+- #3460 - Fixes issue where double parameters were not working for the parameterized include
 
-- #3420 - Redirect Map Manager - enable Redirect Map Manager in AEM CS (would require a specific - not public yet - AEM
-  CS release version, TBA)
+### Changed
+- #3385 Made nesting parameterized includes inside a multi-field (ignored resource types) possible
 
 ## 6.8.0 - 2024-10-17
 

bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImpl.java

@@ -104,30 +104,39 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
 
         ValueMap parameters = ValueMap.EMPTY;
 
-        if(servletRequest instanceof SlingHttpServletRequest){
-
-            SlingHttpServletRequest request = (SlingHttpServletRequest) servletRequest;
-
-            Predicate<String> typeCheckFn = (resourceType) -> request.getResourceResolver().isResourceType(request.getResource(), resourceType);
-
-            if(typeCheckFn.test(RESOURCE_TYPE)){
-                performFilter(request, servletResponse, chain, parameters);
-                return;
-            }else if(resourceTypesIgnoreChildren.stream().anyMatch(typeCheckFn)){
-                boolean ignoreChildren = resourceTypesIgnoreChildren.stream().anyMatch(typeCheckFn);
-                if(ignoreChildren){
-                    request.setAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE, request.getResource().getResourceType());
-                }
-                chain.doFilter(servletRequest, servletResponse);
-                if(ignoreChildren){
-                    request.removeAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE);
-                }
-                return;
-            }
+        if(!(servletRequest instanceof SlingHttpServletRequest)){
+            chain.doFilter(servletRequest, servletResponse);
+            return;
+        }
+        SlingHttpServletRequest request = (SlingHttpServletRequest) servletRequest;
 
+        Predicate<String> typeCheckFn = (resourceType) -> request.getResourceResolver().isResourceType(request.getResource(), resourceType);
+
+        if(typeCheckFn.test(RESOURCE_TYPE)){
+            Object ignoreResourceType = request.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE);
+            Object namespace = request.getAttribute(REQ_ATTR_NAMESPACE);
+            //if children ignore is active, but we have a new include, we de-activate the ignore children.
+            if(ignoreResourceType != null){
+                request.removeAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE);
+                request.removeAttribute(REQ_ATTR_NAMESPACE);
+            }
+            performFilter(request, servletResponse, chain, parameters);
+            // we are now out of the nested include context. re-activate the ignore children if it was active before.
+            if(ignoreResourceType != null){
+                request.setAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE, ignoreResourceType);
+                request.setAttribute(REQ_ATTR_NAMESPACE, namespace);
+            }
+        }else if(resourceTypesIgnoreChildren.stream().anyMatch(typeCheckFn)){
+            boolean ignoreChildren = resourceTypesIgnoreChildren.stream().anyMatch(typeCheckFn);
+            if(ignoreChildren){
+                request.setAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE, request.getResource().getResourceType());
+            }
+            chain.doFilter(servletRequest, servletResponse);
+            if(ignoreChildren){
+                request.removeAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE);
+            }
         }
 
-        chain.doFilter(servletRequest, servletResponse);
     }
 
     private void performFilter(SlingHttpServletRequest request, ServletResponse servletResponse, FilterChain chain, ValueMap parameters) throws IOException, ServletException {

bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceDecoratedValueMapBuilder.java

@@ -47,12 +47,17 @@ public class NamespaceDecoratedValueMapBuilder {
     private final Map<String,Object> copyMap;
     private final String[] namespacedProperties;
 
-    static final Pattern PLACEHOLDER_PATTERN = Pattern.compile("(\\$\\{\\{([a-zA-Z0-9]+?)(:(.+?))??\\}\\})+?");
-    static final Pattern PLACEHOLDER_TYPE_HINTED_PATTERN = Pattern.compile("(.*)\\$\\{\\{(\\(([a-zA-Z]+)\\)){1}([a-zA-Z0-9]+)(:(.+))?\\}\\}(.*)?");
+    static final Pattern PLACEHOLDER_PATTERN = Pattern.compile("\\$\\{\\{(?:\\(([a-zA-Z]+)\\))?([a-zA-Z0-9]+)(:(.*?))?\\}\\}");
 
-    public NamespaceDecoratedValueMapBuilder(SlingHttpServletRequest request, Resource resource, String[] namespacedProperties) {
+    public NamespaceDecoratedValueMapBuilder(SlingHttpServletRequest request, Resource resource, String[] namespacedProperties, boolean copyToplevelProperties) {
         this.request = request;
-        this.copyMap = new HashMap<>(resource.getValueMap());
+        this.copyMap = new HashMap<>();
+        if(copyToplevelProperties && request.getResourceResolver().isResourceType(resource, RESOURCE_TYPE)){
+            // if the node is the include node, we have to preload the properties from the snippet root level
+            preloadPropsOnRootLevel(request, resource);
+        }
+        this.copyMap.putAll(resource.getValueMap());
+
         this.namespacedProperties = Optional.ofNullable(namespacedProperties)
                 .map(array -> Arrays.copyOf(array, array.length))
                 .orElse(new String[0]);
@@ -61,6 +66,25 @@ public NamespaceDecoratedValueMapBuilder(SlingHttpServletRequest request, Resour
         this.applyNameSpacing();
     }
 
+    private void preloadPropsOnRootLevel(SlingHttpServletRequest request, Resource resource) {
+        String path = resource.getValueMap().get("path", "");
+
+        if(StringUtils.isNotBlank(path)){
+            Resource snippetResource = request.getResourceResolver().getResource(path);
+
+            if(snippetResource != null){
+                ValueMap inclProps = snippetResource.getValueMap();
+                this.copyMap.putAll(inclProps);
+
+                // if we have sling resourceType on the snippet, we have to put it in front as resourceType.
+                if(inclProps.containsKey("sling:resourceType")){
+                    this.copyMap.put("resourceType", inclProps.get("sling:resourceType"));
+                }
+            }
+
+        }
+    }
+
     /**
      * Checks whether the resource type given is the request resource's resource type.
      * Using a separate branch for unit tests, as the AEM Mocks are bugged (returns the jcr:primaryType instead)
@@ -129,59 +153,34 @@ private void applyDynamicVariables() {
 
 
     private Object filter(String value, SlingHttpServletRequest request) {
-        Object filtered = applyTypeHintedPlaceHolders(value, request);
-
-        if(filtered != null){
-            return filtered;
-        }
-
-        return applyPlaceHolders(value, request);
-    }
-
-    private Object applyTypeHintedPlaceHolders(String value, SlingHttpServletRequest request) {
-        Matcher matcher = PLACEHOLDER_TYPE_HINTED_PATTERN.matcher(value);
-
-        if (matcher.find()) {
-
-            String prefix = matcher.group(1);
-            String typeHint = matcher.group(3);
-            String paramKey = matcher.group(4);
-            String defaultValue = matcher.group(6);
-            String suffix = matcher.group(7);
-
-            String requestParamValue = (request.getAttribute(PREFIX + paramKey) != null) ? request.getAttribute(PREFIX + paramKey).toString() : null;
-            String chosenValue = defaultString(requestParamValue, defaultValue);
-            String finalValue = defaultIfEmpty(prefix, EMPTY) + chosenValue + defaultIfEmpty(suffix, EMPTY);
-
-            return isNotEmpty(typeHint) ? castTypeHintedValue(typeHint, finalValue) : finalValue;
-        }
-
-        return null;
-    }
-
-    private String applyPlaceHolders(String value, SlingHttpServletRequest request) {
         Matcher matcher = PLACEHOLDER_PATTERN.matcher(value);
-        StringBuffer buffer = new StringBuffer();
+
+        // Replace all occurrences
+        StringBuffer result = new StringBuffer();
 
         while (matcher.find()) {
+            // Retrieve groups for Typecast, paramKey, and default value
 
+            String typeHint = matcher.group(1);
             String paramKey = matcher.group(2);
             String defaultValue = matcher.group(4);
 
             String requestParamValue = (request.getAttribute(PREFIX + paramKey) != null) ? request.getAttribute(PREFIX + paramKey).toString() : null;
             String chosenValue = defaultString(requestParamValue, defaultValue);
 
-            if(chosenValue == null){
-                chosenValue = StringUtils.EMPTY;
-            }
-
-            matcher.appendReplacement(buffer, chosenValue);
+            String replacement =  isNotEmpty(typeHint) ? castTypeHintedValue(typeHint, chosenValue).toString() : chosenValue;
 
+            if(replacement == null){
+                replacement = "";
+            }
+            // Append the replacement to the result
+            matcher.appendReplacement(result, replacement);
         }
 
-        matcher.appendTail(buffer);
+        // Append the remaining text
+        matcher.appendTail(result);
 
-        return buffer.toString();
+        return result.toString();
     }
 
     private Object castTypeHintedValue(String typeHint, String chosenValue) {

bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapper.java

@@ -41,17 +41,22 @@ public class NamespaceResourceWrapper extends FilteringResourceWrapper {
 
     private final ValueMap valueMap;
 
+    private boolean copyToplevelProperties;
+
     public NamespaceResourceWrapper(@NotNull Resource resource, @NotNull ExpressionResolver expressionResolver,
                                     @NotNull SlingHttpServletRequest request,
-                                    String[] namespacedProperties) {
+                                    String[] namespacedProperties,
+                                    boolean copyToplevelProperties) {
         super(resource, expressionResolver, request);
         this.expressionResolver = expressionResolver;
         this.request = request;
         this.namespacedProperties = Optional.ofNullable(namespacedProperties)
                 .map(array -> Arrays.copyOf(array, array.length))
                 .orElse(new String[0]);
 
-        valueMap = new NamespaceDecoratedValueMapBuilder(request, resource, namespacedProperties).build();
+        this.copyToplevelProperties = copyToplevelProperties;
+
+        valueMap = new NamespaceDecoratedValueMapBuilder(request, resource, namespacedProperties,copyToplevelProperties).build();
     }
 
     @Override
@@ -62,7 +67,7 @@ public Resource getChild(String relPath) {
             return null;
         }
 
-        NamespaceResourceWrapper wrapped =new NamespaceResourceWrapper(child, expressionResolver, request,namespacedProperties);
+        NamespaceResourceWrapper wrapped =new NamespaceResourceWrapper(child, expressionResolver, request, namespacedProperties, copyToplevelProperties);
 
         if(!isVisible(wrapped)){
             return null;
@@ -75,8 +80,8 @@ public Resource getChild(String relPath) {
     public Iterator<Resource> listChildren() {
         return new TransformIterator(
                 new FilterIterator(super.listChildren(),
-                        o -> isVisible(new NamespaceResourceWrapper((Resource) o, expressionResolver, request,namespacedProperties))),
-                        o -> new NamespaceResourceWrapper((Resource) o, expressionResolver, request,namespacedProperties)
+                        o -> isVisible(new NamespaceResourceWrapper((Resource) o, expressionResolver, request, namespacedProperties, copyToplevelProperties))),
+                        o -> new NamespaceResourceWrapper((Resource) o, expressionResolver, request, namespacedProperties, copyToplevelProperties)
         );
     }
 

bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespacedTransformedResourceProviderImpl.java

@@ -43,17 +43,25 @@ public class NamespacedTransformedResourceProviderImpl implements NamespacedTran
                 description = "Properties that should be namespaced"
         )
         String[] properties() default {"name", "fileNameParameter", "fileReferenceParameter"};
+
+        @AttributeDefinition(
+                name = "Copy top level properties",
+                description = "Copy the top level properties of the snippets to the include node"
+        )
+        boolean copyToplevelProperties() default true;
     }
 
     @Reference
     private ExpressionResolver expressionResolver;
 
     private String[] namespacedProperties;
+    private boolean copyToplevelProperties;
 
     @Activate
     @Modified
     public void init(Config config) {
         this.namespacedProperties = config.properties();
+        this.copyToplevelProperties = config.copyToplevelProperties();
     }
 
     @Override
@@ -62,7 +70,9 @@ public Resource transformResourceWithNameSpacing(SlingHttpServletRequest request
                 targetResource,
                 expressionResolver,
                 request,
-                namespacedProperties);
+                namespacedProperties,
+                copyToplevelProperties
+        );
     }
 
 }

bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImplTest.java

@@ -37,13 +37,13 @@
 
 import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE;
 import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.REQ_ATTR_NAMESPACE;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 import static org.mockito.ArgumentMatchers.any;
 
 @RunWith(MockitoJUnitRunner.class)
 public class IncludeDecoratorFilterImplTest {
 
+    public static final String IGNORE_CHILDREN_RESOURCE_TYPE = "ignore/children/resource/type";
     @Mock
     FilterChain filterChain;
 
@@ -69,7 +69,7 @@ public Class<? extends Annotation> annotationType() {
 
             @Override
             public String[] resourceTypesIgnoreChildren() {
-                return new String[]{"ignore/children/resource/type"};
+                return new String[]{IGNORE_CHILDREN_RESOURCE_TYPE};
             }
         });
 
@@ -98,7 +98,7 @@ public void test_disable_namespacing_children() throws IOException, ServletExcep
         Mockito.doAnswer(invocationOnMock -> {
 
             SlingHttpServletRequest captured = invocationOnMock.getArgument(0, SlingHttpServletRequest.class);
-            assertEquals("ignore/children/resource/type", captured.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE));
+            assertEquals(IGNORE_CHILDREN_RESOURCE_TYPE, captured.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE));
             return null;
 
         }).when(filterChain).doFilter(any(SlingHttpServletRequest.class), any(SlingHttpServletResponse.class));
@@ -109,6 +109,26 @@ public void test_disable_namespacing_children() throws IOException, ServletExcep
         assertTrue("namespace is removed after the filter is performed", context.request().getAttribute(REQ_ATTR_NAMESPACE) == null);
     }
 
+    @Test
+    public void test_ignored_types_reactivate() throws IOException, ServletException {
+
+        Mockito.doAnswer(invocationOnMock -> {
+
+            SlingHttpServletRequest captured = invocationOnMock.getArgument(0, SlingHttpServletRequest.class);
+            assertEquals("block1", captured.getAttribute(REQ_ATTR_NAMESPACE));
+            assertNull(captured.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE));
+            return null;
+
+        }).when(filterChain).doFilter(any(SlingHttpServletRequest.class), any(SlingHttpServletResponse.class));
+
+        context.request().setAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE, IGNORE_CHILDREN_RESOURCE_TYPE);
+        context.request().setAttribute(REQ_ATTR_NAMESPACE, "nested");
+        systemUnderTest.doFilter(context.request(), context.response(), filterChain);
+
+        assertEquals("nested", context.request().getAttribute(REQ_ATTR_NAMESPACE));
+        assertEquals(IGNORE_CHILDREN_RESOURCE_TYPE, context.request().getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE));
+    }
+
     @Test
     public void test_nested_include() throws IOException, ServletException {