Skip to content

Commit 5f3ad68

Browse files
christophstroblmp911de
authored andcommitted
DATAMONGO-1986 - Always provide a typed AggregationOperationContext for TypedAggregation.
We now initialize a TypeBasedAggregationOperationContext for TypedAggregations if no context is provided. This makes sure that potential Criteria objects are run trough the QueryMapper. In case the default context is used we now also make sure to at least run the aggregation pipeline through the QueryMapper to avoid passing on non MongoDB simple types to the driver. Original pull request: #564.
1 parent 28b18d2 commit 5f3ad68

File tree

9 files changed

+299
-29
lines changed

9 files changed

+299
-29
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.*;
2828
import java.util.Map.Entry;
2929
import java.util.concurrent.TimeUnit;
30+
import java.util.stream.Collectors;
3031

3132
import org.bson.Document;
3233
import org.bson.conversions.Bson;
@@ -204,7 +205,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
204205
* @param databaseName must not be {@literal null} or empty.
205206
*/
206207
public MongoTemplate(MongoClient mongoClient, String databaseName) {
207-
this(new SimpleMongoDbFactory(mongoClient, databaseName), null);
208+
this(new SimpleMongoDbFactory(mongoClient, databaseName), null);
208209
}
209210

210211
/**
@@ -2576,6 +2577,73 @@ private Document addFieldsForProjection(Document fields, Class<?> domainType, Cl
25762577
return fields;
25772578
}
25782579

2580+
/**
2581+
* Prepare the {@link AggregationOperationContext} for a given aggregation by either returning the context itself it
2582+
* is not {@literal null}, create a {@link TypeBasedAggregationOperationContext} if the aggregation contains type
2583+
* information (is a {@link TypedAggregation}) or use the {@link Aggregation#DEFAULT_CONTEXT}.
2584+
*
2585+
* @param aggregation must not be {@literal null}.
2586+
* @param context can be {@literal null}.
2587+
* @return the root {@link AggregationOperationContext} to use.
2588+
*/
2589+
private AggregationOperationContext prepareAggregationContext(Aggregation aggregation,
2590+
@Nullable AggregationOperationContext context) {
2591+
2592+
if (context != null) {
2593+
return context;
2594+
}
2595+
2596+
if (aggregation instanceof TypedAggregation) {
2597+
return new TypeBasedAggregationOperationContext(((TypedAggregation) aggregation).getInputType(), mappingContext,
2598+
queryMapper);
2599+
}
2600+
2601+
return Aggregation.DEFAULT_CONTEXT;
2602+
}
2603+
2604+
/**
2605+
* Extract and map the aggregation pipeline.
2606+
*
2607+
* @param aggregation
2608+
* @param context
2609+
* @return
2610+
*/
2611+
private Document aggregationToPipeline(String inputCollectionName, Aggregation aggregation, AggregationOperationContext context) {
2612+
2613+
if (!ObjectUtils.nullSafeEquals(context, Aggregation.DEFAULT_CONTEXT)) {
2614+
return aggregation.toDocument(inputCollectionName, context);
2615+
}
2616+
2617+
return queryMapper.getMappedObject(aggregation.toDocument(inputCollectionName, context), Optional.empty());
2618+
}
2619+
2620+
/**
2621+
* Extract the command and map the aggregation pipeline.
2622+
*
2623+
* @param aggregation
2624+
* @param context
2625+
* @return
2626+
*/
2627+
private Document aggregationToCommand(String collection, Aggregation aggregation,
2628+
AggregationOperationContext context) {
2629+
2630+
Document command = aggregation.toDocument(collection, context);
2631+
2632+
if (!ObjectUtils.nullSafeEquals(context, Aggregation.DEFAULT_CONTEXT)) {
2633+
return command;
2634+
}
2635+
2636+
command.put("pipeline", mapAggregationPipeline(command.get("pipeline", List.class)));
2637+
2638+
return command;
2639+
}
2640+
2641+
private List<Document> mapAggregationPipeline(List<Document> pipeline) {
2642+
2643+
return pipeline.stream().map(val -> queryMapper.getMappedObject(val, Optional.empty()))
2644+
.collect(Collectors.toList());
2645+
}
2646+
25792647
/**
25802648
* Tries to convert the given {@link RuntimeException} into a {@link DataAccessException} but returns the original
25812649
* exception if the conversation failed. Thus allows safe re-throwing of the return value.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,8 @@ protected <O> Flux<O> aggregate(Aggregation aggregation, String collectionName,
733733
Assert.hasText(collectionName, "Collection name must not be null or empty!");
734734
Assert.notNull(outputType, "Output type must not be null!");
735735

736-
AggregationOperationContext rootContext = context == null ? Aggregation.DEFAULT_CONTEXT : context;
737-
Document command = aggregation.toDocument(collectionName, rootContext);
736+
AggregationOperationContext rootContext = prepareAggregationContext(aggregation, context);
737+
Document command = aggregationToPipeline(collectionName, aggregation, rootContext);
738738
AggregationOptions options = AggregationOptions.fromDocument(command);
739739

740740
Assert.isTrue(!options.isExplain(), "Cannot use explain option with streaming!");
@@ -752,8 +752,8 @@ protected <O> Flux<O> aggregate(Aggregation aggregation, String collectionName,
752752
private <O> Flux<O> aggregateAndMap(MongoCollection<Document> collection, List<Document> pipeline,
753753
AggregationOptions options, ReadDocumentCallback<O> readCallback) {
754754

755-
AggregatePublisher<Document> cursor = collection.aggregate(pipeline).allowDiskUse(options.isAllowDiskUse())
756-
.useCursor(true);
755+
AggregatePublisher<Document> cursor = collection.aggregate(pipeline)
756+
.allowDiskUse(options.isAllowDiskUse());
757757

758758
if (options.getCollation().isPresent()) {
759759
cursor = cursor.collation(options.getCollation().map(Collation::toMongoCollation).get());
@@ -2197,6 +2197,46 @@ private Function<Throwable, Throwable> translateException() {
21972197
};
21982198
}
21992199

2200+
/**
2201+
* Prepare the {@link AggregationOperationContext} for a given aggregation by either returning the context itself it
2202+
* is not {@literal null}, create a {@link TypeBasedAggregationOperationContext} if the aggregation contains type
2203+
* information (is a {@link TypedAggregation}) or use the {@link Aggregation#DEFAULT_CONTEXT}.
2204+
*
2205+
* @param aggregation must not be {@literal null}.
2206+
* @param context can be {@literal null}.
2207+
* @return the root {@link AggregationOperationContext} to use.
2208+
*/
2209+
private AggregationOperationContext prepareAggregationContext(Aggregation aggregation,
2210+
@Nullable AggregationOperationContext context) {
2211+
2212+
if (context != null) {
2213+
return context;
2214+
}
2215+
2216+
if (aggregation instanceof TypedAggregation) {
2217+
return new TypeBasedAggregationOperationContext(((TypedAggregation) aggregation).getInputType(), mappingContext,
2218+
queryMapper);
2219+
}
2220+
2221+
return Aggregation.DEFAULT_CONTEXT;
2222+
}
2223+
2224+
/**
2225+
* Extract and map the aggregation pipeline.
2226+
*
2227+
* @param aggregation
2228+
* @param context
2229+
* @return
2230+
*/
2231+
private Document aggregationToPipeline(String inputCollectionName, Aggregation aggregation, AggregationOperationContext context) {
2232+
2233+
if (!ObjectUtils.nullSafeEquals(context, Aggregation.DEFAULT_CONTEXT)) {
2234+
return aggregation.toDocument(inputCollectionName, context);
2235+
}
2236+
2237+
return queryMapper.getMappedObject(aggregation.toDocument(inputCollectionName, context), Optional.empty());
2238+
}
2239+
22002240
/**
22012241
* Tries to convert the given {@link RuntimeException} into a {@link DataAccessException} but returns the original
22022242
* exception if the conversation failed. Thus allows safe re-throwing of the return value.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class AggregationOperationRenderer {
4141
* {@link Document} representation.
4242
*
4343
* @param operations must not be {@literal null}.
44-
* @param context must not be {@literal null}.
44+
* @param rootContext must not be {@literal null}.
4545
* @return the {@link List} of {@link Document}.
4646
*/
4747
static List<Document> toDocument(List<AggregationOperation> operations, AggregationOperationContext rootContext) {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/ReactiveMongoTemplateTests.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,10 @@ public void readsPlainDbObjectById() {
660660
@Test // DATAMONGO-1444
661661
public void geoNear() {
662662

663-
List<Venue> venues = Arrays.asList(new Venue("Penn Station", -73.99408, 40.75057), //
664-
new Venue("10gen Office", -73.99171, 40.738868), //
665-
new Venue("Flatiron Building", -73.988135, 40.741404), //
666-
new Venue("Maplewood, NJ", -74.2713, 40.73137));
663+
List<Venue> venues = Arrays.asList(TestEntities.geolocation().pennStation(), //
664+
TestEntities.geolocation().tenGenOffice(), //
665+
TestEntities.geolocation().flatironBuilding(), //
666+
TestEntities.geolocation().maplewoodNJ());
667667

668668
StepVerifier.create(template.insertAll(venues)).expectNextCount(4).verifyComplete();
669669

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
/*
2+
* Copyright 2018 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.mongodb.core;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
21+
/**
22+
* A simple collection of grouped test entities used throughout the test suite.
23+
*
24+
* @author Christoph Strobl
25+
*/
26+
public class TestEntities {
27+
28+
private static final GeoEntities GEO = new GeoEntities();
29+
30+
public static GeoEntities geolocation() {
31+
return GEO;
32+
}
33+
34+
public static class GeoEntities {
35+
36+
/**
37+
* <pre>
38+
* X: -73.99408
39+
* Y: 40.75057
40+
* </pre>
41+
*
42+
* @return new {@link Venue}
43+
*/
44+
public Venue pennStation() {
45+
return new Venue("Penn Station", -73.99408, 40.75057);
46+
}
47+
48+
/**
49+
* <pre>
50+
* X: -73.99171
51+
* Y: 40.738868
52+
* </pre>
53+
*
54+
* @return new {@link Venue}
55+
*/
56+
57+
public Venue tenGenOffice() {
58+
return new Venue("10gen Office", -73.99171, 40.738868);
59+
}
60+
61+
/**
62+
* <pre>
63+
* X: -73.988135
64+
* Y: 40.741404
65+
* </pre>
66+
*
67+
* @return new {@link Venue}
68+
*/
69+
public Venue flatironBuilding() {
70+
return new Venue("Flatiron Building", -73.988135, 40.741404);
71+
}
72+
73+
/**
74+
* <pre>
75+
* X: -74.2713
76+
* Y: 40.73137
77+
* </pre>
78+
*
79+
* @return new {@link Venue}
80+
*/
81+
public Venue maplewoodNJ() {
82+
return new Venue("Maplewood, NJ", -74.2713, 40.73137);
83+
}
84+
85+
public List<Venue> newYork() {
86+
87+
List<Venue> venues = new ArrayList<>();
88+
89+
venues.add(pennStation());
90+
venues.add(tenGenOffice());
91+
venues.add(flatironBuilding());
92+
venues.add(new Venue("Players Club", -73.997812, 40.739128));
93+
venues.add(new Venue("City Bakery ", -73.992491, 40.738673));
94+
venues.add(new Venue("Splash Bar", -73.992491, 40.738673));
95+
venues.add(new Venue("Momofuku Milk Bar", -73.985839, 40.731698));
96+
venues.add(new Venue("Shake Shack", -73.98820, 40.74164));
97+
venues.add(new Venue("Penn Station", -73.99408, 40.75057));
98+
venues.add(new Venue("Empire State Building", -73.98602, 40.74894));
99+
venues.add(new Venue("Ulaanbaatar, Mongolia", 106.9154, 47.9245));
100+
venues.add(maplewoodNJ());
101+
102+
return venues;
103+
}
104+
}
105+
}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,13 @@
5353
import org.springframework.dao.DataAccessException;
5454
import org.springframework.data.annotation.Id;
5555
import org.springframework.data.domain.Sort.Direction;
56+
import org.springframework.data.geo.Box;
5657
import org.springframework.data.geo.Metrics;
58+
import org.springframework.data.geo.Point;
5759
import org.springframework.data.mapping.MappingException;
5860
import org.springframework.data.mongodb.core.CollectionCallback;
5961
import org.springframework.data.mongodb.core.MongoTemplate;
62+
import org.springframework.data.mongodb.core.TestEntities;
6063
import org.springframework.data.mongodb.core.Venue;
6164
import org.springframework.data.mongodb.core.aggregation.AggregationTests.CarDescriptor.Entry;
6265
import org.springframework.data.mongodb.core.aggregation.BucketAutoOperation.Granularities;
@@ -143,6 +146,8 @@ private void cleanDb() {
143146
mongoTemplate.dropCollection(Sales2.class);
144147
mongoTemplate.dropCollection(Employee.class);
145148
mongoTemplate.dropCollection(Art.class);
149+
mongoTemplate.dropCollection("personQueryTemp");
150+
mongoTemplate.dropCollection(Venue.class);
146151
}
147152

148153
/**
@@ -1484,9 +1489,8 @@ public void shouldRetrieveDateTimeFragementsCorrectly() throws Exception {
14841489
@Test // DATAMONGO-1127
14851490
public void shouldSupportGeoNearQueriesForAggregationWithDistanceField() {
14861491

1487-
mongoTemplate.insert(new Venue("Penn Station", -73.99408, 40.75057));
1488-
mongoTemplate.insert(new Venue("10gen Office", -73.99171, 40.738868));
1489-
mongoTemplate.insert(new Venue("Flatiron Building", -73.988135, 40.741404));
1492+
mongoTemplate.insertAll(Arrays.asList(TestEntities.geolocation().pennStation(),
1493+
TestEntities.geolocation().tenGenOffice(), TestEntities.geolocation().flatironBuilding()));
14901494

14911495
mongoTemplate.indexOps(Venue.class).ensureIndex(new GeospatialIndex("location"));
14921496

@@ -1898,6 +1902,36 @@ public void facetShouldCreateFacets() {
18981902
assertThat(categorizeByYear, hasSize(3));
18991903
}
19001904

1905+
@Test // DATAMONGO-1986
1906+
public void runMatchOperationCriteriaThroughQueryMapperForTypedAggregation() {
1907+
1908+
mongoTemplate.insertAll(TestEntities.geolocation().newYork());
1909+
1910+
Aggregation aggregation = newAggregation(Venue.class,
1911+
match(Criteria.where("location")
1912+
.within(new Box(new Point(-73.99756, 40.73083), new Point(-73.988135, 40.741404)))),
1913+
project("id", "location", "name"));
1914+
1915+
AggregationResults<Document> groupResults = mongoTemplate.aggregate(aggregation, "newyork", Document.class);
1916+
1917+
assertThat(groupResults.getMappedResults().size(), is(4));
1918+
}
1919+
1920+
@Test // DATAMONGO-1986
1921+
public void runMatchOperationCriteriaThroughQueryMapperForUntypedAggregation() {
1922+
1923+
mongoTemplate.insertAll(TestEntities.geolocation().newYork());
1924+
1925+
Aggregation aggregation = newAggregation(
1926+
match(Criteria.where("location")
1927+
.within(new Box(new Point(-73.99756, 40.73083), new Point(-73.988135, 40.741404)))),
1928+
project("id", "location", "name"));
1929+
1930+
AggregationResults<Document> groupResults = mongoTemplate.aggregate(aggregation, "newyork", Document.class);
1931+
1932+
assertThat(groupResults.getMappedResults().size(), is(4));
1933+
}
1934+
19011935
private void createUsersWithReferencedPersons() {
19021936

19031937
mongoTemplate.dropCollection(User.class);

0 commit comments

Comments
 (0)