Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DecimalNode type not retained during JDK serialization, deserialization #3296

Closed
victornoel opened this issue Oct 6, 2021 · 2 comments
Closed
Labels
will-not-fix Closed as either non-issue or something not planned to be worked on

Comments

@victornoel
Copy link

Describe the bug

When a DecimalNode is cloned via serialization (serialized then deserialized), the clone is not equal to the original.

I know there are a lot of issues related to BigDecimal in jackson, but I don't think I have seen one about this. Don't hesitate to close it if there are other planned/tracked issues that covers this :)

Version information

2.12.1

To Reproduce

    @Test
    public void should_properly_deserialize_decimal_node() {
        var node = new ObjectMapper().getNodeFactory().numberNode(BigDecimal.ONE);
        Assert.assertTrue(node instanceof DecimalNode);
        var clone = clone(node);
        Assert.assertTrue(clone.equals(node));
        Assert.assertTrue(node.equals(clone));
    }

    @SuppressWarnings("unchecked")
    private <T> T clone(T original) {
        try(var baos = new ByteArrayOutputStream(); var oos = new ObjectOutputStream(baos)) {
            oos.writeObject(original);
            try (var bais = new ByteArrayInputStream(baos.toByteArray()); var ois = new ObjectInputStream(bais)) {
                return (T) ois.readObject();
            }
        } catch (Exception e) {
            throw new IllegalArgumentException(e);
        }
    }
@cowtowncoder
Copy link
Member

Thank you for reporting this, @victornoel ! I can see how this might happen, and you are right it is different/not yet reported.

Problem I think is due to JDK serialization of JsonNode basically serializing as JSON, which does not encode actual type information at all -- and when reading back, type might not match.
And come to think of it, some aspects of precision might rely on textual representation as well.

Alternatively this could also be related to the fact that JsonNodeDeserializer only supports targeted reading for ObjectNode and ArrayNode (I think), so even if asked to deserialize as DecimalNode, result might not be that but whatever is "default" JsonNode representation used for floating-point numbers.

I wish I had time to dig into this; seems like there are ways to go about it. Ideally it would "just work", of course.
So leaving open for someone to work on when time allows, interest is piqued.

@cowtowncoder cowtowncoder removed the to-evaluate Issue that has been received but not yet evaluated label Oct 15, 2021
@cowtowncoder
Copy link
Member

cowtowncoder commented Jan 26, 2023

Hmmh. Ok, what is happening is that BigDecimal.ONE, 1, gets serialized as JSON value 1 on serialization.
And gets deserialized as IntNode because, well it is an integral JSON number.

This is due to my deciding to use a compact & light-weight format for serialization of JsonNodes: JSON.
It results in serialization sizes well below what default JDK serialization logic would produce (at least for small graphs).
But it does not always retain the exact Object type as shown here.

I think I'll consider it a flaw but one that probably will not be fixed: effort to create alternate parallel encoding schema that would retain type seems rather expensive for benefits. In general intent with JsonNode really is to handle it as JSON representation.

@cowtowncoder cowtowncoder changed the title DecimalNode is improperly (de)serialized DecimalNode type not retained during JDK serialization, deserialization Jan 26, 2023
@cowtowncoder cowtowncoder added the will-not-fix Closed as either non-issue or something not planned to be worked on label Jan 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will-not-fix Closed as either non-issue or something not planned to be worked on
Projects
None yet
Development

No branches or pull requests

2 participants