Issue with circular

[garpinc]

In BeanDiffer i've got a property node with a toString of:

DefaultNode(state=CIRCULAR, type=java.util.Collection, no children, accessed via property 'structureDiagram')

In print visitor it says:
Property at path '/xxxxx' has already been processed at another position. (Circular reference!) (Circular reference detected: The property has already been processed at another position.)

What I would expect here instead is for it to have the previous diff reported or some reference to the node that had previously been processed.

Is that possible?

[SQiShER]

Adding a reference to the previous node should be possible. Not straightforward, but doable. I think that would be a very nice feature. I'll see what I can do!

[garpinc]

How would you go about it? I'm on the hot seat to get this work based on your tool done. From what I can tell in the node you don't keep a reference to the instance that the node corresponded to so I can't look in the nodes for the matching node... Any ideas to keep me going would be helpful.

[SQiShER]

That's a good question. Took me a few hours to figure it out. Turns out that the CircularReferenceDetector and the DifferDelegator are the best places to do this. So far I've been able to set the PropertyPath of the node at the beginning of the circle as property of the circular node. By using this path you can retrieve the corresponding node from the root node. Maybe that's already enough to solve your problem. I would prefer to assign the actual circle start node as property of the circular node, but I haven't been able to get this done in time.

If you like, you can try to find a way to get the actual node referenced by the PropertyPath in line 75 of the DifferDelegator in the head of the master branch and assign it to the circular node. It could be as easy as following the parent nodes to the beginning and matching their property paths against the one returned by the circular reference detector. But I have a bad feeling about this. Something tells me, that it's not going to be that easy. ;-)

Please let me know if the solution we have so far already helps you. Once we found a working solution, we can add some more tests and start preparing the next release.

[garpinc]

What I've done is modified this method:
private Node delegateWithCircularReferenceTracking(final Node parentNode, final Instances instances) {
Node node;
try {
rememberInstances(instances);
try {
node = compare(parentNode, instances);
INSTANCE_TO_NODE_THREAD_LOCAL.get().put(instances.getBase(), node);
INSTANCE_TO_NODE_THREAD_LOCAL.get().put(instances.getWorking(), node);
} finally {
forgetInstances(instances);
}
} catch (CircularReferenceException e) {
Node knownInstanceNode = INSTANCE_TO_NODE_THREAD_LOCAL.get().get(e.getKnownInstance());
if (knownInstanceNode != null) {
node = copyNode(knownInstanceNode, instances.getSourceAccessor(), parentNode);
} else {
node = newCircularNode(parentNode, instances);
}
logCircularReference(node);
}
if (parentNode == null) {
resetInstanceMemory();
}
return node;
}

Where INSTANCE_TO_NODE_THREAD_LOCAL is a threadlocal with an object to node mapping
and where the exception is thrown I add the instance that was found in the stack to the exception so I can use the map to find the node.

BUT I'm encountering an issue. When you use the visitor to visit the nodes and you get to the circular node then what do you do to show the diffs at that parent node?

I resorted to making a copy of the detected node and it's children buy I'm running into issues since the accessor doesn't seem to be the right one for the object being navigated with following exception:

Caused by: org.openanzo.exceptions.AnzoException: ErrorCode[4430] An error occurred while executing the transaction callback within a transaction template. See this exceptions cause for more information. Root Cause: de.danielbechler.diff.accessor.exception.PropertyReadException Failed to invoke read method. Property 'structureDiagram' on target of type class java.util.ArrayList.

[SQiShER]

I would highly recommend to us a property, like I did in the changes that I committed to the master branch. The problem with simply copying the base node (aside from mixed up accessors) is, that a visitor would run into infinite loops very easily, since the node hierarchy would also contain circular references. I rather have someone conciously opt-in to tackle circular references than let others run into infinite loops, because they weren't aware they had any in the first place.

Since the circular node represents the same instance as the one that started the circle, it should do no harm to explicitly look at a previous node at a given path, since their contents would be identical.

