Shouldn't Long and Int Nodes be equivalent?

[tommack]

If I build an ObjectNode with small "Long" or "long" values, I cannot round trip the the node into an equivalent node.

    @Test
    public void test_long_obj() throws IOException {
        ObjectMapper om = new ObjectMapper();
        ObjectNode node = om.createObjectNode();
        // this node tree will contain a LongNode
        node.put("long", Long.valueOf(1234));
        String string = om.writeValueAsString(node);
        // this newTree will contain an IntNode
        JsonNode newTree = om.readTree(string);
        // this will be false because IntNode never equals LongNode
        assertEquals(node, newTree);
    }

    // this test also fails

    @Test
    public void test_long_primitive() throws IOException {
        ObjectMapper om = new ObjectMapper();
        ObjectNode node = om.createObjectNode();
        node.put("long", 1234L);
        String string = om.writeValueAsString(node);
        JsonNode newTree = om.readTree(string);
        assertEquals(node, newTree);
    }

    // these two tests pass

    @Test
    public void test_int_obj() throws IOException {
        ObjectMapper om = new ObjectMapper();
        ObjectNode node = om.createObjectNode();
        node.put("int", Integer.valueOf(1234));
        String string = om.writeValueAsString(node);
        JsonNode newTree = om.readTree(string);
        assertEquals(node, newTree);
    }

    @Test
    public void test_int_primitive() throws IOException {
        ObjectMapper om = new ObjectMapper();
        ObjectNode node = om.createObjectNode();
        node.put("int", 1234);
        String string = om.writeValueAsString(node);
        JsonNode newTree = om.readTree(string);
        assertEquals(node, newTree);
    }

Shouldn't the same logic that is used when deserializing the String be used when creating the tree? If I pass in a long value that will fit in an int, why not create an IntNode? Is there a way for me to access this logic when I'm creating a tree? Is there a way to force a tree into a canonical representation that can be compared against another tree? Is there a method besides "equals" that compares two trees as JSON entities and not Maps and Lists of Java Objects?

[cowtowncoder]

Equality in Java is very tricky to get right, and unfortunately I am not sure what the right answer is there. One can certainly argue that comparison for special case of long vs int should produce equality; but I have this nagging feeling that something, somewhere would break...

The idea about coercing creation of LongNode into IntNode iff value fits in range makes sense to me, and should reduce likelihood of problems. This should probably work for other cases too (Short).

As to other parts; when constructing trees from JsonParser, limits are used as expected. There is no code to canonicalize things at this point.

I think this is something that might be good to discuss on jackson-dev list: there are others who have keen interest in tree model and especially numeric aspects. I am not the supreme authority in this particular part, even though I have done most changes. :)

[cowtowncoder]

Quick note: construction of canonical NumberNodes is possible with custom JsonNodeFactory, even if default implementation did construct exact requested type.

I will make one immediate change, such that default JsonNodeFactory will only use LongNode if value range requires it; otherwise it will use IntNode.

Regarding equality; the reason I think this can not be solved well with any JsonNode equals() implementation is better explained by others (Josh Bloch wrote about it in "Effective Java", I think, but others have as well).
One obvious problem is transitivity (all types have to know how to compare with all others); and extension by sub-classing will make it even more difficult problem.
This is to say: for semantically correct equality comparison it is probably necessary to use explicit, external code, where caller can specify exact semantics needed.

[tommack]

I think your suggested change to JsonNodeFactory would solve my particular case. Thanks!

[cowtowncoder]

One more note: #790 addresses the equality question as well, allowing bit simpler external/custom equality comparison.

[cowtowncoder]

Actually, no. I have to contradict myself and claim that it does not make sense to add such logic into JsonNodeFactory. For 2.6.0, I will add a switch that will allow such coercion, but leave it disabled by default. Otherwise we can not support features like #504 which will need more control over exact kind of NumberNode being constructed.

This should not lead to changes in actual JsonNode tree being built, as coercion should occur based on incoming NumberType anyway.

[cowtowncoder]

Come to think of it, I won't add a configuration method. It is easy enough to sub-class factory, add functionality that way; and adding more state into base implementation seems like a wrong idea due to extensive sharing of the default node factory.