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

[http whiteboard] Clarification when failure DTOs are created #2630

Closed bjhargrave closed 9 years ago

bjhargrave commented 9 years ago

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

bjhargrave commented 9 years ago

Comment author: @cziegeler

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

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

For the sake of moving on, I hope this summarizes it then:

a) for Servlets, Filters, Resources and ServletContextHelper services - only the services with the required properties are tracked - if a required property is missing, no failure DTO is created. (If the property is available but invalid (e.g. empty value) a failure DTO is created) b) For all listener services we add the new required service property osgi.http.whiteboard.listener - this is of type string with two allowed values, "true" and "false". As with a) if this property is missing, no failure DTO is created; the same happens if "false" is specified. If any other value than "true" or "false" is specified, a failure DTO is created for the invalid value

I think we agreed that if the value of osgi.http.whiteboard.listener is "false" then a ListenerDTO is create and we must add a field to ListenerDTO to indicate the listener is enabled or not ... is that a boolean field in the DTO?

Ok, I'm fine with creating a failure DTO in that case - however I see no sense in adding the enabled information to the ListenerDTO - by definition, if a listener is enabled, there is a ListenerDTO - if a listener is not enabled, there is no ListenerDTO. If there is some error, a FailedListenerDTO is created - so I suggest we add a constant for this as an error code. The failureReason field can represent the state using this error code

bjhargrave commented 9 years ago

Comment author: @bosschaert

I've changed the user-provided Boolean properties to String in the spec here: https://www.osgi.org/members/gitweb/build.git/commitdiff/540655a1871535eeeeb67fe1c5a71816a082d487

... and added clarification around what services end up in the failure DTOs here: https://www.osgi.org/members/gitweb/build.git/commitdiff/1f8dada84534ca6020834b353d649fe19b8be172

bjhargrave commented 9 years ago

Comment author: @bjhargrave

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

I think we agreed that if the value of osgi.http.whiteboard.listener is "false" then a ListenerDTO is create and we must add a field to ListenerDTO to indicate the listener is enabled or not ... is that a boolean field in the DTO?

The field in ListenerDTO should be a String with the value of the property. FailedListenerDTO needs a new failureReason for the invalid property value.

bjhargrave commented 9 years ago

Comment author: @cziegeler

The "normal" DTOs are for active services, the failure DTOs are for not active ones. So we should create a failure DTO if the enabled property is invalid (neither true nor false) but also if false is specified.

For the invalid value case we can use the FAILURE_REASON_VALIDATION_FAILED. We have to create a new reason for the disabled case.

In addition, I would prefer if we could specify for the binary properties, that either Boolean or String is allowed (String with the values true and false, case insensitive). I seriously want to use the new component property type stuff of DS where it's easy to use boolean.

bjhargrave commented 9 years ago

Comment author: @bjhargrave

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

I seriously want to use the new component property type stuff of DS where it's easy to use boolean.

Can you sketch out what you are thinking here?

This is a service property on the listener service and not really interesting to the component instance itself.

I can see doing

@ Component(property="osgi.http.whiteboard.listener=true")

But doing

@ interface ServiceProps { // need to "mangle" property name . => _ boolean osgi_http_whiteboard_listener default true; }

@ Component class Impl { @ Activate void activate(ServiceProps unused) {} }

Seems like gross overkill to get the property in the xml. You require a Java type to be defined, an activate method to be written and DS must instantiate the object at runtime even it is not needed.

Or am I missing a more simple way to do it with component property types?

bjhargrave commented 9 years ago

Comment author: @bjhargrave

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

The "normal" DTOs are for active services, the failure DTOs are for not active ones.

Failure are not active, but I think it is not entirely correct to say not active are failures.

So we should create a failure DTO if the enabled property is invalid (neither true nor false) but also if false is specified.

A failure DTO for an invalid configuration make sense. But a failure DTO for a valid configuration seem wrong to me.

No osgi.http.whiteboard.listener prop: OK osgi.http.whiteboard.listener=true: OK osgi.http.whiteboard.listener=foobar: Failure due to invalid configuration osgi.http.whiteboard.listener=false: Failure even though valid configuration

bjhargrave commented 9 years ago

Comment author: @cziegeler

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

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

I seriously want to use the new component property type stuff of DS where it's easy to use boolean.

Can you sketch out what you are thinking here?

This is a service property on the listener service and not really interesting to the component instance itself.

I can see doing

@ Component(property="osgi.http.whiteboard.listener=true")

But doing

@ interface ServiceProps { // need to "mangle" property name . => _ boolean osgi_http_whiteboard_listener default true; }

@ Component class Impl { @ Activate void activate(ServiceProps unused) {} }

Seems like gross overkill to get the property in the xml. You require a Java type to be defined, an activate method to be written and DS must instantiate the object at runtime even it is not needed.

Or am I missing a more simple way to do it with component property types?

No, you're not - I agree that this doesn't look elegant, but it allows me to generate metatype information with the metatype annotations and then I can use tools to switch the button.

bjhargrave commented 9 years ago

Comment author: @cziegeler

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

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

The "normal" DTOs are for active services, the failure DTOs are for not active ones.

Failure are not active, but I think it is not entirely correct to say not active are failures.

Yes, I agree that these are not failures.

So we should create a failure DTO if the enabled property is invalid (neither true nor false) but also if false is specified.

A failure DTO for an invalid configuration make sense. But a failure DTO for a valid configuration seem wrong to me.

Yes, but I don't want to create a third category for this case :) So far, before we started with having the enabled property, all non failure DTOs represented activate services. And we really should keep that. For me, this means either we come up with a different way of handling the listener enablement, we put this into the failure category or we put it in a third bucket.

No osgi.http.whiteboard.listener prop: OK osgi.http.whiteboard.listener=true: OK osgi.http.whiteboard.listener=foobar: Failure due to invalid configuration osgi.http.whiteboard.listener=false: Failure even though valid configuration

bjhargrave commented 9 years ago

Comment author: @bjhargrave

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

In addition, I would prefer if we could specify for the binary properties, that either Boolean or String is allowed (String with the values true and false, case insensitive).

Given that we accept Tim's observation that we are modelling an enum which today has only two values (true, false), allowing Boolean as a type does not support some future where we define additional enum values.

Allowing Boolean will permanently look this property to only ever have 2 values: true and false.

So it either has to be String (it is an enum which today has two values) or Boolean (it is a boolean).

bjhargrave commented 9 years ago

Comment author: @cziegeler

EEG-Call 11-FEB-2015:

The property osgi.http.whiteboard.listener is required for listener service and has the meaning of an opt-in:

The value comparision is done ignoring case.

We keep the type of those binary/enum properties as String.

bjhargrave commented 9 years ago

Comment author: @cziegeler

Updated javadoc accordingly

https://www.osgi.org/members/gitweb/build.git/commit/7b4d0f3863668fbdc36c19eb1149659c390733cb

bjhargrave commented 9 years ago

Comment author: @bosschaert

Updated the osgi.http.whiteboard.listener property in the spec as summarized by Carsten above: https://www.osgi.org/members/gitweb/build.git/commitdiff/51cf984e1f30ccbdf90cd124b93a86463dec22ce