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

GenericJackson2JsonRedisSerializer can't deserialize previously serialized Stream.toList() #2697

Open
gcharondkt opened this issue Sep 4, 2023 · 7 comments
Labels
for: team-attention An issue we need to discuss as a team to make progress type: bug A general bug

Comments

@gcharondkt
Copy link

gcharondkt commented Sep 4, 2023

Hi there,

I'm facing an issue with the GenericJackson2JsonRedisSerializer() when using a @Cacheable annotation on a method returning a List resulting of a Stream.toList() operation.

I can easily reproduce with the following unit test :

    List<Integer> toSerialize = Stream.of(2953).toList();

    @Test
    void test() {
        RedisSerializer<Object> serializer = RedisSerializer.json();

        var serialized = serializer.deserialize(serializer.serialize(new ArrayList<>(toSerialize))); // This is OK as the list is embedded in an ArrayList
        serialized = serializer.deserialize(serializer.serialize(toSerialize)); // EXCEPTION !!
    }

Please tell me if i'm doing something wrong.

SpringBoot version : 3.1.3

Stacktrace :

Caused by: com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id '2953' as a subtype of `java.lang.Object`: no such class found
 at [Source: (byte[])"[2953]"; line: 1, column: 6]
	at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
	at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:2084)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnknownTypeId(DeserializationContext.java:1575)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver._typeFromId(ClassNameIdResolver.java:76)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver.typeFromId(ClassNameIdResolver.java:66)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:159)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:97)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromArray(AsArrayTypeDeserializer.java:53)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromAny(AsPropertyTypeDeserializer.java:238)
	at com.fasterxml.jackson.databind.deser.std.UntypedObjectDeserializerNR.deserializeWithType(UntypedObjectDeserializerNR.java:115)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4825)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3874)
	at org.springframework.data.redis.serializer.JacksonObjectReader.lambda$create$0(JacksonObjectReader.java:54)
	at org.springframework.data.redis.serializer.GenericJackson2JsonRedisSerializer.deserialize(GenericJackson2JsonRedisSerializer.java:250)
	... 70 more
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 4, 2023
@mp911de
Copy link
Member

mp911de commented Sep 4, 2023

Would you mind attaching the full stack trace?

@mp911de mp911de added the status: waiting-for-feedback We need additional information before we can continue label Sep 4, 2023
@gcharondkt
Copy link
Author

Oh yes sorry i've forgotten, here it is.

In debug mode, I can see a difference between the two serialized values :

  • With ArrayList : ["java.util.ArrayList",[2953]]
  • Without : [2953]
Caused by: com.fasterxml.jackson.databind.exc.InvalidTypeIdException: Could not resolve type id '2953' as a subtype of `java.lang.Object`: no such class found
 at [Source: (byte[])"[2953]"; line: 1, column: 6]
	at com.fasterxml.jackson.databind.exc.InvalidTypeIdException.from(InvalidTypeIdException.java:43)
	at com.fasterxml.jackson.databind.DeserializationContext.invalidTypeIdException(DeserializationContext.java:2084)
	at com.fasterxml.jackson.databind.DeserializationContext.handleUnknownTypeId(DeserializationContext.java:1575)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver._typeFromId(ClassNameIdResolver.java:76)
	at com.fasterxml.jackson.databind.jsontype.impl.ClassNameIdResolver.typeFromId(ClassNameIdResolver.java:66)
	at com.fasterxml.jackson.databind.jsontype.impl.TypeDeserializerBase._findDeserializer(TypeDeserializerBase.java:159)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer._deserialize(AsArrayTypeDeserializer.java:97)
	at com.fasterxml.jackson.databind.jsontype.impl.AsArrayTypeDeserializer.deserializeTypedFromArray(AsArrayTypeDeserializer.java:53)
	at com.fasterxml.jackson.databind.jsontype.impl.AsPropertyTypeDeserializer.deserializeTypedFromAny(AsPropertyTypeDeserializer.java:238)
	at com.fasterxml.jackson.databind.deser.std.UntypedObjectDeserializerNR.deserializeWithType(UntypedObjectDeserializerNR.java:115)
	at com.fasterxml.jackson.databind.deser.impl.TypeWrappedDeserializer.deserialize(TypeWrappedDeserializer.java:74)
	at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
	at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4825)
	at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3874)
	at org.springframework.data.redis.serializer.JacksonObjectReader.lambda$create$0(JacksonObjectReader.java:54)
	at org.springframework.data.redis.serializer.GenericJackson2JsonRedisSerializer.deserialize(GenericJackson2JsonRedisSerializer.java:250)
	... 70 more

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 5, 2023
@mp911de
Copy link
Member

mp911de commented Sep 6, 2023

The difference between the two values is that the ArrayList variant is non-final while Stream.toList() produces a final and private type ImmutableCollections$ListN. The typing mechanism skips adding type hints if the type is final and originates from the java. namespace as these types are typically private and cannot be accessed via reflection.

