diff --git a/CHANGELOG.md b/CHANGELOG.md index d358042603..785f222695 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com) ## Fixed +- #3040 - Fixed bug where namespaced multi-fields would have the namespace 2 times - #3140 - Fixed issue where malformed MCP process nodes can cause a NPE that breaks the entire MPC reporting UI. Now displays more friendly values in UI to help remove the invalid nodes. - #3150 - Support for case-insensitive redirect rules ( [NC] flag equivalent of apache) - #3138 - Re-arrange action removes data from redirect node diff --git a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImpl.java b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImpl.java index 813dde0295..1b58f28092 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImpl.java +++ b/bundle/src/main/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImpl.java @@ -22,8 +22,12 @@ import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.engine.EngineConstants; +import org.osgi.service.component.annotations.Activate; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.ConfigurationPolicy; +import org.osgi.service.component.annotations.Modified; +import org.osgi.service.metatype.annotations.Designate; +import org.osgi.service.metatype.annotations.ObjectClassDefinition; import javax.annotation.CheckForNull; import javax.servlet.Filter; @@ -35,6 +39,9 @@ import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.function.Predicate; @Component( service = Filter.class, @@ -43,6 +50,7 @@ EngineConstants.SLING_FILTER_SCOPE+"="+ EngineConstants.FILTER_SCOPE_INCLUDE } ) +@Designate(ocd = IncludeDecoratorFilterImpl.Config.class) public class IncludeDecoratorFilterImpl implements Filter { static final String RESOURCE_TYPE = "acs-commons/granite/ui/components/include"; @@ -65,7 +73,28 @@ public class IncludeDecoratorFilterImpl implements Filter { * Prefix value parameters passed downwards */ static final String PREFIX = "ACS_AEM_COMMONS_INCLUDE_PREFIX_"; - + + + static final String REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE = "ACS_AEM_COMMONS_EXCLUDE_CHILDREN_RESOURCE_TYPE"; + + private List resourceTypesIgnoreChildren; + + @ObjectClassDefinition( + name = "ACS AEM Commons - IncludeDecoratorFilterImpl", + description = "Used to perform the namespacing / parameterization in the include context") + public @interface Config { + String[] resourceTypesIgnoreChildren() default { + "granite/ui/components/coral/foundation/form/multifield" + }; + } + + @Activate + @Modified + public void init(Config config) { + this.resourceTypesIgnoreChildren = Arrays.asList(config.resourceTypesIgnoreChildren()); + } + + @Override public void init(FilterConfig filterConfig) throws ServletException { // no-op @@ -77,16 +106,28 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo ValueMap parameters = ValueMap.EMPTY; if(servletRequest instanceof SlingHttpServletRequest){ - + SlingHttpServletRequest request = (SlingHttpServletRequest) servletRequest; - if(request.getResourceResolver().isResourceType(request.getResource(), RESOURCE_TYPE)){ + Predicate 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; } - + } - + chain.doFilter(servletRequest, servletResponse); } 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 67da5d1822..661a45f4d7 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 @@ -19,27 +19,29 @@ import com.adobe.acs.commons.util.TypeUtil; import org.apache.commons.lang.StringUtils; +import org.apache.sling.api.SlingConstants; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.wrappers.ValueMapDecorator; +import java.util.Map; import java.util.Arrays; import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; import java.util.Optional; +import java.util.Iterator; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; -import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.REQ_ATTR_NAMESPACE; -import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.PREFIX; +import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.*; import static org.apache.commons.lang3.StringUtils.*; public class NamespaceDecoratedValueMapBuilder { + public static final String REQ_ATTR_TEST_FLAG = "ACS_COMMONS_TEST_FLAG"; private final SlingHttpServletRequest request; private final Map copyMap; @@ -47,7 +49,7 @@ public class NamespaceDecoratedValueMapBuilder { 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]+)(:(.+))?\\}\\}(.*)?"); - + public NamespaceDecoratedValueMapBuilder(SlingHttpServletRequest request, Resource resource, String[] namespacedProperties) { this.request = request; this.copyMap = new HashMap<>(resource.getValueMap()); @@ -59,9 +61,30 @@ public NamespaceDecoratedValueMapBuilder(SlingHttpServletRequest request, Resour this.applyNameSpacing(); } - private void applyNameSpacing() { + /** + * 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) + * @param resourceType + * @return + */ + private boolean isResourceType(String resourceType){ + if(request.getAttribute(REQ_ATTR_TEST_FLAG) != null + && resourceType.equals(copyMap.get( SlingConstants.NAMESPACE_PREFIX +":"+SlingConstants.PROPERTY_RESOURCE_TYPE))){ + return true; + }else { + return request.getResourceResolver().isResourceType(request.getResource(), request.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE).toString()); + } + } - if (request.getAttribute(REQ_ATTR_NAMESPACE) != null) { + private void applyNameSpacing() { + ; + Supplier shouldConsiderNamespacing = () -> + request.getAttribute(REQ_ATTR_NAMESPACE) != null + && ( request.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE) == null + || isResourceType(request.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE).toString()) + ); + + if (shouldConsiderNamespacing.get()) { for (String namespacedProp : namespacedProperties) { if (copyMap.containsKey(namespacedProp)) { @@ -86,85 +109,85 @@ private void applyNameSpacing() { } } - + public ValueMap build(){ return new ValueMapDecorator(copyMap); } - + private void applyDynamicVariables() { for(Iterator> iterator = this.copyMap.entrySet().iterator(); iterator.hasNext();){ - + Map.Entry entry = iterator.next(); - + if(entry.getValue() instanceof String) { final Object filtered = filter(entry.getValue().toString(), this.request); copyMap.put(entry.getKey(), filtered); } - + } } - - + + 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(); - + while (matcher.find()) { - + 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); - + } - + matcher.appendTail(buffer); - + return buffer.toString(); } - + private Object castTypeHintedValue(String typeHint, String chosenValue) { - + final Class clazz; - + switch(typeHint.toLowerCase()){ case "boolean": clazz = Boolean.class; @@ -179,7 +202,7 @@ private Object castTypeHintedValue(String typeHint, String chosenValue) { clazz = String.class; break; } - + return TypeUtil.toObjectType(chosenValue, clazz); } } 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 24c52e9e69..9dfb1d418f 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 @@ -23,6 +23,7 @@ import org.apache.commons.collections.iterators.TransformIterator; import org.apache.sling.api.SlingHttpServletRequest; import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.api.resource.ValueMap; import javax.annotation.Nonnull; @@ -37,10 +38,12 @@ public class NamespaceResourceWrapper extends FilteringResourceWrapper { private final SlingHttpServletRequest request; private final String[] namespacedProperties; + private final ValueMap valueMap; public NamespaceResourceWrapper(@Nonnull Resource resource, @Nonnull ExpressionResolver expressionResolver, - @Nonnull SlingHttpServletRequest request, String[] namespacedProperties) { + @Nonnull SlingHttpServletRequest request, + String[] namespacedProperties) { super(resource, expressionResolver, request); this.expressionResolver = expressionResolver; this.request = request; @@ -59,7 +62,7 @@ public Resource getChild(String relPath) { return null; } - NamespaceResourceWrapper wrapped =new NamespaceResourceWrapper(child, expressionResolver, request,namespacedProperties); + NamespaceResourceWrapper wrapped =new NamespaceResourceWrapper(child, expressionResolver, request,namespacedProperties); if(!isVisible(wrapped)){ return null; @@ -99,4 +102,4 @@ public ValueMap getValueMap() { } -} \ No newline at end of file +} 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 2c99a1adc9..44fdead4b2 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 @@ -33,7 +33,7 @@ @Component(service = NamespacedTransformedResourceProvider.class) @Designate(ocd = NamespacedTransformedResourceProviderImpl.Config.class) public class NamespacedTransformedResourceProviderImpl implements NamespacedTransformedResourceProvider { - + @ObjectClassDefinition( name = "ACS AEM Commons - NamespacedTransformedResourceProvider", description = "Transforms a resource underlying children with a namespace. Used for granite dialog snippets to be included in a granular way.") @@ -44,21 +44,25 @@ public class NamespacedTransformedResourceProviderImpl implements NamespacedTran ) String[] properties() default {"name", "fileNameParameter", "fileReferenceParameter"}; } - + @Reference private ExpressionResolver expressionResolver; - + private String[] namespacedProperties; - + @Activate @Modified public void init(Config config) { this.namespacedProperties = config.properties(); } - + @Override public Resource transformResourceWithNameSpacing(SlingHttpServletRequest request, Resource targetResource) { - return new NamespaceResourceWrapper(targetResource, expressionResolver, request, namespacedProperties); + return new NamespaceResourceWrapper( + targetResource, + expressionResolver, + request, + namespacedProperties); } - + } diff --git a/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImplTest.java b/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImplTest.java index d9255a1e92..57c73c9d37 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImplTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/granite/ui/components/impl/include/IncludeDecoratorFilterImplTest.java @@ -33,7 +33,9 @@ import javax.servlet.ServletException; import java.io.IOException; import java.io.InputStream; +import java.lang.annotation.Annotation; +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; @@ -59,7 +61,17 @@ public void setUp() throws Exception { context.currentResource("/apps/tab/items/column/items/include"); systemUnderTest = new IncludeDecoratorFilterImpl(); - + systemUnderTest.init(new IncludeDecoratorFilterImpl.Config(){ + @Override + public Class annotationType() { + return null; + } + + @Override + public String[] resourceTypesIgnoreChildren() { + return new String[]{"ignore/children/resource/type"}; + } + }); } @@ -80,6 +92,23 @@ public void test() throws IOException, ServletException { } + + @Test + public void test_disable_namespacing_children() throws IOException, ServletException { + Mockito.doAnswer(invocationOnMock -> { + + SlingHttpServletRequest captured = invocationOnMock.getArgument(0, SlingHttpServletRequest.class); + assertEquals("ignore/children/resource/type", captured.getAttribute(REQ_ATTR_IGNORE_CHILDREN_RESOURCE_TYPE)); + return null; + + }).when(filterChain).doFilter(any(SlingHttpServletRequest.class), any(SlingHttpServletResponse.class)); + + context.currentResource("/apps/tab/items/column/items/shouldIgnoreChildren"); + systemUnderTest.doFilter(context.request(), context.response(), filterChain); + + assertTrue("namespace is removed after the filter is performed", context.request().getAttribute(REQ_ATTR_NAMESPACE) == null); + } + @Test public void test_nested_include() throws IOException, ServletException { @@ -97,4 +126,23 @@ public void test_nested_include() throws IOException, ServletException { assertEquals("nested", context.request().getAttribute(REQ_ATTR_NAMESPACE)); } -} \ No newline at end of file + + + @Test + public void test_ignore_children() throws IOException, ServletException { + + Mockito.doAnswer(invocationOnMock -> { + + SlingHttpServletRequest captured = invocationOnMock.getArgument(0, SlingHttpServletRequest.class); + assertEquals("nested/block1", captured.getAttribute(REQ_ATTR_NAMESPACE)); + return null; + + }).when(filterChain).doFilter(any(SlingHttpServletRequest.class), any(SlingHttpServletResponse.class)); + + context.request().setAttribute(REQ_ATTR_NAMESPACE, "nested"); + systemUnderTest.doFilter(context.request(), context.response(), filterChain); + + assertEquals("nested", context.request().getAttribute(REQ_ATTR_NAMESPACE)); + + } +} 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 132a33daff..e4cbce3ae2 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 @@ -34,8 +34,8 @@ import java.util.Locale; import java.util.Map; -import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.REQ_ATTR_NAMESPACE; -import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.PREFIX; +import static com.adobe.acs.commons.granite.ui.components.impl.include.IncludeDecoratorFilterImpl.*; +import static com.adobe.acs.commons.granite.ui.components.impl.include.NamespaceDecoratedValueMapBuilder.REQ_ATTR_TEST_FLAG; import static org.junit.Assert.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; @@ -134,6 +134,23 @@ public void test_namespacing() { } + @Test + 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); + + Resource shouldIgnoreChildrenField = systemUnderTest.getChild("fieldWithChildrenThatShouldBeIgnored"); + + assertEquals("./block1/shouldIgnoreChildren", shouldIgnoreChildrenField.getValueMap().get("name", String.class)); + + Resource child = shouldIgnoreChildrenField.getChild("items/someRegularField"); + assertEquals("./displayButton",child.getValueMap().get("name", String.class)); + + } + private void setParameters(Map parameters){ for (Map.Entry entry : parameters.entrySet()) { diff --git a/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/filter-test.json b/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/filter-test.json index dd27c8b784..0206780dba 100644 --- a/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/filter-test.json +++ b/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/filter-test.json @@ -25,8 +25,12 @@ "jcr:primaryType": "nt:unstructured", "title": "${{firstItemTitle}}" } + }, + "shouldIgnoreChildren": { + "jcr:primaryType": "nt:unstructured", + "sling:resourceType": "ignore/children/resource/type" } } } } -} \ No newline at end of file +} diff --git a/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/namespace-wrapper-test.json b/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/namespace-wrapper-test.json index e15047b344..43502b7490 100644 --- a/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/namespace-wrapper-test.json +++ b/bundle/src/test/resources/com/adobe/acs/commons/granite/ui/components/impl/include/namespace-wrapper-test.json @@ -44,8 +44,22 @@ "fieldLabel": "${{(String)fieldLabelText:defaultText}}", "value": "true", "sling:resourceType": "granite/ui/components/foundation/form/textfield" + }, + "fieldWithChildrenThatShouldBeIgnored": { + "jcr:primaryType": "nt:unstructured", + "sling:resourceType": "ignore/children/resource/type", + "name": "./shouldIgnoreChildren", + "items": { + "someRegularField": { + "jcr:primaryType": "nt:unstructured", + "name": "./displayButton", + "fieldLabel": "${{(String)fieldLabelText:defaultText}}", + "value": "true", + "sling:resourceType": "granite/ui/components/foundation/form/textfield" + } + } } } } } -} \ No newline at end of file +}