On 31.01.15 14:34, Przemyslaw Bielicki wrote:
> *JsonbContext.Builder*
>
> Arrays should be avoided in public APIs - getClasses should return
> unmodifiable Collection/List<Class<?>>. On the other hand setClasses
> should stay as it is now.
Good catch, will replace.
> *JSON-P
>
> *
> As you said Martin, JSON-P support would be a natural step, just after
> replacing File with Streams :)*
> *
I assume we will eventually add streams, just wanted to hear the need. I
think File helps, there's a set of apps/tests which would leverage
direct File approach. I do not think this is critical and do not hold
strong position here though. Note that even though I agree
Servlet/JAX-RS are important usecases for the api, they are not the only
ones.
> I agree with Eugene that we should limit the number of possibilities
> of creating API objects (but keep shortcuts as they are extremely
> useful if users do not need any customizations) - Builder seems
> flexible enough. With Builder we can automatically guarantee immutability.
Will reply to Eugene on this, tend to agree, need to give it some more
thought.
> If user wants to have flexible marshalling / unmarshalling we could
> overload marshall/unmasrhall methods with settings Map. We would not
> loose immutability and we could keep flexibility.
Yes, this is going to be needed.
> Let's not forget about abilities to customize JSONB default behavior
> using system properties.
Do you have a specific usecase in mind? I'd consider this to be
implementation specific, not really needed in the spec itself.
> *Project structure*
>
> It would be nice to have a parent pom.xml in the jsonb directory. We
> could build all the projects with "one click".
I intentionally left it somewhat separate so that the examples are not
part of the specification, they're used more for discussion points and
helpers to showcase the usecases and api and in the end I'd assume most
examples will make it to the javadoc itself.
> In the examples I would focus on extensive unit testing (using main()
> method for testing is not really modern ;)
Will be adding some tests, though it's not that practical at this point
and also it's not easy to test api without implementation. Note we will
be providing reference implementation and will be implementing TCK kit
as well, so no worries I'm sure things will get covered properly ;O)
> It would be nice if the build fails on checkstyle violations - it's
> rather for you Martin to enforce quality.
I assume you refer to the checkstyle plugin reporting errors when
running 'mvn clean install'. Could be just a misconfiguration of the
plugin, since it works properly and reports no errors when invoked with
'mvn checkstyle:checkstyle'.
MartiNG
> The same for code coverage - tests should be everywhere and target
> coverage should not be less than 80%.
>
> Have a great weekend (what's left of it)!
> Przemyslaw
>
> On Fri, Jan 30, 2015 at 10:34 PM, Eugen Cepoi <cepoi.eugen_at_gmail.com
> <mailto:cepoi.eugen_at_gmail.com>> wrote:
>
> Martin, here are some (opinionated :p) comments...*
> *
>
> *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... :( )?
>
> Missing unmarshal/marshal methods taking input/outputstream and bytes.
>
> 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...)
>
> *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?
>
> *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?
>
> *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.
>
> /"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.
>
>
> *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.
>
> - 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.
>
> - I have the impression that JsonbContext/Builder, Un/Marhsaller
> and Jsonb overlap and could be redesigned a bit (see below).
>
> - I was expecting to see jsonp reader/writers instead of std java
> io classes in Un/Marhsaller.
>
> - Do we want the Un/Marshaller to be mutable (I see those
> setProperty methods)?
>
> - 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(...);
>
> 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=visible
>
>
>
>
--
Martin Grebac, SW Engineering Manager
Oracle Czech, Prague