Bugzilla – Full Text Bug Listing
| Summary: | [prov] Need reliable trigger for profile modification | ||
|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | Susan McCourt <susan> |
| Component: | Incubator | Assignee: | Simon Kaegi <simon_kaegi> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | john.arthorne, pascal, simon_kaegi |
| Version: | 3.4 | ||
| Target Milestone: | 3.4 M4 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
| Bug Depends on: | |||
| Bug Blocks: | 197701, 208047, 211939 | ||
|
Description
Susan McCourt
Yep. I wouldn't use the commit/rollback events to determine whether things have changed. These are the borders on the engine transaction and as I see it should scope visibility of changes done during a perform. e.g. we shouldn't be able to see the changes made by the engine externally until the pseudo-transaction is either committed or rolledback and the perform call returns. This seems like a good thing to re-visit in M4. We don't have a particularly good listener story here for discovering what changes (if any) occurred as a result of stuff done in the engine. The first problem here is the trigger used by the UI. Instead of being the commit event (which we agreed to use for M2), it should be the result of some computation done by various touchpoints. In fact the UI should not even have to worry about this and the touchpoint should make use of the callable User UI service (bug #206903) to prompt the user. However this problem raised an interesting question: - The engine is being used to perform non-modifying operations (for example sizing). - Is this the right thing to do? - Should the engine always send a commit / rollback event, or should that somehow be part of the phase set being passed in. If touchpoints trigger a restart through the callable service, we just need to make sure the requests to restart are accumulated so that it only happens once, after everyone is configured. Regardless of that particular use case, I do need to know which event I would listen to in order to discover that a profile has been modified. Other profile-watching clients: - the admin UI profiles view - the user UI "what's installed" view etc... I still think that the commit/rollback events are appropriate to update the content of views. Maybe we need the events to carry the ProfileDelta instead of just the operations. I'm not going to reenable the apply/restart dialog until we have an answer here, so marking this as blocking bug #208047. This is starting to bite me in other places, so for now, I am going to add a public method to TransactionEvent to return the phase set. At least I can determine when I'm getting this event only because of my sizing computations. Obviously we need a better answer and I've marked the code with TODO and refs to this bug. Renaming this bug to reflect the real issue. I believe Simon is planning on working on this. Tentatively marking for m4. I've checked in some intial changes to allow us to use the various ProfileEvents to "reliably" detect the sorts of changes you're looking for (...without listenting for commit and trying to interpret the phaseset etc.) When I tried in the Admin UI with the code in HEAD it didn't automatically refresh the profile after installing or uninstalling the SDK. I played around with StructuredViewerProvisioningListener and got the behaviour that I think is right but I'm reluctant to provide a patch in that area. Susan, I'm hoping you can take a look and help validate we're getting the right events now. I modified the UI listeners (StructuredViewerProvisioningListener and the profile change listener in ProvUIActivator). From an admin UI point of view, when install root properties aren't involved, things are looking good. But I think we have some timing issues with install roots, and possibly a bigger problem. Note this isn't a problem with the UI listeners per se, but with the changes made to the underlying events. 1 - We have a timing problem with install. Both SimpleDirector.install and ProvisioningUtil.performInstall mark the install roots after the engine has returned from doing its work. Now that the profile changed event is fired from the engine, the viewers are notified before the director marks the install roots. So there is nothing to be shown in the end user UI because it updates before the roots are marked. To solve this problem, what is needed is a way to explicitly tell the engine to mark the roots during an install. 2 - It also seems like the install roots aren't being marked at all, and I'm not sure why. If I bring up the end user UI after an install, I don't see anything, and if I check the property page for the installed IU, it doesn't have the root marker set. I'm starting to believe that we might not want to change the way these events are handled (underneath or in the UI) until we have a way to explicitly describe a profile delta to the engine (see bug #206077). We need to be able to ask the engine to mark the install roots on an install so that the properties get set before the event fires. If we back out of the event changes, I still want a chance to change the UI listeners for M4. In particular: - the UI should refresh on Profile.CHANGED to be paranoid and to catch the install root change. - it should probably refresh more generally on the commit event rather than isolate the change to a particular profile element (this might be the cause of bug #211138, although we didn't always observe that bug). If we want to keep the event changes, we need to ensure that Profile.CHANGED is received after the install roots are updated in ProvisioningUtil.performInstall(...) or SimpleDirector.install(...). In this case the currently released UI listeners are doing the right thing. We should be marking the roots in the profile before doing the Engine.perform(). Until the perform is committed the profile is a disconnected snapshot so we can safely make changes to its properties and the engine will only reconcile the differences when it commits the changes. The end user UI was not updating properly on install/uninstall. The problem is that under the new mechanism, profile instances represent a snapshot in time of the profile, so anyone who wants to have a "relationship" with a profile really needs to hang onto the id. Up to now, the UI has happily held onto profile instances all over the place. The problem was most noticeable in the end user UI becauses the "installed features" page uses a profile element which caches a profile instance as the root of its viewer model. Refreshing the viewer (which we do on profile change), requeries the profile, but the profile was the stale instance. To solve this, I hacked a custom refresh() method into the viewer created by the end user UI. It retrieves the profile by id on every refresh and resets its root model element's profile. This seems to solve the problem. I'm worried about other places where the UI is holding onto the profile instances, but many of those places (update/install/uninstall dialogs) do not stay up during a profile change, so this fix may be good enough. I tried agressively replacing profile instances whenever a model element is requested, but this broke the mechanism by which the deferred/lazy content provider finds its model element when it asynchronously retrieves children. In other words, always regenerating the instance can break all of the views whereby expanding the content of a profile loses its content in the viewer. I think this hack is safe enough for M4. I can look at a more thorough solution once we know for sure where the profile implementation is going (for example, will profile instances always be a snapshot in time even after we get a profile delta mechanism?) Marking FIXED. We can track any changes that occur as a result of ProfileDeltas in bug 206077 |