Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 272227

Summary: Do not extend ImportOperation in AbstractExampleInstallerWizard
Product: [Modeling] EMF Reporter: Dave Steinberg <davidms>
Component: CoreAssignee: Dave Steinberg <davidms>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: Ed.Merks, marcelop
Version: 2.5.0   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 253483    
Attachments:
Description Flags
Fix none

Description Dave Steinberg CLA 2009-04-14 19:22:16 EDT 
AbstractExampleInstallerWizard.createZipImportOperation() creates an anonymous subclass of ImportOperation. But ImportOperation specifically says that clients may not extend it. The common usage of ImportOperation is to create it and run it in the same place.  With a bit of refactoring, we could do that, and then we could just close the zip file there.

That would be a better approach anyway, since the current design doesn't close the zip file if the operation isn't run (for example, if creating the project threw an exception).
Comment 1 Dave Steinberg CLA 2009-04-14 19:34:12 EDT 
Created attachment 131866 [details]
Fix

Here's a patch that removes the non-API usage of ImportOperation.

Basically, I've replaced createImportOperation(), createDirectoryImportOperation(), and createFileImportOperation() by installProject(), installProjectFromDirectory(), and installProjectFromFile(). Instead of just creating the ImportOperation, these methods actually create the project and run the operation as well (allowing the zip file to be closed afterwards, or any other cleanup that might be needed).  I've folded zip handling into installProjectFromFile(), since it doesn't really do anything else -- I couldn't see any justification for yet another a duplicated method, plus this way allows me to open the zip file once less.

All of the existing createImportOperation methods have been deprecated. The existing sequence of calls is preserved to continue to honour any overrides, but in the absence of any overrides ultimately they return null, which causes installExample() to use the new approach.
Comment 2 Marcelo Paternostro CLA 2009-04-16 11:48:25 EDT 
I've applied the patch and reviewed the changes. They look good!
Comment 3 Dave Steinberg CLA 2009-04-16 14:40:42 EDT 
Thanks Marcelo!

The fix is in CVS for EMF 2.5.
Comment 4 Dave Steinberg CLA 2009-04-23 02:56:16 EDT 
Fix available in HEAD: 2.5.0.I200904211800.
Comment 5 Dave Steinberg CLA 2009-06-25 02:49:20 EDT 
Fix available in HEAD: 2.5.0 (R200906151043).