Author: kwin
Date: Fri Jun 16 13:42:53 2017
New Revision: 1798927
URL: http://svn.apache.org/viewvc?rev=1798927&view=rev
Log:
SLING-6956 do not touch order when no new resources have been defined in an
overlying resource
Modified:
sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
sling/trunk/contrib/extensions/resourcemerger/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java
Modified:
sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
URL:
http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java?rev=1798927&r1=1798926&r2=1798927&view=diff
==============================================================================
---
sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
(original)
+++
sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/impl/MergingResourceProvider.java
Fri Jun 16 13:42:53 2017
@@ -297,10 +297,12 @@ public class MergingResourceProvider ext
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 ext
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 ext
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 ext
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++;
}
}
}
Modified:
sling/trunk/contrib/extensions/resourcemerger/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java
URL:
http://svn.apache.org/viewvc/sling/trunk/contrib/extensions/resourcemerger/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java?rev=1798927&r1=1798926&r2=1798927&view=diff
==============================================================================
---
sling/trunk/contrib/extensions/resourcemerger/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java
(original)
+++
sling/trunk/contrib/extensions/resourcemerger/src/test/java/org/apache/sling/resourcemerger/impl/CommonMergedResourceProviderTest.java
Fri Jun 16 13:42:53 2017
@@ -282,7 +282,7 @@ public class CommonMergedResourceProvide
@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 CommonMergedResourceProvide
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 CommonMergedResourceProvide
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 {
