This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to annotated tag org.apache.sling.resourcemerger-1.3.4 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourcemerger.git
commit 3d0cf29c37551ed6c02c81cbe3a5110a8e2e1ec2 Author: Konrad Windszus <[email protected]> AuthorDate: Fri Jun 16 13:42:53 2017 +0000 SLING-6956 do not touch order when no new resources have been defined in an overlying resource git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/contrib/extensions/resourcemerger@1798927 13f79535-47bb-0310-9956-ffa450edef68 --- .../impl/MergingResourceProvider.java | 40 ++++++-- .../impl/CommonMergedResourceProviderTest.java | 102 ++++++++++++++++++++- 2 files changed, 131 insertions(+), 11 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 7e55c4f..8cf6d2b 100644 --- a/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java +++ b/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java @@ -297,10 +297,12 @@ public class MergingResourceProvider extends ResourceProvider<Void> { final String relativePath = getRelativePath(parent.getPath()); if (relativePath != null) { + // candidates is the list of holders from which the children are being constructed! 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()) { Resource parentResource = resources.next(); @@ -313,16 +315,21 @@ public class MergingResourceProvider extends ResourceProvider<Void> { while (iter.hasNext()) { final ResourceHolder holder = iter.next(); if (handler.isHidden(holder.name, false)) { - iter.remove(); + iter.remove(); // remove from the candidates list } } } + 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 is this an overlaid resource (i.e. has the resource with the same name already be exposed through the underlying resource) + + // 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); if (current.name.equals(rsrcName)) { @@ -331,14 +338,24 @@ public class MergingResourceProvider extends ResourceProvider<Void> { break; } } + // for new resources, i.e. no underlying resource found... if (holder == null) { // remove the hidden child resources from the local resource if (handler != null && handler.isHidden(rsrcName, true)) { continue; // skip this child } holder = new ResourceHolder(rsrcName); - candidates.add(holder); - } + if (previousChildPositionInCandidateList != -1) { + // either add after the previous child position + candidates.add(previousChildPositionInCandidateList+1, holder); + previousChildPositionInCandidateList++; + } else { + // or add to the end of the list + candidates.add(holder); + previousChildPositionInCandidateList = candidates.size() - 1; + } + } + // in all cases the holder should get the current child! holder.resources.add(child); // Check if children need reordering @@ -357,15 +374,20 @@ public class MergingResourceProvider extends ResourceProvider<Void> { index++; } } - + // either reorder because of explicit reording property if (orderBeforeIndex > -1) { candidates.add(orderBeforeIndex, holder); candidates.remove(candidates.size() - 1); + previousChildPositionInCandidateList = orderBeforeIndex; } else { - // if there was no explicit order, just assume the order given by the overlying resource - if (childPositionInCandidateList != -1) { - candidates.add(holder); + // or reorder because overlaid resource has a different order + if (childPositionInCandidateList != -1 && previousChildPositionInCandidateList != -1) { candidates.remove(childPositionInCandidateList); + if (childPositionInCandidateList < previousChildPositionInCandidateList) { + previousChildPositionInCandidateList--; + } + candidates.add(previousChildPositionInCandidateList+1, holder); + previousChildPositionInCandidateList++; } } } diff --git a/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java b/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java index 3a58c5f..0d20d74 100644 --- a/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java +++ b/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java @@ -282,7 +282,7 @@ public class CommonMergedResourceProviderTest { @Test public void testOrderOfPartiallyOverwrittenChildren() throws PersistenceException { // see https://issues.apache.org/jira/browse/SLING-4915 - + // and https://issues.apache.org/jira/browse/SLING-6956 // create new child nodes below base and overlay MockHelper.create(this.resolver) .resource("/apps/base/child1") @@ -298,11 +298,74 @@ public class CommonMergedResourceProviderTest { IteratorIterable<Resource> iterable = new IteratorIterable<Resource>(provider.listChildren(ctx, mergedResource), true); Assert.assertThat(iterable, Matchers.contains(ResourceMatchers.name("child1"),ResourceMatchers.name("child4"), ResourceMatchers.name("child2"), ResourceMatchers.name("child3"))); + + + } + + @Test + public void testOrderOfPartiallyOverwrittenChildren() throws PersistenceException { + // create new child nodes below base and overlay + MockHelper.create(this.resolver) + .resource("/apps/base/child1") + .resource("/apps/base/child2") + .resource("/apps/base/child3") + .resource("/apps/overlay/child2") + .commit(); + + Resource mergedResource = this.provider.getResource(ctx, "/merged", 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("child1"),ResourceMatchers.name("child2"), ResourceMatchers.name("child3"))); } @Test + public void testOrderOfPartiallyOverlappingChildren() throws PersistenceException { + // see https://issues.apache.org/jira/browse/SLING-4915 + // and https://issues.apache.org/jira/browse/SLING-6956 + // create new child nodes below base and overlay + MockHelper.create(this.resolver) + .resource("/apps/base/child1") + .resource("/apps/base/child2") + .resource("/apps/base/child3") + .resource("/apps/overlay/child4") + .resource("/apps/overlay/child2") + .resource("/apps/overlay/child3") + .commit(); + + Resource mergedResource = this.provider.getResource(ctx, "/merged", 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("child1"),ResourceMatchers.name("child4"), ResourceMatchers.name("child2"), ResourceMatchers.name("child3"))); + } + + @Test + public void testOrderOfPartiallyOverlappingChildrenWithDifferentOrder() throws PersistenceException { + // see https://issues.apache.org/jira/browse/SLING-4915 + // and https://issues.apache.org/jira/browse/SLING-6956 + // create new child nodes below base and overlay + MockHelper.create(this.resolver) + .resource("/apps/base/child1") + .resource("/apps/base/child3") + .resource("/apps/base/child5") + .resource("/apps/overlay/child2") + .resource("/apps/overlay/child3") + .resource("/apps/overlay/child4") + .resource("/apps/overlay/child1") + .commit(); + + Resource mergedResource = this.provider.getResource(ctx, "/merged", 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("child5"),ResourceMatchers.name("child2"), ResourceMatchers.name("child3"), ResourceMatchers.name("child4"),ResourceMatchers.name("child1"))); + } + + @Test public void testOrderOfNonOverlappingChildren() throws PersistenceException { - // create new child nodes below base and overlay + // create new child nodes below base and overlay + // https://issues.apache.org/jira/browse/SLING-6956 MockHelper.create(this.resolver) .resource("/apps/base/child1") .resource("/apps/base/child2") @@ -315,6 +378,41 @@ public class CommonMergedResourceProviderTest { Assert.assertThat(iterable, Matchers.contains(ResourceMatchers.name("child1"),ResourceMatchers.name("child2"), ResourceMatchers.name("child3"), ResourceMatchers.name("child4"))); } + + @Test + public void testOrderOfFullyOverlappingChildren() throws PersistenceException { + // create new child nodes below base and overlay + // https://issues.apache.org/jira/browse/SLING-6956 + MockHelper.create(this.resolver) + .resource("/apps/base/child1") + .resource("/apps/base/child2") + .resource("/apps/base/child3") + .resource("/apps/overlay/child2") + .commit(); + Resource mergedResource = this.provider.getResource(ctx, "/merged", 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("child1"),ResourceMatchers.name("child2"), ResourceMatchers.name("child3"))); + } + + @Test + public void testOrderOfFullyOverlappingChildrenWithDifferentOrder() throws PersistenceException { + // create new child nodes below base and overlay + // https://issues.apache.org/jira/browse/SLING-6956 + MockHelper.create(this.resolver) + .resource("/apps/base/child1") + .resource("/apps/base/child2") + .resource("/apps/base/child3") + .resource("/apps/overlay/child3") + .resource("/apps/overlay/child2") + .commit(); + Resource mergedResource = this.provider.getResource(ctx, "/merged", 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("child1"),ResourceMatchers.name("child2"), ResourceMatchers.name("child3"))); + } @Test public void testOrderBeingModifiedThroughOrderBefore() throws PersistenceException { -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
