From e25559d3c33b10029395de0a9037d454418da5ea Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 19 Jun 2023 20:14:17 -0500 Subject: [PATCH 1/4] Tests for unmodifiable Collectors methods --- .../google/gwt/emultest/EmulJava10Suite.java | 2 + .../java10/util/stream/CollectorsTest.java | 172 ++++++++++++++++++ 2 files changed, 174 insertions(+) create mode 100644 user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java diff --git a/user/test/com/google/gwt/emultest/EmulJava10Suite.java b/user/test/com/google/gwt/emultest/EmulJava10Suite.java index e691477626..2cd6369bb8 100644 --- a/user/test/com/google/gwt/emultest/EmulJava10Suite.java +++ b/user/test/com/google/gwt/emultest/EmulJava10Suite.java @@ -19,6 +19,7 @@ import com.google.gwt.emultest.java10.util.OptionalIntTest; import com.google.gwt.emultest.java10.util.OptionalLongTest; import com.google.gwt.emultest.java10.util.OptionalTest; +import com.google.gwt.emultest.java10.util.stream.CollectorsTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; import org.junit.runners.Suite.SuiteClasses; @@ -26,6 +27,7 @@ /** Test JRE emulations. */ @RunWith(Suite.class) @SuiteClasses({ + CollectorsTest.class, OptionalDoubleTest.class, OptionalIntTest.class, OptionalLongTest.class, diff --git a/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java b/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java new file mode 100644 index 0000000000..6c34a08901 --- /dev/null +++ b/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java @@ -0,0 +1,172 @@ +/* + * Copyright 2023 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.emultest.java10.util.stream; + +import static com.google.gwt.emultest.java8.util.stream.CollectorsTest.applyItems; +import static java.util.stream.Collectors.toUnmodifiableList; +import static java.util.stream.Collectors.toUnmodifiableMap; +import static java.util.stream.Collectors.toUnmodifiableSet; + +import com.google.gwt.emultest.java.util.EmulTestBase; + +import java.util.Collection; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Stream; + +/** + * Tests for java.util.stream.Collectors Java 10 API emulation. + */ +public class CollectorsTest extends EmulTestBase { + private static boolean unmodifiableCollection(Collection c, T existingSample, T newSample) { + try { + c.remove(existingSample); + return false; + } catch (Exception ignore) { + // expected + } + try { + c.add(newSample); + return false; + } catch (Exception ignore) { + // expected + } + Iterator itr = c.iterator(); + itr.next(); + try { + itr.remove(); + return false; + } catch (Exception e) { + // expected + } + return true; + } + + public void testToUnmodifiableList() { + applyItems(List.of("a", "b"), toUnmodifiableList(), "a", "b", (a, b) -> { + if (!a.equals(b)) { + return false; + } + + // check both, so it is obvious we got the right one + if (!unmodifiableCollection(a, "a", "z")) { + return false; + } + if (!unmodifiableCollection(b, "a", "z")) { + return false; + } + + return true; + }); + + // verify nulls fail + try { + Stream.of("a").map(ignore -> null).collect(toUnmodifiableList()); + fail("Expected NPE"); + } catch (NullPointerException ignore) { + // expected + } + } + + public void testToUnmodifiableMap() { + // verify simple cases copy all values and results are unmodifiable + applyItems(Map.of("a", 0, "b", 1), toUnmodifiableMap(Function.identity(), + k -> k.charAt(0) - 'a'), "a", "b", (a, b) -> { + if (!a.equals(b)) { + return false; + } + + // check both, so it is obvious we got the right one + if (!unmodifiableMap(a, "a", 0, "z", 100)) { + return false; + } + if (!unmodifiableMap(b, "a", 0, "z", 100)) { + return false; + } + + return true; + }); + + // verify merge works with only one key (but this is just passing through to the toMap func anyway...) + applyItems(Map.of("a", 2), toUnmodifiableMap(Function.identity(), ignore -> 1, Integer::sum), + "a", "a"); + + // verify nulls blow up for both keys and values + try { + Stream.of("a").collect(toUnmodifiableMap(obj -> null, Function.identity())); + fail("Expected NPE"); + } catch (NullPointerException ignore) { + // expected + } + try { + Stream.of("a").collect(toUnmodifiableMap(Function.identity(), obj -> null)); + fail("Expected NPE"); + } catch (Exception ignore) { + // expected + } + } + + private boolean unmodifiableMap(Map a, K existingKey, V existingValue, K newKey, + V newValue) { + if (!unmodifiableCollection(a.keySet(), existingKey, newKey)) { + return false; + } + if (!unmodifiableCollection(a.values(), existingValue, newValue)) { + return false; + } + + try { + a.put(newKey, newValue); + return false; + } catch (Exception ignore) { + // expected + } + try { + a.remove(existingKey); + return false; + } catch (Exception ignore) { + // expected + } + + return true; + } + + public void testToUnmodifiableSet() { + applyItems(Set.of("a", "b"), toUnmodifiableSet(), "a", "b", (a, b) -> { + if (!a.equals(b)) { + return false; + } + if (!unmodifiableCollection(a, "a", "z")) { + return false; + } + if (!unmodifiableCollection(b, "a", "z")) { + return false; + } + return true; + }); + + // verify nulls fail + try { + Stream.of("a").map(ignore -> null).collect(toUnmodifiableSet()); + fail("Expected NPE"); + } catch (NullPointerException ignore) { + // expected + } + } +} From da3c4916e8111476ef4d01c67c3426854cd8e041 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Fri, 23 Jun 2023 19:30:44 -0500 Subject: [PATCH 2/4] Added missing impls, stuck on another branch --- .../gwt/emul/java/util/stream/Collectors.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java index 60d908edb9..27983def0b 100644 --- a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java +++ b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java @@ -339,6 +339,11 @@ public static Collector> toList() { return toCollection(ArrayList::new); } + public static Collector> toUnmodifiableList() { + Collector> mapping = mapping(Objects::requireNonNull, toList()); + return collectingAndThen(mapping, Collections::unmodifiableList); + } + public static Collector> toMap( final Function keyMapper, final Function valueMapper) { @@ -357,6 +362,27 @@ public static Collector> toList() { return toMap(keyMapper, valueMapper, mergeFunction, HashMap::new); } + public static Collector> toUnmodifiableMap(Function keyMapper, Function valueMapper) { + return collectingAndThen( + toMap(disallowNulls(keyMapper), disallowNulls(valueMapper)), + Collections::unmodifiableMap + ); + } + + public static Collector> toUnmodifiableMap(Function keyMapper, Function valueMapper, BinaryOperator + mergeFunction) { + return collectingAndThen( + toMap(disallowNulls(keyMapper), disallowNulls(valueMapper), mergeFunction), + Collections::unmodifiableMap + ); + } + + private static Function disallowNulls(Function func) { + return func.andThen(Objects::requireNonNull); + } + public static > Collector toMap( final Function keyMapper, final Function valueMapper, @@ -389,6 +415,11 @@ public static Collector> toSet() { ); } + public static Collector> toUnmodifiableSet() { + Collector> mapping = mapping(Objects::requireNonNull, toSet()); + return collectingAndThen(mapping, Collections::unmodifiableSet); + } + private static D streamAndCollect(Collector downstream, List list) { A a = downstream.supplier().get(); for (T t : list) { From 62e0d87db60e13f0613814eed7a0c60ea0760535 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 28 Jun 2023 13:35:42 -0500 Subject: [PATCH 3/4] Add extra imports added/removed upstream --- user/super/com/google/gwt/emul/java/util/stream/Collectors.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java index 27983def0b..cae4eada5f 100644 --- a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java +++ b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java @@ -18,6 +18,7 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.DoubleSummaryStatistics; import java.util.HashMap; @@ -27,6 +28,7 @@ import java.util.List; import java.util.LongSummaryStatistics; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.StringJoiner; From 0ea22cf3000c90eca653fbb6b7b3de1895343afd Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 23 Dec 2023 11:48:40 -0600 Subject: [PATCH 4/4] Review feedback --- .../gwt/emul/java/util/stream/Collectors.java | 12 +++--- .../java10/util/stream/CollectorsTest.java | 40 +++++++------------ 2 files changed, 22 insertions(+), 30 deletions(-) diff --git a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java index cae4eada5f..0509fb2fe8 100644 --- a/user/super/com/google/gwt/emul/java/util/stream/Collectors.java +++ b/user/super/com/google/gwt/emul/java/util/stream/Collectors.java @@ -364,17 +364,19 @@ public static Collector> toList() { return toMap(keyMapper, valueMapper, mergeFunction, HashMap::new); } - public static Collector> toUnmodifiableMap(Function keyMapper, Function valueMapper) { + public static Collector> toUnmodifiableMap( + Function keyMapper, + Function valueMapper) { return collectingAndThen( toMap(disallowNulls(keyMapper), disallowNulls(valueMapper)), Collections::unmodifiableMap ); } - public static Collector> toUnmodifiableMap(Function keyMapper, Function valueMapper, BinaryOperator - mergeFunction) { + public static Collector> toUnmodifiableMap( + Function keyMapper, + Function valueMapper, + BinaryOperator mergeFunction) { return collectingAndThen( toMap(disallowNulls(keyMapper), disallowNulls(valueMapper), mergeFunction), Collections::unmodifiableMap diff --git a/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java b/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java index 6c34a08901..ecf6654f19 100644 --- a/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java +++ b/user/test/com/google/gwt/emultest/java10/util/stream/CollectorsTest.java @@ -38,13 +38,13 @@ private static boolean unmodifiableCollection(Collection c, T existingSam try { c.remove(existingSample); return false; - } catch (Exception ignore) { + } catch (UnsupportedOperationException ignore) { // expected } try { c.add(newSample); return false; - } catch (Exception ignore) { + } catch (UnsupportedOperationException ignore) { // expected } Iterator itr = c.iterator(); @@ -52,23 +52,19 @@ private static boolean unmodifiableCollection(Collection c, T existingSam try { itr.remove(); return false; - } catch (Exception e) { + } catch (UnsupportedOperationException e) { // expected } return true; } public void testToUnmodifiableList() { - applyItems(List.of("a", "b"), toUnmodifiableList(), "a", "b", (a, b) -> { - if (!a.equals(b)) { + applyItems(List.of("a", "b"), toUnmodifiableList(), "a", "b", (expected, actual) -> { + if (!expected.equals(actual)) { return false; } - // check both, so it is obvious we got the right one - if (!unmodifiableCollection(a, "a", "z")) { - return false; - } - if (!unmodifiableCollection(b, "a", "z")) { + if (!unmodifiableCollection(actual, "a", "z")) { return false; } @@ -87,23 +83,20 @@ public void testToUnmodifiableList() { public void testToUnmodifiableMap() { // verify simple cases copy all values and results are unmodifiable applyItems(Map.of("a", 0, "b", 1), toUnmodifiableMap(Function.identity(), - k -> k.charAt(0) - 'a'), "a", "b", (a, b) -> { - if (!a.equals(b)) { + k -> k.charAt(0) - 'a'), "a", "b", (expected, actual) -> { + if (!expected.equals(actual)) { return false; } - // check both, so it is obvious we got the right one - if (!unmodifiableMap(a, "a", 0, "z", 100)) { - return false; - } - if (!unmodifiableMap(b, "a", 0, "z", 100)) { + if (!unmodifiableMap(actual, "a", 0, "z", 100)) { return false; } return true; }); - // verify merge works with only one key (but this is just passing through to the toMap func anyway...) + // verify merge works with only one key (but this is just passing through to the toMap func + // anyway...) applyItems(Map.of("a", 2), toUnmodifiableMap(Function.identity(), ignore -> 1, Integer::sum), "a", "a"); @@ -123,7 +116,7 @@ public void testToUnmodifiableMap() { } private boolean unmodifiableMap(Map a, K existingKey, V existingValue, K newKey, - V newValue) { + V newValue) { if (!unmodifiableCollection(a.keySet(), existingKey, newKey)) { return false; } @@ -148,14 +141,11 @@ private boolean unmodifiableMap(Map a, K existingKey, V existingVal } public void testToUnmodifiableSet() { - applyItems(Set.of("a", "b"), toUnmodifiableSet(), "a", "b", (a, b) -> { - if (!a.equals(b)) { - return false; - } - if (!unmodifiableCollection(a, "a", "z")) { + applyItems(Set.of("a", "b"), toUnmodifiableSet(), "a", "b", (expected, actual) -> { + if (!expected.equals(actual)) { return false; } - if (!unmodifiableCollection(b, "a", "z")) { + if (!unmodifiableCollection(actual, "a", "z")) { return false; } return true;