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 option DefaultTyping.EVERYTHING to support Kotlin data classes #2349

Closed
sdeleuze opened this issue Jun 9, 2019 · 10 comments
Closed

Add option DefaultTyping.EVERYTHING to support Kotlin data classes #2349

sdeleuze opened this issue Jun 9, 2019 · 10 comments

Comments

@sdeleuze
Copy link

sdeleuze commented Jun 9, 2019

Most usages of ObjectMapper.DefaultTyping are NON_FINAL which seems the wider scope available, which is real issue with Kotlin data classes which are final by design and can't be open. As a consequence, usage in Spring Data Redis turn to be a very difficult journey (GenericJackson2JsonRedisSerializer is very popular but not usable with Kotlin).

Is it possible to include a more Kotlin friendly enum value that would support final classes?

@cowtowncoder
Copy link
Member

I am not sure I understand this request: default typing enabling is based on nominal base type (declared type of properties), and any such type declared final could never have any subtypes.
So would one ever want to enable default typing for final types?

Or could this be misunderstanding wrt role of DefaultTyping for selecting properties affected?

@sdeleuze
Copy link
Author

sdeleuze commented Jun 10, 2019

It can, let me describe the use case we have. GenericJackson2JsonRedisSerializer purpose is to serialize / deserialize objects in Redis without specifying explicitly the Class parameter (Object.class is used for deserialization). It is leveraging an ObjectMapper instance configured with mapper.enableDefaultTyping(DefaultTyping.NON_FINAL, As.PROPERTY) which allows to serialize the class as a property. That allows users to get the right instance when deserializing with Object.class. But since classes are final by default in Kotlin, the type is not stored in a property, and I get a HashMap deserialized instead of the original type.

Is there another way to get the expected behavior?

@cowtowncoder
Copy link
Member

@sdeleuze No, that still does not make sense.

DefaultTyping criteria is applied to base class -- in this case, that'd be Object -- and not to instance (subtype). Subtypes can be final.

@sdeleuze
Copy link
Author

Ok so let's take a concrete focused repro where I implement the logic from our GenericJackson2JsonRedisSerializer.

@Test
fun genericSerializationDeserialization() {
	val testUser = TestUser("1", "user")
	val mapper = ObjectMapper()
	mapper.registerModule(KotlinModule())
	mapper.enableDefaultTyping(ObjectMapper.DefaultTyping.NON_FINAL, JsonTypeInfo.As.PROPERTY)
	val valueAsBytes = mapper.writeValueAsBytes(testUser)
	val returnValue = mapper.readValue(valueAsBytes, Object::class.java)
	assertEquals(testUser::class, returnValue::class)
}

This test is green with open class TestUser(val id: String, val name: String) and red with class TestUser(val id: String, val name: String) with the following error:
com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Missing type id when trying to resolve subtype of [simple type, class java.lang.Object]: missing type id property '@class' at [Source: (byte[])"{"id":"1","name":"user"}"; line: 1, column: 24]

If I have a look to the Java bytecode, the only difference is the final qualifier (expected since in Kotlin classes are final by default). The behavior I see seems not consistent with what you said. Did I miss something? Is it a bug?

@cowtowncoder
Copy link
Member

No, the problem is that due to Type Erasure, finding nominal base type differs for root values (no reference via property, only Class<?> of runtime type). If so, in case of root value, you are saying that the runtime type is base type. This will not work very well. But I can see now why you are asking for the feature.

I actually have come to conclusion that it is often/usually bad idea to use either generic or polymorphic types as root values, and that it is better to have a wrapper as necessary. So you would have something like:

public class Wrapper {
   @JsonTypeInfo(....)
   public Object value;
}

which then establishes polymorphic nature of value, using java.lang.Object as base type (or whatever type you want).

But as to your specific case: what would be nominal type to request on deserialization? That type would need to be the base type, and since it would not make much sense to use original type (if that is known why enable polymorphic type info in the first place).
In practice use of different base type on deserialization may work with default typing, as long as inclusion rules match (which probably does work here).

And on solution: I am not completely opposed to adding another type indicating "everything" if that is needed in practice. It should be also possible to separate intended base type from value type (former is for type id handling; latter actual Json(De)Serializer), and I think there is probably even an old issue for this. But it is not yet implemented.

I will add this issue on my (long) W-I-P list since that old issue (once I locate) probably should be addressed.

@cowtowncoder
Copy link
Member

cowtowncoder commented Aug 28, 2019

Hmmh. So after re-reading this again I finally understand the usage.
So if serializing one of leaf types, nominal type is that final type but just because "real" base type can not be known without caller providing it.

I guess that although there are other problems involved here (ideally base type should be indicated somehow), adding of new choice is probably not a bad idea.

I will add DefaultTyping.EVERYTHING.

@cowtowncoder cowtowncoder changed the title ObjectMapper.DefaultTyping is not Kotlin friendly Add option DefaultTyping.EVERYTHING to support Kotlin data classes Aug 28, 2019
@isadounikau
Copy link

@cowtowncoder could you please write an example for DefaultTyping.EVERYTHING and Kotlin data class with Object Mapper confgiguration?

@cowtowncoder
Copy link
Member

@Sadovnikov94 Unfortunately I don't use Kotlin myself so I don't think I can write good example code. But I don't think there is anything much more than enabling default typing with different setting.

Note, however, that this setting is not needed at all if enabling polymorphic typing with @JsonTypeInfo on data class definition itself.

@tmdgusya
Copy link

@isadounikau

val om = ObjectMapper()
            .registerModule(KotlinModule())
            .registerModule(JavaTimeModule())
            .activateDefaultTyping(BasicPolymorphicTypeValidator.builder()
                .allowIfBaseType(Any::class.java)
                .build(), ObjectMapper.DefaultTyping.EVERYTHING)

val serializer = GenericJackson2JsonRedisSerializer(om)

@echo-youn
Copy link

You better check the following issues before anyone uses DefaultTyping.EVERYTHING

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

No branches or pull requests

5 participants