Refactoring on delegates

[clebert.suconic]

I just created a wiki page (1st draft) to support a discussion on the refactoring of our current client delegates into an agnostic model:


http://wiki.jboss.org/wiki/Wiki.jsp?page=JBossMessagingDelegateRefactoring



My goal is to make this refactoring in 3 or 4 steps, to make the process easier.

1st - Get rid os aspects (but keep any aspect acting as a generic interceptor such as Failover/Exception/Close)
2nd - Introduce proxies finally replacing the AOP pointcuts.
3rd - Rename packages & classes to what we are targeting as the final model. (for example, instead of ConnectionDelegate, rename it to just Connection... also rename the package to something reflecting the agnostic idea)
4th - Remove the DelegateSupport and place IOSession (mina objects) into Connection


For the 1st iteration there is little or no discussions at all. We can just proceed on removing those aspects as this is a just refactoring.


The wiki page will have more details and lets kick the discussion here now. I wanted to reuse what was already discussed on our previous meetings and on forums. Please let me know if you remember of any point previously discussed.

[timfox]

A few initial comments.

The "agnostic" layer doesn't talk directly to MINA - it talks to Jeff's new Dispatcher which in turn talks to an abstracted MINA-like set of interfaces (so they can be mocked) which in turn talks to MINA.

I don't want to see any state or delegates methods remainaing - that's a half way hack. There should just be ConnectionFactory, Connection, Session and Consumer classes in the agnostic layer - all the code from the delegate, state and aspect classes should end up in those classes.

Actually ConnectionFactory, Connection, Session and Consumer should be interfaces, not concrete classes so they can be easily mocked.

I don't think we have need of any interceptors (apart from * see below), ClosedInterceptor can be easily implemented without being an interceptor.

Failover doesn't need interceptors either. We just have a simple valve on Jeff's dispatcher send methods. So no need for a factory or anything else to create any proxies.

The current wiki page describes various intermediate versions. That's fine, but I'm not so interested in the intermediate ones, but really the end goal. I think it's a bit confusing describing intermediate stages.

* - one place where an interceptor will be necessary is to allow an extension point for users to add their own client and server side interceptor(s) - this can probably be a simple java.lang.reflect.Proxy implementation

[clebert.suconic]

I don't want to see any state or delegates methods remainaing - that's a half way hack. There should just be ConnectionFactory, Connection, Session and Consumer classes in the agnostic layer - all the code from the delegate, state and aspect classes should end up in those classes.

that's ok... but looking at the code now (with all the fields we have on states) it seems that would be reasonable to keep separated data for Connection, Session.. etc... like SessionData, ConnectionData and so on... This wouldn't be like the SessionState we have now.. just a separated POJO.

I don't think we have need of any interceptors (apart from * see below), ClosedInterceptor can be easily implemented without being an interceptor.

Failover doesn't need interceptors either. We just have a simple valve on Jeff's dispatcher send methods. So no need for a factory or anything else to create any proxies.

That's exactly what I thought one hour after I sent the post/finished the doc (I didn't remember about the Customer's needs on interceptors tough). I thought I would be able to send another post before you wake up in London (6 hours time difference) but apparently you never sleep :-)

The current wiki page describes various intermediate versions. That's fine, but I'm not so interested in the intermediate ones, but really the end goal. I think it's a bit confusing describing intermediate stages.

I was more concerned about how to get on the final model, than the final model itself.

[timfox]

that's ok... but looking at the code now (with all the fields we have on states) it seems that would be reasonable to keep separated data for Connection, Session.. etc... like SessionData, ConnectionData and so on... This wouldn't be like the SessionState we have now.. just a separated POJO.

We're going for an OO approach - let's just keep it simple and have one object with data and behaviour - standard OO approach.

I was more concerned about how to get on the final model, than the final model itself.

Let's concentrate on the goal first, if you want to add extra pages on how to get there, that's fine, but these aren't central - this is a design document not a roadmap.

[clebert.suconic]

I have updated the WIKI page.

[timfox]

I don't think there's any need for a specific Browser Interface on the agnostic layer - browsing can just be methods on the session.

[clebert.suconic]

Anyone: Please avoid any big refactoring on delegates and aspects until I have merged my code into trunk.

All the aspects have disappeared on my branch, so if you make big changes on any of those places will make me do manual merges. The bigger the change you make on those places, bigger the work I will have to solve those changes.

And talking about my branch... I believe I am on a good shape to merge it now.

This is an intermediate state where I still have States, and am using Proxies in replacement of the interceptors we were using before.

As part of the API is not on interfaces, I had few issues on places we had to convert the Proxy into Delegate again (like, accessing the State object and other things that were not exposed on the interface). But this conversion won't be necessary after the next step on the refactoring, where States are going to disappear.



After I have deleted all the Aspects on trunk, I'm going to remove the State Objects. And I will also verify the possibility of not using Proxies for ClosingInterceptor at all.

[timfox]

Ok, great Clebert.

If you can integrate this with TRUNK, and make sure all tests are passing, can you do this ASAP?

Thx

[clebert.suconic]

I have merged my changes into trunk...

It's better for everyone to perform a "svn update" since this has affected many files.

I have changed the IDEA project, and with few small fixes in your libraries (like ant, and jboss as) you will be able to run testcases from your IDE. (You won't need to use runtest for simple tests after this).

If you use Eclipse, the changes should be simple... You only need to make sure you have the xml config dirs in your classpath. (besides the required jars)