Skip to content

Commit

Permalink
Refactor DocumentAccessor, consider readNull/writeNull for null values.
Browse files Browse the repository at this point in the history 
Original pull request: #4728
See #4710
  • Loading branch information
@mp911de
mp911de committed Aug 28, 2024
1 parent 791f8b2 commit bc59095
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 56 deletions.
20 changes: 4 additions & 16 deletions ...mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> parts = Arrays.asList(prop.getMongoField().getName().parts()).iterator();
Bson document = this.document;

Expand Down Expand Up @@ -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);
}
}
};

}
}
73 changes: 42 additions & 31 deletions ...db/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())) {
Expand Down Expand Up @@ -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> T getPropertyValue(MongoPersistentProperty property) {
return (T) persistentPropertyAccessor.getProperty(property);
}
}, prop, this, spELContext)));
accessor.put(prop, applyPropertyConversion(obj, prop, persistentPropertyAccessor));
return;
}

Expand Down Expand Up @@ -987,8 +984,8 @@ public <T> 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;
}

Expand Down Expand Up @@ -1290,24 +1287,34 @@ 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> T getPropertyValue(MongoPersistentProperty property) {
return (T) persistentPropertyAccessor.getProperty(property);
}
}, property, this, spELContext)));
accessor.put(property, applyPropertyConversion(value, property, persistentPropertyAccessor));
return;
}

accessor.put(property, getPotentiallyConvertedSimpleWrite(value,
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> T getPropertyValue(MongoPersistentProperty property) {
return (T) persistentPropertyAccessor.getProperty(property);
}
}, property, this, spELContext);
PropertyValueConverter<Object, Object, ValueConversionContext<MongoPersistentProperty>> 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.
Expand Down Expand Up @@ -1948,14 +1955,18 @@ public <T> 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<Object, Object, ValueConversionContext<MongoPersistentProperty>> 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);
Expand Down
72 changes: 63 additions & 9 deletions ...st/java/org/springframework/data/mongodb/core/convert/MappingMongoConverterUnitTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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<String, String>() {
@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() {

Expand Down Expand Up @@ -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();
Expand All @@ -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 -");
}

Expand Down Expand Up @@ -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(
Expand All @@ -4201,6 +4250,11 @@ static class WithValueConverters {

@ValueConverter(Converter2.class) String converterEnum;

String viaRegisteredConverter;
}

static class WithContextValueConverters {

@ValueConverter(Converter3.class) String converterBean;

String viaRegisteredConverter;
Expand Down

0 comments on commit bc59095

Please sign in to comment.