Skip to content

Commit

Permalink
Feature/issue 1193 property merge improvements (#1196)
Browse files Browse the repository at this point in the history 
* #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 4fa337d.

* removing extra spying from PropertyMergePostProcessorTest

* missing license header
  • Loading branch information
@badvision @justinedelson
badvision authored and justinedelson committed Dec 4, 2017
1 parent cdba304 commit d190a17
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 117 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
244 changes: 161 additions & 83 deletions bundle/src/main/java/com/adobe/acs/commons/wcm/impl/PropertyMergePostProcessor.java
View file
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -21,183 +22,260 @@
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
*/
@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<Modification> modifications) throws Exception {
final List<Modification> modifications) throws Exception {

final List<PropertyMerge> propertyMerges = this.getPropertyMerges(request.getRequestParameterMap());
final List<PropertyMerge> 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<PropertyMerge> getPropertyMerges(final RequestParameterMap requestParameterMap) {
final HashMap<String, List<String>> mapping = new HashMap<String, List<String>>();
private List<PropertyMerge> getPropertyMerges(final SlingHttpServletRequest request) {
final RequestParameterMap requestParameters = request.getRequestParameterMap();
final HashMap<String, Set<String>> mapping = new HashMap<>();
boolean isBulkUpdate = Boolean.valueOf(getParamValue(requestParameters, "dam:bulkUpdate"));

// Collect the Destination / Source mappings

for (final RequestParameterMap.Entry<String, RequestParameter[]> 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<String> sources = mapping.get(destination);

if (sources == null) {
sources = new ArrayList<String>();
Function<String, String> 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<PropertyMerge> propertyMerges = new ArrayList<PropertyMerge>();
private void trackMergeParameters(final HashMap<String, Set<String>> mapping, final String source, String destination) {
mapping.merge(destination, new HashSet<>(), (a, b) -> a).add(source);
}

for (final Map.Entry<String, List<String>> entry : mapping.entrySet()) {
final String destination = entry.getKey();
final List<String> sources = entry.getValue();
private void trackAssetMergeParameters(final RequestParameterMap requestParameters, final String source, String destination, final HashMap<String, Set<String>> 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<String, Set<String>> 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 <T> boolean merge(final Resource resource, final String destination,
final List<String> sources, final Class<T> typeHint,
final boolean allowDuplicates) throws PersistenceException {
protected final <T> Optional<Resource> merge(final Resource resource, final String destination,
final Collection<String> sources, final Class<T> 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<T> collectedValues = null;
Collection<T> collectedValues;

if (allowDuplicates) {
collectedValues = new ArrayList<T>();
collectedValues = new ArrayList<>();
} else {
collectedValues = new LinkedHashSet<T>();
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<String> sources;
private Collection<String> sources;

public PropertyMerge(String destination, List<String> sources, boolean allowDuplicates, String typeHint) {
public PropertyMerge(String destination, Collection<String> sources, boolean allowDuplicates, String typeHint) {
this.destination = destination;
this.sources = sources;
this.allowDuplicates = allowDuplicates;
this.typeHint = this.convertTypeHint(typeHint);
}

/**
* 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
Expand Down Expand Up @@ -229,7 +307,7 @@ public String getDestination() {
return destination;
}

public List<String> getSources() {
public Collection<String> getSources() {
return sources;
}
}
Expand Down
Loading

0 comments on commit d190a17

Please sign in to comment.