[jira] Commented: (FELIX-1201) Issue with DM and CM

["Marcel Offermans (JIRA)" <ji...@apache.org>]

    [ 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.