add a ClientBuilder service already prepared with json #56
Conversation
… set Signed-off-by: Raymond Augé <raymond.auge@liferay.com>
Signed-off-by: Raymond Augé <raymond.auge@liferay.com>
Signed-off-by: Raymond Augé <raymond.auge@liferay.com>
Signed-off-by: Raymond Augé <raymond.auge@liferay.com>
| @@ -101,6 +101,11 @@ | |||
| <groupId>org.apache.geronimo.specs</groupId> | |||
| <artifactId>geronimo-jsonb_1.0_spec</artifactId> | |||
| </dependency> | |||
| <dependency> | |||
There was a problem hiding this comment.
Looks wrong to have jsonb and jaxrs modules at once
They are typically never used together
There was a problem hiding this comment.
geronimo-metrics uses JSON-B
geronimo-jwt-auth uses JSON-P
are we not supposed to use those together?
Secondly, these are just services, if you don't want to use them in the same app, don't! But having a single module makes using this on OSGi easier.
There was a problem hiding this comment.
Jsonp comes with jsonb so all good without jaxrs - if not we must fix it in jsonb module.
Also note jaxrs module is about johnzon mapper jaxrs integration, not anything else, and is pre-jsonb time.
Issue is not if they are used but their transitivity.
If you make the scope provided then it would be ok but will require tests ensjring it stays provided in the long term.
Hope it makes sense
There was a problem hiding this comment.
Maybe I'm confused by the code in johnzon-jsonb module: https://github.com/apache/johnzon/blob/master/johnzon-jsonb/src/main/java/org/apache/johnzon/jaxrs/jsonb/jaxrs/JsonbJaxrsProvider.java#L262
That does not appear to support jsonp. It explicitly rejects it. Is this a bug?
There was a problem hiding this comment.
We originally didnt support jsonp in jsonb, we added it after and jaxrs support hasnt been update to tell the story
| highest(serviceReferences(ClientBuilder.class)).flatMap(OSGi::prototypes).map( | ||
| serviceObjects -> new JsonClientBuilderFactory(context, serviceObjects) | ||
| ).flatMap( | ||
| factory -> register( |
There was a problem hiding this comment.
Looks wrong in a json layer to register a jaxrs component, should stay in jaxrs whiteboard which should pull providers as per jaxrs spec imho, wdyt?
There was a problem hiding this comment.
hey @rmannibucau I sent the wrong branch in this PR. let's not get ahead of ourselves just yet. :) I will close this and send it in the order I intended.
No description provided.