This is an automated email from the ASF dual-hosted git repository.
daim pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 44851487c5 OAK-11747 : removed usage of Guava's ComparisonChain with
JDK comparator (#2319)
44851487c5 is described below
commit 44851487c5e4d0f9b9b46a40da79d9c78a3c83b9
Author: Rishabh Kumar <[email protected]>
AuthorDate: Thu Jun 5 21:41:45 2025 +0530
OAK-11747 : removed usage of Guava's ComparisonChain with JDK comparator
(#2319)
Co-authored-by: Rishabh Kumar <[email protected]>
---
.../index/lucene/LucenePropertyIndexTest.java | 11 +--
.../jackrabbit/oak/plugins/tika/BinaryStats.java | 17 ++--
.../oak/plugins/tika/BinaryStatsTest.java | 78 +++++++++++++++
.../apache/jackrabbit/oak/segment/MapEntry.java | 10 +-
.../apache/jackrabbit/oak/segment/MapRecord.java | 9 +-
.../jackrabbit/oak/segment/PropertyTemplate.java | 11 +--
.../oak/segment/PropertyTemplateTest.java | 107 +++++++++++++++++++++
.../oak/plugins/document/FormatVersion.java | 12 +--
8 files changed, 217 insertions(+), 38 deletions(-)
diff --git
a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
index 65bbbce892..bc7edbb592 100644
---
a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
+++
b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
@@ -31,6 +31,7 @@ import java.util.ArrayList;
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 java.util.concurrent.Executors;
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 class LucenePropertyIndexTest extends
AbstractQueryTest {
}
@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
diff --git
a/oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStats.java
b/oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStats.java
index 2d00e4d4c1..ed91303318 100644
---
a/oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStats.java
+++
b/oak-run/src/main/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStats.java
@@ -24,12 +24,13 @@ import java.io.PrintWriter;
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 @@ class BinaryStats {
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 @@ class BinaryStats {
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 @@ class BinaryStats {
}
@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);
}
}
}
diff --git
a/oak-run/src/test/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStatsTest.java
b/oak-run/src/test/java/org/apache/jackrabbit/oak/plugins/tika/BinaryStatsTest.java
new file mode 100644
index 0000000000..83a34bd6e0
--- /dev/null
+++
b/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));
+ }
+}
\ No newline at end of file
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapEntry.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapEntry.java
index 033d8b2108..e797a6d7ad 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapEntry.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapEntry.java
@@ -24,7 +24,6 @@ import static
org.apache.jackrabbit.oak.segment.MapRecord.HASH_MASK;
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 @@ class MapEntry extends AbstractChildNodeEntry
@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);
}
}
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
index 80006efc17..5e2b0b5da6 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
@@ -27,11 +27,11 @@ import static
org.apache.jackrabbit.oak.segment.MapEntry.newMapEntry;
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 @@ public class MapRecord extends Record {
} 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);
}
}
diff --git
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/PropertyTemplate.java
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/PropertyTemplate.java
index 73c958436d..6e5ab706d8 100644
---
a/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/PropertyTemplate.java
+++
b/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/PropertyTemplate.java
@@ -26,7 +26,7 @@ import org.apache.jackrabbit.oak.api.Type;
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 class PropertyTemplate implements
Comparable<PropertyTemplate> {
@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 >--
diff --git
a/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/PropertyTemplateTest.java
b/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/PropertyTemplateTest.java
new file mode 100644
index 0000000000..481c1c86d9
--- /dev/null
+++
b/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());
+ }
+
+}
\ No newline at end of file
diff --git
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FormatVersion.java
b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FormatVersion.java
index 43cede1b15..ad1cbd065a 100644
---
a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FormatVersion.java
+++
b/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 final class FormatVersion implements
Comparable<FormatVersion> {
@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() {
