From be549b3d23b560ffbff14e7d457a393a36fd0bb3 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 17 Aug 2016 16:52:36 +0200 Subject: [PATCH] DATAMONGO-1471 - Converter only applies identifier values if actually available. Setting the value for the identifier property is an explicit step in MappingMongoConverter and always executed if the type to be created has an identifier property. If the source document doesn't contain an _id field (e.g. because it has been excluded explicitly) that previously caused null to be set on the identifier. This caused an exception if the identifier property is a primitive type. We now explicitly check whether the field backing the identifier property is actually present in the source document and only explicitly set the value if so. --- .../core/convert/DBObjectAccessor.java | 34 +++++++++++++++++++ .../core/convert/MappingMongoConverter.java | 2 +- .../convert/DBObjectAccessorUnitTests.java | 14 ++++++++ .../MappingMongoConverterUnitTests.java | 8 +++++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java index a3d9cc9b1..993dd0ff1 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java @@ -114,6 +114,40 @@ class DBObjectAccessor { return result; } + /** + * Returns whether the underlying {@link DBObject} has a value ({@literal null} or non-{@literal null}) for the given + * {@link MongoPersistentProperty}. + * + * @param property must not be {@literal null}. + * @return + */ + public boolean hasValue(MongoPersistentProperty property) { + + Assert.notNull(property, "Property must not be null!"); + + String fieldName = property.getFieldName(); + + if (!fieldName.contains(".")) { + return this.dbObject.containsField(fieldName); + } + + String[] parts = fieldName.split("\\."); + Map source = this.dbObject; + Object result = null; + + for (int i = 1; i < parts.length; i++) { + + result = source.get(parts[i - 1]); + source = getAsMap(result); + + if (source == null) { + return false; + } + } + + return source.containsKey(parts[parts.length - 1]); + } + /** * Returns the given source object as map, i.e. {@link BasicDBObject}s and maps as is or {@literal null} otherwise. * 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 947b82057..59049c01c 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 @@ -260,7 +260,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App // make sure id property is set before all other properties Object idValue = null; - if (idProperty != null) { + if (idProperty != null && new DBObjectAccessor(dbo).hasValue(idProperty)) { idValue = getValueInternal(idProperty, dbo, evaluator, path); accessor.setProperty(idProperty, idValue); } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DBObjectAccessorUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DBObjectAccessorUnitTests.java index b4ad05236..298d99c1d 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DBObjectAccessorUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/DBObjectAccessorUnitTests.java @@ -101,6 +101,20 @@ public class DBObjectAccessorUnitTests { assertThat(nestedA.get("c"), is((Object) "c")); } + /** + * @see DATAMONGO-1471 + */ + @Test + public void exposesAvailabilityOfFields() { + + DBObjectAccessor accessor = new DBObjectAccessor(new BasicDBObject("a", new BasicDBObject("c", "d"))); + MongoPersistentEntity entity = context.getPersistentEntity(ProjectingType.class); + + assertThat(accessor.hasValue(entity.getPersistentProperty("foo")), is(false)); + assertThat(accessor.hasValue(entity.getPersistentProperty("a")), is(true)); + assertThat(accessor.hasValue(entity.getPersistentProperty("name")), is(false)); + } + static class ProjectingType { String name; 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 4430a0726..9328ad1fc 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 @@ -2066,6 +2066,14 @@ public class MappingMongoConverterUnitTests { assertThat(target.map.get(FooBarEnum.FOO), is("spring")); } + /** + * @see DATAMONGO-1471 + */ + @Test + public void readsDocumentWithPrimitiveIdButNoValue() { + assertThat(converter.read(ClassWithIntId.class, new BasicDBObject()), is(notNullValue())); + } + static class GenericType { T content; }