From efd74956dce6224610ccb76cb2d70c8f20d579cb Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Tue, 11 Feb 2014 17:37:47 +0100 Subject: [PATCH] DATAMONGO-812 - Polishing. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed convertToMongoType(…) to forward type hints to recursive calls to make sure type information is written if a TypeInformation was provided initially. Make sure that UpdateMapper hands in an initial type hint to the converter to make sure type information gets written. Changed the signature of QueryMapper.getMappedObjectForField(…) to allow customizing the entire entry being added to the result. This is in preparation of more advanced mappings that might have to customize the mapped key. Fixed newly introduced test cases in MongoTemplateTests. Original pull request: #112. --- .../core/convert/MappingMongoConverter.java | 26 +++++---------- .../mongodb/core/convert/QueryMapper.java | 19 +++++++---- .../mongodb/core/convert/UpdateMapper.java | 25 ++++++++++---- .../data/mongodb/core/MongoTemplateTests.java | 33 ++++++++++--------- .../MappingMongoConverterUnitTests.java | 24 +++++++++++--- .../core/convert/UpdateMapperUnitTests.java | 1 + 6 files changed, 77 insertions(+), 51 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 3da9c5a87..b73302f23 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 @@ -56,7 +56,6 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; -import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import com.mongodb.BasicDBList; @@ -904,15 +903,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return getPotentiallyConvertedSimpleWrite(obj); } + TypeInformation typeHint = typeInformation == null ? null : ClassTypeInformation.OBJECT; + if (obj instanceof BasicDBList) { - return maybeConvertList((BasicDBList) obj); + return maybeConvertList((BasicDBList) obj, typeHint); } if (obj instanceof DBObject) { DBObject newValueDbo = new BasicDBObject(); for (String vk : ((DBObject) obj).keySet()) { Object o = ((DBObject) obj).get(vk); - newValueDbo.put(vk, convertToMongoType(o)); + newValueDbo.put(vk, convertToMongoType(o, typeHint)); } return newValueDbo; } @@ -920,17 +921,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App if (obj instanceof Map) { DBObject result = new BasicDBObject(); for (Map.Entry entry : ((Map) obj).entrySet()) { - result.put(entry.getKey().toString(), convertToMongoType(entry.getValue())); + result.put(entry.getKey().toString(), convertToMongoType(entry.getValue(), typeHint)); } return result; } if (obj.getClass().isArray()) { - return maybeConvertList(Arrays.asList((Object[]) obj)); + return maybeConvertList(Arrays.asList((Object[]) obj), typeHint); } if (obj instanceof Collection) { - return maybeConvertList((Collection) obj); + return maybeConvertList((Collection) obj, typeHint); } DBObject newDbo = new BasicDBObject(); @@ -943,25 +944,16 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return !obj.getClass().equals(typeInformation.getType()) ? newDbo : removeTypeInfoRecursively(newDbo); } - public BasicDBList maybeConvertList(Iterable source) { + public BasicDBList maybeConvertList(Iterable source, TypeInformation typeInformation) { BasicDBList newDbl = new BasicDBList(); for (Object element : source) { - Object convertedTypeDdo = convertToMongoType(element); - if (!isSimpleOrCollectionType(element.getClass())) { - this.getTypeMapper().writeType(element.getClass(), (DBObject) convertedTypeDdo); - } - newDbl.add(convertedTypeDdo); + newDbl.add(convertToMongoType(element, typeInformation)); } return newDbl; } - private boolean isSimpleOrCollectionType(Class type) { - return this.conversions.isSimpleType(type) || type.getClass().isArray() - || ClassUtils.isAssignable(Collection.class, type); - } - /** * Removes the type information from the conversion result. * 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 928c4df4a..700c48bf9 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 @@ -17,7 +17,9 @@ package org.springframework.data.mongodb.core.convert; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map.Entry; import java.util.Set; import org.bson.types.ObjectId; @@ -106,11 +108,9 @@ public class QueryMapper { } Field field = createPropertyField(entity, key, mappingContext); + Entry entry = getMappedObjectForField(field, query.get(key)); - Object rawValue = query.get(key); - String newKey = field.getMappedKey(); - - result.put(newKey, getMappedObjectForField(field, rawValue)); + result.put(entry.getKey(), entry.getValue()); } return result; @@ -123,14 +123,19 @@ public class QueryMapper { * @param rawValue * @return */ - protected Object getMappedObjectForField(Field field, Object rawValue) { + protected Entry getMappedObjectForField(Field field, Object rawValue) { + + String key = field.getMappedKey(); + Object value; if (isNestedKeyword(rawValue) && !field.isIdField()) { Keyword keyword = new Keyword((DBObject) rawValue); - return getMappedKeyword(field, keyword); + value = getMappedKeyword(field, keyword); + } else { + value = getMappedValue(field, rawValue); } - return getMappedValue(field, rawValue); + return Collections.singletonMap(key, value).entrySet().iterator().next(); } /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java index 3733a4650..20802ae7f 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java @@ -16,7 +16,9 @@ package org.springframework.data.mongodb.core.convert; import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; +import java.util.Map.Entry; import org.springframework.core.convert.converter.Converter; import org.springframework.data.mapping.context.MappingContext; @@ -25,6 +27,7 @@ import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty; import org.springframework.data.mongodb.core.mapping.MongoPersistentProperty.PropertyToFieldNameConverter; import org.springframework.data.mongodb.core.query.Update.Modifier; import org.springframework.data.mongodb.core.query.Update.Modifiers; +import org.springframework.data.util.ClassTypeInformation; import org.springframework.util.Assert; import com.mongodb.BasicDBObject; @@ -70,33 +73,43 @@ public class UpdateMapper extends QueryMapper { * @see org.springframework.data.mongodb.core.convert.QueryMapper#getMappedObjectForField(org.springframework.data.mongodb.core.convert.QueryMapper.Field, java.lang.Object) */ @Override - protected Object getMappedObjectForField(Field field, Object rawValue) { + protected Entry getMappedObjectForField(Field field, Object rawValue) { if (!isUpdateModifier(rawValue)) { return super.getMappedObjectForField(field, rawValue); } + Object value = null; + if (rawValue instanceof Modifier) { - return getMappedValue((Modifier) rawValue); + + value = getMappedValue((Modifier) rawValue); + } else if (rawValue instanceof Modifiers) { DBObject modificationOperations = new BasicDBObject(); + for (Modifier modifier : ((Modifiers) rawValue).getModifiers()) { modificationOperations.putAll(getMappedValue(modifier).toMap()); } - return modificationOperations; + value = modificationOperations; + } else { + + throw new IllegalArgumentException(String.format("Unable to map value of type '%s'!", rawValue.getClass())); } - throw new IllegalArgumentException(String.format("Unable to map value of type '%s'!", rawValue.getClass())); + return Collections.singletonMap(field.getMappedKey(), value).entrySet().iterator().next(); } private boolean isUpdateModifier(Object value) { - return (value instanceof Modifier || value instanceof Modifiers); + return value instanceof Modifier || value instanceof Modifiers; } private DBObject getMappedValue(Modifier modifier) { - return new BasicDBObject(modifier.getKey(), this.converter.convertToMongoType(modifier.getValue())); + + Object value = converter.convertToMongoType(modifier.getValue(), ClassTypeInformation.OBJECT); + return new BasicDBObject(modifier.getKey(), value); } /* 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 5202129e4..2a9b6de6c 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 @@ -26,6 +26,7 @@ import static org.springframework.data.mongodb.core.query.Update.*; import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -2213,16 +2214,16 @@ public class MongoTemplateTests { @Test public void updatesShouldRetainTypeInformationEvenForCollections() { - DocumentWithCollection doc = new DocumentWithCollection(); + List models = Arrays. asList(new ModelA("foo")); + + DocumentWithCollection doc = new DocumentWithCollection(models); doc.id = "4711"; - doc.model = new ArrayList(); - doc.model.add(new ModelA("foo")); template.insert(doc); Query query = new Query(Criteria.where("id").is(doc.id)); - query.addCriteria(where("model.value").is("foo")); + query.addCriteria(where("models.value").is("foo")); String newModelValue = "bar"; - Update update = Update.update("model.$", new ModelA(newModelValue)); + Update update = Update.update("models.$", new ModelA(newModelValue)); template.updateFirst(query, update, DocumentWithCollection.class); Query findQuery = new Query(Criteria.where("id").is(doc.id)); @@ -2230,9 +2231,9 @@ public class MongoTemplateTests { assertThat(result, is(notNullValue())); assertThat(result.id, is(doc.id)); - assertThat(result.model, is(notNullValue())); - assertThat(result.model, hasSize(1)); - assertThat(result.model.get(0).value(), is(newModelValue)); + assertThat(result.models, is(notNullValue())); + assertThat(result.models, hasSize(1)); + assertThat(result.models.get(0).value(), is(newModelValue)); } /** @@ -2241,15 +2242,15 @@ public class MongoTemplateTests { @Test public void updateMultiShouldAddValuesCorrectlyWhenUsingPushEachWithComplexTypes() { - DocumentWithCollection document = new DocumentWithCollection(new ModelA("model-a")); + DocumentWithCollection document = new DocumentWithCollection(Collections. emptyList()); template.save(document); Query query = query(where("id").is(document.id)); - assumeThat(template.findOne(query, DocumentWithCollection.class).model, hasSize(1)); + assumeThat(template.findOne(query, DocumentWithCollection.class).models, hasSize(1)); - Update update = new Update().push("model").each(new ModelA("model-b"), new ModelA("model-c")); + Update update = new Update().push("models").each(new ModelA("model-b"), new ModelA("model-c")); template.updateMulti(query, update, DocumentWithCollection.class); - assertThat(template.findOne(query, DocumentWithCollection.class).model, hasSize(3)); + assertThat(template.findOne(query, DocumentWithCollection.class).models, hasSize(3)); } /** @@ -2273,11 +2274,11 @@ public class MongoTemplateTests { static class DocumentWithCollection { - @Id public String id; - public List model; + @Id String id; + List models; - DocumentWithCollection(Model... models) { - this.model = Arrays.asList(models); + DocumentWithCollection(List models) { + this.models = models; } } 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 49ba55c11..d0e534039 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 @@ -59,6 +59,7 @@ 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.PersonPojoStringId; +import org.springframework.data.util.ClassTypeInformation; import org.springframework.test.util.ReflectionTestUtils; import com.mongodb.BasicDBList; @@ -1023,7 +1024,7 @@ public class MappingMongoConverterUnitTests { address.city = "London"; address.street = "Foo"; - Object result = converter.convertToMongoType(Collections.singleton(address)); + Object result = converter.convertToMongoType(Collections.singleton(address), ClassTypeInformation.OBJECT); assertThat(result, is(instanceOf(BasicDBList.class))); Set readResult = converter.read(Set.class, (BasicDBList) result); @@ -1383,6 +1384,9 @@ public class MappingMongoConverterUnitTests { assertThat(aValue.get("c"), is((Object) "C")); } + /** + * @see DATAMONGO-812 + */ @Test public void convertsListToBasicDBListAndRetainsTypeInformationForComplexObjects() { @@ -1390,15 +1394,19 @@ public class MappingMongoConverterUnitTests { address.city = "London"; address.street = "Foo"; - Object result = converter.convertToMongoType(Collections.singletonList(address)); + Object result = converter.convertToMongoType(Collections.singletonList(address), + ClassTypeInformation.from(Address.class)); assertThat(result, is(instanceOf(BasicDBList.class))); BasicDBList dbList = (BasicDBList) result; assertThat(dbList, hasSize(1)); - assertThat(getTypedValue(getAsDBObject(dbList, 0), ("_class"), String.class), equalTo(Address.class.getName())); + assertThat(getTypedValue(getAsDBObject(dbList, 0), "_class", String.class), equalTo(Address.class.getName())); } + /** + * @see DATAMONGO-812 + */ @Test public void convertsListToBasicDBListWithoutTypeInformationForSimpleTypes() { @@ -1411,6 +1419,9 @@ public class MappingMongoConverterUnitTests { assertThat(dbList.get(0), instanceOf(String.class)); } + /** + * @see DATAMONGO-812 + */ @Test public void convertsArrayToBasicDBListAndRetainsTypeInformationForComplexObjects() { @@ -1418,15 +1429,18 @@ public class MappingMongoConverterUnitTests { address.city = "London"; address.street = "Foo"; - Object result = converter.convertToMongoType(new Address[] { address }); + Object result = converter.convertToMongoType(new Address[] { address }, ClassTypeInformation.OBJECT); assertThat(result, is(instanceOf(BasicDBList.class))); BasicDBList dbList = (BasicDBList) result; assertThat(dbList, hasSize(1)); - assertThat(getTypedValue(getAsDBObject(dbList, 0), ("_class"), String.class), equalTo(Address.class.getName())); + assertThat(getTypedValue(getAsDBObject(dbList, 0), "_class", String.class), equalTo(Address.class.getName())); } + /** + * @see DATAMONGO-812 + */ @Test public void convertsArrayToBasicDBListWithoutTypeInformationForSimpleTypes() { diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java index 61323dbce..2136fb753 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java @@ -258,6 +258,7 @@ public class UpdateMapperUnitTests { */ @Test public void testUpdateShouldAllowMultiplePushEachForDifferentFields() { + Update update = new Update().push("category").each("spring", "data").push("type").each("mongodb"); DBObject mappedObject = mapper.getMappedObject(update.getUpdateObject(), context.getPersistentEntity(Object.class));