OAK-11747 : removed usage of Guava's ComparisonChain with JDK comparator (#2319)

Co-authored-by: Rishabh Kumar <diam@adobe.com>

Author: rishabhdaim

oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java

@@ -31,6 +31,7 @@
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -44,7 +45,6 @@
 import javax.jcr.PropertyType;
 
 import org.apache.commons.io.input.CountingInputStream;
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.JcrConstants;
@@ -3264,11 +3264,10 @@ public Tuple2(Comparable value, Comparable value2, String path) {
         }
 
         @Override
-        public int compareTo(Tuple2 o) {
-            return ComparisonChain.start()
-                    .compare(value, o.value)
-                    .compare(value2, o.value2, Collections.reverseOrder())
-                    .result();
+        public int compareTo(@NotNull Tuple2 o) {
+            return Comparator.comparing((Tuple2 t) -> t.value)
+                    .thenComparing( t -> ((Tuple2) t).value2, Comparator.reverseOrder())
+                    .compare(this, o);
         }
 
         @Override

oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStats.java

@@ -24,12 +24,13 @@
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
 import org.codehaus.groovy.runtime.StringGroovyMethods;
+import org.jetbrains.annotations.NotNull;
 
 import static org.apache.jackrabbit.oak.commons.IOUtils.humanReadableByteCount;
 
@@ -131,7 +132,7 @@ private String getSummary(List<MimeTypeStats> stats) {
         return sw.toString();
     }
 
-    private MimeTypeStats createStat(String mimeType) {
+    MimeTypeStats createStat(String mimeType) {
         MimeTypeStats stats = new MimeTypeStats(mimeType);
         stats.setIndexed(tika.isIndexed(mimeType));
         stats.setSupported(tika.isSupportedMediaType(mimeType));
@@ -142,7 +143,7 @@ private static String center(String s, int width) {
         return StringGroovyMethods.center(s, width);
     }
 
-    private static class MimeTypeStats implements Comparable<MimeTypeStats> {
+    static class MimeTypeStats implements Comparable<MimeTypeStats> {
         private final String mimeType;
         private int count;
         private long totalSize;
@@ -187,11 +188,11 @@ public boolean isSupported() {
         }
 
         @Override
-        public int compareTo(MimeTypeStats o) {
-            return ComparisonChain.start()
-                    .compareFalseFirst(indexed, o.indexed)
-                    .compare(totalSize, o.totalSize)
-                    .result();
+        public int compareTo(@NotNull MimeTypeStats o) {
+            return Comparator
+                    .comparing(MimeTypeStats::isIndexed)  // false comes before true by default
+                    .thenComparingLong(MimeTypeStats::getTotalSize)        // then compare by totalSize
+                    .compare(this, o);
         }
     }
 }

oak-run/src/test/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStatsTest.java

@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.tika;
+
+import org.apache.jackrabbit.guava.common.collect.FluentIterable;
+import org.apache.jackrabbit.oak.plugins.tika.BinaryStats.MimeTypeStats;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class BinaryStatsTest {
+
+    @Test
+    public void testMimeTypeStatsComparison() throws IOException {
+        // Create BinaryStats instance with a minimal implementation of BinaryResourceProvider
+        BinaryStats stats = new BinaryStats(null, path -> FluentIterable.of());
+
+        // Create test MimeTypeStats instances
+        MimeTypeStats indexedLargeSize = stats.createStat("application/pdf");
+        indexedLargeSize.setIndexed(true);
+        indexedLargeSize.addSize(1000);
+
+        MimeTypeStats indexedSmallSize = stats.createStat("text/plain");
+        indexedSmallSize.setIndexed(true);
+        indexedSmallSize.addSize(100);
+
+        MimeTypeStats notIndexedLargeSize = stats.createStat("image/png");
+        notIndexedLargeSize.setIndexed(false);
+        notIndexedLargeSize.addSize(2000);
+
+        MimeTypeStats notIndexedSmallSize = stats.createStat("audio/mp3");
+        notIndexedSmallSize.setIndexed(false);
+        notIndexedSmallSize.addSize(500);
+
+        // Test case 1: Compare by indexed status (false first, then true)
+        Assert.assertTrue(notIndexedLargeSize.compareTo(indexedLargeSize) < 0);
+        Assert.assertTrue(indexedLargeSize.compareTo(notIndexedLargeSize) > 0);
+
+        // Test case 2: Same indexed status, compare by size (larger comes first)
+        Assert.assertTrue(indexedLargeSize.compareTo(indexedSmallSize) > 0);
+        Assert.assertTrue(notIndexedLargeSize.compareTo(notIndexedSmallSize) > 0);
+
+        // Test case 3: Sort a list and verify ordering
+        List<MimeTypeStats> statsList = new ArrayList<>();
+        statsList.add(indexedLargeSize);
+        statsList.add(notIndexedSmallSize);
+        statsList.add(indexedSmallSize);
+        statsList.add(notIndexedLargeSize);
+
+        Collections.sort(statsList);
+
+        // Verify sort order: not indexed first (larger to smaller), then indexed (larger to smaller)
+        Assert.assertSame(notIndexedSmallSize, statsList.get(0));
+        Assert.assertSame(notIndexedLargeSize, statsList.get(1));
+        Assert.assertSame(indexedSmallSize, statsList.get(2));
+        Assert.assertSame(indexedLargeSize, statsList.get(3));
+    }
+}

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapEntry.java

@@ -24,7 +24,6 @@
 import java.util.Comparator;
 import java.util.Map;
 
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
 import org.apache.jackrabbit.oak.commons.conditions.Validate;
 import org.apache.jackrabbit.oak.spi.state.AbstractChildNodeEntry;
 import org.jetbrains.annotations.NotNull;
@@ -149,11 +148,10 @@ public RecordId setValue(RecordId value) {
 
     @Override
     public int compareTo(@NotNull MapEntry that) {
-        return ComparisonChain.start()
-                .compare(getHash() & HASH_MASK, that.getHash() & HASH_MASK)
-                .compare(name, that.name)
-                .compare(value, that.value, Comparator.nullsLast(Comparator.naturalOrder()))
-                .result();
+        return Comparator.comparingLong((MapEntry me) -> me.getHash() & HASH_MASK)
+                .thenComparing(MapEntry::getName)
+                .thenComparing(MapEntry::getValue, Comparator.nullsLast(Comparator.naturalOrder()))
+                .compare(this, that);
     }
 
 }

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java

@@ -27,11 +27,11 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
 import org.apache.jackrabbit.oak.commons.collections.IterableUtils;
 import org.apache.jackrabbit.oak.spi.state.DefaultNodeStateDiff;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -631,10 +631,9 @@ private static int compare(MapEntry before, MapEntry after) {
         } else if (after == null) {
             return -1;  // see above
         } else {
-            return ComparisonChain.start()
-                    .compare(before.getHash() & HASH_MASK, after.getHash() & HASH_MASK)
-                    .compare(before.getName(), after.getName())
-                    .result();
+            return Comparator.comparingLong((MapEntry me) -> me.getHash() & HASH_MASK)
+                    .thenComparing(MapEntry::getName)
+                    .compare(before, after);
         }
     }
 

oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/PropertyTemplate.java

@@ -26,7 +26,7 @@
 import org.apache.jackrabbit.oak.commons.StringUtils;
 import org.jetbrains.annotations.NotNull;
 
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
+import java.util.Comparator;
 
 /**
  * A property definition within a template (the property name, the type, and the
@@ -74,11 +74,10 @@ public Type<?> getType() {
     @Override
     public int compareTo(@NotNull PropertyTemplate template) {
         requireNonNull(template);
-        return ComparisonChain.start()
-                .compare(hashCode(), template.hashCode()) // important
-                .compare(name, template.name)
-                .compare(type, template.type)
-                .result();
+        return Comparator.comparingInt(PropertyTemplate::hashCode)
+                .thenComparing(PropertyTemplate::getName)
+                .thenComparing(PropertyTemplate::getType)
+                .compare(this, template);
     }
 
     //------------------------------------------------------------< Object >--

oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/PropertyTemplateTest.java

@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.segment;
+
+import org.apache.jackrabbit.oak.api.Type;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class PropertyTemplateTest {
+
+    @Test
+    public void testCompareToWithDifferentNames() {
+        PropertyTemplate p1 = new PropertyTemplate(0, "aName", Type.STRING);
+        PropertyTemplate p2 = new PropertyTemplate(1, "bName", Type.STRING);
+
+        // Since hashCode is used for primary comparison and hashCode uses only name,
+        // templates with different names should be ordered by name's hashCode
+        Assert.assertTrue(p1.hashCode() < p2.hashCode());
+        Assert.assertTrue(p1.compareTo(p2) < 0);
+        Assert.assertTrue(p2.compareTo(p1) > 0);
+    }
+
+    @Test
+    public void testCompareToWithSameNamesDifferentTypes() {
+        PropertyTemplate p1 = new PropertyTemplate(0, "sameName", Type.STRING);
+        PropertyTemplate p2 = new PropertyTemplate(0, "sameName", Type.BOOLEAN);
+
+        // Same name (so same hashCode), different types
+        Assert.assertEquals(p1.hashCode(), p2.hashCode());
+        // Type comparison is determined by the Type enum's natural order
+        Assert.assertNotEquals(0, p1.compareTo(p2));
+    }
+
+    @Test
+    public void testCompareToWithSameNamesSameTypesDifferentIndices() {
+        PropertyTemplate p1 = new PropertyTemplate(0, "sameName", Type.STRING);
+        PropertyTemplate p2 = new PropertyTemplate(1, "sameName", Type.STRING);
+
+        // Same name and type, different indices
+        Assert.assertEquals(p1.hashCode(), p2.hashCode());
+        Assert.assertEquals(0, p1.compareTo(p2)); // Index is not used in comparison
+        Assert.assertEquals(p1, p2); // Index is not used in equals either
+    }
+
+    @Test
+    public void testConsistencyBetweenEqualsAndCompareTo() {
+        PropertyTemplate p1 = new PropertyTemplate(0, "name", Type.STRING);
+        PropertyTemplate p2 = new PropertyTemplate(0, "name", Type.STRING);
+        PropertyTemplate p3 = new PropertyTemplate(0, "name", Type.BOOLEAN);
+
+        // Basic equality check
+        Assert.assertEquals(p1, p1); // Reflexivity
+        Assert.assertEquals(p1, p2); // Symmetry
+        Assert.assertEquals(0, p1.compareTo(p2));
+
+        // Different type
+        Assert.assertNotEquals(p1, p3);
+        Assert.assertNotEquals(0, p1.compareTo(p3));
+
+        // Not a PropertyTemplate
+        Assert.assertNotEquals("not a template", p1);
+    }
+
+    @Test
+    public void testSortingBehavior() {
+        List<PropertyTemplate> templates = new ArrayList<>();
+
+        // Create templates with varying names and types
+        templates.add(new PropertyTemplate(0, "c", Type.STRING));
+        templates.add(new PropertyTemplate(1, "a", Type.STRING));
+        templates.add(new PropertyTemplate(2, "b", Type.STRING));
+        templates.add(new PropertyTemplate(3, "a", Type.BOOLEAN));
+
+        Collections.sort(templates);
+
+        // Check the expected sort order based on hashCode, then name, then type
+        Assert.assertEquals("a", templates.get(0).getName()); // 'a' has lowest hashCode
+        // BOOLEAN comes after STRING for same name 'a'
+        // since they are compared by Type enum tag value (javax.jcr.PropertyType) which is higher for BOOLEAN
+        Assert.assertEquals(Type.STRING, templates.get(0).getType());
+        Assert.assertEquals("a", templates.get(1).getName());
+        Assert.assertEquals(Type.BOOLEAN, templates.get(1).getType());
+        Assert.assertEquals("b", templates.get(2).getName());
+        Assert.assertEquals("c", templates.get(3).getName());
+    }
+
+}

oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FormatVersion.java

@@ -17,10 +17,9 @@
 package org.apache.jackrabbit.oak.plugins.document;
 
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.List;
 
-import org.apache.jackrabbit.guava.common.collect.ComparisonChain;
-
 import static java.util.Objects.requireNonNull;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.SETTINGS;
 
@@ -259,11 +258,10 @@ public boolean equals(Object obj) {
     @Override
     public int compareTo(@NotNull FormatVersion other) {
         requireNonNull(other);
-        return ComparisonChain.start()
-                .compare(major, other.major)
-                .compare(minor, other.minor)
-                .compare(micro, other.micro)
-                .result();
+        return Comparator.comparingInt((FormatVersion fv) -> fv.major)
+                .thenComparingInt(fv -> fv.minor)
+                .thenComparingInt(fv -> fv.micro)
+                .compare(this, other);
     }
 
     private static DocumentStoreException concurrentUpdate() {