Skip to content

Commit b0f3ccc

Browse files
christophstroblchristophstrobl
committed
Polishing.
Use previous context instead of root for mapping objects within an Inheriting context. This avoids accidental mapping of fields against the root entity after eg. a projection stage. Add missing tests for AggregationOperationRenderer to ensure intended context propagation. Original Pull Request: #4459
1 parent fae0e70 commit b0f3ccc

File tree

5 files changed

+145
-6
lines changed

5 files changed

+145
-6
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ static List<Document> toDocument(List<AggregationOperation> operations, Aggregat
6363
contextToUse = new InheritingExposedFieldsAggregationOperationContext(fields, contextToUse);
6464
} else {
6565
contextToUse = fields.exposesNoFields() ? DEFAULT_CONTEXT
66-
: new ExposedFieldsAggregationOperationContext(exposedFieldsOperation.getFields(), contextToUse);
66+
: new ExposedFieldsAggregationOperationContext(fields, contextToUse);
6767
}
6868
}
6969
}

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

-5
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,6 @@ public Document getMappedObject(Document document, @Nullable Class<?> type) {
6060
return rootContext.getMappedObject(document, type);
6161
}
6262

63-
@Override
64-
public Document getMappedObject(Document document) {
65-
return rootContext.getMappedObject(document);
66-
}
67-
6863
@Override
6964
public FieldReference getReference(Field field) {
7065

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

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

18+
import org.bson.Document;
1819
import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference;
1920

2021
/**
2122
* {@link ExposedFieldsAggregationOperationContext} that inherits fields from its parent
2223
* {@link AggregationOperationContext}.
2324
*
2425
* @author Mark Paluch
26+
* @author Christoph Strobl
2527
* @since 1.9
2628
*/
2729
class InheritingExposedFieldsAggregationOperationContext extends ExposedFieldsAggregationOperationContext {
@@ -43,6 +45,11 @@ public InheritingExposedFieldsAggregationOperationContext(ExposedFields exposedF
4345
this.previousContext = previousContext;
4446
}
4547

48+
@Override
49+
public Document getMappedObject(Document document) {
50+
return previousContext.getMappedObject(document);
51+
}
52+
4653
@Override
4754
protected FieldReference resolveExposedField(Field field, String name) {
4855

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

+118
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright 2023 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+
* https://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.aggregation;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.mockito.Mockito.*;
20+
21+
import java.util.List;
22+
23+
import org.assertj.core.api.InstanceOfAssertFactories;
24+
import org.junit.jupiter.api.Test;
25+
import org.mockito.ArgumentCaptor;
26+
import org.springframework.data.mongodb.core.aggregation.FieldsExposingAggregationOperation.InheritsFieldsAggregationOperation;
27+
28+
/**
29+
* @author Christoph Strobl
30+
*/
31+
public class AggregationOperationRendererUnitTests {
32+
33+
@Test // GH-4443
34+
void nonFieldsExposingAggregationOperationContinuesWithSameContextForNextStage() {
35+
36+
AggregationOperationContext rootContext = mock(AggregationOperationContext.class);
37+
AggregationOperation stage1 = mock(AggregationOperation.class);
38+
AggregationOperation stage2 = mock(AggregationOperation.class);
39+
40+
AggregationOperationRenderer.toDocument(List.of(stage1, stage2), rootContext);
41+
42+
verify(stage1).toPipelineStages(eq(rootContext));
43+
verify(stage2).toPipelineStages(eq(rootContext));
44+
}
45+
46+
@Test // GH-4443
47+
void fieldsExposingAggregationOperationNotExposingFieldsForcesUseOfDefaultContextForNextStage() {
48+
49+
AggregationOperationContext rootContext = mock(AggregationOperationContext.class);
50+
FieldsExposingAggregationOperation stage1 = mock(FieldsExposingAggregationOperation.class);
51+
ExposedFields stage1fields = mock(ExposedFields.class);
52+
AggregationOperation stage2 = mock(AggregationOperation.class);
53+
54+
when(stage1.getFields()).thenReturn(stage1fields);
55+
when(stage1fields.exposesNoFields()).thenReturn(true);
56+
57+
AggregationOperationRenderer.toDocument(List.of(stage1, stage2), rootContext);
58+
59+
verify(stage1).toPipelineStages(eq(rootContext));
60+
verify(stage2).toPipelineStages(eq(AggregationOperationRenderer.DEFAULT_CONTEXT));
61+
}
62+
63+
@Test // GH-4443
64+
void fieldsExposingAggregationOperationForcesNewContextForNextStage() {
65+
66+
AggregationOperationContext rootContext = mock(AggregationOperationContext.class);
67+
FieldsExposingAggregationOperation stage1 = mock(FieldsExposingAggregationOperation.class);
68+
ExposedFields stage1fields = mock(ExposedFields.class);
69+
AggregationOperation stage2 = mock(AggregationOperation.class);
70+
71+
when(stage1.getFields()).thenReturn(stage1fields);
72+
when(stage1fields.exposesNoFields()).thenReturn(false);
73+
74+
ArgumentCaptor<AggregationOperationContext> captor = ArgumentCaptor.forClass(AggregationOperationContext.class);
75+
76+
AggregationOperationRenderer.toDocument(List.of(stage1, stage2), rootContext);
77+
78+
verify(stage1).toPipelineStages(eq(rootContext));
79+
verify(stage2).toPipelineStages(captor.capture());
80+
81+
assertThat(captor.getValue()).isInstanceOf(ExposedFieldsAggregationOperationContext.class)
82+
.isNotInstanceOf(InheritingExposedFieldsAggregationOperationContext.class);
83+
}
84+
85+
@Test // GH-4443
86+
void inheritingFieldsExposingAggregationOperationForcesNewContextForNextStageKeepingReferenceToPreviousContext() {
87+
88+
AggregationOperationContext rootContext = mock(AggregationOperationContext.class);
89+
InheritsFieldsAggregationOperation stage1 = mock(InheritsFieldsAggregationOperation.class);
90+
InheritsFieldsAggregationOperation stage2 = mock(InheritsFieldsAggregationOperation.class);
91+
InheritsFieldsAggregationOperation stage3 = mock(InheritsFieldsAggregationOperation.class);
92+
93+
ExposedFields exposedFields = mock(ExposedFields.class);
94+
when(exposedFields.exposesNoFields()).thenReturn(false);
95+
when(stage1.getFields()).thenReturn(exposedFields);
96+
when(stage2.getFields()).thenReturn(exposedFields);
97+
when(stage3.getFields()).thenReturn(exposedFields);
98+
99+
ArgumentCaptor<AggregationOperationContext> captor = ArgumentCaptor.forClass(AggregationOperationContext.class);
100+
101+
AggregationOperationRenderer.toDocument(List.of(stage1, stage2, stage3), rootContext);
102+
103+
verify(stage1).toPipelineStages(captor.capture());
104+
verify(stage2).toPipelineStages(captor.capture());
105+
verify(stage3).toPipelineStages(captor.capture());
106+
107+
assertThat(captor.getAllValues().get(0)).isEqualTo(rootContext);
108+
109+
assertThat(captor.getAllValues().get(1))
110+
.asInstanceOf(InstanceOfAssertFactories.type(InheritingExposedFieldsAggregationOperationContext.class))
111+
.extracting("previousContext").isSameAs(captor.getAllValues().get(0));
112+
113+
assertThat(captor.getAllValues().get(2))
114+
.asInstanceOf(InstanceOfAssertFactories.type(InheritingExposedFieldsAggregationOperationContext.class))
115+
.extracting("previousContext").isSameAs(captor.getAllValues().get(1));
116+
}
117+
118+
}

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

+19
Original file line numberDiff line numberDiff line change
@@ -2010,6 +2010,25 @@ void shouldHonorFieldAliasesForFieldReferencesUsingFieldExposingOperation() {
20102010
assertThat(mappedResults.get(0)).containsEntry("item_id", "1");
20112011
}
20122012

2013+
@Test // GH-4443
2014+
void projectShouldResetContextToAvoidMappingFieldsAgainstANoLongerExistingTarget() {
2015+
2016+
Item item1 = Item.builder().itemId("1").tags(Arrays.asList("a", "b")).build();
2017+
Item item2 = Item.builder().itemId("1").tags(Arrays.asList("a", "c")).build();
2018+
mongoTemplate.insert(Arrays.asList(item1, item2), Item.class);
2019+
2020+
TypedAggregation<Item> aggregation = newAggregation(Item.class,
2021+
match(where("itemId").is("1")),
2022+
unwind("tags"),
2023+
project().and("itemId").as("itemId").and("tags").as("tags"),
2024+
match(where("itemId").is("1").and("tags").is("c")));
2025+
2026+
AggregationResults<Document> results = mongoTemplate.aggregate(aggregation, Document.class);
2027+
List<Document> mappedResults = results.getMappedResults();
2028+
assertThat(mappedResults).hasSize(1);
2029+
assertThat(mappedResults.get(0)).containsEntry("itemId", "1");
2030+
}
2031+
20132032
private void createUsersWithReferencedPersons() {
20142033

20152034
mongoTemplate.dropCollection(User.class);

0 commit comments

Comments
 (0)