This is a working log of a multi-phased spike to experiment with the drools-core design. The phases I have currently have in mind are:

Perform a series of refactorings on the drools-core extracting out crosscutting concerns and extensions into aspects. These include (maybe more, maybe less):
- events
- duration
- noloop
- dynamic facts (ie, fact property change notifications)
- salience conflict resolution
- recency conflict resolution
Incorpoate (at least) not-condition nodes into reteoo

The basic approach (at first) will be to tear out all but the essential behavior from core, making the implementation as simple as possible. From this point the features will be weaved back in. At first I may just comment out lots of code with markers indicating what it did. This will be so that I can quickly view the previous implementation when weaving it back in. But I'm not sure I'll stick with this, as I have a feeling that all that noise will annoy me too much.

For the duration of this spike, I may be violating some of our naming guidelines. Specifically, I may prefix newly extracted interfaces with "I" to avoid renaming the concrete class. The reason being that I want to rentain as much CVS history as possible. I'll probably also reformat any code that I work on heavily to a more compressed style (like drools-spring).

If you want to play also, or just track the changes, simply checkout the memelet-aspectj branch from cvs.

02 June 05

Getting started

Create a CVS branch named memelet-aspectj.
Commented or disabled all event related code (comment key=event).
Commented or disabled all duration related code (comment key=duration).
Commented or disabled all fact bean-property change notification (comment key=dynamic).

Ok, now that all this has been disabled and marked I got started with the event handler aspects. Its so clean! But, once again (this has been a theme for the last six months) the aspectj compiler and/or ajdt plugin get very confused and random. I posted to the newsgroup, we'll see if someone can make sense of what I am seeing.

03 June 05

ObjectAsserted/Retracted/ModifiedEvent (tag 'memelet-aspectj-day2a')

After a some help from the newsgroup and a new compiler from Andy Clement I'm off again.

I had to restructure the eclipse projects a bit. I am introducting methods into WorkingMemory (eg, addListener, removeListener). However, from within the drools-core project, a regular java test class cannot see these methods unless you open the file with the aspectj editor. Since our users will probably not want to do that, I needed to write tests that simulate the general usage environment. To achieve this, I created the project drools-core-aspectjtest. This project does not depend directly on drools-core, but rather includes its output path as a dependency, thus simulating the use of drools-core.jar. Now tests in drools-core-aspectjtest can invoke introduced methods without any knowledge of aspectj. (Other than including aspectrt.jar in the classpath, that is.)

I have green bars that cover WorkingMemory addListener, removeListener, assertObject, retractObject, and modifyObject. I needed to change the signatures of WorkingMemory assertObject, retractObject, and modifyObject to return FactHandle, Object, Object, respectively. Without these return values, after advice would not have all the arguments to send the events. Orignally, I was using advice around the calls to RuleBase so I could get these extra values, but that was very smelly.

So here's some snippets from the aspect. First we introduce into WorkingMemory the WorkingMemoryEventSupport instance variable and the addListener/removeListener methods:

private WorkingMemoryEventSupport WorkingMemory.eventSupport = new WorkingMemoryEventSupport(this);

public void WorkingMemory.addListener(WorkingMemoryEventListener listener) {
    eventSupport.addEventListener(listener);
}

public void WorkingMemory.removeListener(WorkingMemoryEventListener listener) {
    eventSupport.removeEventListener(listener);
}

Then we define the pointcuts:

pointcut assertObject(WorkingMemory workingMemory, Object object)
: execution(FactHandle WorkingMemory+.assertObject(Object))
    && this(workingMemory) && args(object);

pointcut retractObject(WorkingMemory workingMemory, FactHandle handle)
: execution(Object WorkingMemory+.retractObject(FactHandle))
    && this(workingMemory) && args(handle);

pointcut modifyObject(WorkingMemory workingMemory, FactHandle handle, Object object)
: execution(Object WorkingMemory+.modifyObject(FactHandle, Object))
    && this(workingMemory) && args(handle, object);

Finally, we send the events via after advice:

after(WorkingMemory workingMemory, Object object) returning (FactHandle handle)
: assertObject(workingMemory, object) {
    workingMemory.eventSupport.fireObjectAsserted(handle, object);
}

