Bug 131440 - [Model Decoration] Problems with current support for model element decoration
Summary: [Model Decoration] Problems with current support for model element decoration
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P1 normal (vote)
Target Milestone: 3.2 M6   Edit
Assignee: Michael Valenta CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2006-03-11 18:48 EST by Michael Valenta CLA
Modified: 2006-03-17 12:02 EST (History)
3 users (show)

See Also:


Attachments
Patch with proposed API changes (108.94 KB, patch)
2006-03-16 10:42 EST, Michael Valenta CLA
no flags Details | Diff
Updated path (108.85 KB, patch)
2006-03-17 10:44 EST, Michael Valenta CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Valenta CLA 2006-03-11 18:48:14 EST
The following problems with the model decoration API have been identified:

- The SynchronizationStateTester currently only supports decoration of known state on model elements (i.e. Incoming and Outgoing). However, some repository vendors may have custom state that should appear on model elements. 

- Repository tooling may allow the user to configure which state is decorated. The specification of the decorated state for a repository provider is currently declarative and is fixed.

- The current API relies on the use of a Subscriber but only a small portion of the subscriber funtionality is required. It would be better to define an API that identifies the minimum API required to perform model decoration. We could then provide a bridge for those repositories that provide a subscriber so they would not need to do additional work.
Comment 1 Michael Valenta CLA 2006-03-16 10:42:26 EST
Created attachment 36419 [details]
Patch with proposed API changes

Here's the patch with the proposed changs. It includes the new API, cjhanges to CVS to decorate using the new API and changes to JDT/UI to support working set label updates.
Comment 2 John Arthorne CLA 2006-03-16 19:40:14 EST
Meta-comment: it seems like a lot of new API to be adding without any chance of getting feedback before it's set in stone...

ITeamStateDescription
  getProperties(): getPropertyNames() would make more sense. I would expect getProperties() to return values rather than keys.  I guess this is to match the API of IDecorationContext?  If so, should this interface extends IDecorationContext?

SubscriberTeamStateProvider
 - There are references in the comments to DecoratedStateProvider (rather than ITeamStateProvider)
 - should the protected methods and dispose() be final?  If not, can subclasses override or extend (must they call super dispose, for example)?

TeamStateChangeEvent
 - Can clients subclass it? Typically events like this aren't subclassed because listeners won't know what subclass they are getting

TeamStateProvider
 - More doc references to decorated state provider
 - Can clients subclass?

SynchronizationStateTester
 - I don't understand how this class is intended to be used - I'll have to ask you about it
 - This has all kinds of references to decorated state provider instead of team state provider.
 - Can it be subclassed? Presumably so. Should it be abstract?
 - It's odd that clients can directly instantiate this, since it's stateless (all instances will be the same).
Comment 3 Michael Valenta CLA 2006-03-17 10:44:05 EST
Created attachment 36493 [details]
Updated path

Here's the updated patch with the changes suggested by Boris and John. In response to the concern of the size of the API, I agree that it is more than usual for this late in the cycle but we didn't get any feedback from clients until post-M5. Althought the API change is not as small as I would like, I am comfortable with the API and have structured it is such a way to solve the issues that were identified by clients and to allow as much flexibility as possible in the future. Also, as a proof of the API, I have also implemented a client (working sets in the packages explorer update properly to team state changes) and a provider (CVS decorates logical model elements with the dirty state and the tag).

Do I have the go-ahead to add this API?
Comment 4 Mike Wilson CLA 2006-03-17 11:28:01 EST
+1. ok to proceed.
Comment 5 Michael Valenta CLA 2006-03-17 12:02:23 EST
Thanks. Fix released to HEAD.