Merging JSON Structure

[ariens]

I'm using Dropwizard to serve up REST-ful APIs which of course use Jackson to serialize/de-serialize the request/response. I've written an abstract class that simplifies persisting and retrieving those API objects to ZooKeeper.

https://github.com/blackberry/BB-BigData-Common-Utils/blob/0.1.0/src/main/java/com/blackberry/bdp/common/versioned/ZkVersioned.java

That class also has some helper logic around associating named roles to Mixins that allow me to suppress certain attributes. When I update those partially restricted objects I first fetch the stored object, then merge in the object from the request. I use ZooKeepers built in znode version metadata to ensure that there are never any overwrites/race conditions.

Anyway--can you please review the merge() method from the above class and advise if:

a) There's already some code in Jackson somewhere that does this, or if not
b) If there's a possibility of including this in Jackson

This pattern of Dropwizard, Jackson, ZkVersioned has allowed me to very rapidly build out some pretty impressive APIs, but I'd rather not have to own the rather complex json merging logic. I know enough about Jackson/json to write something that passes my tests, but for all I know I may have made assumptions that aren't accurate, or could have bugs...

From the jackson-user list:

Yes indeed, the idea of merging has been floated around for quite a long time.

There are multiple approaches to take, and the use of JsonNode is probably the simplest. The other main alternative is to try to do it at streaming level (JsonParser), but that is more challenging.

Going with JsonNode idea, there is then the distinction between trying to extend JsonNode itself, and external handlers. I generally prefer external approach, although from usability perspective it is bit more cumbersome compared to invoking methods on JsonNode.

So far so good: I think your code makes sense. The next question would be as to where should it go. While ObjectMapper would seem like the usual place, longer term strategy has been to work more on ObjectReader/ObjectWriter. Perhaps "both" is the answer here (i.e. can invoke via ObjectMapper or ObjectReader).

Assuming functionality was integrated, the next thing users tend to ask are overrides to behavior; some users do not like existence of nulls; some want to append array contents whereas others want to only use ones from second array, except if first one exists. And so forth.
So the code that does actual merging should probably be pluggable; meaning that it'd be good to specify a simple interface for ObjectMapper/ObjectReader to invoke, to do merging, along with standard/default implementation.

It is also possible that instead of modifying one of the trees, code should instead build a new tree; such immutable handling is useful for many cases.
I don't know if it is possible to easily combine approaches under same API.

But I digress. :)

So: could you file an issue under jackson-databind, for adding the merge feature (there may or may not be such an issue; I can verify and close older one if we have), linking to your implementation?
From that point on, someone can come up with a PR (possibly me, or perhaps someone with an itch to get this in).

Thank you for the idea, sample implementation,

-+ Tatu +-

After spending the weekend thinking about this issue, I suggest this feature be implemented within the JsonNode class. This would provide the most flexibility. One could merge the full representation of theiir JSON structure or just as easily merge only a sub-node via myJsonNode.get("node1").get("node2").get("node3").merge(anotherJsonNode). They wouldn't need to handle instantiating a new JsonNode to hold the sub-structure, then merge, then casting the original to an ObjectNode to set the merged node. This would also make it easier for folks that don't need full/deep recursion and only wish to merge a specific sub-node. For those users then the tricky implementation details (below) aren't as important.

As for implementation, I agree that this functionality should be introduced via a well defined interface, with a default implementation that uses a default configuration. I would like to see some reasonable defaults (which I attempted to capture with my explicit null and missing node handling) that could be overwritten with optional pre-defined behaviour (ignore null: bool, ignore missing: bool, etc) or even replaced with user-defined callbacks. I'm not super knowledgeable about best practices in this area but providing there's an extensible default implementation, that should sufficient.

[cowtowncoder]

I think this would, then, be covered by #584.