From c2516946e9407cbd7846aa946fd109475095923f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 17 Apr 2018 09:39:11 +0200 Subject: [PATCH] DATAMONGO-1890 - Polishing. Remove mapReduce default methods in favor of adding variants through a fluent API at a later stage. Assert mapReduce arguments and remove subsequent null guards. Adapt tests. Original pull request: #548. --- .../mongodb/core/ReactiveMongoOperations.java | 171 ------------------ .../mongodb/core/ReactiveMongoTemplate.java | 79 ++++---- .../core/mapreduce/MapReduceOptions.java | 4 +- .../mapreduce/ReactiveMapReduceTests.java | 7 +- src/main/asciidoc/new-features.adoc | 1 + 5 files changed, 46 insertions(+), 216 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java index f2bdde4d5..1dca1b0d8 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoOperations.java @@ -1227,107 +1227,6 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations { Flux> changeStream(List filter, Class resultType, ChangeStreamOptions options, String collectionName); - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param resultType the mapping target of the operations result documents. Also used to determine the input - * collection mame. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Class resultType, String mapFunction, String reduceFunction) { - return mapReduce(resultType, resultType, mapFunction, reduceFunction); - } - - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param domainType source type used to determine the input collection name. Must not be {@literal null}. - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Class domainType, Class resultType, String mapFunction, String reduceFunction) { - return mapReduce(new Query(), domainType, resultType, mapFunction, reduceFunction, MapReduceOptions.options()); - } - - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param filterQuery the selection criteria for the documents going input to the map function. Must not be - * {@literal null}. - * @param resultType the mapping target of the operations result documents. Also used to determine the input - * collection mame. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Query filterQuery, Class resultType, String mapFunction, String reduceFunction) { - return mapReduce(filterQuery, resultType, resultType, mapFunction, reduceFunction); - } - - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param filterQuery the selection criteria for the documents going input to the map function. Must not be - * {@literal null}. - * @param domainType source type used to determine the input collection name and map the filter {@link Query} against. - * Must not be {@literal null}. - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Query filterQuery, Class domainType, Class resultType, String mapFunction, - String reduceFunction) { - - return mapReduce(filterQuery, domainType, resultType, mapFunction, reduceFunction, MapReduceOptions.options()); - } - - /** - * Execute a map-reduce operation. Use {@link MapReduceOptions} to optionally specify an output collection and other - * args. - * - * @param resultType he mapping target of the operations result documents. Also used to determine the input collection - * mame. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @param options additional options like output collection. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Class resultType, String mapFunction, String reduceFunction, - MapReduceOptions options) { - - return mapReduce(new Query(), resultType, mapFunction, reduceFunction, options); - } - - /** - * Execute a map-reduce operation. Use {@link MapReduceOptions} to optionally specify an output collection and other - * args. - * - * @param filterQuery the selection criteria for the documents going input to the map function. Must not be - * {@literal null}. - * @param resultType he mapping target of the operations result documents. Also used to determine the input collection - * mame. Must not be {@literal null}. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @param options additional options like output collection. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Query filterQuery, Class resultType, String mapFunction, String reduceFunction, - MapReduceOptions options) { - - return mapReduce(filterQuery, resultType, resultType, mapFunction, reduceFunction, options); - } - /** * Execute a map-reduce operation. Use {@link MapReduceOptions} to optionally specify an output collection and other * args. @@ -1346,76 +1245,6 @@ public interface ReactiveMongoOperations extends ReactiveFluentMongoOperations { Flux mapReduce(Query filterQuery, Class domainType, Class resultType, String mapFunction, String reduceFunction, MapReduceOptions options); - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param collectionName the input collection. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Class resultType, String collectionName, String mapFunction, String reduceFunction) { - return mapReduce(new Query(), resultType, collectionName, mapFunction, reduceFunction); - } - - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param filterQuery the selection criteria for the documents going input to the map function. Must not be - * {@literal null}. - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param collectionName the input collection. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Query filterQuery, Class resultType, String collectionName, String mapFunction, - String reduceFunction) { - - return mapReduce(filterQuery, resultType, resultType, collectionName, mapFunction, reduceFunction); - } - - /** - * Execute an inline map-reduce operation returning the operation result without storing it in an collection. - * - * @param filterQuery the selection criteria for the documents going input to the map function. Must not be - * {@literal null}. - * @param domainType source type used to map the filter {@link Query} against. Must not be {@literal null}. - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param collectionName the input collection. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Query filterQuery, Class domainType, Class resultType, String collectionName, - String mapFunction, String reduceFunction) { - - return mapReduce(filterQuery, domainType, collectionName, resultType, mapFunction, reduceFunction, - MapReduceOptions.options()); - } - - /** - * Execute a map-reduce operation. Use {@link MapReduceOptions} to optionally specify an output collection and other - * args. - * - * @param resultType the mapping target of the operations result documents. Must not be {@literal null}. - * @param collectionName the input collection. - * @param mapFunction the JavaScript map function. Must not be {@literal null}. - * @param reduceFunction the JavaScript reduce function. Must not be {@literal null}. - * @param options additional options like output collection. Must not be {@literal null}. - * @return a {@link Flux} emitting the result document sequence. Never {@literal null}. - * @since 2.1 - */ - default Flux mapReduce(Class resultType, String collectionName, String mapFunction, String reduceFunction, - MapReduceOptions options) { - - return mapReduce(new Query(), resultType, collectionName, resultType, mapFunction, reduceFunction, options); - } - /** * Execute a map-reduce operation. Use {@link MapReduceOptions} to optionally specify an output collection and other * args. diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index 9ff74fb21..d78c2a287 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java @@ -1936,74 +1936,73 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati public Flux mapReduce(Query filterQuery, Class domainType, String inputCollectionName, Class resultType, String mapFunction, String reduceFunction, MapReduceOptions options) { - verifyFunctions(mapFunction, reduceFunction); - - Class mappingTarget = domainType != null ? domainType : resultType; + Assert.notNull(filterQuery, "Filter query must not be null!"); + Assert.notNull(domainType, "Domain type must not be null!"); + Assert.hasText(inputCollectionName, "Input collection name must not be null or empty!"); + Assert.notNull(resultType, "Result type must not be null!"); + Assert.notNull(options, "MapReduceOptions must not be null!"); + Assert.notNull(mapFunction, "Map function must not be null!"); + Assert.notNull(reduceFunction, "Reduce function must not be null!"); + assertLocalFunctionNames(mapFunction, reduceFunction); return createFlux(inputCollectionName, collection -> { Document mappedQuery = queryMapper.getMappedObject(filterQuery.getQueryObject(), - mappingContext.getPersistentEntity(mappingTarget)); + mappingContext.getPersistentEntity(domainType)); MapReducePublisher publisher = collection.mapReduce(mapFunction, reduceFunction, Document.class); - if (options != null) { - if (StringUtils.hasText(options.getOutputCollection())) { - publisher = publisher.collectionName(options.getOutputCollection()); - } + if (StringUtils.hasText(options.getOutputCollection())) { + publisher = publisher.collectionName(options.getOutputCollection()); } publisher.filter(mappedQuery); - publisher.sort(getMappedSortObject(filterQuery, mappingTarget)); + publisher.sort(getMappedSortObject(filterQuery, domainType)); - if (filterQuery.getMeta() != null && filterQuery.getMeta().getMaxTimeMsec() != null) { + if (filterQuery.getMeta().getMaxTimeMsec() != null) { publisher.maxTime(filterQuery.getMeta().getMaxTimeMsec(), TimeUnit.MILLISECONDS); } - if (filterQuery.getLimit() > 0 || (options != null && options.getLimit() != null)) { + if (filterQuery.getLimit() > 0 || (options.getLimit() != null)) { - if (filterQuery.getLimit() > 0 && (options != null && options.getLimit() != null)) { - throw new IllegalArgumentException("which one do ya want?"); + if (filterQuery.getLimit() > 0 && (options.getLimit() != null)) { + throw new IllegalArgumentException("Both Query and MapReduceOptions define a limit. Please provide the limit only via one of the two."); } if (filterQuery.getLimit() > 0) { publisher.limit(filterQuery.getLimit()); } - if ((options != null && options.getLimit() != null)) { + if (options.getLimit() != null) { publisher.limit(options.getLimit()); } - } Optional collation = filterQuery.getCollation(); - if (options != null) { + Optionals.ifAllPresent(filterQuery.getCollation(), options.getCollation(), (l, r) -> { + throw new IllegalArgumentException( + "Both Query and MapReduceOptions define a collation. Please provide the collation only via one of the two."); + }); - Optionals.ifAllPresent(filterQuery.getCollation(), options.getCollation(), (l, r) -> { - throw new IllegalArgumentException( - "Both Query and MapReduceOptions define a collation. Please provide the collation only via one of the two."); - }); + if (options.getCollation().isPresent()) { + collation = options.getCollation(); + } - if (options.getCollation().isPresent()) { - collation = options.getCollation(); - } - - if (!CollectionUtils.isEmpty(options.getScopeVariables())) { - publisher = publisher.scope(new Document(options.getScopeVariables())); - } - if (options.getLimit() != null && options.getLimit().intValue() > 0) { - publisher = publisher.limit(options.getLimit()); - } - if (options.getFinalizeFunction().filter(StringUtils::hasText).isPresent()) { - publisher = publisher.finalizeFunction(options.getFinalizeFunction().get()); - } - if (options.getJavaScriptMode() != null) { - publisher = publisher.jsMode(options.getJavaScriptMode()); - } - if (options.getOutputSharded().isPresent()) { - publisher = publisher.sharded(options.getOutputSharded().get()); - } + if (!CollectionUtils.isEmpty(options.getScopeVariables())) { + publisher = publisher.scope(new Document(options.getScopeVariables())); + } + if (options.getLimit() != null && options.getLimit() > 0) { + publisher = publisher.limit(options.getLimit()); + } + if (options.getFinalizeFunction().filter(StringUtils::hasText).isPresent()) { + publisher = publisher.finalizeFunction(options.getFinalizeFunction().get()); + } + if (options.getJavaScriptMode() != null) { + publisher = publisher.jsMode(options.getJavaScriptMode()); + } + if (options.getOutputSharded().isPresent()) { + publisher = publisher.sharded(options.getOutputSharded().get()); } publisher = collation.map(Collation::toMongoCollation).map(publisher::collation).orElse(publisher); @@ -2013,7 +2012,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati }); } - private void verifyFunctions(String... functions) { + private static void assertLocalFunctionNames(String... functions) { for (String function : functions) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapreduce/MapReduceOptions.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapreduce/MapReduceOptions.java index 0f9ce1c32..f86aebabc 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapreduce/MapReduceOptions.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapreduce/MapReduceOptions.java @@ -37,8 +37,8 @@ public class MapReduceOptions { private Optional outputDatabase = Optional.empty(); private MapReduceCommand.OutputType outputType = MapReduceCommand.OutputType.REPLACE; - private Map scopeVariables = new HashMap(); - private Map extraOptions = new HashMap(); + private Map scopeVariables = new HashMap<>(); + private Map extraOptions = new HashMap<>(); private @Nullable Boolean jsMode; private Boolean verbose = Boolean.TRUE; private @Nullable Integer limit; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/ReactiveMapReduceTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/ReactiveMapReduceTests.java index 1fca402d4..2d12e9a24 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/ReactiveMapReduceTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/ReactiveMapReduceTests.java @@ -29,6 +29,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.data.mongodb.core.Person; import org.springframework.data.mongodb.core.ReactiveMongoTemplate; import org.springframework.data.mongodb.core.SimpleReactiveMongoDatabaseFactory; import org.springframework.data.mongodb.core.mapping.Field; @@ -69,7 +70,7 @@ public class ReactiveMapReduceTests { createMapReduceData(); StepVerifier - .create(template.mapReduce(new Query(), null, "jmr1", ValueObject.class, mapFunction, reduceFunction, + .create(template.mapReduce(new Query(), Person.class, "jmr1", ValueObject.class, mapFunction, reduceFunction, MapReduceOptions.options()).buffer(4)) // .consumeNextWith(result -> { assertThat(result).containsExactlyInAnyOrder(new ValueObject("a", 1), new ValueObject("b", 2), @@ -145,8 +146,8 @@ public class ReactiveMapReduceTests { @Test // DATAMONGO-1890 public void throwsExceptionWhenTryingToLoadFunctionsFromDisk() { - assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> template.mapReduce(new Query(), null, - "foo", ValueObject.class, "classpath:map.js", "classpath:reduce.js", null)) + assertThatExceptionOfType(IllegalArgumentException.class).isThrownBy(() -> template.mapReduce(new Query(), Person.class, + "foo", ValueObject.class, "classpath:map.js", "classpath:reduce.js", MapReduceOptions.options())) .withMessageContaining("classpath:map.js"); } diff --git a/src/main/asciidoc/new-features.adoc b/src/main/asciidoc/new-features.adoc index f248ca094..47edadc08 100644 --- a/src/main/asciidoc/new-features.adoc +++ b/src/main/asciidoc/new-features.adoc @@ -5,6 +5,7 @@ == What's new in Spring Data MongoDB 2.1 * Cursor-based aggregation execution. * <> for imperative and reactive Template API. +* Support for Map/Reduce through reactive Template API. * <>. * <> for queries and collection creation. * <> for imperative and reactive drivers.