Hi,
thanks all for all the comments, let me try to address at least some
of them ;O)
Just a general comment - as I mentioned I intentionally left out many
areas in the initial proposal, such as JSON-P, io streams, ... because I
did not consider them important to deal with wrt the initial api skeleton.
On 30.01.15 22:34, Eugen Cepoi wrote:
> Martin, here are some (opinionated :p) comments...*
> *
Opinions are needed, let's work to reach some consensus in our opinions :).
> *Jsonb*
>
> createContext(final Class<?> ... classes) => Does that mean that the
> user will have to explicitly specify what will be the root classes
> being ser/de?? If yes, it looks too restrictive and how will this
> integrate with Jersey (JsonRootElement... :( )?
I'd like to hear more opinions here, the question here is whether the
usecase is strong enough such that it is not going to be too big
requirement for all binding implementations to support it. Also, in
javadoc the api allows empty set.
> Missing unmarshal/marshal methods taking input/outputstream and bytes.
Intentionally left out, the reason is similar to the above, to evaluate
whether streams are really needed once there are readers/writers and
JSON being inherently a UTF text source/output so one does not have to
deal with encodings. I recall I've also seen binding implementations
which left io streams out.
So, is there really a strong need to keep streams?
> I am not sure that having those shortcut methods is a good idea. Using
> them wouldn't take advantage of any caching thus would yield poor
> performances. At least it should be explained in the javadoc (but
> people tend to not read it...)
Whether or not I like shortcut methods, there are usecases where I think
they make sense, where e.g. writing concise and easy to understand code
is more important than achieving optimal performance. Such as tests,
e.g., or minor use of the api within an application. Wrt javadoc comment
I'd swear I had the javadoc comment in there, but it seems it
disappeared within some of my edits. My choice would be to keep shortcut
access and clarify the javadoc.
> *JsonbContext*
>
> We can read at the beginning of the javadoc /"information necessary to
> implement the *JAXB* binding framework operations:"/, I guess it is a
> typo?
Professional deformation, happens to me, hopefully less over time :).
Will be gone, found some more typos like these.
> *JsonbMarshaller*
>
> I don't get the part about the callbacks. Are they supposed to be
> defined on the object being ser. or by the impl. of the Marshaller?
> What use cases do you have in mind? For deser. I would see validation
> or instantiation, but here...
>
> /"Invoked by JsonbMarshaller after it has created an instance of this
> object."/ I guess it is an instance of the JsonbMarshaller?
For the purpose of the discussion I'd leave the callbacks out for now
for both marshaller & unmarshaller and remove them from the initial
proposal, they really do not belong to the discussion now. Will open up
the discussion later.
> *JsonbUnmarshaller*
>
> I would remove all the untyped unmarshal methods that return Object.
> If people want the deser. to be fully driven by the input, they can
> use unmarshal(json, Object.class). I think in most cases they would
> deser to a node structure (jsonp) or map/list.
I think makes sense, the code is not much shorter. If there are no
objections, I will remove these in next iteration.
> /"An unmarshal method never returns null."/, I would consider an empty
> input as null rather than malformed. Some libs also allow root json
> values different from object and array.
So, you're suggesting for null input the method should return null? Why?
I think the application code would need to deal with malformed code path
in most cases, it does not have to deal with null though.
> *Overall impression*
>
> - Is sticking to JAXB design a goal of this jsr? I would prefer
> not...**some parts look to me over complicated for no apparent reason.
If there is a specific reason to deviate, we can do so, but keeping the
apis with similar feel has its benefits in adoption and lower learning
curve for all those already aware of JAXB api.
> - Users that want to provide some custom ser/de for a specific type
> (the usual custom Serializer) would they use a kind of Adapter like in
> JAXB? I think it can be easier to use but less efficient than directly
> using a streaming API in the custom code.
Yes, dealing with custom is something we'll discuss later.
> - I was expecting to see jsonp reader/writers instead of std java io
> classes in Un/Marhsaller.
Left out for now.
> - Do we want the Un/Marshaller to be mutable (I see those setProperty
> methods)?
That is a design decision. There's more dicussion following in this
thread, so I'll reply there.
> - Do you have an example of what would be the recommended single way
> to use this API?
>
> Jsonb instance could
> - use the JsonbProvider as a factory to obtain instances of
> marshaller/unmarshaller configured with some typed config object or a
> plain map
> - obtain a low level reader/writer instance from jsonp for current
> std reader/writer
> - do some other stuff where having an spi does not provide any win
> - reuse the marshallers (or let it be the responsability of
> JsonbProvider)
> - then call marshaller.marshal(object, jsonpWriter)
>
> // or just new Jsonb() with default config
> Jsonb jsonb = new Jsonb.Builder()
> .setClasses(Book.class, Author.class, Language.class)
> .setProperty(JsonbMarshaller.JSON_BIND_FORMATTED_OUTPUT, true)
> .use(SomeJsonbProvider()) // optional - by default would load one
> using ServiceLoader
> .use(JsonpProvider()) // used to obtain jsonp read/write instances
> .build();
>
> // also accepting jsonp reader/writer
> jsonb.marshal(...);
> jsonb.unmarshal(...);
>
I think this goes together with the mutability issue above. Say for your
example here, if the application code wants to use 2 ways to use
marshaller - with formatted output and without, it has to create and
hold 2 Jsonb instances, whereas with mutable marshaller/unmarshaller
this usecase would perform better and code would be shorter. Also, the
above does not allow to configure marshaller and unmarshaller
differently and as well more-less assumes that marshaller and
unmarshaller instances both are going to be used in the usecase. There
are usecases and applications where only one is needed (either
marshaller or unmarshaller).
MartiNG
> Unmarshall/Marshall could take only jsonp reader/writer and no std
> java reader/writer. It is discutable, but if the goal is to provide
> databinding over the already existing "std parsing API" then it should
> be the way to go IMO.
>
> People are getting used with APIs that provide a single entry point +
> a builder to configure it and the underlying components.
> Providing multiple ways to use this API could be error prone (In the
> javadoc and examples I see 3 ways to achieve near the same thing).
>
> Maybe I am missing the point and you already have in mind use cases
> that could take advantage of the current design?
>
> Eugen
>
> 2015-01-30 19:52 GMT+01:00 Eugen Cepoi <cepoi.eugen_at_gmail.com
> <mailto:cepoi.eugen_at_gmail.com>>:
>
> Hey Martin,
>
> Do you expect us to post our comments only on the ML or perhaps is
> there a way to comment directly the code "à la github" on java.net
> <http://java.net>?
> BTW I see ssh/user_name in your URL, are we supposed to have a
> write access to the repo and can maybe contribute some code?
>
> Eugen
>
> 2015-01-30 18:02 GMT+01:00 Martin Grebac <martin.grebac_at_oracle.com
> <mailto:martin.grebac_at_oracle.com>>:
>
> Hi experts,
> welcome again. We're well ahead in 2015, and ready to run
> full speed on the JSON Binding API. As you may have already
> noticed, I've put together an initial list of individual
> API/spec [1] items which should lead us to our first quickly
> approaching milestone - Early Draft. I expect this to be a
> fairly live list, updated regularly based on progress and status.
>
> As a first step, I'd like to submit for review proposal for
> runtime api. One thing to state first, this initial step does
> not contain 2 obvious items: full generic types and JSON-P
> support. I'll open these in coming subsequent proposals as I
> believe they warrant separate discussion.
>
> To review, please checkout the remote branch:
>
> git clone ssh://<user_name>@git.java.net/jsonb-spec~git
> <http://git.java.net/jsonb-spec%7Egit> jsonb
> cd jsonb
> git checkout runtime_api_init
>
> Folder description:
> api - api code
> examples - code examples, not expected to run
> spec - spec document source code in tex, spec/spec.pdf is
> current snapshot of the document
>
> Build with Maven (3.2, JDK8), built API javadoc is located in
> api/target/apidocs folder.
>
> Looking forward to your feedback - please send your comments
> by Friday next week (Feb06).
>
> Thank you,
> MartiNG
>
> [1]
> https://java.net/jira/secure/RapidBoard.jspa?rapidView=27&view=planning.nodetail&versions=visibl
> <https://java.net/jira/secure/RapidBoard.jspa?rapidView=27&view=planning.nodetail&versions=visible>
>
>