From b96707a0e25f8ee36f3a37ff8e5f5264b77c5e5b Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 20 Sep 2017 11:07:54 +0200 Subject: [PATCH] DATAMONGO-1784 - Polishing. Update JavaDoc, enforce nullability constraints and add tests. Original Pull Request: #501 --- .../core/aggregation/GroupOperation.java | 35 ++++++++++++------- .../core/aggregation/AggregationTests.java | 14 ++++---- .../aggregation/GroupOperationUnitTests.java | 26 ++++++++++++++ 3 files changed, 55 insertions(+), 20 deletions(-) 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 4f023de69..3d9d22ddc 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 @@ -19,12 +19,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Locale; import org.springframework.data.mongodb.core.aggregation.ExposedFields.ExposedField; import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference; import org.springframework.util.Assert; -import org.springframework.util.StringUtils; import com.mongodb.BasicDBObject; import com.mongodb.DBObject; @@ -162,10 +160,14 @@ public class GroupOperation implements FieldsExposingAggregationOperation { * Generates an {@link GroupOperationBuilder} for an {@code $sum}-expression for the given * {@link AggregationExpression}. * - * @param expr - * @return + * @param expr must not be {@literal null}. + * @return new instance of {@link GroupOperationBuilder}. Never {@literal null}. + * @throws IllegalArgumentException when {@code expr} is {@literal null}. + * @since 1.10.8 */ public GroupOperationBuilder sum(AggregationExpression expr) { + + Assert.notNull(expr, "Expr must not be null!"); return newBuilder(GroupOps.SUM, null, expr); } @@ -208,7 +210,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $last}-expression for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $last}-expression for the given + * {@link AggregationExpression}. * * @param expr * @return @@ -228,7 +231,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for a {@code $first}-expression for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for a {@code $first}-expression for the given + * {@link AggregationExpression}. * * @param expr * @return @@ -248,7 +252,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $avg}-expression for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $avg}-expression for the given + * {@link AggregationExpression}. * * @param expr * @return @@ -292,7 +297,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $min}-expression that for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $min}-expression that for the given + * {@link AggregationExpression}. * * @param expr * @return @@ -312,7 +318,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $max}-expression that for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $max}-expression that for the given + * {@link AggregationExpression}. * * @param expr * @return @@ -334,7 +341,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $stdDevSamp}-expression that for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $stdDevSamp}-expression that for the given + * {@link AggregationExpression}. * * @param expr must not be {@literal null}. * @return never {@literal null}. @@ -356,7 +364,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { } /** - * Generates an {@link GroupOperationBuilder} for an {@code $stdDevPop}-expression that for the given {@link AggregationExpression}. + * Generates an {@link GroupOperationBuilder} for an {@code $stdDevPop}-expression that for the given + * {@link AggregationExpression}. * * @param expr must not be {@literal null}. * @return never {@literal null}. @@ -430,7 +439,8 @@ public class GroupOperation implements FieldsExposingAggregationOperation { private static enum GroupOps implements Keyword { - SUM("$sum"), LAST("$last"), FIRST("$first"), PUSH("$push"), AVG("$avg"), MIN("$min"), MAX("$max"), ADD_TO_SET("$addToSet"), STD_DEV_POP("$stdDevPop"), STD_DEV_SAMP("$stdDevSamp"); + SUM("$sum"), LAST("$last"), FIRST("$first"), PUSH("$push"), AVG("$avg"), MIN("$min"), MAX("$max"), ADD_TO_SET( + "$addToSet"), STD_DEV_POP("$stdDevPop"), STD_DEV_SAMP("$stdDevSamp"); private String mongoOperator; @@ -438,7 +448,6 @@ public class GroupOperation implements FieldsExposingAggregationOperation { this.mongoOperator = mongoOperator; } - @Override public String toString() { return mongoOperator; diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java index 67a8054ea..6190fbdb0 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/AggregationTests.java @@ -152,7 +152,7 @@ public class AggregationTests { /** * Imports the sample dataset (zips.json) if necessary (e.g. if it doesn't exist yet). The dataset can originally be * found on the mongodb aggregation framework example website: - * + * * @see MongoDB Aggregation Examples */ private void initSampleDataIfNecessary() { @@ -344,7 +344,7 @@ public class AggregationTests { public void complexAggregationFrameworkUsageLargestAndSmallestCitiesByState() { /* //complex mongodb aggregation framework example from https://docs.mongodb.org/manual/tutorial/aggregation-examples/#largest-and-smallest-cities-by-state - db.zipInfo.aggregate( + db.zipInfo.aggregate( { $group: { _id: { @@ -451,18 +451,18 @@ public class AggregationTests { @Test // DATAMONGO-586 public void findStatesWithPopulationOver10MillionAggregationExample() { /* - //complex mongodb aggregation framework example from + //complex mongodb aggregation framework example from https://docs.mongodb.org/manual/tutorial/aggregation-examples/#largest-and-smallest-cities-by-state - - db.zipcodes.aggregate( + + db.zipcodes.aggregate( { $group: { _id:"$state", totalPop:{ $sum:"$pop"} } }, - { - $sort: { _id: 1, "totalPop": 1 } + { + $sort: { _id: 1, "totalPop": 1 } }, { $match: { 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 ad765224c..794b33d9e 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 @@ -24,6 +24,7 @@ import java.util.Arrays; import org.junit.Test; import org.springframework.data.mongodb.core.DBObjectTestUtils; +import org.springframework.data.mongodb.core.query.Criteria; import com.mongodb.BasicDBObject; import com.mongodb.DBObject; @@ -218,6 +219,31 @@ public class GroupOperationUnitTests { assertThat(push, is((DBObject) new BasicDBObject("$stdDevPop", "$field"))); } + @Test // DATAMONGO-1784 + public void shouldRenderSumWithExpressionInGroup() { + + GroupOperation groupOperation = Aggregation // + .group("username") // + .sum(ConditionalOperators // + .when(Criteria.where("foo").is("bar")) // + .then(1) // + .otherwise(-1)) // + .as("foobar"); + + DBObject groupClause = extractDbObjectFromGroupOperation(groupOperation); + DBObject foobar = DBObjectTestUtils.getAsDBObject(groupClause, "foobar"); + + assertThat((DBObject) foobar.get("$sum"), + is((DBObject) new BasicDBObject("$cond", + new BasicDBObject("if", new BasicDBObject("$eq", Arrays.asList("$foo", "bar"))).append("then", 1) + .append("else", -1)))); + } + + @Test(expected = IllegalArgumentException.class) // DATAMONGO-1784 + public void sumWithNullExpressionShouldThrowException() { + Aggregation.group("username").sum((AggregationExpression) null); + } + private DBObject extractDbObjectFromGroupOperation(GroupOperation groupOperation) { DBObject dbObject = groupOperation.toDBObject(Aggregation.DEFAULT_CONTEXT); DBObject groupClause = DBObjectTestUtils.getAsDBObject(dbObject, "$group");