From 033b68fa7a5ce84fa5a078cc2a8fb7de5fc0aa99 Mon Sep 17 00:00:00 2001 From: amine jaoui Date: Tue, 19 Aug 2025 18:04:41 +0100 Subject: [PATCH] Fix BucketOperationSupport _id field exposure. Closes #5046 Signed-off-by: Amine jaoui --- .../aggregation/BucketOperationSupport.java | 6 +++-- .../core/aggregation/ProjectionOperation.java | 9 +------ .../aggregation/AggregationUnitTests.java | 23 +++++++++++------- .../aggregation/BucketOperationUnitTests.java | 24 +++++++++++++++---- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/BucketOperationSupport.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/BucketOperationSupport.java index 3d5ded05c2..3309c1817a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/BucketOperationSupport.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/BucketOperationSupport.java @@ -36,6 +36,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author Amine Jaoui * @since 1.10 */ public abstract class BucketOperationSupport, B extends OutputBuilder> @@ -421,12 +422,13 @@ private Outputs(Collection current, Output output) { */ protected ExposedFields asExposedFields() { + // The _id is always present after the bucket operation + ExposedFields fields = ExposedFields.from(new ExposedField(Fields.UNDERSCORE_ID, true)); // The count field is included by default when the output is not specified. if (isEmpty()) { - return ExposedFields.from(new ExposedField("count", true)); + fields = fields.and(new ExposedField("count", true)); } - ExposedFields fields = ExposedFields.from(); for (Output output : outputs) { fields = fields.and(output.getExposedField()); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java index af7cf5bfb2..286cdea926 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperation.java @@ -47,6 +47,7 @@ * @author Oliver Gierke * @author Christoph Strobl * @author Mark Paluch + * @author Amine Jaoui * @since 1.3 * @see MongoDB Aggregation Framework: * $project @@ -1404,14 +1405,6 @@ private Object renderFieldValue(AggregationOperationContext context) { return field.getTarget(); } - if (field.getTarget().equals(Fields.UNDERSCORE_ID)) { - try { - return context.getReference(field).getReferenceValue(); - } catch (java.lang.IllegalArgumentException e) { - return Fields.UNDERSCORE_ID_REF; - } - } - // check whether referenced field exists in the context return context.getReference(field).getReferenceValue(); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java index 8e18f511b2..2c4749f922 100755 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationUnitTests.java @@ -16,6 +16,7 @@ package org.springframework.data.mongodb.core.aggregation; import static org.springframework.data.mongodb.core.DocumentTestUtils.getAsDocument; +import static org.springframework.data.mongodb.core.aggregation.AddFieldsOperation.addField; import static org.springframework.data.mongodb.core.aggregation.Aggregation.DEFAULT_CONTEXT; import static org.springframework.data.mongodb.core.aggregation.Aggregation.bucket; import static org.springframework.data.mongodb.core.aggregation.Aggregation.group; @@ -43,7 +44,6 @@ import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.mongodb.core.aggregation.ConditionalOperators.Cond; -import org.springframework.data.mongodb.core.aggregation.ProjectionOperationUnitTests.BookWithFieldAnnotation; import org.springframework.data.mongodb.core.convert.MappingMongoConverter; import org.springframework.data.mongodb.core.convert.NoOpDbRefResolver; import org.springframework.data.mongodb.core.convert.QueryMapper; @@ -61,6 +61,7 @@ * @author Christoph Strobl * @author Mark Paluch * @author Julia Lee + * @author Amine Jaoui */ public class AggregationUnitTests { @@ -605,18 +606,22 @@ void shouldAllowInternalThisAndValueReferences() { "{\"attributeRecordArrays\": {\"$reduce\": {\"input\": \"$attributeRecordArrays\", \"initialValue\": [], \"in\": {\"$concatArrays\": [\"$$value\", \"$$this\"]}}}}")); } - @Test // DATAMONGO-2644 - void projectOnIdIsAlwaysValid() { - - MongoMappingContext mappingContext = new MongoMappingContext(); - Document target = new Aggregation(bucket("start"), project("_id")).toDocument("collection-1", - new TypeBasedAggregationOperationContext(BookWithFieldAnnotation.class, mappingContext, - new QueryMapper(new MappingMongoConverter(NoOpDbRefResolver.INSTANCE, mappingContext)), - FieldLookupPolicy.relaxed())); + @Test // GH-5046 + void projectOnBucketIntervalId() { + Document target = new Aggregation(bucket("start"), project("_id")).toDocument("collection-1", DEFAULT_CONTEXT); assertThat(extractPipelineElement(target, 1, "$project")).isEqualTo(Document.parse(" { \"_id\" : 1 }")); } + @Test // GH-5046 + void addFieldFromBucketIntervalId() { + + Document target = new Aggregation(bucket("start"), addField("bucketLabel").withValueOf("_id").build()) + .toDocument("collection-1", DEFAULT_CONTEXT); + assertThat(extractPipelineElement(target, 1, "$addFields")) + .isEqualTo(Document.parse(" { \"bucketLabel\" : \"$_id\" }")); + } + @Test // GH-3898 void shouldNotConvertIncludeExcludeValuesForProjectOperation() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/BucketOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/BucketOperationUnitTests.java index 36a943d1c1..1d136750fe 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/BucketOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/BucketOperationUnitTests.java @@ -15,8 +15,10 @@ */ package org.springframework.data.mongodb.core.aggregation; -import static org.assertj.core.api.Assertions.*; -import static org.springframework.data.mongodb.core.aggregation.Aggregation.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.springframework.data.mongodb.core.aggregation.Aggregation.bucket; import org.bson.Document; import org.junit.jupiter.api.Test; @@ -25,6 +27,7 @@ * Unit tests for {@link BucketOperation}. * * @author Mark Paluch + * @author Amine Jaoui */ public class BucketOperationUnitTests { @@ -194,15 +197,26 @@ public void shouldRenderSumWithOwnOutputExpression() { .isEqualTo(Document.parse("{ total : { $multiply: [ {$add : [\"$netPrice\", \"$tax\"]}, 5] } }")); } - @Test // DATAMONGO-1552 - public void shouldExposeDefaultCountField() { + @Test // GH-5046 + public void shouldExposeDefaultFields() { BucketOperation operation = bucket("field"); - assertThat(operation.getFields().exposesSingleFieldOnly()).isTrue(); + assertThat(operation.getFields().exposesNoNonSyntheticFields()).isTrue(); + assertThat(operation.getFields().getField(Fields.UNDERSCORE_ID)).isNotNull(); assertThat(operation.getFields().getField("count")).isNotNull(); } + @Test // GH-5046 + public void shouldExposeIDWhenCustomOutputField() { + + BucketOperation operation = bucket("field").andOutputCount().as("score"); + + assertThat(operation.getFields().getField(Fields.UNDERSCORE_ID)).isNotNull(); + assertThat(operation.getFields().getField("score")).isNotNull(); + assertThat(operation.getFields().getField("count")).isNull(); + } + private static Document extractOutput(Document fromBucketClause) { return (Document) ((Document) fromBucketClause.get("$bucket")).get("output"); }