Author: cziegeler Date: Wed Oct 12 08:18:06 2016 New Revision: 1764401 URL: http://svn.apache.org/viewvc?rev=1764401&view=rev Log: SLING-6131 : MapEntries: Invalid logic around added/changed/removed property names
Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java Modified: sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java?rev=1764401&r1=1764400&r2=1764401&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java (original) +++ sling/trunk/bundles/resourceresolver/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java Wed Oct 12 08:18:06 2016 @@ -26,6 +26,7 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Dictionary; @@ -68,7 +69,6 @@ import org.osgi.framework.Constants; import org.osgi.framework.ServiceRegistration; import org.osgi.service.event.Event; import org.osgi.service.event.EventAdmin; -import org.osgi.service.event.EventConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -199,6 +199,7 @@ public class MapEntries implements Resou final Dictionary<String, Object> props = new Hashtable<String, Object>(); props.put(ResourceChangeListener.PATHS, factory.getObservationPaths()); + log.info("Registering for {}", Arrays.toString(factory.getObservationPaths())); props.put(Constants.SERVICE_DESCRIPTION, "Apache Sling Map Entries Observation"); props.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation"); this.registration = bundleContext.registerService(ResourceChangeListener.class, this, props); @@ -947,6 +948,11 @@ public class MapEntries implements Resou return entryMap; } + /** + * Check if the resoruce is a valid vanity path resource + * @param resource The resource to check + * @return {@code true} if this is valid, {@code false} otherwise + */ private boolean isValidVanityPath(Resource resource){ // ignore system tree if (resource.getPath().startsWith(JCR_SYSTEM_PREFIX)) { @@ -968,8 +974,7 @@ public class MapEntries implements Resou return false; } } - // require properties - return resource.getValueMap() != null; + return true; } private String getActualContentPath(String path){ @@ -1122,9 +1127,6 @@ public class MapEntries implements Resou return false; } - // require properties - final ValueMap props = resource.getValueMap(); - final String resourceName; final String parentPath; if (resource.getName().equals("jcr:content")) { @@ -1154,37 +1156,43 @@ public class MapEntries implements Resou } boolean hasAlias = false; if ( parentPath != null ) { - Map<String, String> parentMap = map.get(parentPath); - for (final String alias : props.get(ResourceResolverImpl.PROP_ALIAS, String[].class)) { - if (parentMap != null && parentMap.containsKey(alias)) { - log.warn("Encountered duplicate alias {} under parent path {}. Refusing to replace current target {} with {}.", new Object[] { - alias, - parentPath, - parentMap.get(alias), - resourceName - }); - } else { - // check alias - boolean invalid = alias.equals("..") || alias.equals("."); - if ( !invalid ) { - for(final char c : alias.toCharArray()) { - // invalid if / or # or a ? - if ( c == '/' || c == '#' || c == '?' ) { - invalid = true; - break; + // require properties + final ValueMap props = resource.getValueMap(); + final String[] aliasArray = props.get(ResourceResolverImpl.PROP_ALIAS, String[].class); + + if ( aliasArray != null ) { + Map<String, String> parentMap = map.get(parentPath); + for (final String alias : aliasArray) { + if (parentMap != null && parentMap.containsKey(alias)) { + log.warn("Encountered duplicate alias {} under parent path {}. Refusing to replace current target {} with {}.", new Object[] { + alias, + parentPath, + parentMap.get(alias), + resourceName + }); + } else { + // check alias + boolean invalid = alias.equals("..") || alias.equals("."); + if ( !invalid ) { + for(final char c : alias.toCharArray()) { + // invalid if / or # or a ? + if ( c == '/' || c == '#' || c == '?' ) { + invalid = true; + break; + } } } - } - if ( invalid ) { - log.warn("Encountered invalid alias {} under parent path {}. Refusing to use it.", - alias, parentPath); - } else { - if (parentMap == null) { - parentMap = new LinkedHashMap<String, String>(); - map.put(parentPath, parentMap); + if ( invalid ) { + log.warn("Encountered invalid alias {} under parent path {}. Refusing to use it.", + alias, parentPath); + } else { + if (parentMap == null) { + parentMap = new LinkedHashMap<String, String>(); + map.put(parentPath, parentMap); + } + parentMap.put(alias, resourceName); + hasAlias = true; } - parentMap.put(alias, resourceName); - hasAlias = true; } } } @@ -1460,38 +1468,6 @@ public class MapEntries implements Resou } } - /** - * Returns a filter which matches if any of the nodeProps (JCR properties - * modified) is listed in any of the eventProps (event properties listing - * modified JCR properties) this allows to only get events interesting for - * updating the internal structure - */ - private static String createFilter(final boolean vanityPathEnabled) { - final String[] nodeProps = { - PROP_REDIRECT_EXTERNAL_REDIRECT_STATUS, PROP_REDIRECT_EXTERNAL, - ResourceResolverImpl.PROP_REDIRECT_INTERNAL, PROP_REDIRECT_EXTERNAL_STATUS, - PROP_REG_EXP, ResourceResolverImpl.PROP_ALIAS }; - final String[] eventProps = { SlingConstants.PROPERTY_ADDED_ATTRIBUTES, SlingConstants.PROPERTY_CHANGED_ATTRIBUTES, SlingConstants.PROPERTY_REMOVED_ATTRIBUTES }; - final StringBuilder filter = new StringBuilder(); - filter.append("(|"); - for (final String eventProp : eventProps) { - filter.append("(|"); - if ( vanityPathEnabled ) { - filter.append('(').append(eventProp).append('=').append(PROP_VANITY_PATH).append(')'); - filter.append('(').append(eventProp).append('=').append(PROP_VANITY_ORDER).append(')'); - } - for (final String nodeProp : nodeProps) { - filter.append('(').append(eventProp).append('=').append(nodeProp).append(')'); - } - filter.append(")"); - } - filter.append("(").append(EventConstants.EVENT_TOPIC).append("=").append(SlingConstants.TOPIC_RESOURCE_REMOVED).append(")"); - filter.append("(").append(EventConstants.EVENT_TOPIC).append("=").append(SlingConstants.TOPIC_RESOURCE_ADDED).append(")"); - filter.append(")"); - - return filter.toString(); - } - private final class MapEntryIterator implements Iterator<MapEntry> { private final Map<String, List<MapEntry>> resolveMapsMap; Modified: sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java URL: http://svn.apache.org/viewvc/sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java?rev=1764401&r1=1764400&r2=1764401&view=diff ============================================================================== --- sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java (original) +++ sling/trunk/bundles/resourceresolver/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java Wed Oct 12 08:18:06 2016 @@ -1479,9 +1479,8 @@ public class MapEntriesTest { assertFalse((Boolean)method.invoke(mapEntries, resource)); when(resource.getPath()).thenReturn("/justVanityPath"); - assertFalse((Boolean)method.invoke(mapEntries, resource)); - when(resource.getValueMap()).thenReturn(mock(ValueMap.class)); + assertTrue((Boolean)method.invoke(mapEntries, resource)); }