Is the only way to configure ObjectMapper in Json class to access it directly?

[Psycho Punch]

I'm trying to register some additional modules to the ObjectMapper used by the Json class. I just want to check if the only way to do this is by directly accessing the ObjectMapper through Json.objectMapper (or Json.prettyMapper). Personally, I find this a little worrisome as practically mutating a static object accessible from anywhere can be pretty dangerous, but if this is the only way, then I guess I have not much choice (other than, of course, using my own ObjectMapper).

[Thomas SEGISMONT]

I'm not aware of any other way.

2017-06-02 20:43 GMT+02:00 Psycho Punch <rdg...@gmail.com>:

I'm trying to register some additional modules to the ObjectMapper used by the Json class. I just want to check if the only way to do this is by directly accessing the ObjectMapper through Json.objectMapper (or Json.prettyMapper). Personally, I find this a little worrisome as practically mutating a static object accessible from anywhere can be pretty dangerous, but if this is the only way, then I guess I have not much choice (other than, of course, using my own ObjectMapper).

--
You received this message because you are subscribed to the Google Groups "vert.x" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vertx+unsubscribe@googlegroups.com.
Visit this group at https://groups.google.com/group/vertx.
To view this discussion on the web, visit https://groups.google.com/d/msgid/vertx/8354e032-57f7-48c2-8e2e-0edde30f8161%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Tim Fox]

I guess you're suggesting that each Vert.x instance instead maintained its own mapper instance? The trouble with that is every JsonObject and JsonArray would then need to have a reference to that instance, so you'd have to pass it in every time you constructed one - I'm sure that wouldn't be a popular solution for our users.

[javadevmtl]

If I recall there is an issue registered to give us access somehow to register our own modules?

[Tim Fox]

Maybe, but that would still involve statics afaict.

[Psycho Punch]

Nope, that's not what I'm suggesting. Pretty much the Json utility is independent from any particular instance of Vert.x instance anyway. I guess I'm just concerned that accessing the ObjectMappers used by the Json class directly through Json.objectMapper is generally seen as bad design. I guess if I were to implement this, I'd keep Json mostly as is keep the ObjectMappers to itself. Any other client code that would want to register custom modules can just get their own instance of Json.

Json.getInstance().decodeValue(...);

The original Json.decodeValue(...) can be delegated internally for backward compatibility? Something like:

static ... decodeValue(...) {
    return getInstance().decodeValue(...);
}

Alternatively, for usage with custom modules:

Json.withModule(moduleA).withModule(moduleB).build();

[Tim Fox]

On Wednesday, 7 June 2017 19:40:02 UTC+1, Psycho Punch wrote:

Nope, that's not what I'm suggesting. Pretty much the Json utility is independent from any particular instance of Vert.x instance anyway. I guess I'm just concerned that accessing the ObjectMappers used by the Json class directly through Json.objectMapper is generally seen as bad design.

In what sense? From your previous post I assume you're referring to the use of statics?

 

I guess if I were to implement this, I'd keep Json mostly as is keep the ObjectMappers to itself. Any other client code that would want to register custom modules can just get their own instance of Json.

Json.getInstance().decodeValue(...);

Isn't Json.getInstance() another static method?

 

The original Json.decodeValue(...) can be delegated internally for backward compatibility? Something like:

static ... decodeValue(...) {
    return getInstance().decodeValue(...);
}

I'm not sure I understand what this adds. If the objection is to statics, then this solves nothing.

[Psycho Punch]

On Thursday, June 8, 2017 at 3:05:00 AM UTC+8, Tim Fox wrote:

I'm not sure I understand what this adds. If the objection is to statics, then this solves nothing.

 

It's not the static access per se that's the problem; it's the global access to the ObjectMapper that is supposed to be encapsulated inside Json. The code I provided is meant to illustrate how I think it would be better to design encapsulation, by treating Json class as the utility object itself rather than just essentially a collection of utility methods.

Effectively, in the approach I illustrated, we keep a system-wide singleton of the default Json object accessible through the Json.getInstance() method. The builder pattern though provides a way to customize the default settings, which was optimized for use with Vert.X, with new Jackson modules, and maybe other configuration. This way, client code can decide whether to stick with the defaults, or roll with something on top of the base utility code. However, building a custom Json utility object will not affect the default singleton that will always work the same way.

[Tim Fox]

I really don't understand what you're suggestion. If Json.getInstance() returns a singleton, and then you configure that, it's going to effect everyone. No different to the current behaviour.

 

[Psycho Punch]

On Thursday, June 8, 2017 at 5:35:21 PM UTC+8, Tim Fox wrote:

I really don't understand what you're suggestion. If Json.getInstance() returns a singleton, and then you configure that, it's going to effect everyone. No different to the current behaviour.

 

No, like I said earlier you do not expose the ObjectMapper anymore in this approach. You encapsulate Json.objectMapper, which is the source of concern to begin with. Client code either sticks with the default configuration (Json.getInstance()) or build their own based on the given defaults (Json.withModule(...).build()). The builder pattern will create new ObjectMapper for each custom Json utility object.

[Tim Fox]

So.. if your code decides to use a custom Json instance, not the default one, how are JsonObject and JsonArray going to know which one to use for encoding/decoding?

 

[Psycho Punch]

On Thursday, June 8, 2017 at 5:52:19 PM UTC+8, Tim Fox wrote:

So.. if your code decides to use a custom Json instance, not the default one, how are JsonObject and JsonArray going to know which one to use for encoding/decoding?

 

You can create factory classes for those that use Json utility object with custom ObjectMapper, or a more simpler approach is to pass the Json utility class you want JsonObject or JsonArray to use when encoding/decoding:

