[hibernate-dev] [infinispan-dev] [ISPN-6] (Infinispan cache provider for Hibernate) Remaining TODOs, notes and questions

[Manik Surtani]

On 11 Aug 2009, at 16:59, Galder Zamarreno wrote:

>
>
> On 08/11/2009 05:42 PM, Manik Surtani wrote:
>>
>> On 11 Aug 2009, at 16:34, Galder Zamarreno wrote:
>>
>>>
>>>
>>> On 08/10/2009 01:09 PM, Galder Zamarreno wrote:
>>>>
>>>> On 08/10/2009 12:42 PM, Manik Surtani wrote:
>>>
>>> </snip>
>>>
>>>>> Well, I think we need both.
>>>>>
>>>>> getConfiguration(String name) to retrieve a configuration  
>>>>> template and
>>>>> modify it.
>>>>> defineCache(String newName, String templateName, Configuration
>>>>> overrides) to create a new configuration based on an existing one.
>>>>> Maybe
>>>>> this name should change to something more intuitive? Perhaps  
>>>>> instead of
>>>>> defineCache, we have:
>>>>>
>>>>> Configuration defineConfiguration(String configurationName,
>>>>> Configuration overrides) // registers and names a NEW  
>>>>> configuration,
>>>>> based on the default cfg and the overrides passed in
>>>>>
>>>>> Configuration defineConfiguration(String configurationName, String
>>>>> templateName, Configuration overrides) // registers and names a  
>>>>> NEW
>>>>> configuration, based on an existing, predefined configuration  
>>>>> and the
>>>>> overrides passed in
>>>>>
>>>>> WDYT?
>>>>
>>>> That would be a great way to combine the two API requirements. I  
>>>> assume
>>>> that the returned Configuration instances returned would have had  
>>>> the
>>>> overrides applied to them already.
>>>>
>>>> Let me work on this APIs in parallel to the work I'm doing for  
>>>> ISPN-6 so
>>>> that I can fully exercise/test and see if there's anything we'd  
>>>> need to
>>>> change.
>>>>
>>>> https://jira.jboss.org/jira/browse/ISPN-152
>>>
>>> A quick update on this. I have implemented ISPN-152, integrated it
>>> with the cache provider and the tests I have are now passing. Just a
>>> heads up on a change of behaviour.
>>>
>>> defineConfiguration(*) methods do no longer throw
>>> DuplicateCacheNameException since this limits the API a lot  
>>> disabling
>>> any type of configuration redefininitions/overriding. Example:
>>>
>>> defineConfiguration("entity", "entity", overrides);
>>>
>>> This means, take the configuration for named cache "entity", apply
>>> overrides and use that as new "entity" cache name configuration. If
>>> we're throwing DuplicateCacheNameException(s), we wouldn't be able  
>>> to
>>> do this and this seems like a valid use case to me.
>>>
>>> By the way, I've already noted this in the javadoc but doing the
>>> following:
>>>
>>> Configuration c = defineConfiguration("entity", new  
>>> Configuration());
>>>
>>> is a way to return "entity" named cache's configuration! (equivalent
>>> to doing something like: manager.getConfiguration("entity") if such
>>> method existed!) IOW, this defineConfiguration method is saying:
>>> "define a configuration, based on "entity" and apply no overrides  
>>> (cos
>>> no setters have been called in the Configuration instance!) and  
>>> return
>>> new configuration instance"
>>>
>>> I've detailed all this in CacheManager's javadoc that you can find
>>> attached. If you get the chance, have a read to it and let me know  
>>> if
>>> anything is not clear enough.
>>
>> Most of it sounds good, but the javadoc on defineConfiguration() is a
>> bit misleading.
>>
>> " * <p/>
>> * If cache name hasn't been defined before, this method creates a  
>> clone
>> of the configuration of the cache whose
>> * name matches the given template cache name, then applies the
>> configuration overrides passed and returns this
>> * configuration instance.
>> * <p/>
>> * If cache name has been previously defined, this method creates a  
>> clone
>> of this cache's existing configuration,
>> * applies the overrides and return this configuration instance.
>> * <p/>"
>>
>> The first bit makes sense. The second bit though, I would expect the
>> behaviour to be to clone the template cfg, apply changes, and  
>> overwrite
>> the existing named config rather than use the existing named config  
>> as
>> the template (rather than templateCacheName).
>
> Hmmm, right, so defineConfiguration(String, String, Configuration)  
> should work the same way regardless of whether the configuration for  
> the named cache exists or not, correct?
>
> In both cases, a clone a clone of the template cache's configuration  
> would be made, apply a clone of the overrides, and overwrite  
> existing named cache's configuration.

Yup.

>
> In fact, a side thing, this has reminded my that "then applies the  
> configuration overrides passed" should say "then applies a clone of  
> the configuration overrides passed in", this makes it clear that the  
> configuration passed in cannot be modified after calling  
> defineConfiguration() and expect its changes to be reflected in the  
> CacheManager.

+1.

--
Manik Surtani
manik at jboss.org
Lead, Infinispan
Lead, JBoss Cache
http://www.infinispan.org
http://www.jbosscache.org