osgi / bugzilla-archive

Archive of OSGi Alliance Specification Bugzilla bugs. The Specification Bugzilla system was decommissioned with the move to GitHub. The issues in this repository are imported from the Specification Bugzilla system for archival purposes.
0 stars 1 forks source link

[RFC 190][ds] supply all the service.pids that went into the actual delivered component configuration #2560

Closed bjhargrave closed 10 years ago

bjhargrave commented 10 years ago

Original bug ID: BZ#2692 From: David Jencks <djencks@us.ibm.com> Reported version: R6

bjhargrave commented 10 years ago

Comment author: David Jencks <djencks@us.ibm.com>

In previous DS specs a component configured through CA would get one service.pid from the single normal or factory configuration used for the component. With multiple pid support and the current merging rules we'll only get the last configuration used, which could be a normal pid or a (generated) pid for a factory configuration. If the factory configuration pid doesn't happen to be for the last pid in the list, it will be suppressed and there will be no way to correlate the component configuration with a particular configuration present in CA. Also for configuration-optional components there will be no way to tell which configuration pids were used and which were absent.

I find the ability to correlate "factory" pids with components essential in the system I work on and think everyone else using significant numbers of factory configurations would too.

Lets as a special case for the merging rules have the service.pid property be an String[] the same length as the set of pids specified for the component, filled in with the actual pids merged for the component configuration.

If we decide to do this, or supply this information under a different property name, I'm happy to drop my request for a more complicated DTO structure for factory components, since the information I want there will be possible (although somewhat roundabout) to extract from this info plus the service registry and config admin. (https://www.osgi.org/members/bugzilla/show_bug.cgi?id=2683 and https://www.osgi.org/members/bugzilla/show_bug.cgi?id=2690)

bjhargrave commented 10 years ago

Comment author: @bjhargrave

I think this makes sense.

We should aggregate all the service.pid properties (even from the component XML and ComponentFactory.newInstance dictionary) into a String+ property so that all specified service.pid values are present in the resulting String+ property.

I am not sure that we need to specify that the property is a String[] or Collection. It seems this can be an implementation choice. Also the length of the property must be just enough to hold all of the aggregated service.pid properties (no empty slots).

bjhargrave commented 10 years ago

Comment author: @tjwatson

(In reply to BJ Hargrave from comment BZ#1)

I think this makes sense.

We should aggregate all the service.pid properties (even from the component XML and ComponentFactory.newInstance dictionary) into a String+ property so that all specified service.pid values are present in the resulting String+ property.

I am not sure that we need to specify that the property is a String[] or Collection. It seems this can be an implementation choice. Also the length of the property must be just enough to hold all of the aggregated service.pid properties (no empty slots).

Are these not properties that get received by a component? It seems it would be important to know what types of values to expect for the service.pid property in the map.

bjhargrave commented 10 years ago

Comment author: David Jencks <djencks@us.ibm.com>

I don't care whether we use String[] or Collection but I think we should specify what the component will get. For stuff supplied from configuration you can specify in metatype what the component should expect, so there won't be surprises. I think we should avoid surprises here too.

I'm OK with leaving out nulls and including possible service.pid values from configuration.xml and newInstance().

I think the values will be the targetedPID values rather than stripped down "plain" pids when targetedPIDs are used?

bjhargrave commented 10 years ago

Comment author: @bjhargrave

(In reply to David Jencks from comment BZ#3)

I think the values will be the targetedPID values rather than stripped down "plain" pids when targetedPIDs are used?

They have to be the plain pids as delivered in the configuration dictionary (as they are today). They are not the pid values specified in the component xml which may be targeted.

bjhargrave commented 10 years ago

Comment author: @bjhargrave

(In reply to Thomas Watson from comment BZ#2)

Are these not properties that get received by a component? It seems it would be important to know what types of values to expect for the service.pid property in the map.

We often specify String+ in the spec for these sorts of things. I guess you and David would prefer we be more precise here and say String[] or Collection here. Then I would vote for Collection.

bjhargrave commented 10 years ago

Comment author: @tjwatson

(In reply to BJ Hargrave from comment BZ#5)

(In reply to Thomas Watson from comment BZ#2)

Are these not properties that get received by a component? It seems it would be important to know what types of values to expect for the service.pid property in the map.

We often specify String+ in the spec for these sorts of things. I guess you and David would prefer we be more precise here and say String[] or Collection here. Then I would vote for Collection.

I prefer a more precise spec here because of the receiver of these properties. I may be incorrect, but I think we only use String+ for things that are typically matched against with a filter and filters don't care if String, String[] or Collection is used. But here the receiver will always have to cast the type to a non Object to evaluate it. Knowing what to cast that to ahead of type without instanceof checks is good. With that in mind would we force Collection even in the case where there is only one pid?

bjhargrave commented 10 years ago

Comment author: @bjhargrave

(In reply to Thomas Watson from comment BZ#6)

With that in mind would we force Collection even in the case where there is only one pid?

Yes. If we dont say String+, and we say Collection or String[], then it must always be that even for a single String.

bjhargrave commented 10 years ago

Comment author: David Jencks <djencks@us.ibm.com>

(In reply to BJ Hargrave from comment BZ#4)

They have to be the plain pids as delivered in the configuration dictionary (as they are today). They are not the pid values specified in the component xml which may be targeted.

The main point here that I agree with is that the contents of the service.pid String+ is derived from the pids in the configurations (and possibly the xml configuration properties and newInstance call) and not the pids specified in the component xml. The details I think BJ got wrong are that the configurations may contain targeted PIDs (enterprise spec 104.4.5) and the DS component xml pid String[] is going to have plain pids -- targeted pids wouldn't make sense here because we know all the extra targeted pid properties from the bundle the DS descriptor is in.

bjhargrave commented 10 years ago

Comment author: @bjhargrave

CPEG call 2014-06-05: Agreed the service.pid property type will be Collection with the iteration order matching aggregation order of the properties. That is service.pid values from the component description, followed by the service.pid values from the configurations, followed by the service.pid values from the argument to ComponentFactory.newInstance (if a factory component).

BJ to update the spec.

bjhargrave commented 10 years ago

Comment author: @bjhargrave

Added to RFC 190.