From 624ec3ce556554074a583badf148c89de7b2e96d Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 29 Nov 2018 12:58:10 +0100 Subject: [PATCH] DATAMONGO-2149 - Fix $slice in fields projection when pointing to array of DBRefs. We now no longer try to convert the actual slice parameters into a DBRef. Original pull request: #623. --- .../mongodb/core/convert/QueryMapper.java | 14 +++++- ...tractPersonRepositoryIntegrationTests.java | 47 +++++++++++++++++++ .../mongodb/repository/PersonRepository.java | 6 +++ .../query/StringBasedMongoQueryUnitTests.java | 18 +++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 9747b45bd..c2f8d6b85 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -18,6 +18,7 @@ package org.springframework.data.mongodb.core.convert; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map.Entry; @@ -270,7 +271,7 @@ public class QueryMapper { */ protected DBObject getMappedKeyword(Field property, Keyword keyword) { - boolean needsAssociationConversion = property.isAssociation() && !keyword.isExists(); + boolean needsAssociationConversion = property.isAssociation() && !keyword.isExists() && keyword.mayHoldDbRef(); Object value = keyword.getValue(); Object convertedValue = needsAssociationConversion ? convertAssociation(value, property) @@ -544,10 +545,12 @@ public class QueryMapper { static class Keyword { private static final String N_OR_PATTERN = "\\$.*or"; + private static final Set NON_DBREF_CONVERTING_KEYWORDS = new HashSet(Arrays.asList("$", "$size", "$slice", "$gt", "$lt")); private final String key; private final Object value; + public Keyword(DBObject source, String key) { this.key = key; this.value = source.get(key); @@ -607,6 +610,15 @@ public class QueryMapper { public T getValue() { return (T) value; } + + /** + * + * @return {@literal true} if key may hold a DbRef. + * @since 1.10.18 + */ + public boolean mayHoldDbRef() { + return !NON_DBREF_CONVERTING_KEYWORDS.contains(key); + } } /** diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java index 96786dcc9..9b5ad96cc 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/AbstractPersonRepositoryIntegrationTests.java @@ -23,7 +23,9 @@ import static org.springframework.data.geo.Metrics.*; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -56,6 +58,7 @@ import org.springframework.data.geo.Polygon; import org.springframework.data.mongodb.core.MongoOperations; import org.springframework.data.mongodb.core.geo.GeoJsonPoint; import org.springframework.data.mongodb.core.query.BasicQuery; +import org.springframework.data.mongodb.core.query.Query; import org.springframework.data.mongodb.repository.Person.Sex; import org.springframework.data.mongodb.repository.SampleEvaluationContextExtension.SampleSecurityContextHolder; import org.springframework.data.querydsl.QSort; @@ -1181,4 +1184,48 @@ public abstract class AbstractPersonRepositoryIntegrationTests { assertThat(repository.findByFirstnameRegex(Pattern.compile(fn)), hasSize(0)); assertThat(repository.findByFirstnameRegex(Pattern.compile(fn, Pattern.CASE_INSENSITIVE)), hasSize(1)); } + + @Test // DATAMONGO-2149 + public void annotatedQueryShouldAllowSliceInFieldsProjectionWithDbRef() { + + operations.remove(new Query(), User.class); + + List users = new ArrayList(); + + for (int i = 0; i < 10; i++) { + + User user = new User(); + user.id = "id" + i; + user.username = "user" + i; + + users.add(user); + + operations.save(user); + } + + alicia.fans = new ArrayList(users); + operations.save(alicia); + + Person target = repository.findWithSliceInProjection(alicia.getId(), 0, 5); + assertThat(target.getFans(), hasSize(5)); + } + + @Test // DATAMONGO-2149 + public void annotatedQueryShouldAllowPositionalParameterInFieldsProjection() { + + Set
addressList = new LinkedHashSet
(); + + for (int i = 0; i < 10; i++) { + addressList.add(new Address("street-" + i, "zip", "lnz")); + } + + alicia.setShippingAddresses(addressList); + operations.save(alicia); + + Person target = repository.findWithArrayPositionInProjection(1); + + assertThat(target, notNullValue()); + assertThat(target.getShippingAddresses(), hasSize(1)); + } + } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java index ec391804f..b372f5102 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/PersonRepository.java @@ -328,4 +328,10 @@ public interface PersonRepository extends MongoRepository, Query void deleteByThePersonsFirstname(String firstname); List findByFirstnameRegex(Pattern pattern); + + @Query(value = "{ 'id' : ?0 }", fields = "{ 'fans': { '$slice': [ ?1, ?2 ] } }") + Person findWithSliceInProjection(String id, int skip, int limit); + + @Query(value = "{ 'shippingAddresses' : { '$elemMatch' : { 'city' : { '$eq' : 'lnz' } } } }", fields = "{ 'shippingAddresses.$': ?0 }") + Person findWithArrayPositionInProjection(int position); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java index 8e467a508..397454678 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/repository/query/StringBasedMongoQueryUnitTests.java @@ -529,6 +529,18 @@ public class StringBasedMongoQueryUnitTests { assertThat(query.getQueryObject(), is((DBObject) new BasicDBObject().append("arg0", null))); } + @Test // DATAMONGO-2149 + public void shouldParseFieldsProjectionWithSliceCorrectly() throws Exception { + + StringBasedMongoQuery mongoQuery = createQueryForMethod("findWithSliceInProjection", String.class, int.class, + int.class); + ConvertingParameterAccessor accessor = StubParameterAccessor.getAccessor(converter, "Bruce Banner", 0, 5); + + org.springframework.data.mongodb.core.query.Query query = mongoQuery.createQuery(accessor); + + assertThat(query.getFieldsObject(), is(equalTo(JSON.parse("{ \"fans\" : { \"$slice\" : [0, 5] } }")))); + } + private StringBasedMongoQuery createQueryForMethod(String name, Class... parameters) throws Exception { Method method = SampleRepository.class.getMethod(name, parameters); @@ -623,5 +635,11 @@ public class StringBasedMongoQueryUnitTests { @Query("{ arg0 : ?#{[0]} }") List findByUsingSpel(Object arg0); + + @Query("{ 'lastname' : { '$regex' : ?#{[0].lastname} } }") + Person findByPersonLastnameRegex(Person key); + + @Query(value = "{ 'id' : ?0 }", fields = "{ 'fans': { '$slice': [ ?1, ?2 ] } }") + Person findWithSliceInProjection(String id, int skip, int limit); } }