[jboss-dev-forums] [Design of JBossCache] - Re: POJOization of config elements

[bstansberry@jboss.com]

As mentioned on the dev list, a lot has been checked in on this.  There are some bits of ugliness.  Manik, you pointed out the problem with the use of inner classes; I should be able to fix that today.

Other ugliness:

1) MC doesn't like overloading the setter on a property, which is an issue for the properties that are enum types.  E.g.:


  | public CacheMode getCacheMode()
  | public void setCacheMode(CacheMode mode)
  | public void setCacheMode(String mode)
  | 

doesn't work; the MC wants to pass String "REPL_ASYNC" to the overloaded method that takes CacheMode, and thus blows up.

To fix, I had to do this for CacheMode, IsolationLevel and NodeLockingScheme:


  | public CacheMode getCacheMode()
  | public void setCacheMode(CacheMode mode)
  | public void setCacheMode(String mode) // maybe redundant??
  | public String getCacheModeString()
  | public void setCacheModeString(String mode)
  | 

I don't know if it's practical to add a PropertyEditor to MC to handle enum conversions; for now the above is ugly but works.


2) Names of properties in Configuration.  Configuration can take 2 different types for each of the 3 complex sub-configurations (BR, cache loading, eviction).  These are an Element, for legacy support, and a pojo for configuration via the MC. So, 2 properties for each sub-configuration.  Problem is naming these properties; the Element properties already have names, but the naming conventions are inconsistent.  Changing the Element properties will break existing config files, so I worked around that when naming the pojo properties.  Ugly, completely inconsistent result is:


  | // BR
  | public Element getBuddyReplicationConfig()
  | public void setBuddyReplicationConfig(Element config)
  | 
  | public BuddyReplicationConfig getBuddyReplicationConfiguration()
  | public void setBuddyReplicationConfiguration(BuddyReplicationConfig config)
  | 
  | // Cache Loader
  | public Element getCacheLoaderConfiguration()
  | public void setCacheLoaderConfiguration(Element config)
  | 
  | public CacheLoaderConfig getCacheLoaderConfig()
  | public void setCacheLoaderConfig(CacheLoaderConfig config)
  | 
  | // Eviction
  | public Element getEvictionPolicyConfig()
  | public void setEvictionPolicyConfig(Element config)
  | 
  | public EvictionConfig getEvictionConfig()
  | public void setEvictionConfig(EvictionConfig config)
  | 

One temptation is to break compatibility with existing config files for BR by swapping the property names. There aren't likely to be many existing configs that have BR.  E.g.


  | public Element getBuddyReplicationConfiguration()
  | public void setBuddyReplicationConfiguration(Element config)
  | 
  | public BuddyReplicationConfig getBuddyReplicationConfig()
  | public void setBuddyReplicationConfig(BuddyReplicationConfig config)
  | 


3) Eviction configuration naming.  The Pojo configuration looks like this:

class EvictionConfig -- top level object.  Configure your default eviction policy here, plus the interval for the eviction thread.  Also takes a list of...

class EvictionRegionConfig -- one per region.  Configure the Fqn here, plus the class name of the EvictionPolicy (if you want to override the overall default).  Also takes an impl of....

interface EvictionConfiguration. the type-specific config for the EvictionPolicy.

Couple problems here.  First "EvictionConfiguration" would be better named "EvictionPolicyConfig", since its a config for a specific policy.  But, that interface already exists in 1.4, so changing that name will break any existing custom impls.  Don't know if we care about that.  Second, there's already an unused interface named EvictionPolicyConfig, along with a DefaultEvictionPolicyConfig impl.  That code is unused; looks like it was meant more as a top-level config object.  We should drop that code after ensuring that any design ideas from it get picked up in the new stuff.


Any thoughts/suggestions are most welcome.

View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3980128#3980128

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=3980128