Hi Martin,
It is good that they are in queue)
Could you comment on #4:
> Why decision has been made to throw NPEs upon retrieval of null values
> across API instead of IllegalArgumentExceptions? I don't remember it has
> been discussed in public?
Thank you,
Oleg
2015-03-17 10:30 GMT+02:00 Martin Grebac <martin.grebac_at_oracle.com>:
> Hi Oleg,
> no worry, they're still in the queue, just did not manage to push an
> updated version yet. Also will include the latest discussed updates. Stay
> tuned.
> MartiNG
>
>
> On 16.03.15 22:16, Oleg Tsal-Tsalko wrote:
>
>> Hi Martin,
>>
>> Just wanted to remind you couple of changes you kind of agreed with but
>> haven't committed yet (here I'm referring master branch):
>>
>> 1)
>>
>> Is it ever possible that JsonbConfig#getProperty()/setProperty()
>> methods will throw JsonbConfigException is they are simply calling
>> Map#get()/put() methods?Thus JsonbConfigException looks redundant
>> for now at all and methods signatures + javadocs should be fixed?
>>
>> Is it ever possible that JsonbConfig#getProperty()/setProperty()
>> methods will throw JsonbConfigException is they are simply calling
>> Map#get()/put() methods?Thus JsonbConfigException looks redundant
>> for now at all and methods signatures + javadocs should be fixed?
>> Thanks, somehow this got dropped - makes sense.
>>
>>
>> 2)
>>
>> However if we really want to leave JsonbConfig object mutable
>> in runtime to quickly change some configuration then we
>> probably need to explicitly highlight in specification that
>> JsonbConfig is not thread safe and could be modified in runtime.
>>
>> Agree.
>>
>>
>> 3) In Runtime example we still have:
>>
>> // Use an explicit implementation of JsonbProvider
>> Jsonb jsonb = new CustomJsonbBuilder().build();
>>
>> where should be smith like below I believe:
>>
>> // Use an explicit implementation of JsonbProvider
>> Jsonb jsonb = JsonbBuilder.newBuilder(new
>> CustomJsonbProvider()).build();
>>
>> And couple of more questions:
>>
>> 4) Why decision has been made to throw NPEs upon retrieval of null values
>> across API instead of IllegalArgumentExceptions? I don't remember it has
>> been discussed in public?
>> 5) Why default JSON-B provider is mentioned as
>> org.eclipse.persistence.json.bind.JsonBindingProvider or it's just a
>> stub for now?
>>
>> Thank you,
>> Oleg
>>
>> 2015-03-02 12:13 GMT+00:00 Martin Grebac <martin.grebac_at_oracle.com
>> <mailto:martin.grebac_at_oracle.com>>:
>>
>>
>> On 28.02.15 18:05, Oleg Tsal-Tsalko wrote:
>>
>>
>> 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) {
>>
>>
>> I meant changing existing implementation of
>>
>> public static JsonbProvider provider(final String
>> providerName)
>>
>> to class load provider class directly as fall back option if
>> no such provider found using ServiceLoader
>> as in
>>
>> public static JsonbProvider provider()
>>
>> I see, understand now. I consider throwing an exception here
>> better, because the user is explicitly requesting specific
>> provider. So the implementation should not choose something
>> different and should let the user know the expected provider could
>> not be found/instantiated.
>>
>>
>> 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.
>>
>>
>> Didn't use JSON-P in practice so far. Was referring to JAXP
>> specification as an example...
>>
>> I'm not opposing here, I'm trying to figure out whether there
>> really is the need, or the above methods are enough. JSON-P e.g.
>> does not define any such overriding mechanism, does not even
>> define the possibility to request specific provider. JSON-P is
>> with us for some time and so far I haven't seen any requests for
>> overriding.
>>
>> 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.
>>
>>
>> We can just add one more method to JsonbConfigBuilder then:
>>
>> JsonbConfigBuilder withSpecificProperty(String propertyName,
>> Object propertyValue)
>>
>> Right. With this you have JCB itself immutable, but not the
>> configuration as such. Object (say some collection) is still mutable.
>>
>> However if we really want to leave JsonbConfig object mutable
>> in runtime to quickly change some configuration then we
>> probably need to explicitly highlight in specification that
>> JsonbConfig is not thread safe and could be modified in runtime.
>>
>> Agree.
>>
>> I saw you've added:
>>
>> Map<String, Object> getAsMap()
>>
>> probably with intention to use this method inside JsonbBuilder
>> to pass unmodifiable Map of props into Jsonb object contractor,
>> however I would rather use JsonbConfig container itself to
>> pass into Jsonb object during construction. WDYT?
>>
>> Right, though it has the same problem as described above. Without
>> deep clone one still has to face mutability. This method allows a
>> clone to be done within the method calls before Jsonb objects are
>> constructed.
>>
>> 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.
>>
>>
>> Having unit test style examples carefully structured will make
>> examples more readable and clear with later ease to make them
>> actually runnable.
>> It's not big effort to do anyway...
>>
>> In general I understand the test obsession - I'd myself be
>> obsessed as well :) - this being a regular implementation project.
>> Though here actually *not* having the examples in the form of
>> tests makes sense, because they are not expected to be so, and
>> besides writing them in such a way is an unnecessary complication.
>> There will be hundreds times more tests within RI implementation
>> or TCK required than just these few within examples, so I don't
>> want anyone to think these are normatives or anything.
>>
>> Also one item been missed:
>>
>> Is it ever possible that
>> JsonbConfig#getProperty()/setProperty()
>> methods will throw JsonbConfigException is they are simply
>> calling
>> Map#get()/put() methods?Thus JsonbConfigException looks
>> redundant
>> for now at all and methods signatures + javadocs should be
>> fixed?
>>
>> Thanks, somehow this got dropped - makes sense.
>>
>> MartiNG
>>
>>
>>
> --
> Martin Grebac, SW Engineering Manager
> Oracle Czech, Prague
>
>