You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Vladislav Pyatkov <vp...@gridgain.com> on 2016/04/04 18:32:49 UTC

Re: Controlling type of object inserted in IgniteCache

Hi Ignite dev comunity,

I an start to work on https://issues.apache.org/jira/browse/IGNITE-2909
The problem occurs when we add objects in cache in node where no
information abut types (for example server node without end user classes in
classpath).
After reserch, I try to find way to solve issue.
The intention:
1) Add method to CacheConfiguration#setTypeNames(String keyClass String
valClass) (like suggested earlier)
2) Before every modification cache need check class matching (in put(),
putAll(), replace() etc).
Maybe it not simple string comparison.
3) If class is matching then continue saving, else throwing exception.

In my opinion, it actualy only if put cache value as BynaryObject (get
class binaryObject.type().typeName()).
If we check the class name, it is not relevant objects do not put to cache.

I would like to hear the comunity opinion.

On Wed, Mar 30, 2016 at 7:08 AM, Denis Magda <dm...@gridgain.com> wrote:

> Sounds good. Created the ticket
> https://issues.apache.org/jira/browse/IGNITE-2909
>
>
> On 3/30/2016 2:40 AM, Dmitriy Setrakyan wrote:
>
>> On Tue, Mar 29, 2016 at 3:49 PM, Valentin Kulichenko <
>> valentin.kulichenko@gmail.com> wrote:
>>
>> How about adding setTypeNames() configuration property that will be
>>> similar
>>> to setTypes(), but for binary objects?
>>>
>>> Agree. Sounds like it will solve the problem, no?
>>
>>
>> -Val
>>>
>>> On Tue, Mar 29, 2016 at 1:56 AM, Denis Magda <dm...@gridgain.com>
>>> wrote:
>>>
>>> Igniters,
>>>>
>>>> In some cases there is necessity to control a type of object that is
>>>>
>>> being
>>>
>>>> inserted into a cache.
>>>> Presently this kind of check is accomplished at compile time only
>>>> relying
>>>> on Java Generics. However it doesn't prevent us from connecting to a
>>>> cluster using a non-generic instance of a cache and put any kind of data
>>>>
>>> in
>>>
>>>> it.
>>>> This may be not a harmful intention but rather a silly developer
>>>> mistake.
>>>>
>>>> I see the following options here.
>>>>
>>>> 1) First CacheInterceptor based approach.
>>>> Specify a unique interceptor per cache setting expected typeName/typeId
>>>>
>>> to
>>>
>>>> it at construction time and the interceptor will validate object type in
>>>> onBeforePut() method.
>>>> Disadvantages:
>>>> - cache interceptor have to be created for every cache;
>>>> - cache interceptor class has to be located in servers classpath. It
>>>>
>>> means
>>>
>>>> that we won't be able to start a new cache with its interceptor later.
>>>>
>>>> 2)  Second CacheInterceptor based approach.
>>>> A generic cache interceptor can be created. It will be populated with a
>>>> map of cacheName->typeName values at initialization time and validation
>>>>
>>> may
>>>
>>>> look like below
>>>>
>>>> @CacheNameResource String cacheName;
>>>>
>>>> public Object onBeforePut(Cache.Entry<String,BinaryObject>
>>>> entry,BinaryObject  newVal) {
>>>>      if (typeId ==null) {
>>>>          synchronized (cachesTypes) {
>>>>              if (typeId ==null) {
>>>>                  String allowedType =cachesTypes.get(cacheName);
>>>>
>>>>                  if (allowedType ==null) {
>>>>                      typeId =0;
>>>>
>>>>                      throw new IgniteException("No type for cache name:"
>>>> +cacheName);
>>>>                  }
>>>>
>>>>                  typeId =
>>>> Ignition.ignite().binary().typeId(allowedType);
>>>>              }
>>>>          }
>>>>      }
>>>>
>>>>      if (newVal.type().typeId() !=typeId)
>>>>          throw new IgniteException("Invalid object type [validType="
>>>> +typeId +", wrongType=" + newVal.type().typeId());
>>>>
>>>>      return newVal;
>>>> }
>>>>
>>>> However if we want to start a new cache then we won't be able to add a
>>>>
>>> new
>>>
>>>> entry to "cacheTypes" map. 3) MutableConfiguration.setTypes(Class<K>
>>>> keyType, Class<V> valueType) based approach According to the spec
>>>>
>>>> Implementations may further perform type checking on mutative cache
>>>> operations * and throw a {@link ClassCastException} if these checks
>>>> fail.
>>>>
>>>> If we see that value type is BinaryObject (this can be a case if there
>>>> BinaryObject is not going to be deserialized because no real class
>>>> exists
>>>> for it) then we can take its typeName/typeId and compare it with the
>>>> same
>>>> parameters at the time instances of BinaryObjects will be put to cache
>>>> using cache.put(). So, every time cache.put()/putAll() are called we can
>>>> check type on client nodes before sending values to servers. In case of
>>>> CacheEntryProcessors servers will double-check an entry type after the
>>>> entry processor does its job. I prefer the latest approach. Any thoughts
>>>>
>>> on
>>>
>>>> this? -- Denis
>>>>
>>>>
>

