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 848c5cccd4..98b5e6415d 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 @@ * 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,65 @@ public ExposedField getField(String name) { return null; } + /** + * Returns whether the {@link ExposedFields} exposes no non-synthetic fields at all. + * + * @return + */ + public boolean exposesNoNonSyntheticFields() { + return originalFields.isEmpty(); + } + + /** + * Returns whether the {@link ExposedFields} exposes a single non-synthetic field only. + * + * @return + */ + public boolean exposesSingleNonSyntheticFieldOnly() { + return originalFields.size() == 1; + } + + /** + * Returns whether the {@link ExposedFields} exposes no synthetic fields at all. + * + * @return + */ + public boolean exposesNoSyntheticFields() { + return syntheticFields.isEmpty(); + } + + /** + * Returns whether the {@link ExposedFields} exposes a single synthetic field only. + * + * @return + */ + public boolean exposesSingleSytheticFieldOnly() { + return syntheticFields.size() == 1; + } + + /** + * Returns whether the {@link ExposedFields} exposes no fields at all. + * + * @return + */ + public 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; + 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 e3d8f1f3a6..7cf3c573f1 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 @@ */ public class GroupOperation extends ExposedFieldsAggregationOperationContext implements AggregationOperation { - 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 extends ExposedFieldsAggregationOperationContext imp */ public GroupOperation(Fields fields) { - this.nonSynthecticFields = ExposedFields.nonSynthetic(fields); + this.idFields = ExposedFields.nonSynthetic(fields); this.operations = new ArrayList(); } @@ -74,7 +78,7 @@ private GroupOperation(GroupOperation groupOperation, List nextOperat 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 @@ private GroupOperationBuilder newBuilder(Keyword keyword, String reference, Obje @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 com.mongodb.DBObject toDBObject(AggregationOperationContext context) { BasicDBObject operationObject = new BasicDBObject(); - if (nonSynthecticFields.exposesSingleFieldOnly()) { + if (idFields.exposesNoNonSyntheticFields()) { + + operationObject.put(Fields.UNDERSCORE_ID, null); + + } else if (idFields.exposesSingleNonSyntheticFieldOnly()) { - FieldReference reference = context.getReference(nonSynthecticFields.iterator().next()); + 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/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 b6d6484184..d5f71cc3a6 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 void rejectsNullFields() { 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() {