[ 
https://issues.apache.org/jira/browse/FELIX-3524?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13286468#comment-13286468
 ] 

Felix Meschberger commented on FELIX-3524:
------------------------------------------

Thanks for providing the patch. I generally like it.

Some comments, though:
  * m_componetHoldersByPid holds a Set<ComponentHolder>. We should implement 
ComponentHolder.equals and .hashCode to make sure we properly support holding 
ComponentHolder in sets.
  * ComponentRegistry.registerComponentHolder does checks getConfigurationPid 
for null. This is probably not required because this is either the configured 
value or the component name (which in turn is either configured value or the 
implementation class name) and thus never null.
  * ComponentRegistry.unregisterComponentHolder could just use 
Set.remove(ComponentHolder) to remove the ComponentHolder from the set (see my 
first point on equals and hashCode)

WDYT ?

Other than that, I think this patch is definitely worth it. Would you be able 
to come up with tests ?

Thanks again.
                
> SCR configuration-pid from compendium 4.3
> -----------------------------------------
>
>                 Key: FELIX-3524
>                 URL: https://issues.apache.org/jira/browse/FELIX-3524
>             Project: Felix
>          Issue Type: New Feature
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Assignee: Felix Meschberger
>            Priority: Minor
>         Attachments: FELIX-3524.patch
>
>
> This issue is about implementing the new "configuration-pid" component 
> attribute specified by the OSGi 4.3 compendium. So far, components were using 
> the component name in order to retrieve the component configuration from 
> config admin. But In the section 112.4.4,  a component can now define an 
> optional specific configuration-pid, in order to use a PID which is different 
> from the component name.
> I have attached to this issue a proposed candidate patch, if someone could 
> review it and hopefully commit it.
> I have not yet made a unit test, but if my propose patch seems reasonable and 
> is accepted, then I will try to learn to implement the corresponding test.
> Here is a brief description of the patch:
> - first the parser (XmlHandler/ComponentMetaData) has been modified in order 
> to parse the new configiration-pid attribute from the component element (the 
> parsing fails if the version is not greater or equals to DS 1.2).
> - next, in ComponentRegistry.java, we are now holding the mapping between 
> configuration pids and their respective components: a new method 
> getComponentHoldersByPid(String pid) method has been added, and returns the 
> iterator on the ComponentHolders which must be configured with the given pid. 
> Please see comments in patch.
> - in ConfigurationSupport.java:  when a configuration update is detected from 
> config admin, then the updated is notified to all components whose 
> configuration-pid are referencing the updated pid.
> thanks.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to