Skip to content

Commit ef7870a

Browse files
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 e1faf26 commit ef7870a

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
@@ -64,7 +64,7 @@ static List<Document> toDocument(List<AggregationOperation> operations, Aggregat
6464
contextToUse = new InheritingExposedFieldsAggregationOperationContext(fields, contextToUse);
6565
} else {
6666
contextToUse = fields.exposesNoFields() ? DEFAULT_CONTEXT
67-
: new ExposedFieldsAggregationOperationContext(exposedFieldsOperation.getFields(), contextToUse);
67+
: new ExposedFieldsAggregationOperationContext(fields, contextToUse);
6868
}
6969
}
7070
}

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

-5
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,6 @@ public Document getMappedObject(Document document, @Nullable Class<?> type) {
6767
* (non-Javadoc)
6868
* @see org.springframework.data.mongodb.core.aggregation.AggregationOperationContext#getReference(org.springframework.data.mongodb.core.aggregation.ExposedFields.AvailableField)
6969
*/
70-
@Override
71-
public Document getMappedObject(Document document) {
72-
return rootContext.getMappedObject(document);
73-
}
74-
7570
@Override
7671
public FieldReference getReference(Field field) {
7772

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 {
@@ -47,6 +49,11 @@ public InheritingExposedFieldsAggregationOperationContext(ExposedFields exposedF
4749
* (non-Javadoc)
4850
* @see org.springframework.data.mongodb.core.aggregation.ExposedFieldsAggregationOperationContext#resolveExposedField(org.springframework.data.mongodb.core.aggregation.Field, java.lang.String)
4951
*/
52+
@Override
53+
public Document getMappedObject(Document document) {
54+
return previousContext.getMappedObject(document);
55+
}
56+
5057
@Override
5158
protected FieldReference resolveExposedField(Field field, String name) {
5259

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
@@ -1980,6 +1980,25 @@ void shouldHonorFieldAliasesForFieldReferencesUsingFieldExposingOperation() {
19801980
assertThat(mappedResults.get(0)).containsEntry("item_id", "1");
19811981
}
19821982

1983+
@Test // GH-4443
1984+
void projectShouldResetContextToAvoidMappingFieldsAgainstANoLongerExistingTarget() {
1985+
1986+
Item item1 = Item.builder().itemId("1").tags(Arrays.asList("a", "b")).build();
1987+
Item item2 = Item.builder().itemId("1").tags(Arrays.asList("a", "c")).build();
1988+
mongoTemplate.insert(Arrays.asList(item1, item2), Item.class);
1989+
1990+
TypedAggregation<Item> aggregation = newAggregation(Item.class,
1991+
match(where("itemId").is("1")),
1992+
unwind("tags"),
1993+
project().and("itemId").as("itemId").and("tags").as("tags"),
1994+
match(where("itemId").is("1").and("tags").is("c")));
1995+
1996+
AggregationResults<Document> results = mongoTemplate.aggregate(aggregation, Document.class);
1997+
List<Document> mappedResults = results.getMappedResults();
1998+
assertThat(mappedResults).hasSize(1);
1999+
assertThat(mappedResults.get(0)).containsEntry("itemId", "1");
2000+
}
2001+
19832002
private void createUsersWithReferencedPersons() {
19842003

19852004
mongoTemplate.dropCollection(User.class);

0 commit comments

Comments
 (0)