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));
     }
 


Reply via email to