Skip to content

GH-1129: Add JacksonMimeTypeModule #1130

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

Merged
merged 4 commits into from
Jun 20, 2019

Conversation

artembilan
Copy link
Member

Fixes #1129

Introduce a JacksonMimeTypeModule with a simple MimeTypeSerializer
for the proper inter-platform MimeType objects serialization.
Essentially we call its toString() which is enough to carry mime-type
info over the network.
Register this module in the JacksonUtils.enhancedObjectMapper() which
is used from the DefaultKafkaHeaderMapper.
Such a module can be registered as a bean in the application context
and will be automatically discovered by Spring Boot auto-configuration
for Jackson

  • Modify a DefaultKafkaHeaderMapper to not deal with MimeType
    directly any more since it is covered by the JacksonMimeTypeModule
    even if MimeType is a part of collection like it is in case of
    mapped HTTP headers.
  • Modify DefaultKafkaHeaderMapperTests to check that MimeType is
    properly serialized into its string representation even if it in the
    collection

Fixes spring-projects#1129

Introduce a `JacksonMimeTypeModule` with a simple `MimeTypeSerializer`
for the proper inter-platform `MimeType` objects serialization.
Essentially we call its `toString()` which is enough to carry mime-type
info over the network.
Register this module in the `JacksonUtils.enhancedObjectMapper()` which
is used from the `DefaultKafkaHeaderMapper`.
Such a module can be registered as a bean in the application context
and will be automatically discovered by Spring Boot auto-configuration
for Jackson
* Modify a `DefaultKafkaHeaderMapper` to not deal with `MimeType`
directly any more since it is covered by the `JacksonMimeTypeModule`
even if `MimeType` is a part of collection like it is in case of
mapped HTTP headers.
* Modify `DefaultKafkaHeaderMapperTests` to check that `MimeType` is
properly serialized into its string representation even if it in the
collection
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Just one issue.

* target JSON as a plain string.
*/
private static final class MimeTypeSerializer extends JsonSerializer<MimeType> {

Copy link
Contributor

Choose a reason for hiding this comment

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

package-protected ctor needed to avoid synthesized ctor with extra arg.

* Document a `JacksonMimeTypeModule`
@garyrussell
Copy link
Contributor

Travis (checkstyle).

@garyrussell
Copy link
Contributor

NPE in testTrustedAndNot (travis)

@garyrussell
Copy link
Contributor

I wonder if the deserializer should now (2.3) always decode to String?

@artembilan
Copy link
Member Author

I wonder if the deserializer should now (2.3) always decode to String?

No, that's OK. See my latest commit.

The problem is that even if we serialize MimeType as a String we still store MimeType class in the JSON_TYPES header.

So, if another side is still DefaultKafkaHeaderMapper it is going to deserialize that into MimeType object.
This problem has a place only if MimeType is top level header. If it is an item in the collection, there is choice unless represent it as a plain string.
That's why Jackson always has a containerType, elementType and also keyType for maps.
We store in JSON_TYPES only top-level classes.

Not perfect, but enough so far...
Not sure what is an ideal solution, but in Spring Integration we use init(JsonTypeInfo.Id.CLASS, null).inclusion(JsonTypeInfo.As.PROPERTY); for ObjectMapper.DefaultTypeResolverBuilder to include a type info into the JSON node for particular object.
It may be ignored on the other side, (e.g. by non-Java consumer) or can be used to reconstruct instances properly.
We use such a trick in the GenericJackson2JsonRedisSerializer for RedisMessageStore.

Just some thoughts to share...

@garyrussell garyrussell merged commit 7d79406 into spring-projects:master Jun 20, 2019
@garyrussell
Copy link
Contributor

This broke backwards compatibility; a 3.0 app can't talk to a 2.1 app because the MimeType is no longer a String.

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.

A MimeTypeSerializer to store MimeType as a string in JSON is needed.
2 participants