Re: Controlling type of object inserted in IgniteCache

Posted by Denis Magda <dm...@gridgain.com>.
Finally, I come up with an idea that such an implementation is quite 
complex and introduces and additional overhead.

It's preferable to rely on the cache interceptor based solution:
- user will create a generic interceptor that will accept "typeName" in 
its constructor;
- class of this interceptor will be placed in the classpath of all the 
nodes;
- when a new cache is started statically (Spring cfg) or dynamically 
this interceptor can be set with a particular allowed "typeName";
- when interceptor is invoked allowed typeId (taken from allowed 
"typeName") will be compared with a typeID of a key/value being inserted;
- exception has to be thrown if allowed typeId is not equal to a 
key/value typeId being inserted.

We need to allow to throw exceptions from interceptor and provide an 
example on how perform type validation using interceptors based approach.

Updated the ticket.

--
Denis

On 4/5/2016 7:40 AM, Denis Magda wrote:
> Hi Vladislav,
>
> I would avoid basing the comparison of valid types on class names 
> because Strings comparisons is linear - O(n).
>
> My suggestion is to convert the class names set in 
> CacheConfiguration#setTypeNames(...) to int type IDs and compare these 
> valid type IDs with type IDs of classes set to cache over put(), 
> replace(), etc.
>
> To convert String class names to int type IDs you can follow this 
> approach:
> - if IgniteCacheObjectProcessor is of type 
> CacheObjectBinaryProcessorImpl, meaning that BinaryMarshaller is used 
> system wide, then you can directly call 
> CacheObjectBinaryProcessorImpl#typeId(*Object *obj) from your code to 
> get type ID of an object being put to cache using put(), replace(), 
> etc. This method also take cares of BinaryObjects created with 
> BinaryObjectBuilderImpl out of the box.
> - otherwise if IgniteCacheObjectProcessor is not of 
> CacheObjectBinaryProcessorImple, meaning that other Marshaller is used 
> system wide, then you can create your own simple function that will 
> convert Object class name to type ID by converting the name to 
> hashCode (clsName.hashCode()) and use such typed IDs for validation.
>
> Also it's wort to mention that if the user decided to use 
> MutableConfiguration.setTypes(Class<K>keyType, Class<V> valueType) 
> then you can internally use the same implementation done for 
> CacheConfiguration#setTypeNames(...) using keyType as 
> keyType.getName() and valueType as valueType.getName().
>
> If the user sets both CacheConfiguration#setTypeNames(...) and 
> MutableConfiguration#setTypes(...) then I would throw an Exception.
>
> --
> Denis
>
>
> On 4/4/2016 7:32 PM, Vladislav Pyatkov wrote:
>> Hi Ignite dev comunity,
>>
>> I an start to work onhttps://issues.apache.org/jira/browse/IGNITE-2909
>> The problem occurs when we add objects in cache in node where no
>> information abut types (for example server node without end user classes in
>> classpath).
>> After reserch, I try to find way to solve issue.
>> The intention:
>> 1) Add method to CacheConfiguration#setTypeNames(String keyClass String
>> valClass) (like suggested earlier)
>> 2) Before every modification cache need check class matching (in put(),
>> putAll(), replace() etc).
>> Maybe it not simple string comparison.
>> 3) If class is matching then continue saving, else throwing exception.
>>
>> In my opinion, it actualy only if put cache value as BynaryObject (get
>> class binaryObject.type().typeName()).
>> If we check the class name, it is not relevant objects do not put to cache.
>>
>> I would like to hear the comunity opinion.
>>
>> On Wed, Mar 30, 2016 at 7:08 AM, Denis Magda<dm...@gridgain.com>  wrote:
>>
>>> Sounds good. Created the ticket
>>> https://issues.apache.org/jira/browse/IGNITE-2909
>>>
>>>
>>> On 3/30/2016 2:40 AM, Dmitriy Setrakyan wrote:
>>>
>>>> On Tue, Mar 29, 2016 at 3:49 PM, Valentin Kulichenko <
>>>> valentin.kulichenko@gmail.com> wrote:
>>>>
>>>> How about adding setTypeNames() configuration property that will be
>>>>> similar
>>>>> to setTypes(), but for binary objects?
>>>>>
>>>>> Agree. Sounds like it will solve the problem, no?
>>>> -Val
>>>>> On Tue, Mar 29, 2016 at 1:56 AM, Denis Magda<dm...@gridgain.com>
>>>>> wrote:
>>>>>
>>>>> Igniters,
>>>>>> In some cases there is necessity to control a type of object that is
>>>>>>
>>>>> being
>>>>>
>>>>>> inserted into a cache.
>>>>>> Presently this kind of check is accomplished at compile time only
>>>>>> relying
>>>>>> on Java Generics. However it doesn't prevent us from connecting to a
>>>>>> cluster using a non-generic instance of a cache and put any kind of data
>>>>>>
>>>>> in
>>>>>
>>>>>> it.
>>>>>> This may be not a harmful intention but rather a silly developer
>>>>>> mistake.
>>>>>>
>>>>>> I see the following options here.
>>>>>>
>>>>>> 1) First CacheInterceptor based approach.
>>>>>> Specify a unique interceptor per cache setting expected typeName/typeId
>>>>>>
>>>>> to
>>>>>
>>>>>> it at construction time and the interceptor will validate object type in
>>>>>> onBeforePut() method.
>>>>>> Disadvantages:
>>>>>> - cache interceptor have to be created for every cache;
>>>>>> - cache interceptor class has to be located in servers classpath. It
>>>>>>
>>>>> means
>>>>>
>>>>>> that we won't be able to start a new cache with its interceptor later.
>>>>>>
>>>>>> 2)  Second CacheInterceptor based approach.
>>>>>> A generic cache interceptor can be created. It will be populated with a
>>>>>> map of cacheName->typeName values at initialization time and validation
>>>>>>
>>>>> may
>>>>>
>>>>>> look like below
>>>>>>
>>>>>> @CacheNameResource String cacheName;
>>>>>>
>>>>>> public Object onBeforePut(Cache.Entry<String,BinaryObject>
>>>>>> entry,BinaryObject  newVal) {
>>>>>>       if (typeId ==null) {
>>>>>>           synchronized (cachesTypes) {
>>>>>>               if (typeId ==null) {
>>>>>>                   String allowedType =cachesTypes.get(cacheName);
>>>>>>
>>>>>>                   if (allowedType ==null) {
>>>>>>                       typeId =0;
>>>>>>
>>>>>>                       throw new IgniteException("No type for cache name:"
>>>>>> +cacheName);
>>>>>>                   }
>>>>>>
>>>>>>                   typeId =
>>>>>> Ignition.ignite().binary().typeId(allowedType);
>>>>>>               }
>>>>>>           }
>>>>>>       }
>>>>>>
>>>>>>       if (newVal.type().typeId() !=typeId)
>>>>>>           throw new IgniteException("Invalid object type [validType="
>>>>>> +typeId +", wrongType=" + newVal.type().typeId());
>>>>>>
>>>>>>       return newVal;
>>>>>> }
>>>>>>
>>>>>> However if we want to start a new cache then we won't be able to add a
>>>>>>
>>>>> new
>>>>>
>>>>>> entry to "cacheTypes" map. 3) MutableConfiguration.setTypes(Class<K>
>>>>>> keyType, Class<V> valueType) based approach According to the spec
>>>>>>
>>>>>> Implementations may further perform type checking on mutative cache
>>>>>> operations * and throw a {@link ClassCastException} if these checks
>>>>>> fail.
>>>>>>
>>>>>> If we see that value type is BinaryObject (this can be a case if there
>>>>>> BinaryObject is not going to be deserialized because no real class
>>>>>> exists
>>>>>> for it) then we can take its typeName/typeId and compare it with the
>>>>>> same
>>>>>> parameters at the time instances of BinaryObjects will be put to cache
>>>>>> using cache.put(). So, every time cache.put()/putAll() are called we can
>>>>>> check type on client nodes before sending values to servers. In case of
>>>>>> CacheEntryProcessors servers will double-check an entry type after the
>>>>>> entry processor does its job. I prefer the latest approach. Any thoughts
>>>>>>
>>>>> on
>>>>>
>>>>>> this? -- Denis
>>>>>>
>>>>>>
>


