[infinispan-dev] [Pull Request] Modular Classloading Compatibility

[Pete Muir]

On 5 May 2011, at 03:31, Galder Zamarreño wrote:

> 
> On May 4, 2011, at 4:08 PM, Pete Muir wrote:
> 
>> 
>> On 4 May 2011, at 09:55, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)" <trustin at gmail.com> wrote:
>>>> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
>>>>> 2011/5/3 "이희승 (Trustin Lee)" <trustin at gmail.com>:
>>>>>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>>>>>> 2011/5/2 Manik Surtani <manik at jboss.org>:
>>>>>>>> 
>>>>>>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>>>>>>> 
>>>>>>>>>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>>>>>>>> 
>>>>>>>>>> Not for put, since you have the class, just get, and I was thinking
>>>>>>>>>> something more like:
>>>>>>>>>> 
>>>>>>>>>> Foo foo = getUsing(key, Foo.class)
>>>>>>>>> 
>>>>>>>>> This would be a pretty useful addition to the API anyway to avoid user casts.
>>>>>>>> 
>>>>>>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>>>>>>>> 
>>>>>>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>>>>>> 
>>>>>>> doesn't look much better than a cast, but is more cryptical :)
>>>>>>> 
>>>>>>> getting back to the classloader issue, what about:
>>>>>>> 
>>>>>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>>>>>> 
>>>>>>> or
>>>>>>> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>>>>>>> 
>>>>>>> BTW if that's an issue on the API, maybe you should propose it to
>>>>>>> JSR-107 as well ?
>>>>>> 
>>>>>> We have a configurable Marshaller, right?  Then why don't we just use
>>>>>> the class loader that the current Marshaller uses?
>>>>> 
>>>>> +1
>>>>> I like the clean approach, not sure how you configure the "current
>>>>> Marshaller" to use the correct CL ?
>>>>> Likely hard to do via configuration file :)
>>>> 
>>>> By default, we could use the class loader that the current Unmarshaller
>>>> uses, and let user choose a class loader for a certain get() call.
>>>> 
>>>> So, we need to deal with the two issues here:
>>>> 
>>>> 1) Make sure that user can configure the default class loader in runtime
>>>> and calling get() (and consequently related unmarshaller) will use the
>>>> default class loader:
>>>> 
>>>> cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>>>> 
>>>> 2) Provide an additional API that allows a user to specify a class
>>>> loader for the current transaction or context:
>>>> 
>>>> cache.get(K key, Class<V> clazz) // +1 to Pete's suggestion
>>>> 
>>> 
>>> If V is Set<MyObject> then Set<MyObject>.class.getClassLoader() will
>>> give you the system classloader, which in most cases won't be able the
>>> MyObject class. It may not even be a Set<MyObject> of course, it could
>>> be just Set<?>.
>>> 
>>> We could have a "shortcut" so the users can pass in a class instead of
>>> a classloader to avoid writing ".getClassLoader()", but we shouldn't
>>> require any relation between the class passed in and the class of the
>>> return value.
>> 
>> There are two forces at play here. Firstly the desire to introduce greater type safety into the Cache API, by returning a type for user, rather than just an object, and secondly the need to specify the CL.
>> 
>> We could have an API like:
>> 
>> <V, CL> V get(Object key, Class<CL> classLoaderType);
>> 
>> and an overloaded form
>> 
>> <V> V get(Object key, ClassLoader cl);
>> 
>> This does involve Infinispan having to do a runtime cast to V without the class type passed in as a parameter, but this may be a necessary evil in the interest of improving the user api.
>> 
>> An alternative which is somewhat more verbose, somewhat safer, and somewhat more complex to explain (but ultimately more logical):
>> 
>> // Use the classloader of the expectedType, this would be the normal case
>> <V> V get(Object key, Class<V> expectedType);
>> 
>> // Specify the classloader to use as a class
>> <V, CL> V get(Object key, Class<V> expectedType, Class<CL> classLoader);
>> 
>> // Specify the classloader to use as a classloader
>> <V> V get(Object key, Class<V> expectedType, ClassLoader classLoader);
>> 
>> We could also include
>> 
>> // Use the TCCL
>> V get(Object key);
>> 
>> Any thoughts?
> 
> There's something that does not convince about adding these classloader and class params to the get() calls and that's the fact that you're not only gonna have to do it for get(), but for all methods that return V and that is: all get variants, getAsync...etc, and all the methods that return the previous value, such as put, putAsync, replace, replaceAsync....etc and that's a fair amount of extra methods. I much prefer an API like we used with flags which has been partly suggested above, i.e.
> 
> cache.withClassloader(classloader).asClass(expectedType).get(Object key)
> cache.withClassloader(classloader).get(Object key)
> cache.asClass(expectedType).get(Object key)

In this case I would prefer to just do a cast :-)

See other email about resetting the discussion.