From 36e0c4efd52b185e000b59f9f215789ec6ae8ee5 Mon Sep 17 00:00:00 2001 From: Rishabh Kumar Date: Tue, 7 Jan 2025 21:21:54 +0530 Subject: [PATCH 1/2] OAK-11350 : removed usage of Guava's Maps.filterKeys --- .../oak/blob/cloud/s3/S3Backend.java | 3 +- .../commons/collections/CollectionUtils.java | 21 +++++ .../collections/CollectionUtilsTest.java | 76 +++++++++++++++++++ .../plugins/index/lucene/IndexTracker.java | 5 +- .../spi/query/FulltextIndexTracker.java | 4 +- .../document/LastRevRecoveryAgent.java | 4 +- .../document/mongo/MongoDocumentStore.java | 3 +- 7 files changed, 105 insertions(+), 11 deletions(-) diff --git a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java index 14583bc4f5b..cdb8934a856 100644 --- a/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java +++ b/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/s3/S3Backend.java @@ -98,7 +98,6 @@ import org.apache.jackrabbit.guava.common.cache.Cache; import org.apache.jackrabbit.guava.common.cache.CacheBuilder; import org.apache.jackrabbit.guava.common.collect.AbstractIterator; -import org.apache.jackrabbit.guava.common.collect.Maps; import static org.apache.jackrabbit.oak.commons.conditions.Validate.checkArgument; import static org.apache.jackrabbit.guava.common.collect.Iterables.filter; @@ -251,7 +250,7 @@ public void init() throws DataStoreException { LOG.error("Error ", e); Map filteredMap = new HashMap<>(); if (properties != null) { - filteredMap = Maps.filterKeys(Utils.asMap(properties), + filteredMap = CollectionUtils.filterKeys(Utils.asMap(properties), input -> !input.equals(S3Constants.ACCESS_KEY) &&!input.equals(S3Constants.SECRET_KEY)); } throw new DataStoreException("Could not initialize S3 from " + filteredMap, e); diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java index 238639078cc..51287a58b35 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java @@ -416,6 +416,27 @@ public static Map fromProperties(final @NotNull Properties prope e -> String.valueOf(e.getValue()))); } + /** + * Create a new {@link Map} after filtering the entries of the given map + * based on the specified predicate applied to the keys. + * + * @param the type of keys in the map + * @param the type of values in the map + * @param map the map to filter, must not be null + * @param predicate the predicate to apply to the keys, must not be null + * @return a new map containing only the entries whose keys match the predicate + * @throws NullPointerException if the map or predicate is null + */ + @NotNull + public static Map filterKeys(final @NotNull Map map, final @NotNull Predicate predicate) { + Objects.requireNonNull(map); + Objects.requireNonNull(predicate); + return map.entrySet() + .stream() + .filter(e -> predicate.test(e.getKey())) + .collect(HashMap::new, (m,v)->m.put(v.getKey(), v.getValue()), HashMap::putAll); + } + /** * Convert an {@code Iterator} to an {@code Iterable}. *

