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.

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

@@ -22,7 +22,6 @@ import java.util.Optional;
 
 import org.bson.Document;
 import org.springframework.dao.IncorrectResultSizeDataAccessException;
-import org.springframework.data.mongodb.core.query.BasicQuery;
 import org.springframework.data.mongodb.core.query.NearQuery;
 import org.springframework.data.mongodb.core.query.Query;
 import org.springframework.data.mongodb.core.query.SerializationUtils;
@@ -37,11 +36,12 @@ import com.mongodb.client.FindIterable;
  * Implementation of {@link ExecutableFindOperation}.
  *
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @since 2.0
  */
 class ExecutableFindOperationSupport implements ExecutableFindOperation {
 
-	private static final Query ALL_QUERY = new BasicQuery(new Document());
+	private static final Query ALL_QUERY = new Query();
 
 	private final MongoTemplate template;
 
@@ -164,7 +164,6 @@ class ExecutableFindOperationSupport implements ExecutableFindOperation {
 		}
 
 		private CloseableIterator<T> doStream() {
-
 			return template.doStream(query, domainType, getCollectionName(), returnType);
 		}
 

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

@@ -587,7 +587,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 
 	public <T> T findOne(Query query, Class<T> entityClass, String collectionName) {
 
-		if (query.getSortObject() == null) {
+		if (ObjectUtils.isEmpty(query.getSortObject()) && !query.getCollation().isPresent()) {
 			return doFindOne(collectionName, query.getQueryObject(), query.getFieldsObject(), entityClass);
 		} else {
 			query.limit(1);
@@ -1470,9 +1470,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 			if (query.getMeta() != null && query.getMeta().getMaxTimeMsec() != null) {
 				result = result.maxTime(query.getMeta().getMaxTimeMsec(), TimeUnit.MILLISECONDS);
 			}
-			if (query.getSortObject() != null) {
+			result = result.sort(query.getSortObject());
-				result = result.sort(query.getSortObject());
-			}
 
 			result = result.filter(queryMapper.getMappedObject(query.getQueryObject(), Optional.empty()));
 		}
@@ -2313,7 +2311,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 
 	private Document getMappedSortObject(Query query, Class<?> type) {
 
-		if (query == null || query.getSortObject() == null) {
+		if (query == null || ObjectUtils.isEmpty(query.getSortObject())) {
 			return null;
 		}
 
@@ -2373,7 +2371,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 
 		public FindOneCallback(Document query, Document fields) {
 			this.query = query;
-			this.fields = Optional.ofNullable(fields);
+			this.fields = Optional.ofNullable(fields).filter(it -> !ObjectUtils.isEmpty(fields));
 		}
 
 		public Document doInCollection(MongoCollection<Document> collection) throws MongoException, DataAccessException {
@@ -2414,7 +2412,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 		public FindCallback(Document query, Document fields) {
 
 			this.query = query != null ? query : new Document();
-			this.fields = Optional.ofNullable(fields);
+			this.fields = Optional.ofNullable(fields).filter(it -> !ObjectUtils.isEmpty(fields));
 		}
 
 		public FindIterable<Document> doInCollection(MongoCollection<Document> collection)
@@ -2607,9 +2605,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 				return cursor;
 			}
 
-			if (query.getSkip() <= 0 && query.getLimit() <= 0
+			if (query.getSkip() <= 0 && query.getLimit() <= 0 && ObjectUtils.isEmpty(query.getSortObject())
-					&& (query.getSortObject() == null || query.getSortObject().isEmpty()) && !StringUtils.hasText(query.getHint())
+					&& !StringUtils.hasText(query.getHint()) && !query.getMeta().hasValues()
-					&& !query.getMeta().hasValues() && !query.getCollation().isPresent()) {
+					&& !query.getCollation().isPresent()) {
 				return cursor;
 			}
 
@@ -2624,7 +2622,7 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware,
 				if (query.getLimit() > 0) {
 					cursorToUse = cursorToUse.limit(query.getLimit());
 				}
-				if (query.getSortObject() != null && !query.getSortObject().isEmpty()) {
+				if (!ObjectUtils.isEmpty(query.getSortObject())) {
 					Document sort = type != null ? getMappedSortObject(query, type) : query.getSortObject();
 					cursorToUse = cursorToUse.sort(sort);
 				}

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

@@ -50,10 +50,10 @@ import org.springframework.data.convert.EntityReader;
 import org.springframework.data.geo.Distance;
 import org.springframework.data.geo.GeoResult;
 import org.springframework.data.geo.Metric;
+import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mapping.PersistentPropertyAccessor;
 import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
-import org.springframework.data.mapping.MappingException;
 import org.springframework.data.mongodb.MongoDbFactory;
 import org.springframework.data.mongodb.ReactiveMongoDatabaseFactory;
 import org.springframework.data.mongodb.core.convert.DbRefProxyHandler;
@@ -1927,7 +1927,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
 
 	private Document getMappedSortObject(Query query, Class<?> type) {
 
-		if (query == null || query.getSortObject() == null) {
+		if (query == null) {
 			return null;
 		}
 
@@ -2279,7 +2279,7 @@ public class ReactiveMongoTemplate implements ReactiveMongoOperations, Applicati
 			findPublisherToUse = query.getCollation().map(Collation::toMongoCollation).map(findPublisher::collation)
 					.orElse(findPublisher);
 
-			if (query.getSkip() <= 0 && query.getLimit() <= 0 && query.getSortObject() == null
+			if (query.getSkip() <= 0 && query.getLimit() <= 0 && ObjectUtils.isEmpty(query.getSortObject())
 					&& !StringUtils.hasText(query.getHint()) && !query.getMeta().hasValues()) {
 				return findPublisherToUse;
 			}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2010-2016 the original author or authors.
+ * Copyright 2010-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -18,13 +18,11 @@ package org.springframework.data.mongodb.core.query;
 import static org.springframework.util.ObjectUtils.*;
 
 import org.bson.Document;
-
+import org.springframework.util.Assert;
-import com.mongodb.DBObject;
-import com.mongodb.util.JSON;
 
 /**
  * Custom {@link Query} implementation to setup a basic query from some arbitrary JSON query string.
- * 
+ *
  * @author Thomas Risberg
  * @author Oliver Gierke
  * @author Christoph Strobl
@@ -35,6 +33,7 @@ import com.mongodb.util.JSON;
 public class BasicQuery extends Query {
 
 	private final Document queryObject;
+
 	private Document fieldsObject;
 	private Document sortObject;
 
@@ -53,7 +52,7 @@ public class BasicQuery extends Query {
 	 * @param queryObject may be {@literal null}.
 	 */
 	public BasicQuery(Document queryObject) {
-		this(queryObject, null);
+		this(queryObject, new Document());
 	}
 
 	/**
@@ -64,17 +63,22 @@ public class BasicQuery extends Query {
 	 */
 	public BasicQuery(String query, String fields) {
 
-		this.queryObject = query != null ? Document.parse(query) : null;
+		this.queryObject = query != null ? Document.parse(query) : new Document();
-		this.fieldsObject = fields != null ? Document.parse(fields) : null;
+		this.fieldsObject = fields != null ? Document.parse(fields) : new Document();
 	}
 
 	/**
 	 * Create a new {@link BasicQuery} given a query {@link Document} and field specification {@link Document}.
 	 *
-	 * @param queryObject may be {@literal null}.
+	 * @param queryObject must not be {@literal null}.
-	 * @param fieldsObject may be {@literal null}.
+	 * @param fieldsObject must not be {@literal null}.
+	 * @throws IllegalArgumentException when {@code sortObject} or {@code fieldsObject} is {@literal null}.
 	 */
 	public BasicQuery(Document queryObject, Document fieldsObject) {
+
+		Assert.notNull(queryObject, "Query document must not be null");
+		Assert.notNull(fieldsObject, "Field document must not be null");
+
 		this.queryObject = queryObject;
 		this.fieldsObject = fieldsObject;
 	}
@@ -85,15 +89,25 @@ public class BasicQuery extends Query {
 	 */
 	@Override
 	public Query addCriteria(CriteriaDefinition criteria) {
+
 		this.queryObject.putAll(criteria.getCriteriaObject());
+
 		return this;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.core.query.Query#getQueryObject()
+	 */
 	@Override
 	public Document getQueryObject() {
 		return this.queryObject;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.core.query.Query#getFieldsObject()
+	 */
 	@Override
 	public Document getFieldsObject() {
 
@@ -112,31 +126,49 @@ public class BasicQuery extends Query {
 		return fieldsObject;
 	}
 
+	/*
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mongodb.core.query.Query#getSortObject()
+	 */
 	@Override
 	public Document getSortObject() {
 
 		Document result = new Document();
+
 		if (sortObject != null) {
 			result.putAll(sortObject);
 		}
 
 		Document overrides = super.getSortObject();
-		if (overrides != null) {
+		result.putAll(overrides);
-			result.putAll(overrides);
-		}
 
 		return result;
 	}
 
+	/**
+	 * Set the sort {@link Document}.
+	 *
+	 * @param sortObject must not be {@literal null}.
+	 * @throws IllegalArgumentException when {@code sortObject} is {@literal null}.
+	 */
 	public void setSortObject(Document sortObject) {
+
+		Assert.notNull(sortObject, "Sort document must not be null");
+
 		this.sortObject = sortObject;
 	}
 
 	/**
+	 * Set the fields (projection) {@link Document}.
+	 *
+	 * @param fieldsObject must not be {@literal null}.
+	 * @throws IllegalArgumentException when {@code fieldsObject} is {@literal null}.
 	 * @since 1.6
-	 * @param fieldsObject
 	 */
 	protected void setFieldsObject(Document fieldsObject) {
+
+		Assert.notNull(sortObject, "Field document must not be null");
+
 		this.fieldsObject = fieldsObject;
 	}
 

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

@@ -19,6 +19,7 @@ import static org.springframework.data.mongodb.core.query.SerializationUtils.*;
 import static org.springframework.util.ObjectUtils.*;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -37,6 +38,8 @@ import org.springframework.data.mongodb.core.Collation;
 import org.springframework.util.Assert;
 
 /**
+ * MongoDB Query object representing criteria, projection, sorting and query hints.
+ *
  * @author Thomas Risberg
  * @author Oliver Gierke
  * @author Thomas Darimont
@@ -212,13 +215,14 @@ public class Query {
 		Assert.notNull(additionalTypes, "AdditionalTypes must not be null");
 
 		restrictedTypes.add(type);
-		for (Class<?> additionalType : additionalTypes) {
+		restrictedTypes.addAll(Arrays.asList(additionalTypes));
-			restrictedTypes.add(additionalType);
-		}
 
 		return this;
 	}
 
+	/**
+	 * @return the query {@link Document}.
+	 */
 	public Document getQueryObject() {
 
 		Document document = new Document();
@@ -234,14 +238,20 @@ public class Query {
 		return document;
 	}
 
+	/**
+	 * @return the field {@link Document}.
+	 */
 	public Document getFieldsObject() {
-		return this.fieldSpec == null ? null : fieldSpec.getFieldsObject();
+		return this.fieldSpec == null ? new Document() : fieldSpec.getFieldsObject();
 	}
 
+	/**
+	 * @return the sort {@link Document}.
+	 */
 	public Document getSortObject() {
 
 		if (this.sort.isUnsorted()) {
-			return null;
+			return new Document();
 		}
 
 		Document document = new Document();

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014-2016 the original author or authors.
+ * Copyright 2014-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -21,8 +21,9 @@ import org.bson.Document;
 
 /**
  * {@link Query} implementation to be used to for performing full text searches.
- * 
+ *
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @since 1.6
  */
 public class TextQuery extends Query {
@@ -36,7 +37,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Creates new {@link TextQuery} using the the given {@code wordsAndPhrases} with {@link TextCriteria}
-	 * 
+	 *
 	 * @param wordsAndPhrases
 	 * @see TextCriteria#matching(String)
 	 */
@@ -48,7 +49,7 @@ public class TextQuery extends Query {
 	 * Creates new {@link TextQuery} in {@code language}. <br />
 	 * For a full list of supported languages see the mongdodb reference manual for
 	 * <a href="https://docs.mongodb.org/manual/reference/text-search-languages/">Text Search Languages</a>.
-	 * 
+	 *
 	 * @param wordsAndPhrases
 	 * @param language
 	 * @see TextCriteria#forLanguage(String)
@@ -62,7 +63,7 @@ public class TextQuery extends Query {
 	 * Creates new {@link TextQuery} using the {@code locale}s language.<br />
 	 * For a full list of supported languages see the mongdodb reference manual for
 	 * <a href="https://docs.mongodb.org/manual/reference/text-search-languages/">Text Search Languages</a>.
-	 * 
+	 *
 	 * @param wordsAndPhrases
 	 * @param locale
 	 */
@@ -72,7 +73,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Creates new {@link TextQuery} for given {@link TextCriteria}.
-	 * 
+	 *
 	 * @param criteria.
 	 */
 	public TextQuery(TextCriteria criteria) {
@@ -81,7 +82,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Creates new {@link TextQuery} searching for given {@link TextCriteria}.
-	 * 
+	 *
 	 * @param criteria
 	 * @return
 	 */
@@ -91,7 +92,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Add sorting by text score. Will also add text score to returned fields.
-	 * 
+	 *
 	 * @see TextQuery#includeScore()
 	 * @return
 	 */
@@ -104,7 +105,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Add field {@literal score} holding the documents textScore to the returned fields.
-	 * 
+	 *
 	 * @return
 	 */
 	public TextQuery includeScore() {
@@ -115,7 +116,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Include text search document score in returned fields using the given fieldname.
-	 * 
+	 *
 	 * @param fieldname
 	 * @return
 	 */
@@ -128,7 +129,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Set the fieldname used for scoring.
-	 * 
+	 *
 	 * @param fieldName
 	 */
 	public void setScoreFieldName(String fieldName) {
@@ -137,7 +138,7 @@ public class TextQuery extends Query {
 
 	/**
 	 * Get the fieldname used for scoring
-	 * 
+	 *
 	 * @return
 	 */
 	public String getScoreFieldName() {
@@ -178,9 +179,7 @@ public class TextQuery extends Query {
 			sort.put(getScoreFieldName(), META_TEXT_SCORE);
 		}
 
-		if (super.getSortObject() != null) {
+		sort.putAll(super.getSortObject());
-			sort.putAll(super.getSortObject());
-		}
 
 		return sort;
 	}

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/IsQuery.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -24,8 +24,9 @@ import org.springframework.util.StringUtils;
 
 /**
  * A {@link TypeSafeMatcher} that tests whether a given {@link Query} matches a query specification.
- * 
+ *
  * @author Christoph Strobl
+ * @author Mark Paluch
  * @param <T>
  */
 public class IsQuery<T extends Query> extends TypeSafeMatcher<T> {
@@ -41,6 +42,7 @@ public class IsQuery<T extends Query> extends TypeSafeMatcher<T> {
 	protected IsQuery() {
 		query = new Document();
 		sort = new Document();
+		fields = new Document();
 	}
 
 	public static <T extends BasicQuery> IsQuery<T> isQuery() {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java

@@ -204,13 +204,13 @@ public class QueryTests {
 		new Query().with(Sort.by(new Sort.Order("foo").ignoreCase()));
 	}
 
-	@Test // DATAMONGO-709
+	@Test // DATAMONGO-709, DATAMONGO-1735
 	@SuppressWarnings("unchecked")
 	public void shouldReturnClassHierarchyOfRestrictedTypes() {
 
 		Query query = new Query(where("name").is("foo")).restrict(SpecialDoc.class);
 		assertThat(query.toString(), is(
-				"Query: { \"name\" : \"foo\", \"_$RESTRICTED_TYPES\" : [ { $java : class org.springframework.data.mongodb.core.SpecialDoc } ] }, Fields: null, Sort: null"));
+				"Query: { \"name\" : \"foo\", \"_$RESTRICTED_TYPES\" : [ { $java : class org.springframework.data.mongodb.core.SpecialDoc } ] }, Fields: { }, Sort: { }"));
 		assertThat(query.getRestrictedTypes(), is(notNullValue()));
 		assertThat(query.getRestrictedTypes().size(), is(1));
 		assertThat(query.getRestrictedTypes(), hasItems(Arrays.asList(SpecialDoc.class).toArray(new Class<?>[0])));

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQueryUnitTests.java

@@ -53,10 +53,11 @@ import com.mongodb.util.JSONParseException;
 
 /**
  * Unit tests for {@link PartTreeMongoQuery}.
- * 
+ *
  * @author Christoph Strobl
  * @author Oliver Gierke
  * @author Thomas Darimont
+ * @author Mark Paluch
  */
 @RunWith(MockitoJUnitRunner.class)
 public class PartTreeMongoQueryUnitTests {
@@ -131,9 +132,9 @@ public class PartTreeMongoQueryUnitTests {
 		deriveQueryFromMethod("findByAge", new Object[] { 1 });
 	}
 
-	@Test // DATAMONGO-1345
+	@Test // DATAMONGO-1345, DATAMONGO-1735
 	public void doesNotDeriveFieldSpecForNormalDomainType() {
-		assertThat(deriveQueryFromMethod("findPersonBy", new Object[0]).getFieldsObject(), is(nullValue()));
+		assertThat(deriveQueryFromMethod("findPersonBy", new Object[0]).getFieldsObject(), is(new Document()));
 	}
 
 	@Test // DATAMONGO-1345
@@ -173,12 +174,12 @@ public class PartTreeMongoQueryUnitTests {
 		assertThat(query.getFieldsObject().get("firstname"), is((Object) 1));
 	}
 
-	@Test // DATAMONGO-1729
+	@Test // DATAMONGO-1729, DATAMONGO-1735
 	public void doesNotCreateFieldsObjectForOpenProjection() {
 
 		org.springframework.data.mongodb.core.query.Query query = deriveQueryFromMethod("findAllBy");
 
-		assertThat(query.getFieldsObject(), is(nullValue()));
+		assertThat(query.getFieldsObject(), is(new Document()));
 	}
 
 	private org.springframework.data.mongodb.core.query.Query deriveQueryFromMethod(String method, Object... args) {