Skip to content

Commit f456851

Browse files
mp911demp911de
committed
DATAMONGO-2153 - Polishing.
Use MongoQueryMethod.getDomainClass() instead of getRepositoryDomainType(). Simplify annotation presence indicator methods hasAnnotatedSort() and hasAnnotatedCollation(). Refactor getAnnotatedAggregation() to non-nullable method throwing IllegalStateException to be consistent with other getXxx() methods. Simplify aggregation execution and consider collection/single element declaration for reactive execution. Tweak docs. Original pull request: #743.
1 parent 221ffb1 commit f456851

File tree

13 files changed

+148
-132
lines changed

13 files changed

+148
-132
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/Aggregation.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@
2525
import org.springframework.data.annotation.QueryAnnotation;
2626

2727
/**
28-
* The {@link Aggregation} annotation can be used to decorate a {@link org.springframework.data.repository.Repository}
29-
* query method so that it runs the {@link Aggregation#pipeline()} on invocation. <br />
30-
* The pipeline stages are mapped against the {@link org.springframework.data.repository.Repository} domain type to
31-
* consider {@link org.springframework.data.mongodb.core.mapping.Field field} mappings and may contain simple
32-
* placeholders {@code ?0} as well as {@link org.springframework.expression.spel.standard.SpelExpression
33-
* SpelExpressions}. <br />
28+
* The {@link Aggregation} annotation can be used to annotate a {@link org.springframework.data.repository.Repository}
29+
* query method so that it runs the {@link Aggregation#pipeline()} on invocation.
30+
* <p />
31+
* Pipeline stages are mapped against the {@link org.springframework.data.repository.Repository} domain type to consider
32+
* {@link org.springframework.data.mongodb.core.mapping.Field field} mappings and may contain simple placeholders
33+
* {@code ?0} as well as {@link org.springframework.expression.spel.standard.SpelExpression SpelExpressions}.
34+
* <p />
3435
* Query method {@link org.springframework.data.domain.Sort} and {@link org.springframework.data.domain.Pageable}
3536
* arguments are applied at the end of the pipeline or can be defined manually as part of it.
3637
*
@@ -121,7 +122,6 @@
121122
* </pre>
122123
*
123124
* @return an empty {@link String} by default.
124-
* @since 2.2
125125
*/
126126
String collation() default "";
127127
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/AggregationUtils.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ class AggregationUtils {
6262
* @see AggregationOptions#getCollation()
6363
* @see CollationUtils#computeCollation(String, ConvertingParameterAccessor, MongoParameters, SpelExpressionParser,
6464
* QueryMethodEvaluationContextProvider)
65-
* @since 2.2
6665
*/
6766
static AggregationOptions.Builder applyCollation(AggregationOptions.Builder builder,
6867
@Nullable String collationExpression, ConvertingParameterAccessor accessor, MongoParameters parameters,
@@ -79,7 +78,7 @@ static AggregationOptions.Builder applyCollation(AggregationOptions.Builder buil
7978
* {@link ParameterBindingDocumentCodec} to obtain the MongoDB native {@link Document} representation returned by
8079
* {@link AggregationOperation#toDocument(AggregationOperationContext)} that is mapped against the domain type
8180
* properties.
82-
*
81+
*
8382
* @param method
8483
* @param accessor
8584
* @param expressionParser
@@ -94,7 +93,7 @@ static List<AggregationOperation> computePipeline(MongoQueryMethod method, Conve
9493

9594
List<AggregationOperation> target = new ArrayList<>(method.getAnnotatedAggregation().length);
9695
for (String source : method.getAnnotatedAggregation()) {
97-
target.add(ctx -> ctx.getMappedObject(CODEC.decode(source, bindingContext), method.getRepositoryDomainType()));
96+
target.add(ctx -> ctx.getMappedObject(CODEC.decode(source, bindingContext), method.getDomainClass()));
9897
}
9998
return target;
10099
}
@@ -174,16 +173,16 @@ static <T> T extractSimpleTypeResult(Document source, Class<T> targetType, Mongo
174173
return getPotentiallyConvertedSimpleTypeValue(converter, source.values().iterator().next(), targetType);
175174
}
176175

177-
Document tmp = new Document(source);
178-
tmp.remove("_id");
176+
Document intermediate = new Document(source);
177+
intermediate.remove("_id");
179178

180-
if (tmp.size() == 1) {
181-
return getPotentiallyConvertedSimpleTypeValue(converter, tmp.values().iterator().next(), targetType);
179+
if (intermediate.size() == 1) {
180+
return getPotentiallyConvertedSimpleTypeValue(converter, intermediate.values().iterator().next(), targetType);
182181
}
183182

184-
for (Map.Entry<String, Object> entry : tmp.entrySet()) {
183+
for (Map.Entry<String, Object> entry : intermediate.entrySet()) {
185184
if (entry != null && ClassUtils.isAssignable(targetType, entry.getValue().getClass())) {
186-
return (T) entry.getValue();
185+
return targetType.cast(entry.getValue());
187186
}
188187
}
189188

@@ -192,14 +191,15 @@ static <T> T extractSimpleTypeResult(Document source, Class<T> targetType, Mongo
192191
}
193192

194193
@Nullable
195-
private static <T> T getPotentiallyConvertedSimpleTypeValue(MongoConverter converter, Object value,
194+
@SuppressWarnings("unchecked")
195+
private static <T> T getPotentiallyConvertedSimpleTypeValue(MongoConverter converter, @Nullable Object value,
196196
Class<T> targetType) {
197197

198198
if (value == null) {
199-
return (T) value;
199+
return null;
200200
}
201201

202-
if (!converter.getConversionService().canConvert(value.getClass(), targetType)) {
202+
if (ClassUtils.isAssignableValue(targetType, value)) {
203203
return (T) value;
204204
}
205205

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/MongoQueryMethod.java

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,12 @@ public MongoEntityMetadata<?> getEntityInformation() {
168168
return this.metadata;
169169
}
170170

171-
/**
172-
* Get the declared {@link org.springframework.data.repository.Repository} domain type.
173-
*
174-
* @return the domain type declared at repository level.
175-
* @see QueryMethod#getDomainClass()
176-
* @since 2.2
171+
/*
172+
* (non-Javadoc)
173+
* @see org.springframework.data.repository.query.QueryMethod#getDomainClass()
177174
*/
178-
Class<?> getRepositoryDomainType() {
179-
return getDomainClass();
175+
protected Class<?> getDomainClass() {
176+
return super.getDomainClass();
180177
}
181178

182179
/*
@@ -317,7 +314,7 @@ public org.springframework.data.mongodb.core.query.Meta getQueryMetaAttributes()
317314
* @since 2.1
318315
*/
319316
public boolean hasAnnotatedSort() {
320-
return lookupQueryAnnotation().map(it -> !it.sort().isEmpty()).orElse(false);
317+
return lookupQueryAnnotation().map(Query::sort).filter(StringUtils::hasText).isPresent();
321318
}
322319

323320
/**
@@ -338,13 +335,18 @@ public String getAnnotatedSort() {
338335
* Check if the query method is decorated with an non empty {@link Query#collation()} or or
339336
* {@link Aggregation#collation()}.
340337
*
341-
* @return true if method annotated with {@link Query} or {@link Aggregation} having an non empty collation attribute.
338+
* @return true if method annotated with {@link Query} or {@link Aggregation} having a non-empty collation attribute.
342339
* @since 2.2
343340
*/
344341
public boolean hasAnnotatedCollation() {
345342

346-
return lookupQueryAnnotation().map(it -> !it.collation().isEmpty())
347-
.orElseGet(() -> lookupAggregationAnnotation().map(it -> !it.collation().isEmpty()).orElse(false));
343+
Optional<String> optionalCollation = lookupQueryAnnotation().map(Query::collation);
344+
345+
if (!optionalCollation.isPresent()) {
346+
optionalCollation = lookupAggregationAnnotation().map(Aggregation::collation);
347+
}
348+
349+
return optionalCollation.filter(StringUtils::hasText).isPresent();
348350
}
349351

350352
/**
@@ -374,15 +376,16 @@ public boolean hasAnnotatedAggregation() {
374376
}
375377

376378
/**
377-
* Returns the query string declared in a {@link Query} annotation or {@literal null} if neither the annotation found
378-
* nor the attribute was specified.
379+
* Returns the aggregation pipeline declared in a {@link Aggregation} annotation.
379380
*
380-
* @return
381+
* @return the aggregation pipeline.
382+
* @throws IllegalStateException if method not annotated with {@link Aggregation}. Make sure to check
383+
* {@link #hasAnnotatedAggregation()} first.
381384
* @since 2.2
382385
*/
383-
@Nullable
384386
public String[] getAnnotatedAggregation() {
385-
return findAnnotatedAggregation().orElse(null);
387+
return findAnnotatedAggregation().orElseThrow(() -> new IllegalStateException(
388+
"Expected to find @Aggregation annotation but did not. Make sure to check hasAnnotatedAggregation() before."));
386389
}
387390

388391
private Optional<String[]> findAnnotatedAggregation() {

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/ReactiveStringBasedAggregation.java

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@
2020
import java.util.List;
2121

2222
import org.bson.Document;
23+
2324
import org.springframework.data.mongodb.core.ReactiveMongoOperations;
2425
import org.springframework.data.mongodb.core.aggregation.Aggregation;
2526
import org.springframework.data.mongodb.core.aggregation.AggregationOperation;
2627
import org.springframework.data.mongodb.core.aggregation.AggregationOptions;
2728
import org.springframework.data.mongodb.core.aggregation.TypedAggregation;
2829
import org.springframework.data.mongodb.core.convert.MongoConverter;
2930
import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes;
30-
import org.springframework.data.mongodb.core.query.BasicQuery;
3131
import org.springframework.data.mongodb.core.query.Query;
3232
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
3333
import org.springframework.data.repository.query.ResultProcessor;
@@ -39,6 +39,7 @@
3939
* {@link AggregationOperation aggregation} pipeline to actually execute.
4040
*
4141
* @author Christoph Strobl
42+
* @author Mark Paluch
4243
* @since 2.2
4344
*/
4445
public class ReactiveStringBasedAggregation extends AbstractReactiveMongoQuery {
@@ -74,10 +75,10 @@ public ReactiveStringBasedAggregation(ReactiveMongoQueryMethod method,
7475
protected Object doExecute(ReactiveMongoQueryMethod method, ResultProcessor processor,
7576
ConvertingParameterAccessor accessor, Class<?> typeToRead) {
7677

77-
Class<?> sourceType = method.getRepositoryDomainType();
78+
Class<?> sourceType = method.getDomainClass();
7879
Class<?> targetType = typeToRead;
7980

80-
List<AggregationOperation> pipeline = computePipeline(method, accessor);
81+
List<AggregationOperation> pipeline = computePipeline(accessor);
8182
AggregationUtils.appendSortIfPresent(pipeline, accessor, typeToRead);
8283
AggregationUtils.appendLimitAndOffsetIfPresent(pipeline, accessor);
8384

@@ -94,17 +95,21 @@ protected Object doExecute(ReactiveMongoQueryMethod method, ResultProcessor proc
9495
Flux<?> flux = reactiveMongoOperations.aggregate(aggregation, targetType);
9596

9697
if (isSimpleReturnType && !isRawReturnType) {
97-
return flux.map(it -> AggregationUtils.extractSimpleTypeResult((Document) it, typeToRead, mongoConverter));
98+
flux = flux.map(it -> AggregationUtils.extractSimpleTypeResult((Document) it, typeToRead, mongoConverter));
9899
}
99100

100-
return flux;
101+
if (method.isCollectionQuery()) {
102+
return flux;
103+
} else {
104+
return flux.next();
105+
}
101106
}
102107

103108
private boolean isSimpleReturnType(Class<?> targetType) {
104109
return MongoSimpleTypes.HOLDER.isSimpleType(targetType);
105110
}
106111

107-
List<AggregationOperation> computePipeline(MongoQueryMethod method, ConvertingParameterAccessor accessor) {
112+
List<AggregationOperation> computePipeline(ConvertingParameterAccessor accessor) {
108113
return AggregationUtils.computePipeline(getQueryMethod(), accessor, expressionParser, evaluationContextProvider);
109114
}
110115

@@ -122,7 +127,7 @@ private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingPar
122127
*/
123128
@Override
124129
protected Query createQuery(ConvertingParameterAccessor accessor) {
125-
return new BasicQuery("{}");
130+
throw new UnsupportedOperationException("No query support for aggregation");
126131
}
127132

128133
/*

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/repository/query/StringBasedAggregation.java

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.stream.Collectors;
2020

2121
import org.bson.Document;
22+
2223
import org.springframework.data.mongodb.core.MongoOperations;
2324
import org.springframework.data.mongodb.core.aggregation.Aggregation;
2425
import org.springframework.data.mongodb.core.aggregation.AggregationOperation;
@@ -27,7 +28,6 @@
2728
import org.springframework.data.mongodb.core.aggregation.TypedAggregation;
2829
import org.springframework.data.mongodb.core.convert.MongoConverter;
2930
import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes;
30-
import org.springframework.data.mongodb.core.query.BasicQuery;
3131
import org.springframework.data.mongodb.core.query.Query;
3232
import org.springframework.data.repository.query.QueryMethodEvaluationContextProvider;
3333
import org.springframework.data.repository.query.ResultProcessor;
@@ -71,7 +71,7 @@ public StringBasedAggregation(MongoQueryMethod method, MongoOperations mongoOper
7171
protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProcessor,
7272
ConvertingParameterAccessor accessor, Class<?> typeToRead) {
7373

74-
Class<?> sourceType = method.getRepositoryDomainType();
74+
Class<?> sourceType = method.getDomainClass();
7575
Class<?> targetType = typeToRead;
7676

7777
List<AggregationOperation> pipeline = computePipeline(method, accessor);
@@ -84,7 +84,7 @@ protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProces
8484
if (isSimpleReturnType) {
8585
targetType = Document.class;
8686
} else if (isRawAggregationResult) {
87-
targetType = method.getReturnType().getActualType().getComponentType().getType();
87+
targetType = method.getReturnType().getRequiredActualType().getRequiredComponentType().getType();
8888
}
8989

9090
AggregationOptions options = computeOptions(method, accessor);
@@ -108,13 +108,11 @@ protected Object doExecute(MongoQueryMethod method, ResultProcessor resultProces
108108
return result.getMappedResults();
109109
}
110110

111-
if (isSimpleReturnType) {
112-
113-
return AggregationUtils.extractSimpleTypeResult((Document) result.getUniqueMappedResult(), typeToRead,
114-
mongoConverter);
115-
}
111+
Object uniqueResult = result.getUniqueMappedResult();
116112

117-
return result.getUniqueMappedResult();
113+
return isSimpleReturnType
114+
? AggregationUtils.extractSimpleTypeResult((Document) uniqueResult, typeToRead, mongoConverter)
115+
: uniqueResult;
118116
}
119117

120118
private boolean isSimpleReturnType(Class<?> targetType) {
@@ -139,7 +137,7 @@ private AggregationOptions computeOptions(MongoQueryMethod method, ConvertingPar
139137
*/
140138
@Override
141139
protected Query createQuery(ConvertingParameterAccessor accessor) {
142-
return new BasicQuery("{}");
140+
throw new UnsupportedOperationException("No query support for aggregation");
143141
}
144142

145143
/*

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1347,7 +1347,7 @@ public void annotatedAggregationWithPageable() {
13471347

13481348
@Test // DATAMONGO-2153
13491349
public void annotatedAggregationWithSingleSimpleResult() {
1350-
assertThat(repository.sumAge()).isInstanceOf(Long.class).isEqualTo(245L);
1350+
assertThat(repository.sumAge()).isEqualTo(245);
13511351
}
13521352

13531353
@Test // DATAMONGO-2153

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonAggregate.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,29 @@
1717

1818
import lombok.Value;
1919

20+
import java.util.Collections;
2021
import java.util.List;
2122

2223
import org.springframework.data.annotation.Id;
24+
import org.springframework.data.annotation.PersistenceConstructor;
2325

2426
/**
2527
* @author Christoph Strobl
28+
* @author Mark Paluch
2629
*/
2730
@Value
28-
public class PersonAggregate {
31+
class PersonAggregate {
2932

3033
@Id private String lastname;
3134
private List<String> names;
35+
36+
@PersistenceConstructor
37+
public PersonAggregate(String lastname, List<String> names) {
38+
this.lastname = lastname;
39+
this.names = names;
40+
}
41+
42+
public PersonAggregate(String lastname, String name) {
43+
this(lastname, Collections.singletonList(name));
44+
}
3245
}

‎spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -381,7 +381,7 @@ Page<Person> findByCustomQueryLastnameAndAddressStreetInList(String lastname, Li
381381
List<PersonAggregate> groupByLastnameAnd(String property, Pageable page);
382382

383383
@Aggregation(pipeline = "{ '$group' : { '_id' : null, 'total' : { $sum: '$age' } } }")
384-
Long sumAge();
384+
int sumAge();
385385

386386
@Aggregation(pipeline = "{ '$group' : { '_id' : null, 'total' : { $sum: '$age' } } }")
387387
AggregationResults<org.bson.Document> sumAgeAndReturnAggregationResultWrapper();

0 commit comments

Comments
 (0)