From f83622709d4b7e75b1904ae69b23526057adcb90 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Tue, 7 Aug 2018 12:41:25 +0200 Subject: [PATCH] DATAMONGO-2027 - Polishing. Remove duplicate tests and fix assertions on existing ones. Move tests over to AssertJ and fix output database not applied correctly. Original Pull Request: #588 --- .../data/mongodb/core/MongoTemplate.java | 14 +- .../core/mapreduce/MapReduceOptions.java | 15 +- .../mongodb/core/MongoTemplateUnitTests.java | 50 +++ .../core/mapreduce/MapReduceTests.java | 342 +++++++++--------- 4 files changed, 251 insertions(+), 170 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 262c9e96f..46a427735 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java @@ -1716,23 +1716,31 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, if (!CollectionUtils.isEmpty(mapReduceOptions.getScopeVariables())) { result = result.scope(new Document(mapReduceOptions.getScopeVariables())); } + if (mapReduceOptions.getLimit() != null && mapReduceOptions.getLimit().intValue() > 0) { result = result.limit(mapReduceOptions.getLimit()); } + if (mapReduceOptions.getFinalizeFunction().filter(StringUtils::hasText).isPresent()) { result = result.finalizeFunction(mapReduceOptions.getFinalizeFunction().get()); } + if (mapReduceOptions.getJavaScriptMode() != null) { result = result.jsMode(mapReduceOptions.getJavaScriptMode()); } + if (mapReduceOptions.getOutputSharded().isPresent()) { result = result.sharded(mapReduceOptions.getOutputSharded().get()); } - MapReduceAction action = mapReduceOptions.getMapReduceAction(); + if (StringUtils.hasText(mapReduceOptions.getOutputCollection()) && !mapReduceOptions.usesInlineOutput()) { - if(action != null && mapReduceOptions.getOutputCollection() != null){ - result = result.action(action).collectionName(mapReduceOptions.getOutputCollection()); + result = result.collectionName(mapReduceOptions.getOutputCollection()) + .action(mapReduceOptions.getMapReduceAction()); + + if (mapReduceOptions.getOutputDatabase().isPresent()) { + result = result.databaseName(mapReduceOptions.getOutputDatabase().get()); + } } } 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 2327f3f4d..601e10d25 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 @@ -24,6 +24,7 @@ import org.springframework.data.mongodb.core.query.Collation; import org.springframework.lang.Nullable; import com.mongodb.MapReduceCommand; +import com.mongodb.MapReduceCommand.OutputType; import com.mongodb.client.model.MapReduceAction; /** @@ -300,7 +301,7 @@ public class MapReduceOptions { * Return the {@link MapReduceAction} derived from {@link com.mongodb.MapReduceCommand.OutputType}. * * @return the mapped action or {@literal null} if the action maps to inline output. - * @since 2.0.9 + * @since 2.0.10 */ @Nullable public MapReduceAction getMapReduceAction() { @@ -312,9 +313,19 @@ public class MapReduceOptions { return MapReduceAction.REDUCE; case REPLACE: return MapReduceAction.REPLACE; + case INLINE: + return null; + default: + throw new IllegalStateException(String.format("Unknown output type %s for map reduce command.", outputType)); } + } - return null; + /** + * @return {@literal true} if {@link OutputType#INLINE} is used. + * @since 2.0.10 + */ + public boolean usesInlineOutput() { + return OutputType.INLINE.equals(outputType); } public Document getOptionsObject() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java index 6a1385573..6dd31b96f 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateUnitTests.java @@ -90,6 +90,7 @@ import com.mongodb.client.model.CountOptions; import com.mongodb.client.model.DeleteOptions; import com.mongodb.client.model.FindOneAndDeleteOptions; import com.mongodb.client.model.FindOneAndUpdateOptions; +import com.mongodb.client.model.MapReduceAction; import com.mongodb.client.model.UpdateOptions; import com.mongodb.client.result.UpdateResult; @@ -139,6 +140,9 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { when(mapReduceIterable.sort(Mockito.any())).thenReturn(mapReduceIterable); when(mapReduceIterable.iterator()).thenReturn(cursor); when(mapReduceIterable.filter(any())).thenReturn(mapReduceIterable); + when(mapReduceIterable.collectionName(any())).thenReturn(mapReduceIterable); + when(mapReduceIterable.databaseName(any())).thenReturn(mapReduceIterable); + when(mapReduceIterable.action(any())).thenReturn(mapReduceIterable); this.mappingContext = new MongoMappingContext(); this.converter = new MappingMongoConverter(new DefaultDbRefResolver(factory), mappingContext); @@ -774,6 +778,52 @@ public class MongoTemplateUnitTests extends MongoOperationsUnitTests { verify(mapReduceIterable).collation(eq(com.mongodb.client.model.Collation.builder().locale("fr").build())); } + @Test // DATAMONGO-2027 + public void mapReduceShouldUseOutputCollectionWhenPresent() { + + template.mapReduce("", "", "", MapReduceOptions.options().outputCollection("out-collection"), + AutogenerateableId.class); + + verify(mapReduceIterable).collectionName(eq("out-collection")); + } + + @Test // DATAMONGO-2027 + public void mapReduceShouldNotUseOutputCollectionForInline() { + + template.mapReduce("", "", "", MapReduceOptions.options().outputCollection("out-collection").outputTypeInline(), + AutogenerateableId.class); + + verify(mapReduceIterable, never()).collectionName(any()); + } + + @Test // DATAMONGO-2027 + public void mapReduceShouldUseOutputActionWhenPresent() { + + template.mapReduce("", "", "", MapReduceOptions.options().outputCollection("out-collection").outputTypeMerge(), + AutogenerateableId.class); + + verify(mapReduceIterable).action(eq(MapReduceAction.MERGE)); + } + + @Test // DATAMONGO-2027 + public void mapReduceShouldUseOutputDatabaseWhenPresent() { + + template.mapReduce("", "", "", + MapReduceOptions.options().outputDatabase("out-database").outputCollection("out-collection").outputTypeMerge(), + AutogenerateableId.class); + + verify(mapReduceIterable).databaseName(eq("out-database")); + } + + @Test // DATAMONGO-2027 + public void mapReduceShouldNotUseOutputDatabaseForInline() { + + template.mapReduce("", "", "", MapReduceOptions.options().outputDatabase("out-database").outputTypeInline(), + AutogenerateableId.class); + + verify(mapReduceIterable, never()).databaseName(any()); + } + @Test // DATAMONGO-1518 public void geoNearShouldUseCollationWhenPresent() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/MapReduceTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/MapReduceTests.java index 81126c7a0..e0142bdf8 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/MapReduceTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/mapreduce/MapReduceTests.java @@ -15,7 +15,7 @@ */ package org.springframework.data.mongodb.core.mapreduce; -import static org.junit.Assert.*; +import static org.assertj.core.api.Assertions.*; import static org.springframework.data.mongodb.core.mapreduce.MapReduceOptions.*; import static org.springframework.data.mongodb.core.query.Criteria.*; @@ -46,13 +46,14 @@ import com.mongodb.client.MongoCollection; * @author Mark Pollack * @author Thomas Darimont * @author Mark Paluch + * @author Christoph Strobl */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration("classpath:infrastructure.xml") public class MapReduceTests { - private String mapFunction = "function(){ for ( var i=0; i results = mongoTemplate.mapReduce("jmr1", mapFunction, reduceFunction, + MapReduceResults results = mongoTemplate.mapReduce("jmr1", MAP_FUNCTION, REDUCE_FUNCTION, ValueObject.class); + for (ValueObject valueObject : results) { System.out.println(valueObject); } } - @Test + @Test // DATAMONGO-260 public void testIssue260() { + createContentAndVersionData(); String map = "function () { emit(this.document_id, this.version); }"; String reduce = "function (key, values) { return Math.max.apply(Math, values); }"; + MapReduceResults results = mongoTemplate.mapReduce("jmr2", map, reduce, new MapReduceOptions().outputCollection("jmr2_out"), ContentAndVersion.class); - int size = 0; + assertThat(results).hasSize(3); for (ContentAndVersion cv : results) { + if ("Resume".equals(cv.getId())) { - assertEquals(6, cv.getValue().longValue()); + assertThat(cv.getValue().longValue()).isEqualTo(6); } if ("Schema".equals(cv.getId())) { - assertEquals(2, cv.getValue().longValue()); + assertThat(cv.getValue().longValue()).isEqualTo(2); } if ("mongoDB How-To".equals(cv.getId())) { - assertEquals(2, cv.getValue().longValue()); + assertThat(cv.getValue().longValue()).isEqualTo(2); } - size++; } - assertEquals(3, size); + } - @Test + @Test // DATAMONGO-260 public void testIssue260Part2() { + createNumberAndVersionData(); String map = "function () { emit(this.number, this.version); }"; String reduce = "function (key, values) { return Math.max.apply(Math, values); }"; + MapReduceResults results = mongoTemplate.mapReduce("jmr2", map, reduce, new MapReduceOptions().outputCollection("jmr2_out"), NumberAndVersion.class); - int size = 0; + for (NumberAndVersion nv : results) { if ("1".equals(nv.getId())) { - assertEquals(2, nv.getValue().longValue()); + assertThat(nv.getValue().longValue()).isEqualTo(2); } if ("2".equals(nv.getId())) { - assertEquals(6, nv.getValue().longValue()); + assertThat(nv.getValue().longValue()).isEqualTo(6); } if ("3".equals(nv.getId())) { - assertEquals(2, nv.getValue().longValue()); + assertThat(nv.getValue().longValue()).isEqualTo(2); } - size++; } - assertEquals(3, size); + + assertThat(results).hasSize(3); + } + + @Test // DATADOC-7, DATAMONGO-2027 + public void testMapReduce() { + + performMapReduce(false, false); + + List results = mongoTemplate.find(new Query(), ValueObject.class, "jmr1_out"); + assertMapReduceResults(copyToMap(results)); + } + + @Test // DATADOC-7, DATAMONGO-2027 + public void testMapReduceInline() { + + performMapReduce(true, false); + assertThat(template.collectionExists("jmr1_out")).isFalse(); + } + + @Test // DATAMONGO-2027 + public void mapReduceWithOutputDatabaseShouldWorkCorrectly() { + + createMapReduceData(); + + mongoTemplate.mapReduce("jmr1", MAP_FUNCTION, REDUCE_FUNCTION, + options().outputDatabase("jmr1-out-db").outputCollection("jmr1-out"), ValueObject.class); + + assertThat(template.getMongoDbFactory().getDb("jmr1-out-db").listCollectionNames().into(new ArrayList<>())) + .contains("jmr1-out"); + } + + @Test // DATADOC-7 + public void testMapReduceWithQuery() { + performMapReduce(false, true); + } + + @Test // DATADOC-7 + public void testMapReduceInlineWithScope() { + + createMapReduceData(); + + Map scopeVariables = new HashMap(); + scopeVariables.put("exclude", "a"); + + String mapWithExcludeFunction = "function(){ for ( var i=0; i results = mongoTemplate.mapReduce("jmr1", mapWithExcludeFunction, REDUCE_FUNCTION, + new MapReduceOptions().scopeVariables(scopeVariables).outputTypeInline(), ValueObject.class); + + assertThat(copyToMap(results)) // + .hasSize(3) // + .containsEntry("b", 2F) // + .containsEntry("c", 2F) // + .containsEntry("d", 1F); + } + + @Test // DATADOC-7 + public void testMapReduceExcludeQuery() { + + createMapReduceData(); + + Query query = new Query(where("x").ne(new String[] { "a", "b" })); + MapReduceResults results = mongoTemplate.mapReduce(query, "jmr1", MAP_FUNCTION, REDUCE_FUNCTION, + ValueObject.class); + + assertThat(copyToMap(results)) // + .hasSize(3) // + .containsEntry("b", 1F) // + .containsEntry("c", 2F) // + .containsEntry("d", 1F); + } + + @Test // DATAMONGO-938 + public void mapReduceShouldUseQueryMapper() { + + MongoCollection c = mongoTemplate.getDb().getCollection("jmrWithGeo", Document.class); + + c.insertOne(new Document("x", Arrays.asList("a", "b")).append("loc", Arrays.asList(0D, 0D))); + c.insertOne(new Document("x", Arrays.asList("b", "c")).append("loc", Arrays.asList(0D, 0D))); + c.insertOne(new Document("x", Arrays.asList("c", "d")).append("loc", Arrays.asList(0D, 0D))); + + Query query = new Query(where("x").ne(new String[] { "a", "b" }).and("loc") + .within(new Box(new double[] { 0, 0 }, new double[] { 1, 1 }))); + + MapReduceResults results = template.mapReduce(query, "jmrWithGeo", MAP_FUNCTION, REDUCE_FUNCTION, + ValueObject.class); + + assertThat(copyToMap(results)) // + .hasSize(3) // + .containsEntry("b", 1F) // + .containsEntry("c", 2F) // + .containsEntry("d", 1F); + } + + private void performMapReduce(boolean inline, boolean withQuery) { + + createMapReduceData(); + MapReduceResults results; + if (inline) { + if (withQuery) { + results = mongoTemplate.mapReduce(new Query(), "jmr1", "classpath:map.js", "classpath:reduce.js", + ValueObject.class); + } else { + results = mongoTemplate.mapReduce("jmr1", MAP_FUNCTION, REDUCE_FUNCTION, ValueObject.class); + } + } else { + if (withQuery) { + results = mongoTemplate.mapReduce(new Query(), "jmr1", MAP_FUNCTION, REDUCE_FUNCTION, + options().outputCollection("jmr1_out"), ValueObject.class); + } else { + results = mongoTemplate.mapReduce("jmr1", MAP_FUNCTION, REDUCE_FUNCTION, + new MapReduceOptions().outputCollection("jmr1_out"), ValueObject.class); + } + } + + assertMapReduceResults(copyToMap(results)); + } + + private void createMapReduceData() { + + MongoCollection c = mongoTemplate.getDb().getCollection("jmr1", Document.class); + c.insertOne(new Document("x", Arrays.asList("a", "b"))); + c.insertOne(new Document("x", Arrays.asList("b", "c"))); + c.insertOne(new Document("x", Arrays.asList("c", "d"))); + } + + private Map copyToMap(Iterable results) { + + List valueObjects = new ArrayList<>(); + for (ValueObject valueObject : results) { + valueObjects.add(valueObject); + } + + Map m = new HashMap<>(); + for (ValueObject vo : valueObjects) { + m.put(vo.getId(), vo.getValue()); + } + return m; + } + + private void assertMapReduceResults(Map map) { + + assertThat(map) // + .hasSize(4) // + .containsEntry("a", 1F) // + .containsEntry("b", 2F) // + .containsEntry("c", 2F) // + .containsEntry("d", 1F); } private void createNumberAndVersionData() { + NumberAndVersion nv1 = new NumberAndVersion(); nv1.setNumber(1L); nv1.setVersion(1L); @@ -170,7 +326,7 @@ public class MapReduceTests { { "_id" : 3, "document_id" : "Resume", "author" : "Author", "content" : "...", "version" : 6 } { "_id" : 4, "document_id" : "Schema", "author" : "Someone Else", "content" : "...", "version" : 0.9 } { "_id" : 5, "document_id" : "Schema", "author" : "Someone Else", "content" : "...", "version" : 1 } - + */ ContentAndVersion cv1 = new ContentAndVersion(); cv1.setDocumentId("mongoDB How-To"); @@ -206,149 +362,5 @@ public class MapReduceTests { cv5.setContent("..."); cv5.setVersion(2L); template.save(cv5, "jmr2"); - } - - @Test - public void testMapReduce() { - performMapReduce(false, false); - } - - @Test - public void testMapReduceInline() { - performMapReduce(true, false); - } - - @Test - public void testMapReduceWithQuery() { - performMapReduce(false, true); - } - - @Test - public void testMapReduceInlineWithScope() { - createMapReduceData(); - - Map scopeVariables = new HashMap(); - scopeVariables.put("exclude", "a"); - - String mapWithExcludeFunction = "function(){ for ( var i=0; i results = mongoTemplate.mapReduce("jmr1", mapWithExcludeFunction, reduceFunction, - new MapReduceOptions().scopeVariables(scopeVariables).outputTypeInline(), ValueObject.class); - Map m = copyToMap(results); - assertEquals(3, m.size()); - assertEquals(2, m.get("b").intValue()); - assertEquals(2, m.get("c").intValue()); - assertEquals(1, m.get("d").intValue()); - } - - @Test // DATAMONGO-2027 - public void shouldStoreResultInCollection() { - - createMapReduceData(); - - String mapWithExcludeFunction = "function(){ for ( var i=0; i results = mongoTemplate.find(new Query(), ValueObject.class, "mapreduceout"); - - Map m = copyToMap(results); - assertEquals(4, m.size()); - assertEquals(1, m.get("a").intValue()); - assertEquals(2, m.get("b").intValue()); - assertEquals(2, m.get("c").intValue()); - assertEquals(1, m.get("d").intValue()); - } - - @Test - public void testMapReduceExcludeQuery() { - createMapReduceData(); - - Query query = new Query(where("x").ne(new String[] { "a", "b" })); - MapReduceResults results = mongoTemplate.mapReduce(query, "jmr1", mapFunction, reduceFunction, - ValueObject.class); - - Map m = copyToMap(results); - assertEquals(3, m.size()); - assertEquals(1, m.get("b").intValue()); - assertEquals(2, m.get("c").intValue()); - assertEquals(1, m.get("d").intValue()); - - } - - @Test // DATAMONGO-938 - public void mapReduceShouldUseQueryMapper() { - - MongoCollection c = mongoTemplate.getDb().getCollection("jmrWithGeo", Document.class); - - c.insertOne(new Document("x", Arrays.asList("a", "b")).append("loc", Arrays. asList(0D, 0D))); - c.insertOne(new Document("x", Arrays.asList("b", "c")).append("loc", Arrays. asList(0D, 0D))); - c.insertOne(new Document("x", Arrays.asList("c", "d")).append("loc", Arrays. asList(0D, 0D))); - - Query query = new Query(where("x").ne(new String[] { "a", "b" }).and("loc") - .within(new Box(new double[] { 0, 0 }, new double[] { 1, 1 }))); - - MapReduceResults results = template.mapReduce(query, "jmrWithGeo", mapFunction, reduceFunction, - ValueObject.class); - - Map m = copyToMap(results); - assertEquals(3, m.size()); - assertEquals(1, m.get("b").intValue()); - assertEquals(2, m.get("c").intValue()); - assertEquals(1, m.get("d").intValue()); - } - - private void performMapReduce(boolean inline, boolean withQuery) { - createMapReduceData(); - MapReduceResults results; - if (inline) { - if (withQuery) { - results = mongoTemplate.mapReduce(new Query(), "jmr1", "classpath:map.js", "classpath:reduce.js", - ValueObject.class); - } else { - results = mongoTemplate.mapReduce("jmr1", mapFunction, reduceFunction, ValueObject.class); - } - } else { - if (withQuery) { - results = mongoTemplate.mapReduce(new Query(), "jmr1", mapFunction, reduceFunction, - options().outputCollection("jmr1_out"), ValueObject.class); - } else { - results = mongoTemplate.mapReduce("jmr1", mapFunction, reduceFunction, - new MapReduceOptions().outputCollection("jmr1_out"), ValueObject.class); - } - } - Map m = copyToMap(results); - assertMapReduceResults(m); - } - - private void createMapReduceData() { - MongoCollection c = mongoTemplate.getDb().getCollection("jmr1", Document.class); - c.insertOne(new Document("x", Arrays.asList("a", "b"))); - c.insertOne(new Document("x", Arrays.asList("b", "c"))); - c.insertOne(new Document("x", Arrays.asList("c", "d"))); - } - - private Map copyToMap(Iterable results) { - List valueObjects = new ArrayList(); - for (ValueObject valueObject : results) { - valueObjects.add(valueObject); - } - - Map m = new HashMap(); - for (ValueObject vo : valueObjects) { - m.put(vo.getId(), vo.getValue()); - } - return m; - } - - private void assertMapReduceResults(Map m) { - assertEquals(4, m.size()); - assertEquals(1, m.get("a").intValue()); - assertEquals(2, m.get("b").intValue()); - assertEquals(2, m.get("c").intValue()); - assertEquals(1, m.get("d").intValue()); - } - }