Add ability to do "deep merge" for JsonNode

[cowtowncoder]

Currently there is no good way to merge two JSON Trees. But with JsonNode it should be relatively simple to add functionality to do that.

[tea-dragon]

I have a method that does something close to that:

https://github.com/addthis/codec/blob/master/src/main/java/com/addthis/codec/jackson/Jackson.java#L133

The particular semantics here are: the primary tree (argument one) is modified, and prefers to keep its own children. The secondary tree is not modified, and I even make deepCopy()ies of children that get copied over to the primary tree. This works well when implementing "default value" logic. You may want to vary who does or does not get modified and what not though. I am guessing it is somewhat use-case specific.

[cowtowncoder]

That actually sounds pretty good as the baseline. One things I always wondered was whether JSON Array values should be combined somehow; I guess there are at least 3 possibilities:

1. Replace primary value with secondary (with deep copy)
2. Append secondary to primary (if one exists; otherwise same as (1))
3. Recursively merge matching entries (i.e. first N entries, where N = min(a1.length, a2.length))

so maybe there'd be need for separate strategy/config object.

As to whether to modify or copy, I guess that to make new copy, it'd be enough to take copy(), then call merge on that.

One other question, too, is what to do with "incompatible" merges (Object vs Array value) -- probably it should either be "overwrite with secondary" or "throw exception". One more setting for strategy/config (and related question of default settings).

Thanks for the link, I hope to have a look at that. As I said, sounds like a good starting point.

Oh. And then there's the question of operation name; ObjectNode.merge() would be one obvious choice. Also: whether method should be available at JsonNode level or not; I am thinking not, as no mutating methods are there anyway. So could go in ContainerNode, since I could see the usefulness of exposing this directly for Arrays (as per above).

[tea-dragon]

My function is somewhat based on hocon merge semantics so it treats every combination other then object-object the same. That is, the value is copied over if and only if it did not previously exist.

That is somewhat of a cheat though since they also handle array-array concatenation vs replacement via special syntax.

[cowtowncoder]

So, would it NOT overwrite scalar values?

[tea-dragon]

Well, it is a static method, so that is just a matter of perspective/ order of the arguments. You could say that the "secondary" tree's scalar values get overwritten. Although, in my particular case, I do not mutate the second argument. It would be simple to change that though, and the end result is still a tree object whose scalar valued children were preferentially derived from one parent over another.

My use case was referencing a tree for "default values". If your semantics are better described as "overwriting/ updating" then the key difference is simply the order you feed in the arguments. (ignoring which argument does or does not get mutated, which is at most a performance concern given that you can just copy both arguments before hand if you want to).

[cowtowncoder]

Right, understood. I think both ways could work indeed; sort of "merge from" (defaulting) and "override with" (overrides). And then details of how to delegate, who should be responsible for pulling what.

Doing this externally has its benefits, but so does a method; latter has more access, knows bit more. But external could be more generic, easier to override handling.

[tea-dragon]

Given the choice, I would probably start with external functions, and then if you run into something that can't be done without access to certain internals, then you can switch over. You can always create methods that delegate to static functions by default, and if you end up deciding against external functions entirely, it is just a couple clicks in intellij to inline them (which would just fill in the object methods).

As a third party, it was an easy choice to use a static function for my original goal. Not really worth extending ObjectNode et al. Your decision is a bit less straightforward.

[cowtowncoder]

I think that for external functions, the best place then is probably outside of Jackson databind -- I am not fan of "Util" classes. So could be 'jackson-merge' package or something.
Put another way: if it need not be within some classes that already exist, it might as well be truly external.

I'll think about this some more, either way.

[cowtowncoder]

This should be covered by #1399; the only caveat being that node types are sometimes handled in a bit of a special way.

[cowtowncoder]

Closing for now, assuming #1399 suffices; if not, may be re-filed with explanation of how improved functionality would work with JsonNode API -- I am not too keen on adding separate handlers for stand-alone operation into Jackson core (they work just fine externally -- maybe new module or library).