From ff28789507800c06f56ae76cb08ba0b64607a59f Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Fri, 23 Sep 2022 14:33:41 +0200 Subject: [PATCH] Polishing. See #4139 Original pull request: #4182. --- .../AbstractAggregationExpression.java | 18 ++- .../core/aggregation/DateOperators.java | 4 +- .../core/aggregation/DensifyOperation.java | 6 +- .../core/aggregation/ObjectOperators.java | 113 +++++++++++++++--- .../core/aggregation/SelectionOperators.java | 24 ++-- .../DensifyOperationUnitTests.java | 10 +- 6 files changed, 134 insertions(+), 41 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AbstractAggregationExpression.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AbstractAggregationExpression.java index f84e427bd4..3d99179e76 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AbstractAggregationExpression.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AbstractAggregationExpression.java @@ -23,9 +23,9 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import org.bson.Document; + import org.springframework.data.domain.Sort; import org.springframework.data.domain.Sort.Order; import org.springframework.data.mongodb.core.aggregation.ExposedFields.FieldReference; @@ -78,7 +78,14 @@ private Object unpack(Object value, AggregationOperationContext context) { } if (value instanceof Fields fields) { - return fields.asList().stream().map(it -> unpack(it, context)).collect(Collectors.toList()); + + List mapped = new ArrayList<>(fields.size()); + + for (Field field : fields) { + mapped.add(unpack(field, context)); + } + + return mapped; } if (value instanceof Sort sort) { @@ -98,7 +105,9 @@ private Object unpack(Object value, AggregationOperationContext context) { List sourceList = (List) value; List mappedList = new ArrayList<>(sourceList.size()); - sourceList.stream().map((item) -> unpack(item, context)).forEach(mappedList::add); + for (Object o : sourceList) { + mappedList.add(unpack(o, context)); + } return mappedList; } @@ -150,7 +159,7 @@ protected List append(Object value) { return append(value, Expand.EXPAND_VALUES); } - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings({ "unchecked" }) protected Map append(String key, Object value) { Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map"); @@ -165,6 +174,7 @@ private Map append(Map existing, String key, Obj return clone; } + @SuppressWarnings("rawtypes") protected Map appendTo(String key, Object value) { Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map"); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DateOperators.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DateOperators.java index c3644432f9..af3f01124c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DateOperators.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DateOperators.java @@ -826,7 +826,7 @@ public DateFromString fromString() { */ public TsIncrement tsIncrement() { - if(timezone != null && !Timezone.none().equals(timezone)) { + if (timezone != null && !Timezone.none().equals(timezone)) { throw new IllegalArgumentException("$tsIncrement does not support timezones"); } @@ -841,7 +841,7 @@ public TsIncrement tsIncrement() { */ public TsSecond tsSecond() { - if(timezone != null && !Timezone.none().equals(timezone)) { + if (timezone != null && !Timezone.none().equals(timezone)) { throw new IllegalArgumentException("$tsSecond does not support timezones"); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DensifyOperation.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DensifyOperation.java index 253db727c9..aba1af1b22 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DensifyOperation.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/DensifyOperation.java @@ -190,7 +190,7 @@ public DensifyRange incrementBy(Number step, DensifyUnit unit) { /** * Set the {@link DensifyUnit unit} for the step field. - * + * * @param unit * @return this. */ @@ -354,6 +354,8 @@ public DensifyOperationBuilder range(Range range) { */ public DensifyOperationBuilder fullRange(Consumer consumer) { + Assert.notNull(consumer, "Consumer must not be null"); + DensifyRange range = Range.full(); consumer.accept(range); @@ -374,7 +376,7 @@ public DensifyOperationBuilder partitionRange(Consumer consumer) { return range(range); } - DensifyOperation build() { + public DensifyOperation build() { return new DensifyOperation(target.field, target.partitionBy, target.range); } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ObjectOperators.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ObjectOperators.java index b574d4a2a6..1058b8936e 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ObjectOperators.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ObjectOperators.java @@ -18,10 +18,8 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Set; import org.bson.Document; -import org.springframework.data.mongodb.core.aggregation.Aggregation.SystemVariable; import org.springframework.util.Assert; /** @@ -135,7 +133,7 @@ public ObjectToArray toArray() { * @since 4.0 */ public GetField getField(String fieldName) { - return GetField.getField(fieldName).from(value); + return GetField.getField(fieldName).of(value); } /** @@ -145,7 +143,7 @@ public GetField getField(String fieldName) { * @since 4.0 */ public SetField setField(String fieldName) { - return SetField.setField(fieldName).of(value); + return SetField.field(fieldName).input(value); } /** @@ -155,7 +153,7 @@ public SetField setField(String fieldName) { * @since 4.0 */ public AggregationExpression removeField(String fieldName) { - return SetField.setField(fieldName).of(value).toValue(SystemVariable.REMOVE); + return SetField.field(fieldName).input(value).toValue(SystemVariable.REMOVE); } } @@ -329,23 +327,49 @@ protected GetField(Object value) { super(value); } + /** + * Creates new {@link GetField aggregation expression} that takes the value pointed to by given {@code fieldName}. + * + * @param fieldName must not be {@literal null}. + * @return new instance of {@link GetField}. + */ public static GetField getField(String fieldName) { return new GetField(Collections.singletonMap("field", fieldName)); } + /** + * Creates new {@link GetField aggregation expression} that takes the value pointed to by given {@link Field}. + * + * @param field must not be {@literal null}. + * @return new instance of {@link GetField}. + */ public static GetField getField(Field field) { return getField(field.getTarget()); } - public GetField from(String fieldRef) { - return from(Fields.field(fieldRef)); + /** + * Creates new {@link GetField aggregation expression} that takes the value pointed to by given + * {@code field reference}. + * + * @param fieldRef must not be {@literal null}. + * @return new instance of {@link GetField}. + */ + public GetField of(String fieldRef) { + return of(Fields.field(fieldRef)); } - public GetField from(AggregationExpression expression) { - return from((Object) expression); + /** + * Creates new {@link GetField aggregation expression} that takes the value pointed to by given + * {@link AggregationExpression}. + * + * @param expression must not be {@literal null}. + * @return new instance of {@link GetField}. + */ + public GetField of(AggregationExpression expression) { + return of((Object) expression); } - private GetField from(Object fieldRef) { + private GetField of(Object fieldRef) { return new GetField(append("input", fieldRef)); } @@ -367,34 +391,87 @@ protected SetField(Object value) { super(value); } - public static SetField setField(String fieldName) { + /** + * Creates new {@link SetField aggregation expression} that takes the value pointed to by given input + * {@code fieldName}. + * + * @param fieldName must not be {@literal null}. + * @return new instance of {@link SetField}. + */ + public static SetField field(String fieldName) { return new SetField(Collections.singletonMap("field", fieldName)); } - public static SetField setField(Field field) { - return setField(field.getTarget()); + /** + * Creates new {@link SetField aggregation expression} that takes the value pointed to by given input {@link Field}. + * + * @param field must not be {@literal null}. + * @return new instance of {@link SetField}. + */ + public static SetField field(Field field) { + return field(field.getTarget()); } - public SetField of(String fieldRef) { - return of(Fields.field(fieldRef)); + /** + * Creates new {@link GetField aggregation expression} that takes the value pointed to by given input + * {@code field reference}. + * + * @param fieldRef must not be {@literal null}. + * @return new instance of {@link GetField}. + */ + public SetField input(String fieldRef) { + return input(Fields.field(fieldRef)); } - public SetField of(AggregationExpression expression) { - return of((Object) expression); + /** + * Creates new {@link SetField aggregation expression} that takes the value pointed to by given input + * {@link AggregationExpression}. + * + * @param expression must not be {@literal null}. + * @return new instance of {@link SetField}. + */ + public SetField input(AggregationExpression expression) { + return input((Object) expression); } - private SetField of(Object fieldRef) { + /** + * Creates new {@link SetField aggregation expression} that takes the value pointed to by given input + * {@code field reference}. + * + * @param fieldRef must not be {@literal null}. + * @return new instance of {@link SetField}. + */ + private SetField input(Object fieldRef) { return new SetField(append("input", fieldRef)); } + /** + * Creates new {@link SetField aggregation expression} providing the {@code value} using {@literal fieldReference}. + * + * @param fieldReference must not be {@literal null}. + * @return new instance of {@link SetField}. + */ public SetField toValueOf(String fieldReference) { return toValue(Fields.field(fieldReference)); } + /** + * Creates new {@link SetField aggregation expression} providing the {@code value} using + * {@link AggregationExpression}. + * + * @param expression must not be {@literal null}. + * @return new instance of {@link SetField}. + */ public SetField toValueOf(AggregationExpression expression) { return toValue(expression); } + /** + * Creates new {@link SetField aggregation expression} providing the {@code value}. + * + * @param value + * @return new instance of {@link SetField}. + */ public SetField toValue(Object value) { return new SetField(append("value", value)); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SelectionOperators.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SelectionOperators.java index c170011841..9c106b98cc 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SelectionOperators.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/SelectionOperators.java @@ -261,7 +261,7 @@ public static First first(int numberOfResults) { * Limits the number of returned elements to the given value. * * @param numberOfResults - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First limit(int numberOfResults) { return limit((Object) numberOfResults); @@ -272,7 +272,7 @@ public First limit(int numberOfResults) { * expression}. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First limit(AggregationExpression expression) { return limit((Object) expression); @@ -286,7 +286,7 @@ private First limit(Object value) { * Define the field to serve as source. * * @param fieldName must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First of(String fieldName) { return input(fieldName); @@ -296,7 +296,7 @@ public First of(String fieldName) { * Define the expression building the value to serve as source. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First of(AggregationExpression expression) { return input(expression); @@ -306,7 +306,7 @@ public First of(AggregationExpression expression) { * Define the field to serve as source. * * @param fieldName must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First input(String fieldName) { return new First(append("input", Fields.field(fieldName))); @@ -316,7 +316,7 @@ public First input(String fieldName) { * Define the expression building the value to serve as source. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link First}. */ public First input(AggregationExpression expression) { return new First(append("input", expression)); @@ -355,7 +355,7 @@ public static Last last(int numberOfResults) { * Limits the number of returned elements to the given value. * * @param numberOfResults - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last limit(int numberOfResults) { return limit((Object) numberOfResults); @@ -366,7 +366,7 @@ public Last limit(int numberOfResults) { * expression}. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last limit(AggregationExpression expression) { return limit((Object) expression); @@ -380,7 +380,7 @@ private Last limit(Object value) { * Define the field to serve as source. * * @param fieldName must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last of(String fieldName) { return input(fieldName); @@ -390,7 +390,7 @@ public Last of(String fieldName) { * Define the expression building the value to serve as source. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last of(AggregationExpression expression) { return input(expression); @@ -400,7 +400,7 @@ public Last of(AggregationExpression expression) { * Define the field to serve as source. * * @param fieldName must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last input(String fieldName) { return new Last(append("input", Fields.field(fieldName))); @@ -410,7 +410,7 @@ public Last input(String fieldName) { * Define the expression building the value to serve as source. * * @param expression must not be {@literal null}. - * @return new instance of {@link Bottom}. + * @return new instance of {@link Last}. */ public Last input(AggregationExpression expression) { return new Last(append("input", expression)); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/DensifyOperationUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/DensifyOperationUnitTests.java index 60284074c1..571ae84028 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/DensifyOperationUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/aggregation/DensifyOperationUnitTests.java @@ -30,6 +30,8 @@ import org.springframework.lang.Nullable; /** + * Unit tests for {@link DensifyOperation}. + * * @author Christoph Strobl */ class DensifyOperationUnitTests { @@ -38,7 +40,8 @@ class DensifyOperationUnitTests { void rendersFieldNamesAsIsForUntypedContext() { DensifyOperation densify = DensifyOperation.builder().densify("ts") - .range(Range.bounded("2021-05-18T00:00:00", "2021-05-18T08:00:00").incrementBy(1).unit(DensifyUnits.HOUR)).build(); + .range(Range.bounded("2021-05-18T00:00:00", "2021-05-18T08:00:00").incrementBy(1).unit(DensifyUnits.HOUR)) + .build(); assertThat(densify.toDocument(contextFor(null))).isEqualTo(""" { @@ -58,7 +61,8 @@ void rendersFieldNamesAsIsForUntypedContext() { void rendersFieldNamesCorrectly() { DensifyOperation densify = DensifyOperation.builder().densify("ts") - .range(Range.bounded("2021-05-18T00:00:00", "2021-05-18T08:00:00").incrementBy(1).unit(DensifyUnits.HOUR)).build(); + .range(Range.bounded("2021-05-18T00:00:00", "2021-05-18T08:00:00").incrementBy(1).unit(DensifyUnits.HOUR)) + .build(); assertThat(densify.toDocument(contextFor(Weather.class))).isEqualTo(""" { @@ -95,7 +99,7 @@ void rendersPartitonNamesCorrectly() { } @Test // GH-4139 - void rendersPartitonRangeCorrectly() { + void rendersPartitionRangeCorrectly() { DensifyOperation densify = DensifyOperation.builder().densify("alt").partitionBy("var") .partitionRange(range -> range.incrementBy(200)).build();