Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 275997 - [ds] Declarative services fails to honor configuration policy
Summary: [ds] Declarative services fails to honor configuration policy
Status: RESOLVED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: Compendium (show other bugs)
Version: 3.5   Edit
Hardware: PC All
: P3 blocker (vote)
Target Milestone: 3.5 RC1   Edit
Assignee: equinox.compendium-inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-05-12 22:40 EDT by Bryan Hunt CLA
Modified: 2009-05-13 11:22 EDT (History)
2 users (show)

See Also:
tjwatson: review+


Attachments
proposed patch (1.15 KB, patch)
2009-05-12 22:41 EDT, Bryan Hunt CLA
tjwatson: iplog+
Details | Diff
optimized patch (1.00 KB, patch)
2009-05-13 04:55 EDT, Stoyan Boshev CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bryan Hunt CLA 2009-05-12 22:40:57 EDT
When a ServiceComponent is loaded using readObject(), the configurationPolicy is set using in.readUTF().  Later in the Resolver, the configurationPolicy is compared against the constants CONF_POLICY_* using == and != which fails because the strings are not the same object.
Comment 1 Bryan Hunt CLA 2009-05-12 22:41:56 EDT
Created attachment 135489 [details]
proposed patch
Comment 2 Stoyan Boshev CLA 2009-05-13 02:46:47 EDT
Good catch!
The patch looks good. I Will release it soon.
Comment 3 Stoyan Boshev CLA 2009-05-13 04:55:24 EDT
Created attachment 135534 [details]
optimized patch

Optimized patch based on the previous one.
Comment 4 Thomas Watson CLA 2009-05-13 09:13:50 EDT
Stoyan, please see the end-game for 3.5 (http://www.eclipse.org/equinox/planning/freeze_plan_3.5.php).  As we enter RC builds we must have additional reviews of fixes.  For RC1 fixes you must get another committer to review the patch.  Let me know when you have a patch to release, I can review.  Use the review flag (?) with my e-mail to ask for a review.  Thanks.
Comment 5 Bryan Hunt CLA 2009-05-13 09:47:24 EDT
I do not agree with the optimized patch.  If the configuration policy is set to optional, the optimized patch will override that setting and set it to require.
Comment 6 Thomas Watson CLA 2009-05-13 09:52:00 EDT
(In reply to comment #5)
> I do not agree with the optimized patch.  If the configuration policy is set to
> optional, the optimized patch will override that setting and set it to require.
> 

In writeObject method the flag is not written out and the default setting is set to CONF_POLICY_OPTIONAL.  You should never read in a "optional" string.
Comment 7 Thomas Watson CLA 2009-05-13 09:52:35 EDT
Comment on attachment 135489 [details]
proposed patch

Flag for IP log.
Comment 8 Bryan Hunt CLA 2009-05-13 10:11:22 EDT
I still respectively disagree.  It doesn't matter the value read.  If the value read does not equal ignore, the config policy gets set to require.
Comment 9 Stoyan Boshev CLA 2009-05-13 10:19:13 EDT
(In reply to comment #8)
> I still respectively disagree.  It doesn't matter the value read.  If the value
> read does not equal ignore, the config policy gets set to require.

Have a look at the writeObject method implementation. If your configuration policy is CONF_POLICY_OPTIONAL, then only one boolean flag is written in the stream having value false.
Then in readObject if the read boolean flag has value "false" then the code reading the configurationPolicy value as string does not get executed. In this case the configurationPolicy field stays with the default value CONF_POLICY_OPTIONAL.


Comment 10 Bryan Hunt CLA 2009-05-13 10:42:40 EDT
Ah, now it makes sense.  I agree with the optimized patch.  Thanks for clarifying.
Comment 11 Stoyan Boshev CLA 2009-05-13 11:22:16 EDT
Patch released