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

Add support for final/Kotlin classes in GenericJackson2JsonRedisSerializer [DATAREDIS-995] #1566

Closed
spring-projects-issues opened this issue Jun 14, 2019 · 6 comments

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Jun 14, 2019

Sébastien Deleuze opened DATAREDIS-995 and commented

As described in detail in this Jackson issue, it is currently not possible to use GenericJackson2JsonRedisSerializer with final classes. That's especially blocking in Kotlin where classes are final by default, and data classes always final.

This issue should be fixed at Jackson level


No further details from DATAREDIS-995

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 9, 2019

Sébastien Deleuze commented

DefaultTyping.EVERYTHING has been added to the upcoming Jackson 2.10 to solve that issue. My proposal is to update Spring Data Redis to use it instead of DefaultTyping.NON_FINAL when Jackson 2.10 is in the classpath, otherwise fallback on DefaultTyping.NON_FINAL

FYI Jackson 2.10 should be here on time for Boot 2.2 GA.

 

 

 

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Sep 27, 2019

Sébastien Deleuze commented

I have just upgraded Jackson to 2.10.0 in Framework, in time for our 5.2 GA, and Boot will pick up as well. Is it possible to use DefaultTyping.EVERYTHING in order to solve this blocking issue that impacts Kotlin developers?

@azizabah
Copy link

@azizabah azizabah commented Jan 10, 2022

@mp911de / @sdeleuze - Is this still open? If so, is there a recommended work-around for Kotlin?

@mp911de
Copy link
Member

@mp911de mp911de commented Jan 10, 2022

GenericJackson2JsonRedisSerializer can be created with a given ObjectMapper so you should be able to construct the serializer yourself. We could switch the default. Since the change should be minimal, feel free to submit a pull request and make sure all tests are green.

@azizabah
Copy link

@azizabah azizabah commented Jan 10, 2022

@mp911de - Started working on a simple PR for that to swap over the DefaultTyping to EVERYTHING enum and added a test for it. Looks like it breaks the null serialization though with shouldSerializeNullValueSoThatItCanBeDeserializedWithDefaultTypingEnabled() throwing a:

Caused by: com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Type id handling not implemented for type org.springframework.cache.support.NullValue

I can still go ahead and submit the PR if you want but I'm a bit at a loss for that one.

@christophstrobl
Copy link
Member

@christophstrobl christophstrobl commented Jan 13, 2022

For the failing test in GenericJackson2JsonRedisSerializerUnitTests we need to add an override for serializeWithType to the NullValueSerializer that delegates to serialize.

Still, there are more problems related to moving to EVERYTHING.

The DefaultTypeResolverBuilder always returns true when requesting useType even when the given one is a TreeNode.
This breaks the integration test for the Jackson2HashMapper. It seems we need to provide our own flavour of DefaultTypeResolverBuilder that does something like

case EVERYTHING:
	return !TreeNode.class.isAssignableFrom(t.getRawClass());

and hook it into the ObjectMapper by overriding _constructDefaultTypeResolverBuilder.

@mp911de mp911de changed the title Add support for final/Kotlin classes in GenericJackson2JsonRedisSerializer [DATAREDIS-995] Add support for final/Kotlin classes in GenericJackson2JsonRedisSerializer [DATAREDIS-995] Jan 13, 2022
@mp911de mp911de added this to the 2.7 M1 (2021.2.0) milestone Jan 13, 2022
@mp911de mp911de closed this in 93c25b0 Jan 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants