From 680d49ca408b36849dfa083fba85152f2a7dcb06 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 17 Jun 2024 11:36:42 +0200 Subject: [PATCH 1/5] Prepare issue branch. --- pom.xml | 2 +- spring-data-mongodb-benchmarks/pom.xml | 2 +- spring-data-mongodb-distribution/pom.xml | 2 +- spring-data-mongodb/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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 From 8c1d7428311f5c458ea0ad93a78de6e905a3a9f8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 17 Jun 2024 15:33:37 +0200 Subject: [PATCH 2/5] Consider null value settings for types with custom conversion. This commit fixes an issue where settings regarding storage of null values had been ignored if a custom converter took care of the conversion. --- .../core/convert/DocumentAccessor.java | 17 +++++++ .../core/convert/MappingMongoConverter.java | 3 +- .../MappingMongoConverterUnitTests.java | 49 ++++++++++++++++++- 3 files changed, 66 insertions(+), 3 deletions(-) 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..0595afb2ce 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 @@ -172,4 +172,21 @@ private static Document getOrCreateNestedDocument(String key, Bson source) { return nested; } + + DocumentAccessor withCheckFieldMapping(boolean checkFieldMapping) { + + if(!checkFieldMapping) { + return this; + } + + return new DocumentAccessor(this.document) { + @Override + public void put(MongoPersistentProperty prop, @Nullable Object value) { + if(value != null || prop.writeNullValues()) { + super.put(prop, value); + } + } + }; + + } } 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..3ed8df42af 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 @@ -1284,7 +1284,8 @@ 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); + + DocumentAccessor accessor = new DocumentAccessor(bson).withCheckFieldMapping(true); if (conversions.hasValueConverter(property)) { accessor.put(property, conversions.getPropertyValueConversions().getValueConverter(property).write(value, 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..64ed54f8b0 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,6 +2694,44 @@ 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"); } @@ -4102,13 +4141,19 @@ 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.Field( + write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson; + + @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 writeNonNullPerson; + 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 writeAlwaysPerson; + write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPersonDBRef; } From 3d4dfe933060e4679a75d0e7dcc8abd3d8927948 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Mon, 17 Jun 2024 16:01:04 +0200 Subject: [PATCH 3/5] think about it --- .../core/convert/MappingMongoConverter.java | 5 +++ .../MappingMongoConverterUnitTests.java | 38 +++++++++++++++++++ 2 files changed, 43 insertions(+) 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 3ed8df42af..8e3c18815c 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 @@ -987,6 +987,11 @@ public T getPropertyValue(MongoPersistentProperty property) { dbRefObj = proxy.toDBRef(); } + if(obj !=null && conversions.hasCustomWriteTarget(obj.getClass())) { + accessor.withCheckFieldMapping(true).put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get())); + return; + } + dbRefObj = dbRefObj != null ? dbRefObj : createDBRef(obj, prop); accessor.put(prop, dbRefObj); 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 64ed54f8b0..3ccdb54a12 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 @@ -2735,6 +2735,44 @@ void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() { 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-3686 void readsCollectionContainingNullValue() { From cb6fadf61c3514c04a127800c0ac13ffedfe8053 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 28 Aug 2024 08:56:50 +0200 Subject: [PATCH 4/5] Refactor DocumentAccessor, consider readNull/writeNull for null values. --- .../core/convert/DocumentAccessor.java | 20 +---- .../core/convert/MappingMongoConverter.java | 73 +++++++++++-------- .../MappingMongoConverterUnitTests.java | 72 +++++++++++++++--- 3 files changed, 109 insertions(+), 56 deletions(-) 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 0595afb2ce..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; @@ -173,20 +177,4 @@ private static Document getOrCreateNestedDocument(String key, Bson source) { return nested; } - DocumentAccessor withCheckFieldMapping(boolean checkFieldMapping) { - - if(!checkFieldMapping) { - return this; - } - - return new DocumentAccessor(this.document) { - @Override - public void put(MongoPersistentProperty prop, @Nullable Object value) { - if(value != null || prop.writeNullValues()) { - super.put(prop, value); - } - } - }; - - } } 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 8e3c18815c..ee7dbc1734 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); @@ -903,7 +904,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())) { @@ -941,14 +945,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,8 +984,8 @@ public T getPropertyValue(MongoPersistentProperty property) { dbRefObj = proxy.toDBRef(); } - if(obj !=null && conversions.hasCustomWriteTarget(obj.getClass())) { - accessor.withCheckFieldMapping(true).put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get())); + if (obj != null && conversions.hasCustomWriteTarget(obj.getClass())) { + accessor.put(prop, doConvert(obj, conversions.getCustomWriteTarget(obj.getClass()).get())); return; } @@ -1290,17 +1287,10 @@ 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).withCheckFieldMapping(true); + 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; } @@ -1308,6 +1298,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. @@ -1948,14 +1955,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); 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 3ccdb54a12..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 @@ -2700,7 +2700,8 @@ void shouldWriteNullPropertyCorrectly() { @Test // GH-4710 void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() { - MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Integer.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList()); + 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); @@ -2719,7 +2720,8 @@ void shouldWriteSimplePropertyCorrectlyAfterConversionReturnsNull() { @Test // GH-4710 void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() { - MongoCustomConversions conversions = new MongoCustomConversions(ConverterBuilder.writing(Person.class, String.class, it -> null).andReading(it -> null).getConverters().stream().toList()); + 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); @@ -2738,7 +2740,9 @@ void shouldWriteComplexPropertyCorrectlyAfterConversionReturnsNull() { @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()); + 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); @@ -2751,13 +2755,14 @@ void shouldDelegateWriteOfDBRefToCustomConversionIfConfigured() { org.bson.Document document = new org.bson.Document(); converter.write(fieldWrite, document); - assertThat(document).containsEntry("writeAlwaysPersonDBRef", new DBRef("persons", "n/a"));//.doesNotContainKey("writeNonNullPersonDBRef"); + 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()); + 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); @@ -2773,6 +2778,50 @@ void shouldDelegateWriteOfDBRefToCustomConversionIfConfiguredAndCheckNulls() { 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() { @@ -3086,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(); @@ -3097,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 -"); } @@ -4180,10 +4229,10 @@ static class WithFieldWrite { write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Integer writeAlways; @org.springframework.data.mongodb.core.mapping.Field( - write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson; + write = org.springframework.data.mongodb.core.mapping.Field.Write.NON_NULL) Person writeNonNullPerson; @org.springframework.data.mongodb.core.mapping.Field( - write = org.springframework.data.mongodb.core.mapping.Field.Write.ALWAYS) Person writeAlwaysPerson; + 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( @@ -4201,6 +4250,11 @@ static class WithValueConverters { @ValueConverter(Converter2.class) String converterEnum; + String viaRegisteredConverter; + } + + static class WithContextValueConverters { + @ValueConverter(Converter3.class) String converterBean; String viaRegisteredConverter; From 67b1c759c2efa5c622aa9afe1bb2ca1bb9736623 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 28 Aug 2024 08:59:26 +0200 Subject: [PATCH 5/5] Polishing. --- .../mongodb/core/convert/MappingMongoConverter.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) 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 ee7dbc1734..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 @@ -280,6 +280,7 @@ public void setCodecRegistryProvider(@Nullable CodecRegistryProvider codecRegist this.codecRegistryProvider = codecRegistryProvider; } + @Override public MappingContext, MongoPersistentProperty> getMappingContext() { return mappingContext; } @@ -432,6 +433,7 @@ public Map getBean() { } } + @Override public S read(Class clazz, Bson bson) { return read(TypeInformation.of(clazz), bson); } @@ -729,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; @@ -795,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) { @@ -933,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) { @@ -1610,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; @@ -1948,6 +1953,7 @@ static class MongoDbPropertyValueProvider implements PropertyValueProvider T getPropertyValue(MongoPersistentProperty property) { @@ -2261,6 +2267,7 @@ default S findContextualEntity(MongoPersistentEntity entity, Document doc return null; } + // TODO: ObjectPath is package-private ObjectPath getPath(); CustomConversions getCustomConversions(); @@ -2421,6 +2428,7 @@ public ConversionContext withPath(ObjectPath currentPath) { collectionConverter, mapConverter, dbRefConverter, elementConverter); } + // TODO: ObjectPath is package-private @Override public ObjectPath getPath() { return path;