From 24f39c29bd34dcd0e709ed5f9a2322b053ecec71 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Thu, 15 May 2025 16:57:26 +1000 Subject: [PATCH] Cherry pick 3932 to remove streams from FPKit --- src/main/java/graphql/util/FpKit.java | 127 ++++++++++-------- .../java/graphql/util/NodeMultiZipper.java | 2 +- src/test/groovy/graphql/util/FpKitTest.groovy | 96 ++++++++++++- 3 files changed, 166 insertions(+), 59 deletions(-) diff --git a/src/main/java/graphql/util/FpKit.java b/src/main/java/graphql/util/FpKit.java index a538450ad3..e82abcbd19 100644 --- a/src/main/java/graphql/util/FpKit.java +++ b/src/main/java/graphql/util/FpKit.java @@ -9,6 +9,7 @@ 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; @@ -18,17 +19,13 @@ 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 { @@ -36,51 +33,76 @@ 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 Map getByName(List namedObjects, Function nameFn, BinaryOperator mergeFunc) { - return namedObjects.stream().collect(Collectors.toMap( - nameFn, - identity(), - mergeFunc, - LinkedHashMap::new) - ); + 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 Map toMap(Collection collection, Function keyFunction, BinaryOperator mergeFunc) { + Map 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; } // normal groupingBy but with LinkedHashMap public static Map> groupingBy(Collection list, Function function) { - return list.stream().collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList()))); + return filterAndGroupingBy(list, ALWAYS_TRUE, function); } + @SuppressWarnings("unchecked") public static Map> filterAndGroupingBy(Collection list, Predicate predicate, Function function) { - return list.stream().filter(predicate).collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList()))); - } + // + // 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 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) 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) builder).build()); - public static Map> groupingBy(Stream stream, Function function) { - return stream.collect(Collectors.groupingBy(function, LinkedHashMap::new, mapping(Function.identity(), ImmutableList.toImmutableList()))); + // make it the right shape - like as if generics were never invented + return (Map>) (Map) resutMap; } - public static Map groupingByUniqueKey(Collection list, Function keyFunction) { - return list.stream().collect(Collectors.toMap( - keyFunction, - identity(), - throwingMerger(), - LinkedHashMap::new) - ); + public static Map toMapByUniqueKey(Collection list, Function keyFunction) { + return toMap(list, keyFunction, throwingMerger()); } - public static Map groupingByUniqueKey(Stream stream, Function keyFunction) { - return stream.collect(Collectors.toMap( - keyFunction, - identity(), - throwingMerger(), - LinkedHashMap::new) - ); - } + + private static final Predicate ALWAYS_TRUE = o -> true; + + private static final BinaryOperator THROWING_MERGER_SINGLETON = (u, v) -> { + throw new IllegalStateException(String.format("Duplicate key %s", u)); + }; + private static BinaryOperator throwingMerger() { - return (u, v) -> { - throw new IllegalStateException(String.format("Duplicate key %s", u)); - }; + //noinspection unchecked + return (BinaryOperator) THROWING_MERGER_SINGLETON; } @@ -240,11 +262,6 @@ public static List valuesToList(Map map) { return new ArrayList<>(map.values()); } - public static List mapEntries(Map map, BiFunction function) { - return map.entrySet().stream().map(entry -> function.apply(entry.getKey(), entry.getValue())).collect(Collectors.toList()); - } - - public static List> transposeMatrix(List> matrix) { int rowCount = matrix.size(); int colCount = matrix.get(0).size(); @@ -261,21 +278,13 @@ public static List> transposeMatrix(List> matrix) return result; } - public static CompletableFuture> flatList(CompletableFuture>> cf) { - return cf.thenApply(FpKit::flatList); - } - - public static List flatList(Collection> listLists) { - return listLists.stream() - .flatMap(List::stream) - .collect(ImmutableList.toImmutableList()); - } - public static Optional findOne(Collection list, Predicate filter) { - return list - .stream() - .filter(filter) - .findFirst(); + for (T t : list) { + if (filter.test(t)) { + return Optional.of(t); + } + } + return Optional.empty(); } public static T findOneOrNull(List list, Predicate filter) { @@ -292,10 +301,13 @@ public static int findIndex(List list, Predicate filter) { } public static List filterList(Collection list, Predicate filter) { - return list - .stream() - .filter(filter) - .collect(Collectors.toList()); + List result = new ArrayList<>(); + for (T t : list) { + if (filter.test(t)) { + result.add(t); + } + } + return result; } public static Set filterSet(Collection input, Predicate filter) { @@ -352,9 +364,10 @@ public static Supplier interThreadMemoize(Supplier delegate) { /** * Faster set intersection. * - * @param for two + * @param for two * @param set1 first set * @param set2 second set + * * @return intersection set */ public static Set intersection(Set set1, Set set2) { diff --git a/src/main/java/graphql/util/NodeMultiZipper.java b/src/main/java/graphql/util/NodeMultiZipper.java index fb67fe43ea..60328c1a2c 100644 --- a/src/main/java/graphql/util/NodeMultiZipper.java +++ b/src/main/java/graphql/util/NodeMultiZipper.java @@ -62,7 +62,7 @@ public T toRootNode() { Map>> sameParent = zipperWithSameParent(deepestZippers); List> newZippers = new ArrayList<>(); - Map> zipperByNode = FpKit.groupingByUniqueKey(curZippers, NodeZipper::getCurNode); + Map> zipperByNode = FpKit.toMapByUniqueKey(curZippers, NodeZipper::getCurNode); for (Map.Entry>> entry : sameParent.entrySet()) { NodeZipper newZipper = moveUp(entry.getKey(), entry.getValue()); Optional> 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 index 455e9b57a1..1996b06d55 100644 --- 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,6 +98,100 @@ 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> 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 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