Re: Controlling type of object inserted in IgniteCache

Posted by Denis Magda <dm...@gridgain.com>.
Hi Vladislav,

I would avoid basing the comparison of valid types on class names 
because Strings comparisons is linear - O(n).

My suggestion is to convert the class names set in 
CacheConfiguration#setTypeNames(...) to int type IDs and compare these 
valid type IDs with type IDs of classes set to cache over put(), 
replace(), etc.

To convert String class names to int type IDs you can follow this approach:
- if IgniteCacheObjectProcessor is of type 
CacheObjectBinaryProcessorImpl, meaning that BinaryMarshaller is used 
system wide, then you can directly call 
CacheObjectBinaryProcessorImpl#typeId(*Object *obj) from your code to 
get type ID of an object being put to cache using put(), replace(), etc. 
This method also take cares of BinaryObjects created with 
BinaryObjectBuilderImpl out of the box.
- otherwise if IgniteCacheObjectProcessor is not of 
CacheObjectBinaryProcessorImple, meaning that other Marshaller is used 
system wide, then you can create your own simple function that will 
convert Object class name to type ID by converting the name to hashCode 
(clsName.hashCode()) and use such typed IDs for validation.

Also it's wort to mention that if the user decided to use 
MutableConfiguration.setTypes(Class<K>keyType, Class<V> valueType) then 
you can internally use the same implementation done for 
CacheConfiguration#setTypeNames(...) using keyType as keyType.getName() 
and valueType as valueType.getName().

