-
1. Re: Consolidate delegating cache loader classes [JBCACHE-113
If you look at the DelegatingCacheLoader, it doesn't really do anything at all except proxy calls to abstract methods that need implementing - e.g., put() -> delegatePut(), get() -> delegateGet(), etc etc. Why not just have the implementation (E.g., TcpDelegatingCacheLoader) just implement CacheLoader directly rather than extend DelegatingCacheLoader (the way CCL does)? You have to implement the same number of methods anyway.
The only thing DCL adds is tracking transactions in a map. This is also done in AbstractCacheLoader, which DCL ought to extend.
Now if DCL provided no-op impls of the delegateXYZ() methods, then this makes sense since the implementation class could only override calls it would like to delegate.
In this case I see a benefit for CCL extending DCL.
AbstractDelegatingCacheLoader (which delegates to other cache loaders) - makes more sense in it's implementation, although I don't quite see much to be consolidated between ADCL and DCL.
What does need refactoring though is the names! AbstractDelegatingCacheLoader (delegates to cache loaders) and DelegatingCacheLoader (delegates to caches)? Hugely confusing. :-) -
2. Re: Consolidate delegating cache loader classes [JBCACHE-113
I wanted to note one thing that I forgot in my initial explanation. The reason that lead me to look at all this was the fact that I need any SingletonStoreCacheLoader implementations to extend AbstractDelegatingCacheLoader. I would rather customer having to implement an interface rather than extend a class as it gives them more flexibility. They can always extend AbstractDelegatingCacheLoader and gain from the benefits of overriding just the methods they want, but if alternatively they can implement an interface with getCL/setCL (a name that implies a CL that delegates to another CL, with the possibility of getting/setting the underlying CL), much better :)
I totally agree with the naming being confusing, hence my attempt to make a separation between cache/CL delegating CLs.
I think TcpDelegatingCacheLoader is a bit different to the rest of cache delegating caches.
The rest, LDCL, CCL, and the possible outcome of http://jira.jboss.com/jira/browse/JBCACHE-789, do pretty much the same, delegate cache loading to other caches. First one to a local cache, second to another cache via MethodCalls (using the caches RPC manager) and the third one via proxies. All three just mentioned could just extend AbstractCacheLoader as you said and override what needs overriding.
I like this idea (which is what u're saying in your 1st paragraph) more than having CCL extend DCL.AbstractDelegatingCacheLoader (which delegates to other cache loaders) - makes more sense in it's implementation
Of course it does!!, i created it myself! hehehehe :-p ;-). This is the cache loader that I want getCL/setCL factored out into an interface to make people who re implement SSCL easier as mentioned earlier. -
3. Re: Consolidate delegating cache loader classes [JBCACHE-113
Quick note on the requirement of SSCL to have getCL/setCL. getCL is necessary so that SSCL delegates to that CL. setCL is there so that when processing the configuration, we can set the CL that it needs to delegate to. In the same fashion that async cl does.
-
4. Re: Consolidate delegating cache loader classes [JBCACHE-113
Ok, here is what I propose:
1) Remove the DelegatingCacheLoader class. I don't see any value add here. TcpDelegatingCacheLoader and LocalDelegatingCacheLoader directly extend AbstractCacheLoader.
2) Add protected methods getCacheLoader()/setCacheLoader() to AbstractDelegatingCacheLoader so all implementors need to do - at a minimum - is to extend the class and call setCacheLoader() in the constructor to set who to delegate to.
WDYT? -
5. Re: Consolidate delegating cache loader classes [JBCACHE-113
Re 1 - OK --> this needed doing to avoid confusion.
Re 2 - setCL/getCL are already in ADCL, so I'm happy to leave them either public or protected. Protected might make more sense cos only o.c.j.loader code or subclasses should be using it.