You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2017/12/05 16:18:35 UTC

[sling-org-apache-sling-servlets-resolver] branch issue/SLING-7134 created (now a39e22f)

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

radu pushed a change to branch issue/SLING-7134
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git.


      at a39e22f  SLING-7134 - Script execution order is not deterministic on Java 9

This branch includes the following new commits:

     new a39e22f  SLING-7134 - Script execution order is not deterministic on Java 9

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


-- 
To stop receiving notification emails like this one, please contact
['"commits@sling.apache.org" <co...@sling.apache.org>'].

[sling-org-apache-sling-servlets-resolver] 01/01: SLING-7134 - Script execution order is not deterministic on Java 9

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch issue/SLING-7134
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git

commit a39e22f6108d93da0a122c4993ad7d262a9ff3e7
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Tue Dec 5 16:58:10 2017 +0100

    SLING-7134 - Script execution order is not deterministic on Java 9
    
    * re-added the ordinal comparison in WeightedResource
    * changed comparator from org.apache.sling.servlets.resolver.internal.helper.AbstractResourceCollector#getServlets
    to first check the extensions for sorting (if existing), then the actual sorting order of the WeightedResources
    if the extensions are the same
---
 .../internal/helper/AbstractResourceCollector.java | 28 +++++++++-------------
 .../resolver/internal/helper/WeightedResource.java |  3 ++-
 .../internal/helper/WeightedResourceTest.java      | 12 ++++++++++
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/AbstractResourceCollector.java b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/AbstractResourceCollector.java
index 7a15896..10d2cad 100644
--- a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/AbstractResourceCollector.java
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/AbstractResourceCollector.java
@@ -75,25 +75,19 @@ public abstract class AbstractResourceCollector {
         final SortedSet<WeightedResource> resources = new TreeSet<>(new Comparator<WeightedResource>() {
             @Override
             public int compare(WeightedResource o1, WeightedResource o2) {
-                if (o1.equals(o2)) {
-                    return 0;
-                }
-                int comparisonResult = o1.compareTo(o2);
-                if (comparisonResult == 0) {
-                    String o1Extension = getScriptExtension(o1.getName());
-                    String o2Extension = getScriptExtension(o2.getName());
-                    if (StringUtils.isNotEmpty(o1Extension) && StringUtils.isNotEmpty(o2Extension)) {
-                        int o1ExtensionIndex = scriptExtensions.indexOf(o1Extension);
-                        int o2ExtensionIndex = scriptExtensions.indexOf(o2Extension);
-                        if (o1ExtensionIndex > o2ExtensionIndex) {
-                            return -1;
-                        } else if (o1ExtensionIndex == o2ExtensionIndex) {
-                            return 0;
-                        }
-                        return 1;
+                String o1Extension = getScriptExtension(o1.getName());
+                String o2Extension = getScriptExtension(o2.getName());
+                if (StringUtils.isNotEmpty(o1Extension) && StringUtils.isNotEmpty(o2Extension)) {
+                    int o1ExtensionIndex = scriptExtensions.indexOf(o1Extension);
+                    int o2ExtensionIndex = scriptExtensions.indexOf(o2Extension);
+                    if (o1ExtensionIndex > o2ExtensionIndex) {
+                        return -1;
+                    } else if (o1ExtensionIndex == o2ExtensionIndex) {
+                        return o1.compareTo(o2);
                     }
+                    return 1;
                 }
-                return comparisonResult;
+                return o1.compareTo(o2);
             }
         });
         final Iterator<String> locations = new LocationIterator(resourceType, resourceSuperType,
diff --git a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResource.java b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResource.java
index ad6244e..470593f 100644
--- a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResource.java
+++ b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResource.java
@@ -131,6 +131,7 @@ final class WeightedResource extends ResourceWrapper implements
             return 1;
         }
 
-        return 0;
+        // extensions are equal, compare ordinal (lower ordinal wins)
+        return (ordinal < o.ordinal) ? -1 : 1;
     }
 }
diff --git a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResourceTest.java b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResourceTest.java
index 85ae4e7..db50e84 100644
--- a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResourceTest.java
+++ b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/WeightedResourceTest.java
@@ -71,5 +71,17 @@ public class WeightedResourceTest extends TestCase {
         assertTrue(lr1.compareTo(lr2) < 0);
         assertTrue(lr2.compareTo(lr1) > 0);
     }
+
+    public void testCompareToOrdinal() {
+        WeightedResource lr1 = new WeightedResource(0, null, 0, WeightedResource.WEIGHT_NONE);
+        WeightedResource lr2 = new WeightedResource(1, null, 0, WeightedResource.WEIGHT_NONE);
+
+        // expect the same objects to compare equal
+        assertEquals(0, lr1.compareTo(lr1));
+        assertEquals(0, lr2.compareTo(lr2));
+
+        assertTrue(lr1.compareTo(lr2) < 0);
+        assertTrue(lr2.compareTo(lr1) > 0);
+    }
  
 }

-- 
To stop receiving notification emails like this one, please contact
"commits@sling.apache.org" <co...@sling.apache.org>.