JsonObject jsonObject = new JsonObject(jsonString, Json.withModule(...).build());

[Tim Fox]

Right so you'd have to pass a reference to the mapper (or enclosing class) into each construction of a JsonObject/JsonArray, which was the point I made in my initial reply on this thread:

[Psycho Punch]

Not when you use factory, instead. I personally think that either of those is better than exposing ObjectMapper globally, which allows any client code to configure while it's being used elsewhere. While ObjectMapper instances are pretty much thread safe, the caveat is that no other code should call configure it after it's been set up, but there really is nothing stopping that from happening. From the documentation:

Mapper instances are fully thread-safe provided that ALL configuration of the instance occurs before ANY read or write calls. If configuration of a mapper is modified after first usage, changes may or may not take effect, and configuration calls themselves may fail.

[Tim Fox]

At this point I am completely confused as to what you are suggesting :(

How does using a factory prevent you having to specify the mapper when constructing a JsonObject?

[Tim Fox]

Without going into crazy hacks there are only three ways that a JsonObject/JsonArray can know what mapper to use:

1. Use a global static. This is what we currently do.

2. Specify the mapper to use when constructing the object.

3. Use a thread local to store the mapper.

2. Has the disadvantage of users having to specify this every time they construct the object.

3. Don't get me started on thread locals!

[Psycho Punch]

On Thursday, June 8, 2017 at 7:08:45 PM UTC+8, Tim Fox wrote:

At this point I am completely confused as to what you are suggesting :(

How does using a factory prevent you having to specify the mapper when constructing a JsonObject?

Something like:

JsonObjectFactory factory = new JsonObjectFactory(Json.withModule(...));
JsonObject fromString = factory.decode(jsonString);
JsonObject fromMap = factory.create(jsonMap);

Users can pass this factory around the system without having to worry about the ObjectMapper being reconfigured elsewhere. Most fairly complex projects will have dependency injection mechanisms that will make sharing of this factory pretty trivial.

[Tim Fox]

On Thursday, 8 June 2017 12:24:38 UTC+1, Psycho Punch wrote:

On Thursday, June 8, 2017 at 7:08:45 PM UTC+8, Tim Fox wrote:

At this point I am completely confused as to what you are suggesting :(

How does using a factory prevent you having to specify the mapper when constructing a JsonObject?

Something like:

JsonObjectFactory factory = new JsonObjectFactory(Json.withModule(...));
JsonObject fromString = factory.decode(jsonString);
JsonObject fromMap = factory.create(jsonMap);

Users can pass this factory around the system without having to worry about the ObjectMapper being reconfigured elsewhere.

Well, passing references around everywhere was exactly what I was hoping to avoid :)

[Psycho Punch]

On Thursday, June 8, 2017 at 7:34:17 PM UTC+8, Tim Fox wrote:

Well, passing references around everywhere was exactly what I was hoping to avoid :)

To be honest, that is a pretty weird thing to avoid. It's not any more tedious for users who do that most of the time anyway.

[Tim Fox]

It's actually a very common theme - users want to avoid passing things around too much, it's a lot of extra work for them to do that. The current thread on MDC context is one such example of a thread where we're trying to avoid passing things around.

I'd be pretty sure that if we instituted this change it would be extremely unpopular, I think most users would rather do:

JsonObject obj = new JsonObject();

than

JsonObject obj = new JsonObject(someReferenceImGonnaHaveToPassToEveryMethodWhereICreateAJsonObject)

Not to mention this would break almost every Vert.x application to date... ;)

It might be possible mitigate this to some extent using dependency injection. But don't assume most Vert.x users use DI, in my experience it's a minority that do.

But maybe I am just "weird" ;)

[Psycho Punch]

On Thursday, June 8, 2017 at 9:22:09 PM UTC+8, Tim Fox wrote:

It's actually a very common theme - users want to avoid passing things around too much, it's a lot of extra work for them to do that. The current thread on MDC context is one such example of a thread where we're trying to avoid passing things around.

I'd be pretty sure that if we instituted this change it would be extremely unpopular, I think most users would rather do:

JsonObject obj = new JsonObject();

than

JsonObject obj = new JsonObject(someReferenceImGonnaHaveToPassToEveryMethodWhereICreateAJsonObject)

I mean, really, how hard is it to pass just one more argument? I guess if you name variables that long, it's gonna take a bit of effort to type, but most modern IDE's will easily autofill that for you.
 

Not to mention this would break almost every Vert.x application to date... ;)

It wouldn't if you continue supporting old interfaces, and just marking them as deprecated. I've shown in one of my previous replies that it can actually be done. By aliasing Json.getInstance().decodeValue() with Json.decodeValue(), you keep the old interface, for example.
 

It might be possible mitigate this to some extent using dependency injection. But don't assume most Vert.x users use DI, in my experience it's a minority that do.

But maybe I am just "weird" ;)

 

 Even without dependency injection, there are many ways to mitigate this. Users can custom build their own utility classes that will allow them to use any of the singletons here anywhere they please.

class Singletons {

    public static final Json JSON_JDK8 = Json.withModule(...).withModule(...).build();
    public static final JsonObjectFactory JSON_OBJECT_FACTORY = new JsonObjectFactory(JSON_JDK8);

}

At the end of the day, you just try to get the encapsulation right.

Anyway, if you're not convinced this is something worth looking into, that's fine. But by exposing ObjectMapper this way, the Vert.x API inherits the same caveat.

[Tim Fox]

Hey, I'm just a user, I have no power around here.

There's never any harm in submitting a pull request :)