From d190a170b188e78fa6853f57df39db8bb2fa075c Mon Sep 17 00:00:00 2001 From: Brendan Robert Date: Mon, 4 Dec 2017 14:32:32 -0600 Subject: [PATCH] Feature/issue 1193 property merge improvements (#1196) * #1193 - Added support for DAM asset metadata form posts * Added All-Tags feature (with some test coverage the pattern detector) * Normalized parameter order across methods (source / destination) * Additional code simplification * Fixed bug in path handling for DAM -- added unit testing to verify path normalization works * Fixed basic path cases, likely to be edge cases in reality * Updating changelog for property merge post processor * Added test coverage to make coveralls happy (and assert merge-all-tags feature) * Incorporated review suggesions * TagManager checks added, but using mock tagmanager in tests * because Code Climate * Updated other classes affected by switch to Servlet 3.1 API * Revert "Updated other classes affected by switch to Servlet 3.1 API" This reverts commit 4fa337d747921b2d01a34ee30f9fee89b864a299. * removing extra spying from PropertyMergePostProcessorTest * missing license header --- CHANGELOG.md | 1 + .../wcm/impl/PropertyMergePostProcessor.java | 244 ++++++++++++------ .../impl/PropertyMergePostProcessorTest.java | 177 ++++++++++--- 3 files changed, 305 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52d703f06e..2aedfc1e50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) - #1174 - Introduced CodeClimate quality checks. Resulted in lots of miscellaneous non-API changes. - #1191 - Ensure that HttpCache works with response objects when `getOutputStream()` throws `IllegalStateException` +- #1193 - Improvements to Property Merge Post Processor, including asset metadata editing and merge-all-tags ### Fixed diff --git a/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessor.java b/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessor.java index 0737437273..ba0d7beb5e 100644 --- a/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessor.java +++ b/bundle/src/main/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessor.java @@ -1,5 +1,6 @@ package com.adobe.acs.commons.wcm.impl; +import com.day.cq.tagging.TagManager; import org.apache.commons.lang.StringUtils; import org.apache.felix.scr.annotations.Component; import org.apache.felix.scr.annotations.Service; @@ -21,9 +22,18 @@ import java.util.Collection; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; -import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.function.Function; +import java.util.regex.Pattern; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.jackrabbit.JcrConstants; +import org.apache.sling.api.resource.ResourceResolver; /** * ACS AEM Commons - Property Merge Sling POST Processor @@ -31,164 +41,232 @@ @Component @Service public class PropertyMergePostProcessor implements SlingPostProcessor { + private static final Logger log = LoggerFactory.getLogger(PropertyMergePostProcessor.class); private static final String AT_SUFFIX = "@PropertyMerge"; private static final String ALLOW_DUPLICATES_SUFFIX = AT_SUFFIX + ".AllowDuplicates"; private static final String TYPE_HINT_SUFFIX = AT_SUFFIX + ".TypeHint"; private static final String IGNORE_PREFIX = ":"; + protected static final String OPERATION_ALL_TAGS = "Operation.mergeAllTags"; + private static final String VALID_JCR_NAME = "[^:/\\[\\]\\|\\s*]+"; + private static final Pattern VALID_TAG = Pattern.compile("^" + VALID_JCR_NAME + ":(" + VALID_JCR_NAME + "/)*(" + VALID_JCR_NAME + ")?$"); @Override public final void process(final SlingHttpServletRequest request, - final List modifications) throws Exception { + final List modifications) throws Exception { - final List propertyMerges = this.getPropertyMerges(request.getRequestParameterMap()); + final List propertyMerges = this.getPropertyMerges(request); final Resource resource = request.getResource(); for (final PropertyMerge propertyMerge : propertyMerges) { - - if (this.merge(resource, + this.merge(resource, propertyMerge.getDestination(), propertyMerge.getSources(), propertyMerge.getTypeHint(), - propertyMerge.isAllowDuplicates())) { - modifications.add(Modification.onModified(resource.getPath())); - log.debug("Merged property values from {} into [ {} ]", - propertyMerge.getSources(), - propertyMerge.getDestination()); - } + propertyMerge.isAllowDuplicates()) + .ifPresent(modifiedResource -> { + modifications.add(Modification.onModified(modifiedResource.getPath())); + log.debug("Merged property values from {} into [ {} ]", + propertyMerge.getSources(), + propertyMerge.getDestination()); + }); } } /** - * Gets the corresponding list of PropertyMerge directives from the RequestParams. + * Gets the corresponding list of PropertyMerge directives from the + * RequestParams. * - * @param requestParameterMap the Request Param Map + * @param requestParameters the Request Param Map * @return a list of the PropertyMerge directives by Destination */ @SuppressWarnings("squid:S3776") - private List getPropertyMerges(final RequestParameterMap requestParameterMap) { - final HashMap> mapping = new HashMap>(); + private List getPropertyMerges(final SlingHttpServletRequest request) { + final RequestParameterMap requestParameters = request.getRequestParameterMap(); + final HashMap> mapping = new HashMap<>(); + boolean isBulkUpdate = Boolean.valueOf(getParamValue(requestParameters, "dam:bulkUpdate")); // Collect the Destination / Source mappings - - for (final RequestParameterMap.Entry entry : requestParameterMap.entrySet()) { - if (!StringUtils.endsWith(entry.getKey(), AT_SUFFIX)) { + requestParameters.forEach((key, values) -> { + if (!StringUtils.endsWith(key, AT_SUFFIX)) { // Not a @PropertyMerge request param - continue; + return; } - final String source = StringUtils.removeStart(StringUtils.substringBefore(entry.getKey(), AT_SUFFIX), IGNORE_PREFIX); - - for (final RequestParameter requestParameter : entry.getValue()) { - if (requestParameter != null) { - final String destination = StringUtils.removeStart(StringUtils.stripToNull(requestParameter - .getString()), IGNORE_PREFIX); - - if (destination != null) { - List sources = mapping.get(destination); - - if (sources == null) { - sources = new ArrayList(); + Function stripPrefix = (s -> StringUtils.removeStart(StringUtils.stripToNull(s), IGNORE_PREFIX)); + final String source = stripPrefix.apply(StringUtils.substringBefore(key, AT_SUFFIX)); + + Stream.of(values) + .map(RequestParameter::getString) + .map(stripPrefix) + .filter(Objects::nonNull) + .forEach(destination -> { + if (source.equalsIgnoreCase(OPERATION_ALL_TAGS)) { + // if this is a request for merging all tags, look at everyting that might be a tag + trackAllTagsMergeParameters(request, destination, mapping); + } else if (isBulkUpdate) { + // if this is a DAM bulk update, search all request params ending with this value + trackAssetMergeParameters(requestParameters, source, destination, mapping); + } else { + trackMergeParameters(mapping, source, destination); } - - sources.add(source); - mapping.put(StringUtils.strip(requestParameter.getString()), sources); - } - } - } - } + }); + }); // Convert the Mappings into PropertyMerge objects + return mapping.entrySet().stream().map( + entry -> new PropertyMerge( + entry.getKey(), + entry.getValue(), + areDuplicatesAllowed(requestParameters, entry.getKey()), + getFieldTypeHint(requestParameters, entry.getKey()) + )) + .collect(Collectors.toList()); + } - final List propertyMerges = new ArrayList(); + private void trackMergeParameters(final HashMap> mapping, final String source, String destination) { + mapping.merge(destination, new HashSet<>(), (a, b) -> a).add(source); + } - for (final Map.Entry> entry : mapping.entrySet()) { - final String destination = entry.getKey(); - final List sources = entry.getValue(); + private void trackAssetMergeParameters(final RequestParameterMap requestParameters, final String source, String destination, final HashMap> mapping) { + requestParameters.keySet().stream() + .map(String::valueOf) + .filter((paramName) -> (paramName.endsWith("/" + source))) + .forEach(adjustedSource -> { + String adjustedDest = alignDestinationPath(adjustedSource, destination); + trackMergeParameters(mapping, adjustedSource, adjustedDest); + }); + } - RequestParameter allowDuplicatesParam = requestParameterMap.getValue(IGNORE_PREFIX + destination - + ALLOW_DUPLICATES_SUFFIX); + private void trackAllTagsMergeParameters(SlingHttpServletRequest request, String destination, HashMap> mapping) { + request.getRequestParameterMap().forEach((source, value) -> { + if (hasTags(request.getResourceResolver(), value)) { + String newDestination = alignDestinationPath(source, destination); + trackMergeParameters(mapping, source, newDestination); + } + }); + } - final boolean allowDuplicates = allowDuplicatesParam != null && Boolean.valueOf(allowDuplicatesParam.getString()); + protected static boolean hasTags(ResourceResolver rr, RequestParameter[] params) { + if (params == null) { + return false; + } else { + TagManager tagManager = rr.adaptTo(TagManager.class); + return Stream.of(params).allMatch(param + -> looksLikeTag(param.getString()) + && tagManager.resolve(param.getString()) != null + ); + } + } - RequestParameter typeHintParam = requestParameterMap.getValue(IGNORE_PREFIX + destination - + TYPE_HINT_SUFFIX); + protected static boolean looksLikeTag(String value) { + return VALID_TAG.asPredicate().test(value); + } - final String typeHint = - typeHintParam != null ? typeHintParam.getString() : String.class.getSimpleName(); + protected static boolean areDuplicatesAllowed(RequestParameterMap params, String field) { + return Boolean.valueOf( + getParamValue(params, IGNORE_PREFIX + field + ALLOW_DUPLICATES_SUFFIX) + ); + } - propertyMerges.add(new PropertyMerge(destination, sources, allowDuplicates, typeHint)); - } + protected static String getFieldTypeHint(RequestParameterMap params, String field) { + return StringUtils.defaultString( + getParamValue(params, IGNORE_PREFIX + field + TYPE_HINT_SUFFIX), + String.class.getSimpleName() + ); + } - return propertyMerges; + protected static String alignDestinationPath(String source, String destination) { + if (source.contains(JcrConstants.JCR_CONTENT)) { + return StringUtils.substringBeforeLast(source, JcrConstants.JCR_CONTENT) + destination; + } else { + return destination; + } } + protected static String getParamValue(RequestParameterMap params, String paramName) { + RequestParameter param = params.getValue(paramName); + return param == null ? null : param.getString(); + } /** - * Merges the values found in the the source properties into the destination property as a multi-value. - * The values of the source properties and destination properties must all be the same property type. + * Merges the values found in the the source properties into the destination + * property as a multi-value. The values of the source properties and + * destination properties must all be the same property type. * * The unique set of properties will be stored in * - * @param resource the resource to look for the source and destination properties on + * @param resource the resource to look for the source and destination + * properties on * @param destination the property to store the collected properties. - * @param sources the properties to collect values from for merging - * @param typeHint the data type that should be used when reading and storing the data - * @param allowDuplicates true to allow duplicates values in the destination property; false to make values unique - * @return true if changes were made to the destination property + * @param sources the properties to collect values from for merging + * @param typeHint the data type that should be used when reading and + * storing the data + * @param allowDuplicates true to allow duplicates values in the destination + * property; false to make values unique + * @return Optional resource updated, if any */ - protected final boolean merge(final Resource resource, final String destination, - final List sources, final Class typeHint, - final boolean allowDuplicates) throws PersistenceException { + protected final Optional merge(final Resource resource, final String destination, + final Collection sources, final Class typeHint, + final boolean allowDuplicates) throws PersistenceException { + + ResourceResolver rr = resource.getResourceResolver(); // Create an empty array of type T @SuppressWarnings("unchecked") final T[] emptyArray = (T[]) Array.newInstance(typeHint, 0); - final ModifiableValueMap properties = resource.adaptTo(ModifiableValueMap.class); - - Collection collectedValues = null; + Collection collectedValues; if (allowDuplicates) { - collectedValues = new ArrayList(); + collectedValues = new ArrayList<>(); } else { - collectedValues = new LinkedHashSet(); + collectedValues = new LinkedHashSet<>(); } for (final String source : sources) { - // Get the source value as type T - final T[] tmp = properties.get(source, emptyArray); - - // If the value is not null, add to collectedValues - if (tmp != null) { - collectedValues.addAll(Arrays.asList(tmp)); + Resource sourceProperties = resource; + String sourceParam = source; + if (source.contains("/")) { + sourceParam = StringUtils.substringAfterLast(source, "/"); + sourceProperties = rr.getResource(resource, StringUtils.substringBeforeLast(source, "/")); } + T[] tmp = sourceProperties.adaptTo(ModifiableValueMap.class).get(sourceParam, emptyArray); + collectedValues.addAll(Arrays.asList(tmp)); + } + + Resource targetResource = resource; + String targetProperty = destination; + if (destination.contains("/")) { + targetProperty = StringUtils.substringAfterLast(destination, "/"); + targetResource = rr.getResource(resource, StringUtils.substringBeforeLast(destination, "/")); } + ModifiableValueMap targetProperties = targetResource.adaptTo(ModifiableValueMap.class); - final T[] currentValues = properties.get(destination, emptyArray); + final T[] currentValues = targetProperties.get(targetProperty, emptyArray); if (!collectedValues.equals(Arrays.asList(currentValues))) { - properties.put(destination, collectedValues.toArray(emptyArray)); + targetProperties.put(targetProperty, collectedValues.toArray(emptyArray)); - return true; + return Optional.of(targetResource); } else { - return false; + return Optional.empty(); } } - /** * Encapsulates a PropertyMerge configuration by Destination. */ private static class PropertyMerge { + private boolean allowDuplicates; private Class typeHint; private String destination; - private List sources; + private Collection sources; - public PropertyMerge(String destination, List sources, boolean allowDuplicates, String typeHint) { + public PropertyMerge(String destination, Collection sources, boolean allowDuplicates, String typeHint) { this.destination = destination; this.sources = sources; this.allowDuplicates = allowDuplicates; @@ -196,8 +274,8 @@ public PropertyMerge(String destination, List sources, boolean allowDupl } /** - * Converts the String type hint to the corresponding class. - * If not valid conversion can be found, default to String. + * Converts the String type hint to the corresponding class. If not + * valid conversion can be found, default to String. * * @param typeHintStr the String representation of the type hint * @return the Class of the type hint @@ -229,7 +307,7 @@ public String getDestination() { return destination; } - public List getSources() { + public Collection getSources() { return sources; } } diff --git a/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java b/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java index 09d8a25e38..6866fa7629 100644 --- a/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java +++ b/bundle/src/test/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessorTest.java @@ -1,11 +1,34 @@ +/* + * #%L + * ACS AEM Commons Bundle + * %% + * Copyright (C) 2017 Adobe + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ package com.adobe.acs.commons.wcm.impl; +import com.day.cq.tagging.Tag; +import com.day.cq.tagging.TagManager; +import java.util.ArrayList; import org.apache.sling.api.resource.ModifiableValueMap; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.wrappers.ModifiableValueMapDecorator; import org.junit.Assert; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -15,13 +38,24 @@ import java.util.Arrays; import java.util.Calendar; import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.apache.sling.servlets.post.Modification; +import org.apache.sling.testing.mock.sling.ResourceResolverType; +import org.apache.sling.testing.mock.sling.junit.SlingContext; +import org.apache.sling.testing.mock.sling.servlet.MockSlingHttpServletRequest; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.any; @RunWith(MockitoJUnitRunner.class) public class PropertyMergePostProcessorTest { + @Rule + public SlingContext context = new SlingContext(ResourceResolverType.JCR_MOCK); + @InjectMocks PropertyMergePostProcessor propertyMerge = new PropertyMergePostProcessor(); @@ -31,7 +65,7 @@ public class PropertyMergePostProcessorTest { @Mock ResourceResolver resourceResolver; - ModifiableValueMap properties = new ModifiableValueMapDecorator(new HashMap()); + ModifiableValueMap properties = new ModifiableValueMapDecorator(new HashMap<>()); @Before public void setUp() throws Exception { @@ -42,8 +76,8 @@ public void setUp() throws Exception { @Test public void testMerge_NoDuplicates_String() throws Exception { - properties.put("cats", new String[]{ "felix", "hobbes", "fluffy" }); - properties.put("dogs", new String[]{ "snoopy", "ira", "fluffy" }); + properties.put("cats", new String[]{"felix", "hobbes", "fluffy"}); + properties.put("dogs", new String[]{"snoopy", "ira", "fluffy"}); properties.put("fish", "nemo"); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); @@ -54,37 +88,35 @@ public void testMerge_NoDuplicates_String() throws Exception { String.class, false); - Assert.assertArrayEquals(new String[]{ "felix", "hobbes", "fluffy", "snoopy", "ira", "nemo" }, + Assert.assertArrayEquals(new String[]{"felix", "hobbes", "fluffy", "snoopy", "ira", "nemo"}, properties.get("animals", String[].class)); } @Test public void testMerge_NoDuplicates_Long() throws Exception { - properties.put("odd", new Long[]{ 1L, 3L }); - properties.put("even", new Long[]{ 2L, 4L }); - properties.put("duplicates", new Long[]{ 1L, 2L, 3L, 4L }); - + properties.put("odd", new Long[]{1L, 3L}); + properties.put("even", new Long[]{2L, 4L}); + properties.put("duplicates", new Long[]{1L, 2L, 3L, 4L}); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); propertyMerge.merge(resource, "longs", Arrays.asList("even", "odd", "duplicates"), - Long.class, + Long.class, false); - Assert.assertArrayEquals(new Long[]{ 2L, 4L, 1L, 3L }, + Assert.assertArrayEquals(new Long[]{2L, 4L, 1L, 3L}, properties.get("longs", Long[].class)); } @Test public void testMerge_NoDuplicates_Double() throws Exception { - properties.put("tenths", new Double[]{ 1.1D, 1.2D }); + properties.put("tenths", new Double[]{1.1D, 1.2D}); properties.put("hundredths", 3.01D); - properties.put("duplicates", new Double[]{ 1.1D }); - + properties.put("duplicates", new Double[]{1.1D}); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); @@ -94,43 +126,42 @@ public void testMerge_NoDuplicates_Double() throws Exception { Double.class, false); - Assert.assertArrayEquals(new Double[]{ 1.1D, 1.2D, 3.01D }, + Assert.assertArrayEquals(new Double[]{1.1D, 1.2D, 3.01D}, properties.get("doubles", Double[].class)); } @Test public void testMerge_NoDuplicates_Boolean() throws Exception { - properties.put("first", new Boolean[]{ true, false, true }); + properties.put("first", new Boolean[]{true, false, true}); properties.put("second", true); - when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); propertyMerge.merge(resource, "booleans", Arrays.asList("first", "second"), - Boolean.class, + Boolean.class, false); - Assert.assertArrayEquals(new Boolean[]{ true, false }, + Assert.assertArrayEquals(new Boolean[]{true, false}, properties.get("booleans", Boolean[].class)); } @Test public void testMerge_NoDuplicates_Calendar() throws Exception { - Calendar january = Calendar.getInstance(); + Calendar january = Calendar.getInstance(); january.set(2015, Calendar.JANUARY, 1); - Calendar july = Calendar.getInstance(); + Calendar july = Calendar.getInstance(); july.set(2015, Calendar.JULY, 4); - Calendar september = Calendar.getInstance(); + Calendar september = Calendar.getInstance(); september.set(2015, Calendar.SEPTEMBER, 16); - properties.put("cold", new Calendar[] { january, september }); - properties.put("hot", new Calendar[]{ july, september }); + properties.put("cold", new Calendar[]{january, september}); + properties.put("hot", new Calendar[]{july, september}); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); @@ -140,15 +171,15 @@ public void testMerge_NoDuplicates_Calendar() throws Exception { Calendar.class, false); - Assert.assertArrayEquals(new Calendar[]{ january, september, july }, + Assert.assertArrayEquals(new Calendar[]{january, september, july}, properties.get("dates", Calendar[].class)); } @Test public void testMerge_Duplicates_String() throws Exception { - properties.put("cats", new String[]{ "felix", "hobbes", "fluffy" }); - properties.put("dogs", new String[]{ "snoopy", "ira", "fluffy" }); + properties.put("cats", new String[]{"felix", "hobbes", "fluffy"}); + properties.put("dogs", new String[]{"snoopy", "ira", "fluffy"}); properties.put("fish", "nemo"); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); @@ -159,24 +190,24 @@ public void testMerge_Duplicates_String() throws Exception { String.class, true); - Assert.assertArrayEquals(new String[]{ "felix", "hobbes", "fluffy", "snoopy", "ira", "fluffy", "nemo" }, + Assert.assertArrayEquals(new String[]{"felix", "hobbes", "fluffy", "snoopy", "ira", "fluffy", "nemo"}, properties.get("animals", String[].class)); } @Test public void testMerge_Duplicates_Calendar() throws Exception { - Calendar january = Calendar.getInstance(); + Calendar january = Calendar.getInstance(); january.set(2015, Calendar.JANUARY, 1); - Calendar july = Calendar.getInstance(); + Calendar july = Calendar.getInstance(); july.set(2015, Calendar.JULY, 4); - Calendar september = Calendar.getInstance(); + Calendar september = Calendar.getInstance(); september.set(2015, Calendar.SEPTEMBER, 16); - properties.put("cold", new Calendar[] { january, september }); - properties.put("hot", new Calendar[]{ july, september }); + properties.put("cold", new Calendar[]{january, september}); + properties.put("hot", new Calendar[]{july, september}); when(resource.adaptTo(ModifiableValueMap.class)).thenReturn(properties); @@ -186,9 +217,87 @@ public void testMerge_Duplicates_Calendar() throws Exception { Calendar.class, true); - Assert.assertArrayEquals(new Calendar[]{ january, september, july, september }, + Assert.assertArrayEquals(new Calendar[]{january, september, july, september}, properties.get("dates", Calendar[].class)); } -} + @Test + public void testTagDetection() { + Assert.assertTrue("Valid root tag detection", PropertyMergePostProcessor.looksLikeTag("Some_Root:")); + Assert.assertTrue("Valid root tag detection", PropertyMergePostProcessor.looksLikeTag("Some_Root:Tag")); + Assert.assertTrue("Valid sub tag detection", PropertyMergePostProcessor.looksLikeTag("Some_Root-123:Tag/Another_Tag456")); + Assert.assertFalse("Invalid tag pattern", PropertyMergePostProcessor.looksLikeTag("Some_Root123")); + Assert.assertFalse("Spaces check 1", PropertyMergePostProcessor.looksLikeTag("Some Root:Tag")); + Assert.assertFalse("Spaces check 2", PropertyMergePostProcessor.looksLikeTag("Some_Root : Tag")); + Assert.assertFalse("Spaces check 3", PropertyMergePostProcessor.looksLikeTag("Some_Root:Tag Name")); + Assert.assertFalse("Spaces check 4", PropertyMergePostProcessor.looksLikeTag("Some_Root:Tag/Other Tag Name")); + } + + @Test + public void testPropertyPathAlignment() { + // Test based on what is actually posted when bulk editing assets + Assert.assertEquals("./asset-share-commons/en/public/pictures/stacey-rozells-288200.jpg/jcr:content/metadata/dam:tags-merged", + PropertyMergePostProcessor.alignDestinationPath( + "./asset-share-commons/en/public/pictures/stacey-rozells-288200.jpg/jcr:content/metadata/dam:tag1", + "jcr:content/metadata/dam:tags-merged")); + + // In the event someone were posting to the asset for updating its metadata, this should still work + Assert.assertEquals("jcr:content/metadata/dam:tags-merged", + PropertyMergePostProcessor.alignDestinationPath( + "jcr:content/metadata/dam:tag1", + "jcr:content/metadata/dam:tags-merged")); + + // Should not break basic cases + Assert.assertEquals("PathB", + PropertyMergePostProcessor.alignDestinationPath( + "PathA", + "PathB")); + } + @Test + public void testMergeAllTags() throws Exception { + + final TagManager mockTagManager = mock(TagManager.class); + Tag fakeTag = mock(Tag.class); + when(mockTagManager.resolve(any())).thenReturn(fakeTag); + + context.registerAdapter(ResourceResolver.class, TagManager.class, mockTagManager); + + ResourceResolver rr = context.resourceResolver(); + MockSlingHttpServletRequest request = context.request(); + request.setParameterMap(new HashMap() { + { + put("./asset/jcr:content/metadata/dam:tag1", new String[]{ + "tag1:tag1a", + "tag1:tag1b" + }); + put("./asset/jcr:content/metadata/dam:tag2", new String[]{ + "tag2:tag2a", + "tag2:tag2b" + }); + put(":" + PropertyMergePostProcessor.OPERATION_ALL_TAGS + "@PropertyMerge", "jcr:content/metadata/dam:combined-tags"); + } + }); + + Map emptyProperties = new HashMap<>(); + Resource content = rr.create(rr.resolve("/"), "content", emptyProperties); + Resource dam = rr.create(content, "dam", emptyProperties); + request.setResource(dam); + Resource asset = rr.create(dam, "asset", emptyProperties); + Resource jcrContent = rr.create(asset, "jcr:content", emptyProperties); + Resource metadata = rr.create(jcrContent, "metadata", new HashMap() { + { + put("dam:tag1", new String[]{"tag1:tag1a", "tag1:tag1b"}); + put("dam:tag2", new String[]{"tag2:tag2a", "tag2:tag2b"}); + } + }); + + PropertyMergePostProcessor processor = new PropertyMergePostProcessor(); + List changeLog = new ArrayList<>(); + processor.process(request, changeLog); + Assert.assertFalse("Should have observed some changes", changeLog.isEmpty()); + String[] tags = metadata.getValueMap().get("dam:combined-tags", String[].class); + Assert.assertArrayEquals(new String[]{"tag1:tag1a", "tag1:tag1b", "tag2:tag2a", "tag2:tag2b"}, tags); + + } +}