Community
Participate
Working Groups
The method org.eclipse.uml2.uml.util.UMLUtil.getNamedElement(ENamedElement, EObject) doesn't support ENamedElements that should resolve to a uml::Package. The method assumes that all EPackage correspond to a uml::Profile This isn't true in at least two cases: - Since UML 2.4, a profile hierarchy may contain packages (IIRC, Profiles could only contain Profiles before that) - Packages may be statically generated (Without any relationship to a profile) The lack of support for Packages in this method makes it difficult to identify datatype instances in Profiles (Where Classes are converted to EClasses, and stereotypes applications can own instances of these - these datatype instances are not stereotype applications, they are not UML Instances, but they are still related to the UML Profile; however, we can't really distinguish them from standard EObjects unrelated to UML). In the past, this also (indirectly) caused performance issues when manipulating stereotypes located in such packages, but this is now mitigated by the cache added in Bug 501740 (Although I suspect the initial lookup would still be slow, but I've not verified that) Note: the real performance issue is caused by the LOOP code in getProfile(org.eclipse.uml2.uml.util.UMLUtil.getProfile(EPackage, EObject)) that iterates over the entire resource set, but IIRC, this was required for some specific applications (Was it CDO?) and couldn't simply be removed. So this ticket is about making best effort to avoid ever entering that LOOP when a matching Package can be found.
Created attachment 272185 [details] Proposed patch Proposed patch (Excluding tests)
This is an "internal" method that was originally designed to be used (only) to look up profile definitions. At the risk of endorsing the notion that this method is "API", I'll take a look at the proposed patch to see if it can be enhanced without any adverse side-effects.
Camille, I looked at the patch and have some feedback. From what I can tell, the methods that were designed to look up profiles have been re-purposed for use with packages and then referenced by lookup method. Unfortunately, what those methods do for profiles doesn't apply to packages. Specifically, Ecore definitions for packages don't get nested in annotations the way those for (dynamic) profiles do and packages don't get registered in the ePackageNsURIToProfileLocationMap registry the way profiles do. The main lookup method already falls back to looking for packages in the event that a matching profile (either nested in an annotation in the case of a dynamically defined profile or in the registry if a statically defined profile) isn't found, so I'm wondering what else it needs to do in order to fully support resolution of packages...
For OCL, where I try to use just the UML model, I find the EPackages something of a distraction. They are an implementation artefact that 'corrupts' some of the original UML model relationships. It is probably better if a bidirectional UML2Ecore and Ecore2UML map is maintained (it's probably there already), then the Ecore lookup arguments can be mapped to UML where the relationships can be accurately traversed by a more rational lookup method before converting the result back to Ecore.
Hi Kenn, To give a little bit more context: in Papyrus, we use this method to determine whether an EObject is a Stereotype Application, an instance of a DataType (From a Profile or Library), or is unrelated to the UML Model (e.g. a Diagram, ...) Typically, our profiles also come with a standard library that is located outside of the profile, but contains UML Types that can be instantiated in Stereotype applications (e.g. DataTypes or Classes) So, we register the Package via the org.eclipse.uml2.uml.generated_package extension point, then for an EObject that we find in the model, we use UMLUtil.getNamedElement(EClass, EObject) to find whether it is an instance of Stereotype, Class, DataType, or nothing related to UML. Unfortunately, UMLUtil.getNamedElement() is unable to resolve Packages registered via the generated_package extension point (If it finds an EPackage, it will always try to resolve it to a Profile). Note: the Package is not located in a Profile, we're not looking for a Root profile owning the Package. It is a standalone Package (That may be imported by a Profile); I don't think this case is supported at all at the moment. An alternative to my patch would be to provide optimized (performance-wide) utility methods to identify the nature of an EObject: - UML Element (That's easy) - Stereotype Application - DataType (Or Class) instance (Typically owned by a Stereotype Application; I don't think such instances could live anywhere else. But more generally: any instance of an EClass that can be mapped to a UML Classifier other than a Stereotype) - Unrelated to UML
(In reply to Camille Letavernier from comment #5) An Ecore2UML lookup should solve all this: > - UML Element (That's easy) > - DataType (Or Class) instance similarly easy - it's a UML element > - Unrelated to UML easy - the lookup returns null > - Stereotype Application This is the nightmare. OMG neglected to specify a StereotypeApplication class forcing everyone into the world of XMI magic. For OCL's Pivot normalization of UML, there is a StereotypeApplication class called ElementExtension. The UML2 project could make the use of the UML model very much easier by ensuring that the magic DynamicEObject is converted to a StereotypeApplication in memory and converted back to a DynamicEObject when saved. With this in place, the OCL support might never have to look at the embedded Ecore ever again.
> similarly easy – it's a UML element No, it's not, that's the trick: DataType instances have 0 relationship to the UML Metamodel. You define a uml::DataType in a UML Package/Model, convert it to an EClass via the UML2Ecore generator, then instantiate this EClass: the resulting EObject is not a UML instance DataType instances are really similar to Stereotype Applications, except that they are not related to a Profile, but to a Package. Such DataType instances exist when you define a Composition Association between a uml::Stereotype and uml::DataType (Or Class), then apply a stereotype: the Stereotype Application may own instances of the DataType (Or Class), via this custom Composite Association (Or simple Property). Neither the Stereotype Application nor the DataType instance can be related to UML without a valid Ecore-to-UML mapping (And although UMLUtil contains the API to do this mapping, it currently fails in the case of the DataType, and succeeds in the case of the StereotypeApplication - assuming the profile was properly generated, and extension points properly populated)
(In reply to Camille Letavernier from comment #7) > No, it's not, that's the trick: DataType instances have 0 relationship to > the UML Metamodel. Ah! OK yet another omission from the OMG UML metamodel that Eclipse OCL evaluation over UML Instances supports using a UMLElementExtension. The org.eclipse.ocl.pivot.uml.internal.library.UMLElementExtension object is not dependent on the OCL Pivot model; only a few helper methods are. UMLElementExtension could usefully be promoted to the UML2 project. UMLElementExtension extends DynamicEObjectImpl and adds: protected final org.eclipse.uml2.uml.@NonNull Element umlElement; protected final org.eclipse.uml2.uml.@NonNull Stereotype umlDynamicStereotype; protected final org.eclipse.uml2.uml.@NonNull Stereotype umlStaticStereotype;
(In reply to comment #5) > So, we register the Package via the org.eclipse.uml2.uml.generated_package > extension point, then for an EObject that we find in the model, we use > UMLUtil.getNamedElement(EClass, EObject) to find whether it is an instance of > Stereotype, Class, DataType, or nothing related to UML. This is a mis-use of the extention point and lookup method as they are currently defined; both were designed to work with profiles (and indeed, the extension element you use to register a UML element is called "profile"). I could imagine an enhancement to the extension point that adds optional 'library' and 'metamodel' elements which could be used to register the correlation between library or metamodel and its generated Ecore counterpart; such an enhancement could then be leveraged to do some of the additional lookups you're asking for (see below). > - Stereotype Application The UML2 project itself uses either UMLUtil#getStereotype(EObject) or UMLUtil#getBaseElement(EObject) for this purpose. > - DataType (Or Class) instance (Typically owned by a Stereotype Application; I > don't think such instances could live anywhere else. But more generally: any > instance of an EClass that can be mapped to a UML Classifier other than a > Stereotype) In order to achieve this, a more appropriate registration mechanism (like the enhancement I suggested above) should be implemented. Then, we could rework the current lookup method(s) to find any UML elements in profiles, metamodels, or libraries that have been registered via the extension point. It would, however, probably need to still be optimized for the profile use cases, in the interest of performance (since that's what they were designed to do in the first place).
Created attachment 272219 [details] Two plug-ins (Example Profile/Library + Test case)
Sounds good to me To clarify the use case, I attach two projects (One with a Profile + Library as we define them today in Papyrus, and a JUnit Plug-in Test case showing what I expect - two of them are failing without my patch, and working with it; but we can consider a different approach. The test is mostly there to highlight the various cases)
(In reply to comment #11) > To clarify the use case, I attach two projects (One with a Profile + Library as > we define them today in Papyrus, and a JUnit Plug-in Test case showing what I > expect - two of them are failing without my patch, and working with it; but we > can consider a different approach. The test is mostly there to highlight the > various cases) Thanks, it's great to have the use cases more clearly identified. Contributions towards implementing the alternative approach (since I don't think it's appropriate to pursue an approach that reinforces improper use of the existing extension point and look-up mechanisms) are most welcome!
Ok, I'll have a look at this. Would it make sense to add a "package" element to the existing (org.eclipse.uml2.uml.generated_package / dynamic_package) extension points? Or should I go for a completely different one? The extension point itself doesn't seem specific to profile (Except for the fact that it currently only accepts a "Profile" element)
New Gerrit change created: https://git.eclipse.org/r/115612
> New Gerrit change created: https://git.eclipse.org/r/115612 This gerrit adds a 'package' choice to the generatedPackage/dynamicPackage extension points, and populates a distinct repository. In that scenario, when trying to resolve a Package from an Ecore URI, we will check both repositories (As a Package may be either a Package or a Profile - or even a Model, but I don't think it would make sense to add a third choice there) I've only slightly changed my previous patch to account for the new extension point, so it may still find a Profile which has been registered as a Package, or a Package registered as a Profile (i.e. both extension points are interchangeable). I may add proper checks if needed. Another potential change would be to define the new (Package) registry as a super-set of the existing (Profile) one, since all Profiles are also Packages. Currently, profiles are stored in the profile registry, and Packages/Model are stored in the package registry (Assuming the extension points were correctly used; since we never load the URI to check if we're actually registering Profiles or Packages)
(In reply to comment #15) > > New Gerrit change created: https://git.eclipse.org/r/115612 Thanks, Camille, I'll take a look as soon as I am able. > Another potential change > would be to define the new (Package) registry as a super-set of the existing > (Profile) one, since all Profiles are also Packages. Currently, profiles are > stored in the profile registry, and Packages/Model are stored in the package > registry (Assuming the extension points were correctly used; since we never load > the URI to check if we're actually registering Profiles or Packages) The need to retain a separate, dedicated repository may depend on the performance implications. The two repositories should probably be treated conceptually as superset and subset; as such, it probably makes sense during plug-in initialization to dump all of the registered profiles (via the 'profile' extension element) into both repositories so that the lookup code only needs to iterate through one of them (the superset) when looking for packages.
(In reply to comment #16) > (In reply to comment #15) > > > New Gerrit change created: https://git.eclipse.org/r/115612 > > Thanks, Camille, I'll take a look as soon as I am able. I added some comment and requested that the contribution be re-based against the 'master' branch.
New Gerrit change created: https://git.eclipse.org/r/115671
> I added some comment and requested that the contribution be re-based against the 'master' branch. Were there actual comments, in addition to rebasing? If so, they don't appear on Gerrit. So for now, I've just rebased the commit
(In reply to comment #19) > Were there actual comments, in addition to rebasing? If so, they don't appear on > Gerrit. So for now, I've just rebased the commit Yes, at least I tried to add them (I'm new to Gerrit so I may have done something wrong). I'll take a second look and include my comments here in case they don't show up in Gerrit.
(In reply to comment #20) > I'll take a second look and include my comments here in case > they don't show up in Gerrit. Here are the comments I just posted: plugins/org.eclipse.uml2.uml/schema/dynamic_package.exsd Line 21: I don’t think the dynamic registry needs to be expanded to handle packages, does it? plugins/org.eclipse.uml2.uml/src/org/eclipse/uml2/uml/UMLPlugin.java Line 65: I think it might be better to keep the original constructor and add the new one with the extra agrument. Line 79: I think this would be better done as two cases, 1) if profile, add it to both registries and 2) if package, add it to just the package registry. plugins/org.eclipse.uml2.uml/src/org/eclipse/uml2/uml/util/UMLUtil.java Line 11575: This doesn’t apply to generic packages, as they don’t have annotations as containers the same way (dynamic) profiles do. Line 11601: Per my comment on the plugin class, if profile registrations are also added to the package registry, we only need to check one of them (the general one) here. Line 11627: This doesn’t apply generally to packages as they don’t get annotated with this annotation the way profiles do.
Gerrit change https://git.eclipse.org/r/115671 was merged to [master]. Commit: http://git.eclipse.org/c/uml2/org.eclipse.uml2.git/commit/?id=f9fb7e8e9e0a00e2630d988830d1c41fe4fee201
The changes have been committed/pushed tothe 'master' branch in git and will be available in the next integration build.
An integration build containing the changes is now available.
The changes are available in UML2 5.4.0 (as part of the simultaneous Eclipse Photon release).
