From 2127ddcbb8ad6f72660686b359d96bed9116502a Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 22 Mar 2018 11:02:33 +0100 Subject: [PATCH] DATAMONGO-1893 - Polishing. Inherit fields from previous operation if at least one field is excluded. Extend FieldsExposingAggregationOperation to conditionally inherit fields. Original pull request: #538. --- .../AggregationOperationRenderer.java | 2 +- .../FieldsExposingAggregationOperation.java | 19 +++++++++++++- .../core/aggregation/ProjectionOperation.java | 19 ++++++++++++++ .../ProjectionOperationUnitTests.java | 10 +++++++ ...dAggregationOperationContextUnitTests.java | 26 +++++++++++++------ 5 files changed, 66 insertions(+), 10 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java index 1ca3f3177..7fbf8b464 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AggregationOperationRenderer.java @@ -59,7 +59,7 @@ class AggregationOperationRenderer { FieldsExposingAggregationOperation exposedFieldsOperation = (FieldsExposingAggregationOperation) operation; ExposedFields fields = exposedFieldsOperation.getFields(); - if (operation instanceof InheritsFieldsAggregationOperation) { + if (operation instanceof InheritsFieldsAggregationOperation || exposedFieldsOperation.inheritsFields()) { contextToUse = new InheritingExposedFieldsAggregationOperationContext(fields, contextToUse); } else { contextToUse = fields.exposesNoFields() ? DEFAULT_CONTEXT diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/FieldsExposingAggregationOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/FieldsExposingAggregationOperation.java index 96cb88c1d..66df36567 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/FieldsExposingAggregationOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/FieldsExposingAggregationOperation.java @@ -33,9 +33,26 @@ public interface FieldsExposingAggregationOperation extends AggregationOperation */ ExposedFields getFields(); + /** + * @return {@literal true} to conditionally inherit fields from previous operations. + * @since 2.0.6 + */ + default boolean inheritsFields() { + return false; + } + /** * Marker interface for {@link AggregationOperation} that inherits fields from previous operations. */ - interface InheritsFieldsAggregationOperation extends FieldsExposingAggregationOperation {} + interface InheritsFieldsAggregationOperation extends FieldsExposingAggregationOperation { + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.aggregation.FieldsExposingAggregationOperation#inheritsFields() + */ + @Override + default boolean inheritsFields() { + return true; + } + } } 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 87a08c947..802681b03 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 @@ -183,6 +183,18 @@ public class ProjectionOperation implements FieldsExposingAggregationOperation { return fields != null ? fields : ExposedFields.empty(); } + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.aggregation.FieldsExposingAggregationOperation#inheritsFields() + */ + @Override + public boolean inheritsFields() { + + return projections.stream().filter(FieldProjection.class::isInstance) // + .map(FieldProjection.class::cast) // + .anyMatch(FieldProjection::isExcluded); + } + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.aggregation.AggregationOperation#toDocument(org.springframework.data.mongodb.core.aggregation.AggregationOperationContext) @@ -1339,6 +1351,13 @@ public class ProjectionOperation implements FieldsExposingAggregationOperation { return projections; } + /** + * @return {@literal true} if this field is excluded. + */ + public boolean isExcluded() { + return Boolean.FALSE.equals(value); + } + /* * (non-Javadoc) * @see org.springframework.data.mongodb.core.aggregation.ProjectionOperation.Projection#toDocument(org.springframework.data.mongodb.core.aggregation.AggregationOperationContext) diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperationUnitTests.java index 2af731b27..898efea51 100755 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/ProjectionOperationUnitTests.java @@ -204,9 +204,19 @@ public class ProjectionOperationUnitTests { ProjectionOperation projectionOp = new ProjectionOperation().andExclude("foo"); Document document = projectionOp.toDocument(Aggregation.DEFAULT_CONTEXT); Document projectClause = DocumentTestUtils.getAsDocument(document, PROJECT); + + assertThat(projectionOp.inheritsFields()).isTrue(); assertThat((Integer) projectClause.get("foo")).isEqualTo(0); } + @Test // DATAMONGO-1893 + public void includeShouldNotInheritFields() { + + ProjectionOperation projectionOp = new ProjectionOperation().andInclude("foo"); + + assertThat(projectionOp.inheritsFields()).isFalse(); + } + @Test // DATAMONGO-758 public void excludeShouldAllowExclusionOfUnderscoreId() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java index 2d5f514b0..bfdc969af 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/TypeBasedAggregationOperationContextUnitTests.java @@ -22,6 +22,8 @@ import static org.springframework.data.mongodb.core.aggregation.Aggregation.*; import static org.springframework.data.mongodb.core.aggregation.Fields.*; import static org.springframework.data.mongodb.test.util.IsBsonObject.*; +import lombok.AllArgsConstructor; + import java.util.Arrays; import java.util.List; @@ -35,8 +37,8 @@ import org.mockito.junit.MockitoJUnitRunner; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.support.GenericConversionService; import org.springframework.data.annotation.Id; -import org.springframework.data.annotation.PersistenceConstructor; import org.springframework.data.convert.CustomConversions; +import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Direction; import org.springframework.data.mapping.MappingException; import org.springframework.data.mongodb.core.aggregation.ExposedFields.DirectFieldReference; @@ -188,6 +190,19 @@ public class TypeBasedAggregationOperationContextUnitTests { .containing("age", "$age.value")); } + @Test // DATAMONGO-1893 + public void considersIncludedFieldsFromSingleExclusionsCorrectly() { + + AggregationOperationContext context = getContext(FooPerson.class); + TypedAggregation agg = newAggregation(FooPerson.class, project() // + .andExclude("name"), sort(Sort.by("age.value", "lastName"))); + + Document dbo = agg.toDocument("person", context); + + Document sort = getPipelineElementFromAggregationAt(dbo, 1); + assertThat(getAsDocument(sort, "$sort"), is(equalTo(new Document("age.value", 1).append("last_name", 1)))); + } + @Test // DATAMONGO-1133 public void shouldHonorAliasedFieldsInGroupExpressions() { @@ -344,18 +359,13 @@ public class TypeBasedAggregationOperationContextUnitTests { } @org.springframework.data.mongodb.core.mapping.Document(collection = "person") + @AllArgsConstructor public static class FooPerson { final ObjectId id; final String name; + @org.springframework.data.mongodb.core.mapping.Field("last_name") final String lastName; final Age age; - - @PersistenceConstructor - FooPerson(ObjectId id, String name, Age age) { - this.id = id; - this.name = name; - this.age = age; - } } public static class Age {