search

kontent-ai / java-packages

Delivery Java SDK for Kontent.ai including examples for Spring, Gradle, Kotlin and Java Android app
https://kontent.ai/learn/tutorials/develop-apps/overview/?tech=java
MIT License
14 stars 28 forks source link

The model classes generated using delivery-sdk-generators don't extend Serializable interface hence not able cache those entries at client side. #148

Closed yayatitech closed 11 months ago

yayatitech commented 1 year ago

Motivation

Why is this feature required? What problems does it solve?

Proposed solution

An ideal solution for the above problems.

Additional context

Add any other context, screenshots, or reference links about the feature request here.

Simply007 commented 1 year ago

Hello @yayatitech,

could you specify your use case? And also what cache you want to use?

I am trying to find a scalable solution, instead of adding a new parameter that adds extension from the Serializable interface. Or would it be sufficient? Since Serializable does not require any method to be implemented, but if you want to adjust the serialization logic - how would you expect this to work?

yayatitech commented 1 year ago

I just want to cache the result from Kontent.ai in my local using following logic in Java Spring client application. Since this ContentItem class is not Serializable, the below method call fails.

@Cacheable(cacheNames = "contentItemsCache", key = "#contentName") //, key = "#contentName"
public ContentItem fetchContentByCodeName(String contentName) {
        try {
            log.info("Invoke Kontent.ai to fetch content");
            ContentItem contentItem = deliveryClient.getItem(contentName, ContentItem.class).toCompletableFuture().get();
            return contentItem;
        } catch (Exception exp) {
            log.error("Error fetching content with codename = {}", contentName);
            throw new RuntimeException("Error fetching content with codename = " + contentName, exp);
          }
}

I think making all the model (POJO) classes extend Serializable would solve the problem. Serializable doesn't require to implement any method but it marks class can be serialized. Also, the generated classes using delivery-sdk-generators should also extend Serializable.

Simply007 commented 1 year ago

Hello @yayatitech,

I drafted a serialization support for the generator in #149.

I am not sure whether it requires making ContentItem serializable as well. I did it in the PR because I suppose your implementation requires it right? If so - do you see any potential problems with releasing it as a minor version -> could this be treated as a non-breaking change?

I can release this as a beta and ask you to test it out with your scenario. Do you have any opinion on testing this serialization in unit tests? Do you think basic serialization and deserialization back and forth and checking the values would be sufficient? Or is the Redis caching technique special somehow in your use case?

yayatitech commented 1 year ago

Thanks @Simply007

I think not only ContentItem but all the nested classes like System, TextElement, ContentType etc. needs to extend this Serializableinterface because we cache the entire response returned from Kontent AI into our Redis cache. This should be non-breaking change, so I don't see any issues releasing it as minor version or beta version.

This is not related to Redis, basic serialization and deserialization test should be fine. I don't know how it can be tested through unit test though.

Simply007 commented 1 year ago

Thanks @yayatitech,

This might solve the issue, but I would like to be able to reproduce the issue and then test out the fix.

I have tried to set up the least basic implementation of the caching to the spring boot sample app based on the Serializable ContentItem models, but it still works. Even without serializable implementation - see 1a9d384

To reproduce the issue it would require Redis instead of ConcurrentMapCacheManager usage.

Redis on Java stack is not my strong field. Can you provide me with a simple reproducible setup we can use?

Based on this stack overflow thread it should be possible to rewrite the default serializer - would this be sufficient for you?

