Bug 266004 - [NotationModel] DiagramLink#diagramLink and MultiDiagramLink#diagramLinks should resolve proxies
Summary: [NotationModel] DiagramLink#diagramLink and MultiDiagramLink#diagramLinks sho...
Status: RESOLVED FIXED
Alias: None
Product: GMF-Runtime
Classification: Modeling
Component: Notation (show other bugs)
Version: 2.1   Edit
Hardware: All All
: P3 normal
Target Milestone: ---   Edit
Assignee: Alex Boyko CLA
QA Contact:
URL:
Whiteboard: 2.1.4 Patch
Keywords:
Depends on:
Blocks:
 
Reported: 2009-02-24 14:20 EST by Alex Boyko CLA
Modified: 2020-02-18 06:11 EST (History)
5 users (show)

See Also:


Attachments
proposed patch for 2.2 (27.12 KB, patch)
2009-02-26 16:34 EST, Alex Boyko CLA
ahunter.eclipse: iplog+
Details | Diff
2.1.3 compatible patch (11.72 KB, patch)
2009-02-26 16:56 EST, Alex Boyko CLA
ahunter.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Boyko CLA 2009-02-24 14:20:48 EST
I asked Ed Merks about what exactly the "Resolve Proxies" option mean (http://www.eclipse.org/newsportal/article.php?id=39654&group=eclipse.tools.emf#39654). It looks like we made a mistake by setting "Resolve Proxies" to false in the past. "Resolve Proxies" means that the reference is not expected to be proxy, while we do allow it to be proxy (i.e. links to diagrams in a different resource).
Not having "Resolve Proxies" option for "diagramLink" reference results in returning proxy object for DiagramLinkStyle#getDiagramLink() all the time, unless resolved manually, so EMF by itself cannot resolve it. Hence, CrossReferenceAdapter#resolveAll() wouldn't resolve the diagramLink. If the resource containing the diagram the link is referencing is loaded, but the link hasn't been resolved manually The inverse cross references of the diagram wouldn't have the diagram link features from diagram link styles that didn't resolve diagram link proxy manually.
BTW, you can always get an unresolved diagram with a basic get. I don't think that diagramLink proxies will be resolved automatically when resource is loaded also, because of basic get. They will be and most likely are resolved if  a name of the diagram is displayed by the editpart corresponding to a view with diagram link on it.
How about we try automatic proxy resolution for diagram link styles in 2.2?
Comment 1 Anthony Hunter CLA 2009-02-25 09:34:44 EST
Adding Mike and Chris so they can add their comments. 
Comment 2 Anthony Hunter CLA 2009-02-25 09:38:46 EST
Hi,
 
 It doesn't sound too dangerous, but I wonder what would be the reason to force proxy resolution? To me, keeping it proxy has certain benefits given usecases for the linked diagrams. Unless really important, I'd refrain from changing it.
 
Artem.
Comment 3 Michael D. Elder CLA 2009-02-25 09:49:29 EST
As stated in the description, the field will only be resolved if accessed via eGet()
or getDiagramLink(). If the field is never access, it'll never be resolved.

If current applications are storing intra-resource references using this style,
they won't be affected because they're not storing the proxy. 

If current applications are storing inter-resource references using this style,
they'll hit the same issue that we have, which is that the reference to the
diagram is never resolved in the model. 

In particular, the ECrossReferenceAdapter (and its various subclasses), make
assumptions that a call to resolveAll() will force the resolution of the
referenced Diagram object, which will then allow the indexer to "see" and find
the link. This leads to errant behavior in search, which can cause problems in
secondary clients, like refactoring. The end result is that it's not possible
for the indexer to find the link when the DiagramLinkStyle is in memory; _even_
if the _referenced_ diagram is already loaded. Even though it's loaded, the
proxied field isn't updated to point to the _referenced_ diagram. We need the
field to be enabled to resolve its proxy in order to fix errant behavior. 

In response to comment 2, we need the diagram to be resolved first for the indexer to work. Second, we do actually show the nested diagram content in the referencing diagram. For the referencer to show the diagram content, the diagram must be resolved. 
Comment 4 Alex Boyko CLA 2009-02-26 16:34:41 EST
Created attachment 126904 [details]
proposed patch for 2.2

proposed patch for 2.2. "Resolves Proxies" == true for DiagramLinkStyle#diagramLink and MultiDiagramLinkStyle#diagramLinks, code regenerated for:
DiagramLinkStyle
HintedDiagramLinkStyle
MultiDiagramLinkStyle
Comment 5 Alex Boyko CLA 2009-02-26 16:56:17 EST
Created attachment 126907 [details]
2.1.3 compatible patch

Ptach compatible with 2.1.3, i.e. 2.1.4 Whiteboard patch, if applicable.
Comment 6 Artem Tikhomirov CLA 2009-02-27 06:20:24 EST
In our code, we use EcoreUtil.getURI(diagramLinkStyle.getDiagramLink()), hence non-resolved proxies are good for us. However, resolved EObject won't harm in this case.

To me, special handling of that "always proxy" DiagramLink object is unfortunate, but reasonable effort, and treating it as a regular EMF object (e.g. processing them uniformly with ECrossReferenceAdapter) may cause performance issues. Hence, neither +1 nor -1, just 0 ;)
Comment 7 Michael D. Elder CLA 2009-02-27 08:29:56 EST
Hi Artem,
    So with a "0", does that mean you do not oppose the patch? ;)
Comment 8 Artem Tikhomirov CLA 2009-02-27 10:03:29 EST
>     So with a "0", does that mean you do not oppose the patch? ;)
Well, yes. Can't support nor can't object. My concern is performance, and once patch's applied we'll see if it has some grounds.
Comment 9 Alex Boyko CLA 2009-02-27 11:03:57 EST
Committed the fox for 2.2.
Comment 10 Anthony Hunter CLA 2009-02-27 11:42:46 EST
2.1.3 Patch looks good. Please commit to R2_1_Maintenance.

The version of the org.eclipse.gmf.runtime.notation bundles also needs to
change to 1.1.4 to indicate a change post GMF 2.1.3.
Comment 11 Alex Boyko CLA 2009-02-27 14:49:34 EST
Committed the fix for maintenance stream
Comment 12 Eclipse Webmaster CLA 2010-07-19 12:30:40 EDT
[GMF Restructure] Bug 319140 : product GMF and component
Runtime Diagram was the original product and component for this bug