Skip to content

Commit fdb4781

Browse files
author
Thomas Darimont
committed
DATAMONGO-753 - Add support for nested field references in group operations.
Introduced FieldsExposingAggregationOperation to mark AggregationOperations that change the set of exposed fields available for processing by later AggregationOperations. Extracted context state out of AggregationOperation to ExposedFieldsAggregationContext for better separation of concerns. Modified toDbObject in Aggregation to only replace the aggregation context when the "current" AggregationOperation was a FieldExposingAggregationOperation. Removed the getFields() method of UnwindOperation and changed it's type to just AggregationOperation since the UnwindOperation doesn't expose new fields to the context and retains all fields that are currently available. Resolution of nested field references are working now.
1 parent eeb74fb commit fdb4781

File tree

9 files changed

+143
-74
lines changed

9 files changed

+143
-74
lines changed

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

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ public DBObject toDbObject(String inputCollectionName, AggregationOperationConte
228228
operationDocuments.add(operation.toDBObject(context));
229229

230230
if (operation instanceof FieldsExposingAggregationOperation) {
231-
context = new CombiningAggregationOperationContext(context, (FieldsExposingAggregationOperation) operation);
231+
context = new WrappingExposedFieldsAggregationOperationContext((FieldsExposingAggregationOperation) operation);
232232
}
233233
}
234234

@@ -282,30 +282,4 @@ public FieldReference getReference(String name) {
282282
return new FieldReference(new ExposedField(new AggregationField(name), true));
283283
}
284284
}
285-
286-
private static class CombiningAggregationOperationContext extends ExposedFieldsAggregationOperationContext {
287-
288-
private final AggregationOperationContext aggregationOperationContext;
289-
private final FieldsExposingAggregationOperation fieldExposingOperation;
290-
291-
public CombiningAggregationOperationContext(AggregationOperationContext aggregationOperationContext,
292-
FieldsExposingAggregationOperation fieldExposingOperation) {
293-
this.aggregationOperationContext = aggregationOperationContext;
294-
this.fieldExposingOperation = fieldExposingOperation;
295-
}
296-
297-
@Override
298-
public FieldReference getReference(String name) {
299-
if (hasReferenceFor(name)) {
300-
return super.getReference(name);
301-
} else {
302-
return aggregationOperationContext.getReference(name);
303-
}
304-
}
305-
306-
@Override
307-
protected ExposedFields getFields() {
308-
return fieldExposingOperation.getFields();
309-
}
310-
}
311285
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,22 @@ public static ExposedFields from(ExposedField... fields) {
4949
return from(Arrays.asList(fields));
5050
}
5151

52+
/**
53+
* Creates a new {@link ExposedFields} instance from the given {@link ExposedFields}.
54+
*
55+
* @param fields must not be {@literal null}.
56+
* @return
57+
*/
58+
public static ExposedFields from(ExposedFields fields) {
59+
60+
List<ExposedField> exposedFields = new ArrayList<ExposedFields.ExposedField>();
61+
for (ExposedField field : fields) {
62+
exposedFields.add(field);
63+
}
64+
65+
return from(exposedFields);
66+
}
67+
5268
/**
5369
* Creates a new {@link ExposedFields} instance from the given {@link ExposedField}s.
5470
*
@@ -134,6 +150,24 @@ public ExposedFields and(ExposedField field) {
134150
return new ExposedFields(field.synthetic ? originalFields : result, field.synthetic ? result : syntheticFields);
135151
}
136152

153+
/**
154+
* Creates a new {@link ExposedFields} adding the given {@link ExposedFields}.
155+
*
156+
* @param field must not be {@literal null}.
157+
* @return
158+
*/
159+
public ExposedFields and(ExposedFields fields) {
160+
161+
Assert.notNull(fields, "Exposed fields must not be null!");
162+
163+
ExposedFields result = from(this);
164+
for (ExposedField field : fields) {
165+
result = result.and(field);
166+
}
167+
168+
return result;
169+
}
170+
137171
/**
138172
* Returns the field with the given name or {@literal null} if no field with the given name is available.
139173
*

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

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package org.springframework.data.mongodb.core.aggregation;
1717

18-
import org.springframework.data.mongodb.core.aggregation.ExposedFields.ExposedField;
1918
import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference;
2019

2120
import com.mongodb.DBObject;
@@ -46,29 +45,4 @@ public DBObject getMappedObject(DBObject dbObject) {
4645
public FieldReference getReference(Field field) {
4746
return getReference(field.getTarget());
4847
}
49-
50-
/*
51-
* (non-Javadoc)
52-
* @see org.springframework.data.mongodb.core.aggregation.AggregationOperationContext#getReference(java.lang.String)
53-
*/
54-
@Override
55-
public FieldReference getReference(String name) {
56-
57-
ExposedField field = getFields().getField(name);
58-
59-
if (field != null) {
60-
return new FieldReference(field);
61-
}
62-
63-
throw new IllegalArgumentException(String.format("Invalid reference '%s'!", name));
64-
}
65-
66-
protected abstract ExposedFields getFields();
67-
68-
/* (non-Javadoc)
69-
* @see org.springframework.data.mongodb.core.aggregation.AggregationOperationContext#hasReferenceFor(java.lang.String)
70-
*/
71-
protected boolean hasReferenceFor(String name) {
72-
return getFields().getField(name) != null;
73-
}
7448
}

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
package org.springframework.data.mongodb.core.aggregation;
1717

