Skip to content

Conversation

@JooHyukKim
Copy link
Member

@JooHyukKim JooHyukKim commented Jul 20, 2023

Fixes #4047

@JooHyukKim JooHyukKim reopened this Jul 25, 2023
@JooHyukKim JooHyukKim changed the title [experimental] Fix #4047 by revert #2989 [DRAFT] Fix #4047 by revert #2989 Jul 25, 2023
@JooHyukKim JooHyukKim changed the title [DRAFT] Fix #4047 by revert #2989 Fix #4047 Jul 25, 2023

static class TestCommandChild extends TestCommandParent { }

// [databind#4047]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although this is possible location, maybe rather add under com.fasterxml.jackson.databind.node; maybe in existing TestConversions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cowtowncoder will do now 👍🏻

Copy link
Member

@cowtowncoder cowtowncoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; I think test should be moved, but otherwise I could merge this.

@cowtowncoder cowtowncoder merged commit 8c820ef into FasterXML:2.16 Jul 26, 2023
// inlined 'writeValue' with minor changes:
// first: disable wrapping when writing
final SerializationConfig config = getSerializationConfig().without(SerializationFeature.WRAP_ROOT_VALUE);
// [databind#4047] ObjectMapper.valueToTree will ignore the configuration SerializationFeature.WRAP_ROOT_VALUE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment is slightly misleading: the issue is that it would ignore WRAP_ROOT_VALUE, but after this change it doesn't anymore. the comment makes it sound like now it's ignoring WRAP_ROOT_VALUE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah sorry; I just copied issue description which is misleading. If anyone can do a PR thatd be great (or I'll fix that when I get home).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True

@JooHyukKim JooHyukKim deleted the 4047-try-revert branch July 28, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants