From 3c16b4db7fecdb058bf99a62c66b41a42f11d612 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 2 Nov 2016 13:33:54 +0100 Subject: [PATCH] DATAMONGO-1509 - Write type hint as last element of a Document. Always add type hint as last property of a Document. This is necessary to assure document equality within MongoDB in cases where the query contains full document comparisons. Unfortunately this also might break existing stuff since the order of properties within a Document is changed with this commit. Original pull request: #411. --- .../core/convert/MappingMongoConverter.java | 14 ++++++------- .../data/mongodb/core/DocumentTestUtils.java | 20 +++++++++++++++++++ .../data/mongodb/core/MongoTemplateTests.java | 19 ++++++++++++++++++ .../MappingMongoConverterUnitTests.java | 19 +++++++++++------- 4 files changed, 58 insertions(+), 14 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 eb01d1dd8..d3585d056 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 @@ -359,20 +359,20 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return; } - Class entityType = obj.getClass(); - boolean handledByCustomConverter = conversions.getCustomWriteTarget(entityType, Document.class) != null; + Class entityType = ClassUtils.getUserClass(obj.getClass()); TypeInformation type = ClassTypeInformation.from(entityType); - if (!handledByCustomConverter && !(bson instanceof Collection)) { - typeMapper.writeType(type, bson); - } - Object target = obj instanceof LazyLoadingProxy ? ((LazyLoadingProxy) obj).getTarget() : obj; writeInternal(target, bson, type); if (asMap(bson).containsKey("_is") && asMap(bson).get("_id") == null) { removeFromMap(bson, "_id"); } + + boolean handledByCustomConverter = conversions.getCustomWriteTarget(entityType, Document.class) != null; + if (!handledByCustomConverter && !(bson instanceof Collection)) { + typeMapper.writeType(type, bson); + } } /** @@ -529,12 +529,12 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App Object existingValue = accessor.get(prop); Document document = existingValue instanceof Document ? (Document) existingValue : new Document(); - addCustomTypeKeyIfNecessary(ClassTypeInformation.from(prop.getRawType()), obj, document); MongoPersistentEntity entity = isSubtype(prop.getType(), obj.getClass()) ? mappingContext.getPersistentEntity(obj.getClass()) : mappingContext.getPersistentEntity(type); writeInternal(obj, document, entity); + addCustomTypeKeyIfNecessary(ClassTypeInformation.from(prop.getRawType()), obj, document); accessor.put(prop, document); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DocumentTestUtils.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DocumentTestUtils.java index 533146753..f6c0f8384 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DocumentTestUtils.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/DocumentTestUtils.java @@ -18,6 +18,7 @@ package org.springframework.data.mongodb.core; import static org.hamcrest.Matchers.*; import static org.junit.Assert.*; +import java.util.Iterator; import java.util.List; import org.bson.Document; @@ -80,4 +81,23 @@ public abstract class DocumentTestUtils { return (T) value; } + + public static void assertTypeHint(Document document, Class type) { + assertTypeHint(document, type.getName()); + } + + public static void assertTypeHint(Document document, String expectedTypeString) { + + Iterator keyIterator = document.keySet().iterator(); + while (keyIterator.hasNext()) { + String key = keyIterator.next(); + if (key.equals("_class")) { + assertThat((String) document.get(key), is(equalTo(expectedTypeString))); + assertThat(keyIterator.hasNext(), is(false)); + return; + } + } + + fail(String.format("Expected to find type info %s in %s.", document, expectedTypeString)); + } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index feb1bd8e1..1b567948d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -44,6 +44,7 @@ import org.hamcrest.collection.IsMapContaining; import org.joda.time.DateTime; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -3486,6 +3487,22 @@ public class MongoTemplateTests { assertThat(document.id, is(notNullValue())); } + + /** + * @see DATAMONGO-1509 + */ + @Test + public void findsByGnericNestedListElements() { + + List modelList = Arrays.asList(new ModelA("value")); + DocumentWithCollection dwc = new DocumentWithCollection(modelList); + + template.insert(dwc); + + Query query = query(where("models").is(modelList)); + assertThat(template.findOne(query, DocumentWithCollection.class), is(equalTo(dwc))); + } + static class TypeWithNumbers { @Id String id; @@ -3565,6 +3582,7 @@ public class MongoTemplateTests { @org.springframework.data.mongodb.core.mapping.DBRef(lazy = true) public Map lazyDbRefAnnotatedMap; } + @EqualsAndHashCode static class DocumentWithCollection { @Id String id; @@ -3617,6 +3635,7 @@ public class MongoTemplateTests { String id(); } + @EqualsAndHashCode static class ModelA implements Model { @Id String id; 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 75b65ff86..adff1b333 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 @@ -526,6 +526,9 @@ public class MappingMongoConverterUnitTests { assertThat(converter.convertToMongoType(null), is(nullValue())); } + /** + * @see DATAMONGO-1509 + */ @Test public void writesGenericTypeCorrectly() { @@ -537,7 +540,7 @@ public class MappingMongoConverterUnitTests { converter.write(type, result); org.bson.Document content = (org.bson.Document) result.get("content"); - assertThat(content.get("_class"), is(notNullValue())); + assertTypeHint(content, Address.class); assertThat(content.get("city"), is(notNullValue())); } @@ -1272,6 +1275,7 @@ public class MappingMongoConverterUnitTests { /** * @see DATAMONGO-523 + * @see DATAMONGO-1509 */ @Test public void considersTypeAliasAnnotation() { @@ -1282,9 +1286,7 @@ public class MappingMongoConverterUnitTests { org.bson.Document result = new org.bson.Document(); converter.write(aliased, result); - Object type = result.get("_class"); - assertThat(type, is(notNullValue())); - assertThat(type.toString(), is("_")); + assertTypeHint(result, "_"); } /** @@ -1409,6 +1411,7 @@ public class MappingMongoConverterUnitTests { /** * @see DATAMONGO-812 * @see DATAMONGO-893 + * @see DATAMONGO-1509 */ @Test public void convertsListToBasicDBListAndRetainsTypeInformationForComplexObjects() { @@ -1424,7 +1427,7 @@ public class MappingMongoConverterUnitTests { List dbList = (List) result; assertThat(dbList, hasSize(1)); - assertThat(getTypedValue(getAsDocument(dbList, 0), "_class", String.class), equalTo(Address.class.getName())); + assertTypeHint(getAsDocument(dbList, 0), Address.class); } /** @@ -1444,6 +1447,7 @@ public class MappingMongoConverterUnitTests { /** * @see DATAMONGO-812 + * @see DATAMONGO-1509 */ @Test public void convertsArrayToBasicDBListAndRetainsTypeInformationForComplexObjects() { @@ -1458,7 +1462,7 @@ public class MappingMongoConverterUnitTests { List dbList = (List) result; assertThat(dbList, hasSize(1)); - assertThat(getTypedValue(getAsDocument(dbList, 0), "_class", String.class), equalTo(Address.class.getName())); + assertTypeHint(getAsDocument(dbList, 0), Address.class); } /** @@ -1802,6 +1806,7 @@ public class MappingMongoConverterUnitTests { /** * @see DATAMONGO-1001 + * @see DATAMONGO-1509 */ @Test public void shouldWriteCglibProxiedClassTypeInformationCorrectly() { @@ -1814,7 +1819,7 @@ public class MappingMongoConverterUnitTests { org.bson.Document document = new org.bson.Document(); converter.write(proxied, document); - assertThat(document.get("_class"), is((Object) GenericType.class.getName())); + assertTypeHint(document, GenericType.class); } /**