diff --git a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java index 5bd8a148fb2..5f35e7db6bf 100644 --- a/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java +++ b/oak-commons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java @@ -24,6 +24,7 @@ import java.util.ArrayDeque; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; @@ -33,6 +34,7 @@ import java.util.Properties; import java.util.Set; import java.util.TreeSet; +import java.util.function.Predicate; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -558,6 +560,80 @@ public void testFromPropertiesNull() { Assert.assertThrows(NullPointerException.class, () -> CollectionUtils.fromProperties(null)); } + @Test + public void testFilterKeys() { + final Map map = new HashMap<>(); + map.put("one", 1); + map.put("two", 2); + map.put("three", 3); + + final Predicate predicate = key -> key.startsWith("t"); + + final Map result = CollectionUtils.filterKeys(map, predicate); + + Assert.assertEquals(2, result.size()); + Assert.assertTrue(result.containsKey("two")); + Assert.assertTrue(result.containsKey("three")); + Assert.assertFalse(result.containsKey("one")); + } + + @Test + public void testFilterKeysEmptyMap() { + final Map map = new HashMap<>(); + final Predicate predicate = key -> key.startsWith("t"); + + final Map result = CollectionUtils.filterKeys(map, predicate); + + Assert.assertTrue(result.isEmpty()); + } + + @Test + public void testFilterNullKeys() { + final Map map = new HashMap<>(); + map.put("one", 1); + map.put("two", 2); + map.put("three", 3); + map.put(null, 4); + + final Predicate predicate = Objects::isNull; + + final Map result = CollectionUtils.filterKeys(map, predicate); + + Assert.assertEquals(1, result.size()); + } + + @Test + public void testFilterNonNullKeys() { + final Map map = new HashMap<>(); + map.put("one", 1); + map.put("two", 2); + map.put("three", 3); + map.put(null, 4); + + final Predicate predicate = Objects::nonNull; + + final Map result = CollectionUtils.filterKeys(map, predicate); + + Assert.assertEquals(3, result.size()); + } + + @Test + public void testFilterKeysNullMap() { + final Predicate predicate = key -> key.startsWith("t"); + + Assert.assertThrows(NullPointerException.class, () -> CollectionUtils.filterKeys(null, predicate)); + } + + @Test + public void testFilterKeysNullPredicate() { + final Map map = new HashMap<>(); + map.put("one", 1); + map.put("two", 2); + map.put("three", 3); + + Assert.assertThrows(NullPointerException.class, () -> CollectionUtils.filterKeys(map, null)); + } + @Test public void ensureCapacity() { int capacity = CollectionUtils.ensureCapacity(8); diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java index 91631d331ff..e4b9281e2a5 100644 --- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java +++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexTracker.java @@ -29,6 +29,7 @@ import org.apache.jackrabbit.guava.common.collect.Maps; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.commons.PerfLogger; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoService; import org.apache.jackrabbit.oak.plugins.index.lucene.hybrid.NRTIndexFactory; import org.apache.jackrabbit.oak.plugins.index.lucene.reader.DefaultIndexReaderFactory; @@ -49,8 +50,6 @@ import org.slf4j.LoggerFactory; import static java.util.Objects.requireNonNull; - - import static java.util.Collections.emptyMap; import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.TYPE_LUCENE; import static org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexHelper.isLuceneIndexNode; @@ -186,7 +185,7 @@ public void leave(NodeState before, NodeState after) { if (!updates.isEmpty()) { Map builder = new HashMap<>(); - builder.putAll(Maps.filterKeys(original, x -> !updates.keySet().contains(x))); + builder.putAll(CollectionUtils.filterKeys(original, x -> !updates.containsKey(x))); builder.putAll(Maps.filterValues(updates, x -> x != null)); indices = Collections.unmodifiableMap(builder); diff --git a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java index dfb36fdda8c..ac9b5c47985 100644 --- a/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java +++ b/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java @@ -27,6 +27,7 @@ import org.apache.jackrabbit.guava.common.collect.Iterables; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.commons.PerfLogger; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoService; import org.apache.jackrabbit.oak.plugins.index.search.BadIndexTracker; import org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition; @@ -45,7 +46,6 @@ import static java.util.Objects.requireNonNull; -import static org.apache.jackrabbit.guava.common.collect.Maps.filterKeys; import static org.apache.jackrabbit.guava.common.collect.Maps.filterValues; import static java.util.Collections.emptyMap; import static org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition.INDEX_DEFINITION_NODE; @@ -148,7 +148,7 @@ public void leave(NodeState before, NodeState after) { if (!updates.isEmpty()) { Map builder = new HashMap<>(); - builder.putAll(filterKeys(original, x -> !updates.keySet().contains(x))); + builder.putAll(CollectionUtils.filterKeys(original, x -> !updates.containsKey(x))); builder.putAll(filterValues(updates, x -> x != null)); indices = Collections.unmodifiableMap(builder); diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java index b3b44582db7..f5cf1cf646d 100644 --- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java +++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java @@ -20,7 +20,6 @@ import static org.apache.jackrabbit.guava.common.collect.Iterables.filter; import static org.apache.jackrabbit.guava.common.collect.Iterables.transform; -import static org.apache.jackrabbit.guava.common.collect.Maps.filterKeys; import static java.util.Collections.singletonList; import static org.apache.jackrabbit.oak.plugins.document.util.Utils.asISO8601; import static org.apache.jackrabbit.oak.plugins.document.Collection.JOURNAL; @@ -46,6 +45,7 @@ import org.apache.jackrabbit.guava.common.collect.Iterables; import org.apache.jackrabbit.oak.commons.TimeDurationFormatter; +import org.apache.jackrabbit.oak.commons.collections.CollectionUtils; import org.apache.jackrabbit.oak.commons.properties.SystemPropertySupplier; import org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor; import org.apache.jackrabbit.oak.plugins.document.cache.CacheInvalidationStats; @@ -726,7 +726,7 @@ private Revision determineLastModification(NodeDocument doc, int clusterId) { for (String property : doc.keySet().stream().filter(PROPERTY_OR_DELETED).collect(Collectors.toSet())) { Map valueMap = doc.getLocalMap(property); // collect committed changes of this cluster node - for (Map.Entry entry : filterKeys(valueMap, cp::test).entrySet()) { + for (Map.Entry entry : CollectionUtils.filterKeys(valueMap, cp).entrySet()) { Revision rev = entry.getKey(); String cv = revisionContext.getCommitValue(rev, doc); if (isCommitted(cv)) { diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java index 224c733ca49..a4e3eb1a576 100644 --- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java +++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java @@ -119,7 +119,6 @@ import static java.util.concurrent.TimeUnit.NANOSECONDS; import static java.util.stream.Collectors.toList; import static org.apache.jackrabbit.guava.common.collect.Iterables.filter; -import static org.apache.jackrabbit.guava.common.collect.Maps.filterKeys; import static com.mongodb.client.model.Projections.include; import static java.lang.Integer.MAX_VALUE; import static java.util.Collections.emptyList; @@ -1487,7 +1486,7 @@ private Map bulkUpdate(Collection collectio if (collection == Collection.NODES) { List docsToCache = new ArrayList<>(); - for (UpdateOp op : filterKeys(bulkOperations, x -> bulkResult.upserts.contains(x)).values()) { + for (UpdateOp op : CollectionUtils.filterKeys(bulkOperations, bulkResult.upserts::contains).values()) { NodeDocument doc = Collection.NODES.newDocument(this); UpdateUtils.applyChanges(doc, op); docsToCache.add(doc); From 2c0fe2df59ce0e3451b704c6ef47b16329b3e4f7 Mon Sep 17 00:00:00 2001 From: Rishabh Kumar Date: Wed, 8 Jan 2025 07:40:12 +0530 Subject: [PATCH 2/2] OAK-11350 : fixed test failures due to insertion order mismatch --- .../jackrabbit/oak/commons/collections/CollectionUtils.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java index 51287a58b35..d97ac059f58 100644 --- a/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java +++ b/oak-commons/src/main/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtils.java @@ -26,6 +26,7 @@ import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; @@ -433,8 +434,8 @@ public static Map filterKeys(final @NotNull Map map, final @No Objects.requireNonNull(predicate); return map.entrySet() .stream() - .filter(e -> predicate.test(e.getKey())) - .collect(HashMap::new, (m,v)->m.put(v.getKey(), v.getValue()), HashMap::putAll); + .filter(e -> predicate.test(e.getKey())) // using LinkedHashMap to maintain the order of previous map + .collect(LinkedHashMap::new, (m, v)->m.put(v.getKey(), v.getValue()), LinkedHashMap::putAll); } /**