`TreeBuildingGenerator#writeNumber(String)` writes Number as String

[gaeljw]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

Hello folks,

As part of some work in upgrading from Jackson 2.x to 3.x (but I think it's unrelated in the end), I came upon the behaviour of TreeBuildingGenerator#writeNumber(String) which seems odd to me.

jackson-databind/src/main/java/tools/jackson/databind/node/TreeBuildingGenerator.java

Lines 407 to 409 in f946c3d

	public JsonGenerator writeNumber(String encodedValue) {
	return writeString(encodedValue);
	}

It writes the number (encoded as a string in input parameters) as a string in the underlying representation, especially it adds quotes around the input string. And it's then deserialized as a StringNode rather than a NumericNode.

This doesn't match what other implementations of JsonGenerator do, like WriterBasedJsonGenerator where the fact that it's a number is preserved: https://github.com/FasterXML/jackson-core/blob/0406a51a186125a451e3353ee1824f9a0407b152/src/main/java/tools/jackson/core/json/WriterBasedJsonGenerator.java#L907-L918

Am I missing something ; is this the intended behaviour?

Version Information

3.0.3

Reproduction

None yet but I can come up with something if needed.

Expected behavior

Write numbers as numbers.

Additional context

No response

[pjfanning]

I don't immediately see a tidy way to create a TreeNode representing a number where you start with a number in text form.
This might be useful as a utility method and may exist but I don't see it. A method that looks at the String and creates a Numeric Node that best fits the number.

@gaeljw is there any way that you could work around this issue until we can get a fix out by converting your number Strings to TreeNodes yourself? Then you can call writeNumber(TreeNode).
DecimalNode.valueOf(new BigDecimal(str)) is one way to do this but you might want to choose Node types that match the number type (LongNode, DoubleNode, etc.).

[gaeljw]

I can sure work around this for now, no worries :)

[cowtowncoder]

Hmmmh. This is tricky -- I'll have to think about it. Definitely a flaw, although use of writeNumber(String) will (and should) also fail on all binary-backed format generators: intent is NOT to convert from String to Number but allow optimized short-cut for specific textual formats (like CSV or YAML).

Ideally TreeBuildingGenerator would retain information to call writeNumber(String) on target generator, but not sure if that is possible or not, without digging deeper.

Alternatively perhaps this method should outright fail with an exception. That might be better option than failing in non-obvious way -- it really should not become JSON String.

[gaeljw]

Alternatively perhaps this method should outright fail with an exception. That might be better option than failing in non-obvious way -- it really should not become JSON String.

This sounds reasonable to me.

I guess usage of this method is quite rare, otherwise the bug would have been caught earlier 🤷🏼‍♂️

[cowtowncoder]

Yeah whole TreeBuildingGenerator is internal class, used only by ObjectMapper.valueToTree(). So I'm actually curious how it ended up hitting the issue since conversion should not trigger that call path...

[gaeljw]

@cowtowncoder the call path is as follows:

• mapper.valueToTree for a value of a custom type T
• this custom type T is registered with a JsonSerializer<T>
  • the serialize method calls generator.writeNumber(String)

Complete code: https://github.com/playframework/play-json/blob/main/play-json%2Fjvm%2Fsrc%2Fmain%2Fscala%2Fplay%2Fapi%2Flibs%2Fjson%2Fjackson%2FJacksonJson.scala#L102 (note that this actual version of the code does not hit this code path because it ends up using another JsonGenerator class due to using mapper().getFactory.createGenerator(out) to get a generator which I removed as it felt unnecessary initially)

Thanks for the quick feedback and action anyways, happy holidays folks 🎅

[pjfanning]

@gaeljw I wonder if play-json should create its own custom JsonGenerator class that includes the number formatting logic from play-json. The custom class could extend WriterBasedJsonGenerator.

https://github.com/playframework/play-json/blob/db3a71e9f643c33550e731146c6093da0b69132f/play-json/jvm/src/main/scala/play/api/libs/json/jackson/JacksonJson.scala#L88-L93

play-json would then be able to call jgen.writeNumber(BigDecimal v) and the custom class would use the number formatting logic in JacksonJson (above link).