If the user sets both CacheConfiguration#setTypeNames(...) and 
MutableConfiguration#setTypes(...) then I would throw an Exception.

--
Denis


On 4/4/2016 7:32 PM, Vladislav Pyatkov wrote:
> Hi Ignite dev comunity,
>
> I an start to work on https://issues.apache.org/jira/browse/IGNITE-2909
> The problem occurs when we add objects in cache in node where no
> information abut types (for example server node without end user classes in
> classpath).
> After reserch, I try to find way to solve issue.
> The intention:
> 1) Add method to CacheConfiguration#setTypeNames(String keyClass String
> valClass) (like suggested earlier)
> 2) Before every modification cache need check class matching (in put(),
> putAll(), replace() etc).
> Maybe it not simple string comparison.
> 3) If class is matching then continue saving, else throwing exception.
>
> In my opinion, it actualy only if put cache value as BynaryObject (get
> class binaryObject.type().typeName()).
> If we check the class name, it is not relevant objects do not put to cache.
>
> I would like to hear the comunity opinion.
>
> On Wed, Mar 30, 2016 at 7:08 AM, Denis Magda <dm...@gridgain.com> wrote:
>
>> Sounds good. Created the ticket
>> https://issues.apache.org/jira/browse/IGNITE-2909
>>
>>
>> On 3/30/2016 2:40 AM, Dmitriy Setrakyan wrote:
>>
>>> On Tue, Mar 29, 2016 at 3:49 PM, Valentin Kulichenko <
>>> valentin.kulichenko@gmail.com> wrote:
>>>
>>> How about adding setTypeNames() configuration property that will be
>>>> similar
>>>> to setTypes(), but for binary objects?
>>>>
>>>> Agree. Sounds like it will solve the problem, no?
>>>
>>> -Val
>>>> On Tue, Mar 29, 2016 at 1:56 AM, Denis Magda <dm...@gridgain.com>
>>>> wrote:
>>>>
>>>> Igniters,
>>>>> In some cases there is necessity to control a type of object that is
>>>>>
>>>> being
>>>>
>>>>> inserted into a cache.
>>>>> Presently this kind of check is accomplished at compile time only
>>>>> relying
>>>>> on Java Generics. However it doesn't prevent us from connecting to a
>>>>> cluster using a non-generic instance of a cache and put any kind of data
>>>>>
>>>> in
>>>>
>>>>> it.
>>>>> This may be not a harmful intention but rather a silly developer
>>>>> mistake.
>>>>>
>>>>> I see the following options here.
>>>>>
>>>>> 1) First CacheInterceptor based approach.
>>>>> Specify a unique interceptor per cache setting expected typeName/typeId
>>>>>
>>>> to
>>>>
>>>>> it at construction time and the interceptor will validate object type in
>>>>> onBeforePut() method.
>>>>> Disadvantages:
>>>>> - cache interceptor have to be created for every cache;
>>>>> - cache interceptor class has to be located in servers classpath. It
>>>>>
>>>> means
>>>>
>>>>> that we won't be able to start a new cache with its interceptor later.
>>>>>
>>>>> 2)  Second CacheInterceptor based approach.
>>>>> A generic cache interceptor can be created. It will be populated with a
>>>>> map of cacheName->typeName values at initialization time and validation
>>>>>
>>>> may
>>>>
>>>>> look like below
>>>>>
>>>>> @CacheNameResource String cacheName;
>>>>>
>>>>> public Object onBeforePut(Cache.Entry<String,BinaryObject>
>>>>> entry,BinaryObject  newVal) {
>>>>>       if (typeId ==null) {
>>>>>           synchronized (cachesTypes) {
>>>>>               if (typeId ==null) {
>>>>>                   String allowedType =cachesTypes.get(cacheName);
>>>>>
>>>>>                   if (allowedType ==null) {
>>>>>                       typeId =0;
>>>>>
>>>>>                       throw new IgniteException("No type for cache name:"
>>>>> +cacheName);
>>>>>                   }
>>>>>
>>>>>                   typeId =
>>>>> Ignition.ignite().binary().typeId(allowedType);
>>>>>               }
>>>>>           }
>>>>>       }
>>>>>
>>>>>       if (newVal.type().typeId() !=typeId)
>>>>>           throw new IgniteException("Invalid object type [validType="
>>>>> +typeId +", wrongType=" + newVal.type().typeId());
>>>>>
>>>>>       return newVal;
>>>>> }
>>>>>
>>>>> However if we want to start a new cache then we won't be able to add a
>>>>>
>>>> new
>>>>
>>>>> entry to "cacheTypes" map. 3) MutableConfiguration.setTypes(Class<K>
>>>>> keyType, Class<V> valueType) based approach According to the spec
>>>>>
>>>>> Implementations may further perform type checking on mutative cache
>>>>> operations * and throw a {@link ClassCastException} if these checks
>>>>> fail.
>>>>>
>>>>> If we see that value type is BinaryObject (this can be a case if there
>>>>> BinaryObject is not going to be deserialized because no real class
>>>>> exists
>>>>> for it) then we can take its typeName/typeId and compare it with the
>>>>> same
>>>>> parameters at the time instances of BinaryObjects will be put to cache
>>>>> using cache.put(). So, every time cache.put()/putAll() are called we can
>>>>> check type on client nodes before sending values to servers. In case of
>>>>> CacheEntryProcessors servers will double-check an entry type after the
>>>>> entry processor does its job. I prefer the latest approach. Any thoughts
>>>>>
>>>> on
>>>>
>>>>> this? -- Denis
>>>>>
>>>>>