Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 357417 - DiagramEditorInput should add IFile in getAdapter()
Summary: DiagramEditorInput should add IFile in getAdapter()
Status: CLOSED FIXED
Alias: None
Product: Graphiti
Classification: Modeling
Component: Core (show other bugs)
Version: 0.8.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 0.9.0   Edit
Assignee: Tim Kaiser CLA
QA Contact:
URL:
Whiteboard: Juno M2 Theme_round_offs
Keywords:
Depends on:
Blocks:
 
Reported: 2011-09-12 16:35 EDT by Hernan Gonzalez CLA
Modified: 2012-06-28 10:43 EDT (History)
1 user (show)

See Also:
michael.wenz: juno+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Hernan Gonzalez CLA 2011-09-12 16:35:20 EDT
Build Identifier: 20110301-1815

Currently the DiagramEditor and its DiagramEditorInput are rather hermetic... For example, I have a RCP application that can have several editors opened (Graphiti among them), and sometimes I need to find out the currently active project by asking the currently active editor (if it exists) for its current file. I wish to do that without knowing the concrete editors (perhaps someone adds another editor later, my code should work). A natural way is 

public IProject getCurrentProject() {
 // first test if a resource is selected, eg,  in the package explorer..
 // ...
 // elsewhere ask the active editor file
 IEditorInput ied = getWorkbench().getActiveWorkbenchWindow().getActivePage().getActiveEditor().getEditorInput();
 if( ied instanceof IFileEditorInput ) project = ((IFileEditorInput) ied).getFile().getProject();
 else {  
 IFile file = (IFile)(ied.getAdapter(IFile.class));
 if(file!=null) project = file.getProject();
}

I cannot do this with the DiagramEditorInput (and I have no alternative than to couple with Graphiti) because its adapter does not "adapt" to IFile, which it could easily do:

 DiagramEditorInput.getAdapter(Class adapter) : 
     ...
     else if (adapter == IFile.class) {
 	return getFile();
     } 

   public IFile getFile() {
	if(file==null) {
	file = GraphitiUiInternal.getEmfService().getFile(getUri(),(TransactionalEditingDomain)null);
	}
   return file;
   }

I'd prefer to save the Ifile as a field, because then it could also be set at construction time.

I implemented that, and it works for me.



  
    
  




Reproducible: Always
Comment 1 Hernan Gonzalez CLA 2011-09-12 16:39:19 EDT
Two additional little points:

1. In EmfService.getFile() is not clear whether the second argument can be null. In one case, it can (actually, it's never used!), in the other, it breaks:

My fix:
  public IFile getFile(URI uri, TransactionalEditingDomain editingDomain) {
   return getFile(uri, editingDomain != null ? editingDomain.getResourceSet() : null);
 }

2. It's really good idea that DiagramEditorInput stores the main URI as string instead as a "real" URI ? I wonder.
Comment 2 Tim Kaiser CLA 2011-09-20 06:33:28 EDT
I added the adapter for IFile for DiagramEditorInput. 
Also i cleaned up the interface and implementation for the EmfService.
The superfluous parameters were removed. The corresponding methods deprecated
and a new method introduced.
(Regarding your question: Probably the uri is stored as string since serialization needs a string.)
Comment 3 Michael Wenz CLA 2011-09-21 03:28:44 EDT
Verified and set the Juno flag

Another reason for storing the uri as a String is keeping the input object as lightweight as possible as it is stored for quite some time (used e.g. in the Eclipse history buttons in the toolbar). That's also the reason for not introducing the file member variable in the input object.
Comment 4 Hernan Gonzalez CLA 2011-09-21 15:58:04 EDT
(In reply to comment #3)
> Verified and set the Juno flag
> 
> Another reason for storing the uri as a String is keeping the input object as
> lightweight as possible as it is stored for quite some time (used e.g. in the
> Eclipse history buttons in the toolbar). That's also the reason for not
> introducing the file member variable in the input object.

Well, I'm not very convinced, I think the URI is lightweight, and that the need to convert it to string at saveState() is not enough reason: the method getUri(), which must to the inverse conversion is equally important, and I feel we are loosing some type safety. Just compare with the standard FileEditorInput class.

Actually, looking inside DiagramEditorInput, I'm getting more uncomfortable with some design issues (I don't think it should do the loading itself and store the resources tree inside it - and yes, i've seen the dispose method). But I'll probably comment in the forum.
Comment 5 Hernan Gonzalez CLA 2011-09-28 17:14:08 EDT
I'm even less satisfied with the addition of a method updateUri() to the DiagramEditorInput , I think the idea goes against the EditorInput concept.

Apart from this, I doubt that its inclusion in DomainModelWorkspaceSynchronizerDelegate.handleResourceMoved() is correct, because it's too tied to the scenario (not exclusive, nor even recommended) of the diagram and model in the same file. If the moved resource corresponds to a URI that is not that of the opened file, this calls for trouble. I doubt that the event of a  file renamed should be handled in that way.
Comment 6 Michael Wenz CLA 2012-04-11 10:37:19 EDT
Bookkeeping: Set target release
Comment 7 Michael Wenz CLA 2012-06-28 10:43:11 EDT
Part of Graphiti 0.9.0 (Eclipse Juno)