On the other side, we attempt to preserve custom collection types when reading arrays. Unfortunately, all we have to read the serialized value is java.lang.Object as type hint and we need to determine the type to read, so we default to parse the type hint.

The suggested workaround is to wrap the resulting value in a value object instead of caching the top-level value. Paging @christophstrobl for further insights.

@gcharondkt
Copy link
Author

I have implemented a workaround like this :

@Cacheable
public List<Integer> getValue() {
    return new ArrayList<>(result.stream().toList());
}

Instead of this :

@Cacheable
public List<Integer> getValue() {
    return result.stream().toList();
}

But still, i thought it was a good idea to open an issue because the error is difficult to detect : the execution was not stopped by any exception, the only impact is that the @Cacheable was not working, resulting in many unexpected API calls.

Also, even if I understand why this is raising an error, in my opinion a serializer should not serialize something that he won't be able to deserialize, don't you think ?

@mp911de
Copy link
Member

mp911de commented Sep 7, 2023

Also, even if I understand why this is raising an error, in my opinion a serializer should not serialize something that he won't be able to deserialize, don't you think ?

I fully agree, I just do not have an idea how to tackle top-level arrays as we don't seem to have sufficient hooks to either always or never enable typing.

@mp911de mp911de added type: bug A general bug for: team-attention An issue we need to discuss as a team to make progress and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Sep 7, 2023
@mp911de mp911de changed the title Can't deserialize Stream.toList() GenericJackson2JsonRedisSerializer can't deserialize previously serialized Stream.toList() Sep 7, 2023
@jxblum
Copy link
Contributor

jxblum commented Sep 9, 2023

I reproduced this problem in the following test case method that I am currently adding to the GenericJackson2JsonRedisSerializerUnitTests class in Spring Data Redis to debug and explore possible solutions.

@Test // GH-2697
void serializingDeserializingIntegerListIsHandledCorrectly() {

    GenericJackson2JsonRedisSerializer redisSerializer = 
        new GenericJackson2JsonRedisSerializer();

    List<Integer> integers = Stream.of(2953).toList();
    //List<Integer> integers = List.of(2953);
    //List<Integer> integers = new ArrayList<>(Stream.of(2953).toList());
    //List<Integer> integers = new LinkedList<>(List.of(2953));

    Object deserializedIntegers = 
        redisSerializer.deserialize(redisSerializer.serialize(integers));

    assertThat(deserializedIntegers)
        .isInstanceOf(List.class)
        .asInstanceOf(InstanceOfAssertFactories.type(List.class))
        .extracting(list -> list.get(0))
        .isEqualTo(2953);

    deserializedIntegers = 
        redisSerializer.deserialize(redisSerializer.serialize(deserializedIntegers));

    assertThat(deserializedIntegers)
        .isInstanceOf(List.class)
        .asInstanceOf(InstanceOfAssertFactories.type(List.class))
        .extracting(list -> list.get(0))
        .isEqualTo(2953);
}

Clearly, you can see that I am playing around with different combinations of List types.

NOTE: I was also playing around with multiple serializations/deserializations to make sure what we return in the workable case still works after the first serialization/deserialization.

What many developers fail to realize is that the Collection types returned by operations like Stream.toList(), or List.of(..), and even Arrays.asList(..), which seemingly is benign when looking at the source code, because Arrays.asList(..) returns an "ArrayList" even, though it is NOT a java.util.ArrayList, but rather a java.util.Arrays.ArrayList instead! So, these are NOT typical, public (and standard) Collection types in the java.util package that developers are familiar with and use most often (they are "internal").

This leads to many surprises when users try to use the List, such as in a mutating manner, which they are not. These Collections effectively are read-only once created.

Additionally, as @mp911de pointed out, these Collection types are private, or often package private, such as the (internal) Collection types in the java.util.Collections class.

I agree with Mark. I think we have room for improvement, but I am not exactly certain what that is yet. I will continue to think on this next week and talk with both @mp911de and @christophstrobl about possible ideas.

@NathanD001
Copy link

NathanD001 commented Feb 14, 2024

The issue is that ImmutableListN is a final class

it does work if you use ObjectMapper.DefaultTyping.EVERYTHING however I'd rather not use that solution, and Jackson is removing it in 3.0
https://github.com/FasterXML/jackson-databind/blob/2.17/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java#L227

I also found an alternative using Guava ImmutableList instead of Java ImmutableListN and jackson-datatype-guava

.collect(ImmutableList.toImmutableList()) intead of .toList()

then add cacheObjectMapper.registerModule(new GuavaModule());

I do wish it would work with out either of these workarounds. Does anyone else have suggestions?

paynezhuang added a commit to paynezhuang/panis-boot that referenced this issue Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants