Skip to content

Commit 434e553

Browse files
committed
DATAMONGO-1250 - Fixed accidental duplicate invocation of value conversion in UpdateMapper.
UpdateMapper.getMappedObjectForField(…) invokes the very same method of the super class but handed in an already mapped value so that value conversion was invoked twice. This was especially problematic in cases a dedicated converter had been registered for an object that is already a Mongo-storable one (e.g. an enum-to-string converter and back) without indicating which of the tow converter is the reading or the writing one. This basically caused the source value converted back and forth during the update mapping creating the impression the value wasn't converted at all. This is now fixed by removing the superfluous mapping.
1 parent de5b5ee commit 434e553

File tree

2 files changed

+78
-1
lines changed

2 files changed

+78
-1
lines changed

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/UpdateMapper.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ protected Entry<String, Object> getMappedObjectForField(Field field, Object rawV
9090
return getMappedUpdateModifier(field, rawValue);
9191
}
9292

93-
return super.getMappedObjectForField(field, getMappedValue(field, rawValue));
93+
return super.getMappedObjectForField(field, rawValue);
9494
}
9595

9696
private Entry<String, Object> getMappedUpdateModifier(Field field, Object rawValue) {

spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/UpdateMapperUnitTests.java

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@
4242
import org.springframework.data.mapping.model.MappingException;
4343
import org.springframework.data.mongodb.MongoDbFactory;
4444
import org.springframework.data.mongodb.core.DBObjectTestUtils;
45+
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.Allocation;
46+
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.AllocationToStringConverter;
47+
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.StringToAllocationConverter;
4548
import org.springframework.data.mongodb.core.mapping.Field;
4649
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
4750
import org.springframework.data.mongodb.core.query.Criteria;
@@ -762,6 +765,33 @@ public void mappingShouldNotContainTypeInformationWhenValueTypeOfMapMatchesDecla
762765
assertThat(mappedUpdate, isBsonObject().notContaining("$set.concreteMap.jasnah._class"));
763766
}
764767

768+
/**
769+
* @see DATAMONGO-1250
770+
*/
771+
@Test
772+
@SuppressWarnings("unchecked")
773+
public void mapsUpdateWithBothReadingAndWritingConverterRegistered() {
774+
775+
CustomConversions conversions = new CustomConversions(
776+
Arrays.asList(AllocationToStringConverter.INSTANCE, StringToAllocationConverter.INSTANCE));
777+
778+
MongoMappingContext mappingContext = new MongoMappingContext();
779+
mappingContext.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
780+
mappingContext.afterPropertiesSet();
781+
782+
MappingMongoConverter converter = new MappingMongoConverter(mock(DbRefResolver.class), mappingContext);
783+
converter.setCustomConversions(conversions);
784+
converter.afterPropertiesSet();
785+
786+
UpdateMapper mapper = new UpdateMapper(converter);
787+
788+
Update update = new Update().set("allocation", Allocation.AVAILABLE);
789+
DBObject result = mapper.getMappedObject(update.getUpdateObject(),
790+
mappingContext.getPersistentEntity(ClassWithEnum.class));
791+
792+
assertThat(result, isBsonObject().containing("$set.allocation", Allocation.AVAILABLE.code));
793+
}
794+
765795
static class DomainTypeWrappingConcreteyTypeHavingListOfInterfaceTypeAttributes {
766796
ListModelWrapper concreteTypeWithListAttributeOfInterfaceType;
767797
}
@@ -984,4 +1014,51 @@ static class EntityWithObjectMap {
9841014
Map<Object, Object> map;
9851015
Map<Object, NestedDocument> concreteMap;
9861016
}
1017+
1018+
static class ClassWithEnum {
1019+
1020+
Allocation allocation;
1021+
1022+
static enum Allocation {
1023+
1024+
AVAILABLE("V"), ALLOCATED("A");
1025+
1026+
String code;
1027+
1028+
private Allocation(String code) {
1029+
this.code = code;
1030+
}
1031+
1032+
public static Allocation of(String code) {
1033+
1034+
for (Allocation value : values()) {
1035+
if (value.code.equals(code)) {
1036+
return value;
1037+
}
1038+
}
1039+
1040+
throw new IllegalArgumentException();
1041+
}
1042+
}
1043+
1044+
static enum AllocationToStringConverter implements Converter<Allocation, String> {
1045+
1046+
INSTANCE;
1047+
1048+
@Override
1049+
public String convert(Allocation source) {
1050+
return source.code;
1051+
}
1052+
}
1053+
1054+
static enum StringToAllocationConverter implements Converter<String, Allocation> {
1055+
1056+
INSTANCE;
1057+
1058+
@Override
1059+
public Allocation convert(String source) {
1060+
return Allocation.of(source);
1061+
}
1062+
}
1063+
}
9871064
}

0 commit comments

Comments
 (0)