Skip to content

Commit 26f0a1c

Browse files
committed
DATAMONGO-2622 - Polishing.
Rename AggregationPipeline.requiresRelaxedChecking() to containsUnionWith() to avoid the concept of field validation leaking into AggregationPipeline. Refactor AggregationOperation to consistently check their type and fallback to the operator check to allow for consistent checks when using custo AggregationOperations. Original pull request: #886.
1 parent 230c320 commit 26f0a1c

File tree

4 files changed

+43
-27
lines changed

4 files changed

+43
-27
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ AggregationOperationContext prepareAggregationContext(Aggregation aggregation,
8282

8383
Class<?> inputType = ((TypedAggregation) aggregation).getInputType();
8484

85-
if (aggregation.getPipeline().requiresRelaxedChecking()) {
85+
if (aggregation.getPipeline().containsUnionWith()) {
8686
return new RelaxedTypeBasedAggregationOperationContext(inputType, mappingContext, queryMapper);
8787
}
8888

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

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.ArrayList;
1919
import java.util.Collections;
2020
import java.util.List;
21+
import java.util.function.Predicate;
2122

2223
import org.bson.Document;
2324
import org.springframework.util.Assert;
@@ -26,6 +27,7 @@
2627
* The {@link AggregationPipeline} holds the collection of {@link AggregationOperation aggregation stages}.
2728
*
2829
* @author Christoph Strobl
30+
* @author Mark Paluch
2931
* @since 3.0.2
3032
*/
3133
public class AggregationPipeline {
@@ -84,42 +86,39 @@ List<Document> toDocuments(AggregationOperationContext context) {
8486
*/
8587
public boolean isOutOrMerge() {
8688

87-
if (pipeline.isEmpty()) {
89+
if (isEmpty()) {
8890
return false;
8991
}
9092

91-
String operator = pipeline.get(pipeline.size() - 1).getOperator();
92-
return operator.equals("$out") || operator.equals("$merge");
93+
AggregationOperation operation = pipeline.get(pipeline.size() - 1);
94+
return isOut(operation) || isMerge(operation);
9395
}
9496

9597
void verify() {
9698

9799
// check $out/$merge is the last operation if it exists
98-
for (AggregationOperation aggregationOperation : pipeline) {
100+
for (AggregationOperation operation : pipeline) {
99101

100-
if (aggregationOperation instanceof OutOperation && !isLast(aggregationOperation)) {
102+
if (isOut(operation) && !isLast(operation)) {
101103
throw new IllegalArgumentException("The $out operator must be the last stage in the pipeline.");
102104
}
103105

104-
if (aggregationOperation instanceof MergeOperation && !isLast(aggregationOperation)) {
106+
if (isMerge(operation) && !isLast(operation)) {
105107
throw new IllegalArgumentException("The $merge operator must be the last stage in the pipeline.");
106108
}
107109
}
108110
}
109111

110-
private boolean isLast(AggregationOperation aggregationOperation) {
111-
return pipeline.indexOf(aggregationOperation) == pipeline.size() - 1;
112-
}
113-
114112
/**
115-
* @return {@literal true} if field names might get computed by one of the pipeline stages, that the
116-
* {@link AggregationOperationContext} might not be aware of. A strongly typed context might fail to resolve
117-
* field references, so if {@literal true} usage of a {@link RelaxedTypeBasedAggregationOperationContext}
118-
* might be the better choice.
113+
* Return whether this aggregation pipeline defines a {@code $unionWith} stage that may contribute documents from
114+
* other collections. Checking for presence of union stages is useful when attempting to determine the aggregation
115+
* element type for mapping metadata computation.
116+
*
117+
* @return {@literal true} the aggregation pipeline makes use of {@code $unionWith}.
119118
* @since 3.1
120119
*/
121-
public boolean requiresRelaxedChecking() {
122-
return pipelineContainsValueOfType(UnionWithOperation.class);
120+
public boolean containsUnionWith() {
121+
return containsOperation(AggregationPipeline::isUnionWith);
123122
}
124123

125124
/**
@@ -130,17 +129,34 @@ public boolean isEmpty() {
130129
return pipeline.isEmpty();
131130
}
132131

133-
private boolean pipelineContainsValueOfType(Class<?> type) {
132+
private boolean containsOperation(Predicate<AggregationOperation> predicate) {
134133

135134
if (isEmpty()) {
136135
return false;
137136
}
138137

139-
for (Object element : pipeline) {
140-
if (type.isInstance(element)) {
138+
for (AggregationOperation element : pipeline) {
139+
if (predicate.test(element)) {
141140
return true;
142141
}
143142
}
143+
144144
return false;
145145
}
146+
147+
private boolean isLast(AggregationOperation aggregationOperation) {
148+
return pipeline.indexOf(aggregationOperation) == pipeline.size() - 1;
149+
}
150+
151+
private static boolean isUnionWith(AggregationOperation operator) {
152+
return operator instanceof UnionWithOperation || operator.getOperator().equals("$unionWith");
153+
}
154+
155+
private static boolean isMerge(AggregationOperation operator) {
156+
return operator instanceof MergeOperation || operator.getOperator().equals("$merge");
157+
}
158+
159+
private static boolean isOut(AggregationOperation operator) {
160+
return operator instanceof OutOperation || operator.getOperator().equals("$out");
161+
}
146162
}

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ public class UnionWithOperation implements AggregationOperation {
4545

4646
private final String collection;
4747

48-
@Nullable //
49-
private final AggregationPipeline pipeline;
48+
private final @Nullable AggregationPipeline pipeline;
5049

51-
@Nullable //
52-
private final Class<?> domainType;
50+
private final @Nullable Class<?> domainType;
5351

5452
public UnionWithOperation(String collection, @Nullable AggregationPipeline pipeline, @Nullable Class<?> domainType) {
5553

54+
Assert.notNull(collection, "Collection must not be null!");
55+
5656
this.collection = collection;
5757
this.pipeline = pipeline;
5858
this.domainType = domainType;
@@ -67,16 +67,14 @@ public UnionWithOperation(String collection, @Nullable AggregationPipeline pipel
6767
* @throws IllegalArgumentException if the required argument is {@literal null}.
6868
*/
6969
public static UnionWithOperation unionWith(String collection) {
70-
71-
Assert.notNull(collection, "Collection must not be null!");
7270
return new UnionWithOperation(collection, null, null);
7371
}
7472

7573
/**
7674
* Set the {@link AggregationPipeline} to apply to the specified collection. The pipeline corresponds to the optional
7775
* {@code pipeline} field of the {@code $unionWith} aggregation stage and is used to compute the documents going into
7876
* the result set.
79-
*
77+
*
8078
* @param pipeline the {@link AggregationPipeline} that computes the documents. Must not be {@literal null}.
8179
* @return new instance of {@link UnionWithOperation}.
8280
* @throws IllegalArgumentException if the required argument is {@literal null}.

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import org.springframework.lang.Nullable;
3131

3232
/**
33+
* Unit tests for {@link UnionWithOperation}.
34+
*
3335
* @author Christoph Strobl
3436
*/
3537
class UnionWithOperationUnitTests {

0 commit comments

Comments
 (0)