Feedback on adding JSON Pointer based add methods for JsonNode

[Tatu Saloranta]

There is one potentially great addition that may go in 2.4:

https://github.com/FasterXML/jackson-databind/pull/393

contributed by Jeremy G. I think it looks very promising.

But I wanted to get others ideas, suggestions & comments on this one.
One conceptual concern I have is that JsonNode so far has only exposed read-methods. This is intentional, partly since mutate methods vary between object and array nodes; but more imporantly to make it possible to implement alternate tree models.
Whether this is something that makes sense is open for debate; especially given that there is new base type, TreeNode, which is a subset, and because JsonNode itself is probably not something that can act as a base of truly pluggable alternate tree model.
So it may be just as well to allow addition of these methods.

In addition, due to use of generic JsonPointer, this approach may be bit more general as well.

What do you think?

-+ Tatu +-

ps. Comments and suggestions for smaller details are fine too; for minor tweaks it may be best to add suggestions directly on pull request.

[Jeremy Gustie]

Hi, I am the author of PR #393. I am trying to see if there is any interest to get JSON Pointer based mutators added to the JsonNode in 2.7. The changes offer a simple generic means of modifying a JSON tree; in their current state, just two methods are introduced:

JsonNode add(JsonPointer, JsonNode)

JsonNode remove(JsonPointer)

The add method returns a "this" reference, the remove method returns the removed node or null if nothing was removed. Both methods fail with an IllegalArgumentException if the path is invalid. Both methods can also fail with an UnsupportedOperationException if the path resolves to a node which cannot be modified (e.g. you can't add a node to a value).

These additions greatly simplify the implementation of something like JSON Patch without introducing anything too specific to any one standard. I noticed a separate discussion (https://groups.google.com/forum/#!topic/jackson-dev/rhkjVcw_FSk/discussion) about the potential of adding JSON Patch support. I did start an RFC 6902 implementation based on these changes, it wasn't a lot of code and I could dig that back up so the JSON Patch effort isn't starting from scratch.

I would be interested if anyone has any feedback on the approach, or if they would find this addition useful. Of particular interest: are the return values and exceptions used by the current implementation sufficient or is there a better approach I did not consider?

-Jeremy

[Tatu Saloranta]

It would be great to get some comments on this, if anyone has opinions.

Initially I was bit concerned on adding mutability methods (add, remove) in JsonNode, which for now has only contained read-methods. But after thinking about it a bit, I think that it is acceptable trade-off (with proper failure modes) between convenience and purity.

But the return value is something I still feel ambivalent about. Most other methods allow "fail-safe" operation: for example, using "node.path("someField")" will never fail OR return null -- if no such property exists, a `MissingNode` instance is returned; node which may still be traversed, but never evaluates to anything.

In same way, one could suggest that `add()` and/or `remove()` could quietly ignore operations that are not applicable (add/remove on scalar value node, or even MissingNode), and either return normally (where 'this' is returned), return bogus node ('MissingNode'), or perhaps use boolean value to indicate success/fail of the operation.

Alternatively, one may choose to throw an exception on invalid operation. There are some benefits to doing this as well.

Regardless, I think this is a great addition; and in the end in absence of suggestions otherwise we may just proceed with proposed implementation as-is. So I take absence of comments to be tacit LGTM. :)

-+ 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 jackson-dev...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.