Fix BucketOperationSupport _id field exposure.

Closes #5046

Signed-off-by: Amine jaoui <[email protected]>

Author: this-amine

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<T extends BucketOperationSupport<T, B>, B extends OutputBuilder<B, T>>
@@ -421,12 +422,13 @@ private Outputs(Collection<Output> 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());

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 <a href="https://docs.mongodb.com/manual/reference/operator/aggregation/project/">MongoDB Aggregation Framework:
  *      $project</a>
@@ -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();
 

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() {
 

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");
 	}