Skip to content

DeserializationContext.readTreeAsValue() handles null nodes differently from ObjectMapper.treeToValue() #4934

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

Closed
1 task done
FWest98 opened this issue Jan 27, 2025 · 7 comments
Labels
2.19 Issues planned at 2.19 or later
Milestone

Comments

@FWest98
Copy link

FWest98 commented Jan 27, 2025

Search before asking

  • I searched in the issues and found nothing similar.

Describe the bug

The ObjectMapper functions handle the case where the root node is the null node by specifically calling getNullValue of the root deserializer. The equivalent DeserializationContext functions do not. I would expect the two functions to behave similarly, or the differences should be documented.

See below for a reproducer and use case.

Version Information

2.18.1

Reproduction

My use case is that I have a custom deserializer that reads the defaultValue metadata field that is populated by the JsonProperty annotation. I implement a custom getAbsentValue function that will handle the default values. However, I also want to allow "null" as a default value.

override fun getAbsentValue(ctxt: DeserializationContext): Property<*> {
    val defaultString = property?.metadata?.defaultValue // This is the string "null"

    if(defaultString != null) {
        val tree = ctxt.parser.codec.factory.createParser(defaultString).readValueAsTree<JsonNode>()
        val thisWorks = (ctxt.parser.codec as ObjectMapper).treeToValue<Any>(tree, valueType)
        val doesNotWork = ctxt.readTreeAsValue<Any>(tree, valueType)

        // Further handling
}

The workaround is to cast the codec to an ObjectMapper, but that does not seem very nice and I would prefer not to do it, since there are no guarantees of the cast being allowed.

@FWest98 FWest98 added the to-evaluate Issue that has been received but not yet evaluated label Jan 27, 2025
@cowtowncoder
Copy link
Member

Ok, I'll need to think of how to write a test case. I think you are right since although this:

    public <T> T readTreeAsValue(JsonNode n, Class<T> targetType) throws IOException
    {
        if (n == null) {
            return null;
        }
        try (TreeTraversingParser p = _treeAsTokens(n)) {
            return readValue(p, targetType);
        }
    }

at first looks correct, I forgot that JsonDeserializers are not expected to actually deal with JsonToken.VALUE_NULL themselves; it's "parent deserializer" that should handle that -- or in case of root value, DeserializationContext.

But getting access to a DeserializationContext from test, outside handler callbacks is bit trickier.

cowtowncoder added a commit that referenced this issue Jan 29, 2025
@cowtowncoder
Copy link
Member

@FWest98 I fail to reproduce the issue against 2.18 branch (for 2.18.3), see #4940. So could use help here. It looks like getNullValue() would be called as expected from limited test I added.

(I did add a small fix to handle "missing node" case tho so that's useful).

@JooHyukKim
Copy link
Member

@FWest98 Yeah would be helpful if we ca get the actual working reproduction.

@FWest98
Copy link
Author

FWest98 commented Jan 29, 2025

It seems like the BooleanDeserializer handles null tokens explicitly, without getNullValue being called. In my usecase, things went wrong with Duration, but I can reproduce it with an arbitrary bean as follows (adapted from your tests):

// [databind#4934]
@Test
public void testTreeAsValueFromNulls() throws Exception
{
    final JsonNodeFactory nodeF = MAPPER.getNodeFactory();
    try (JsonParser p = MAPPER.createParser("abc")) {
        DeserializationContext ctxt = MAPPER.readerFor(String.class).createDeserializationContext(p);

        assertNull(MAPPER.treeToValue(nodeF.nullNode(), Bean.class)); // works
        assertNull(ctxt.readTreeAsValue(nodeF.nullNode(), Bean.class)); // fails, throws exception
    }
}

@cowtowncoder
Copy link
Member

Interesting. Thank you @FWest98.

@cowtowncoder
Copy link
Member

I can reproduce the issue and know what is needed. But fix is likely bit riskier than what should go in a patch so will target 2.19, instead of 2.18 like I initially planned.

@cowtowncoder cowtowncoder added 2.19 Issues planned at 2.19 or later and removed to-evaluate Issue that has been received but not yet evaluated labels Jan 31, 2025
@cowtowncoder cowtowncoder modified the milestones: 2.1.7, 2.19.0 Jan 31, 2025
cowtowncoder added a commit that referenced this issue Jan 31, 2025
@cowtowncoder
Copy link
Member

@FWest98 Ok, fixed for 2.19 for eventual 2.19.0. Thank you for reporting this issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.19 Issues planned at 2.19 or later
Projects
None yet
Development

No branches or pull requests

3 participants