FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.53k stars 1.38k forks source link

Parse as JsonNode w/o Java Type information #3482

Open dude-abides opened 2 years ago

dude-abides commented 2 years ago

If I compare a JsonNode parsed from raw JSON to a JsonNode generated from a Java Object, the test will fail.

Raw JSON:

{  "id": 1 }

Java Object:

public class Datum
{
  public long id = 1;
}
new Datum()

The equals method will return false even though the JSON representing both sides are identical. Its because Jackson is aware of the Java Datatype and the equals function fails because one is int and the other is long. It is counter intuitive.

Describe the solution you'd like 1) Have an equals method that compares JSON Datatypes not Java Datatypes, or 2) Have a way to convert a Java object into a JsonNode that independent of the underlying Java Class.

This seems a common need for testing.

Usage example If you have a clear idea of how to use proposed new/modified feature, please show an example.

Additional context Add any other context about the feature request here.

cowtowncoder commented 2 years ago

I am not sure I understand this exactly, but I suspect the difference is for number where long property value will become LongNode whereas regular 1 from JSON would become IntNode. These are not considered equal.

If this is the underlying reason, I don't think this can be reliably changed because it would require complicated logic in equals() methods to coerce different integral number types.

However: to support this use case (it has been requested before) you should be able to implement Comparator<JsonNode> and then call JsonNode.equals(comparator, otherNode) -- your Comparator can implement all equality checks as you prefer; some delegating to simple JsonNode.equals(other), others using coercion logic.

dude-abides commented 2 years ago

Bottom line is that you can take a Java object, serialize it to JSON, read the JSON back in and compare the root JsonNode of the Java Object and the parsed JSON and the will not equate. Certainly you can see how counterintuitive that is.

This seem to be a common unit test scenario. Store some expected result as JSON, invoke a REST endpoint, parse the returned Java Object and compare to the JSON in the file (reference).

cowtowncoder commented 2 years ago

While I agree this is unexpected, I tried to explain why this occurs, and reasons why this is nor easy to fix nor likely to be changed in near future. And then suggesting how to be able to do comparison you need. Seems like I failed with my explanation.

But I'll try again. The issue is two fold.

First, implicit type information: JSON has only "numbers" (or, possibly integer and floating-point numbers), but no further subtypes. Java has multiple integer types. When parsing JSON, "smallest" type from (int, long, BigInteger) that can represent JSON integer number is used. There are matching JsonNode (or NumberNode) subtypes matching these integer sizes. Number 1 becomes IntNode.

When converting from POJO into something else, contents are "written" into TokenBuffer. Actual POJO property type is retained: so long value of 1L becomes LongNode.

Second part, then, is the complications with actual comparison; Java equals() implementations across different physical types -- but with same logical contents, by some logic -- are complicated to implement. Worse, for some use cases one may want to distinguish between type (IntNode vs LongNode) as well. This is why external comparator support was added: to allow for developers to define their own more useful equality comparisons.

As to changing existing implementation: I could see the possibility of maybe forcing a "resize" on write methods of TokenBuffer which would make method writeLong(1L) essentially becoming writeInt(1); when some configuration setting is enabled. That would solve this problem and possibly a few similar use cases. I would be open to such setting, although not as default one (since this is a change in behavior and someone somewhere is likely to be relying on it, as surprising as it may sound).