Skip to content

Commit e1cab60

Browse files
SQiShERSQiShER
committed
SQiShER#33 - Allowing to return child nodes of added or removed collection items
1 parent c12ed26 commit e1cab60

16 files changed

Lines changed: 520 additions & 246 deletions

‎src/main/java/de/danielbechler/diff/AbstractDiffer.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@
1818

1919
import de.danielbechler.diff.node.*;
2020
import de.danielbechler.util.*;
21+
import org.slf4j.*;
2122

2223
/** @author Daniel Bechler */
2324
abstract class AbstractDiffer<T extends Node> implements Differ<T>, Configurable
2425
{
25-
static final ThreadLocal<CircularReferenceDetector> CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL;
26+
private static final Logger logger = LoggerFactory.getLogger(AbstractDiffer.class);
27+
28+
static final ThreadLocal<CircularReferenceDetector> WORKING_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL;
29+
static final ThreadLocal<CircularReferenceDetector> BASE_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL;
2630

2731
static
2832
{
29-
CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL = new CircularReferenceDetectorThreadLocal();
33+
WORKING_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL = new CircularReferenceDetectorThreadLocal();
34+
BASE_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL = new CircularReferenceDetectorThreadLocal();
3035
}
3136

3237
private DelegatingObjectDiffer delegate;
@@ -44,30 +49,38 @@ protected AbstractDiffer(final DelegatingObjectDiffer delegate)
4449
@Override
4550
public final T compare(final Node parentNode, final Instances instances)
4651
{
47-
final Object working = instances.getWorking();
48-
final CircularReferenceDetector circularReferenceDetector = CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.get();
49-
final boolean destroyOnReturn = circularReferenceDetector.isNew();
52+
final CircularReferenceDetector workingCircularReferenceDetector =
53+
WORKING_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.get();
54+
final CircularReferenceDetector baseCircularReferenceDetector =
55+
BASE_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.get();
56+
final boolean destroyOnReturn = workingCircularReferenceDetector.isNew();
5057
T node;
5158
try
5259
{
53-
circularReferenceDetector.push(working);
60+
workingCircularReferenceDetector.push(instances.getWorking());
61+
baseCircularReferenceDetector.push(instances.getBase());
5462
try
5563
{
5664
node = internalCompare(parentNode, instances);
5765
}
5866
finally
5967
{
60-
circularReferenceDetector.remove(working);
68+
workingCircularReferenceDetector.remove(instances.getWorking());
69+
baseCircularReferenceDetector.remove(instances.getBase());
6170
}
6271
}
6372
catch (CircularReferenceDetector.CircularReferenceException e)
6473
{
6574
node = newNode(parentNode, instances);
6675
node.setState(Node.State.CIRCULAR);
76+
logger.warn("Detected circular reference in node at path {}. " +
77+
"Going deeper would cause an infinite loop, so I'll stop looking at " +
78+
"this instance along the current path.", node.getPropertyPath());
6779
}
6880
if (destroyOnReturn)
6981
{
70-
CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.remove();
82+
WORKING_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.remove();
83+
BASE_CIRCULAR_REFERENCE_DETECTOR_THREAD_LOCAL.remove();
7184
}
7285
return node;
7386
}

‎src/main/java/de/danielbechler/diff/BeanDiffer.java‎

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,12 @@ private Node compareBean(final Node parentNode, final Instances instances)
8282
final Node node = newNode(parentNode, instances);
8383
if (instances.hasBeenAdded())
8484
{
85+
compareNodeWithAppropriateMethod(node, instances);
8586
node.setState(Node.State.ADDED);
8687
}
8788
else if (instances.hasBeenRemoved())
8889
{
90+
compareNodeWithAppropriateMethod(node, instances);
8991
node.setState(Node.State.REMOVED);
9092
}
9193
else if (instances.areSame())
@@ -94,18 +96,23 @@ else if (instances.areSame())
9496
}
9597
else
9698
{
97-
if (getDelegate().isEqualsOnly(node))
98-
{
99-
compareWithEquals(node, instances);
100-
}
101-
else
102-
{
103-
compareProperties(node, instances);
104-
}
99+
compareNodeWithAppropriateMethod(node, instances);
105100
}
106101
return node;
107102
}
108103

