diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java index 848c5cccd..0529c5110 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ExposedFields.java @@ -29,6 +29,7 @@ import org.springframework.util.CompositeIterator; * Value object to capture the fields exposed by an {@link AggregationOperation}. * * @author Oliver Gierke + * @author Thomas Darimont * @since 1.3 */ public class ExposedFields implements Iterable { @@ -151,13 +152,47 @@ public class ExposedFields implements Iterable { return null; } + /** + * Returns whether the {@link ExposedFields} exposes no non-synthetic fields at all. + * + * @return + */ + boolean exposesNoNonSyntheticFields() { + return originalFields.isEmpty(); + } + + /** + * Returns whether the {@link ExposedFields} exposes a single non-synthetic field only. + * + * @return + */ + boolean exposesSingleNonSyntheticFieldOnly() { + return originalFields.size() == 1; + } + + /** + * Returns whether the {@link ExposedFields} exposes no fields at all. + * + * @return + */ + boolean exposesNoFields() { + return exposedFieldsCount() == 0; + } + /** * Returns whether the {@link ExposedFields} exposes a single field only. * * @return */ - public boolean exposesSingleFieldOnly() { - return originalFields.size() + syntheticFields.size() == 1; + boolean exposesSingleFieldOnly() { + return exposedFieldsCount() == 1; + } + + /** + * @return + */ + private int exposedFieldsCount() { + return originalFields.size() + syntheticFields.size(); } /* diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GroupOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GroupOperation.java index 9d16ee1bb..521a2bf85 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GroupOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/GroupOperation.java @@ -40,7 +40,11 @@ import com.mongodb.DBObject; */ public class GroupOperation implements FieldsExposingAggregationOperation { - private final ExposedFields nonSynthecticFields; + /** + * Holds the non-synthetic fields which are the fields of the group-id structure. + */ + private final ExposedFields idFields; + private final List operations; /** @@ -50,7 +54,7 @@ public class GroupOperation implements FieldsExposingAggregationOperation { */ public GroupOperation(Fields fields) { - this.nonSynthecticFields = ExposedFields.nonSynthetic(fields); + this.idFields = ExposedFields.nonSynthetic(fields); this.operations = new ArrayList(); } @@ -74,7 +78,7 @@ public class GroupOperation implements FieldsExposingAggregationOperation { Assert.notNull(groupOperation, "GroupOperation must not be null!"); Assert.notNull(nextOperations, "NextOperations must not be null!"); - this.nonSynthecticFields = groupOperation.nonSynthecticFields; + this.idFields = groupOperation.idFields; this.operations = new ArrayList(nextOperations.size() + 1); this.operations.addAll(groupOperation.operations); this.operations.addAll(nextOperations); @@ -261,7 +265,7 @@ public class GroupOperation implements FieldsExposingAggregationOperation { @Override public ExposedFields getFields() { - ExposedFields fields = this.nonSynthecticFields.and(new ExposedField(Fields.UNDERSCORE_ID, true)); + ExposedFields fields = this.idFields.and(new ExposedField(Fields.UNDERSCORE_ID, true)); for (Operation operation : operations) { fields = fields.and(operation.asField()); @@ -279,16 +283,20 @@ public class GroupOperation implements FieldsExposingAggregationOperation { BasicDBObject operationObject = new BasicDBObject(); - if (nonSynthecticFields.exposesSingleFieldOnly()) { + if (idFields.exposesNoNonSyntheticFields()) { - FieldReference reference = context.getReference(nonSynthecticFields.iterator().next()); + operationObject.put(Fields.UNDERSCORE_ID, null); + + } else if (idFields.exposesSingleNonSyntheticFieldOnly()) { + + FieldReference reference = context.getReference(idFields.iterator().next()); operationObject.put(Fields.UNDERSCORE_ID, reference.toString()); } else { BasicDBObject inner = new BasicDBObject(); - for (ExposedField field : nonSynthecticFields) { + for (ExposedField field : idFields) { FieldReference reference = context.getReference(field); inner.put(field.getName(), reference.toString()); } 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 9cd503109..bd26e582f 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 @@ -155,9 +155,9 @@ public class ProjectionOperation implements FieldsExposingAggregationOperation { return new ProjectionOperation(this.projections, FieldProjection.from(fields, true)); } - /* + /* * (non-Javadoc) - * @see org.springframework.data.mongodb.core.aggregation.AggregationOperationContextSupport#getFields() + * @see org.springframework.data.mongodb.core.aggregation.FieldsExposingAggregationOperation#getFields() */ @Override public ExposedFields getFields() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GroupOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GroupOperationUnitTests.java index b6d648418..d5f71cc3a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GroupOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/GroupOperationUnitTests.java @@ -37,6 +37,38 @@ public class GroupOperationUnitTests { new GroupOperation((Fields) null); } + /** + * @see DATAMONGO-759 + */ + @Test + public void groupOperationWithNoGroupIdFieldsShouldGenerateNullAsGroupId() { + + GroupOperation operation = new GroupOperation(Fields.from()); + ExposedFields fields = operation.getFields(); + DBObject groupClause = extractDbObjectFromGroupOperation(operation); + + assertThat(fields.exposesSingleFieldOnly(), is(true)); + assertThat(fields.exposesNoFields(), is(false)); + assertThat(groupClause.get(UNDERSCORE_ID), is(nullValue())); + } + + /** + * @see DATAMONGO-759 + */ + @Test + public void groupOperationWithNoGroupIdFieldsButAdditionalFieldsShouldGenerateNullAsGroupId() { + + GroupOperation operation = new GroupOperation(Fields.from()).count().as("cnt").last("foo").as("foo"); + ExposedFields fields = operation.getFields(); + DBObject groupClause = extractDbObjectFromGroupOperation(operation); + + assertThat(fields.exposesSingleFieldOnly(), is(false)); + assertThat(fields.exposesNoFields(), is(false)); + assertThat(groupClause.get(UNDERSCORE_ID), is(nullValue())); + assertThat((BasicDBObject) groupClause.get("cnt"), is(new BasicDBObject("$sum", 1))); + assertThat((BasicDBObject) groupClause.get("foo"), is(new BasicDBObject("$last", "$foo"))); + } + @Test public void createsGroupOperationWithSingleField() {