Bugzilla – Full Text Bug Listing
| Summary: | Stereotype garbage left in .uml file after removing profile (crash reason?) | ||
|---|---|---|---|
| Product: | [Modeling] Papyrus | Reporter: | Toni Siljamäki <toni.siljamaki> |
| Component: | Core | Assignee: | Christian Damus <give.a.damus> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P2 | CC: | cletavernier, give.a.damus, klaas.gadeyne, papyrus-bugs, ronan.barrett |
| Version: | unspecified | Flags: | cletavernier:
review+
|
| Target Milestone: | M7 | ||
| Hardware: | PC | ||
| OS: | Windows 7 | ||
| Whiteboard: | bbi deploy | ||
| Bug Depends on: | 408491 | ||
| Bug Blocks: | 432479, 436666, 458036, 458197 | ||
|
Description
Toni Siljamäki
Yep I agree Papyrus must inform the user if the .uml file contains profile data that is not resolvable. The user should be prompted if they want to remove this data or if they wish to leave it. There are reasons for the latter such as during an unsuccessful model profile upgrade. If the data is left in the .uml file papyrus should inform the user every time the model is loaded and prompt. This looks to me like a duplicate (or the same cause, at least) of bug 418542. The problems in that bug and this all stem from the incomplete implementation of profile switching, bug 408491. The profile switch as currently implemented actually corrupts the stereotype applications, or at least it leaves them as an orphaned "schema" that is not managed by a profile application, so the UML tools can provide no way to clean them up. Fixing/completing 408491 will make this problem go away. I'm sorry but it does not. Bug 408491 is a separate, isolated issue, to be fixed and tested standalone. It solves nothing wrt current and future bugs in this area. 1) and 2) above are new functionalities that MUST be introduced to ensure safety in future usage of Papyrus while scaling up, at the same time as it gets in place to warn about future bugs in this area when they happen. Ronan clarified this need pretty well. 1) and 2) should have regression test cases as well. Also... Having an automatic scanning/warning/cleaning function as proposed it means that Papyrus engineering will get immediate feedback a.s.a problems occur wrt stereotype garbage in the model file. I'm starting work, now, on a clean-up function to run on opening a model. Halleluja! :) Will it also solve multiple-applied-versions-of-same-profile ? I will walk through all .uml files manually, just in case... I plan something like this:
* on loading of a resource (the root resource of the model, or some controlled unit),
scan for zombie stereotype applications (those for which the profile definition is
either not available or not definition currently applied in the base element's
package context)
* the presence of zombies, if found, is reported to the user in a dialog. The options
available to deal with zombies, on a per namespace basis (there could be multiple
schema namespaces contributing zombies) the user are:
* delete all zombies
* try to migrate zombies to another profile definition (zombies are grouped by namespace)
* retain all zombies (do nothing for now)
* create problem markers to review zombies when convenient, later (these markers should
have an associated "quick fix" to implement the clean-up)
* in all cases, provide a "Don't report again for this resource" option. This decision can be
reverted in a new "Model Repair" property page for the resource and a preference page
to re-enable for all resources
(In reply to comment #6) > Halleluja! :) > > Will it also solve multiple-applied-versions-of-same-profile ? Yes, that would be solved implicitly. Unless you are referring to a scenario in which different packages have different versions of a profile applied, which is "normal" (if somewhat rare). It all sounds like a great solution. :) So... Next time, if stereotype/profile inconsistency happens again and is detected, we should "backup" all error log files. When new versions of profiles are deployed users get a pop-up window with lots of info in it (in Papyrus and in other tools) which often is confusing for users. A colleague just said that we often get calls from users asking what to do and which button to press. Our DSL profiles get updated quite frequently, as a natural part of the development process e.g. when adapting the DSL to new requirements. QUESTION: If a backwards compatible version of a profile is deployed, couldn't user models automatically and safely get updated in the background when the model is loaded, and users only get an "update completed" message to confirm with an advice to save the model if the update was successful? Right after the profile update your new clean-up function can be run and pop-up this new dialog if shit happened as a result of the profile update. (?) (In reply to Toni Siljamäki from comment #10) > > QUESTION: If a backwards compatible version of a profile is deployed, > couldn't user models automatically and safely get updated in the background > when the model is loaded, and users only get an "update completed" message > to confirm with an advice to save the model if the update was successful? I would suggest adding a "Don't ask again for this profile" button to the dialog, which would then enable automatic updates for that profile only. Please open a new bugzilla for this enhancement. > Right after the profile update your new clean-up function can be run and > pop-up this new dialog if shit happened as a result of the profile update. > (?) The implementation in progress would, as it happens, always run before the profile update (I think; I need to verify that). But it doesn't matter, because profile update does not lead to zombie stereotypes (unless they are in resources that are not loaded, but then the repair agent wouldn't see those, either). Bug 432479 opened on your request. In the case of a profile update, any unpredictable / weird thing can happen, like if there are profiles applied to profiles applied to ..., and special consideration to Ronans Comment 1 above, depending on existing bugs in Papyrus, or newly created bugs related to release of new Papyrus functions, etc. My point is: If this scanning & checking always is run before profile update, which I think it should, and there is a profile update right after, this scanning & checking must be run again after the profile update. ...since things may happen anytime in Papurus rutime, and we need to be sure that the model is consistent after any profile update. Ronan: Can you provide some additional wisdom here? I have pushed a new branch bugs/431953-stereo-repair with an implementation of automatic model repair for broken stereotype applications. It covers stereotype applications conforming to schemas that are not the current definition of any applied profile, in the following scenarios:
(a) the schema is simply not the currently applied profile version
(b) the schema is a version of a profile that is not applied at all (the profile
application was deleted but the stereotype instances remained somehow)
(c) the schema is not recognized at all (such as happens when the profile is
moved to another location such that EMF cannot resolve the EPackage)
The resolutions that are available in all cases (on a per-schema basis) are:
(1) Postpone: do nothing for now. The dialog will appear again when next
the model is opened
(2) Delete all stereotype instances conforming to the schema
(3) Create problem markers for each "broken" stereotype instance, attached
to the base UML element where that can be determined, to review at
the user's leisure
In cases (a) and (b) where Papyrus can match the XML namespace of the "broken" stereotype instances with some known profile version, another resolution is available:
(4) Apply the latest version of the matched profile to the model and migrate
the stereotype instances to it. This implies that if the model actually has
the profile applied, but it is not the latest version that is applied, then the
model will be upgraded to the latest version even for stereotype applications
that are not "broken". I think this is probably best for the user anyways,
besides that Eclipse UML2 has never supported migrating to any profile
version other than the latest anyways (and has no plans to support this)
In case (c) where Papyrus cannot match the XML namespace of the "broken" stereotype instances with any known profile, the user has the option to
(5) Choose some profile to apply to the model and attempt to migrate
the stereotype instances to it
Resolutions (4) and (5) both, similarly to the Switch Profiles action, can result in imperfect migration of stereotype instance data if the target profile definition does not accommodate everything from the older schema (i.e., it is an incompatible migration). As with Switch Profiles, any such mismatches are reported to the user (they imply data loss) and the user has the option of creating problem markers to review later.
See http://youtu.be/m9WLlPvmq2c for a demonstration (sorry, it's 18 minutes long; there are a bunch of scenarios to cover!).
A new ModelSet snippet detects whenever a resource is loaded in a Papyrus editor (the main model resource or a controlled unit) and scans it for "broken" stereotype applications. If any are found, they are reported in a Repair Stereotypes dialog that is popped up for the user to take action. In the case that multiple resources are loaded at once (such as controlled units) and are found to have problems, the dialog gathers all problems from all affected resources. However, this does mean that although the dialog may appear on first opening an editor, it may also appear again later as the user works with the model and causes new resources to be loaded. There's really no way around that: we can't scan a resource until it has been loaded.
I have decided not to implement the "Don't report again for this resource" option, in part because I think repairing stereotypes really demands user attention (even if just to postpone or create markers) and in part because it would be a confusing UI that lets a user reverse that decision (for a particular resource or for all resources?). I will entertain discussion on this point. :-)
The repairing of stereotypes is an undoable operation.
A follow-up on this branch (or on master, depending on when it is merged) will implement the quick-fix for the problem markers, which basically would re-launch the Repair Stereotypes dialog.
This branch so far has five distinct commits, as described below.
Commit 33884c5:
Some groundwork improving the ServicesRegistry and, in particular, how the ModelSet service is initialized. The ModelSet snippet that implements model repair needs to be able to get the context of the editor that is editing the model, when such editor is present. But, snippets run as soon as the model is loaded, which so far has been before the editor's ServicesRegistry is properly started. Thus, the ModelSet doesn't yet have its link to the ServicesRegistry provided by the auxiliary service that performs this function (the ServiceUtilsForResourceSetinitializer service).
That initializer service is now deprecated and removed, in favour of having the ModelSetServiceFactory do the association of the ModelSet to the registry, itself. However, that doesn't account for the many cases (both in the Papyrus application code and in test code) where a client of the registry injects an already-existing ModelSet into the registry. So, I introduce a new concept of service initializers: the IServiceInitializer interface is a mix-in for service factories that support the initialization of service instances that they did not create, but were injected into the registry. With a few adjustments, especially in some test framework bits, this seems to work (there are no test regressions).
Commit 6f5a8ea:
Implements the Repair Stereotypes model-set snippet, user interface, and repair actions. This includes a refactoring of the Switch Profiles infrastructure to re-use code for migration of stereotype instances as well as some UI bits.
Commit defdb02:
Adds JUnit tests to cover the various "broken stereotype" scenarios described above.
Commit 2128c2a:
The Repair Stereotypes dialog started popping up in execution of the all-tests suite, because of broken test models for the Extended Element Types framework. So, this commit fixes those models.
Commit 25058c6:
Fixes a corner case of failure to report (or exception instead of a proper Diagnostic) unmatched attribute and element features in the unknown-schema scenario.
Commit 89d4610:
Fixes attachment of problem markers to the base elements of stereotype applications from an unknown schema by parsing the AnyType contents for the base_Xyz reference.
(In reply to Christian W. Damus from comment #13) I recommend watching the demonstration video in Chrome at 1080p resolution and 1.5x speed. :-) I have pushed a new branch bugs/431953-stereo-repair2 that revises the core changes in the ServicesRegistry API detailed in comment 13. Commit a69ae2b: Revises commit 33884c5 to remove the new API, which took the form of an IServiceInitializer interface that confused the notion of what a service factory is. Now, when a POJO service instance is added to it, the ServicesRegistry looks for an adapter that can provide (externally) the IService lifecycle hooks for the POJO service. If such an adapter (per the Eclipse adapter mechanism) is found, then a new kind of registry entry is created for the pojo that delegates the service lifecycle to the IService adapter on the POJO's behalf. The ServiceUtilsForResourceInitializerService that previously attached the service registry to the ModelSet is still deprecated, but is now replaced by the service adapter mechanism described above. Commit 5ebf7f2: Essentially just a rebase of commit 6f5a8ea, which implemented the Repair Stereotypes snippet and UI. Also incorporates the minor fixes from commits 25058c6 and 89d4610. Commit b3df7e4: Essentially just a rebase of commit defdb02, which implemented JUnit tests for the stereotype repair back-end. Commit 1c7d260: Just a rebase of commit 2128c2a, which fixed the Extended Element Types tests to prevent the stereotype repair dialog popping up to hang the tests. I've merged the latest master into the bugs/431953-stereo-repair2 branch. Hopefully we can complete code review and merge it back into master this week! > Commit a69ae2b: > Revises commit 33884c5 to remove the new API, which took the form of an IServiceInitializer interface that confused the notion of what a service factory is. Now, when a POJO service instance is added to it, the ServicesRegistry looks for an adapter that can provide (externally) the IService lifecycle hooks for the POJO service. If such an adapter (per the Eclipse adapter mechanism) is found, then a new kind of registry entry is created for the pojo that delegates the service lifecycle to the IService adapter on the POJO's behalf. Looks good to me Thanks, Camille! The changes are merged into master and pushed. The fix for bug 436666 comment 47 addresses what surely was the most common (if not the only) case of dangling stereotype applications (the "stereotype garbage" referenced in this bug) in Stereotype Repair. The PW protected attachment in Bug 436666 can be used for testing. https://bugs.eclipse.org/bugs/attachment.cgi?id=250095 The NWA model in this test case have been updated many time after this bugzilla was closed as fixed, but the problem still remain: The model attached to Bug 436666 contains stereotype garbage / "zombies" both before-and-after the model is opened and saved and after profile upgrade. No clean-up function is doing its tricks. (testing on latest nightly Luna) (In reply to Toni Siljamäki from comment #20) > The model attached to Bug 436666 contains stereotype garbage / "zombies" > both before-and-after the model is opened and saved and after profile > upgrade. These are now deleted by Stereotype Repair (if the dialog's default repair actions are accepted). > No clean-up function is doing its tricks. (testing on latest nightly Luna) Did you test with today's build #300? [1] That is the first build that includes the fix for bug 436666, which enhances the Stereotype Repair function to clean out dangling stereotype instances. [1] https://hudson.eclipse.org/papyrus/view/Luna/job/Papyrus-Luna/300/ > Did you test with today's build #300? [1] That is the first build that includes the fix for bug 436666, which enhances the Stereotype Repair function to clean out dangling stereotype instances. > > [1] https://hudson.eclipse.org/papyrus/view/Luna/job/Papyrus-Luna/300/ Luna update sites (.../papyrus/nightly/luna) are still based on the Shared Hudson instance, so that would rather be https://hudson.eclipse.org/hudson/job/papyrus-trunk-nightly/3713/ (In reply to Camille Letavernier from comment #22) > > Luna update sites (.../papyrus/nightly/luna) are still based on the Shared > Hudson instance, so that would rather be > https://hudson.eclipse.org/hudson/job/papyrus-trunk-nightly/3713/ AH, good point. Well, now that a build including the fixed has been published on every channel, I shall resolve the bug again to prompt verification. Please, it is important when reopening a bug that fails validation to indicate which build specifically was tested so that we can avoid confusion about what changes are expected to be in the build. The cleaning out of "zombies" (garbage) described in Comment 7 has been fixed, at least what can be tested using the teste case attached to bug 436666. Just verified it on latest Luna & Mars nightly. As discussed in Comment 22 and Comment 23, are there still remaining things that needs to be solved and verified? I put this one as VERIFIED, but pleae reopen this bug if you have oher test/error cases that you need to solve. |