From ecd5246a3ecaa90a37156d7e4b851b171331ff70 Mon Sep 17 00:00:00 2001 From: iamdanfox Date: Thu, 23 May 2024 15:27:35 +0100 Subject: [PATCH] MoreCollectors.toImmutableMap() convenience method (#328) --- README.md | 22 +++++ changelog/@unreleased/pr-328.v2.yml | 5 ++ .../common/streams/MoreCollectors.java | 12 +++ .../common/streams/MoreCollectorsTests.java | 82 ++++++++++++++----- 4 files changed, 102 insertions(+), 19 deletions(-) create mode 100644 changelog/@unreleased/pr-328.v2.yml diff --git a/README.md b/README.md index 512fd12..26daf04 100644 --- a/README.md +++ b/README.md @@ -66,3 +66,25 @@ the order in which they complete. [ListenableFuture]: https://google.github.io/guava/releases/23.0/api/docs/com/google/common/util/concurrent/ListenableFuture.html [Map]: https://docs.oracle.com/javase/8/docs/api/java/util/Map.html [Stream]: https://docs.oracle.com/javase/8/docs/api/java/util/stream/Stream.html + +## MoreCollectors + +### `MoreCollectors.toImmutableMap()` + +Collect a Stream of Map.Entry (e.g. a StreamEx EntryStream) into a Guava ImmutableMap, which preserves iteration order. + +Beware that using StreamEx's `EntryStream#toImmutableMap()` does NOT preserve iteration order, as it uses a regular HashMap under the hood. + +```diff + StreamEx.of(items) + .mapToEntry( + key -> computeValue(key.foo()) +- .toImmutableMap() // does not preserve iteration order ++ .collect(MoreCollectors.toImmutableMap()); // preserves iteration order +``` + +This is equivalent to writing out the slightly more verbose: + +```java + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); +``` diff --git a/changelog/@unreleased/pr-328.v2.yml b/changelog/@unreleased/pr-328.v2.yml new file mode 100644 index 0000000..c3f6c68 --- /dev/null +++ b/changelog/@unreleased/pr-328.v2.yml @@ -0,0 +1,5 @@ +type: feature +feature: + description: MoreCollectors.toImmutableMap() convenience method + links: + - https://github.com/palantir/streams/pull/328 diff --git a/streams/src/main/java/com/palantir/common/streams/MoreCollectors.java b/streams/src/main/java/com/palantir/common/streams/MoreCollectors.java index acb2eeb..1e7a75b 100644 --- a/streams/src/main/java/com/palantir/common/streams/MoreCollectors.java +++ b/streams/src/main/java/com/palantir/common/streams/MoreCollectors.java @@ -61,6 +61,18 @@ public final class MoreCollectors { ImmutableList.Builder::build); } + /** + * Collect a Stream of Map.Entry (e.g. a StreamEx EntryStream) into a Guava {@link ImmutableMap}, which preserves + * iteration order. Duplicate keys will result in an error. Throws NullPointerException if any key or value is null. + * + * For behaviour details, see docs on {@link ImmutableMap#toImmutableMap}. + * + * Beware that {@code EntryStream#toImmutableMap()} does NOT preserve iteration order, as it uses a regular HashMap. + */ + public static Collector, ?, ImmutableMap> toImmutableMap() { + return ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue); + } + /** * This collector has similar semantics to {@link Collectors#toMap} except that the resulting map will be * immutable. Duplicate keys will result in an error. diff --git a/streams/src/test/java/com/palantir/common/streams/MoreCollectorsTests.java b/streams/src/test/java/com/palantir/common/streams/MoreCollectorsTests.java index ba4dacb..0c5dcc4 100644 --- a/streams/src/test/java/com/palantir/common/streams/MoreCollectorsTests.java +++ b/streams/src/test/java/com/palantir/common/streams/MoreCollectorsTests.java @@ -18,13 +18,16 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.google.common.collect.Maps; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; public class MoreCollectorsTests { @@ -60,27 +63,68 @@ public void test_parallel_immutable_set() { private Function valueMap = x -> x * 2; - @Test - public void test_immutable_map() { - Map map = LARGE_LIST.stream().collect(MoreCollectors.toImmutableMap(k -> k, valueMap)); - assertThat(map.keySet()).containsExactlyElementsOf(LARGE_LIST); - map.forEach((k, _v) -> assertThat(map.get(k)).isEqualTo(valueMap.apply(k))); - } + @Nested + class ToImmutableMapDeprecated { + @Test + public void test_immutable_map() { + Map map = LARGE_LIST.stream().collect(MoreCollectors.toImmutableMap(k -> k, valueMap)); + assertThat(map.keySet()).containsExactlyElementsOf(LARGE_LIST); + map.forEach((k, _v) -> assertThat(map.get(k)).isEqualTo(valueMap.apply(k))); + } - @Test - @SuppressWarnings("DangerousParallelStreamUsage") // explicitly testing parallel streams - public void test_parallel_immutable_map() { - Map map = - LARGE_LIST.parallelStream().collect(MoreCollectors.toImmutableMap(k -> k, valueMap)); - assertThat(map.keySet()).containsExactlyElementsOf(LARGE_LIST); - map.forEach((k, _v) -> assertThat(map.get(k)).isEqualTo(valueMap.apply(k))); + @Test + @SuppressWarnings("DangerousParallelStreamUsage") // explicitly testing parallel streams + public void test_parallel_immutable_map() { + Map map = + LARGE_LIST.parallelStream().collect(MoreCollectors.toImmutableMap(k -> k, valueMap)); + assertThat(map.keySet()).containsExactlyElementsOf(LARGE_LIST); + map.forEach((k, _v) -> assertThat(map.get(k)).isEqualTo(valueMap.apply(k))); + } + + @Test + public void test_immutable_map_duplicate_keys() { + Stream stream = Stream.of(1, 1); + assertThatThrownBy(() -> stream.collect(MoreCollectors.toImmutableMap(k -> k, _k -> 2))) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Multiple entries with same key: 1=2 and 1=2"); + } } - @Test - public void test_immutable_map_duplicate_keys() { - Stream stream = Stream.of(1, 1); - assertThatThrownBy(() -> stream.collect(MoreCollectors.toImmutableMap(k -> k, _k -> 2))) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Multiple entries with same key: 1=2 and 1=2"); + @Nested + class ToImmutableMap { + + @Test + void toImmutableMap_preserves_iteration_order() { + Map map = LARGE_LIST.stream() + .map(i -> Maps.immutableEntry(i, valueMap.apply(i))) + .collect(MoreCollectors.toImmutableMap()); + assertThat(map.keySet()).containsExactlyElementsOf(LARGE_LIST); + map.forEach((k, _v) -> assertThat(map.get(k)).isEqualTo(valueMap.apply(k))); + } + + @Test + public void toImmutableMap_throws_on_duplicate_keys() { + AtomicInteger counter = new AtomicInteger(888); + Stream> stream = + Stream.of(1, 1).map(i -> Maps.immutableEntry(i, counter.getAndIncrement())); + + assertThatThrownBy(() -> stream.collect(MoreCollectors.toImmutableMap())) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Multiple entries with same key: 1=889 and 1=888"); + } + + @Test + void throws_if_keys_are_null() { + assertThatThrownBy(() -> Stream.of(Maps.immutableEntry(null, 1)).collect(MoreCollectors.toImmutableMap())) + .isInstanceOf(NullPointerException.class) + .hasMessage("null key in entry: null=1"); + } + + @Test + void throws_if_values_are_null() { + assertThatThrownBy(() -> Stream.of(Maps.immutableEntry(1, null)).collect(MoreCollectors.toImmutableMap())) + .isInstanceOf(NullPointerException.class) + .hasMessage("null value in entry: 1=null"); + } } }