From 6c85bb39a34e1eacaddb833cc411bf70b58206e8 Mon Sep 17 00:00:00 2001 From: Christoph Strobl Date: Wed, 23 Jul 2014 14:48:03 +0200 Subject: [PATCH] DATAMONGO-1001 - Fix saving lazy loaded object. We now resolve the target type for CGLib-proxied objects and initialize lazy loaded ones before saving. As it turns out CustomConversions already knows how to deal with proxies correctly. Ee added an explicit test to assert that. Original pull request: #208. --- .../core/convert/MappingMongoConverter.java | 26 +++++---- .../data/mongodb/core/MongoTemplateTests.java | 36 ++++++++++++ .../convert/CustomConversionsUnitTests.java | 57 +++++++++++++++++++ .../MappingMongoConverterUnitTests.java | 32 +++++++++++ 4 files changed, 141 insertions(+), 10 deletions(-) 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 b81f19206..ad2b32d76 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 @@ -56,6 +56,7 @@ import org.springframework.data.util.ClassTypeInformation; import org.springframework.data.util.TypeInformation; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.util.Assert; +import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import com.mongodb.BasicDBList; @@ -315,14 +316,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return; } - boolean handledByCustomConverter = conversions.getCustomWriteTarget(obj.getClass(), DBObject.class) != null; - TypeInformation type = ClassTypeInformation.from(obj.getClass()); + Class entityType = obj.getClass(); + boolean handledByCustomConverter = conversions.getCustomWriteTarget(entityType, DBObject.class) != null; + TypeInformation type = ClassTypeInformation.from(entityType); if (!handledByCustomConverter && !(dbo instanceof BasicDBList)) { typeMapper.writeType(type, dbo); } - writeInternal(obj, dbo, type); + Object target = obj instanceof LazyLoadingProxy ? ((LazyLoadingProxy) obj).getTarget() : obj; + + writeInternal(target, dbo, type); } /** @@ -338,7 +342,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return; } - Class customTarget = conversions.getCustomWriteTarget(obj.getClass(), DBObject.class); + Class entityType = obj.getClass(); + Class customTarget = conversions.getCustomWriteTarget(entityType, DBObject.class); if (customTarget != null) { DBObject result = conversionService.convert(obj, DBObject.class); @@ -346,17 +351,17 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return; } - if (Map.class.isAssignableFrom(obj.getClass())) { + if (Map.class.isAssignableFrom(entityType)) { writeMapInternal((Map) obj, dbo, ClassTypeInformation.MAP); return; } - if (Collection.class.isAssignableFrom(obj.getClass())) { + if (Collection.class.isAssignableFrom(entityType)) { writeCollectionInternal((Collection) obj, ClassTypeInformation.LIST, (BasicDBList) dbo); return; } - MongoPersistentEntity entity = mappingContext.getPersistentEntity(obj.getClass()); + MongoPersistentEntity entity = mappingContext.getPersistentEntity(entityType); writeInternal(obj, dbo, entity); addCustomTypeKeyIfNecessary(typeHint, obj, dbo); } @@ -463,7 +468,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * If we have a LazyLoadingProxy we make sure it is initialized first. */ if (obj instanceof LazyLoadingProxy) { - obj = ((LazyLoadingProxy) obj).initialize(); + obj = ((LazyLoadingProxy) obj).getTarget(); } // Lookup potential custom target type @@ -683,10 +688,11 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App TypeInformation actualType = type != null ? type.getActualType() : null; Class reference = actualType == null ? Object.class : actualType.getType(); + Class valueType = ClassUtils.getUserClass(value.getClass()); - boolean notTheSameClass = !value.getClass().equals(reference); + boolean notTheSameClass = !valueType.equals(reference); if (notTheSameClass) { - typeMapper.writeType(value.getClass(), dbObject); + typeMapper.writeType(valueType, dbObject); } } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java index 44ef093d2..f937da253 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/MongoTemplateTests.java @@ -2511,6 +2511,37 @@ public class MongoTemplateTests { assertThat(template.getDb().getCollection("sample").find(new BasicDBObject("field", "data")).count(), is(1)); } + /** + * @see DATAMONGO-1001 + */ + @Test + public void shouldAllowSavingOfLazyLoadedDbRefs() { + + template.dropCollection(SomeTemplate.class); + template.dropCollection(SomeMessage.class); + template.dropCollection(SomeContent.class); + + SomeContent content = new SomeContent(); + content.id = "content-1"; + content.text = "spring"; + template.save(content); + + SomeTemplate tmpl = new SomeTemplate(); + tmpl.id = "template-1"; + tmpl.content = content; // @DBRef(lazy=true) tmpl.content + + template.save(tmpl); + + SomeTemplate savedTmpl = template.findById(tmpl.id, SomeTemplate.class); + + SomeContent loadedContent = savedTmpl.getContent(); + loadedContent.setText("data"); + template.save(loadedContent); + + assertThat(template.findById(content.id, SomeContent.class).getText(), is("data")); + + } + /** * @see DATAMONGO-880 */ @@ -2956,6 +2987,11 @@ public class MongoTemplateTests { return name; } + public void setText(String text) { + this.text = text; + + } + public String getId() { return id; } diff --git a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java index e75094444..e43160df5 100644 --- a/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java +++ b/spring-data-mongodb/src/test/java/org/springframework/data/mongodb/core/convert/CustomConversionsUnitTests.java @@ -1,3 +1,18 @@ +/* + * Copyright 2011-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.springframework.data.mongodb.core.convert; import static org.hamcrest.Matchers.*; @@ -15,6 +30,7 @@ import org.bson.types.Binary; import org.bson.types.ObjectId; import org.joda.time.DateTime; import org.junit.Test; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.core.convert.converter.Converter; import org.springframework.core.convert.support.DefaultConversionService; import org.springframework.core.convert.support.GenericConversionService; @@ -26,6 +42,7 @@ import com.mongodb.DBRef; * Unit tests for {@link CustomConversions}. * * @author Oliver Gierke + * @auhtor Christoph Strobl */ public class CustomConversionsUnitTests { @@ -197,6 +214,35 @@ public class CustomConversionsUnitTests { assertThat(conversionService.convert(new DateTime(), Date.class), is(new Date(0))); } + /** + * @see DATAMONGO-1001 + */ + @Test + public void shouldSelectPropertCustomWriteTargetForCglibProxiedType() { + + CustomConversions conversions = new CustomConversions(Arrays.asList(FormatToStringConverter.INSTANCE)); + assertThat(conversions.getCustomWriteTarget(createProxyTypeFor(Format.class)), is(typeCompatibleWith(String.class))); + } + + /** + * @see DATAMONGO-1001 + */ + @Test + public void shouldSelectPropertCustomReadTargetForCglibProxiedType() { + + CustomConversions conversions = new CustomConversions(Arrays.asList(CustomObjectToStringConverter.INSTANCE)); + assertThat(conversions.hasCustomReadTarget(createProxyTypeFor(Object.class), String.class), is(true)); + } + + private static Class createProxyTypeFor(Class type) { + + ProxyFactory factory = new ProxyFactory(); + factory.setProxyTargetClass(true); + factory.setTargetClass(type); + + return factory.getProxy().getClass(); + } + enum FormatToStringConverter implements Converter { INSTANCE; @@ -251,4 +297,15 @@ public class CustomConversionsUnitTests { return new Date(0); } } + + enum CustomObjectToStringConverter implements Converter { + + INSTANCE; + + @Override + public String convert(Object source) { + return source != null ? source.toString() : null; + } + + } } 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 aff93e606..b7b973afe 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 @@ -49,6 +49,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.context.ApplicationContext; import org.springframework.core.convert.converter.Converter; @@ -1834,6 +1835,37 @@ public class MappingMongoConverterUnitTests { assertThat(entity.score, equalTo(5F)); } + /** + * @see DATAMONGO-1001 + */ + @Test + public void shouldWriteCglibProxiedClassTypeInformationCorrectly() { + + ProxyFactory factory = new ProxyFactory(); + factory.setTargetClass(GenericType.class); + factory.setProxyTargetClass(true); + + GenericType proxied = (GenericType) factory.getProxy(); + BasicDBObject dbo = new BasicDBObject(); + converter.write(proxied, dbo); + + assertThat(dbo.get("_class"), is((Object) GenericType.class.getName())); + } + + /** + * @see DATAMONGO-1001 + */ + @Test + public void shouldUseTargetObjectOfLazyLoadingProxyWhenWriting() { + + LazyLoadingProxy mock = mock(LazyLoadingProxy.class); + + BasicDBObject dbo = new BasicDBObject(); + converter.write(mock, dbo); + + verify(mock, times(1)).getTarget(); + } + static class GenericType { T content; }