after(WorkingMemory workingMemory, FactHandle handle) returning (Object object)
: retractObject(workingMemory, handle) {
    workingMemory.eventSupport.fireObjectRetracted(handle, object);
}

after(WorkingMemory workingMemory, FactHandle handle, Object object) returning (Object prevObject)
: modifyObject(workingMemory, handle, object) {
    workingMemory.eventSupport.fireObjectModified(handle, prevObject, object);
}

To see the test, checkout WorkingMemoryEventSupportTest.java.

ConditionTestedEvent (tag 'memelet-aspectj-day2b')

Next I'll tackle ConditionTestedEvent. The class that sent this event, org.drools.reteoo.ConditionNode is concrete and package private. In order to add not-conditions, we are going to need an interface for condition-nodes, so I created org.drools.spi.IConditionNode. The first problem I hit is that ConditionNode dependes on concrete reteoo classes (eg, ReteTuple, WorkignMemoryImpl). ReteTuple I change to Tuple, and WorkingMemoryImpl I change to WorkingMemory, but there is also a dependency on reteoo.TupleKey. This class does not seem to depend on anything in reteoo, so I move it spi.

After moving TupleKey I see that ReteTuple internally defines variables of type FactHandleImpl and tries to assign them from TupleKey.get* methods. So I change the fields to be of the type FactHandle. But this fails, because ReteTuple is depending on methods of FactHandleImpl. Specifically, "recency" values for use by RecencyConflictResolver. This is a smell, since conflict strategies are pluggable and should not require other classes to carry data for them. So, out all the recency stuff comes. I'll add this to the list of things to aspectize.

At this point pretty all hell is breaking loose. The package structure of drools-core seems way off the main sequence. Also many interfaces and classes depend on concrete classes. Few of the interfaces that do exist are proper behavioral abstractions, so when I refactor code to use the interfaces, they rarely export the required methods. Many of the problems seem to stem from violating Law-of-Demeter (ie, foo.getBar().getBla().contains(thng)), or at least that's what I'm seeing right now.

I am making all kinds of changes to accomadate this simple refactoring. To see all the changes you will have to compare between the cvs tags 'memelet-aspectj-day2a' and 'memelet-aspectj-day2b'.

Whew. I had to roll everything back. There were too many required changes to keep the tests green. So I'm going to take another approach (for now at least). I'll use a control-flow pointcut. Gotta do some reading...

05 June 05

ConditionTestedEvent, continued (tag 'memelet-aspectj-day5')

Ok, back to condition-testing events. We need a pointcut that will match the call to Condition.isAllowed from the ConditionNode.assertTuple. This joinpoint will match the call, and also provide is the ConditionNode instance:

pointcut conditionTested(Object object):
    call(boolean Condition.isAllowed(..)) &&
    withincode(void org.drools.reteoo.ConditionNode.assertTuple(..)) &&
    this(object);

But in order to send the ConditionTestedEvent we also need the arguments of the ConditionNode.assertTuple call:

pointcut conditionNodeAssertTupleExecution(ConditionNode conditionNode, ReteTuple tuple, WorkingMemoryImpl workingMemory):
    execution(void org.drools.reteoo.ConditionNode.assertTuple(..)) &&
    this(conditionNode) && args(tuple, workingMemory);

pointcut conditionTested(ConditionNode conditionNode, ReteTuple tuple, WorkingMemoryImpl workingMemory, Condition condition):
    cflow(conditionNodeAssertTupleExecution(conditionNode, tuple, workingMemory)) &&
    call(boolean Condition.isAllowed(..)) &&
    withincode(void org.drools.reteoo.ConditionNode.assertTuple(ReteTuple, WorkingMemoryImpl)) &&
    target(condition);

after(ConditionNode conditionNode, ReteTuple tuple, WorkingMemoryImpl workingMemory, Condition condition)
returning (boolean result)
: conditionTested(conditionNode, tuple, workingMemory, condition) {
    workingMemory.getEventSupport().fireConditionTested(conditionNode.rule, condition, tuple, result);
}

ActivationCreate/CancelledEvent

Now we get to hard stuff. Its hard only because of the implementation of WorkingMemory.modifyObject, which does a retract followed by an assert. This could cause lots of activation cancels, immediatly followed by activation creates for the same rule. In this case, we don't want to send any events because the activation changes were only an internal implemenation detail.

drools-core-aspectj