From 760d7d6a327ca282cc6f600e06379614589d7a73 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Fri, 19 Aug 2016 16:54:04 +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 7cb22d51f..a539a64b5 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 @@ -265,7 +265,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; }