Skip to content

Commit 217be64

Browse files
mp911demp911de
committed
DATAMONGO-2623 - Polishing.
Avoid nullable method arguments and add assertions. Introduce build() method to AccumulatorFinalizeBuilder to build Accumulator without specifying a finalize function. Original pull request: #887.
1 parent 0ef852a commit 217be64

File tree

3 files changed

+95
-45
lines changed

3 files changed

+95
-45
lines changed

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/AbstractAggregationExpression.java‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,11 @@
2929
import org.springframework.util.ObjectUtils;
3030

3131
/**
32+
* Support class for {@link AggregationExpression} implementations.
33+
*
3234
* @author Christoph Strobl
3335
* @author Matt Morrissette
36+
* @author Mark Paluch
3437
* @since 1.10
3538
*/
3639
abstract class AbstractAggregationExpression implements AggregationExpression {
@@ -49,7 +52,6 @@ public Document toDocument(AggregationOperationContext context) {
4952
return toDocument(this.value, context);
5053
}
5154

52-
@SuppressWarnings("unchecked")
5355
public Document toDocument(Object value, AggregationOperationContext context) {
5456
return new Document(getMongoMethod(), unpack(value, context));
5557
}
@@ -101,17 +103,19 @@ private Object unpack(Object value, AggregationOperationContext context) {
101103
return value;
102104
}
103105

106+
@SuppressWarnings("unchecked")
104107
protected List<Object> append(Object value, Expand expandList) {
105108

106109
if (this.value instanceof List) {
107110

108-
List<Object> clone = new ArrayList<Object>((List) this.value);
111+
List<Object> clone = new ArrayList<>((List<Object>) this.value);
109112

110113
if (value instanceof Collection && Expand.EXPAND_VALUES.equals(expandList)) {
111114
clone.addAll((Collection<?>) value);
112115
} else {
113116
clone.add(value);
114117
}
118+
115119
return clone;
116120
}
117121

@@ -129,22 +133,23 @@ protected List<Object> append(Object value) {
129133
return append(value, Expand.EXPAND_VALUES);
130134
}
131135

132-
@SuppressWarnings("unchecked")
133-
protected java.util.Map<String, Object> append(String key, Object value) {
136+
@SuppressWarnings({ "unchecked", "rawtypes" })
137+
protected Map<String, Object> append(String key, Object value) {
134138

135139
Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map!");
136140

137-
java.util.Map<String, Object> clone = new LinkedHashMap<>((java.util.Map) this.value);
141+
Map<String, Object> clone = new LinkedHashMap<>((java.util.Map) this.value);
138142
clone.put(key, value);
139143
return clone;
140144

141145
}
142146

143-
protected java.util.Map<String, Object> remove(String key) {
147+
@SuppressWarnings({ "unchecked", "rawtypes" })
148+
protected Map<String, Object> remove(String key) {
144149

145150
Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map!");
146151

147-
java.util.Map<String, Object> clone = new LinkedHashMap<>((java.util.Map) this.value);
152+
Map<String, Object> clone = new LinkedHashMap<>((java.util.Map) this.value);
148153
clone.remove(key);
149154
return clone;
150155
}
@@ -158,14 +163,15 @@ protected java.util.Map<String, Object> remove(String key) {
158163
* @return
159164
* @since 3.1
160165
*/
161-
protected java.util.Map<String, Object> appendAt(int index, String key, Object value) {
166+
@SuppressWarnings({ "unchecked" })
167+
protected Map<String, Object> appendAt(int index, String key, Object value) {
162168

163169
Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map!");
164170

165-
java.util.LinkedHashMap<String, Object> clone = new java.util.LinkedHashMap<>();
171+
Map<String, Object> clone = new LinkedHashMap<>();
166172

167173
int i = 0;
168-
for (Map.Entry<String, Object> entry : ((java.util.Map<String, Object>) this.value).entrySet()) {
174+
for (Map.Entry<String, Object> entry : ((Map<String, Object>) this.value).entrySet()) {
169175

170176
if (i == index) {
171177
clone.put(key, value);
@@ -182,14 +188,17 @@ protected java.util.Map<String, Object> appendAt(int index, String key, Object v
182188

183189
}
184190

191+
@SuppressWarnings({ "rawtypes" })
185192
protected List<Object> values() {
186193

187194
if (value instanceof List) {
188195
return new ArrayList<Object>((List) value);
189196
}
197+
190198
if (value instanceof java.util.Map) {
191199
return new ArrayList<Object>(((java.util.Map) value).values());
192200
}
201+
193202
return new ArrayList<>(Collections.singletonList(value));
194203
}
195204

@@ -219,7 +228,7 @@ protected <T> T get(Object key) {
219228

220229
Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map!");
221230

222-
return (T) ((java.util.Map<String, Object>) this.value).get(key);
231+
return (T) ((Map<String, Object>) this.value).get(key);
223232
}
224233

225234
/**
@@ -229,11 +238,11 @@ protected <T> T get(Object key) {
229238
* @return
230239
*/
231240
@SuppressWarnings("unchecked")
232-
protected java.util.Map<String, Object> argumentMap() {
241+
protected Map<String, Object> argumentMap() {
233242

234243
Assert.isInstanceOf(Map.class, this.value, "Value must be a type of Map!");
235244

236-
return Collections.unmodifiableMap((java.util.Map) value);
245+
return Collections.unmodifiableMap((java.util.Map<String, Object>) value);
237246
}
238247

239248
/**
@@ -250,7 +259,7 @@ protected boolean contains(Object key) {
250259
return false;
251260
}
252261

253-
return ((java.util.Map<String, Object>) this.value).containsKey(key);
262+
return ((Map<String, Object>) this.value).containsKey(key);
254263
}
255264

256265
protected abstract String getMongoMethod();

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/aggregation/ScriptOperators.java‎

Lines changed: 56 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
* <a href="https://docs.mongodb.com/master/reference/configuration-options/#security.javascriptEnabled">enabled</a>.
3737
*
3838
* @author Christoph Strobl
39+
* @author Mark Paluch
3940
* @since 3.1
4041
*/
4142
public class ScriptOperators {
@@ -83,7 +84,6 @@ public static AccumulatorInitBuilder accumulatorBuilder() {
8384
*
8485
* @see <a href="https://docs.mongodb.com/master/reference/operator/aggregation/function/">MongoDB Documentation:
8586
* $function</a>
86-
* @since 3.1
8787
*/
8888
public static class Function extends AbstractAggregationExpression {
8989

@@ -99,6 +99,8 @@ private Function(Map<String, Object> values) {
9999
*/
100100
public static Function function(String body) {
101101

102+
Assert.notNull(body, "Function body must not be null!");
103+
102104
Map<String, Object> function = new LinkedHashMap<>(2);
103105
function.put(Fields.BODY.toString(), body);
104106
function.put(Fields.ARGS.toString(), Collections.emptyList());
@@ -126,6 +128,7 @@ public Function args(Object... args) {
126128
public Function args(List<Object> args) {
127129

128130
Assert.notNull(args, "Args must not be null! Use an empty list instead.");
131+
129132
return new Function(appendAt(1, Fields.ARGS.toString(), args));
130133
}
131134

@@ -137,7 +140,8 @@ public Function args(List<Object> args) {
137140
*/
138141
public Function lang(String lang) {
139142

140-
Assert.hasText(lang, "Lang must not be null nor emtpy! The default would be 'js'.");
143+
Assert.hasText(lang, "Lang must not be null nor empty! The default would be 'js'.");
144+
141145
return new Function(appendAt(2, Fields.LANG.toString(), lang));
142146
}
143147

@@ -198,7 +202,6 @@ public String toString() {
198202
*
199203
* @see <a href="https://docs.mongodb.com/master/reference/operator/aggregation/accumulator/">MongoDB Documentation:
200204
* $accumulator</a>
201-
* @since 3.1
202205
*/
203206
public static class Accumulator extends AbstractAggregationExpression {
204207

@@ -293,10 +296,10 @@ default AccumulatorAccumulateBuilder initArgs(Object... args) {
293296
/**
294297
* Define the optional {@code initArgs} for the {@link AccumulatorInitBuilder#init(String)} function.
295298
*
296-
* @param args can be {@literal null}.
299+
* @param args must not be {@literal null}.
297300
* @return this.
298301
*/
299-
AccumulatorAccumulateBuilder initArgs(@Nullable List<Object> args);
302+
AccumulatorAccumulateBuilder initArgs(List<Object> args);
300303
}
301304

302305
public interface AccumulatorAccumulateBuilder {
@@ -355,10 +358,10 @@ default AccumulatorMergeBuilder accumulateArgs(Object... args) {
355358
* Define additional {@code accumulateArgs} for the {@link AccumulatorAccumulateBuilder#accumulate(String)}
356359
* function.
357360
*
358-
* @param args can be {@literal null}.
361+
* @param args must not be {@literal null}.
359362
* @return this.
360363
*/
361-
AccumulatorMergeBuilder accumulateArgs(@Nullable List<Object> args);
364+
AccumulatorMergeBuilder accumulateArgs(List<Object> args);
362365
}
363366

364367
public interface AccumulatorMergeBuilder {
@@ -398,9 +401,16 @@ public interface AccumulatorFinalizeBuilder {
398401
* @return new instance of {@link Accumulator}.
399402
*/
400403
Accumulator finalize(String function);
404+
405+
/**
406+
* Build the {@link Accumulator} object without specifying a {@link #finalize(String) finalize function}.
407+
*
408+
* @return new instance of {@link Accumulator}.
409+
*/
410+
Accumulator build();
401411
}
402412

403-
public static class AccumulatorBuilder
413+
static class AccumulatorBuilder
404414
implements AccumulatorInitBuilder, AccumulatorInitArgsBuilder, AccumulatorAccumulateBuilder,
405415
AccumulatorAccumulateArgsBuilder, AccumulatorMergeBuilder, AccumulatorFinalizeBuilder {
406416

@@ -426,6 +436,7 @@ public static class AccumulatorBuilder
426436
* @param function must not be {@literal null}.
427437
* @return this.
428438
*/
439+
@Override
429440
public AccumulatorBuilder init(String function) {
430441

431442
this.initFunction = function;
@@ -435,12 +446,15 @@ public AccumulatorBuilder init(String function) {
435446
/**
436447
* Define the optional {@code initArgs} for the {@link #init(String)} function.
437448
*
438-
* @param args can be {@literal null}.
449+
* @param function must not be {@literal null}.
439450
* @return this.
440451
*/
441-
public AccumulatorBuilder initArgs(@Nullable List<Object> args) {
452+
@Override
453+
public AccumulatorBuilder initArgs(List<Object> args) {
454+
455+
Assert.notNull(args, "Args must not be null");
442456

443-
this.initArgs = args != null ? new ArrayList<>(args) : Collections.emptyList();
457+
this.initArgs = new ArrayList<>(args);
444458
return this;
445459
}
446460

@@ -458,21 +472,27 @@ public AccumulatorBuilder initArgs(@Nullable List<Object> args) {
458472
* @param function must not be {@literal null}.
459473
* @return this.
460474
*/
475+
@Override
461476
public AccumulatorBuilder accumulate(String function) {
462477

478+
Assert.notNull(function, "Accumulate function must not be null");
479+
463480
this.accumulateFunction = function;
464481
return this;
465482
}
466483

467484
/**
468485
* Define additional {@code accumulateArgs} for the {@link #accumulate(String)} function.
469486
*
470-
* @param args can be {@literal null}.
487+
* @param args must not be {@literal null}.
471488
* @return this.
472489
*/
473-
public AccumulatorBuilder accumulateArgs(@Nullable List<Object> args) {
490+
@Override
491+
public AccumulatorBuilder accumulateArgs(List<Object> args) {
474492

475-
this.accumulateArgs = args != null ? new ArrayList<>(args) : Collections.emptyList();
493+
Assert.notNull(args, "Args must not be null");
494+
495+
this.accumulateArgs = new ArrayList<>(args);
476496
return this;
477497
}
478498

@@ -491,8 +511,11 @@ public AccumulatorBuilder accumulateArgs(@Nullable List<Object> args) {
491511
* @param function must not be {@literal null}.
492512
* @return this.
493513
*/
514+
@Override
494515
public AccumulatorBuilder merge(String function) {
495516

517+
Assert.notNull(function, "Merge function must not be null");
518+
496519
this.mergeFunction = function;
497520
return this;
498521
}
@@ -505,6 +528,8 @@ public AccumulatorBuilder merge(String function) {
505528
*/
506529
public AccumulatorBuilder lang(String lang) {
507530

531+
Assert.hasText(lang, "Lang must not be null nor empty! The default would be 'js'.");
532+
508533
this.lang = lang;
509534
return this;
510535
}
@@ -523,10 +548,26 @@ public AccumulatorBuilder lang(String lang) {
523548
* @param function must not be {@literal null}.
524549
* @return new instance of {@link Accumulator}.
525550
*/
551+
@Override
526552
public Accumulator finalize(String function) {
527553

554+
Assert.notNull(function, "Finalize function must not be null");
555+
528556
this.finalizeFunction = function;
529557

558+
Map<String, Object> args = createArgumentMap();
559+
args.put(Fields.FINALIZE.toString(), finalizeFunction);
560+
561+
return new Accumulator(args);
562+
}
563+
564+
@Override
565+
public Accumulator build() {
566+
return new Accumulator(createArgumentMap());
567+
}
568+
569+
private Map<String, Object> createArgumentMap() {
570+
530571
Map<String, Object> args = new LinkedHashMap<>();
531572
args.put(Fields.INIT.toString(), initFunction);
532573
if (!CollectionUtils.isEmpty(initArgs)) {
@@ -537,12 +578,10 @@ public Accumulator finalize(String function) {
537578
args.put(Fields.ACCUMULATE_ARGS.toString(), accumulateArgs);
538579
}
539580
args.put(Fields.MERGE.toString(), mergeFunction);
540-
args.put(Fields.FINALIZE.toString(), finalizeFunction);
541581
args.put(Fields.LANG.toString(), lang);
542582

543-
return new Accumulator(args);
583+
return args;
544584
}
545-
546585
}
547586
}
548587
}

0 commit comments

Comments
 (0)