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

unbox value class in Collection etc. when serializing #468

Merged
merged 6 commits into from Jun 23, 2021

Conversation

k163377
Copy link
Contributor

@k163377 k163377 commented Jun 19, 2021

Fix for #464.

Currently, getter/field for value class returns unboxed content, but not if it is contained in a Collection or Array.
So I added a serializer to KotlinSerializers that unboxes the content (the details of the process are commented).
And also added tests for various situations.

About unbox processing of key.

I have not yet worked on the unbox handling of key in Map.
If there are no problems with this PR, I will try to address it in another PR.

Although ignored as a test case, serialization of value in Map has already been addressed as of this PR.

Known Issue

Currently, in Jackson, when multiple Serializer/Serializers are registered in ObjectMapper, the later registered Serializer seems to have priority.
Therefore, if a user registers a Module before a KotlinModule, the added ValueClassUnboxSerializer will have priority over the user-defined Serializer.

I was not able to solve this problem.
However, as far as I read Extentions.kt and so on, I think I can ignore this problem because KotlinModule is the first one to be registered in normal use cases.
Is there any problem with this policy?

Repletion

In the following comment, I wrote about my concern about the behavior when JsonValue is given, but it seems to be unfounded.
#464 (comment)

Jackson overrides the annotations given to classes or fields.
FasterXML/jackson-databind#586

@dinomite
Copy link
Member

This makes sense to me. I'm reading through the test file to make sure I understand it (looks good and thorough at a glance), expect merging later today.

@dinomite
Copy link
Member

I made a new issue (#469) for the map keys bit and updated that test to used expectFailure() rather than @Ignoreing the test (more on that: https://github.com/FasterXML/jackson-module-kotlin/blob/2.13/src/test/kotlin/com/fasterxml/jackson/module/kotlin/README.md).

@dinomite
Copy link
Member

I added you to the credits as wrongwrong (k163377@github) and I'll be adding more entries for the prior PRs where I missed doing so (sorry!). Happy to use another name if you'd like.

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.

None yet

2 participants