Bug 532484 - The EMF Compare UI should allow proxy resolution
Summary: The EMF Compare UI should allow proxy resolution
Status: NEW
Alias: None
Product: EMFCompare
Classification: Modeling
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: EMF Compare CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-15 07:00 EDT by Laurent Goubet CLA
Modified: 2018-03-19 05:52 EDT (History)
1 user (show)

See Also:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Laurent Goubet CLA 2018-03-15 07:00:23 EDT 
The logical model of EMF Compare might remove parts of the resources from the scope of comparison in order to save up memory space during the comparison process. However, the UI should allow proxy resolution since some models might fail to display properly when the proxies are not resolved.

For example, if we consider the case of a genmodel and ecore file, there is the possibility that only the genmodel has changed between the two compared revisions, in which case the ecore file will not be part of the comparison scope. The comparison editor however, should allow the references from genmodel towards ecore to be resolved in order to ensure proper labels when shown to the user.

Likewise, there is a chance that the reference towards one model to the other has changed, but the "other" is not in the comparison scope, in which case we'll just create a diff pointing to the proxy value (since there was no reason to load that other model). The UI will not be able to load that proxy since the proxy itself would thus be contained in the Comparison (through the "ReferenceChange.value" reference), but EMF cannot resolve proxies without a scope.

For differenceson proxies, we need either one of:
1. attach an adapter to the proxies we set to "ReferenceChange.value" that will point to the proper resource/resourceset (left, right or origin) for that proxy's resolution and override "ReferenceChangeImpl#getValue()" so that we use that adapter's scope for the call to "eResolveProxy", or
2. create a new kind of diff (a specialized ReferenceChange?) that instead of just a "value" will know on "which object" through "which reference" at "which index" there is a proxy to resolve, so that we can just leave EMF do the rest when the UI needs to resolve that proxy to display the changed value.
Comment 1 Laurent Goubet CLA 2018-03-15 07:02:18 EDT 
This bug makes the current implementation of "NotLoadingResourceSet#setAllowResourceLoad" useless; that method had been thought of in order to reallow resource loading from the UI (post-comparison) but in turn since the UI doesn't have a resolution context it ends up not being able to resolve proxies at all.
Comment 2 Philip Langer CLA 2018-03-15 10:10:52 EDT 
Please also take this change into account: https://git.eclipse.org/r/#/c/106454/
Comment 3 Laurent Goubet CLA 2018-03-15 10:22:11 EDT 
Thanks, I'd probably have stumbled on that second bug lurking behind the impossibility to resolve proxies after I fixed the first.

I'll take approach 1. of overriding the "ReferenceChange#getValue()"... but it actually doesn't require a new adapter. We have all information on the diff itself to compute the resolving scope.
Comment 4 Eclipse Genie CLA 2018-03-16 06:49:52 EDT 
New Gerrit change created: https://git.eclipse.org/r/119552
Comment 5 Eclipse Genie CLA 2018-03-16 06:49:54 EDT 
New Gerrit change created: https://git.eclipse.org/r/119551
Comment 6 Laurent Goubet CLA 2018-03-16 06:50:12 EDT 
There were three bugs behind this.

First was the fact that we were caching the "null" resources when we weren't able to load them and that cache was preventing resolution even after the "allowResourceLoad" boolean was set. This was fixed by https://git.eclipse.org/r/c/106454/ (thanks Philip).

Second was described as this bug's description : the "value" of a ReferenceChange didn't know about its resolution scope and thus could never be resolved.

Third were the implementations of the IStructuralFeatureAccessor that were also avoiding proxy resolution even though they were allowed to.

There might still be issues with feature maps but I'm unsure on how to test and debug that.
Comment 7 Laurent Goubet CLA 2018-03-16 09:52:46 EDT 
The failing unit tests highlight a fourth bug lurking behind this : there are bits of code that do try and resolve references during the comparison process (i.e. not from the UI).
Comment 8 Laurent Goubet CLA 2018-03-16 10:26:29 EDT 
Both approaches outlined in this bug's comments are wrong. We cannot change the "value" of a ReferenceChange after the comparison since we're using this value (which may be a proxy") as the basis for all merge operations, the Matched values... allowing their resolution will only change the Matches to be out-of-sync with the diffs and cause issues down the line.

We can, however, change the displayed values in the UI by focusing on the ReferenceChangeItemProvider, which is a less intrusive change and that does belong to the UI.