`MissingNode.toString()` returns `null` (4 character token) instead of empty string

[arakelian]

Recently upgraded from 2.9 to 2.10.1, and several internal unit tests failed. Determine that cause of failure was toString() method was removed from MissingNode.

[arakelian]

Just read blog post https://medium.com/@cowtowncoder/jackson-2-10-feature-jsonnode-improvements-18894c3ac3b5, which said: "Although users sometimes used toString() implementation of JsonNode values, this was actually not supported usage by Jackson.".

I'll close ticket, because I assume this change was by design, and I'm supposed to use asText(). My only input is that I wish the major version was bumped with breaking changes like this, Apache style. Thanks for an excellent library.

[arakelian]

I changed my mind on this one, after looking at the code before/after. Open to your thoughts, @cowtowncoder.

Here's the new code in 2.10 (see BaseJsonNode):

   @Override
   public String toString() {
       return InternalNodeMapper.nodeToString(this);
   }

   @Override
   public String toPrettyString() {
       return InternalNodeMapper.nodeToPrettyString(this);
   }

I read this code as saying, "if you want a pretty printed rendering of the Json, use toPrettyString(), or, for non-pretty version, using toString()".

And looking at the toString() method for MissingNode in 2.9, we see:

@Override
public String toString() {
    // toString() should never return null
    return "";
}

As a user of version 2.9 of the library, I think it's reasonable to believe that MissingNode would continue to return an empty string in version 2.x of the library (e.g. 2.10+) on call to toString().

That's also what I got from your comment this ticket (at the bottom): #25. You wrote: "Anyone reading this issue: if you would prefer to see a String value (most likely either "" or "null"), please file a new issue with the request for change. Such could be considered for Jackson 3.0."

Personally, I like the current behavior. I think returning "null" for a MissingNode.toString() is counter-intuitive. A NullNode represents a "null" value. MissingNode represents something entirely different -- it's really just a placeholder, sort of like java.lang.Optional, to avoid null checks everywhere.

Thanks for your consideration.

[cowtowncoder]

Hmmh. Tricky question, this one. This was not an intentional functional change, although I am unsure how to categorize this since behavior of toString() for JsonNode was never specified as anything that users should rely on, aside from diagnostics output.

My first thought is that the ideal outcome of attempts to serialize MissingNode would perhaps be throwing IllegalArgumentException (or UnsupportedOperationException), since it does not represent any legal value, but absence of one. Sort of like Java 8 Optional.

But given that there is existing behavior that is not completely unreasonable -- output of "" -- that does sound like it should either have been continued as such, or at very least should not have become same as NullNode.

I will have to think about this a bit, but I am leaning towards your suggestion of reverting the change for 2.10.2.

[cowtowncoder]

Ok so for what it is worth, the way things work in 2.10 (before I change it) is:

1. Instead of explicit toString(), actual serialization methods are used (serialize())
2. Since serialization these methods must produce valid JSON (so MissingNode can not simply not output anything without potentially causing exception), MissingNode chose to write VALUE_NULL

Interestingly enough there is actually a test (TestMissingNode) that suggests null is expected, to keep behavior same as if explicitly serializing as json.