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

Bug 295888

Summary: ProvUIProvisioningListener treats all repo changes as added metadata
Product: [Eclipse Project] Equinox Reporter: John Arthorne <john.arthorne>
Component: p2Assignee: Susan McCourt <susan>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3    
Version: 3.6   
Target Milestone: 3.6 M5   
Hardware: PC   
OS: Windows XP   
Whiteboard:

Description John Arthorne CLA 2009-11-23 11:04:25 EST 
Build: Latest p2 api branch as of 23-nov-2009.

ProvUIProvisioningListener#notify calls refreshItem(Object) when it receives OperationEndingEvent. The refreshItem method does the following:

repositoryAdded(new RepositoryEvent((URI) item, IRepository.TYPE_METADATA, RepositoryEvent.ADDED, true));

I.e., it treats the event as an added metadata repository. However OperationEndingEvent could indicate that a repository was *removed*, in which case it's probably doing the wrong thing here. Also, the changed repository could be an artifact repository in which case it may be reacting to the change inappropriately.

I didn't actually observe a bug, but happened to be tracing through the code and it looked odd. As I mentioned in another bug, OperationEndingEvent doesn't seem to have enough detail for a client to know how to react to it.
Comment 1 Susan McCourt CLA 2009-11-23 11:14:31 EST 
Right now this code is relying on the knowledge of what the receivers do with the information.  We refresh the view completely if a repo gets added or removed, and if one gets added, we update the selection in the combo.  This was kind of a stopgap to prevent the problem where everything blinks and selections get lost because of the underlying repo reference events.

But it does need to get cleaned up.  Originally I didn't use these events for artifact repos, but recently added them, so there does need to be a better description of what's happening, and probably this event should be recast as a "UserRepositoryEvent" or something like that.

I'm marking M4 for now...at a minimum the api needs to be cleaned up coming out of the branch.
Comment 2 Susan McCourt CLA 2009-11-23 19:45:18 EST 
As part of the cleanup in bug 295874, this code has moved around a bit.

The two things I was trying to accomplish here were:
1 - the code that triggers the operation end event needs to be able to say what changed.  I was trying to be agnostic about what kind of provisinoing operation might be involved and use an Object "item."  As you mention, using a RepositoryEvent rather than a URI would be more specific.  I ended up using an event in the method signature, and making it clear that the whole batching mechanism is in support of repo events, not any provisioning event.

2 - We still need to be able to treat some operations (in particular a repository load) as an add of that repository.  This is needed for the combo selection to remain stable when repos are added in the background.  So there is still a hack treats a load as an add, but at least it's more isolated now.

I'm going to mark this one fixed because the hack is more specific/localized, and also better explained in the code.
Comment 3 Susan McCourt CLA 2009-11-23 19:46:02 EST 
fixed.  If you are curious, the hack is now in session.loadMetadataRepository
Comment 4 Susan McCourt CLA 2009-11-24 13:05:17 EST 
had a better idea, the hack is now in the UI where it belongs.
Comment 5 Susan McCourt CLA 2009-12-01 12:57:04 EST 
changing milestone to M5.  The p2 api branch will be merged back into HEAD early in M5
Comment 6 Susan McCourt CLA 2010-01-26 18:49:07 EST 
verified on I20100126-0100, Win7 via source inspection.
Batching is done in the UI and clients can now determine:
- what event should be used as the "final" event
- whether the UI should update as a result of this event