Permalink
Browse files

Fixed issue #6.

  • Loading branch information...
1 parent ed826dd commit 3107b8c517fae7fc73f43e454a6c0bb7b2fb7347 @jhalterman jhalterman committed Jan 6, 2013
@@ -243,28 +243,25 @@ private void setDestinationValue(MappingContextImpl<?, ?> context,
if (destinationValue == null)
context.shadePath(mapping.getPath());
} else {
- Object intermediateDest = null;
-
- // Obtain from existing destinations
- if (!context.intermediateDestinations.isEmpty()) {
- for (Object intermediateDestination : context.intermediateDestinations) {
- // Match intermediate destinations to mutator by type
- if (intermediateDestination.getClass().equals(mutator.getType())) {
- intermediateDest = intermediateDestination;
- context.destinationCache.put(destPath, intermediateDest);
- mutator.setValue(destination, intermediateDest);
- break;
+ // Obtain from cache
+ Object intermediateDest = context.destinationCache.get(destPath);
+
+ if (intermediateDest != null) {
+ mutator.setValue(destination, intermediateDest);
+ } else {
+ // Obtain from circular destinations
+ if (!context.intermediateDestinations.isEmpty()) {
+ for (Object intermediateDestination : context.intermediateDestinations) {
+ // Match intermediate destinations to mutator by type
+ if (intermediateDestination.getClass().equals(mutator.getType())) {
+ intermediateDest = intermediateDestination;
+ mutator.setValue(destination, intermediateDest);
+ break;
+ }
}
}
- }
-
- // Obtain from cache
- if (intermediateDest == null) {
- intermediateDest = context.destinationCache.get(destPath);
- if (intermediateDest != null) {
- mutator.setValue(destination, intermediateDest);
- } else {
+ if (intermediateDest == null) {
// Obtain from accessor on provided destination
if (context.providedDestination) {
Accessor accessor = TypeInfoRegistry.typeInfoFor(destination.getClass(),
@@ -277,19 +274,23 @@ private void setDestinationValue(MappingContextImpl<?, ?> context,
// Obtain from new instance
if (intermediateDest == null) {
- // Via global provider
- if (configuration.getProvider() != null)
- intermediateDest = configuration.getProvider().get(
- new ProvisionRequestImpl(context.parentSource(), mutator.getType()));
+ if (propertyContext.getSource() == null)
+ return;
+
+ Provider<?> globalProvider = configuration.getProvider();
+ if (globalProvider != null)
+ intermediateDest = globalProvider.get(new ProvisionRequestImpl(
+ context.parentSource(), mutator.getType()));
else
intermediateDest = instantiate(mutator.getType(), context.errors);
if (intermediateDest == null)
return;
- context.destinationCache.put(destPath, intermediateDest);
mutator.setValue(destination, intermediateDest);
}
}
+
+ context.destinationCache.put(destPath, intermediateDest);
}
destination = intermediateDest;
@@ -0,0 +1,145 @@
+package org.modelmapper.bugs;
+
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+
+import org.modelmapper.AbstractTest;
+import org.testng.annotations.Test;
+
+/**
+ * https://github.com/jhalterman/modelmapper/issues/6
+ *
+ * @author Jonathan Halterman
+ */
+@Test
+public class GH6 extends AbstractTest {
+ public static class Source {
+ SourceMessage message;
+
+ public SourceMessage getMessage() {
+ return message;
+ }
+
+ public void setMessage(SourceMessage message) {
+ this.message = message;
+ }
+ }
+
+ public static class SourceMessage {
+ SourceMessageBody body;
+
+ public SourceMessageBody getBody() {
+ return body;
+ }
+
+ public void setBody(SourceMessageBody body) {
+ this.body = body;
+ }
+ }
+
+ public static class SourceMessageBody {
+ String value;
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+ }
+
+ public static class Dest {
+ DestMessage message;
+
+ public DestMessage getMessage() {
+ return message;
+ }
+
+ public void setMessage(DestMessage message) {
+ this.message = message;
+ }
+ }
+
+ public static class DestMessage {
+ DestMessageBody body;
+
+ public DestMessageBody getBody() {
+ return body;
+ }
+
+ public void setBody(DestMessageBody body) {
+ this.body = body;
+ }
+ }
+
+ public static class DestMessageBody {
+ String value = "originalValue";
+
+ public String getValue() {
+ return value;
+ }
+
+ public void setValue(String value) {
+ this.value = value;
+ }
+ }
+
+ public void shouldNotInstantiateIntermediateObjects() {
+ Dest dest = modelMapper.map(new Source(), Dest.class);
+ assertNull(dest.message);
+
+ Source source = new Source();
+ source.message = new SourceMessage();
+ dest = modelMapper.map(source, Dest.class);
+ assertNull(dest.message);
+ }
+
+ public void shouldNotInstantiateIntermediateObjectOnProvidedDestination() {
+ Source source = new Source();
+ Dest dest = new Dest();
+ dest.message = new DestMessage();
+ modelMapper.map(source, dest);
+ assertNull(dest.message.body);
+
+ source.message = new SourceMessage();
+ modelMapper.map(source, dest);
+ assertNull(dest.message.body);
+ }
+
+ public void shouldIgnoreIntermediateDestinationWhenSourceIsNotNull() {
+ Source source = new Source();
+ source.message = new SourceMessage();
+ Dest dest = new Dest();
+ dest.message = new DestMessage();
+ modelMapper.map(source, dest);
+ assertNotNull(dest.message);
+
+ source.message.body = new SourceMessageBody();
+ dest.message.body = new DestMessageBody();
+ modelMapper.map(source, dest);
+ assertNotNull(dest.message.body);
+ }
+
+ /**
+ * Cannot support this for now due to potentially asymmetric mappings. We don't know for sure that
+ * source.message maps to dest.message and therefore can't null dest.message just because
+ * source.message is null.
+ *
+ * TODO enable once better information is captured for asymmetric mappings
+ */
+ @Test(enabled = false)
+ public void shouldNullifyIntermediateDestinationWhenSourceIsNull() {
+ Source source = new Source();
+ Dest dest = new Dest();
+ dest.message = new DestMessage();
+ modelMapper.map(source, dest);
+ assertNull(dest.message);
+
+ source.message = new SourceMessage();
+ dest.message = new DestMessage();
+ dest.message.body = new DestMessageBody();
+ modelMapper.map(source, dest);
+ assertNull(dest.message.body);
+ }
+}
@@ -9,6 +9,7 @@
/**
* @author Jonathan Halterman
+ * @see {@link org.modelmapper.bugs.GH6} for additional null behavior tests
*/
@Test(groups = "functional")
public class NullBehavior extends AbstractTest {
@@ -64,10 +64,7 @@ public void shouldShadeWhenNullEncountered() {
ContainerDTO dto = modelMapper.map(c, ContainerDTO.class);
for (int i = 0; i < dto.items.size(); i++)
if (i == 2) {
- // TODO this line should pass since the source message is null
- // assertNull(dto.items.get(i).message);
- assertNull(dto.items.get(i).message.value1);
- assertNull(dto.items.get(i).message.value2);
+ assertNull(dto.items.get(i).message);
} else {
assertNotNull(dto.items.get(i).message.value1);
assertNotNull(dto.items.get(i).message.value2);
@@ -42,6 +42,6 @@ public void shouldShadeNullValue() {
modelMapper.getConfiguration().setMatchingStrategy(MatchingStrategies.LOOSE);
DestOne dest = modelMapper.map(a, DestOne.class);
- assertNull(dest.two.three.value);
+ assertNull(dest.two);
}
}

0 comments on commit 3107b8c

Please sign in to comment.