Skip to content

Commit 39e8f64

Browse files
christophstroblmp911de
authored andcommitted
Fix conversion of types when mapping Aggregation pipeline.
This change makes sure to apply conversion to non native mongo types when the context does not expose fields. Closes: #4722 Original pull request: #4723
1 parent 7c255ec commit 39e8f64

File tree

2 files changed

+154
-4
lines changed

2 files changed

+154
-4
lines changed

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

+34-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ static List<Document> toDocument(List<AggregationOperation> operations, Aggregat
6262
if (operation instanceof InheritsFieldsAggregationOperation || exposedFieldsOperation.inheritsFields()) {
6363
contextToUse = contextToUse.inheritAndExpose(fields);
6464
} else {
65-
contextToUse = fields.exposesNoFields() ? DEFAULT_CONTEXT
65+
contextToUse = fields.exposesNoFields() ? ConverterAwareNoOpContext.instance(rootContext)
6666
: contextToUse.expose(fields);
6767
}
6868
}
@@ -72,6 +72,39 @@ static List<Document> toDocument(List<AggregationOperation> operations, Aggregat
7272
return operationDocuments;
7373
}
7474

75+
private static class ConverterAwareNoOpContext implements AggregationOperationContext {
76+
77+
AggregationOperationContext ctx;
78+
79+
static ConverterAwareNoOpContext instance(AggregationOperationContext ctx) {
80+
81+
if(ctx instanceof ConverterAwareNoOpContext noOpContext) {
82+
return noOpContext;
83+
}
84+
85+
return new ConverterAwareNoOpContext(ctx);
86+
}
87+
88+
ConverterAwareNoOpContext(AggregationOperationContext ctx) {
89+
this.ctx = ctx;
90+
}
91+
92+
@Override
93+
public Document getMappedObject(Document document, @Nullable Class<?> type) {
94+
return ctx.getMappedObject(document, null);
95+
}
96+
97+
@Override
98+
public FieldReference getReference(Field field) {
99+
return new DirectFieldReference(new ExposedField(field, true));
100+
}
101+
102+
@Override
103+
public FieldReference getReference(String name) {
104+
return new DirectFieldReference(new ExposedField(new AggregationField(name), true));
105+
}
106+
}
107+
75108
/**
76109
* Simple {@link AggregationOperationContext} that just returns {@link FieldReference}s as is.
77110
*

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

+120-3
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,33 @@
1515
*/
1616
package org.springframework.data.mongodb.core.aggregation;
1717

18-
import static org.mockito.Mockito.eq;
19-
import static org.mockito.Mockito.mock;
20-
import static org.mockito.Mockito.verify;
18+
import static org.mockito.Mockito.*;
19+
import static org.springframework.data.domain.Sort.Direction.*;
20+
import static org.springframework.data.mongodb.core.aggregation.Aggregation.*;
2121

22+
import java.time.ZonedDateTime;
2223
import java.util.List;
24+
import java.util.Set;
25+
import java.util.stream.Stream;
2326

27+
import org.assertj.core.api.Assertions;
28+
import org.bson.Document;
2429
import org.junit.jupiter.api.Test;
30+
import org.junit.jupiter.params.ParameterizedTest;
31+
import org.junit.jupiter.params.provider.Arguments;
32+
import org.junit.jupiter.params.provider.MethodSource;
33+
34+
import org.springframework.data.annotation.Id;
35+
import org.springframework.data.convert.ConverterBuilder;
36+
import org.springframework.data.convert.CustomConversions;
37+
import org.springframework.data.convert.CustomConversions.StoreConversions;
38+
import org.springframework.data.domain.Sort.Direction;
39+
import org.springframework.data.mongodb.core.convert.MappingMongoConverter;
40+
import org.springframework.data.mongodb.core.convert.NoOpDbRefResolver;
41+
import org.springframework.data.mongodb.core.convert.QueryMapper;
42+
import org.springframework.data.mongodb.core.mapping.Field;
43+
import org.springframework.data.mongodb.core.query.Criteria;
44+
import org.springframework.data.mongodb.test.util.MongoTestMappingContext;
2545

