Sergey, There are couple of things which should be addressed: 1. Unnecessary deserialization. 2. Inconsistent behavior. 3. Unclear documentation.
Deserialization is not free and in my mind should be avoided where possible. I think that if some feature (like interceptors) requires deserialization then it should be enabled explicitly and an impact should be clear to user. I can imagine a toggle “withAllowedDeserialization”. If there is an inconsistency between thick and thin clients it should be eliminated. I do not see a reason why behavior should be different. If something is a good thing but it is not intuitive it could be documented. But if there is s really good reason for it. Otherwise simplicity and consistency are better alias. > On 24 Jan 2019, at 17:42, Sergey Antonov <antonovserge...@gmail.com> wrote: > > I think it's bad idea. This contract nowhere defined and it's not clear for > users. > > чт, 24 янв. 2019 г. в 17:18, Pavel Tupitsyn <ptupit...@apache.org>: > >> Yes >> >> On Thu, Jan 24, 2019 at 5:15 PM Sergey Antonov <antonovserge...@gmail.com> >> wrote: >> >>> Pavel, >>> >>> "Leave it as is, use instanceof." >>> You meant always use CacheInterceptor<Object, Object> and in all methods >>> check, that passed arguments is BinaryObject? >>> >>> чт, 24 янв. 2019 г. в 17:10, Pavel Tupitsyn <ptupit...@apache.org>: >>> >>>> I don't think we should complicate things. Leave it as is, use >>> instanceof. >>>> The fact is - you can get anything, BinaryObject or any user class, so >> be >>>> prepared. >>>> Good example of older API is CacheEvent, which actually has oldValue() >>> and >>>> newValue() as Object. >>>> >>>> Igniters, any other thoughts? >>>> >>>> >>>> On Thu, Jan 24, 2019 at 2:16 PM Sergey Antonov < >>> antonovserge...@gmail.com> >>>> wrote: >>>> >>>>> Pavel, how about marker interface DeserializedValueCacheInterceptor? >> We >>>>> will deserialize data and pass it to cache interceptor, if >>>> CacheInterceptor >>>>> implements marker interface. >>>>> >>>>> чт, 24 янв. 2019 г. в 13:41, Pavel Tupitsyn <ptupit...@apache.org>: >>>>> >>>>>> You are exactly right, generic parameters don't make much sense >> here. >>>>>> Ignite caches are not restricted to any type, and there is type >>> erasure >>>>> in >>>>>> Java so you have no runtime guarantees. >>>>>> >>>>>> Maybe Interceptor design should be improved (e.g. add a flag to >> force >>>>>> binary or non-binary mode), >>>>>> but Thin or Thick client connector logic is unrelated here. >>>>>> withKeepBinary() call is valid and should not depend on Interceptor >>>>>> presence or implementation. >>>>>> >>>>>> On Thu, Jan 24, 2019 at 1:17 PM Sergey Antonov < >>>>> antonovserge...@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> Hi, Pavel, >>>>>>> >>>>>>> "Interceptor should support both modes, binary or not. Any code >> can >>>>> call >>>>>>> withKeepBinary(), this should be expected. >>>>>>> Just add if (x instanceof BinaryObject) and go from there. " >>>>>>> I don't agree. The cache interceptor[1] is a parametrized class >> and >>>> you >>>>>>> couldn't pass multiple cache interceptors in cache configuration. >>> So >>>>> all >>>>>>> cache interceptors must have Object, Object parameters for >>> supporting >>>>>> both >>>>>>> modes: binary and deserialized. In this case parametrized class >> no >>>>> sense. >>>>>>> >>>>>>> [1] >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/CacheInterceptor.html >>>>>>> >>>>>>> чт, 24 янв. 2019 г. в 13:06, Pavel Tupitsyn < >> ptupit...@apache.org >>>> : >>>>>>> >>>>>>>> Hi Sergey, >>>>>>>> >>>>>>>> I don't think this is a bug. >>>>>>>> >>>>>>>> Thick or thin clients always work in binary mode on server >> side, >>>>>> because >>>>>>>> you receive data in serialized form and there is no point in >>>>>>> deserializing >>>>>>>> it. >>>>>>>> Moreover, in most cases you don't have classes on the server, >> so >>>>> binary >>>>>>>> mode is the only way. >>>>>>>> >>>>>>>> Interceptor should support both modes, binary or not. Any code >>> can >>>>> call >>>>>>>> withKeepBinary(), this should be expected. >>>>>>>> Just add if (x instanceof BinaryObject) and go from there. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Pavel >>>>>>>> >>>>>>>> On Thu, Jan 24, 2019 at 12:38 PM Sergey Antonov < >>>>>>> antonovserge...@gmail.com >>>>>>>>> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> I did a little investigation. In >>>>>>>> o.a.i.i.p.p.c.c.ClientCacheRequest#cache() >>>>>>>>> enforced cache with keep binary. Why we should always work >>> binary >>>>>>>> objects? >>>>>>>>> >>>>>>>>> чт, 24 янв. 2019 г. в 12:29, Sergey Antonov < >>>>>> antonovserge...@gmail.com >>>>>>>> : >>>>>>>>> >>>>>>>>>> Hello, Igniters! >>>>>>>>>> >>>>>>>>>> I have ignite node with configured cache. The cache have >>> cache >>>>>>>>>> interceptor. I wiil got ClassCastException on cache >>>> interceptor, >>>>>> If I >>>>>>>> put >>>>>>>>>> some entry to the cache (without keepBinary) from thin java >>>>> client. >>>>>>>>>> >>>>>>>>>> I think it's a bug. I'd like to find out yours view! >>>>>>>>>> >>>>>>>>>> Also I made JIRA ticket with reproducer [1]. >>>>>>>>>> >>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-10789 >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> BR, Sergey Antonov >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> BR, Sergey Antonov >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> BR, Sergey Antonov >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> BR, Sergey Antonov >>>>> >>>> >>> >>> >>> -- >>> BR, Sergey Antonov >>> >> > > > -- > BR, Sergey Antonov
