Browse files

Merge pull request #46 from Spikhalsky/issue-45

Fixed #45
  • Loading branch information...
2 parents fa39fd3 + 6214333 commit 6b53af5f1a168ca71a99a4783f15373704043f4f @buzdin buzdin committed Jan 3, 2013
View
3 core/src/main/java/org/dozer/MappedFieldsTracker.java
@@ -30,7 +30,8 @@
*/
public class MappedFieldsTracker {
- // Hash Code is ignored as it can serve application specific needs
+ // Hash Code is ignored as it can serve application specific needs
+ // <srcObject, <hashCodeOfDestination, mappedDestinationObject>>
private final Map<Object, Map<Integer,Object>> mappedFields = new IdentityHashMap<Object, Map<Integer,Object>>();
public void put(Object src, Object dest) {
View
14 core/src/main/java/org/dozer/MappingProcessor.java
@@ -147,10 +147,14 @@ public void map(final Object srcObj, final Object destObj, final String mapId) {
Class<?> converterClass = MappingUtils.findCustomConverter(converterByDestTypeCache, classMap.getCustomConverters(), srcObj
.getClass(), destType);
- // If this is a nested MapperAware conversion this mapping can be already processed
- Object alreadyMappedValue = mappedFields.getMappedValue(srcObj, destClass);
- if (alreadyMappedValue != null) {
- return (T) alreadyMappedValue;
+
+ if (destObj == null) {
+ // If this is a nested MapperAware conversion this mapping can be already processed
+ // but we can do this optimization only in case of no destObject, instead we must copy to the dest object
+ Object alreadyMappedValue = mappedFields.getMappedValue(srcObj, destType);
+ if (alreadyMappedValue != null) {
+ return (T) alreadyMappedValue;
+ }
}
if (converterClass != null) {
@@ -219,7 +223,7 @@ private void map(ClassMap classMap, Object srcObj, Object destObj, boolean bypas
// be referred to later to avoid recursive mapping loops
mappedFields.put(srcObj, destObj);
- // If class map hasnt already been determined, find the appropriate one for
+ // If class map hasn't already been determined, find the appropriate one for
// the src/dest object combination
if (classMap == null) {
classMap = getClassMap(srcObj.getClass(), destObj.getClass(), mapId);
View
265 core/src/test/java/org/dozer/MappingProcessorTest.java
@@ -1,127 +1,138 @@
-package org.dozer;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-
-import java.util.ArrayList;
-import java.util.List;
-
-
-import org.dozer.MappingProcessor;
-import org.junit.Before;
-import org.junit.Test;
-
-/**
- * @author Dmitry Buzdin
- */
-public class MappingProcessorTest extends AbstractDozerTest{
-
- private ArrayList<Object> sourceList;
- private ArrayList<Object> destinationList;
-
- @Before
- public void setUp() throws Exception {
- sourceList = new ArrayList<Object>();
- destinationList = new ArrayList<Object>();
- }
-
- @Test
- public void testPrepareDetinationList_OK() {
- List<?> result = MappingProcessor.prepareDestinationList(sourceList, destinationList);
- assertEquals(destinationList, result);
-
- destinationList.add("");
- result = MappingProcessor.prepareDestinationList(sourceList, destinationList);
- assertEquals(destinationList, result);
- }
-
- @Test
- public void testPrepareDetinationList_Null() {
- List<?> result = MappingProcessor.prepareDestinationList(sourceList, null);
- assertNotNull(result);
- assertEquals(new ArrayList<Object>(), result);
- }
-
- @Test
- public void testPrepareDetinationList_Array() {
- List<?> result = MappingProcessor.prepareDestinationList(sourceList, new Object[] { "A" });
- assertNotNull(result);
- assertEquals(1, result.size());
- assertEquals("A", result.iterator().next());
- }
-
- @Test
- public void testPrepareDetinationList_StrangeCase() {
- List<?> result = MappingProcessor.prepareDestinationList(sourceList, "Hullo");
- assertNotNull(result);
- assertEquals(new ArrayList<Object>(), result);
- }
-
- @Test
- public void testRemoveOrphans_OK() {
- destinationList.add("A");
- MappingProcessor.removeOrphans(sourceList, destinationList);
- assertTrue(destinationList.isEmpty());
- }
-
- @Test
- public void testRemoveOrphans_Many() {
- destinationList.add("A");
- destinationList.add("B");
- destinationList.add("C");
-
- sourceList.add("B");
- sourceList.add("D");
-
- MappingProcessor.removeOrphans(sourceList, destinationList);
- assertEquals(2, destinationList.size());
- assertEquals("B", destinationList.get(0));
- assertEquals("D", destinationList.get(1));
- }
-
- @Test
- public void testRemoveOrphans_Ordering() {
- destinationList.add(new Ordered(1));
- destinationList.add(new Ordered(2));
- destinationList.add(new Ordered(3));
-
- sourceList.add(new Ordered(0));
- sourceList.add(new Ordered(3));
- sourceList.add(new Ordered(2));
- sourceList.add(new Ordered(1));
-
- MappingProcessor.removeOrphans(sourceList, destinationList);
- assertEquals(4, destinationList.size());
- assertEquals(new Ordered(1), destinationList.get(0));
- assertEquals(new Ordered(2), destinationList.get(1));
- assertEquals(new Ordered(3), destinationList.get(2));
- assertEquals(new Ordered(0), destinationList.get(3));
- }
-
- private static class Ordered {
- private int id;
-
- private Ordered(int id) {
- this.id = id;
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o)
- return true;
- if (o == null || getClass() != o.getClass())
- return false;
- Ordered ordered = (Ordered) o;
- if (id != ordered.id)
- return false;
- return true;
- }
-
- @Override
- public int hashCode() {
- return id;
- }
- }
-
-}
+package org.dozer;
+
+import org.dozer.vo.A;
+import org.dozer.vo.B;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * @author Dmitry Spikhalskiy
+ * @since 03.01.13
+ */
+public class MappingProcessorTest extends AbstractDozerTest {
+ private ArrayList<Object> sourceList;
+ private ArrayList<Object> destinationList;
+
+ @Before
+ public void setUp() {
+ sourceList = new ArrayList<Object>();
+ destinationList = new ArrayList<Object>();
+ }
+
+ @Test
+ public void testTwiceObjectToObjectConvert() {
+ DozerBeanMapper mapper = new DozerBeanMapper();
+ Mapper mappingProcessor = mapper.getMappingProcessor();
+
+ A src = new A();
+ src.setB(new B());
+
+ A dest1 = new A();
+ mappingProcessor.map(src, dest1);
+ A dest2 = new A();
+ mappingProcessor.map(src, dest2);
+
+ assertSame(dest1.getB(), dest2.getB());
+ }
+
+ @Test
+ public void testPrepareDetinationList_OK() {
+ List<?> result = MappingProcessor.prepareDestinationList(sourceList, destinationList);
+ assertEquals(destinationList, result);
+
+ destinationList.add("");
+ result = MappingProcessor.prepareDestinationList(sourceList, destinationList);
+ assertEquals(destinationList, result);
+ }
+
+ @Test
+ public void testPrepareDetinationList_Null() {
+ List<?> result = MappingProcessor.prepareDestinationList(sourceList, null);
+ assertNotNull(result);
+ assertEquals(new ArrayList<Object>(), result);
+ }
+
+ @Test
+ public void testPrepareDetinationList_Array() {
+ List<?> result = MappingProcessor.prepareDestinationList(sourceList, new Object[] { "A" });
+ assertNotNull(result);
+ assertEquals(1, result.size());
+ assertEquals("A", result.iterator().next());
+ }
+
+ @Test
+ public void testPrepareDetinationList_StrangeCase() {
+ List<?> result = MappingProcessor.prepareDestinationList(sourceList, "Hullo");
+ assertNotNull(result);
+ assertEquals(new ArrayList<Object>(), result);
+ }
+
+ @Test
+ public void testRemoveOrphans_OK() {
+ destinationList.add("A");
+ MappingProcessor.removeOrphans(sourceList, destinationList);
+ assertTrue(destinationList.isEmpty());
+ }
+
+ @Test
+ public void testRemoveOrphans_Many() {
+ destinationList.add("A");
+ destinationList.add("B");
+ destinationList.add("C");
+
+ sourceList.add("B");
+ sourceList.add("D");
+
+ MappingProcessor.removeOrphans(sourceList, destinationList);
+ assertEquals(2, destinationList.size());
+ assertEquals("B", destinationList.get(0));
+ assertEquals("D", destinationList.get(1));
+ }
+
+ @Test
+ public void testRemoveOrphans_Ordering() {
+ destinationList.add(new Ordered(1));
+ destinationList.add(new Ordered(2));
+ destinationList.add(new Ordered(3));
+
+ sourceList.add(new Ordered(0));
+ sourceList.add(new Ordered(3));
+ sourceList.add(new Ordered(2));
+ sourceList.add(new Ordered(1));
+
+ MappingProcessor.removeOrphans(sourceList, destinationList);
+ assertEquals(4, destinationList.size());
+ assertEquals(new Ordered(1), destinationList.get(0));
+ assertEquals(new Ordered(2), destinationList.get(1));
+ assertEquals(new Ordered(3), destinationList.get(2));
+ assertEquals(new Ordered(0), destinationList.get(3));
+ }
+
+ private static class Ordered {
+ private int id;
+
+ private Ordered(int id) {
+ this.id = id;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o)
+ return true;
+ if (o == null || getClass() != o.getClass())
+ return false;
+ Ordered ordered = (Ordered) o;
+ if (id != ordered.id)
+ return false;
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ return id;
+ }
+ }
+}
View
1 core/src/test/java/org/dozer/functional_tests/mapperaware/CachingCustomConverterTest.java
@@ -89,6 +89,7 @@ public void setMapper(Mapper mapper) {
this.mapper = mapper;
}
+ @Override
public BidirectionalOneConvert convertFrom(BidirectionalOne arg0, BidirectionalOneConvert arg1) {
return mapper.map(arg0, BidirectionalOneConvert.class);
}
View
73 core/src/test/java/org/dozer/functional_tests/mapperaware/SecondUsingMapInternalTest.java
@@ -0,0 +1,73 @@
+package org.dozer.functional_tests.mapperaware;
+
+import org.dozer.CustomConverter;
+import org.dozer.DozerBeanMapper;
+import org.dozer.Mapper;
+import org.dozer.MapperAware;
+import org.dozer.vo.mapperaware.MapperAwareSimpleDest;
+import org.dozer.vo.mapperaware.MapperAwareSimpleInternal;
+import org.dozer.vo.mapperaware.MapperAwareSimpleSrc;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertNotNull;
+
+
+/**
+ * @author Dmitry Spikhalskiy
+ * @see <a href="https://github.com/DozerMapper/dozer/issues/45">issue</a>
+ */
+public class SecondUsingMapInternalTest {
+ DozerBeanMapper mapper;
+
+ @Before
+ public void setup() {
+ mapper = new DozerBeanMapper();
+ List<String> mappingFileUrls = new ArrayList<String>();
+ mappingFileUrls.add("mapper-aware.xml");
+
+ Map<String, CustomConverter> customConvertersWithId = new HashMap<String, CustomConverter>();
+ customConvertersWithId.put("oneConverter", new TwiceInnerMapperAwareConverter());
+
+ mapper.setCustomConvertersWithId(customConvertersWithId);
+ mapper.setMappingFiles(mappingFileUrls);
+ }
+
+ /**
+ * @see <a href="https://github.com/DozerMapper/dozer/issues/45">issue</a>
+ */
+ @Test
+ public void twiceInnerMapperAwareConverterMapping() {
+ MapperAwareSimpleSrc src = new MapperAwareSimpleSrc();
+ src.setOne(new MapperAwareSimpleInternal());
+ MapperAwareSimpleDest dst = new MapperAwareSimpleDest();
+ mapper.map(src, dst);
+ assertNotNull(dst.getOne());
+ }
+
+ private static class TwiceInnerMapperAwareConverter implements CustomConverter, MapperAware {
+
+ private Mapper mapper;
+
+ @Override
+ public Object convert(Object existingDestinationFieldValue, Object sourceFieldValue, Class<?> destinationClass, Class<?> sourceClass) {
+ MapperAwareSimpleInternal a = (MapperAwareSimpleInternal)sourceFieldValue;
+
+ MapperAwareSimpleInternal b = new MapperAwareSimpleInternal();
+ mapper.map(a, b);
+ MapperAwareSimpleInternal b2 = new MapperAwareSimpleInternal();
+ mapper.map(a, b2); //throws NPE in issue45
+ return b2;
+ }
+
+ @Override
+ public void setMapper(Mapper mapper) {
+ this.mapper = mapper;
+ }
+ }
+}
View
17 core/src/test/java/org/dozer/vo/mapperaware/MapperAwareSimpleDest.java
@@ -0,0 +1,17 @@
+package org.dozer.vo.mapperaware;
+
+/**
+ * @author Dmitry Spikhalskiy
+ * @since 03.01.13
+ */
+public class MapperAwareSimpleDest {
+ private MapperAwareSimpleInternal one;
+
+ public MapperAwareSimpleInternal getOne() {
+ return one;
+ }
+
+ public void setOne(MapperAwareSimpleInternal one) {
+ this.one = one;
+ }
+}
View
8 core/src/test/java/org/dozer/vo/mapperaware/MapperAwareSimpleInternal.java
@@ -0,0 +1,8 @@
+package org.dozer.vo.mapperaware;
+
+/**
+ * @author Dmitry Spikhalskiy
+ * @since 03.01.13
+ */
+public class MapperAwareSimpleInternal {
+}
View
17 core/src/test/java/org/dozer/vo/mapperaware/MapperAwareSimpleSrc.java
@@ -0,0 +1,17 @@
+package org.dozer.vo.mapperaware;
+
+/**
+ * @author Dmitry Spikhalskiy
+ * @since 03.01.13
+ */
+public class MapperAwareSimpleSrc {
+ private MapperAwareSimpleInternal one;
+
+ public MapperAwareSimpleInternal getOne() {
+ return one;
+ }
+
+ public void setOne(MapperAwareSimpleInternal one) {
+ this.one = one;
+ }
+}
View
9 core/src/test/resources/mapper-aware.xml
@@ -21,4 +21,13 @@
</field>
</mapping>
+ <mapping>
+ <class-a>org.dozer.vo.mapperaware.MapperAwareSimpleSrc</class-a>
+ <class-b>org.dozer.vo.mapperaware.MapperAwareSimpleDest</class-b>
+ <field custom-converter-id="oneConverter">
+ <a>one</a>
+ <b>one</b>
+ </field>
+ </mapping>
+
</mappings>
View
4 pom.xml
@@ -80,8 +80,8 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>2.3.2</version>
<configuration>
- <source>1.5</source>
- <target>1.5</target>
+ <source>1.6</source>
+ <target>1.6</target>
</configuration>
</plugin>
<plugin>

0 comments on commit 6b53af5

Please sign in to comment.