ObjectMapper.valueToTree() will ignore the configuration SerializationFeature.WRAP_ROOT_VALUE

[jessepys]

Search before asking

• I searched in the issues and found nothing similar.

Describe the bug

When we upgrade the jackson-databind version, then we found the ObjectMapper.valueToTree will return the different result with the previous version. Actually, we configured the SerializationFeature.WRAP_ROOT_VALUE.

The class is like this:

@JsonRootName("event")
public class Event {

}

The previous ObjectMapper.valueToTree result:

After upgraded the version result:

This should caused by the commit.
2e986df
Can we re-enbale SerializationFeature.WRAP_ROOT_VALUE in ObjectMapper.valueToTree method to keep consistent with method writeValueAsString?

Version Information

2.14.2 (The version after 2.13 should have this issue)

Reproduction

<-- Any of the following

1. Configure the object mapper objectMapper.enable(SerializationFeature.WRAP_ROOT_VALUE);
2. Call objectMapper.valueToTree(event) method
3. You can the SerializationFeature.WRAP_ROOT_VALUE does not take effect after version 2.13.x
   -->

 public ObjectMapper objectMapper() {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.findAndRegisterModules();
        objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
        objectMapper.configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
        objectMapper.enable(SerializationFeature.WRAP_ROOT_VALUE);
        objectMapper.setSerializationInclusion(JsonInclude.Include.NON_NULL);
        objectMapper.registerModule(new JSR310Module());
        return objectMapper;
    }

@JsonRootName("event")
public class Event {
    private Long id;
    private String name;
}
//call valueToTree method
objectMapper.valueToTree(event)

Expected behavior

SerializationFeature.WRAP_ROOT_VALUE configuration should be global and take effect in method valueToTree to keep the same with writeValueAsString

Additional context

No response

[yawkat]

it looks to me like the snippet was incorrectly copied from the convert implementation.

[cowtowncoder]

We'd probably need a more slimmed-down reproduction here.

[yawkat]

@cowtowncoder reproducer:

    public static void main(String[] args) throws JsonProcessingException {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.enable(SerializationFeature.WRAP_ROOT_VALUE);

        Event value = new Event();
        value.id = 1L;
        value.name = "foo";
        System.out.println(objectMapper.writeValueAsString(value)); // {"event":{"id":1,"name":"foo"}}
        System.out.println(objectMapper.valueToTree(value)); // {"id":1,"name":"foo"}
    }

    @JsonRootName("event")
    public static class Event {
        public Long id;
        public String name;
    }

The bug was introduced in 2e986df because valueToTree now explicitly drops WRAP_ROOT_VALUE. imo this should be reverted

[cowtowncoder]

Hmmh. Does sounds problematic: wish I had added notes on why I felt this should be disabled. I think there's fundamental friction between WRAP_ROOT_VALUE usage, JsonNode, so that's likely why. But I guess easy enough to see which unit tests fail if leaving that feature on in a PR.

[JooHyukKim]

The bug was introduced in .... ... WRAP_ROOT_VALUE. imo this should be reverted

I will see if we can revert the commit. But until then, anyone can jump on the #4049 refer to.

[JooHyukKim]

Hmmh. Does sounds problematic: wish I had added notes on why I felt this should be disabled. I think there's fundamental friction between WRAP_ROOT_VALUE usage, JsonNode.... // rest-omitted

By refering to #2989, seems like it.

[JooHyukKim]

But I guess easy enough to see which unit tests fail if leaving that feature on in a PR.

#4050 Doesn't break anything 🤔. Was the commit used somewhere? So currently there are two PR's heading to different approach.

	#4049	#4050
Approach	Does not revert commit 2e986df and finds another solution.	Reverts commit 2e986df.
Considerations	1. Need to find a solution	1. Is the commit the optimal way to handle support/resolve #2989? 2. Will there be side effects? like other modules already depend on it.

[yawkat]

another alternative is to only revert the change to valueToTree.

[JooHyukKim]

another alternative is to only revert the change to valueToTree.

True 👍🏻, as long as it does not introduce another inconsistent behavior. Reading through #2989 tho, it seems like the modification was targeted a broader range (out of databind) to cover formats other than JSON.

[cowtowncoder]

Quick note: I don't think reversal of that sizable commit is the way to go. But I need time to look into alternative approaches.

[JooHyukKim]

Quick note: I don't think reversal of that sizable commit is the way to go. But I need time to look into alternative approaches.

Agreed. Closed #4050.

Btw, It seems like there might be another issue as pointed in this comment in PR #4049.

[JooHyukKim]

another alternative is to only revert the change to valueToTree.

Trying to apply change to valueToTree by removing the explicit .without(SerializationFeature.WRAP_ROOT_VALUE).
Seems to work with current test cases #4050. Any possible test case that can break?