[rfc 222][DS] Simplify property based configuration

[bjhargrave]

Original bug ID: BZ#2952 From: @cziegeler Reported version: R6

[bjhargrave]

Comment author: @cziegeler

Specifying service properties (which are not used in the component code) is a little bit ugly, especially if you compare it the component property types. For example, setting the service ranking is something like this

@ Component( property=Constants.SERVICE.RANKING + ":Integer=50" )

or for a http context: property = { HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=" + AppServletContext.NAME, HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_PATH + "=/guessinggame" }

Many DS users complain about this.

I think it would be nice, if you could do something like

@ Component @ ServiceProperties(service_ranking=50)

or @ Component @ ServletContext( osgi_http_whiteboard_context_name = AppServletContext.NAME, osgi_http_whiteboard_context_path = "/guessinggame" )

All we need to do for the DS spec is: We add a marker annotation to the DS annotations, e.g @ ComponentPropertyType. If a component class has a class annotation (like @ ServiceProperties or @ ServletContext in the examples above) which is marked with the marker annotation, then this is handled as a component property type like we do today. All these class annotations are processed in alphabetical order before what is done today to calculate the properties during the build.

bnd can implement this easily.

Of course this only makes sense if there are annotations like @ ServiceProperties and @ ServletContext - but I think that is out of the scope of the DS spec - or any other spec. To supprt this we can start an open source project under the OSGi umbrella where we define such annotation property types for common things like event admin, http whiteboard etc.

An even nicer way would be if there is an additional name mapping, so for the example above it would look like: @ Component @ ServiceProperties(ranking=50)

or @ Component @ ServletContext( name = AppServletContext.NAME, path = "/guessinggame" )

[bjhargrave]

Comment author: @bjhargrave

Your proposal would need to include rules for mapping the types of annotation elements to property types.

@ ComponentPropertyType @ interface ServiceProperties { int service_ranking(); // int maps to Integer String service_pid(); // String maps to String }

What about other types allowed for annotation elements? Annotation, Class?

What about null values? (service_pid=null)

Also what about types not allowed for annotation elements but supported for service properties? Version?

While this might seem simple at first glance, there would be much to specify here.

All these class annotations are processed in alphabetical order before what is done today to calculate the properties during the build.

I would instead state the annotations are processed in the order they are specified in the class which is the order they are returned by getDeclaredAnnotations(). See http://stackoverflow.com/questions/28067074/java-annotations-reflection-ordering/30222541#30222541

[bjhargrave]

Comment author: @cziegeler

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

Your proposal would need to include rules for mapping the types of annotation elements to property types.

@ ComponentPropertyType @ interface ServiceProperties { int service_ranking(); // int maps to Integer String service_pid(); // String maps to String }

What about other types allowed for annotation elements? Annotation, Class?

Right, so the mapping is straight forward:

• primitives are mapped to their object wrapper type
• arrays of primitives are mapped to arrays of their object wrapper type
• String and String[] don't need a type mapping, can be directly used
• Class and annotation are not supported - we can either throw an exception at build time or decide that these fields are ignored
• enum is mapped to Integer, and enum[] to Integer[]

What about null values? (service_pid=null) These are ignored

Also what about types not allowed for annotation elements but supported for service properties? Version? The proposal is to make the development of components simpler and nicer. Therefore it covers maybe 95% of the use cases. But not 100%, so I don't think we need to find a solution for the remaining 5% and leave Version out.

While this might seem simple at first glance, there would be much to specify here. I don't think it is "much", with your input and my additions I think we might already have all we need.

All these class annotations are processed in alphabetical order before what is done today to calculate the properties during the build.

I would instead state the annotations are processed in the order they are specified in the class which is the order they are returned by getDeclaredAnnotations(). See http://stackoverflow.com/questions/28067074/java-annotations-reflection- ordering/30222541#30222541

Ah great, didn't know that the order is available. Yes, that's better

[bjhargrave]

Comment author: @bjhargrave

(In reply to Carsten Ziegeler from comment BZ#2)

Right, so the mapping is straight forward:

• primitives are mapped to their object wrapper type
• arrays of primitives are mapped to arrays of their object wrapper type
• String and String[] don't need a type mapping, can be directly used
• Class and annotation are not supported - we can either throw an exception at build time or decide that these fields are ignored

For Class, I think the property must be of type String and the value is the FQCN. This is the mapping in 112.8.2.2 for type coercion. We can support Class[] as well.

For annotations and arrays of annotations, I think we do not generate a property (ignore). 112.8.2.2 does not support annotations. If anyone calls the element at runtime, they'll get an exception.

• enum is mapped to Integer, and enum[] to Integer[]

For enum, we must map to String? String mapping is what we do in 112.8.2.2 for type coercion. enum to String mapping is also what the Converter does.

What about null values? (service_pid=null) These are ignored

My bad here, null is not a compile-time constant so it cannot be used as a value for an annotation element. But we do need to consider how to handle empty arrays which can occur.

String[] service_pid(); // service_pid = {}

The spec says "If the value attribute is not specified, the body of the property element must contain one or more values." So if any empty array is specified, then a property is not generated (ignore).

Also what about types not allowed for annotation elements but supported for service properties? Version? The proposal is to make the development of components simpler and nicer. Therefore it covers maybe 95% of the use cases. But not 100%, so I don't think we need to find a solution for the remaining 5% and leave Version out.

My bad here again. Turns out Version is not even an allowed type for component properties. So never mind :-)

We also need to make sure any solution here works well with the current component property types. So to take the example from 112.8.2 in the spec and add this new support:

@ ComponentPropertyType @ interface Config { boolean enabled() default true; String[] names() default { "a", "b" }; String topic() default "default/topic"; }

@ Component @ Config(names = "Foo", topic = "foo/bar") public class MyComponent { void activate(Config config) { if (config.enabled()) { // do something } for (String name : config.names()) { // do something with each name } } }

This annotates Config with @ ComponentPropertyType and the uses Config to annotate the component class. The spec says that when the tool sees the activate method use an annotation for an argument, then it must process the annotations defaults for component properties. That text would need to be changed to now allow for the annotation to be applied to the component class and then use the annotations values, instead of the annotations defaults, for component properties.

Making sure this integrates well with the current component property type design is very important. This is starting to look compelling. Perhaps I add to RFC 222?

[bjhargrave]

Comment author: @cziegeler

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

Making sure this integrates well with the current component property type design is very important. This is starting to look compelling. Perhaps I add to RFC 222?

Sorry, I should have visited 112.8.2.2 before making suggestions...it's too long ago. But yes, aligning is important. And big +1 for adding it to RFC 222

[bjhargrave]

Comment author: @bjhargrave

Added to RFC 222.