svn commit: r949834 - in /sling/trunk/bundles/servlets/resolver/src: main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java

[cziegeler]

Author: cziegeler
Date: Mon May 31 19:06:05 2010
New Revision: 949834

URL: http://svn.apache.org/viewvc?rev=949834&view=rev
Log:
SLING-1529 : Endless iteration in LocationIterator with circular resource super 
types

Modified:
    
sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java
    
sling/trunk/bundles/servlets/resolver/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java

Modified: 
sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java?rev=949834&r1=949833&r2=949834&view=diff
==============================================================================
--- 
sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java
 (original)
+++ 
sling/trunk/bundles/servlets/resolver/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationIterator.java
 Mon May 31 19:06:05 2010
@@ -18,11 +18,14 @@
  */
 package org.apache.sling.servlets.resolver.internal.helper;
 
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.NoSuchElementException;
+import java.util.Set;
 
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceUtil;
+import org.slf4j.LoggerFactory;
 
 /**
  * The <code>LocationIterator</code> provides access to an ordered collection
@@ -80,6 +83,9 @@ public class LocationIterator implements
     // if there is no more location to return
     private String nextLocation;
 
+    /** Set of used resource types to detect a circular resource type 
hierarchy. */
+    private final Set<String> usedResourceTypes = new HashSet<String>();
+
     /**
      * Creates an instance of this iterator starting with a location built from
      * the resource type of the <code>resource</code> and ending with the
@@ -105,6 +111,8 @@ public class LocationIterator implements
         // we start with the first resource type
         this.resourceType = firstResourceType;
 
+        this.usedResourceTypes.add(this.resourceType);
+
         nextLocation = seek();
     }
 
@@ -196,6 +204,15 @@ public class LocationIterator implements
                 resourceType);
         }
 
+        // detect circular dependency
+        if ( superType != null ) {
+            if ( this.usedResourceTypes.contains(superType) ) {
+                LoggerFactory.getLogger(this.getClass()).error("Circular 
dependency in resource type hierarchy detected! Check super types of {}", 
superType);
+                superType = null;
+            } else {
+                this.usedResourceTypes.add(superType);
+            }
+        }
         // use default resource type if there is no super type any more
         if (superType == null) {
             superType = baseResourceType;

Modified: 
sling/trunk/bundles/servlets/resolver/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java
URL: 
http://svn.apache.org/viewvc/sling/trunk/bundles/servlets/resolver/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java?rev=949834&r1=949833&r2=949834&view=diff
==============================================================================
--- 
sling/trunk/bundles/servlets/resolver/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java
 (original)
+++ 
sling/trunk/bundles/servlets/resolver/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationIteratorTest.java
 Mon May 31 19:06:05 2010
@@ -22,6 +22,7 @@ import static org.apache.sling.servlets.
 
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceUtil;
+import org.apache.sling.commons.testing.sling.MockResource;
 
 public class LocationIteratorTest extends HelperTestBase {
 
@@ -465,4 +466,47 @@ public class LocationIteratorTest extend
         assertEquals(root1 + "/" + DEFAULT_SERVLET_NAME, li.next());
         assertFalse(li.hasNext());
     }
+
+    public void testCircularResourceTypeHierarchy() {
+        final String root1 = "/libs";
+        resourceResolver.setSearchPath(root1);
+
+        // resource type and super type for start resource
+        final String resourceType = "foo/bar";
+        final String resourceSuperType = "foo/check1";
+        final String resourceSuperType2 = "foo/check2";
+
+        final Resource resource2 = new MockResource(resourceResolver,
+                root1 + '/' + resourceSuperType,
+                resourceType, resourceSuperType2);
+        resourceResolver.addResource(resource2);
+        final Resource resource3 = new MockResource(resourceResolver,
+                root1 + '/' + resourceSuperType2,
+                resourceType, resourceType);
+        resourceResolver.addResource(resource3);
+
+        LocationIterator li = new LocationIterator(resourceType,
+                resourceSuperType,
+                DEFAULT_SERVLET_NAME,
+                resourceResolver);
+
+        // 1. /libs/foo/bar
+        assertTrue(li.hasNext());
+        assertEquals(root1 + '/' + resourceType, li.next());
+
+        // 1. /libs/foo/check1
+        assertTrue(li.hasNext());
+        assertEquals(root1 + "/" + resourceSuperType, li.next());
+
+        // 3. /libs/foo/check2
+        assertTrue(li.hasNext());
+        assertEquals(root1 + "/" + resourceSuperType2, li.next());
+
+        // 4. /libs/sling/servlet/default
+        assertTrue(li.hasNext());
+        assertEquals(root1 + "/" + DEFAULT_SERVLET_NAME, li.next());
+
+        // 5. finished
+        assertFalse(li.hasNext());
+    }
 }