Background:

While working on refactoring interceptor chain in JBossCache 2.1 we came with some more ample refactoring ideas which can be performed in JBossCache 2.2. A proof-of-concept was created and we internally had some discussions.

This wiki is intended as an place to centralize ideas around this subject.

Target

This is targeted for JBoss Cache 2.2.0. 

JIRA

JBCACHE-1222

Design Forum Thread

http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4138604

Problems with existing architecture

Generally speaking there are issues relate to maintainability, reliability and unit testability of the existing code. In particular we identified CacheImpl (approx 2400 LOC) as a good candidate for refactoring, as presented bellow.

At the moment CacheImpl is doing four different things (bad smell, see reference 1):

1. manage cache life cycle (start(), stop(), destroy(), create())

2. manage interceptor chain (addInterceptor(), removeInterceptor(), invoke())

3. Cache interface implementation (the _underscore methods, e.g. _put(), _putData())

4. Cache data management and search (e.g. peek(), findNode(), findNodeWithData())

As this class is heavily wired, none of the above aspects of the class/methods can be unit tested in isolation. This makes changes within the class difficult to perform, and as a result, functionality enhancements in JBoss Cache.

Refactoring - proof of concept

The trunk has been modified to split CacheImpl functionality into four components, as described above. As a result the logic from CacheImpl has been migrated and the class was removed (private API).

Attached is the patch you can apply on trunk for following for a better understanding of the resulted components. Disclaimer: the code is not compilable nor final.

Some notes on the achieved design:

• life cycle methods were moved in CacheLifecycleManager. This is a heavily wired class that removed most of the dependencies from CacheImpl.

• interceptor chain management was moved to InterceptorChainManager. This is a a lightweight class that is totally unit testable

• Cache data management and search was moved into CacheData class. Again, this is unit testable. We realized that this class is more or less a wrapper around root node, and that much logic from this class can be moved to a Node implementation (perhaps all of it?). While progressing with refactoring, more repetitive logic (especially from the interceptors) could be moved here.

• Cache interface implementation. Deals with Cache interface implementing methods (the _ 'underscore' methods in CacheImpl); most significant change. The approach we have taken is to create a class for each API method. You can find all the migrated methods within org.jboss.cache.commands package.

See diagram below for UML diagrams:

Notes:

• CacheCommand is at the top of the hierarchy. It has two methods: perform() and accept().

• perform() implementations are for performing business logic, e.g. remove a key from a node's data map.

• accept() is for allowing this command (see reference 2) to be handled by a visitor/interceptor. 

• CacheCommandInterceptor is a CacheCommand visitor (see reference 3). It knows how to visit each command through corresponding methods (e.g. handleRemoveDataCommand() would be called while visiting a RemoveDataCommand, etc.) It is intended to be at the top of interceptor hierarchy, each interceptor implementing a subset of methods. It's purpose is to make the interceptor chain strongly typed and define what can be visited exactly.

• BaseTxCacheCommand is an abstract class that adds a rollback() method. It is the base class for all the commands that we might want to rollback at some point, e.g. RemoveDataCommand, and implementations will add logic here to reverse it's perform() logic.

• All the methods are lightweight and unit testable in isolation

• rolling back is simpler since no 'corresponding' method should be registered, which was kind of cumbersome and restrictive

• There should be no performance drawback through using strongly typed commands rather than reflection, performance will improve.  Also less object construction at every stage.

• Not all currently intercepted methods really need interception. E.g. blockChannel()/unblockChannel() methods are only being intercepted for the sole purpose of sending a notification. This can be performed in one place only, i.e. not no need for interception. Other methods might be removed as well.

Backward compatibility

Most of the changes won't affect the public API. Following concerns should be addressed:

1. API compatibility for custom interceptors

2. Wire format compatibility for talking to old (2.x) instances.

these will be detailed soon.

Attached is an archive containing modified files for POC. You can unzip the archive over an trunk copy, in the /src/main/java directory, overwriting the existing files. The project is not compilable.

1 - Single Responsibility Principle

2 - Command Pattern

3 - Visitor Pattern