There is also a (caching capability right in the delivery SDK](https://github.com/kontent-ai/java-packages/blob/master/delivery-sdk/src/main/java/kontent/ai/delivery/DeliveryClient.java#L378) - would it be something you could use?

yayatitech commented 1 year ago

I have created a pull request with redis cache

yayatitech commented 1 year ago

"org.springframework.data.redis.serializer.SerializationException: Cannot serialize; nested exception is org.springframework.core.serializer.support.SerializationFailedException: Failed to serialize object using DefaultSerializer; nested exception is java.io.NotSerializableException: kontent.ai.delivery.TextElement\n\tat org.springframework.data.redis.serializer

Since ConentItem is already Serializable it is working fine. I am getting serialization exception on TextElement now. Please let me know if you need any other information to recreate this scenario.

Simply007 commented 1 year ago

Hello @yayatitech,

I tried my best to test this out. I have created a Redis cache on redis labs. I was able to reproduce the issue and then set all classes based on Elementsertializable (including all the subclasses like Option, Asset, Taxonomy).

Then I got an error that I also need to mark StronglyTypedContentItemConverter and ContentItemResponse - which was confusing, but I tried to mark them as well.

After that, I got the error requesting to mark KontentConfiguration as serializable - this was even more confusing and after marking this one too, I still keep getting the serialization error on this class.

Whitelabel Error Page
This application has no explicit mapping for /error, so you are seeing this as a fallback.

Tue Sep 12 09:00:39 CEST 2023
There was an unexpected error (type=Internal Server Error, status=500).
Cannot serialize; nested exception is org.springframework.core.serializer.support.SerializationFailedException: Failed to serialize object using DefaultSerializer; nested exception is java.io.NotSerializableException: kontent.ai.delivery.sample.dancinggoat.springboot.KontentConfiguration$1
org.springframework.data.redis.serializer.SerializationException: Cannot serialize; nested exception is org.springframework.core.serializer.support.SerializationFailedException: Failed to serialize object using DefaultSerializer; nested exception is java.io.NotSerializableException: kontent.ai.delivery.sample.dancinggoat.springboot.KontentConfiguration$1
    at org.springframework.data.redis.serializer.JdkSerializationRedisSerializer.serialize(JdkSerializationRedisSerializer.java:96)
    at org.springframework.data.redis.serializer.DefaultRedisElementWriter.write(DefaultRedisElementWriter.java:43)
    at org.springframework.data.redis.serializer.RedisSerializationContext$SerializationPair.write(RedisSerializationContext.java:287)
    at org.springframework.data.redis.cache.RedisCache.serializeCacheValue(RedisCache.java:244)
    at org.springframework.data.redis.cache.RedisCache.put(RedisCache.java:150)
    at org.springframework.cache.interceptor.AbstractCacheInvoker.doPut(AbstractCacheInvoker.java:87)
    at org.springframework.cache.interceptor.CacheAspectSupport$CachePutRequest.apply(CacheAspectSupport.java:821)
    at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:430)
    at org.springframework.cache.interceptor.CacheAspectSupport.execute(CacheAspectSupport.java:346)
    at org.springframework.cache.interceptor.CacheInterceptor.invoke(CacheInterceptor.java:61)
    at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:186)
    at org.springframework.aop.framework.CglibAopProxy$CglibMethodInvocation.proceed(CglibAopProxy.java:749)
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:691)
    at kontent.ai.delivery.sample.dancinggoat.springboot.TestService$$EnhancerBySpringCGLIB$$3d77227d.fetchContentByCodeName(<generated>)
    at kontent.ai.delivery.sample.dancinggoat.controllers.TestController.getTest(TestController.java:30)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.springframework.web.method.support.InvocableHandlerMethod.doInvoke(InvocableHandlerMethod.java:190)
    at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:138)
    at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:105)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:879)
    at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:793)
    at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
    at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1040)
    at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:943)
    at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006)
    at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:634)
    at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:741)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201)
    at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:119)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:193)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:202)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:96)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:541)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:139)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:74)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:343)
    at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:373)
    at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65)
    at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1590)
    at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.base/java.lang.Thread.run(Thread.java:829)
Caused by: org.springframework.core.serializer.support.SerializationFailedException: Failed to serialize object using DefaultSerializer; nested exception is java.io.NotSerializableException: kontent.ai.delivery.sample.dancinggoat.springboot.KontentConfiguration$1
    at org.springframework.core.serializer.support.SerializingConverter.convert(SerializingConverter.java:64)
    at org.springframework.core.serializer.support.SerializingConverter.convert(SerializingConverter.java:33)
    at org.springframework.data.redis.serializer.JdkSerializationRedisSerializer.serialize(JdkSerializationRedisSerializer.java:94)
    ... 64 more
