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.
This commit is contained in:
Oliver Gierke
2015-07-04 14:12:35 +02:00
parent ab4fb8477d
commit 234009cf53
2 changed files with 78 additions and 1 deletions

View File

@@ -90,7 +90,7 @@ public class UpdateMapper extends QueryMapper {
return getMappedUpdateModifier(field, rawValue);
}
return super.getMappedObjectForField(field, getMappedValue(field, rawValue));
return super.getMappedObjectForField(field, rawValue);
}
private Entry<String, Object> getMappedUpdateModifier(Field field, Object rawValue) {

View File

@@ -42,6 +42,9 @@ import org.springframework.data.convert.WritingConverter;
import org.springframework.data.mapping.model.MappingException;
import org.springframework.data.mongodb.MongoDbFactory;
import org.springframework.data.mongodb.core.DBObjectTestUtils;
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.Allocation;
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.AllocationToStringConverter;
import org.springframework.data.mongodb.core.convert.UpdateMapperUnitTests.ClassWithEnum.StringToAllocationConverter;
import org.springframework.data.mongodb.core.mapping.Field;
import org.springframework.data.mongodb.core.mapping.MongoMappingContext;
import org.springframework.data.mongodb.core.query.Criteria;
@@ -762,6 +765,33 @@ public class UpdateMapperUnitTests {
assertThat(mappedUpdate, isBsonObject().notContaining("$set.concreteMap.jasnah._class"));
}
/**
* @see DATAMONGO-1250
*/
@Test
@SuppressWarnings("unchecked")
public void mapsUpdateWithBothReadingAndWritingConverterRegistered() {
CustomConversions conversions = new CustomConversions(
Arrays.asList(AllocationToStringConverter.INSTANCE, StringToAllocationConverter.INSTANCE));
MongoMappingContext mappingContext = new MongoMappingContext();
mappingContext.setSimpleTypeHolder(conversions.getSimpleTypeHolder());
mappingContext.afterPropertiesSet();
MappingMongoConverter converter = new MappingMongoConverter(mock(DbRefResolver.class), mappingContext);
converter.setCustomConversions(conversions);
converter.afterPropertiesSet();
UpdateMapper mapper = new UpdateMapper(converter);
Update update = new Update().set("allocation", Allocation.AVAILABLE);
DBObject result = mapper.getMappedObject(update.getUpdateObject(),
mappingContext.getPersistentEntity(ClassWithEnum.class));
assertThat(result, isBsonObject().containing("$set.allocation", Allocation.AVAILABLE.code));
}
static class DomainTypeWrappingConcreteyTypeHavingListOfInterfaceTypeAttributes {
ListModelWrapper concreteTypeWithListAttributeOfInterfaceType;
}
@@ -984,4 +1014,51 @@ public class UpdateMapperUnitTests {
Map<Object, Object> map;
Map<Object, NestedDocument> concreteMap;
}
static class ClassWithEnum {
Allocation allocation;
static enum Allocation {
AVAILABLE("V"), ALLOCATED("A");
String code;
private Allocation(String code) {
this.code = code;
}
public static Allocation of(String code) {
for (Allocation value : values()) {
if (value.code.equals(code)) {
return value;
}
}
throw new IllegalArgumentException();
}
}
static enum AllocationToStringConverter implements Converter<Allocation, String> {
INSTANCE;
@Override
public String convert(Allocation source) {
return source.code;
}
}
static enum StringToAllocationConverter implements Converter<String, Allocation> {
INSTANCE;
@Override
public Allocation convert(String source) {
return Allocation.of(source);
}
}
}
}