Bugzilla – Full Text Bug Listing
| Summary: | Wrong three-way diff produced when object that is referenced from other objects in the model is deleted | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMFCompare | Reporter: | Vygantas Gedgaudas <vygantas.gedgaudas> | ||||
| Component: | Core | Assignee: | EMF Compare <emf.compare-inbox> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | cedric.brun, laurent.goubet, saulius.tvarijonas, vygantas.gedgaudas | ||||
| Version: | unspecified | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | PC | ||||||
| OS: | Linux | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | 373245 | ||||||
| Bug Blocks: | |||||||
| Attachments: |
|
||||||
|
Description
Vygantas Gedgaudas
Created attachment 209629 [details]
Test cases
I have added two test cases in the attachment. They both use the test model from EMF Compare project's source repository and can be simply pasted into org.eclipse.emf.compare.tests.unit.diff.ThreeWayDiffTest and run.
The first test case named testRemotelyDeletedReferenced roughly reflects the steps that I wrote in "Steps to Reproduce" section.
The second test case is a bit more complex. It deletes one Book remotely same way the first test case does. But besides that, it also edits the Book locally.
I also have a question about the second test case. Does deleting object on one side and editing it (or any object that it contains) on the other side produce a conflict?
I have debugged the source code and I think I have found the problem. Or at least what I have found maybe could help for you to resolve the issue. When an object is remotely deleted, MatchModel will contain one UnmatchedElement for that object. This UnmatchedElement will have the following attributes: remote=true, side=Left. Problem is, when the object also goes as reference target the UnmatchedElement is not taken into account in: org.eclipse.emf.compare.diff.engine.check.ReferencesCheck.populateThreeWayReferencesChanges(Match3Elements, EReference, List<EObject>, List<EObject>, List<EObject>, List<EObject>). The method invokes org.eclipse.emf.compare.diff.engine.IMatchManager.getMatchedEObject(EObject, MatchSide) in the second loop where it That is, it does not find a Because of this, "Object added locally to reference" is generated ins *** PLEASE IGNORE THE PREVIOUS COMMENT, I HAVE COMMITTED IT UNFINISHED IT ACCIDENTALLY*** I have debugged the source code and I think I have found the problem. Or at least what I have found probably could help for you to resolve the issue. When an object is remotely deleted, MatchModel will contain one UnmatchedElement for that object. This UnmatchedElement will have the following attributes: remote=true, side=Left. Problem is, when the object also goes as reference target the UnmatchedElement is not taken into account in: org.eclipse.emf.compare.diff.engine.check.ReferencesCheck.populateThreeWayReferencesChanges(Match3Elements, EReference, List<EObject>, List<EObject>, List<EObject>, List<EObject>). In the second loop where remotely deleted and locally added lists are populated, the method invokes the org.eclipse.emf.compare.diff.engine.IMatchManager.getMatchedEObject(EObject, MatchSide) method. This method only takes into account Match2Elements and Match3Elements. I think that at this point the UnmatchedElement should not be overlooked. Because in this particular case, the UnmatchedElement indicates that the object has been found in local copy and ancestor, but was not found in remote copy. What happens is that the populateThreeWayReferencesChanges method does not find match in the ancestor, although it exists. This produces a diff element as if the object has been locally added, but it is not the case. Vygantas, I recently made a lot of changes to the detection algorithm in order to properly detect some new differences. Some of these changes were pretty early in the comparison process and I know that Unmatched EObjects are now to be taken into account to properly detect the actual differences. What you stumbled upon might be one of these case I have yet to detect. I'll add your new test cases and try to debug from there asap. Hi Laurent, Did you have time to look at this issue? If so, were my assumptions right about the problem, that is, is it actually just a matter of taking unmatched objects into account? Vygantas, Sorry, I have been kept quite busy with another project and haven't really gotten to look at this. I did test your use case and reproduce the issue (one change that should be remote is detected as local), but I haven't delved any further than that yet. Vygantas, Just took the time to look into this, and the underlying issue is indeed as you surmised. I did commit a quick & dirty fix for the first of these issues, but I am convinced that some other remain with the current mangement of (un)matches. We are currently in the process of rearchitecturing the whole process in order to avoid the pitfalls we stumbled upon (and are stuck with) with the current approach, this aims at solving the many issues faced because the diff does not have access, nor is directly correlated, to the match. This will mean EMF Compare 2, and we expect a first version of this to ship along with Juno. I do not believe that we can solve all issues we face in EMF Compare with the current code base. This "quick and dirty" fix will solve this particular bug, but not the whole handling of remotes. As for the second example, we should have detected a conflict ... yet did not. This is a separate bug. I've included the first of your two unit tests in the global test suite, the second will have to wait for a fix (looking into it). Vygantas, in order to commit your test cases on the repository, I'll need to add a mention to the author in the copyright notice of the class. Will something like this do? * Contributors: * Obeo - initial API and implementation * Victor Roldan Betancort - [352002] introduce IMatchManager * Vygantas Gedgaudas - [368841] Introduce new test for 3way cases *******************************************************************************/ Or would you rather the name of your company to appear in there (and how)? Laurent, I am glad to hear that you are addressing my issue. I am looking forward to try EMF Compare after you finish current architectural changes. As for the copyright notice, I think it is sufficient enough if you just put my name as shown in the example. I just tried with the current tip of master (after the restructuration has been done) and the returned results are the expected one. Good :) We now have to adapt the testcase you gave us to the new APIs. We'll close once done. The bug has been fixed a good while ago; all that remained here were the unit tests... which have been pushed to the repository on march 2012. Closing. |