Hi Oleg (and others),
thank you for interest thorough review and feedback. Please see my
comments below.
On 26.02.15 11:59, Oleg Tsal-Tsalko wrote:
> Hi guys,
>
> We (Kiev UA JUG) come up with list of comments/questions/suggestions
> reg Runtime API proposed so far:
>
> 1. Having pluggable JsonbProviders, do we really want to leave an
> option to also setup diff JsonProviders via
> JsonBuilder#withProvider(...) method? Specific JsonbProvider could
> decide what JsonProvider to use underneath by itself? Such a mix
> brings a lot of confusion and introduces dependency to JSON-P api.
> We would vote to remove JsonBuilder#withProvider(...) method. WDYT?
>
As for dependency, JSON Binding will have JSON-P dependency anyway
(direct or indirect), given we plan to support JSON-P artifacts within
default mapping (see the default mapping examples from MartinV). This is
similar to JAXP/JAXB alignment.
The main reason I added this method is that JSON-P only allows
ServiceLoader or DEFAULT_PROVIDER for selecting the provider, which
inherently causes problems which you even describe below, and makes it
hard or impossible to configure a specific JSON-P provider for use
within JSON Binding ifthat is what the application requires.
Question is, whether to impose this requirement on all JSON Binding
implementations - by this requirement, the spec requires all JSON
Binding implementations to be JSON-P implementation agnostic, which
could limit some implementations, so I'd agree the requirement may be
too strong. I'd like to hear some more voices in EG though.
> 1. Don't we want to add fallback option to
> JsonbProvider#provide(String className) method to load class
> directly using ClassLoader if no class found using ServiceLoader
> similar to default JsonbProvider#provide() method?
>
Are you looking to solve a specific usecase? We already have a specific
provider lookup using provider name (which should support also jdk9
module mode without changes in future):
public static JsonbProvider provider(final String providerName) {
> 1. It was already discussed couple of times however why not support
> JVM params to specify JSON-B Provider to be used in runtime
> similar to specifying TransformerFactory impl in JAXP for example?
> It might be usefull in some cases and will preserve consistency
> with other specs. Imagine you have couple of provider libs in
> classpath and would like in runtime to specify which one to use
> without recompiling/rebuilding project. How can we cover this use
> case with specific JsonbConfig as some of you suggested?
>
Are you missing this option in JSON-P as well? Looking at consistency,
we should be looking at multiple places.
> 1. From my point of view JsonConfig objects should be thread-safe and
> immutable to be safelly passed around. From the other hand in
> order to contruct JsonbConfig instances we can use implicit inner
> class builder with comprehensive builder methods. See JsonbConfig
> class in attach or on GitHub -
> https://github.com/olegts/jsonb-spec/blob/simplified_api/api/src/main/java/javax/json/bind/JsonbConfig.java
> 2. JsonbConfig properties/methods naming could be improved to be more
> readable and follow builder style. See JsonbConfig class in
> attach. From client code it will looks like:
> * JsonbConfig config =
> jsonbConfig().withInputEncoding(UTF_16).withPrettyOutput().build();
>
I believe the full is: new
JsonbConfig.JsonbConfigBuilder().withInputEncoding(StandardCharsets.UTF_16).withPrettyOutput().build();
> * instead
> * JsonbConfig config = new
> JsonbConfig().fromJsonEncoding(StandardCharsets.UTF_16.name
> <http://standardcharsets.utf_16.name/>()).toJsonFormatting(true);
>
The problem is this implementation is missing a method for generic,
provider specific properties, which will make the immutability and
thread safety hard to achieve. We need an extension mechanism to support
provider specific features.
Also, my motivation was to avoid creating multiple instances of
JsonbConfig in every simple case even if e.g. one just wants to quickly
set a pretty printing on existing configuration to e.g. log some messages.
> 1. As has been already proposed lets provide API examples as unit
> tests which should pass using mocks or dummy implementation
> instead of main method classes that are failing.
>
At this early stage, compiling the examples should be enough, as
mentioned they are more to guide the discussion.
> 1. As has been already mentioned 'public' modifier could be omitted
> in interfaces.
>
This has been brought up number of times, I prefer to be explicit, for
public health and safety I removed them :)
> 1. Runtime "Custom providers with explicit provider impl" example
> should be changed to explicitly instantiate provider instead of
> builder itself I believe: Jsonb jsonb = new
> CustomJsonbBuilder().build();in Runtime class line 111.
> 2. TYPO: Json class line 222.
>
Fixed.
> 1. JsonbBuilder methods could be more builder like with better names.
> See JsonbBuilder class in attach or on GitHub -
> https://github.com/olegts/jsonb-spec/blob/simplified_api/api/src/main/java/javax/json/bind/JsonbBuilder.java
>
> From client code it will looks like:
>
> Jsonb jsonb = newJsonbWith(config);
>
> Jsonb jsonb =
> jsonBuilderFrom("jug.kiev.jsonb.impl.CustomJsonbProvider").build();
>
> Jsonb jsonb = newJsonb();
>
> instead
>
> Jsonb jsonb = JsonbBuilder.create(config);
>
> Jsonb jsonb =
> JsonbBuilder.newBuilder("foo.bar.ProviderImpl").build();
>
> Jsonb jsonb = JsonbBuilder.create();
>
>
> Hope some of our comments/suggestions will be useful...
You example is not fully complete, and to me it still is more verboseand
not that much consistent with what other apis are providing, such as
JAX-RS client or JSON-P e.g. so I'm still inclined to the latter.
MartiNG
> Thank you,
> Oleg
>
> 2015-02-24 17:53 GMT+02:00 Martin Grebac <martin.grebac_at_oracle.com
> <mailto:martin.grebac_at_oracle.com>>:
>
> Yes. I don't see how the provider approach complicates the api,
> and given the JSON Binding is filed as a standalone JSR, with
> inclusion to Java EE, we just have to support providers. Also
> circling back to Inder's previous question, there was a reason to
> deal with providers in JAXP world, too - number of applications
> decided to use Woodstox e.g. (or even other non O/S providers) to
> address specific needs.
> MartiNG
>
>
> On 24.02.15 16:21, Hendrik Dev wrote:
>
> so this means we keep the provider approach (in the first place) ?
>
> On Mon, Feb 23, 2015 at 4:57 PM, Martin Grebac
> <martin.grebac_at_oracle.com <mailto:martin.grebac_at_oracle.com>>
> wrote:
>
> On 20.02.15 10:36, Hendrik Dev wrote:
>
> On Fri, Feb 20, 2015 at 2:50 AM, Inderjeet Singh
> <inder_at_alumni.stanford.edu
> <mailto:inder_at_alumni.stanford.edu>> wrote:
>
> Hi Hendrik,
>
> approach cause with some sensitive default
> shortcuts. I aggree with
> Inderjeet that Jsonb() should be sufficient
> the most use cases. I do
> NOT aggree with Inderjeet when he says
>
> "There is typically NO developer concern about
> plugging in an
> alternate implementation. So, we should just
> have a really good
> default implementation and get rid of providers."
>
> cause thats a) not true and b) if you mean
> with default implementation
> the RI then this is also not true. RI has IMHO
> a total other emphasis
> than other non-RI implementations (and of
> course other licenses :-).
> Inder, maybe you can clarify this, thanks :-)
>
> I am not sure I fully understand your points here.
> What I mean is this: Developers use whichever
> implementation comes with
> their environment.
> This JSR may become part of the JDK, and in that
> case, I don't think we
> care
> about alternate implementations.
>
> I was not aware of this, is this really the case?
> At least for EE i like the provider approach.
>
> It may, it may not - with Jigsaw and modularity it may not
> be that required
> to grow Java SE too much. The primary goal of this JSR as
> of this point is
> certainly Java EE (8).
>
--
Martin Grebac, SW Engineering Manager
Oracle Czech, Prague