You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2019/02/27 07:56:32 UTC

[sling-org-apache-sling-resourcemerger] branch master updated: SLING-8296 : MergingResourceProvider purges the last item if existing item is overlaid and has sling:orderBefore property set

This is an automated email from the ASF dual-hosted git repository.

cziegeler pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourcemerger.git


The following commit(s) were added to refs/heads/master by this push:
     new 056c3a0  SLING-8296 : MergingResourceProvider purges the last item if existing item is overlaid and has sling:orderBefore property set
056c3a0 is described below

commit 056c3a0507ebb250952ca82adf40008dd308b132
Author: Carsten Ziegeler <cz...@apache.org>
AuthorDate: Wed Feb 27 08:56:22 2019 +0100

    SLING-8296 : MergingResourceProvider purges the last item if existing item is overlaid and has sling:orderBefore property set
---
 .../impl/MergingResourceProvider.java              | 23 +++++++++++++-------
 ...sourceProviderTestForSearchPathBasedPicker.java | 25 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java b/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
index d34d219..d281970 100644
--- a/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
+++ b/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
@@ -81,7 +81,7 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
         private List<ExcludeEntry> entries = new ArrayList<ExcludeEntry>();
 
         /**
-         * 
+         *
          * @param parent the underlying resource
          * @param traverseParent if true will also continue with the parent's parent recursively
          */
@@ -135,7 +135,7 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
         }
 
         /**
-         * 
+         *
          * @param name the name of the resource to check
          * @param isLocalResource {@code true} if the check is on a local resource, {@code false} if the check is on an underlying/inherited resource
          * @return {@code true} if the local/inherited resource should be hidden, otherwise {@code false}
@@ -301,7 +301,7 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
             final List<ResourceHolder> candidates = new ArrayList<ResourceHolder>();
 
             final Iterator<Resource> resources = picker.pickResources(resolver, relativePath, parent).iterator();
-            
+
             // start with the base resource
             boolean isUnderlying = true;
             while (resources.hasNext()) {
@@ -319,16 +319,16 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
                         }
                     }
                 }
-                
+
                 int previousChildPositionInCandidateList = -1;
-                
+
                 // get children of current resource (might be overlaid resource)
                 for (final Resource child : parentResource.getChildren()) {
                     final String rsrcName = child.getName();
                     // the holder which should end up in the children list
                     ResourceHolder holder = null;
                     int childPositionInCandidateList = -1;
-                    
+
                     // check if this an overlaid resource (i.e. has the resource with the same name already be exposed through the underlying resource)
                     for (int index=0; index < candidates.size(); index++) {
                         ResourceHolder current = candidates.get(index);
@@ -377,7 +377,14 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
                     // either reorder because of explicit reording property
                     if (orderBeforeIndex > -1) {
                         candidates.add(orderBeforeIndex, holder);
-                        candidates.remove(candidates.size() - 1);
+                        if (childPositionInCandidateList == -1) {
+                            candidates.remove(candidates.size() - 1);
+                        } else {
+                            if (childPositionInCandidateList > orderBeforeIndex) {
+                                childPositionInCandidateList++;
+                            }
+                            candidates.remove(childPositionInCandidateList);
+                        }
                     } else {
                         // or reorder because overlaid resource has a different order
                         if (childPositionInCandidateList != -1 && previousChildPositionInCandidateList != -1) {
@@ -394,7 +401,7 @@ public class MergingResourceProvider extends ResourceProvider<Void> {
                         }
                     }
                 }
-                
+
             }
             final List<Resource> children = new ArrayList<Resource>();
             for (final ResourceHolder holder : candidates) {
diff --git a/src/test/java/org/apache/sling/resourcemerger/impl/MergedResourceProviderTestForSearchPathBasedPicker.java b/src/test/java/org/apache/sling/resourcemerger/impl/MergedResourceProviderTestForSearchPathBasedPicker.java
index 1bc3718..e74d531 100644
--- a/src/test/java/org/apache/sling/resourcemerger/impl/MergedResourceProviderTestForSearchPathBasedPicker.java
+++ b/src/test/java/org/apache/sling/resourcemerger/impl/MergedResourceProviderTestForSearchPathBasedPicker.java
@@ -28,6 +28,7 @@ import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.commons.collections4.iterators.IteratorIterable;
 import org.apache.sling.api.resource.ModifiableValueMap;
 import org.apache.sling.api.resource.PersistenceException;
 import org.apache.sling.api.resource.Resource;
@@ -35,12 +36,15 @@ import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceResolverFactory;
 import org.apache.sling.api.resource.ResourceUtil;
 import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.hamcrest.ResourceMatchers;
 import org.apache.sling.resourcemerger.impl.picker.SearchPathBasedResourcePicker;
 import org.apache.sling.spi.resource.provider.ResolveContext;
 import org.apache.sling.spi.resource.provider.ResourceContext;
 import org.apache.sling.testing.resourceresolver.MockHelper;
 import org.apache.sling.testing.resourceresolver.MockResourceResolverFactory;
 import org.apache.sling.testing.resourceresolver.MockResourceResolverFactoryOptions;
+import org.hamcrest.Matchers;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -342,4 +346,25 @@ public class MergedResourceProviderTestForSearchPathBasedPicker {
         }
     }
 
+    @Test
+    public void testOrderBefore() throws PersistenceException {
+        // create new child nodes below base and overlay
+        MockHelper.create(this.resolver).resource("/libs/orderbeforetest").resource("/apps/orderbeforetest")
+                .resource("/libs/orderbeforetest/create").resource("/libs/orderbeforetest/edit")
+                .resource("/libs/orderbeforetest/update").resource("/libs/orderbeforetest/delete")
+                .resource("/libs/orderbeforetest/rollout").resource("/apps/orderbeforetest/move")
+                .resource("/apps/orderbeforetest/create").p(MergedResourceConstants.PN_ORDER_BEFORE, "edit").commit();
+
+        Resource mergedResource = this.provider.getResource(ctx, "/merged/orderbeforetest",
+                ResourceContext.EMPTY_CONTEXT, null);
+        // convert the iterator returned by list children into an iterable (to be able
+        // to perform some tests)
+        IteratorIterable<Resource> iterable = new IteratorIterable<Resource>(provider.listChildren(ctx, mergedResource),
+                true);
+
+        Assert.assertThat(iterable,
+                Matchers.contains(ResourceMatchers.name("create"), ResourceMatchers.name("edit"),
+                        ResourceMatchers.name("update"), ResourceMatchers.name("delete"),
+                        ResourceMatchers.name("rollout"), ResourceMatchers.name("move")));
+    }
 }