reschke commented on code in PR #131:
URL: 
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/131#discussion_r1923538584


##########
src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntryIterator.java:
##########
@@ -71,69 +78,89 @@ public MapEntry next() {
         return result;
     }
 
-    /**
-     * @see java.util.Iterator#remove()
-     */
     @Override
     public void remove() {
         throw new UnsupportedOperationException();
     }
 
     private void seek() {
+
+        // compute candidate for next entry from global list
+
         if (this.nextGlobal == null && this.globalListIterator.hasNext()) {
             this.nextGlobal = this.globalListIterator.next();
         }
+
+        // compute candidate for next entry from "special" vanity path list
+
         if (this.nextSpecial == null) {
-            if (specialIterator != null && !specialIterator.hasNext()) {
-                specialIterator = null;
+            // reset specialIterator when exhausted
+            if (!this.specialIterator.hasNext()) {
+                this.specialIterator = Collections.emptyIterator();
             }
-            while (specialIterator == null && key != null) {
-                // remove selectors and extension
-                final int lastSlashPos = key.lastIndexOf('/');
-                final int lastDotPos = key.indexOf('.', lastSlashPos);
-                if (lastDotPos != -1) {
-                    key = key.substring(0, lastDotPos);
-                }
-
-                final List<MapEntry> special = 
this.getCurrentMapEntryForVanityPath.apply(this.key);
-                if (special != null) {
-                    specialIterator = special.iterator();
-                }
-
-                // recurse to the parent
-                if (key.length() > 1) {
-                    final int lastSlash = key.lastIndexOf("/");
-                    if (lastSlash == 0) {
-                        key = null;
-                    } else {
-                        key = key.substring(0, lastSlash);
-                    }
-                } else {
-                    key = null;
-                }
+
+            // given the vanity path in key, walk up the hierarchy until we 
find
+            // map entries for that path (or stop when root is reached)
+            while (!this.specialIterator.hasNext() && this.key != null) {
+                this.key = removeSelectorsAndExtension(this.key);
+                this.specialIterator = 
this.getCurrentMapEntryIteratorForVanityPath.apply(this.key);
+                this.key = getParent(key);
             }
-            if (this.specialIterator != null && 
this.specialIterator.hasNext()) {
+
+            if (this.specialIterator.hasNext()) {
                 this.nextSpecial = this.specialIterator.next();
             }
         }
-        if (this.nextSpecial == null) {
+
+        // choose based on presence, vanity path preferences and pattern 
lengths
+
+        if (useNextGlobal()) {
             this.next = this.nextGlobal;
             this.nextGlobal = null;
-        } else if (!this.vanityPathPrecedence){
-            if (this.nextGlobal == null) {
-                this.next = this.nextSpecial;
-                this.nextSpecial = null;
-            } else if (this.nextGlobal.getPattern().length() >= 
this.nextSpecial.getPattern().length()) {
-                this.next = this.nextGlobal;
-                this.nextGlobal = null;
-
-            }else {
-                this.next = this.nextSpecial;
-                this.nextSpecial = null;
-            }
         } else {
             this.next = this.nextSpecial;
             this.nextSpecial = null;
         }
     }
+
+    private boolean useNextGlobal() {

Review Comment:
   it is; the original code was hard to read and had code duplication.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to