104+
private void compareNodeWithAppropriateMethod(final Node node, final Instances instances)
105+
{
106+
if (getDelegate().isEqualsOnly(node))
107+
{
108+
compareWithEquals(node, instances);
109+
}
110+
else
111+
{
112+
compareProperties(node, instances);
113+
}
114+
}
115+
109116
private static void compareWithEquals(final Node node, final Instances instances)
110117
{
111118
if (!instances.areEqual())
@@ -116,34 +123,25 @@ private static void compareWithEquals(final Node node, final Instances instances
116123

117124
private void compareProperties(final Node parentNode, final Instances instances)
118125
{
126+
final DelegatingObjectDiffer delegate = getDelegate();
119127
for (final Accessor accessor : introspect(instances.getType()))
120128
{
121-
final Node node = new DefaultNode(parentNode, accessor, instances.getType());
122-
if (getDelegate().isIgnored(node))
123-
{
124-
if (getDelegate().isReturnable(node))
125-
{
126-
node.setState(Node.State.IGNORED);
127-
parentNode.addChild(node);
128-
}
129-
continue;
130-
}
131-
final Node child = getDelegate().delegate(parentNode, instances.access(accessor));
132-
if (child.hasChanges())
129+
final Node propertyNode = delegate.delegate(parentNode, instances.access(accessor));
130+
if (delegate.isReturnable(propertyNode))
133131
{
134-
parentNode.setState(Node.State.CHANGED);
135-
parentNode.addChild(child);
136-
}
137-
else if (getConfiguration().isReturnable(child))
138-
{
139-
parentNode.addChild(child);
132+
parentNode.addChild(propertyNode);
140133
}
141134
}
142135
}
143136

144137
private Iterable<Accessor> introspect(final Class<?> type)
145138
{
146-
return introspector.introspect(type);
139+
final Iterable<Accessor> accessorIterable = introspector.introspect(type);
140+
if (accessorIterable != null)
141+
{
142+
return accessorIterable;
143+
}
144+
return java.util.Collections.emptyList();
147145
}
148146

149147
void setIntrospector(final Introspector introspector)

‎src/main/java/de/danielbechler/diff/CollectionDiffer.java‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public CollectionNode compare(final Collection<?> working, final Collection<?> b
4747
@Override
4848
protected CollectionNode internalCompare(final Node parentNode, final Instances instances)
4949
{
50-
final CollectionNode node = new CollectionNode(parentNode, instances.getSourceAccessor(), instances.getType());
50+
final CollectionNode node = newNode(parentNode, instances);
5151
if (getDelegate().isIgnored(node))
5252
{
5353
node.setState(Node.State.IGNORED);
@@ -88,12 +88,7 @@ private void handleItems(final CollectionNode collectionNode,
8888
for (final Object item : items)
8989
{
9090
final Node child = compareItem(collectionNode, instances, item);
91-
if (child.hasChanges())
92-
{
93-
collectionNode.addChild(child);
94-
collectionNode.setState(Node.State.CHANGED);
95-
}
96-
else if (getConfiguration().isReturnable(child))
91+
if (getDelegate().isReturnable(child))
9792
{
9893
collectionNode.addChild(child);
9994
}
@@ -104,8 +99,7 @@ private Node compareItem(final CollectionNode collectionNode,
10499
final Instances instances,
105100
final Object item)
106101
{
107-
final Accessor accessor = collectionNode.accessorForItem(item);
108-
return getDelegate().delegate(collectionNode, instances.access(accessor));
102+
return getDelegate().delegate(collectionNode, instances.access(collectionNode.accessorForItem(item)));
109103
}
110104

111105
private static Collection<?> findAddedItems(final Instances instances)

‎src/main/java/de/danielbechler/diff/Configuration.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -183,18 +183,26 @@ public boolean isEqualsOnly(final Node node)
183183
@Override
184184
public boolean isReturnable(final Node node)
185185
{
186-
if (node.isUntouched())
187-
{
188-
return returnUnchangedNodes;
189-
}
190-
else if (node.isIgnored())
186+
if (node.isIgnored())
191187
{
192188
return returnIgnoredNodes;
193189
}
194190
else if (node.isCircular())
195191
{
196192
return returnCircularNodes;
197193
}
194+
else if (node.isUntouched())
195+
{
196+
if (node.hasChanges())
197+
{
198+
return true;
199+
}
200+
else if (node.hasChildren())
201+
{
202+
return true;
203+
}
204+
return returnUnchangedNodes;
205+
}
198206
return true;
199207
}
200208
}

‎src/main/java/de/danielbechler/diff/DelegatingObjectDifferImpl.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,11 @@ public <T> Node compare(final T working, final T base)
6464
*/
6565
public Node delegate(final Node parentNode, final Instances instances)
6666
{
67-
if (Collection.class.isAssignableFrom(instances.getType()))
67+
if (instances.areNull())
68+
{
69+
return newNode(parentNode, instances);
70+
}
71+
else if (Collection.class.isAssignableFrom(instances.getType()))
6872
{
6973
return collectionDiffer.compare(parentNode, instances);
7074
}
@@ -78,6 +82,11 @@ else if (Map.class.isAssignableFrom(instances.getType()))
7882
}
7983
}
8084

85+
private static Node newNode(final Node parentNode, final Instances instances)
86+
{
87+
return new DefaultNode(parentNode, instances.getSourceAccessor(), instances.getType());
88+
}
89+
8190
public boolean isIgnored(final Node node)
8291
{
8392
return configuration.isIgnored(node);

‎src/main/java/de/danielbechler/diff/Instances.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ public boolean areSame()
143143
return working == base;
144144
}
145145

146+
public boolean areNull()
147+
{
148+
return working == null && base == null;
149+
}
150+
146151
public Class<?> getType()
147152
{
148153
if (sourceAccessor instanceof TypeAwareAccessor)

‎src/main/java/de/danielbechler/diff/MapDiffer.java‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public MapNode compare(final Map<?, ?> modified, final Map<?, ?> base)
4747
@Override
4848
protected MapNode internalCompare(final Node parentNode, final Instances instances)
4949
{
50-
final MapNode node = new MapNode(parentNode, instances.getSourceAccessor(), instances.getType());
50+
final MapNode node = newNode(parentNode, instances);
5151

5252
if (getConfiguration().isIgnored(node))
5353
{
@@ -104,27 +104,20 @@ private void handleEntries(final Instances instances, final MapNode parent, fina
104104
private void handleEntries(final Object key, final Instances instances, final MapNode parent)
105105
{
106106
final Node node = compareEntry(key, instances, parent);
107-
if (node.hasChanges())
108-
{
109-
parent.setState(Node.State.CHANGED);
110-
parent.addChild(node);
111-
}
112-
else if (getConfiguration().isReturnable(node))
107+
if (getDelegate().isReturnable(node))
113108
{
114109
parent.addChild(node);
115110
}
116111
}
117112

118-
private Node compareEntry(final Object key, Instances instances, final MapNode parent)
113+
private Node compareEntry(final Object key, final Instances instances, final MapNode parent)
119114
{
120-
final Accessor accessor = parent.accessorForKey(key);
121-
instances = instances.access(accessor);
122-
if (instances.areSame())
123-
{
124-
// if the instances are the same, there is no need to delegate
125-
return new DefaultNode(parent, accessor, instances.getType());
126-
}
127-
return getDelegate().delegate(parent, instances);
115+
// if (instances.areSame())
116+
// {
117+
// // if the instances are the same, there is no need to delegate
118+
// return new DefaultNode(parent, accessor, instances.getType());
119+
// }
120+
return getDelegate().delegate(parent, instances.access(parent.accessorForKey(key)));
128121
}
129122

130123
private static Collection<?> findAddedKeys(final Instances instances)

‎src/main/java/de/danielbechler/diff/node/DefaultNode.java‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -222,14 +222,20 @@ else if (node.getParentNode() != null && node.getParentNode() != this)
222222
{
223223
node.setParentNode(this);
224224
children.put(pathElement, node);
225-
return true;
226225
}
227226
else if (node.getParentNode() == this)
228227
{
229228
children.put(pathElement, node);
230-
return true;
231229
}
232-
throw new IllegalStateException("Detected attempt to replace the parent node of node at path '" + getPropertyPath() + "'");
230+
else
231+
{
232+
throw new IllegalStateException("Detected attempt to replace the parent node of node at path '" + getPropertyPath() + "'");
233+
}
234+
if (state == State.UNTOUCHED && node.hasChanges())
235+
{
236+
state = State.CHANGED;
237+
}
238+
return true;
233239
}
234240

235241
public final void visit(final Visitor visitor)
@@ -374,11 +380,13 @@ public void canonicalUnset(Object target)
374380
public String toString()
375381
{
376382
final StringBuilder sb = new StringBuilder();
377-
sb.append(getPropertyPath());
378-
sb.append(" = { ").append(getState().toString().toLowerCase());
383+
sb.append(getClass().getSimpleName());
384+
sb.append("(");
385+
sb.append("state=");
386+
sb.append(getState().toString());
379387
if (getType() != null)
380388
{
381-
sb.append(", type is ").append(getType().getCanonicalName());
389+
sb.append(", type=").append(getType().getCanonicalName());
382390
}
383391
if (getChildren().size() == 1)
384392
{
@@ -396,7 +404,8 @@ else if (getChildren().size() > 1)
396404
{
397405
sb.append(", categorized as ").append(getCategories());
398406
}
399-
sb.append(" }");
407+
sb.append(", accessed via ").append(accessor);
408+
sb.append(')');
400409
return sb.toString();
401410
}
402411

0 commit comments

Comments
 (0)