Skip to content

Commit d19ea88

Browse files
mp911demp911de
committed
DATAMONGO-1735 - Query sort and field documents no longer allow null.
We now require Sort and Fields (Projection) documents in Query. Absent sorting and projection uses empty documents. Original pull request: #479.
1 parent d3b9f91 commit d19ea88

File tree

9 files changed

+101
-60
lines changed

9 files changed

+101
-60
lines changed

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import org.bson.Document;
2424
import org.springframework.dao.IncorrectResultSizeDataAccessException;
25-
import org.springframework.data.mongodb.core.query.BasicQuery;
2625
import org.springframework.data.mongodb.core.query.NearQuery;
2726
import org.springframework.data.mongodb.core.query.Query;
2827
import org.springframework.data.mongodb.core.query.SerializationUtils;
@@ -37,11 +36,12 @@
3736
* Implementation of {@link ExecutableFindOperation}.
3837
*
3938
* @author Christoph Strobl
39+
* @author Mark Paluch
4040
* @since 2.0
4141
*/
4242
class ExecutableFindOperationSupport implements ExecutableFindOperation {
4343

44-
private static final Query ALL_QUERY = new BasicQuery(new Document());
44+
private static final Query ALL_QUERY = new Query();
4545

4646
private final MongoTemplate template;
4747

@@ -164,7 +164,6 @@ private List<T> doFind(CursorPreparer preparer) {
164164
}
165165

166166
private CloseableIterator<T> doStream() {
167-
168167
return template.doStream(query, domainType, getCollectionName(), returnType);
169168
}
170169

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

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ public <T> T findOne(Query query, Class<T> entityClass) {
587587

588588
public <T> T findOne(Query query, Class<T> entityClass, String collectionName) {
589589

590-
if (query.getSortObject() == null) {
590+
if (ObjectUtils.isEmpty(query.getSortObject()) && !query.getCollation().isPresent()) {
591591
return doFindOne(collectionName, query.getQueryObject(), query.getFieldsObject(), entityClass);
592592
} else {
593593
query.limit(1);
@@ -1470,9 +1470,7 @@ public <T> MapReduceResults<T> mapReduce(Query query, String inputCollectionName
14701470
if (query.getMeta() != null && query.getMeta().getMaxTimeMsec() != null) {
14711471
result = result.maxTime(query.getMeta().getMaxTimeMsec(), TimeUnit.MILLISECONDS);
14721472
}
1473-
if (query.getSortObject() != null) {
1474-
result = result.sort(query.getSortObject());
1475-
}
1473+
result = result.sort(query.getSortObject());
14761474

14771475
result = result.filter(queryMapper.getMappedObject(query.getQueryObject(), Optional.empty()));
14781476
}
@@ -2313,7 +2311,7 @@ private static final MongoConverter getDefaultMongoConverter(MongoDbFactory fact
23132311

23142312
private Document getMappedSortObject(Query query, Class<?> type) {
23152313

2316-
if (query == null || query.getSortObject() == null) {
2314+
if (query == null || ObjectUtils.isEmpty(query.getSortObject())) {
23172315
return null;
23182316
}
23192317

@@ -2373,7 +2371,7 @@ private static class FindOneCallback implements CollectionCallback<Document> {
23732371

23742372
public FindOneCallback(Document query, Document fields) {
23752373
this.query = query;
2376-
this.fields = Optional.ofNullable(fields);
2374+
this.fields = Optional.ofNullable(fields).filter(it -> !ObjectUtils.isEmpty(fields));
23772375
}
23782376

23792377
public Document doInCollection(MongoCollection<Document> collection) throws MongoException, DataAccessException {
@@ -2414,7 +2412,7 @@ public FindCallback(Document query) {
24142412
public FindCallback(Document query, Document fields) {
24152413

24162414
this.query = query != null ? query : new Document();
2417-
this.fields = Optional.ofNullable(fields);
2415+
this.fields = Optional.ofNullable(fields).filter(it -> !ObjectUtils.isEmpty(fields));
24182416
}
24192417

24202418
public FindIterable<Document> doInCollection(MongoCollection<Document> collection)
@@ -2607,9 +2605,9 @@ public FindIterable<Document> prepare(FindIterable<Document> cursor) {
26072605
return cursor;
26082606
}
26092607

2610-
if (query.getSkip() <= 0 && query.getLimit() <= 0
2611-
&& (query.getSortObject() == null || query.getSortObject().isEmpty()) && !StringUtils.hasText(query.getHint())
2612-
&& !query.getMeta().hasValues() && !query.getCollation().isPresent()) {
2608+
if (query.getSkip() <= 0 && query.getLimit() <= 0 && ObjectUtils.isEmpty(query.getSortObject())
2609+
&& !StringUtils.hasText(query.getHint()) && !query.getMeta().hasValues()
2610+
&& !query.getCollation().isPresent()) {
26132611
return cursor;
26142612
}
26152613

@@ -2624,7 +2622,7 @@ public FindIterable<Document> prepare(FindIterable<Document> cursor) {
26242622
if (query.getLimit() > 0) {
26252623
cursorToUse = cursorToUse.limit(query.getLimit());
26262624
}
2627-
if (query.getSortObject() != null && !query.getSortObject().isEmpty()) {
2625+
if (!ObjectUtils.isEmpty(query.getSortObject())) {
26282626
Document sort = type != null ? getMappedSortObject(query, type) : query.getSortObject();
26292627
cursorToUse = cursorToUse.sort(sort);
26302628
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,10 @@
5050
import org.springframework.data.geo.Distance;
5151
import org.springframework.data.geo.GeoResult;
5252
import org.springframework.data.geo.Metric;
53+
import org.springframework.data.mapping.MappingException;
5354
import org.springframework.data.mapping.PersistentPropertyAccessor;
5455
import org.springframework.data.mapping.context.MappingContext;
5556
import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
56-
import org.springframework.data.mapping.MappingException;
5757
import org.springframework.data.mongodb.MongoDbFactory;
5858
import org.springframework.data.mongodb.ReactiveMongoDatabaseFactory;
5959
import org.springframework.data.mongodb.core.convert.DbRefProxyHandler;
@@ -1927,7 +1927,7 @@ private static MappingMongoConverter getDefaultMongoConverter() {
19271927

19281928
private Document getMappedSortObject(Query query, Class<?> type) {
19291929

1930-
if (query == null || query.getSortObject() == null) {
1930+
if (query == null) {
19311931
return null;
19321932
}
19331933

@@ -2279,7 +2279,7 @@ public <T> FindPublisher<T> prepare(FindPublisher<T> findPublisher) {
22792279
findPublisherToUse = query.getCollation().map(Collation::toMongoCollation).map(findPublisher::collation)
22802280
.orElse(findPublisher);
22812281

2282-
if (query.getSkip() <= 0 && query.getLimit() <= 0 && query.getSortObject() == null
2282+
if (query.getSkip() <= 0 && query.getLimit() <= 0 && ObjectUtils.isEmpty(query.getSortObject())
22832283
&& !StringUtils.hasText(query.getHint()) && !query.getMeta().hasValues()) {
22842284
return findPublisherToUse;
22852285
}

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2010-2016 the original author or authors.
2+
* Copyright 2010-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,13 +18,11 @@
1818
import static org.springframework.util.ObjectUtils.*;
1919

2020
import org.bson.Document;
21-
22-
import com.mongodb.DBObject;
23-
import com.mongodb.util.JSON;
21+
import org.springframework.util.Assert;
2422

2523
/**
2624
* Custom {@link Query} implementation to setup a basic query from some arbitrary JSON query string.
27-
*
25+
*
2826
* @author Thomas Risberg
2927
* @author Oliver Gierke
3028
* @author Christoph Strobl
@@ -35,6 +33,7 @@
3533
public class BasicQuery extends Query {
3634

3735
private final Document queryObject;
36+
3837
private Document fieldsObject;
3938
private Document sortObject;
4039

@@ -53,7 +52,7 @@ public BasicQuery(String query) {
5352
* @param queryObject may be {@literal null}.
5453
*/
5554
public BasicQuery(Document queryObject) {
56-
this(queryObject, null);
55+
this(queryObject, new Document());
5756
}
5857

5958
/**
@@ -64,17 +63,22 @@ public BasicQuery(Document queryObject) {
6463
*/
6564
public BasicQuery(String query, String fields) {
6665

67-
this.queryObject = query != null ? Document.parse(query) : null;
68-
this.fieldsObject = fields != null ? Document.parse(fields) : null;
66+
this.queryObject = query != null ? Document.parse(query) : new Document();
67+
this.fieldsObject = fields != null ? Document.parse(fields) : new Document();
6968
}
7069

7170
/**
7271
* Create a new {@link BasicQuery} given a query {@link Document} and field specification {@link Document}.
7372
*
74-
* @param queryObject may be {@literal null}.
75-
* @param fieldsObject may be {@literal null}.
73+
* @param queryObject must not be {@literal null}.
74+
* @param fieldsObject must not be {@literal null}.
75+
* @throws IllegalArgumentException when {@code sortObject} or {@code fieldsObject} is {@literal null}.
7676
*/
7777
public BasicQuery(Document queryObject, Document fieldsObject) {
78+
79+
Assert.notNull(queryObject, "Query document must not be null");
80+
Assert.notNull(fieldsObject, "Field document must not be null");
81+
7882
this.queryObject = queryObject;
7983
this.fieldsObject = fieldsObject;
8084
}
@@ -85,15 +89,25 @@ public BasicQuery(Document queryObject, Document fieldsObject) {
8589
*/
8690
@Override
8791
public Query addCriteria(CriteriaDefinition criteria) {
92+
8893
this.queryObject.putAll(criteria.getCriteriaObject());
94+
8995
return this;
9096
}
9197

98+
/*
99+
* (non-Javadoc)
100+
* @see org.springframework.data.mongodb.core.query.Query#getQueryObject()
101+
*/
92102
@Override
93103
public Document getQueryObject() {
94104
return this.queryObject;
95105
}
96106

107+
/*
108+
* (non-Javadoc)
109+
* @see org.springframework.data.mongodb.core.query.Query#getFieldsObject()
110+
*/
97111
@Override
98112
public Document getFieldsObject() {
99113

@@ -112,31 +126,49 @@ public Document getFieldsObject() {
112126
return fieldsObject;
113127
}
114128

129+
/*
130+
* (non-Javadoc)
131+
* @see org.springframework.data.mongodb.core.query.Query#getSortObject()
132+
*/
115133
@Override
116134
public Document getSortObject() {
117135

118136
Document result = new Document();
137+
119138
if (sortObject != null) {
120139
result.putAll(sortObject);
121140
}
122141

123142
Document overrides = super.getSortObject();
124-
if (overrides != null) {
125-
result.putAll(overrides);
126-
}
143+
result.putAll(overrides);
127144

128145
return result;
129146
}
130147

148+
/**
149+
* Set the sort {@link Document}.
150+
*
151+
* @param sortObject must not be {@literal null}.
152+
* @throws IllegalArgumentException when {@code sortObject} is {@literal null}.
153+
*/
131154
public void setSortObject(Document sortObject) {
155+
156+
Assert.notNull(sortObject, "Sort document must not be null");
157+
132158
this.sortObject = sortObject;
133159
}
134160

135161
/**
162+
* Set the fields (projection) {@link Document}.
163+
*
164+
* @param fieldsObject must not be {@literal null}.
165+
* @throws IllegalArgumentException when {@code fieldsObject} is {@literal null}.
136166
* @since 1.6
137-
* @param fieldsObject
138167
*/
139168
protected void setFieldsObject(Document fieldsObject) {
169+
170+
Assert.notNull(sortObject, "Field document must not be null");
171+
140172
this.fieldsObject = fieldsObject;
141173
}
142174

‎spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.springframework.util.ObjectUtils.*;
2020

2121
import java.util.ArrayList;
22+
import java.util.Arrays;
2223
import java.util.Collections;
2324
import java.util.HashSet;
2425
import java.util.LinkedHashMap;
@@ -37,6 +38,8 @@
3738
import org.springframework.util.Assert;
3839

3940
/**
41+
* MongoDB Query object representing criteria, projection, sorting and query hints.
42+
*
4043
* @author Thomas Risberg
4144
* @author Oliver Gierke
4245
* @author Thomas Darimont
@@ -212,13 +215,14 @@ public Query restrict(Class<?> type, Class<?>... additionalTypes) {
212215
Assert.notNull(additionalTypes, "AdditionalTypes must not be null");
213216

214217
restrictedTypes.add(type);
215-
for (Class<?> additionalType : additionalTypes) {
216-
restrictedTypes.add(additionalType);
217-
}
218+
restrictedTypes.addAll(Arrays.asList(additionalTypes));
218219

219220
return this;
220221
}
221222

223+
/**
224+
* @return the query {@link Document}.
225+
*/
222226
public Document getQueryObject() {
223227

224228
Document document = new Document();
@@ -234,14 +238,20 @@ public Document getQueryObject() {
234238
return document;
235239
}
236240

241+
/**
242+
* @return the field {@link Document}.
243+
*/
237244
public Document getFieldsObject() {
238-
return this.fieldSpec == null ? null : fieldSpec.getFieldsObject();
245+
return this.fieldSpec == null ? new Document() : fieldSpec.getFieldsObject();
239246
}
240247

248+
/**
249+
* @return the sort {@link Document}.
250+
*/
241251
public Document getSortObject() {
242252

243253
if (this.sort.isUnsorted()) {
244-
return null;
254+
return new Document();
245255
}
246256

247257
Document document = new Document();

0 commit comments

Comments
 (0)