From d19ea88670b9963543ef2d9ef24848ac1a5dacac Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 7 Jul 2017 14:07:33 +0200 Subject: [PATCH] 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. --- .../core/ExecutableFindOperationSupport.java | 5 +- .../data/mongodb/core/MongoTemplate.java | 20 +++---- .../mongodb/core/ReactiveMongoTemplate.java | 6 +- .../data/mongodb/core/query/BasicQuery.java | 60 ++++++++++++++----- .../data/mongodb/core/query/Query.java | 20 +++++-- .../data/mongodb/core/query/TextQuery.java | 29 +++++---- .../data/mongodb/core/query/IsQuery.java | 6 +- .../data/mongodb/core/query/QueryTests.java | 4 +- .../query/PartTreeMongoQueryUnitTests.java | 11 ++-- 9 files changed, 101 insertions(+), 60 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ExecutableFindOperationSupport.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ExecutableFindOperationSupport.java index 67f8ddf6b..15a3952c4 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ExecutableFindOperationSupport.java +++ b/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 doStream() { - return template.doStream(query, domainType, getCollectionName(), returnType); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java index 31a2fbe35..3b42a293a 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/MongoTemplate.java +++ b/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 findOne(Query query, Class 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 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 doInCollection(MongoCollection collection) @@ -2607,9 +2605,9 @@ public class MongoTemplate implements MongoOperations, ApplicationContextAware, return cursor; } - if (query.getSkip() <= 0 && query.getLimit() <= 0 - && (query.getSortObject() == null || query.getSortObject().isEmpty()) && !StringUtils.hasText(query.getHint()) - && !query.getMeta().hasValues() && !query.getCollation().isPresent()) { + if (query.getSkip() <= 0 && query.getLimit() <= 0 && ObjectUtils.isEmpty(query.getSortObject()) + && !StringUtils.hasText(query.getHint()) && !query.getMeta().hasValues() + && !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); } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java index f520a7713..b279bfe19 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/ReactiveMongoTemplate.java +++ b/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; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java index 539956713..dfc452648 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/BasicQuery.java +++ b/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 com.mongodb.DBObject; -import com.mongodb.util.JSON; +import org.springframework.util.Assert; /** * 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.fieldsObject = fields != null ? Document.parse(fields) : null; + this.queryObject = query != null ? Document.parse(query) : new Document(); + 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 fieldsObject may be {@literal null}. + * @param queryObject must not 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; } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java index 87f2427f0..25e9dffeb 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/Query.java +++ b/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.add(additionalType); - } + restrictedTypes.addAll(Arrays.asList(additionalTypes)); 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(); diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/TextQuery.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/TextQuery.java index 3ea6f7056..ac58dbc54 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/query/TextQuery.java +++ b/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}.
* For a full list of supported languages see the mongdodb reference manual for * Text Search Languages. - * + * * @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.
* For a full list of supported languages see the mongdodb reference manual for * Text Search Languages. - * + * * @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; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/IsQuery.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/IsQuery.java index 0d8a855e6..614c180d9 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/IsQuery.java +++ b/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 */ public class IsQuery extends TypeSafeMatcher { @@ -41,6 +42,7 @@ public class IsQuery extends TypeSafeMatcher { protected IsQuery() { query = new Document(); sort = new Document(); + fields = new Document(); } public static IsQuery isQuery() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java index 18032fc5e..cfbcf4f0a 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/query/QueryTests.java +++ b/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]))); diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQueryUnitTests.java index 341ea8880..15bea55e7 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/PartTreeMongoQueryUnitTests.java +++ b/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) {