OAK-11351 : removed usage of Guava's Maps.filterValues (#1959)

Co-authored-by: Rishabh Kumar <diam@adobe.com>
apache · Jan 9, 2025 · 91b7fcd · 91b7fcd
1 parent bcb6527
commit 91b7fcd
Show file tree

Hide file tree

Showing 6 changed files with 108 additions and 12 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
@@ -427,6 +427,7 @@ public static Map<String, String> fromProperties(final @NotNull Properties prope
      * @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
+     * @see CollectionUtils#filterValues(Map, Predicate)
      */
     @NotNull
     public static <K,V> Map<K, V> filterKeys(final @NotNull Map<K, V> map, final @NotNull Predicate<? super K> predicate) {
@@ -438,6 +439,28 @@ public static <K,V> Map<K, V> filterKeys(final @NotNull Map<K, V> map, final @No
                 .collect(LinkedHashMap::new, (m, v)->m.put(v.getKey(), v.getValue()), LinkedHashMap::putAll);
     }
 
+    /**
+     * Create a new {@link Map} after filtering the entries of the given map
+     * based on the specified predicate applied to the values.
+     *
+     * @param <K> the type of keys in the map
+     * @param <V> 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 values, must not be null
+     * @return a new map containing only the entries whose values match the predicate
+     * @throws NullPointerException if the map or predicate is null
+     * @see CollectionUtils#filterKeys(Map, Predicate)
+     */
+    @NotNull
+    public static <K,V> Map<K, V> filterValues(final @NotNull Map<K, V> map, final @NotNull Predicate<? super V> predicate) {
+        Objects.requireNonNull(map);
+        Objects.requireNonNull(predicate);
+        return map.entrySet()
+                .stream()
+                .filter(e -> predicate.test(e.getValue())) // using LinkedHashMap to maintain the order of previous map
+                .collect(LinkedHashMap::new, (m,v)->m.put(v.getKey(), v.getValue()), LinkedHashMap::putAll);
+    }
+
     /**
      * Convert an {@code Iterator} to an {@code Iterable}.
      * <p>

diff --git a/...mons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java b/...mons/src/test/java/org/apache/jackrabbit/oak/commons/collections/CollectionUtilsTest.java
@@ -634,6 +634,80 @@ public void testFilterKeysNullPredicate() {
         Assert.assertThrows(NullPointerException.class, () -> CollectionUtils.filterKeys(map, null));
     }
 
+    @Test
+    public void testFilterValues() {
+        final Map<Integer, String> map = new HashMap<>();
+        map.put(1, "one");
+        map.put(2, "two");
+        map.put(3, "three");
+
+        final Predicate<String> predicate = value -> value.startsWith("t");
+
+        final Map<Integer, String> result = CollectionUtils.filterValues(map, predicate);
+
+        Assert.assertEquals(2, result.size());
+        Assert.assertTrue(result.containsKey(2));
+        Assert.assertTrue(result.containsKey(3));
+        Assert.assertFalse(result.containsKey(1));
+    }
+
+    @Test
+    public void testFilterValuesEmptyMap() {
+        final Map<String, String> map = new HashMap<>();
+        final Predicate<String> predicate = key -> key.startsWith("t");
+
+        final Map<String, String> result = CollectionUtils.filterValues(map, predicate);
+
+        Assert.assertTrue(result.isEmpty());
+    }
+
+    @Test
+    public void testFilterNullValues() {
+        final Map<String, Integer> map = new HashMap<>();
+        map.put("one", 1);
+        map.put("two", 2);
+        map.put("three", 3);
+        map.put(null, null);
+
+        final Predicate<Integer> predicate = Objects::isNull;
+
+        final Map<String, Integer> result = CollectionUtils.filterValues(map, predicate);
+
+        Assert.assertEquals(1, result.size());
+    }
+
+    @Test
+    public void testFilterNonNullValues() {
+        final Map<String, Integer> map = new HashMap<>();
+        map.put("one", 1);
+        map.put("two", 2);
+        map.put("three", 3);
+        map.put(null, null);
+
+        final Predicate<Integer> predicate = Objects::nonNull;
+
+        final Map<String, Integer> result = CollectionUtils.filterValues(map, predicate);
+
+        Assert.assertEquals(3, result.size());
+    }
+
+    @Test
+    public void testFilterValuesNullMap() {
+        final Predicate<String> predicate = key -> key.startsWith("t");
+
+        Assert.assertThrows(NullPointerException.class, () -> CollectionUtils.filterKeys(null, predicate));
+    }
+
+    @Test
+    public void testFilterValuesNullPredicate() {
+        final Map<String, Integer> 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
@@ -23,10 +23,10 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.jackrabbit.guava.common.collect.Iterables;
-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;
@@ -186,7 +186,7 @@ public void leave(NodeState before, NodeState after) {
         if (!updates.isEmpty()) {
             Map<String, LuceneIndexNodeManager> builder = new HashMap<>();
             builder.putAll(CollectionUtils.filterKeys(original, x -> !updates.containsKey(x)));
-            builder.putAll(Maps.filterValues(updates, x -> x != null));
+            builder.putAll(CollectionUtils.filterValues(updates, Objects::nonNull));
             indices = Collections.unmodifiableMap(builder);
 
             badIndexTracker.markGoodIndexes(updates.keySet());

diff --git a/...n/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java b/...n/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexTracker.java
@@ -22,6 +22,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.jackrabbit.guava.common.collect.Iterables;
@@ -46,7 +47,6 @@
 
 import static java.util.Objects.requireNonNull;
 
-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;
 import static org.apache.jackrabbit.oak.plugins.index.search.IndexDefinition.STATUS_NODE;
@@ -149,7 +149,7 @@ public void leave(NodeState before, NodeState after) {
         if (!updates.isEmpty()) {
             Map<String, I> builder = new HashMap<>();
             builder.putAll(CollectionUtils.filterKeys(original, x -> !updates.containsKey(x)));
-            builder.putAll(filterValues(updates, x -> x != null));
+            builder.putAll(CollectionUtils.filterValues(updates, Objects::nonNull));
             indices = Collections.unmodifiableMap(builder);
 
             badIndexTracker.markGoodIndexes(updates.keySet());

diff --git a/.../src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java b/.../src/main/java/org/apache/jackrabbit/oak/plugins/document/memory/MemoryDocumentStore.java
@@ -32,7 +32,6 @@
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.jackrabbit.guava.common.base.Splitter;
-import org.apache.jackrabbit.guava.common.collect.Maps;
 import org.apache.jackrabbit.oak.cache.CacheStats;
 import org.apache.jackrabbit.oak.commons.properties.SystemPropertySupplier;
 import org.apache.jackrabbit.oak.plugins.document.Collection;
@@ -226,10 +225,10 @@ public <T extends Document> int remove(Collection<T> collection,
         Lock lock = rwLock.writeLock();
         lock.lock();
         try {
-            Maps.filterValues(map, doc -> {
-                    Long modified = Utils.asLong((Number) doc.get(indexedProperty));
-                    return startValue < modified && modified < endValue;
-                }).clear();
+            map.entrySet().removeIf(entry -> {
+                Long modified = Utils.asLong((Number) entry.getValue().get(indexedProperty));
+                return startValue < modified && modified < endValue;
+            });
         } finally {
             lock.unlock();
         }

diff --git a/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java b/oak-store-spi/src/main/java/org/apache/jackrabbit/oak/plugins/memory/ModifiedNodeState.java
@@ -20,7 +20,6 @@
 
 import static org.apache.jackrabbit.guava.common.collect.Iterables.concat;
 import static org.apache.jackrabbit.guava.common.collect.Iterables.filter;
-import static org.apache.jackrabbit.guava.common.collect.Maps.filterValues;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
@@ -33,6 +32,7 @@
 import java.util.function.Predicate;
 
 import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.commons.collections.CollectionUtils;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
@@ -215,7 +215,7 @@ static Iterable<String> getChildNodeNames(
             final Set<String> keys = nodes.keySet(); 
             return concat(
                     filter(base.getChildNodeNames(), x -> !keys.contains(x)),
-                    filterValues(nodes, NodeState.EXISTS::test).keySet());
+                    CollectionUtils.filterValues(nodes, NodeState.EXISTS).keySet());
         }
     }
 
@@ -348,7 +348,7 @@ public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
                     x -> !keys.contains(x == null ? null : x.getName());
             return concat(
                     filter(base.getChildNodeEntries(), predicate::test),
-                    iterable(filterValues(nodes, NodeState.EXISTS::test).entrySet()));
+                    iterable(CollectionUtils.filterValues(nodes, NodeState.EXISTS).entrySet()));
         }
     }