Revert "FpKit now longer uses streams for performance reasons"

bbakerman · bbakerman · commit 262ff2f5a5fc · 2025-04-27T10:50:00.000+10:00
This reverts commit 5e7ce82.
diff --git a/src/main/java/graphql/util/FpKit.java b/src/main/java/graphql/util/FpKit.java
@@ -9,7 +9,6 @@
 import java.lang.reflect.Array;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -19,87 +18,69 @@
 import java.util.OptionalInt;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
+import java.util.function.BiFunction;
 import java.util.function.BinaryOperator;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Collections.singletonList;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.mapping;
 
 @Internal
 public class FpKit {
 
     //
     // From a list of named things, get a map of them by name, merging them according to the merge function
     public static <T> Map<String, T> getByName(List<T> namedObjects, Function<T, String> nameFn, BinaryOperator<T> mergeFunc) {
-        return toMap(namedObjects, nameFn, mergeFunc);
-    }
-
-    //
-    // From a collection of keyed things, get a map of them by key, merging them according to the merge function
-    public static <T, NewKey> Map<NewKey, T> toMap(Collection<T> collection, Function<T, NewKey> keyFunction, BinaryOperator<T> mergeFunc) {
-        Map<NewKey, T> resultMap = new LinkedHashMap<>();
-        for (T obj : collection) {
-            NewKey key = keyFunction.apply(obj);
-            if (resultMap.containsKey(key)) {
-                T existingValue = resultMap.get(key);
-                T mergedValue = mergeFunc.apply(existingValue, obj);
-                resultMap.put(key, mergedValue);
-            } else {
-                resultMap.put(key, obj);
-            }
-        }
-        return resultMap;
+        return namedObjects.stream().collect(Collectors.toMap(
+                nameFn,
+                identity(),
+                mergeFunc,
+                LinkedHashMap::new)
+        );
     }
 
     // normal groupingBy but with LinkedHashMap
     public static <T, NewKey> Map<NewKey, ImmutableList<T>> groupingBy(Collection<T> list, Function<T, NewKey> function) {
-        return filterAndGroupingBy(list, t -> true, function);
+        return list.stream().collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList())));
     }
 
-    @SuppressWarnings("unchecked")
     public static <T, NewKey> Map<NewKey, ImmutableList<T>> filterAndGroupingBy(Collection<T> list,
                                                                                 Predicate<? super T> predicate,
                                                                                 Function<T, NewKey> function) {
-        //
-        // The cleanest version of this code would have two maps, one of immutable list builders and one
-        // of the built immutable lists.  BUt we are trying to be performant and memory efficient so
-        // we treat it as a map of objects and cast like its Java 4x
-        //
-        Map<NewKey, Object> resutMap = new LinkedHashMap<>();
-        for (T item : list) {
-            if (predicate.test(item)) {
-                NewKey key = function.apply(item);
-                // we have to use an immutable list builder as we built it out
-                ((ImmutableList.Builder<Object>) resutMap.computeIfAbsent(key, k -> ImmutableList.builder()))
-                        .add(item);
-            }
-        }
-        if (resutMap.isEmpty()) {
-            return Collections.emptyMap();
-        }
-        // Convert builders to ImmutableLists in place to avoid an extra allocation
-        // yes the code is yuck -  but its more performant yuck!
-        resutMap.replaceAll((key, builder) ->
-                ((ImmutableList.Builder<Object>) builder).build());
-
-        // make it the right shape - like as if generics were never invented
-        return (Map<NewKey, ImmutableList<T>>) (Map<?, ?>) resutMap;
+        return list.stream().filter(predicate).collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList())));
     }
 
-    public static <T, NewKey> Map<NewKey, T> toMapByUniqueKey(Collection<T> list, Function<T, NewKey> keyFunction) {
-        return toMap(list, keyFunction, throwingMerger());
+    public static <T, NewKey> Map<NewKey, ImmutableList<T>> groupingBy(Stream<T> stream, Function<T, NewKey> function) {
+        return stream.collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList())));
     }
 
+    public static <T, NewKey> Map<NewKey, T> groupingByUniqueKey(Collection<T> list, Function<T, NewKey> keyFunction) {
+        return list.stream().collect(Collectors.toMap(
+                keyFunction,
+                identity(),
+                throwingMerger(),
+                LinkedHashMap::new)
+        );
+    }
 
-    private static final BinaryOperator<Object> THROWING_MERGER_SINGLETON = (u, v) -> {
-        throw new IllegalStateException(String.format("Duplicate key %s", u));
-    };
+    public static <T, NewKey> Map<NewKey, T> groupingByUniqueKey(Stream<T> stream, Function<T, NewKey> keyFunction) {
+        return stream.collect(Collectors.toMap(
+                keyFunction,
+                identity(),
+                throwingMerger(),
+                LinkedHashMap::new)
+        );
+    }
 
     private static <T> BinaryOperator<T> throwingMerger() {
-        //noinspection unchecked
-        return (BinaryOperator<T>) THROWING_MERGER_SINGLETON;
+        return (u, v) -> {
+            throw new IllegalStateException(String.format("Duplicate key %s", u));
+        };
     }
 
 
@@ -259,6 +240,11 @@ public static <T> List<T> valuesToList(Map<?, T> map) {
         return new ArrayList<>(map.values());
     }
 
