Skip to content

OAK-11324 : removed usage of Guava's Sets.symmetricDifference #1924

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rishabhdaim merged 2 commits into from
Dec 30, 2024
Merged

OAK-11324 : removed usage of Guava's Sets.symmetricDifference #1924

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09cf30a
OAK-11324 : removed usage of Guava's Sets.symmetricDifference
Dec 30, 2024
78590cb
OAK-11324 : replaced empty hashset constructor usage wth Set.of
Dec 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions ...plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/BlobGCCheckpointRefTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

import javax.management.openmbean.TabularData;

import org.apache.jackrabbit.guava.common.collect.Sets;
import org.apache.jackrabbit.oak.api.jmx.CheckpointMBean;
import org.apache.jackrabbit.oak.commons.collections.CollectionUtils;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
Expand Down Expand Up @@ -66,7 +65,7 @@ public void gcCheckpointHeld() throws Exception {
log.info("{} blobs remaining : {}", present.size(), present);

Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(maxGcAge), false);
assertTrue(Sets.symmetricDifference(present, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(present, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -80,7 +79,7 @@ public void gcCheckpointHeldNoAddition() throws Exception {
long maxGcAge = checkpointMBean.getOldestCheckpointCreationTimestamp() - afterSetupTime;

Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(maxGcAge), false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -101,7 +100,7 @@ public void gcCheckpointHeldMaxAgeChange() throws Exception {
log.info("Max age configured {}", maxGcAge);

Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(maxGcAge), false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
}

/**
Expand Down
23 changes: 11 additions & 12 deletions oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/BlobGCTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@

import ch.qos.logback.classic.Level;
import org.apache.jackrabbit.guava.common.collect.Iterators;
import org.apache.jackrabbit.guava.common.collect.Sets;
import org.apache.jackrabbit.guava.common.io.Closer;
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.CommitFailedException;
Expand Down Expand Up @@ -221,7 +220,7 @@ public void sharedGC() throws Exception {
executeGarbageCollection(cluster, cluster.getCollector(0), true);
Set<String> existingAfterGC = executeGarbageCollection(secondCluster, secondCluster.getCollector(0), false);

assertTrue(Sets.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
assertStats(secondCluster.statsProvider, 1, 0, totalAdded.size() - totalPresent.size(),
totalAdded.size() - totalPresent.size(), secondCluster.blobStoreState.blobsPresent.size(),
cluster.blobSize, NAME);
Expand Down Expand Up @@ -263,7 +262,7 @@ public void sharedGCRepositoryCloned() throws Exception {
executeGarbageCollection(cluster, cluster.getCollector(0), true);
Set<String> existingAfterGC = executeGarbageCollection(secondCluster, secondCluster.getCollector(0), false);

assertTrue(Sets.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -288,7 +287,7 @@ public void sharedGCRefsOld() throws Exception {

Set<String> existingAfterGC = executeGarbageCollection(secondCluster, secondCluster.getCollector(5, false, true), false);

assertTrue(Sets.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(totalPresent, existingAfterGC).isEmpty());
assertStats(secondCluster.statsProvider, 1, 0, totalAdded.size() - totalPresent.size(),
totalAdded.size() - totalPresent.size(), secondCluster.blobStoreState.blobsPresent.size(),
secondCluster.blobSize, NAME);
Expand Down Expand Up @@ -316,7 +315,7 @@ public void sharedGCRefsNotOld() throws Exception {

Set<String> existingAfterGC = executeGarbageCollection(secondCluster, secondCluster.getCollector(6, false, true), false);

assertTrue(Sets.symmetricDifference(totalAdded, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(totalAdded, existingAfterGC).isEmpty());
assertStats(secondCluster.statsProvider, 1, 1, 0,0, secondCluster.blobStoreState.blobsPresent.size(),
secondCluster.blobSize, NAME);
}
Expand All @@ -326,7 +325,7 @@ public void gc() throws Exception {
log.info("Starting gc()");

Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(0), false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
Expand All @@ -340,7 +339,7 @@ public void gcWithConsistencyCheck() throws Exception {

MarkSweepGarbageCollector collector = cluster.getCollector(0, true, false);
Set<String> existingAfterGC = executeGarbageCollection(cluster, collector, false);
assertFalse(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertFalse(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size() + 1,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size() + 1,
Expand All @@ -356,7 +355,7 @@ public void gcWithNoDeleteDirectBinary() throws Exception {

setupDirectBinary(1, 0);
Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(0), false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
Expand All @@ -369,7 +368,7 @@ public void gcWithDeleteDirectBinary() throws Exception {

setupDirectBinary(5, 2);
Set<String> existingAfterGC = executeGarbageCollection(cluster, cluster.getCollector(0), false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
Expand All @@ -386,7 +385,7 @@ public void noGc() throws Exception {
Set<String> existingAfterGC =
executeGarbageCollection(cluster, cluster.getCollector(afterSetupTime - cluster.startReferenceTime + 2),
false);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsAdded, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsAdded, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0, 0,
cluster.blobStoreState.blobsAdded.size() - cluster.blobStoreState.blobsPresent.size(),
cluster.blobStoreState.blobsPresent.size(), cluster.blobSize, NAME);
Expand Down Expand Up @@ -553,7 +552,7 @@ public void gcCheckDeletedSize() throws Exception {
cluster.blobStoreState.blobsPresent.size(), cluster.blobSize, NAME);
assertEquals(deletedSize, getStatCount(cluster.statsProvider, NAME, TOTAL_SIZE_DELETED));
customLogs.finished();
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsPresent, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -562,7 +561,7 @@ public void gcMarkOnly() throws Exception {

Set<String> existingAfterGC =
executeGarbageCollection(cluster, cluster.getCollector(0),true);
assertTrue(Sets.symmetricDifference(cluster.blobStoreState.blobsAdded, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(cluster.blobStoreState.blobsAdded, existingAfterGC).isEmpty());
assertStats(cluster.statsProvider, 1, 0, 0, 0, cluster.blobStoreState.blobsPresent.size(), cluster.blobSize,
NAME);
assertEquals(cluster.blobStoreState.blobsPresent.size(), getStatCount(cluster.statsProvider, NAME,
Expand Down
5 changes: 2 additions & 3 deletions ...rc/test/java/org/apache/jackrabbit/oak/plugins/blob/datastore/BlobIdTrackerStoreTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.Set;
import java.util.concurrent.CountDownLatch;

import org.apache.jackrabbit.oak.commons.collections.CollectionUtils;
import org.apache.jackrabbit.oak.plugins.blob.SharedDataStore;
import org.apache.jackrabbit.oak.plugins.blob.datastore.BlobIdTracker.BlobIdStore;
import org.junit.After;
Expand All @@ -40,7 +41,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.apache.jackrabbit.guava.common.collect.Sets.symmetricDifference;
import static java.lang.String.valueOf;
import static java.util.UUID.randomUUID;
import static org.apache.commons.io.IOUtils.closeQuietly;
Expand Down Expand Up @@ -129,8 +129,7 @@ public void addSnapshotAdd() throws Exception {
Set<String> retrieved = retrieve(store);

assertTrue("Incorrect elements with add before snapshot",
symmetricDifference(initAdd, retrieved)
.containsAll(Set.of("10001", "10002", "10003")));
CollectionUtils.symmetricDifference(initAdd, retrieved).containsAll(Set.of("10001", "10002", "10003")));
}

@Test
Expand Down
20 changes: 20 additions & 0 deletions oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
Expand Down Expand Up @@ -240,6 +241,25 @@ public static <T> Set<T> intersection(@NotNull final Set<T> s1, @NotNull final S
return s1.stream().filter(s2::contains).collect(Collectors.toSet());
}

/**
* Returns a new set containing the symmetric difference of the two specified sets.
* The symmetric difference of two sets is a set containing elements that are in either of the sets,
* but not in their intersection.
*
* @param <T> the type of elements in the sets
* @param s1 the first set, must not be null
* @param s2 the second set, must not be null
* @return a new set containing the symmetric difference of the two specified sets
* @throws NullPointerException if either of the sets is null
*/
public static <T> Set<T> symmetricDifference(final Set<T> s1, final Set<T> s2) {
Objects.requireNonNull(s1);
Objects.requireNonNull(s2);
final Set<T> result = new HashSet<>(s1);
s2.stream().filter(Predicate.not(result::add)).forEach(result::remove);
return result;
}

/**
* Convert an iterable to a {@link java.util.ArrayDeque}.
* The returning array deque is mutable and supports all optional operations.
Expand Down
62 changes: 62 additions & 0 deletions ...mons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,68 @@ public void testIntersectionWithNoCommonElements() {
Assert.assertEquals(Collections.EMPTY_SET, CollectionUtils.intersection(set1, set2));
}

@Test
public void testSymmetricDifferenceWithNonEmptySets() {
final Set<String> set1 = Set.of("a", "b", "c");
final Set<String> set2 = Set.of("b", "c", "d");

final Set<String> result = CollectionUtils.symmetricDifference(set1, set2);

final Set<String> expected = Set.of("a", "d");
Assert.assertEquals(expected, result);
}

@Test
public void testSymmetricDifferenceWithEmptySet() {
final Set<String> set1 = Set.of("a", "b", "c");
final Set<String> set2 = new HashSet<>();

final Set<String> result = CollectionUtils.symmetricDifference(set1, set2);

final Set<String> expected = Set.of("a", "b", "c");
Assert.assertEquals(expected, result);
}

@Test
public void testSymmetricDifferenceWithBothEmptySets() {
final Set<String> set1 = new HashSet<>();
final Set<String> set2 = new HashSet<>();

final Set<String> result = CollectionUtils.symmetricDifference(set1, set2);

final Set<String> expected = new HashSet<>();
Assert.assertEquals(expected, result);
}

@Test(expected = NullPointerException.class)
public void testSymmetricDifferenceWithNullFirstSet() {
final Set<String> set1 = null;
final Set<String> set2 = Set.of("a", "b", "c");

CollectionUtils.symmetricDifference(set1, set2);
fail("Shouldn't reach here");
}

@Test(expected = NullPointerException.class)
public void testSymmetricDifferenceWithNullSecondSet() {
Set<String> set1 = Set.of("a", "b", "c");
Set<String> set2 = null;

CollectionUtils.symmetricDifference(set1, set2);
fail("Shouldn't reach here");
}

@Test
public void testSymmetricDifferenceWithNoCommonElements() {
final Set<String> set1 = Set.of("a", "b", "c");
final Set<String> set2 = Set.of("d", "e", "f");

final Set<String> result = CollectionUtils.symmetricDifference(set1, set2);

final Set<String> expected = Set.of("a", "b", "c", "d", "e", "f");
Assert.assertEquals(expected, result);
}

@Test
public void iteratorToIIteratable() {
Iterator<String> iterator = List.of("a", "b", "c").iterator();
Expand Down
12 changes: 6 additions & 6 deletions ...segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentDataStoreBlobGCIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public void gc() throws Exception {
log.info("{} blobs that should remain after gc : {}", state.blobsPresent.size(), state.blobsPresent);
log.info("{} blobs for nodes which are deleted : {}", state.blobsPresent.size(), state.blobsPresent);
Set<String> existingAfterGC = gcInternal(0);
assertTrue(Sets.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
}

@Test
Expand Down Expand Up @@ -299,7 +299,7 @@ public void noGc() throws Exception {
log.info("{} blobs that should remain after gc : {}", state.blobsAdded.size(), state.blobsAdded);
log.info("{} blobs for nodes which are deleted : {}", state.blobsPresent.size(), state.blobsPresent);
Set<String> existingAfterGC = gcInternal(86400);
assertTrue(Sets.symmetricDifference(state.blobsAdded, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(state.blobsAdded, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -309,7 +309,7 @@ public void gcSpecialChar() throws Exception {
state.blobsAdded.addAll(specialCharNodeBlobs);
state.blobsPresent.addAll(specialCharNodeBlobs);
Set<String> existingAfterGC = gcInternal(0);
assertTrue(Sets.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
}

@Test
Expand All @@ -326,7 +326,7 @@ public void consistencyCheckInit() throws Exception {
public void consistencyCheckWithGc() throws Exception {
DataStoreState state = setUp();
Set<String> existingAfterGC = gcInternal(0);
assertTrue(Sets.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());

ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(10);
MarkSweepGarbageCollector gcObj = init(86400, executor);
Expand Down Expand Up @@ -372,7 +372,7 @@ public void gcLongRunningBlobCollection() throws Exception {
log.info("{} Blobs existing after gc {}", existingAfterGC.size(), existingAfterGC);

assertTrue(Sets.difference(state.blobsPresent, existingAfterGC).isEmpty());
assertEquals(gc.additionalBlobs, Sets.symmetricDifference(state.blobsPresent, existingAfterGC));
assertEquals(gc.additionalBlobs, CollectionUtils.symmetricDifference(state.blobsPresent, existingAfterGC));
}

@Test
Expand All @@ -383,7 +383,7 @@ public void gcWithInlined() throws Exception {
log.info("{} blobs that should remain after gc : {}", state.blobsAdded.size(), state.blobsAdded);
log.info("{} blobs for nodes which are deleted : {}", state.blobsPresent.size(), state.blobsPresent);
Set<String> existingAfterGC = gcInternal(0);
assertTrue(Sets.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
assertTrue(CollectionUtils.symmetricDifference(state.blobsPresent, existingAfterGC).isEmpty());
}

@Test
Expand Down
Loading
Loading