directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Trustin Lee <trus...@gmail.com>
Subject [mina] Refactoring MINA IoFilterChain (Was: IoFilters: DIRMINA-121 / 122)
Date Tue, 15 Nov 2005 02:15:58 GMT
Hi all,

We've talked about refactoring the current IoFilterChain implementation, and
I'd like to address some issues:

1) There should be only *one* NextFilter implementation for all filters in
the same chain for thread safety, and upstream/downstream (forwarding)
operations should be executed in head or tail filter. It can be overriden by
overriding createHeadFilter and createTailFilter. (and this is the same way
Dave suggested to us, but the current implementation hides head and tail
from the filter list to keep OOP encapsulation principal :)

2) We need to carefully consider if we have any other use case than having
concrete number of filter chain. I think we can have only three chains per
session at maximum. So I'm afraid this is an overengineering. Of course we
can provide more than one chain in session level, but I think we don't need
it actually because we can simply append the chain by adding
IoFilterChain.addAllLast() and addAllFirst(), and etc.

3) Should we pass IoHandler as a parameter even if I can get it
IoSession.getHandler()? Please let me know if I am missing something.

4) We cannot simply expose IoFilterChain.setIoProcessor() method because it
can break MINA so easily. So I'd like to suggest hiding setIoProcessor()
like this:

public void setFilterChain( IoFilterChain chain ) {
this.chain = new TransportTypeSpecificFilterChain( chain ); //
TransportTypeSpecificFilterChain extends IoFilterChainProxy.
}

public IoFilterChain getFilterChain() {
return this.chain.getFilterChain(); // Unwrap
}

Only TransportTypeSpecificFilterChain will provide setIoHandler or something
like that. But I think we already have doWrite and doClose in
AbstrctIoFilterChain and there are existing subclasses that implement them.
So I don't see much difference here except the behavior of
get/setFilterChain().

And please note that per-port and per-session filter chain should be cleared
(destroying all filters) when port is unbound or session is closed. This
means they are one-time use only because destroy call on a filter means
removal from a chain and vice versa. So we cannot have
IoSession.setFilterChain() because the specified chain will be cleared when
the connection is closed so it will prevent from specifying the same chain
for more than one sessions.

The biggest problem is that IoFilter.init() and destroy() is invoked
together when IoFilter.add(), remove(), and clear() is invoked. (It's
because the filter can be added or removed at any time.) To do so, IoFilter
has to remember who its parent is (IoSession or IoSessionManager). This
means we have to specify the correct parent when we construct a chain
unfortunately.

So I have to admit that there are so much complexity unfortunately to
provide setFilterChain() simply. Any solutions?

Not let's consider Jose's IoFilterChainBuilder. It is really simple solution
and work without any big changes. The only problem with the builder pattern
is that it can be abused to contain more than just building. A user can even
call System.exit(0) there. :) But it is entirely up to users eventually. So
I'd like to choose Jose's approach. I thought his approach adds more
complexity to the API, but now I think it is a way simpler and easy to
understand.

Cheers,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Mime
View raw message