This is an automated email from the ASF dual-hosted git repository. daim pushed a commit to branch OAK-11747 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit ae75afdbe874844aca178251e7aaeb0346677695 Author: Rishabh Kumar <[email protected]> AuthorDate: Fri May 30 21:07:41 2025 +0530 OAK-11747 : removed usage of Guava's ComparisonChain with JDK comparator --- .../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() {