Caused by: java.io.NotSerializableException: kontent.ai.delivery.sample.dancinggoat.springboot.KontentConfiguration$1
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1175)
    at java.base/java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:345)
    at java.base/java.util.HashMap.internalWriteEntries(HashMap.java:1858)
    at java.base/java.util.HashMap.writeObject(HashMap.java:1412)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at java.base/java.io.ObjectStreamClass.invokeWriteObject(ObjectStreamClass.java:1016)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1487)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1543)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1500)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1543)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1500)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:345)
    at java.base/java.util.LinkedHashMap.internalWriteEntries(LinkedHashMap.java:333)
    at java.base/java.util.HashMap.writeObject(HashMap.java:1412)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at java.base/java.io.ObjectStreamClass.invokeWriteObject(ObjectStreamClass.java:1016)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1487)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1543)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1500)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1543)
    at java.base/java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1500)
    at java.base/java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1423)
    at java.base/java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1169)
    at java.base/java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:345)
    at org.springframework.core.serializer.DefaultSerializer.serialize(DefaultSerializer.java:46)
    at org.springframework.core.serializer.Serializer.serializeToByteArray(Serializer.java:56)
    at org.springframework.core.serializer.support.SerializingConverter.convert(SerializingConverter.java:60)

All of these you can test out on commit https://github.com/kontent-ai/java-packages/pull/149/commits/d0b5b4b8ea71e18f8ba8ce47d8b9f3925cfe3ec3.

Simply007 commented 1 year ago

I did some digging and I was able to re-use the Jackson configuration which is already implemented in Delivery SDK and I was able to get it running 🎉 by configuring it like that:


 @Bean
    public RedisCacheManager cacheManager(RedisConnectionFactory redisConnectionFactory) {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.enable(JsonGenerator.Feature.IGNORE_UNKNOWN);
        objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
        objectMapper.registerModule(new JavaTimeModule());
        objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);

        RedisSerializer<Object> serializer = new GenericJackson2JsonRedisSerializer(objectMapper);

        RedisCacheConfiguration config = RedisCacheConfiguration.defaultCacheConfig()
                .serializeKeysWith(RedisSerializationContext.SerializationPair.fromSerializer(new StringRedisSerializer()))
                .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(serializer))
                .entryTtl(Duration.ofMinutes(1));

        return RedisCacheManager.builder(redisConnectionFactory)
                .cacheDefaults(config)
                .build();
    }

You can check the sample app implementation in accb089.

I think this approach is more suitable for implementation for Redis if you want to use embedded support from Springboot.

Still, I think it might be more future-proof to use the embedded capability of delivery SDK for caching and using a custom implementation of the Cache manager using Redis.

Simply007 commented 11 months ago

I did some digging and I was able to re-use the Jackson configuration which is already implemented in Delivery SDK and I was able to get it running 🎉 by configuring it like that:

 @Bean
    public RedisCacheManager cacheManager(RedisConnectionFactory redisConnectionFactory) {
        ObjectMapper objectMapper = new ObjectMapper();
        objectMapper.enable(JsonGenerator.Feature.IGNORE_UNKNOWN);
        objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
        objectMapper.registerModule(new JavaTimeModule());
        objectMapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);

        RedisSerializer<Object> serializer = new GenericJackson2JsonRedisSerializer(objectMapper);

        RedisCacheConfiguration config = RedisCacheConfiguration.defaultCacheConfig()
                .serializeKeysWith(RedisSerializationContext.SerializationPair.fromSerializer(new StringRedisSerializer()))
                .serializeValuesWith(RedisSerializationContext.SerializationPair.fromSerializer(serializer))
                .entryTtl(Duration.ofMinutes(1));

        return RedisCacheManager.builder(redisConnectionFactory)
                .cacheDefaults(config)
                .build();
    }

You can check the sample app implementation in accb089.

I think this approach is more suitable for implementation for Redis if you want to use embedded support from Springboot.

Still, I think it might be more future-proof to use the embedded capability of delivery SDK for caching and using a custom implementation of the Cache manager using Redis.

Hello @yayatitech,

what do you think about the suggested solution above?

I would make this a practice.

Simply007 commented 11 months ago

Hi @yayatitech!

This issue has gone quiet. 👻 It’s been a while since the last update here.

If we missed anything on this issue or if you want to keep it open, please reply here.

For now, we will consider this approach as a practice for Redis and Spring: https://github.com/kontent-ai/java-packages/issues/148#issuecomment-1715778397

Thanks for being a part of the Kontent.ai community!