[
https://issues.apache.org/jira/browse/FELIX-1201?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12735248#action_12735248
]
Marcel Offermans commented on FELIX-1201:
-----------------------------------------
Regarding invoking the updated() method with a null argument, I agree, that is
a bug, it should not activate the dependencies. It's safe to assume that if
someone requires a configuration, the configuration actually needs to contain
at least something.
I have some doubts about the second part. I like the fact that you don't need
to implement ManagedService anymore (even though you can argue about that being
an "OSGi dependency", you could view that as just an interface to deal with
configurability of a POJO, but I see your point)
Getting injected with multiple configurations in OSGi is no real issue, all
configurations can go to the same updated() method because each configuration
will contain a service.pid which allows you to recognize which configuration it
actually is. However, I can see the value of having the option to redirect
those to different methods.
What I don't like is the way the updated() signature is changed. Currently it
throws ConfigurationException which allows you to explicitly tell the system
what property you don't like and why. In the proposed patch, that exception
gets lost. I would at least want to change that in such a way that these
exceptions do not get lost, and I think that is possible. The "general"
signature could become "updated(Dictionary props) throws Exception" since the
JavaDocs of the spec actually state that any exception other than
ConfigurationException should be caught and logged by ConfigurationAdmin anyway
(we might have to check if implementations actually do this, but other than
that we're fine).
So, with a few changes, I think I can apply the patch. WDYT? :)
> Issue with DM and CM
> --------------------
>
> Key: FELIX-1201
> URL: https://issues.apache.org/jira/browse/FELIX-1201
> Project: Felix
> Issue Type: Bug
> Components: Dependency Manager
> Affects Versions: dependencymanager-2.0.1
> Environment: linux FC10, jdk 1.5, 1.6
> Reporter: Pierre De Rop
> Assignee: Marcel Offermans
> Attachments: ConfigurationDependency.java
>
>
> I am using DM for configuring my POJOs from Configuration Admin Service.
> This issue is actually about a bug, but also ask for a change request:.
> 1/ first, I think there are two bugs in ConfigurationDependency.java:
> - When the configuration is not currently available from CM, POJOs are
> invoked in their "updated" method with a null Dictionary
> - Morover, DM requires POJO to implement the ManagedService interface, while
> in the online doc, it is stated that POJOs just have
> to provide an "updated(Dictionary)" method signature.
> Concerning the null dictionary passed to my updated method: for example:
> dm.add(createService()
> .setImplementation(new Log4jConfigurator(property))
> .add(createConfigurationDependency()
> .setPid("log4j")));
> -> My Pojo "Log4jConfigurator" is invoked in its "updated" method with a null
> Dictionary if the configuration is not yet available from CM.
> I have attached to this issue a proposed patch (in
> ConfigurationDependency.java).
> I just check if CM provides a null Dictionary and I just don't activate the
> dependencies ...
> 2/ Now, there is something I would like you to add concerning configuration
> callbacks (I also have implemeted it in the proposed patch):
> Indeed, by default, DM assumes that the pojo implements the
> org.osgi.service.cm.ManagedService interface.
> But the point is: I need my POJOs to be reused outside OSGi; and I don't want
> to introduce a dependency over the OSGi CM managed service interface.
> Moreover, I need to get injected with several PIDS.
> That's why I would like to be able to invoke a method "setCallback" in the
> ConfigurationDependency class (like in ServiceDependency.java).
> This callback would match a method which takes as parameter a Dictionary.
> So, adding such setCallback method would also let me listen to more than one
> PID like this:
> For example:
> dm.add(createService()
> .setImplementation(new Log4jConfigurator(property))
> .add(createConfigurationDependency()
> .setPid("log4j")
> .setCallback("updateLog4jConfig"))
> .add(createConfigurationDependency()
> .setPid("system")
> .setCallback("updateSystemConfig")));
> The patch attached to this issue sounds like to work fine.
> Marcel, WDYT ?
> Could you please commit this patch (and then make a new release of DM) ?
> Thanks a lot for your help;
> /pierre
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
