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

Bug 352685

Summary: Local feature IDs used in PackageImpl classes can be wrong
Product: [Modeling] EMF Reporter: Mirko Seifert <mirko.seifert>
Component: CoreAssignee: Ed Merks <Ed.Merks>
Status: CLOSED FIXED QA Contact:
Severity: normal    
Priority: P3 Flags: Ed.Merks: iplog+
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
Initial patch (add -reload option to Generator class) none

Description Mirko Seifert CLA 2011-07-20 19:06:30 EDT
Build Identifier: I20110613-1736

I have an Ecore model and a respective Genmodel. The order of the features within the two models differs. When I generate model code using the Genmodel, the resulting PackageImpl class is defect. It uses wrong local feature IDs in the getMetaclass_Feature() methods. As a result calls to getEStructuralFeature() return wrong features even if the correct feature constant is passed as an argument.

This is caused by method GenClassImpl.getLocalFeatureIndex() which assumes that the order of the features in the Ecore model is the same as in the Genmodel. If I replace:

return Integer.toString(getEcoreClass().getEStructuralFeatures().indexOf(genFeature.getEcoreFeature()));

with 

return Integer.toString(getGenFeatures().indexOf(genFeature));

the code is generated as expected and works fine.

Strangely, this can only be observed when generating the code headless using org.eclipse.emf.codegen.ecore.Generator. When I generate code manually in Eclipse this behavior is not present. Maybe the genmodel is reloaded in this case?

There is multiple options to resolve this issue:

1. Use the code mentioned above
2. Reload the genmodel also in headless mode (if this is performed when invoking the code generation manually)
3. Use the feature constants that are part of the generated Package class instead of the magic number returned by getLocalFeatureIndex()
4. Extend the Genmodel validator to detect cases where the order of the features in the ecore model does not match the order in the genmodel

I'd prefer option 3.

Reproducible: Always
Comment 1 Ed Merks CLA 2011-07-20 20:23:12 EDT
Generally when the model is changed, it must be reloaded...  

Of course there are many cases that are handled nicely by calling GenModel.reconcile, which the editor does, so you don't realize it's the case.

What you describe though is just one possible problem.  Another might be that you added or removed things or moved things between containers.  Reconcile would also handle those things while fixing just the case for ordered features wouldn't help with that.  So it seems better to think about calling reconcile in the headless case than to fix one of many problems that can turn up otherwise. 

I'm not sure if calling it could cause any problems. If not, we'd not need to provide a new option for it.  We do have a reload option already.  Why isn't that enough?
Comment 2 Mirko Seifert CLA 2011-07-21 03:42:38 EDT
I agree. Reconciling the genmodel in the headless case is probably the better choice. This way the code generation behaves the same as performed manually.

I can't find the reload option. Can you give me a pointer?

Thanks a lot!
Comment 3 Ed Merks CLA 2011-07-28 13:58:47 EDT
I think it's just the -reload option.  All the generator tasks support that, I think...
Comment 4 Mirko Seifert CLA 2011-07-28 14:03:42 EDT
I can't find the option in the source code of class 'org.eclipse.emf.codegen.ecore.Generator' and if a call the generator application with '-reload', I get:

org.eclipse.core.runtime.CoreException: Unrecognized argument: '-reload'
        at org.eclipse.emf.codegen.ecore.Generator$PlatformRunnable$1.run(Generator.java:277)

Maybe the reload option is implemented in the ANT tasks only?
Comment 5 Ed Merks CLA 2011-07-28 14:15:02 EDT
Yes, it's only in the ant tasks.
Comment 6 Mirko Seifert CLA 2011-07-28 14:17:51 EDT
Shall I try to prepare a patch that adds the 'reload' option to the Generator class?
Comment 7 Ed Merks CLA 2011-07-28 14:27:52 EDT
Sure.
Comment 8 Mirko Seifert CLA 2011-07-29 14:00:32 EDT
Created attachment 200612 [details]
Initial patch (add -reload option to Generator class)
Comment 9 Mirko Seifert CLA 2011-07-29 14:05:09 EDT
Hi Ed,

the attached patch works for me. It's a pretty simple one :)

The Ecore2Java Ant task does more to reload genmodels and it has more options (like 'create' or ' override'), but this is probably due to the fact that this task can also run on Ecore models only and creates a genmodel if necessary. The Generator application assumes that there is an existing genmodel.

Let me know if you need further information.

Mirko
Comment 10 Ed Merks CLA 2011-10-27 12:57:45 EDT
I changed the option to reconcile (because reload really does the full Reload... that you can use in the IDE) and committed that change for EMF 2.8.  Thanks for the excellent patch!
Comment 11 Mirko Seifert CLA 2011-10-27 14:12:27 EDT
You're very welcome :)

Thanks for intergrating the patch!
Comment 12 Ed Merks CLA 2011-11-22 05:26:44 EST
The changes are available in builds.