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

[Dan Berindei]

2011/5/4 Pete Muir <pmuir at redhat.com>:
>
> 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.
>

Cache<K,V> extends Map<K,V>, so we already have runtime casts to V for

V get(Object key)

It seems natural to extend it to also return objects of the cache's V
type parameter instead of returning an arbitrary type:

<CL> V get(Object key, Class<CL> classLoaderType);
V get(Object key, ClassLoader cl);

Besides, allowing get() to return an arbitrary type will only make the
user code look more type safe, without changing its behaviour at all.
If the user wants type safety he can already define different caches
with different V type parameters and in that case we will actually
enforce that the put argument is assignable to V at compile time.

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

I'd allow the user to set another default classloader on the cache
instead of the TCCL, like in Sanne's proposal:

Cache c = cacheManager.getCache( cacheName, defaultClassLoader );
or
Cache c = cacheManager.getCache( cacheName ).usingDefaultClassLoader(
classLoader );

Dan