Skip to content

Commit 6b05bda

Browse files
this-aminechristophstrobl
authored andcommitted
Fix BucketOperationSupport _id field exposure.
Closes: #5046 Original Pull Request: #5047 Signed-off-by: Amine jaoui <[email protected]>
1 parent cc91246 commit 6b05bda

File tree

4 files changed

+37
-17
lines changed

4 files changed

+37
-17
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
*
3636
* @author Mark Paluch
3737
* @author Christoph Strobl
38+
* @author Amine Jaoui
3839
* @since 1.10
3940
*/
4041
public abstract class BucketOperationSupport<T extends BucketOperationSupport<T, B>, B extends OutputBuilder<B, T>>
@@ -415,12 +416,13 @@ private Outputs(Collection<Output> current, Output output) {
415416
*/
416417
protected ExposedFields asExposedFields() {
417418

419+
// The _id is always present after the bucket operation
420+
ExposedFields fields = ExposedFields.from(new ExposedField(Fields.UNDERSCORE_ID, true));
418421
// The count field is included by default when the output is not specified.
419422
if (isEmpty()) {
420-
return ExposedFields.from(new ExposedField("count", true));
423+
fields = fields.and(new ExposedField("count", true));
421424
}
422425

423-
ExposedFields fields = ExposedFields.from();
424426

425427
for (Output output : outputs) {
426428
fields = fields.and(output.getExposedField());

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
* @author Oliver Gierke
4747
* @author Christoph Strobl
4848
* @author Mark Paluch
49+
* @author Amine Jaoui
4950
* @since 1.3
5051
* @see <a href="https://docs.mongodb.com/manual/reference/operator/aggregation/project/">MongoDB Aggregation Framework:
5152
* $project</a>
@@ -1399,14 +1400,6 @@ private Object renderFieldValue(AggregationOperationContext context) {
13991400
return field.getTarget();
14001401
}
14011402

1402-
if (field.getTarget().equals(Fields.UNDERSCORE_ID)) {
1403-
try {
1404-
return context.getReference(field).getReferenceValue();
1405-
} catch (java.lang.IllegalArgumentException e) {
1406-
return Fields.UNDERSCORE_ID_REF;
1407-
}
1408-
}
1409-
14101403
// check whether referenced field exists in the context
14111404
return context.getReference(field).getReferenceValue();
14121405

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

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

1818
import static org.springframework.data.mongodb.core.DocumentTestUtils.*;
19+
import static org.springframework.data.mongodb.core.aggregation.AddFieldsOperation.addField;
1920
import static org.springframework.data.mongodb.core.aggregation.Aggregation.*;
2021
import static org.springframework.data.mongodb.core.query.Criteria.*;
2122
import static org.springframework.data.mongodb.test.util.Assertions.*;
@@ -50,6 +51,7 @@
5051
* @author Christoph Strobl
5152
* @author Mark Paluch
5253
* @author Julia Lee
54+
* @author Amine Jaoui
5355
*/
5456
public class AggregationUnitTests {
5557

@@ -594,8 +596,8 @@ void shouldAllowInternalThisAndValueReferences() {
594596
"{\"attributeRecordArrays\": {\"$reduce\": {\"input\": \"$attributeRecordArrays\", \"initialValue\": [], \"in\": {\"$concatArrays\": [\"$$value\", \"$$this\"]}}}}"));
595597
}
596598

597-
@Test // DATAMONGO-2644
598-
void projectOnIdIsAlwaysValid() {
599+
@Test // GH-5046
600+
void projectOnBucketIntervalId() {
599601

600602
MongoMappingContext mappingContext = new MongoMappingContext();
601603
Document target = new Aggregation(bucket("start"), project("_id")).toDocument("collection-1",
@@ -605,6 +607,15 @@ void projectOnIdIsAlwaysValid() {
605607
assertThat(extractPipelineElement(target, 1, "$project")).isEqualTo(Document.parse(" { \"_id\" : 1 }"));
606608
}
607609

610+
@Test // GH-5046
611+
void addFieldFromBucketIntervalId() {
612+
613+
Document target = new Aggregation(bucket("start"), addField("bucketLabel").withValueOf("_id").build())
614+
.toDocument("collection-1", DEFAULT_CONTEXT);
615+
assertThat(extractPipelineElement(target, 1, "$addFields"))
616+
.isEqualTo(Document.parse(" { \"bucketLabel\" : \"$_id\" }"));
617+
}
618+
608619
@Test // GH-3898
609620
void shouldNotConvertIncludeExcludeValuesForProjectOperation() {
610621

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
*/
1616
package org.springframework.data.mongodb.core.aggregation;
1717

18-
import static org.assertj.core.api.Assertions.*;
19-
import static org.springframework.data.mongodb.core.aggregation.Aggregation.*;
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
20+
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
21+
import static org.springframework.data.mongodb.core.aggregation.Aggregation.bucket;
2022

2123
import org.bson.Document;
2224
import org.junit.jupiter.api.Test;
@@ -25,6 +27,7 @@
2527
* Unit tests for {@link BucketOperation}.
2628
*
2729
* @author Mark Paluch
30+
* @author Amine Jaoui
2831
*/
2932
public class BucketOperationUnitTests {
3033

@@ -194,15 +197,26 @@ public void shouldRenderSumWithOwnOutputExpression() {
194197
.isEqualTo(Document.parse("{ total : { $multiply: [ {$add : [\"$netPrice\", \"$tax\"]}, 5] } }"));
195198
}
196199

197-
@Test // DATAMONGO-1552
198-
public void shouldExposeDefaultCountField() {
200+
@Test // GH-5046
201+
public void shouldExposeDefaultFields() {
199202

200203
BucketOperation operation = bucket("field");
201204

202-
assertThat(operation.getFields().exposesSingleFieldOnly()).isTrue();
205+
assertThat(operation.getFields().exposesNoNonSyntheticFields()).isTrue();
206+
assertThat(operation.getFields().getField(Fields.UNDERSCORE_ID)).isNotNull();
203207
assertThat(operation.getFields().getField("count")).isNotNull();
204208
}
205209

210+
@Test // GH-5046
211+
public void shouldExposeIDWhenCustomOutputField() {
212+
213+
BucketOperation operation = bucket("field").andOutputCount().as("score");
214+
215+
assertThat(operation.getFields().getField(Fields.UNDERSCORE_ID)).isNotNull();
216+
assertThat(operation.getFields().getField("score")).isNotNull();
217+
assertThat(operation.getFields().getField("count")).isNull();
218+
}
219+
206220
private static Document extractOutput(Document fromBucketClause) {
207221
return (Document) ((Document) fromBucketClause.get("$bucket")).get("output");
208222
}

0 commit comments

Comments
 (0)