Get rid of complexity of the current IoFilterChain.

• Key: DIRMINA-131
• Type: Improvement
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Trustin Lee
• Reporter: Trustin Lee
• Affects Version/s: 0.8.0
• Fix Version/s: 0.9.0

Description

This issue is the summary of these threads in the Apache Directory Project mailing list:

http://www.nabble.com/-mina-Refactoring-MINA-IoFilterChain-%28Was%3A-IoFilters%3A-DIRMINA-121-122%29-t553121.html
http://www.nabble.com/-mina-IoFilters%3A-DIRMINA-121-122-t548297.html

Currently, IoFilterChains are categorized into two; one is IoSessionManagerFilterChain, and the other is IoSessionFilterChain. IoSessionManagerFilterChain is shared by all sessions managed by the same IoSessionManager, and IoSessionFilterChain is individual for each session. But this design made the internal architecture of MINA filter chain very complex comparing its usefulness.

What about just getting rid of the original IoSessionManagerFilterChain, and provide a simple data structure which just stores a list of filters but still implements IoFilter interface? By doing so, we can simply copy the chain into the IoSessionFilterChain before we start the communication, so the chain implementation gets simplified dramatically.

Besides that, we have another option instead of using a simple data structure that implements IoFilterChain. It is called 'IoFilterChainBuilder'. It is a kind of command pattern which configures the filter chain instead of simply appending the filter list the chain. It provides great flexibility. For example, you can override the settings of per-manager filter chain in your per-port (or per-session) chain builder.

I think it will be best if we can combine these two approach by providing an IoFilterChainBuilder implementation which simply appends the specified filter list to the per-session chain.

Comments

[Trustin Lee]

Oops, I was working on this issue. Niklas, I hope you didn't work too much on this issue (and the related ones: DIRMINA-121, DIRMINA-122) I checked in my works into branches/chain_refactoring

Please review this diff log:

http://svn.apache.org/viewcvs?rev=349659&view=rev

The changes are not complete, but I checked in anyway after I saw you assign this issue to yourself. It will work fine only if we clear the compilation errors. What we need to do is to provide a few utility classes which helps users to create a IoFilterChainBuilder easily.

Please let me know what you think.

[Niklas Therning]

No, I haven't started to work on this issue yet. I've had a quick look at the diff and it looks very good. Just let me know when you're finished and I can take care of the Spring stuff before the merge with trunk.

Great. Then I'll try to fix most compilation errors tonight if I can. I'll be back likely in 5 hrs later. Could you review again in case you missed something if was just a quick look? ;) And I'll appreciate any idea on the utility classes if you have any.

[Niklas Therning]

Reassign to Trustin since he's already working on this issue.

[Niklas Therning]

About the utility classes. Why not have a DefaultIoFilterChainBuilder which has all the add/remove methods IoFilterChain has. Internally it will keep a list of IoFilters. In DefaultIoFilterChainBuilder.build(chain) the filters in the internal list would simply be added last to the chain being built.

The SimpleServiceRegistry would use DefaultIoFilterChainBuilder which would make it quite simple to migrate old code:

reg.getAcceptor( TransportType.SOCKET ).getFilterChain().addLast( "foo", fooFilter )

would become

( ( DefaultIoFilterChainBuilder ) reg.getAcceptor( TransportType.SOCKET ).getFilterChainBuilder() ).addLast( "foo", fooFilter )

Instead of the NOOP chain builder the concrete SessionManagers which come with MINA could use DefaultIoFilterChainBuilder by default.

[Trustin Lee]

All changes have been made as we specified here.