FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
314 stars 136 forks source link

Create SmileMapper based on a existing ObjectMapper? #205

Closed StephenOTT closed 1 year ago

StephenOTT commented 4 years ago

is it possible to replace a JsonFactory in a copy of a ObjectMapper, or configure the SmileMapper to use all of the configs of a existing ObjectMapper?

cowtowncoder commented 4 years ago

As of 2.x, neither is possible, although you can replace JsonFactory for ObjectReader and ObjectWriter using with(JsonFactory) method. This should be closest to what is possible. In theory it would seem quite easy to add overload of copy() method for ObjectMapper but I am bit hesitance because all of this will become impossible in 3.0 -- mappers will be more closely bound to TokenStreamFactory (new base class of JsonFactory). This is done to allow more format-specific configuration.

But it is an interesting use case still, since much/most of ObjectMapper configuration is still shared even if at lower level (TokenStreamFactory) more things are not shared (and yet even there some ware). 3.0 adds new ObjectMapper.rebuild() option, too, to replace copy() as mappers become truly immutable and all configuration moves to builders (and remaining configurability for ObjectWriter / ObjectReader instances).

Which settings are you specifically interested in keeping? I'd like to understand usage to think of whether and how support could be added.

StephenOTT commented 4 years ago

My use case is around Hazelcast usage:

I have my primary ObjectMapper that is used for regular JSON with HTTP requests, etc. But i also have the SmileMapper for serialization of objects in Hazelcast.

All of the de/serializers, preferences, etc are the same in both. It is just that Hazelcast requires the binary format.

StephenOTT commented 4 years ago

@cowtowncoder the with(...) looks like a good solution. The with() would not be called continually? It should be similar to a app wide instance? With a reader and writer pre-configured with the smile json factory?

cowtowncoder commented 4 years ago

@StephenOTT For this particular setting I would probably try to keep references to ObjectWriter and ObjectReader (assuming you need both read and write; if not, just applicable one), although overhead of with(...) itself is negligible (it literally just creates new instance with nothing else).

StephenOTT commented 4 years ago

@cowtowncoder after some further investigation into the use case, I have found that with() is good in principal but ultimately fails due to module reuse:

The higher level scenario here is you have your JSON mapper for typical json usage (APIs, etc). But then you have your Smile mapper for storage/transit purposes. Generally you want the same configurations. BUT you end up needing to remove your custom serializers/deserializers, etc, if you have injections of your higher level services.

you end up with a DI circular dependency because the de/serializers may have DI in them.

So it would seem a more powerful copy() would be better. Looking at the copy() method, it would be interesting to make this a functional interface, so we can modify the objects/logic of the copy as a lambda.

such as

val newMapper = objectMapper.copy { copyConfiguration ->
....
}

Where copy configuration is some wrapper class that holds all of the refs that are in the ObjectMapper copy constructor protected ObjectMapper(ObjectMapper src)

The goal being to provide some more granular copy controls to prevent certain parts from copying over (such as certain modules, replacement of JsonFactory, etc) (it could even be to remove all modules, and you have to explicitly define the re-import).

cowtowncoder commented 4 years ago

While I can see added power from lambda-style approach (can't be real lambda since Jackson 2.x is still Pre-java8 minimum, but functional interface would do fine), it also sounds like bit of a potential rat hole just wrt complexity (of figuring out API to use).

It is too bad that 3.0 is so far out (and moves things in bit different direction), since it will cleanly separate configuration behind builders, and sort of solves the problem with module registration you refer to: in 2.x, module registration simply causes configuration changes and there is no way to separate such settings from any other changes. 3.0 changes this completely so that there is a full copy of configuration settings Builder has, including Modules to add -- and although build() will let Modules make configuration changes for the instance, there is also a copy of settings before that, so ObjectMapper.rebuild() recreates 100% same configuration to allow addition/removal of Modules. At the same time, as of now there is no way to "mutate" mapper of different type: this is something I might want to add in master for 3.0 (although with caveat that only shared features would be copied). I like this idea and will file a new issue for this. (there are a few complications I can think of , but best discussed under separate issue).

But back to 2.x and specific problem: the only really immutable aspect of ObjectMapper is JsonFactory, so copyWith(JsonFactory) might be sufficient for just that original aspect. Yet for Module issue, it would not really solve the problem -- from application/framework end, dividing construction into different parts, configuration first, module adds second, copy() in-between could solve that, but in context of frameworks may not be practical?

Since registration effects of modules can not really be undone, I guess the main alternative addition could be to have some sort of "partial copy" that would only copy some subset of settings, like XxxFeatures. But beyond that there are many other settings and this is where complexity seems difficult to manage....

So I don't know of clean solution. But it could perhaps help to know sort of prioritized list/set of configuration settings that would be most valuable to retain? Alternatively maybe addition of methods to remove registered handlers ("clear all custom [de]serializers") would make sense.

pfyod commented 1 year ago

This seems to be possible now with the ObjectMapper.copyWith(JsonFactory) method (see https://github.com/FasterXML/jackson-databind/issues/3212)

cowtowncoder commented 1 year ago

Good point @pfyod. Should work for Smile mapper at least. Closing.