Refactoring if `JsonFactory` into `TokenStreamFactory`, couple of intermediate factories (one for binary-formats, other for textual), and then "new" `JsonFactory` (but in package `com.fasterxml.jackson.core.json` instead of `com.fasterxml.jackson.core`) was completed a while ago.
As a follow-up, I proceeded to change the constructor to use the good old Builder pattern, as per: https://github.com/FasterXML/jackson-core/issues/433 which in turn allows making all stream factories immutable, not just when used correctly ("config-then-use") but, well, always. I did leave the default no-args constructor for convenience, so you can still do, say: ObjectMapper mapper = new ObjectMapper(new CBORFactory()); but for any other changes, like enabling/disabling features, builder is needed. For example: ObjectMapper mapper = new ObjectMapper(JsonFactory.builder() .with(JsonParser.Feature.ALLOW_YAML_COMMENTS) .build()); You do not have to start with a builder always, however; you can "rebuild" an existing factory: JsonFactory f = ....; JsonFactory f2 = f.rebuild().set(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false).build(); Construction of factories is type-safe so that chaining works for case of other factories (each format has its own; many have format-specific features, some other settings, like XML backend has Stax factories). The benefits of this change are mostly 2 things: 1. Goodness of immutability in general: it is not impossible to cause thread-safety issues by sneakingly reconfiguring JsonFactory that an ObjectMapper has, during usage 2. Further expansion to make ObjectMapper, too, use Builders and remove direct mutability Of these, (2) is the real price. It will be much more complicated process, partly because I will have to figure out how Modules need to work wrt building. But with experience from factories (and the fact factories are now immutable) this should be bit easier process. -+ Tatu +- On Fri, Sep 29, 2017 at 5:22 PM, Tatu Saloranta <[email protected]> wrote: > On Wed, Sep 27, 2017 at 9:37 PM, Tatu Saloranta <[email protected]> wrote: >> (note: issue to track this thread is >> https://github.com/FasterXML/jackson3-dev/issues/5) >> >> Jackson 3.x major version lets us do a few changes that minor version does >> not. >> This includes both actual refactoring/-packaging of functionality, and >> renaming, where beneficial. >> >> In case of core streaming package (`jackson-core`), one long-term >> issue is the historical precedence of json over all other formats. >> This is due to Jackson starting as pure json processing package, and >> other formats sort of only overriding pieces that differ: usually by >> sub-classing actual full json implementation (as is case with >> `JsonFactory`), or at least partial implementation (`JsonParser`, >> `JsonGenerator`). Despite sub-optimal modeling this has actually >> worked ok so far, from functionality perspective. >> But although there is no burning acute problem with it, there are real >> problems resulting from this modeling where there is no separation of >> format-agnostic API and json implementation. >> There have been occasional bugs where some format implementations have >> not overridden all methods in `JsonFactory` (or `JsonParser`, >> `JsonGenerator`) they should have, leading to odd bugs like CBOR >> factory constructing json parser for certain input source. >> >> With Jackson 3.x, then, one of first things should be cleaving out >> JSON-implementation of the factory that constructs parsers (decoders) >> and generators (encoders). This new abstraction needs name. >> I think split itself is non-controversial, and although there is some >> extra work at higher level (databind) with respect to change of >> `JsonFactory` type to something else, I think changes are relatively >> modest. At least compared to complexity of possibly renaming >> `JsonParser`, `JsonGenerator`... but that's a different story, and >> we'll burn that bridge once we get there. >> >> This leaves actual naming. >> >> I was initially thinking of factory producing encoders and coders -- >> codecs, in short -- leading to maybe `CodecFactory`. This could be bit >> confusing due to existence of `ObjectCodec` abstraction, although I >> don't know if that's a big deal. >> >> But when I thought about this more, I realized that it is perhaps >> better consider it a "stream factory", constructing stream readers >> (parsers) and writers (generators). >> So, my leading name candidate is actually: >> >> StreamFactory >> >> which would then lead to implementations like SmileStreamFactory (or >> SmileFactory?), Avro[StreamFactory] and so forth. >> It would even be possible to retain `JsonFactory` as actual >> Json-backed implementation, although I am not sure if that would help >> or hinder upgrade from 2.x, since semantics of the class would be >> somewhat different. >> >> So: I have really 2 questions: >> >> 1. What should be name of the new low-level stream factory class: >> (a) StreamFactory, or >> (b) CodecFactory, or >> (c) something else (and if so, what) >> 2. Should implementations use short or long form: >> (a) Short form: JsonFactory, SmileFactory >> (b) Long form: JsonStreamFactory, SmileStreamFactory >> >> and my default choice would be: >> >> StreamFactory >> JsonStreamFactory extends StreamFactory >> AvroStreamFactory extends StreamFactory >> >> Please let me know what you think; suggestions, comments, other >> feedback welcome. > > One refinement: I think I'd actually go with `TokenStreamFactory` > instead of `StreamFactory`, since that emphasizes token-based nature > of input/output. But then leave implementation name as shorter > `JsonFactory`, `SmileFactory` etc, unless better pattern can be > identified. > > I plan to proceed with this soon, since it seems relatively > straight-forward and non-controversial change, as far as renaming and > refactoring goes. There a few trickier to tackle, in both categories, > > -+ Tatu +- -- You received this message because you are subscribed to the Google Groups "jackson-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
