From 0a442d73bd1b78bc92700e850e45cea0cb15f2ef Mon Sep 17 00:00:00 2001 From: Niek Raaijmakers Date: Tue, 29 Oct 2024 19:33:57 +0100 Subject: [PATCH] Granite include top level fixes (#3457) * Fix top level include properties not being passed over. --------- Co-authored-by: Niek Raaijmakers --- CHANGELOG.md | 2 +- .../NamespaceDecoratedValueMapBuilder.java | 29 +++++++++++++++++-- .../include/NamespaceResourceWrapper.java | 15 ++++++---- ...spacedTransformedResourceProviderImpl.java | 12 +++++++- .../include/NamespaceResourceWrapperTest.java | 8 ++--- .../granite/ui/components/include/include.jsp | 4 +-- 6 files changed, 55 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba91ec55bd..e5cca6c850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Unreleased ([details][unreleased changes details]) ### Fixed - +- #3459 - Top level properties in parameterized include are now respected. - #3460 - Fixes issue where double parameters were not working for the parameterized include ### Changed diff --git a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceDecoratedValueMapBuilder.java b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceDecoratedValueMapBuilder.java index ef1ac7d0b6..9f05e4c6a5 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceDecoratedValueMapBuilder.java +++ b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceDecoratedValueMapBuilder.java @@ -49,9 +49,15 @@ public class NamespaceDecoratedValueMapBuilder { 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]); @@ -60,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) diff --git a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapper.java b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapper.java index a89012a938..000057ea42 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapper.java +++ b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapper.java @@ -41,9 +41,12 @@ 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; @@ -51,7 +54,9 @@ public NamespaceResourceWrapper(@NotNull Resource resource, @NotNull ExpressionR .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 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) ); } diff --git a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespacedTransformedResourceProviderImpl.java b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespacedTransformedResourceProviderImpl.java index 44fdead4b2..70a6c382de 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespacedTransformedResourceProviderImpl.java +++ b/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 + ); } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapperTest.java b/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapperTest.java index 94b6eef30f..40e79177a5 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapperTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/NamespaceResourceWrapperTest.java @@ -79,7 +79,7 @@ public void test() { setParameters(parameters); - systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(), properties); + systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(), properties,true); Resource someMultiExpressionField = systemUnderTest.getChild("someMultiExpressionField"); String multiExpressionValue = someMultiExpressionField.getValueMap().get("fieldDescription", ""); @@ -106,7 +106,7 @@ public void test() { @Test public void test_default_values() { - systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties); + systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties, true); Resource someDoubleField = systemUnderTest.getChild("someDoubleField"); Double doubleDefaultValue = someDoubleField.getValueMap().get("defaultValue", Double.class); @@ -134,7 +134,7 @@ public void test_default_values() { public void test_namespacing() { context.request().setAttribute(REQ_ATTR_NAMESPACE, "block1"); - systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties); + systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties,true); Resource someDoubleField = systemUnderTest.getChild("someDoubleField"); @@ -148,7 +148,7 @@ public void test_namespacing_disabled_for_child() { context.request().setAttribute(REQ_ATTR_NAMESPACE, "block1"); context.request().setAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE, "ignore/children/resource/type"); context.request().setAttribute(REQ_ATTR_TEST_FLAG, true); - systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties); + systemUnderTest = new NamespaceResourceWrapper(context.currentResource(), expressionResolver, context.request(),properties,true); Resource shouldIgnoreChildrenField = systemUnderTest.getChild("fieldWithChildrenThatShouldBeIgnored"); diff --git a/ui.apps/src/main/content/jcr_root/apps/acs-commons/granite/ui/components/include/include.jsp b/ui.apps/src/main/content/jcr_root/apps/acs-commons/granite/ui/components/include/include.jsp index 527282115d..e80d886e4f 100644 --- a/ui.apps/src/main/content/jcr_root/apps/acs-commons/granite/ui/components/include/include.jsp +++ b/ui.apps/src/main/content/jcr_root/apps/acs-commons/granite/ui/components/include/include.jsp @@ -22,5 +22,5 @@ NamespacedTransformedResourceProvider transformedResourceProvider = sling.getService(NamespacedTransformedResourceProvider.class); Resource resourceWrapper = transformedResourceProvider.transformResourceWithNameSpacing(slingRequest, targetResource); - cmp.include(resourceWrapper, cfg.get("resourceType", String.class), cmp.getOptions()); -%> \ No newline at end of file + cmp.include(resourceWrapper, cfg.get("resourceType", "granite/ui/components/coral/foundation/container"), cmp.getOptions()); +%>