I think by adding a copy of the original node, you'll increase the complexity of understanding and traversing the graph - which isn't very low to begin with - by quite a bit. Please take a look at my latest commits, to see the alternate route of adding an extra property and see if that might be a solution for your problem. Unfortunately I won't have time to look into this any further until tomorrow evening, but I'll read and start thinking about anything you write until then.

[garpinc]

When you get to it tomorrow please see if you can get #46 integrated so I don't have to re-integrate my changes in order to use your branch.

[SQiShER]

Alright, it's done and committed into the master branch. Now a circular node not only contains the property path to the start of the circular reference, but also the start node itself. Give it a try and please let me know if it works as expected. As soon as everything is alright, I'll release a new version.

[garpinc]

My objects aren't necessarily == when they are equals
Hence can you also add the following:
In public boolean knows(final Object needle) method right after the ==
if (entry.getInstance() != null && needle != null && entry.getInstance().equals(needle)) {
return true;
}

and in private Entry entryForInstance(final Object instance) right after ==
if (entry.getInstance() != null && instance != null && entry.getInstance().equals(instance)) {
return entry;
}

[garpinc]

I personally am all set for a new build. Everything on my end seems to be working.. Can you also add another file to the jar that gets generated in addition to the equals method. I run in an osgi environment so I need the following manifest.
in META-INF\MANIFEST.MF of the jar

Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: java-object-diff
Bundle-SymbolicName: java-object-diff
Bundle-Version: 0.10.1
Bundle-ClassPath: .
Bundle-Vendor: Daniel Bechler
Export-Package: de.danielbechler.diff;version="0.10.1",
de.danielbechler.diff.accessor;version="0.10.1",
de.danielbechler.diff.accessor.exception;version="0.10.1",
de.danielbechler.diff.annotation;version="0.10.1",
de.danielbechler.diff.example;version="0.10.1",
de.danielbechler.diff.introspect;version="0.10.1",
de.danielbechler.diff.node;version="0.10.1",
de.danielbechler.diff.path;version="0.10.1",
de.danielbechler.diff.visitor;version="0.10.1",
de.danielbechler.merge;version="0.10.1",
de.danielbechler.util;version="0.10.1"
Import-Package: org.slf4j;version="1.5.8"
Bundle-RequiredExecutionEnvironment: JavaSE-1.6

[SQiShER]

I added OSGi-Support, no problem.

But I can't add the equality check to the circular reference detection mechanism. Its sole purpose is to prevent the ObjectDiffer from ending up in an infinite loop by analyzing the exact same instance along a path again and again. It is however perfectly fine to nest equal objects, as long as they are different instances. What exactly are you trying to solve? Are you sure it's something that can't be solved with a clever Visitor?

[garpinc]

Firstly when I run the maven build the Manifest in the resulting jar doe not seem to contain the information I supplied above.

Secondly when I don't have the equals method I get the infinite loop situation.

I think the situation happens where you have a nested object that's equal to one previously encountered but is not == in the java object sense.
In this situation it starts doing the diff analyis of that object as if it were an entirely different object where in fact it's already been processed.

adding the equals solves that issue.

[SQiShER]

Lets move the OSGi discussion to this thread: #53

[SQiShER]

Hmm, the equals behavior is really weird. I could imagine this happening when the property getter returns a deep copy of the object. In that case it would make sense to run into an infinite loop. Could you provide me with a test class that reproduces this behavior? I'd really like to understand why this is happening.

[garpinc]

It's going to be hard for me to provide a test case.. We can coordinate on skype so I can show your the behaviour i'm seeing in debug mode and you can direct me in the right breakpoints to set. my handle is garpinc

Date: Sun, 17 Feb 2013 09:45:14 -0800
From: notifications@github.com
To: java-object-diff@noreply.github.com
CC: garpinc@hotmail.com
Subject: Re: [java-object-diff] Issue with circular (#52)

Hmm, the equals behavior is really weird. I could imagine this happening when the property getter returns a deep copy of the object. In that case it would make sense to run into an infinite loop. Could you provide me with a test class that reproduces this behavior? I'd really like to understand why this is happening.

          —

          Reply to this email directly or view it on GitHub.