[jira] [Resolved] (DOSGI-168) RemoteServiceAdminCore service parameters handling bugs

["Christian Schneider (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/DOSGI-168?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Christian Schneider resolved DOSGI-168.
---------------------------------------

    Resolution: Fixed
      Assignee: Christian Schneider

Patch applied
                
> RemoteServiceAdminCore service parameters handling bugs
> -------------------------------------------------------
>
>                 Key: DOSGI-168
>                 URL: https://issues.apache.org/jira/browse/DOSGI-168
>             Project: CXF Distributed OSGi
>          Issue Type: Bug
>    Affects Versions: 1.4.0, 1.5
>            Reporter: Amichai Rothman
>            Assignee: Christian Schneider
>         Attachments: fix_rsac_params.diff
>
>
> There are several little bugs and deviations from the specs in RemoteServiceAdminCore revolving around the service parameter handling:
> - additionalProperties string array values are not converted to lists like the original serviceParameters are (equality comparison bug)
> - IllegalArgumentException should be thrown if there are no configured exported interfaces
> - configured exported interfaces must be a subset of the service's registered interfaces
> - when specifying "*" for exported interfaces, it should use only registered service interfaces, not non-interface classes (though in practice this probably never happens... still good to document it)
> - (design/impl issue) service parameters are used as map keys for internal storage, and are manipulated to enable this, but should actually be passed unmodified to code that uses them (so the values are all spec-compliant). These are two separate things, and should be treated as such - the keys can actually be something other than full maps
> - a bit of minor code complexity can be removed
> The provided patch fixes most of these, or adds comments mentioning the others where appropriate for future fixing. Specifically:
> - fixed additionalProperties values not being converted into comparable types as is done for serviceProperties (implemented via separate makeKey method)
> - changed copyExportRegistration to accept the export registrations to be copied rather than a key in the map. This removes unnecessary coupling of the data structure implementation to the copying, and also unifies a map containsKey+get into a single get
> - separated use of serviceProperties as internal data structure map keys (which require some modifications, implemented in makeKey method), from their use as actual service properties (keeping them unmodified, with full semantics/rules/types kept intact so they can be expected to be consistent with their specs. For an example of this pitfall see the comment in old implementation of getInterfaces).
> - fixed exportService API compliance by throwing IllegalArgumentException if there are no interfaces to export or if they are not a subset of the registered exports
> - added FIXME comment: when specifying "*" for exported interfaces, it should use only registered service interfaces, not non-interface classes (though in practice this probably never happens... still good to document it)
> - added FIXME comment: makeKey should probably require additional conversions, such as normalizing String+ values, in order to be comparable as unique keys (this is another reason why separating raw serviceProperties from their use as map keys is a good idea) 
> - added some javadocs
> One final little change that should be applied is in TopologyManagerExport, which should now catch IllegalArgumentException when calling exportService - a simple try-catch will do. I've left it out of the patch because after several previous bug reports and patches, my working code has diverged enough from trunk that merging all the issues on that class is too much work :-)

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira