Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 209294 - [cm] need an event adapter
Summary: [cm] need an event adapter
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Compendium (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 M4   Edit
Assignee: Chris Aniszczyk CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 202278
Blocks: 162773
  Show dependency tree
 
Reported: 2007-11-08 22:39 EST by Chris Aniszczyk CLA
Modified: 2008-06-04 15:26 EDT (History)
2 users (show)

See Also:
tjwatson: review+

Attachments
org.eclipse.equinox.cm.patch (7.51 KB, patch)
2007-11-12 17:23 EST, Chris Aniszczyk CLA 		no flags Details | Diff
mylyn/context/zip (991 bytes, application/octet-stream)
2007-11-12 17:23 EST, Chris Aniszczyk CLA 		no flags Details
patch (8.13 KB, patch)
2007-11-13 09:25 EST, Thomas Watson CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Aniszczyk CLA 2007-11-08 22:39:52 EST 
In bug 202278, we removed the cm event adapter. The cm event adapter belongs within the actual CM implementation.
Comment 1 Chris Aniszczyk CLA 2007-11-12 17:22:15 EST 
I'll take it.
Comment 2 Chris Aniszczyk CLA 2007-11-12 17:23:18 EST 
Created attachment 82712 [details]
org.eclipse.equinox.cm.patch

Adds an event adapter for configuration admin.
Comment 3 Chris Aniszczyk CLA 2007-11-12 17:23:19 EST 
Created attachment 82713 [details]
mylyn/context/zip
Comment 4 Chris Aniszczyk CLA 2007-11-12 17:25:48 EST 
Tom to review.
Comment 5 Simon Kaegi CLA 2007-11-12 23:31:24 EST 
At first glance the interaction between the tracker and configuration event generation in ConfigurationEventAdapter doesn't look thread-safe. This was probably also the case when the adapter lived in EventAdmin. ;)

We should also consider making the event admin import optional.
Comment 6 Thomas Watson CLA 2007-11-13 09:25:56 EST 
Created attachment 82759 [details]
patch

Here is a patch that makes the event package optional.  I also make the access to the eventAdminTracker thread-safe.  Is this what you were referring to Simon?
Comment 7 Chris Aniszczyk CLA 2007-11-13 09:58:25 EST 
I guess that works ;)
Comment 8 Simon Kaegi CLA 2007-11-13 11:08:52 EST 
Pretty much. Thanks guys.

I also wanted to make sure the event adapter was registered prior to the Config Admin service. Since this is still in the incubator and I'm now more or less happy with the look of things I've checked a slightly tweaked version of this in. I've also added in some new tests to validate and (for now) put them in org.eclipse.equinox.cm.test.

Tom or Chris, could you take a quick look and then I think we can mark this FIXED.
Comment 9 Thomas Watson CLA 2007-11-14 12:27:34 EST 
Your changes are better.  It is better to construct the ServiceTracker in the constructor.  Marking as fixed.
Comment 10 Thomas Watson CLA 2008-06-04 15:26:56 EDT 
clearing out old review request.