From c381d131a7c82fecc8bb4093497be082bb8476c7 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Tue, 24 Sep 2013 15:42:21 +0200 Subject: [PATCH] DATAMONGO-759 - Render group operation without non-synthetic fields correctly. Added isEmpty() method to ExposedFields to be able to test whether fields are going to be exposed or not. Adjusted the rendering of GroupOperations to generate null as group id (_id) when no fields are given. --- .../core/aggregation/ExposedFields.java | 55 ++++++++++++++++++- .../core/aggregation/GroupOperation.java | 22 +++++--- .../aggregation/GroupOperationUnitTests.java | 32 +++++++++++ 3 files changed, 101 insertions(+), 8 deletions(-) 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() {