From abfb98afe15d83a54e0498d4081d6f8384be05f0 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Thu, 3 Apr 2014 13:22:38 +0200 Subject: [PATCH] DATAMONGO-893 - Converter must not write "_class" information for know types. We now actively pass on property type information to MetadataBackedField to ensure type hints get picked up correctly when converting a value to the according DBObject. This has to be done as the fix for DATAMONGO-812 enforced proper writing of _class information for Updates, which caused trouble when querying documents by nested (complex) properties using an 'in' clause. Original pull request: #169. --- .../core/convert/MappingMongoConverter.java | 2 +- .../mongodb/core/convert/QueryMapper.java | 21 ++++++++++-- .../MappingMongoConverterUnitTests.java | 9 +++-- .../core/convert/QueryMapperUnitTests.java | 33 +++++++++++++++++++ ...tractPersonRepositoryIntegrationTests.java | 22 +++++++++++-- .../mongodb/repository/PersonRepository.java | 7 +++- 6 files changed, 84 insertions(+), 10 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index 5b7704cd1..ea5c9f419 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -956,7 +956,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return getPotentiallyConvertedSimpleWrite(obj); } - TypeInformation typeHint = typeInformation == null ? null : ClassTypeInformation.OBJECT; + TypeInformation typeHint = typeInformation == null ? ClassTypeInformation.OBJECT : typeInformation; if (obj instanceof BasicDBList) { return maybeConvertList((BasicDBList) obj, typeHint); 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 1237573ae..283e98772 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 @@ -306,7 +306,7 @@ public class QueryMapper { * @return the converted mongo type or null if source is null */ protected Object delegateConvertToMongoType(Object source, MongoPersistentEntity entity) { - return converter.convertToMongoType(source); + return converter.convertToMongoType(source, entity == null ? null : entity.getTypeInformation()); } protected Object convertAssociation(Object source, Field field) { @@ -611,6 +611,21 @@ public class QueryMapper { */ public MetadataBackedField(String name, MongoPersistentEntity entity, MappingContext, MongoPersistentProperty> context) { + this(name, entity, context, null); + } + + /** + * Creates a new {@link MetadataBackedField} with the given name, {@link MongoPersistentEntity} and + * {@link MappingContext} with the given {@link MongoPersistentProperty}. + * + * @param name must not be {@literal null} or empty. + * @param entity must not be {@literal null}. + * @param context must not be {@literal null}. + * @param property may be {@literal null}. + */ + public MetadataBackedField(String name, MongoPersistentEntity entity, + MappingContext, MongoPersistentProperty> context, + MongoPersistentProperty property) { super(name); @@ -620,7 +635,7 @@ public class QueryMapper { this.mappingContext = context; this.path = getPath(name); - this.property = path == null ? null : path.getLeafProperty(); + this.property = path == null ? property : path.getLeafProperty(); this.association = findAssociation(); } @@ -630,7 +645,7 @@ public class QueryMapper { */ @Override public MetadataBackedField with(String name) { - return new MetadataBackedField(name, entity, mappingContext); + return new MetadataBackedField(name, entity, mappingContext, property); } /* diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java index 2e9b3e524..89355fa9d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java @@ -1390,6 +1390,7 @@ public class MappingMongoConverterUnitTests { /** * @see DATAMONGO-812 + * @see DATAMONGO-893 */ @Test public void convertsListToBasicDBListAndRetainsTypeInformationForComplexObjects() { @@ -1399,7 +1400,7 @@ public class MappingMongoConverterUnitTests { address.street = "Foo"; Object result = converter.convertToMongoType(Collections.singletonList(address), - ClassTypeInformation.from(Address.class)); + ClassTypeInformation.from(InterfaceType.class)); assertThat(result, is(instanceOf(BasicDBList.class))); @@ -1554,7 +1555,11 @@ public class MappingMongoConverterUnitTests { abstract void method(); } - static class Address { + static interface InterfaceType { + + } + + static class Address implements InterfaceType { String street; String city; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java index c30707fac..e424f7b89 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/QueryMapperUnitTests.java @@ -39,6 +39,7 @@ import org.springframework.data.mongodb.core.DBObjectTestUtils; import org.springframework.data.mongodb.core.Person; import org.springframework.data.mongodb.core.mapping.BasicMongoPersistentEntity; import org.springframework.data.mongodb.core.mapping.DBRef; +import org.springframework.data.mongodb.core.mapping.Document; import org.springframework.data.mongodb.core.mapping.Field; import org.springframework.data.mongodb.core.mapping.MongoMappingContext; import org.springframework.data.mongodb.core.mapping.MongoPersistentEntity; @@ -568,10 +569,42 @@ public class QueryMapperUnitTests { assertThat(mappedFields, is(notNullValue())); } + /** + * @see DATAMONGO-893 + */ + @Test + public void classInformationShouldNotBePresentInDBObjectUsedInFinderMethods() { + + EmbeddedClass embedded = new EmbeddedClass(); + embedded.id = "1"; + + EmbeddedClass embedded2 = new EmbeddedClass(); + embedded2.id = "2"; + Query query = query(where("embedded").in(Arrays.asList(embedded, embedded2))); + + DBObject dbo = mapper.getMappedObject(query.getQueryObject(), context.getPersistentEntity(Foo.class)); + assertThat(dbo.toString(), equalTo("{ \"embedded\" : { \"$in\" : [ { \"_id\" : \"1\"} , { \"_id\" : \"2\"}]}}")); + } + + @Document + public class Foo { + @Id private ObjectId id; + EmbeddedClass embedded; + } + + public class EmbeddedClass { + public String id; + } + class IdWrapper { Object id; } + class ClassWithEmbedded { + @Id String id; + Sample sample; + } + class ClassWithDefaultId { String id; 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 fcb47037b..d1d2b6f05 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 @@ -739,7 +739,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { assertThat(result.size(), is(1)); assertThat(result.get(0), is(dave)); } - + /** * @see DATAMONGO-871 */ @@ -751,8 +751,7 @@ public abstract class AbstractPersonRepositoryIntegrationTests { assertThat(result, is(arrayWithSize(1))); assertThat(result, is(arrayContaining(leroi))); } - - + /** * @see DATAMONGO-821 */ @@ -772,4 +771,21 @@ public abstract class AbstractPersonRepositoryIntegrationTests { assertThat(result.getNumberOfElements(), is(1)); assertThat(result.getContent().get(0), is(alicia)); } + + /** + * @see DATAMONGO-893 + */ + @Test + public void findByNestedPropertyInCollectionShouldFindMatchingDocuments() { + + Person p = new Person("Mary", "Poppins"); + Address adr = new Address("some", "2", "where"); + p.setAddress(adr); + + repository.save(p); + + Page result = repository.findByAddressIn(Arrays.asList(adr), new PageRequest(0, 10)); + + assertThat(result.getContent(), 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 d42caf947..72ba9b351 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 @@ -251,10 +251,15 @@ public interface PersonRepository extends MongoRepository, Query * @see DATAMONGO-770 */ List findByFirstnameContainingIgnoreCase(String firstName); - + /** * @see DATAMONGO-821 */ @Query("{ creator : { $exists : true } }") Page findByHavingCreator(Pageable page); + + /** + * @see DATAMONGO-893 + */ + Page findByAddressIn(List
address, Pageable page); }