2646
/**
2747
* @author Christoph Strobl
@@ -40,4 +60,101 @@ void nonFieldsExposingAggregationOperationContinuesWithSameContextForNextStage()
4060
verify(stage1).toPipelineStages(eq(rootContext));
4161
verify(stage2).toPipelineStages(eq(rootContext));
4262
}
63+
64+
@Test
65+
void contextShouldCarryOnRelaxedFieldMapping() {
66+
67+
MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> {
68+
cfg.initialEntitySet(TestRecord.class);
69+
});
70+
71+
MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx);
72+
73+
Aggregation agg = Aggregation.newAggregation(Aggregation.unwind("layerOne.layerTwo"),
74+
project().and("layerOne.layerTwo.layerThree").as("layerOne.layerThree"),
75+
sort(DESC, "layerOne.layerThree.fieldA"));
76+
77+
AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(),
78+
new RelaxedTypeBasedAggregationOperationContext(TestRecord.class, ctx, new QueryMapper(mongoConverter)));
79+
}
80+
81+
@Test // GH-4722
82+
void appliesConversionToValuesUsedInAggregation() {
83+
84+
MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> {
85+
cfg.initialEntitySet(TestRecord.class);
86+
});
87+
88+
MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx);
89+
mongoConverter.setCustomConversions(new CustomConversions(StoreConversions.NONE,
90+
Set.copyOf(ConverterBuilder.writing(ZonedDateTime.class, String.class, ZonedDateTime::toString)
91+
.andReading(it -> ZonedDateTime.parse(it)).getConverters())));
92+
mongoConverter.afterPropertiesSet();
93+
94+
var agg = Aggregation.newAggregation(Aggregation.sort(Direction.DESC, "version"),
95+
Aggregation.group("entityId").first(Aggregation.ROOT).as("value"), Aggregation.replaceRoot("value"),
96+
Aggregation.match(Criteria.where("createdDate").lt(ZonedDateTime.now())) // here is the problem
97+
);
98+
99+
List<Document> document = AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(),
100+
new RelaxedTypeBasedAggregationOperationContext(TestRecord.class, ctx, new QueryMapper(mongoConverter)));
101+
Assertions.assertThat(document).last()
102+
.extracting(it -> it.getEmbedded(List.of("$match", "createdDate", "$lt"), Object.class))
103+
.isInstanceOf(String.class);
104+
}
105+
106+
@ParameterizedTest // GH-4722
107+
@MethodSource("studentAggregationContexts")
108+
void mapsOperationThatDoesNotExposeDedicatedFieldsCorrectly(AggregationOperationContext aggregationContext) {
109+
110+
var agg = newAggregation(Student.class, Aggregation.unwind("grades"), Aggregation.replaceRoot("grades"),
111+
Aggregation.project("grades"));
112+
113+
List<Document> mappedPipeline = AggregationOperationRenderer.toDocument(agg.getPipeline().getOperations(),
114+
aggregationContext);
115+
116+
Assertions.assertThat(mappedPipeline).last().isEqualTo(Document.parse("{\"$project\": {\"grades\": 1}}"));
117+
}
118+
119+
private static Stream<Arguments> studentAggregationContexts() {
120+
121+
MongoTestMappingContext ctx = new MongoTestMappingContext(cfg -> {
122+
cfg.initialEntitySet(Student.class);
123+
});
124+
125+
MappingMongoConverter mongoConverter = new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, ctx);
126+
mongoConverter.afterPropertiesSet();
127+
128+
QueryMapper queryMapper = new QueryMapper(mongoConverter);
129+
130+
return Stream.of(
131+
Arguments
132+
.of(new TypeBasedAggregationOperationContext(Student.class, ctx, queryMapper, FieldLookupPolicy.strict())),
133+
Arguments.of(
134+
new TypeBasedAggregationOperationContext(Student.class, ctx, queryMapper, FieldLookupPolicy.relaxed())));
135+
}
136+
137+
record TestRecord(@Id String field1, String field2, LayerOne layerOne) {
138+
record LayerOne(List<LayerTwo> layerTwo) {
139+
}
140+
141+
record LayerTwo(LayerThree layerThree) {
142+
}
143+
144+
record LayerThree(int fieldA, int fieldB) {
145+
}
146+
}
147+
148+
static class Student {
149+
150+
@Field("mark") List<Grade> grades;
151+
152+
}
153+
154+
static class Grade {
155+
156+
int points;
157+
String grades;
158+
}
159+
43160
}

0 commit comments

Comments
 (0)