+    public static <K, V, U> List<U> mapEntries(Map<K, V> map, BiFunction<K, V, U> function) {
+        return map.entrySet().stream().map(entry -> function.apply(entry.getKey(), entry.getValue())).collect(Collectors.toList());
+    }
+
+
     public static <T> List<List<T>> transposeMatrix(List<? extends List<T>> matrix) {
         int rowCount = matrix.size();
         int colCount = matrix.get(0).size();
@@ -286,12 +272,10 @@ public static <T> List<T> flatList(Collection<List<T>> listLists) {
     }
 
     public static <T> Optional<T> findOne(Collection<T> list, Predicate<T> filter) {
-        for (T t : list) {
-            if (filter.test(t)) {
-                return Optional.of(t);
-            }
-        }
-        return Optional.empty();
+        return list
+                .stream()
+                .filter(filter)
+                .findFirst();
     }
 
     public static <T> T findOneOrNull(List<T> list, Predicate<T> filter) {
@@ -308,13 +292,10 @@ public static <T> int findIndex(List<T> list, Predicate<T> filter) {
     }
 
     public static <T> List<T> filterList(Collection<T> list, Predicate<T> filter) {
-        List<T> result = new ArrayList<>();
-        for (T t : list) {
-            if (filter.test(t)) {
-                result.add(t);
-            }
-        }
-        return result;
+        return list
+                .stream()
+                .filter(filter)
+                .collect(Collectors.toList());
     }
 
     public static <T> Set<T> filterSet(Collection<T> input, Predicate<T> filter) {
@@ -371,10 +352,9 @@ public static <T> Supplier<T> interThreadMemoize(Supplier<T> delegate) {
     /**
      * Faster set intersection.
      *
-     * @param <T>  for two
+     * @param <T> for two
      * @param set1 first set
      * @param set2 second set
-     *
      * @return intersection set
      */
     public static <T> Set<T> intersection(Set<T> set1, Set<T> set2) {
diff --git a/src/main/java/graphql/util/NodeMultiZipper.java b/src/main/java/graphql/util/NodeMultiZipper.java
@@ -62,7 +62,7 @@ public T toRootNode() {
             Map<T, ImmutableList<NodeZipper<T>>> sameParent = zipperWithSameParent(deepestZippers);
 
             List<NodeZipper<T>> newZippers = new ArrayList<>();
-            Map<T, NodeZipper<T>> zipperByNode = FpKit.toMapByUniqueKey(curZippers, NodeZipper::getCurNode);
+            Map<T, NodeZipper<T>> zipperByNode = FpKit.groupingByUniqueKey(curZippers, NodeZipper::getCurNode);
             for (Map.Entry<T, ImmutableList<NodeZipper<T>>> entry : sameParent.entrySet()) {
                 NodeZipper<T> newZipper = moveUp(entry.getKey(), entry.getValue());
                 Optional<NodeZipper<T>> zipperToBeReplaced = Optional.ofNullable(zipperByNode.get(entry.getKey()));
diff --git a/src/test/groovy/graphql/util/FpKitTest.groovy b/src/test/groovy/graphql/util/FpKitTest.groovy
@@ -1,6 +1,6 @@
 package graphql.util
 
-import com.google.common.collect.ImmutableList
+
 import spock.lang.Specification
 
 import java.util.function.Supplier
@@ -98,100 +98,6 @@ class FpKitTest extends Specification {
         l == ["Parrot"]
     }
 
-    class Person {
-        String name
-        String city
-
-        Person(String name) {
-            this.name = name
-        }
-
-        Person(String name, String city) {
-            this.name = name
-            this.city = city
-        }
-
-        String getName() {
-            return name
-        }
-
-        String getCity() {
-            return city
-        }
-    }
-
-    def a = new Person("a", "New York")
-    def b = new Person("b", "New York")
-    def c1 = new Person("c", "Sydney")
-    def c2 = new Person("c", "London")
-
-    def "getByName tests"() {
-
-        when:
-        def map = FpKit.getByName([a, b, c1, c2], { it -> it.getName() })
-        then:
-        map == ["a": a, "b": b, c: c1]
-
-        when:
-        map = FpKit.getByName([a, b, c1, c2], { it -> it.getName() }, { it1, it2 -> it2 })
-        then:
-        map == ["a": a, "b": b, c: c2]
-    }
-
-    def "groupingBy tests"() {
-
-        when:
-        Map<String, ImmutableList<Person>> map = FpKit.groupingBy([a, b, c1, c2], { it -> it.getCity() })
-        then:
-        map == ["New York": [a, b], "Sydney": [c1], "London": [c2]]
-
-        when:
-        map = FpKit.filterAndGroupingBy([a, b, c1, c2], { it -> it != c1 }, { it -> it.getCity() })
-        then:
-        map == ["New York": [a, b], "London": [c2]]
-
-    }
-
-    def "toMapByUniqueKey works"() {
-
-        when:
-        Map<String, Person> map = FpKit.toMapByUniqueKey([a, b, c1], { it -> it.getName() })
-        then:
-        map == ["a": a, "b": b, "c": c1]
-
-        when:
-        FpKit.toMapByUniqueKey([a, b, c1, c2], { it -> it.getName() })
-        then:
-        def e = thrown(IllegalStateException.class)
-        e.message.contains("Duplicate key")
-    }
-
-    def "findOne test"() {
-        when:
-        def opt = FpKit.findOne([a, b, c1, c2], { it -> it.getName() == "c" })
-        then:
-        opt.isPresent()
-        opt.get() == c1
-
-        when:
-        opt = FpKit.findOne([a, b, c1, c2], { it -> it.getName() == "d" })
-        then:
-        opt.isEmpty()
-
-        when:
-        opt = FpKit.findOne([a, b, c1, c2], { it -> it.getName() == "a" })
-        then:
-        opt.isPresent()
-        opt.get() == a
-    }
-
-    def "filterList works"() {
-        when:
-        def list = FpKit.filterList([a, b, c1, c2], { it -> it.getName() == "c" })
-        then:
-        list == [c1, c2]
-    }
-
     def "set intersection works"() {
         def set1 = ["A","B","C"] as Set
         def set2 = ["A","C","D"] as Set
