From deceb867c5afdc6d185171c4eb1f8e3582249a9c Mon Sep 17 00:00:00 2001 From: Oliver Drotbohm Date: Tue, 3 May 2022 13:32:52 +0200 Subject: [PATCH] Support for Eclipse Collections. Using the newly introduced CustomCollectionsRegistrar SPI to provide support for Eclipse Collections (Imm|M)utable(List|Set|Bag|Map). Fixes #2618. --- pom.xml | 17 ++ .../data/util/CustomCollections.java | 244 ++++++++++++++++++ src/main/resources/META-INF/spring.factories | 3 +- .../query/QueryMethodUnitTests.java | 13 +- .../data/util/CustomCollectionsUnitTests.java | 107 +++++++- 5 files changed, 381 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index de35c8364..b917e8389 100644 --- a/pom.xml +++ b/pom.xml @@ -18,6 +18,7 @@ 2.0.6 0.10.4 + 11.0.0 2.11.7 1.4.23 @@ -160,6 +161,22 @@ ${vavr} true + + + + + org.eclipse.collections + eclipse-collections-api + ${eclipse-collections} + true + + + + org.eclipse.collections + eclipse-collections + ${eclipse-collections} + test + diff --git a/src/main/java/org/springframework/data/util/CustomCollections.java b/src/main/java/org/springframework/data/util/CustomCollections.java index 4764cbc6c..cbdc51d43 100644 --- a/src/main/java/org/springframework/data/util/CustomCollections.java +++ b/src/main/java/org/springframework/data/util/CustomCollections.java @@ -33,8 +33,24 @@ import java.util.function.Function; import java.util.function.Supplier; import java.util.stream.Collectors; +import org.eclipse.collections.api.RichIterable; +import org.eclipse.collections.api.bag.ImmutableBag; +import org.eclipse.collections.api.bag.MutableBag; +import org.eclipse.collections.api.factory.Bags; +import org.eclipse.collections.api.factory.Lists; +import org.eclipse.collections.api.factory.Maps; +import org.eclipse.collections.api.factory.Sets; +import org.eclipse.collections.api.list.ImmutableList; +import org.eclipse.collections.api.list.MutableList; +import org.eclipse.collections.api.map.ImmutableMap; +import org.eclipse.collections.api.map.MapIterable; +import org.eclipse.collections.api.map.MutableMap; +import org.eclipse.collections.api.set.ImmutableSet; +import org.eclipse.collections.api.set.MutableSet; import org.springframework.core.convert.TypeDescriptor; +import org.springframework.core.convert.converter.ConditionalConverter; import org.springframework.core.convert.converter.ConditionalGenericConverter; +import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.converter.ConverterRegistry; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.lang.NonNull; @@ -486,4 +502,232 @@ public class CustomCollections { } } } + + static class EclipseCollections implements CustomCollectionRegistrar { + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#isAvailable() + */ + @Override + public boolean isAvailable() { + return ClassUtils.isPresent("org.eclipse.collections.api.list.ImmutableList", + EclipseCollections.class.getClassLoader()); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#getCollectionTypes() + */ + @Override + public Collection> getCollectionTypes() { + return List.of(ImmutableList.class, ImmutableSet.class, ImmutableBag.class, MutableList.class, MutableSet.class, + MutableBag.class); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#getMapTypes() + */ + @Override + public Collection> getMapTypes() { + return List.of(ImmutableMap.class, MutableMap.class); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#getAllowedPaginationReturnTypes() + */ + @Override + public Collection> getAllowedPaginationReturnTypes() { + return List.of(ImmutableList.class, MutableList.class); + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#toJavaNativeCollection() + */ + @Override + public Function toJavaNativeCollection() { + + return source -> source instanceof RichIterable + ? EclipseToJavaConverter.INSTANCE.convert(source) + : source; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.util.CustomCollectionRegistrar#registerConvertersIn(org.springframework.core.convert.converter.ConverterRegistry) + */ + @Override + public void registerConvertersIn(ConverterRegistry registry) { + + registry.addConverter(EclipseToJavaConverter.INSTANCE); + registry.addConverter(JavaToEclipseConverter.INSTANCE); + } + + enum EclipseToJavaConverter implements Converter, ConditionalConverter { + + INSTANCE; + + private static final TypeDescriptor RICH_ITERABLE_DESCRIPTOR = TypeDescriptor.valueOf(RichIterable.class); + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.ConditionalConverter#matches(org.springframework.core.convert.TypeDescriptor, org.springframework.core.convert.TypeDescriptor) + */ + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + + return sourceType.isAssignableTo(RICH_ITERABLE_DESCRIPTOR) + && COLLECTIONS_AND_MAP.contains(targetType.getType()); + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.Converter#convert(java.lang.Object) + */ + @Nullable + @Override + public Object convert(@Nullable Object source) { + + if (source instanceof ImmutableList) { + return ((ImmutableList) source).toList(); + } + + if (source instanceof ImmutableBag) { + return ((ImmutableBag) source).toList(); + } + + if (source instanceof ImmutableSet) { + return ((ImmutableSet) source).toSet(); + } + + if (source instanceof ImmutableMap) { + return ((ImmutableMap) source).toMap(); + } + + return source; + } + } + + enum JavaToEclipseConverter implements ConditionalGenericConverter { + + INSTANCE; + + private static final Set CONVERTIBLE_PAIRS; + + static { + + var pairs = new HashSet(); + pairs.add(new ConvertiblePair(Collection.class, RichIterable.class)); + + pairs.add(new ConvertiblePair(Set.class, MutableSet.class)); + pairs.add(new ConvertiblePair(Set.class, MutableList.class)); + pairs.add(new ConvertiblePair(Set.class, ImmutableSet.class)); + pairs.add(new ConvertiblePair(Set.class, ImmutableList.class)); + + pairs.add(new ConvertiblePair(List.class, MutableList.class)); + pairs.add(new ConvertiblePair(List.class, ImmutableList.class)); + + pairs.add(new ConvertiblePair(Map.class, RichIterable.class)); + pairs.add(new ConvertiblePair(Map.class, MutableMap.class)); + pairs.add(new ConvertiblePair(Map.class, ImmutableMap.class)); + + CONVERTIBLE_PAIRS = Collections.unmodifiableSet(pairs); + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.GenericConverter#getConvertibleTypes() + */ + @NonNull + @Override + public Set getConvertibleTypes() { + return CONVERTIBLE_PAIRS; + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.ConditionalConverter#matches(org.springframework.core.convert.TypeDescriptor, org.springframework.core.convert.TypeDescriptor) + */ + @Override + public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) { + + // Prevent collections to be mapped to maps + if (sourceType.isCollection() && MapIterable.class.isAssignableFrom(targetType.getType())) { + return false; + } + + // Prevent maps to be mapped to collections + if (sourceType.isMap() // + && !(MapIterable.class.isAssignableFrom(targetType.getType()) + || targetType.getType().equals(RichIterable.class))) { + return false; + } + + return true; + } + + /* + * (non-Javadoc) + * @see org.springframework.core.convert.converter.GenericConverter#convert(java.lang.Object, org.springframework.core.convert.TypeDescriptor, org.springframework.core.convert.TypeDescriptor) + */ + @Nullable + @Override + public Object convert(@Nullable Object source, TypeDescriptor sourceDescriptor, TypeDescriptor targetDescriptor) { + + var targetType = targetDescriptor.getType(); + + if (ImmutableList.class.isAssignableFrom(targetType)) { + return Lists.immutable.ofAll((Iterable) source); + } + + if (ImmutableSet.class.isAssignableFrom(targetType)) { + return Sets.immutable.ofAll((Iterable) source); + } + + if (ImmutableBag.class.isAssignableFrom(targetType)) { + return Bags.immutable.ofAll((Iterable) source); + } + + if (ImmutableMap.class.isAssignableFrom(targetType)) { + return Maps.immutable.ofAll((Map) source); + } + + if (MutableList.class.isAssignableFrom(targetType)) { + return Lists.mutable.ofAll((Iterable) source); + } + + if (MutableSet.class.isAssignableFrom(targetType)) { + return Sets.mutable.ofAll((Iterable) source); + } + + if (MutableBag.class.isAssignableFrom(targetType)) { + return Bags.mutable.ofAll((Iterable) source); + } + + if (MutableMap.class.isAssignableFrom(targetType)) { + return Maps.mutable.ofMap((Map) source); + } + + // No dedicated type asked for, probably RichIterable. + // Try to stay as close to the source value. + + if (source instanceof List) { + return Lists.mutable.ofAll((Iterable) source); + } + + if (source instanceof Set) { + return Sets.mutable.ofAll((Iterable) source); + } + + if (source instanceof Map) { + return Maps.mutable.ofMap((Map) source); + } + + return source; + } + } + } } diff --git a/src/main/resources/META-INF/spring.factories b/src/main/resources/META-INF/spring.factories index 45535f3c8..fc6274377 100644 --- a/src/main/resources/META-INF/spring.factories +++ b/src/main/resources/META-INF/spring.factories @@ -1,2 +1,3 @@ org.springframework.data.web.config.SpringDataJacksonModules=org.springframework.data.web.config.SpringDataJacksonConfiguration -org.springframework.data.util.CustomCollectionRegistrar=org.springframework.data.util.CustomCollections.VavrCollections +org.springframework.data.util.CustomCollectionRegistrar=org.springframework.data.util.CustomCollections.VavrCollections, \ + org.springframework.data.util.CustomCollections.EclipseCollections diff --git a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java b/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java index 287873225..6c06c88bc 100755 --- a/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java +++ b/src/test/java/org/springframework/data/repository/query/QueryMethodUnitTests.java @@ -27,8 +27,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.Future; import java.util.stream.Stream; +import org.eclipse.collections.api.list.ImmutableList; import org.junit.jupiter.api.Test; - import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Slice; @@ -248,6 +248,15 @@ class QueryMethodUnitTests { assertThat(returnedType.getDomainType()).isEqualTo(User.class); } + @Test // #1817 + void considersEclipseCollectionCollectionQuery() throws Exception { + + var method = SampleRepository.class.getMethod("returnsEclipseCollection"); + var queryMethod = new QueryMethod(method, metadata, factory); + + assertThat(queryMethod.isCollectionQuery()).isTrue(); + } + interface SampleRepository extends Repository { String pagingMethodWithInvalidReturnType(Pageable pageable); @@ -294,6 +303,8 @@ class QueryMethodUnitTests { Future> returnsFutureOfOption(); Mono> reactiveSlice(); + + ImmutableList returnsEclipseCollection(); } class User { diff --git a/src/test/java/org/springframework/data/util/CustomCollectionsUnitTests.java b/src/test/java/org/springframework/data/util/CustomCollectionsUnitTests.java index fca4864e7..fbe4b157d 100644 --- a/src/test/java/org/springframework/data/util/CustomCollectionsUnitTests.java +++ b/src/test/java/org/springframework/data/util/CustomCollectionsUnitTests.java @@ -27,6 +27,23 @@ import java.util.Map; import java.util.Set; import java.util.stream.Stream; +import org.eclipse.collections.api.RichIterable; +import org.eclipse.collections.api.bag.ImmutableBag; +import org.eclipse.collections.api.bag.MutableBag; +import org.eclipse.collections.api.factory.Lists; +import org.eclipse.collections.api.factory.Maps; +import org.eclipse.collections.api.factory.Sets; +import org.eclipse.collections.api.list.ImmutableList; +import org.eclipse.collections.api.list.ListIterable; +import org.eclipse.collections.api.list.MutableList; +import org.eclipse.collections.api.map.ImmutableMap; +import org.eclipse.collections.api.map.MapIterable; +import org.eclipse.collections.api.map.MutableMap; +import org.eclipse.collections.api.set.ImmutableSet; +import org.eclipse.collections.api.set.MutableSet; +import org.eclipse.collections.api.set.SetIterable; +import org.eclipse.collections.impl.map.immutable.ImmutableUnifiedMap; +import org.eclipse.collections.impl.map.mutable.UnifiedMap; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.Test; @@ -61,6 +78,17 @@ class CustomCollectionsUnitTests { .verify(); } + @TestFactory // #1817 + Stream registersEclipseCollections() { + + return new CustomCollectionTester() + .withCollections(ImmutableSet.class, ImmutableList.class, ImmutableBag.class, MutableSet.class, + MutableList.class, MutableBag.class) + .withMaps(ImmutableMap.class, MutableMap.class) + .withMapImplementations(ImmutableUnifiedMap.class, UnifiedMap.class) + .verify(); + } + @Test // DATACMNS-1065, #1817 void conversListToVavr() { @@ -106,6 +134,63 @@ class CustomCollectionsUnitTests { assertThat(result).isInstanceOf(io.vavr.collection.Map.class); } + @Test // #1817 + void conversListToEclipse() { + + assertThat(conversionService.canConvert(List.class, RichIterable.class)).isTrue(); + assertThat(conversionService.canConvert(List.class, ImmutableList.class)).isTrue(); + assertThat(conversionService.canConvert(List.class, ImmutableSet.class)).isTrue(); + assertThat(conversionService.canConvert(List.class, ImmutableBag.class)).isTrue(); + assertThat(conversionService.canConvert(List.class, ImmutableMap.class)).isFalse(); + + List integers = Arrays.asList(1, 2, 3); + + assertThat(conversionService.convert(integers, RichIterable.class)).isInstanceOf(MutableList.class); + assertThat(conversionService.convert(integers, MutableList.class)).isInstanceOf(MutableList.class); + assertThat(conversionService.convert(integers, MutableSet.class)).isInstanceOf(MutableSet.class); + assertThat(conversionService.convert(integers, MutableBag.class)).isInstanceOf(MutableBag.class); + assertThat(conversionService.convert(integers, ImmutableList.class)).isInstanceOf(ImmutableList.class); + assertThat(conversionService.convert(integers, ImmutableSet.class)).isInstanceOf(ImmutableSet.class); + assertThat(conversionService.convert(integers, ImmutableBag.class)).isInstanceOf(ImmutableBag.class); + } + + @Test // #1817 + void convertsSetToEclipse() { + + assertThat(conversionService.canConvert(Set.class, ImmutableSet.class)).isTrue(); + assertThat(conversionService.canConvert(Set.class, ImmutableBag.class)).isTrue(); + assertThat(conversionService.canConvert(Set.class, ImmutableList.class)).isTrue(); + assertThat(conversionService.canConvert(Set.class, ImmutableMap.class)).isFalse(); + + var integers = Collections.singleton(1); + + assertThat(conversionService.convert(integers, RichIterable.class)).isInstanceOf(MutableSet.class); + assertThat(conversionService.convert(integers, MutableList.class)).isInstanceOf(MutableList.class); + assertThat(conversionService.convert(integers, MutableSet.class)).isInstanceOf(MutableSet.class); + assertThat(conversionService.convert(integers, MutableBag.class)).isInstanceOf(MutableBag.class); + assertThat(conversionService.convert(integers, ImmutableList.class)).isInstanceOf(ImmutableList.class); + assertThat(conversionService.convert(integers, ImmutableSet.class)).isInstanceOf(ImmutableSet.class); + assertThat(conversionService.convert(integers, ImmutableBag.class)).isInstanceOf(ImmutableBag.class); + } + + @Test // #1817 + void convertsMapToEclipse() { + + assertThat(conversionService.canConvert(Map.class, RichIterable.class)).isTrue(); + assertThat(conversionService.canConvert(Map.class, MapIterable.class)).isTrue(); + assertThat(conversionService.canConvert(Map.class, SetIterable.class)).isFalse(); + assertThat(conversionService.canConvert(Map.class, ListIterable.class)).isFalse(); + + var map = Collections.singletonMap("key", "value"); + + assertThat(conversionService.convert(map, RichIterable.class)).isInstanceOf(MutableMap.class); + assertThat(conversionService.convert(map, ImmutableMap.class)).isInstanceOf(ImmutableMap.class); + + // Required as MutableMap implements both Iterable and Map and for the standard + // Java compiler this creates an ambiguity + assertThat((Map) conversionService.convert(map, MutableMap.class)).isInstanceOf(MutableMap.class); + } + @Test // DATACMNS-1065, #1817 void unwrapsVavrCollectionsToJavaOnes() { @@ -114,6 +199,18 @@ class CustomCollectionsUnitTests { assertThat(unwrap(io.vavr.collection.LinkedHashMap.of("key", "value"))).isInstanceOf(Map.class); } + @Test // #1817 + void unwrapsEclipseCollectionsToJavaOnes() { + + assertThat(unwrap(Lists.immutable.of(1, 2, 3))).isInstanceOf(List.class); + assertThat(unwrap(Sets.immutable.of(1, 2, 3))).isInstanceOf(Set.class); + assertThat(unwrap(Maps.immutable.of("key", "value"))).isInstanceOf(Map.class); + + assertThat(unwrap(Lists.mutable.of(1, 2, 3))).isInstanceOf(List.class); + assertThat(unwrap(Sets.mutable.of(1, 2, 3))).isInstanceOf(Set.class); + assertThat(unwrap(Maps.mutable.of("key", "value"))).isInstanceOf(Map.class); + } + @Test // #1817 void rejectsInvalidMapType() { assertThatIllegalArgumentException().isThrownBy(() -> CustomCollections.getMapBaseType(Object.class)); @@ -124,7 +221,15 @@ class CustomCollectionsUnitTests { assertThat(CustomCollections.getPaginationReturnTypes()).contains(io.vavr.collection.Seq.class); } - @Test // DATAJPA-1258. #1817 + @TestFactory // #1817 + Stream eclipseSupportedPaginationReturnTypes() { + + return DynamicTest.stream(Stream.of(ImmutableList.class, MutableList.class), + it -> it.getSimpleName() + " is a pagination return type", + it -> assertThat(CustomCollections.getPaginationReturnTypes()).contains(it)); + } + + @Test // DATAJPA-1258, #1817 void convertsJavaListsToVavrSet() { assertThat(conversionService.convert(Collections.singletonList("foo"), io.vavr.collection.Set.class)) // .isInstanceOf(io.vavr.collection.Set.class);