diff --git a/pom.xml b/pom.xml index f785c3872d..3b683893a2 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-mongodb-parent - 4.4.0-SNAPSHOT + 4.4.x-GH-4710-SNAPSHOT pom Spring Data MongoDB diff --git a/spring-data-mongodb-benchmarks/pom.xml b/spring-data-mongodb-benchmarks/pom.xml index a3dc49f892..c6ec56d465 100644 --- a/spring-data-mongodb-benchmarks/pom.xml +++ b/spring-data-mongodb-benchmarks/pom.xml @@ -7,7 +7,7 @@ org.springframework.data spring-data-mongodb-parent - 4.4.0-SNAPSHOT + 4.4.x-GH-4710-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb-distribution/pom.xml b/spring-data-mongodb-distribution/pom.xml index acdc13437d..5612e61282 100644 --- a/spring-data-mongodb-distribution/pom.xml +++ b/spring-data-mongodb-distribution/pom.xml @@ -15,7 +15,7 @@ org.springframework.data spring-data-mongodb-parent - 4.4.0-SNAPSHOT + 4.4.x-GH-4710-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/pom.xml b/spring-data-mongodb/pom.xml index fafe9c8793..c1a519c90c 100644 --- a/spring-data-mongodb/pom.xml +++ b/spring-data-mongodb/pom.xml @@ -13,7 +13,7 @@ org.springframework.data spring-data-mongodb-parent - 4.4.0-SNAPSHOT + 4.4.x-GH-4710-SNAPSHOT ../pom.xml diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java index bade3ab7fc..0bfdfee010 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java @@ -92,6 +92,10 @@ public void put(MongoPersistentProperty prop, @Nullable Object value) { Assert.notNull(prop, "MongoPersistentProperty must not be null"); + if (value == null && !prop.writeNullValues()) { + return; + } + Iterator parts = Arrays.asList(prop.getMongoField().getName().parts()).iterator(); Bson document = this.document; @@ -172,4 +176,5 @@ private static Document getOrCreateNestedDocument(String key, Bson source) { return nested; } + } 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 4e38ab25c5..8db67109e5 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 @@ -53,7 +53,9 @@ import org.springframework.core.env.StandardEnvironment; import org.springframework.data.annotation.Reference; import org.springframework.data.convert.CustomConversions; +import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.convert.TypeMapper; +import org.springframework.data.convert.ValueConversionContext; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.InstanceCreatorMetadata; import org.springframework.data.mapping.MappingException; @@ -176,12 +178,11 @@ public MappingMongoConverter(DbRefResolver dbRefResolver, this.idMapper = new QueryMapper(this); this.spELContext = new SpELContext(DocumentPropertyAccessor.INSTANCE); - this.dbRefProxyHandler = new DefaultDbRefProxyHandler(mappingContext, - (prop, bson, evaluator, path) -> { + this.dbRefProxyHandler = new DefaultDbRefProxyHandler(mappingContext, (prop, bson, evaluator, path) -> { - ConversionContext context = getConversionContext(path); - return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator); - }, expressionEvaluatorFactory::create); + ConversionContext context = getConversionContext(path); + return MappingMongoConverter.this.getValueInternal(context, prop, bson, evaluator); + }, expressionEvaluatorFactory::create); this.referenceLookupDelegate = new ReferenceLookupDelegate(mappingContext, spELContext); this.documentPointerFactory = new DocumentPointerFactory(conversionService, mappingContext); @@ -279,6 +280,7 @@ public void setCodecRegistryProvider(@Nullable CodecRegistryProvider codecRegist this.codecRegistryProvider = codecRegistryProvider; } + @Override public MappingContext, MongoPersistentProperty> getMappingContext() { return mappingContext; } @@ -431,6 +433,7 @@ public Map getBean() { } } + @Override public S read(Class clazz, Bson bson) { return read(TypeInformation.of(clazz), bson); } @@ -728,6 +731,7 @@ private Object readUnwrapped(ConversionContext context, DocumentAccessor documen return null; } + @Override public DBRef toDBRef(Object object, @Nullable MongoPersistentProperty referringProperty) { org.springframework.data.mongodb.core.mapping.DBRef annotation; @@ -794,6 +798,7 @@ DocumentPointer createDocumentPointer(Object source, @Nullable MongoPersisten * * @see org.springframework.data.mongodb.core.convert.MongoWriter#write(java.lang.Object, java.lang.Object) */ + @Override public void write(Object obj, Bson bson) { if (null == obj) { @@ -903,7 +908,10 @@ private void writeProperties(Bson bson, MongoPersistentEntity entity, Persist Object value = accessor.getProperty(prop); if (value == null) { - if (prop.writeNullValues()) { + + if (conversions.hasValueConverter(prop)) { + dbObjectAccessor.put(prop, applyPropertyConversion(null, prop, accessor)); + } else { dbObjectAccessor.put(prop, null); } } else if (!conversions.isSimpleType(value.getClass())) { @@ -929,6 +937,7 @@ private void writeAssociation(Association association, writePropertyInternal(value, dbObjectAccessor, inverseProp, accessor); } + // TODO: Documentaccessor is package-private @SuppressWarnings({ "unchecked" }) protected void writePropertyInternal(@Nullable Object obj, DocumentAccessor accessor, MongoPersistentProperty prop, PersistentPropertyAccessor persistentPropertyAccessor) { @@ -941,14 +950,7 @@ protected void writePropertyInternal(@Nullable Object obj, DocumentAccessor acce TypeInformation type = prop.getTypeInformation(); if (conversions.hasValueConverter(prop)) { - accessor.put(prop, conversions.getPropertyValueConversions().getValueConverter(prop).write(obj, - new MongoConversionContext(new PropertyValueProvider<>() { - @Nullable - @Override - public T getPropertyValue(MongoPersistentProperty property) { - return (T) persistentPropertyAccessor.getProperty(property); - } - }, prop, this, spELContext))); + accessor.put(prop, applyPropertyConversion(obj, prop, persistentPropertyAccessor)); return; } @@ -987,6 +989,11 @@ public T getPropertyValue(MongoPersistentProperty property) { dbRefObj = proxy.toDBRef(); } + if (obj != null && conversions.hasCustomWriteTarget(obj.getClass())) { + accessor.put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get())); + return; + } + dbRefObj = dbRefObj != null ? dbRefObj : createDBRef(obj, prop); accessor.put(prop, dbRefObj); @@ -1284,17 +1291,11 @@ private void writeSimpleInternal(@Nullable Object value, Bson bson, String key) private void writeSimpleInternal(@Nullable Object value, Bson bson, MongoPersistentProperty property, PersistentPropertyAccessor persistentPropertyAccessor) { + DocumentAccessor accessor = new DocumentAccessor(bson); if (conversions.hasValueConverter(property)) { - accessor.put(property, conversions.getPropertyValueConversions().getValueConverter(property).write(value, - new MongoConversionContext(new PropertyValueProvider<>() { - @Nullable - @Override - public T getPropertyValue(MongoPersistentProperty property) { - return (T) persistentPropertyAccessor.getProperty(property); - } - }, property, this, spELContext))); + accessor.put(property, applyPropertyConversion(value, property, persistentPropertyAccessor)); return; } @@ -1302,6 +1303,23 @@ public T getPropertyValue(MongoPersistentProperty property) { property.hasExplicitWriteTarget() ? property.getFieldType() : Object.class)); } + @Nullable + @SuppressWarnings("unchecked") + private Object applyPropertyConversion(@Nullable Object value, MongoPersistentProperty property, + PersistentPropertyAccessor persistentPropertyAccessor) { + MongoConversionContext context = new MongoConversionContext(new PropertyValueProvider<>() { + + @Nullable + @Override + public T getPropertyValue(MongoPersistentProperty property) { + return (T) persistentPropertyAccessor.getProperty(property); + } + }, property, this, spELContext); + PropertyValueConverter> valueConverter = conversions + .getPropertyValueConversions().getValueConverter(property); + return value != null ? valueConverter.write(value, context) : valueConverter.writeNull(context); + } + /** * Checks whether we have a custom conversion registered for the given value into an arbitrary simple Mongo type. * Returns the converted value if so. If not, we perform special enum handling or simply return the value as is. @@ -1597,7 +1615,7 @@ public Object convertToMongoType(@Nullable Object obj, @Nullable TypeInformation } @Override - public Object convertToMongoType(@Nullable Object obj, MongoPersistentEntity entity) { + public Object convertToMongoType(@Nullable Object obj, MongoPersistentEntity entity) { Document newDocument = new Document(); writeInternal(obj, newDocument, entity); return newDocument; @@ -1935,6 +1953,7 @@ static class MongoDbPropertyValueProvider implements PropertyValueProvider T getPropertyValue(MongoPersistentProperty property) { @@ -1942,14 +1961,18 @@ public T getPropertyValue(MongoPersistentProperty property) { String expression = property.getSpelExpression(); Object value = expression != null ? evaluator.evaluate(expression) : accessor.get(property); - if (value == null) { - return null; - } - CustomConversions conversions = context.getCustomConversions(); if (conversions.hasValueConverter(property)) { - return (T) conversions.getPropertyValueConversions().getValueConverter(property).read(value, - new MongoConversionContext(this, property, context.getSourceConverter(), spELContext)); + MongoConversionContext conversionContext = new MongoConversionContext(this, property, + context.getSourceConverter(), spELContext); + PropertyValueConverter> valueConverter = conversions + .getPropertyValueConversions().getValueConverter(property); + return (T) (value != null ? valueConverter.read(value, conversionContext) + : valueConverter.readNull(conversionContext)); + } + + if (value == null) { + return null; } ConversionContext contextToUse = context.forProperty(property); @@ -2244,6 +2267,7 @@ default S findContextualEntity(MongoPersistentEntity entity, Document doc return null; } + // TODO: ObjectPath is package-private ObjectPath getPath(); CustomConversions getCustomConversions(); @@ -2404,6 +2428,7 @@ public ConversionContext withPath(ObjectPath currentPath) { collectionConverter, mapConverter, dbRefConverter, elementConverter); } + // TODO: ObjectPath is package-private @Override public ObjectPath getPath() { return path; 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 32c7746f94..f74241eabe 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 @@ -63,6 +63,7 @@ import org.springframework.data.annotation.PersistenceConstructor; import org.springframework.data.annotation.Transient; import org.springframework.data.annotation.TypeAlias; +import org.springframework.data.convert.ConverterBuilder; import org.springframework.data.convert.CustomConversions; import org.springframework.data.convert.PropertyValueConverter; import org.springframework.data.convert.PropertyValueConverterFactory; @@ -2693,9 +2694,134 @@ void shouldWriteNullPropertyCorrectly() { converter.write(fieldWrite, document); assertThat(document).containsEntry("writeAlways", null).doesNotContainKey("writeNonNull"); + assertThat(document).containsEntry("writeAlwaysPersonDBRef", null).doesNotContainKey("writeNonNullPersonDBRef"); + } + + @Test // GH-4710 + void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() { + + MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder + .writing(Integer.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList()); + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(conversions); + converter.afterPropertiesSet(); + + WithFieldWrite fieldWrite = new WithFieldWrite(); + fieldWrite.writeAlways = 10; + fieldWrite.writeNonNull = 20; + + org.bson.Document document = new org.bson.Document(); + converter.write(fieldWrite, document); + + assertThat(document).containsEntry("writeAlways", null).doesNotContainKey("writeNonNull"); + } + + @Test // GH-4710 + void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() { + + MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder + .writing(Person.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList()); + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(conversions); + converter.afterPropertiesSet(); + + WithFieldWrite fieldWrite = new WithFieldWrite(); + fieldWrite.writeAlwaysPerson = new Person(); + fieldWrite.writeNonNullPerson = new Person(); + + org.bson.Document document = new org.bson.Document(); + converter.write(fieldWrite, document); + assertThat(document).containsEntry("writeAlwaysPerson", null).doesNotContainKey("writeNonNullPerson"); } + @Test // GH-4710 + void shouldDelegateWriteOfDBRefToCustomConversionIfConfigured() { + + MongoCustomConversions conversions = new MongoCustomConversions( + ConverterBuilder.writing(Person.class, DBRef.class, it -> new DBRef("persons", "n/a")).andReading(it -> null) + .getConverters().stream().toList()); + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(conversions); + converter.afterPropertiesSet(); + + WithFieldWrite fieldWrite = new WithFieldWrite(); + fieldWrite.writeAlwaysPersonDBRef = new Person(); + fieldWrite.writeNonNullPersonDBRef = new Person(); + + org.bson.Document document = new org.bson.Document(); + converter.write(fieldWrite, document); + + assertThat(document).containsEntry("writeAlwaysPersonDBRef", new DBRef("persons", "n/a"));// .doesNotContainKey("writeNonNullPersonDBRef"); + } + + @Test // GH-4710 + void shouldDelegateWriteOfDBRefToCustomConversionIfConfiguredAndCheckNulls() { + + MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder + .writing(Person.class, DBRef.class, it -> null).andReading(it -> null).getConverters().stream().toList()); + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(conversions); + converter.afterPropertiesSet(); + + WithFieldWrite fieldWrite = new WithFieldWrite(); + fieldWrite.writeAlwaysPersonDBRef = new Person(); + fieldWrite.writeNonNullPersonDBRef = new Person(); + + org.bson.Document document = new org.bson.Document(); + converter.write(fieldWrite, document); + + assertThat(document).containsEntry("writeAlwaysPersonDBRef", null).doesNotContainKey("writeNonNullPersonDBRef"); + } + + @Test // GH-4710 + void shouldApplyNullConversionToPropertyValueConverters() { + + MongoCustomConversions conversions = new MongoCustomConversions( + MongoCustomConversions.MongoConverterConfigurationAdapter.from(Collections.emptyList()) + .configurePropertyConversions(registrar -> { + registrar.registerConverter(Person.class, "firstname", new MongoValueConverter() { + @Override + public String readNull(MongoConversionContext context) { + return "NULL"; + } + + @Override + public String writeNull(MongoConversionContext context) { + return "NULL"; + } + + @Override + public String read(String value, MongoConversionContext context) { + return ""; + } + + @Override + public String write(String value, MongoConversionContext context) { + return ""; + } + }); + })); + + converter = new MappingMongoConverter(resolver, mappingContext); + converter.setCustomConversions(conversions); + converter.afterPropertiesSet(); + + org.bson.Document document = new org.bson.Document(); + converter.write(new Person(), document); + + assertThat(document).containsEntry("foo", "NULL"); + + document = new org.bson.Document("foo", null); + Person result = converter.read(Person.class, document); + + assertThat(result.firstname).isEqualTo("NULL"); + } + @Test // GH-3686 void readsCollectionContainingNullValue() { @@ -3009,7 +3135,7 @@ void beanConverter() { })); converter.afterPropertiesSet(); - WithValueConverters wvc = new WithValueConverters(); + WithContextValueConverters wvc = new WithContextValueConverters(); wvc.converterBean = "spring"; org.bson.Document target = new org.bson.Document(); @@ -3020,7 +3146,7 @@ void beanConverter() { assertThat((String) it.get("ooo")).startsWith("spring - "); }); - WithValueConverters read = converter.read(WithValueConverters.class, target); + WithContextValueConverters read = converter.read(WithContextValueConverters.class, target); assertThat(read.converterBean).startsWith("spring -"); } @@ -4102,14 +4228,20 @@ static class WithFieldWrite { @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways; - @org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson; - @org.springframework.data.mongodb.core.mapping.DBRef @org.springframework.data.mongodb.core.mapping.Field( write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson; + @org.springframework.data.mongodb.core.mapping.DBRef + @org.springframework.data.mongodb.core.mapping.Field( + write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPersonDBRef; + + @org.springframework.data.mongodb.core.mapping.DBRef + @org.springframework.data.mongodb.core.mapping.Field( + write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPersonDBRef; + } static class WithValueConverters { @@ -4118,6 +4250,11 @@ static class WithValueConverters { @ValueConverter(Converter2.class) String converterEnum; + String viaRegisteredConverter; + } + + static class WithContextValueConverters { + @ValueConverter(Converter3.class) String converterBean; String viaRegisteredConverter;