1818
/**
19+
* {@link AggregationOperation} that exposes new {@link ExposedFields} that can be used for later aggregation pipeline
20+
* {@code AggregationOperation}s.
21+
*
1922
* @author Thomas Darimont
2023
*/
2124
public interface FieldsExposingAggregationOperation extends AggregationOperation {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
* @author Oliver Gierke
3737
* @since 1.3
3838
*/
39-
public class TypeBasedAggregationOperationContext implements AggregationOperationContext {
39+
public class TypeBasedAggregationOperationContext extends ExposedFieldsAggregationOperationContext {
4040

4141
private final Class<?> type;
4242
private final MappingContext<? extends MongoPersistentEntity<?>, MongoPersistentProperty> mappingContext;

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
* @author Oliver Gierke
3030
* @since 1.3
3131
*/
32-
public class UnwindOperation implements FieldsExposingAggregationOperation {
32+
public class UnwindOperation implements AggregationOperation {
3333

3434
private final ExposedField field;
3535

@@ -44,15 +44,6 @@ public UnwindOperation(Field field) {
4444
this.field = new ExposedField(field, true);
4545
}
4646

47-
/*
48-
* (non-Javadoc)
49-
* @see org.springframework.data.mongodb.core.aggregation.ExposedFieldsAggregationOperationContext#getFields()
50-
*/
51-
@Override
52-
public ExposedFields getFields() {
53-
return ExposedFields.from(field);
54-
}
55-
5647
/*
5748
* (non-Javadoc)
5849
* @see org.springframework.data.mongodb.core.aggregation.AggregationOperation#toDBObject(org.springframework.data.mongodb.core.aggregation.AggregationOperationContext)
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package org.springframework.data.mongodb.core.aggregation;
2+
3+
import org.springframework.data.mongodb.core.aggregation.ExposedFields.ExposedField;
4+
import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference;
5+
6+
/**
7+
* {@link AggregationOperationContext} that combines the available field references from a given
8+
* {@code AggregationOperationContext} and an {@link FieldsExposingAggregationOperation}.
9+
*
10+
* @author Thomas Darimont
11+
* @since 1.4
12+
*/
13+
class WrappingExposedFieldsAggregationOperationContext extends ExposedFieldsAggregationOperationContext {
14+
15+
private final FieldsExposingAggregationOperation fieldExposingOperation;
16+
17+
/**
18+
* Creates a new {@link WrappingExposedFieldsAggregationOperationContext} from the given
19+
* {@link FieldsExposingAggregationOperation}.
20+
*
21+
* @param fieldExposingOperation
22+
*/
23+
public WrappingExposedFieldsAggregationOperationContext(FieldsExposingAggregationOperation fieldExposingOperation) {
24+
this.fieldExposingOperation = fieldExposingOperation;
25+
}
26+
27+
/* (non-Javadoc)
28+
* @see org.springframework.data.mongodb.core.aggregation.ExposedFieldsAggregationOperationContext#getFields()
29+
*/
30+
private ExposedFields getFields() {
31+
return fieldExposingOperation.getFields();
32+
}
33+
34+
/*
35+
* (non-Javadoc)
36+
* @see org.springframework.data.mongodb.core.aggregation.AggregationOperationContext#getReference(java.lang.String)
37+
*/
38+
@Override
39+
public FieldReference getReference(String name) {
40+
41+
ExposedField field = getFields().getField(name);
42+
43+
if (field != null) {
44+
return new FieldReference(field);
45+
}
46+
47+
throw new IllegalArgumentException(String.format("Invalid reference '%s'!", name));
48+
}
49+
}

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -457,33 +457,34 @@ public void arithmenticOperatorsInProjectionExample() {
457457

458458
/**
459459
* @see DATAMONGO-753
460+
* @see http
461+
* ://stackoverflow.com/questions/18653574/spring-data-mongodb-aggregation-framework-invalid-reference-in-group
462+
* -operati
460463
*/
461464
@Test
462-
public void foo() {
465+
public void allowNestedFieldReferencesAsGroupIdsInGroupExpressions() {
463466

464467
mongoTemplate.insert(new DATAMONGO753().withPDs(new PD("A", 1), new PD("B", 1), new PD("C", 1)));
465468
mongoTemplate.insert(new DATAMONGO753().withPDs(new PD("B", 1), new PD("B", 1), new PD("C", 1)));
466469

467470
Aggregation agg = newAggregation( //
468471
unwind("pd"), //
469-
group("pd.pDch") //
472+
group("pd.pDch") // the nested field expression
470473
.sum("pd.up").as("uplift") //
471-
);
474+
, project("_id", "uplift"));
472475

473476
AggregationResults<DBObject> result = mongoTemplate.aggregate(agg, //
474477
DATAMONGO753.class //
475-
// "dATAMONGO753" //
476478
, DBObject.class);
477-
478479
List<DBObject> stats = result.getMappedResults();
480+
479481
assertThat(stats.size(), is(3));
480482
assertThat(stats.get(0).get("_id").toString(), is("C"));
481483
assertThat((Integer) stats.get(0).get("uplift"), is(2));
482484
assertThat(stats.get(1).get("_id").toString(), is("B"));
483485
assertThat((Integer) stats.get(1).get("uplift"), is(3));
484486
assertThat(stats.get(2).get("_id").toString(), is("A"));
485487
assertThat((Integer) stats.get(2).get("uplift"), is(1));
486-
487488
}
488489

489490
private void assertLikeStats(LikeStats like, String id, long count) {

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

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,32 +15,75 @@
1515
*/
1616
package org.springframework.data.mongodb.core.aggregation;
1717

18+
import static org.springframework.data.mongodb.core.aggregation.Aggregation.*;
19+
import static org.springframework.data.mongodb.core.query.Criteria.*;
20+
1821
import org.junit.Test;
1922

2023
/**
2124
* Unit tests for {@link Aggregation}.
2225
*
2326
* @author Oliver Gierke
27+
* @author Thomas Darimont
2428
*/
2529
public class AggregationUnitTests {
2630

2731
@Test(expected = IllegalArgumentException.class)
2832
public void rejectsNullAggregationOperation() {
29-
Aggregation.newAggregation((AggregationOperation[]) null);
33+
newAggregation((AggregationOperation[]) null);
3034
}
3135

3236
@Test(expected = IllegalArgumentException.class)
3337
public void rejectsNullTypedAggregationOperation() {
34-
Aggregation.newAggregation(String.class, (AggregationOperation[]) null);
38+
newAggregation(String.class, (AggregationOperation[]) null);
3539
}
3640

3741
@Test(expected = IllegalArgumentException.class)
3842
public void rejectsNoAggregationOperation() {
39-
Aggregation.newAggregation(new AggregationOperation[0]);
43+
newAggregation(new AggregationOperation[0]);
4044
}
4145

4246
@Test(expected = IllegalArgumentException.class)
4347
public void rejectsNoTypedAggregationOperation() {
44-
Aggregation.newAggregation(String.class, new AggregationOperation[0]);
48+
newAggregation(String.class, new AggregationOperation[0]);
49+
}
50+
51+
/**
52+
* @see DATAMONGO-753
53+
*/
54+
@Test(expected = IllegalArgumentException.class)
55+
public void checkForCorrectFieldScopeTransfer() {
56+
57+
newAggregation( //
58+
project("a", "b"), //
59+
group("a").count().as("cnt"), // a was introduced to the context by the project operation
60+
project("cnt", "b") // b was removed from the context by the group operation
61+
).toDbObject("foo", Aggregation.DEFAULT_CONTEXT); // -> triggers IllegalArgumentException
62+
}
63+
64+
/**
65+
* @see DATAMONGO-753
66+
*/
67+
@Test
68+
public void unwindOperationShouldNotChangeAvailableFields() {
69+
70+
newAggregation( //
71+
project("a", "b"), //
72+
unwind("a"), //
73+
project("a", "b") // b should still be available
74+
).toDbObject("foo", Aggregation.DEFAULT_CONTEXT);
75+
}
76+
77+
/**
78+
* @see DATAMONGO-753
79+
*/
80+
@Test
81+
public void matchOperationShouldNotChangeAvailableFields() {
82+
83+
newAggregation( //
84+
project("a", "b"), //
85+
match(where("a").gte(1)), //
86+
project("a", "b") // b should still be available
87+
).toDbObject("foo", Aggregation.DEFAULT_CONTEXT);
4588
}
4689
}

0 commit comments

Comments
 (0)