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

[David M. Lloyd]

On 05/04/2011 09:08 AM, 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);

See note below about the CL type parameter.

> 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.

There's no reason to do the runtime cast.  This should be spelled:

Object get(Object key, ClassLoader cl);

That way if there's a CCE, the user will stand a better chance of
understanding it (because hey, they did a cast).

> 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);

This one doesn't make a lot of sense to me.  In particular there's no
reason to have a type parameter CL unless you give it a bound in
relationship to V, in which case you could have just used the class
loader class in the first place.  Maybe you meant:

<V> V get(Object key, Class<V> expected, Class<?>
classWhoseClassLoaderShouldBeUsed);

And I would name that parameter exactly thus :)

> // 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);

Or a preset default CL?

-- 
- DML