Bugzilla – Full Text Bug Listing
| Summary: | Generated EMF project leaks implementation details through default factory | ||
|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Alex Blewitt <alex.blewitt> |
| Component: | Core | Assignee: | Ed Merks <Ed.Merks> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | aforsell1971, csabakoncz, darrel.karisch, eclipse-bugzilla, jawr, njbartlett, sebastian.zarnekow, stefan.bosshard.4, stepper |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
If you want to change the eINSTANCE to be initialized using some other pattern, that's fine, but eliminating the eINSTANCE altogether isn't an option we intend to provide. There can only be one instance of the package and there will necessarily be a constant that can be used to refer to that instance. (In reply to comment #1) > If you want to change the eINSTANCE to be initialized using some other pattern, > that's fine, but eliminating the eINSTANCE altogether isn't an option we intend > to provide. There can only be one instance of the package and there will > necessarily be a constant that can be used to refer to that instance. Ed, could you please explain why not. Alex's request is very reasonable - there are much better ways to obtain the factory instance than through a static field on an interface. I understand that some downstream tools may depend on the eINSTANCE field? However I am not using any of those downstream tools. I currently just want to use basic EMF without exposing implementation code from my bundles. I'm just looking ahead and assuming that next someone will notice that the EPackage has such an instance field as well and ask that to be removed too, right? Or not? (In reply to comment #3) > I'm just looking ahead and assuming that next someone will notice that the > EPackage has such an instance field as well and ask that to be removed too, > right? Or not? I would think so. I don't yet understand what the EPackage stuff is for, but again I would expect there to be better ways to obtain an instance of the interface. Given there are well over a hundred enhancement requests, it's highly unlikely I'll ever spend time on something for which I don't see the compelling value. My take is that there does need to be one package instance. That instance has exactly one factory. So we have XyzPackage.eINSTANCE.getXyzFactory(). Is that a "leak" as well? If so, then we're back to my earlier premise. If not, I don't see how XyzFactory.eINSTANCE is a bad thing. It's a good thing because it's more efficient to access... (In reply to comment #5) > Given there are well over a hundred enhancement requests, it's highly unlikely > I'll ever spend time on something for which I don't see the compelling value. > > My take is that there does need to be one package instance. That instance has > exactly one factory. So we have XyzPackage.eINSTANCE.getXyzFactory(). Is that > a "leak" as well? If so, then we're back to my earlier premise. If not, I > don't see how XyzFactory.eINSTANCE is a bad thing. It's a good thing because > it's more efficient to access... The value is as follows. If the interfaces can be kept "clean", i.e. without any references to EMF-generated implementation classes or to EMF itself, then the APIs can be used standalone, for example in a memory constrained environment. In this scenario, normal hand-coded implementation classes could be used. Note, I am happy for the existing behaviour to remain the default. I would just to have this as an option. Believe me I understand the problem of bandwidth to make changes. If Alex or I were able to create a patch along these lines, would you accept it? My fear at the moment is you appear negative to the whole idea, so it would not be worth our time to produce a patch. Right, as I said in the description, what I was suggesting was an option to disable its creation, but that it would be false by default. (Or correspomdingly could be called "create factory instance" that defaults to true) Even if the factory were defined elsewhere (ie not on the interface) as a static class would be better - since then you can expose a pure Java interface. Maybe for OSGi you could create a DS registration. I understand that you don't want to prioritize this over other work, but a help wanted tag would be a way of encouraging others to do the work. I'm sure either Neil or myself would step up. Well, I'm really not sure how this pans out, but certainly I don't have a huge aversion to it if someone else finds it important enough to prototype a complete solution. This seems just a purist concern in which no one else is actually willing to invest time.. |
Build Identifier: 3.6.1 I'm looking at EMF and building an OSGi bundle which exports EMF generated/managed interfaces, but does not expose any of its internals. There appears to be one place where the implementation is leaked through the use of the 'default' factory. Specifically, regardless of the interface options, it's not possible to avoid creating an (e)INSTANCE unless interfaces are skipped. The problem is that regardless of whether an eINSTANCE or INSTANCE is chosen, it refers to the impl package which I'm trying to hide inside the bundle itself. The solution seems to be to have an option not to generate the default (e)INSTANCE in the 'FactoryClass.javajet' package. In this case, perhaps a SCR component could be used to register the (internal) instance of the *Factory under the public interface. When run in an OSGi-DS equipped runtime (such as Eclipse) the service would be made available automatically. I propose the creation of a new option in the model generator that encases the generation of the whole (e)INSTANCE predicated on a 'Default factory in interface' setting, which can thus be disabled. Note that the PackageClass.javajet, which uses genPackage.getQualifiedEFactoryInstanceAccessor(), would need to be modified to refer to the internal implmenetation package instead of via the public interface for this to work. Reproducible: Always Steps to Reproduce: Use this genmodel to see an ecore generated file with references from the public interface in the public package (and assuming an .internal. convention from other systems) <?xml version="1.0" encoding="UTF-8"?> <genmodel:GenModel xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:genmodel="http://www.eclipse.org/emf/2002/GenModel" modelDirectory="/TestEMF/src" modelPluginID="TestEMF" modelName="Test" modelPluginClass="" rootExtendsInterface="" suppressEMFTypes="true" suppressEMFMetaData="true" codeFormatting="true" importerID="org.eclipse.emf.importer.ecore" complianceLevel="6.0" copyrightFields="false" interfaceNamePattern="I{0}" classNamePattern="{0}"> <foreignModel>Test.ecore</foreignModel> <genPackages prefix="example" disposableProviderFactory="true" classPackageSuffix="internal.implementation" ecorePackage="Test.ecore#/"/> </genmodel:GenModel>