[mina] IoFilters: DIRMINA-121 / 122

[Irving, Dave]

Hi,

It looks like Im going to need to get DIRMINA-121 completed before I can
tackle DIRMINA-116.
It also looks like 121 is closely related to 122. Is anyone working on
these at the moment?
If not, I don't mind picking them up - I can probably spend a few hours
beginning to look at it tonight after work.

If that's agreed on - has the factory vs builder route been decided on
yet? (The builder approach looks cleaner to me?).

Any comments / preferences / etc appreciated....

Dave



This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.

[Trustin Lee]

Hi,

2005/11/14, Irving, Dave <dave.irving@...>:

It looks like Im going to need to get DIRMINA-121 completed before I can
tackle DIRMINA-116.
It also looks like 121 is closely related to 122. Is anyone working on
these at the moment?
If not, I don't mind picking them up - I can probably spend a few hours
beginning to look at it tonight after work.

Great.  You can resolve in this order: 121, 116, and 122.

If that's agreed on - has the factory vs builder route been decided on
yet? (The builder approach looks cleaner to me?).

Right both approach looks great.  Actually Jose's Builder approach takes less energy to implement.  You can start implementing the builder approach first.  We're working at a unstable stream, so we can change it later.  I hope Niklas is OK with this.

Cheers,
Trustin

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

[Niklas Therning]

Hi Dave,

I have some thoughts on DIRMINA-121. It would be nice if there wouldn't have
to be specialized IoFilterChain implementations for each kind of
Acceptor/Connector. Today we have SocketSessionManagerFilterChain,
DatagramSessionManagerFilterChain, and so on. This makes it hard to combine
arbitrary IoFilterChains into longer chains which is required for
DIRMINA-121.

Here's my suggestion on how to achieve this:

Add a new interface which is the counter part of IoHandler. I don't have a
good name for it, let's call it IoProcessor for now. IoProcessor looks
something like this:

public interface IoProcessor {
    void write( IoSession session, WriteRequest writeRequest );
    void close( IoSession session, CloseFuture closeFuture );
}

Add some methods to the IoFilterChain interface:

void setIoProcessor( IoProcessor p );

void sessionOpened( IoSession session, IoHandler h ) throws Exception;
void sessionClosed( IoSession session, IoHandler h ) throws Exception;
void messageReceived( IoSession session, Object message, IoHandler h )
throws Exception;
... (I think you get the point)

These are exactly the same as the methods in IoHandler except that the extra
IoHandler argument has been added. The idea is that this should be
propagated through the chain (embedded in the NextFilter class). The tail
filter in a chain will always get the IoHandler from the NextFilter and call
it. When combining IoFilterChains A and B, B is the IoHandler of A while the
real IoHandler is the IoHandler of B.

The nice thing about this is that you can combine an arbitrary number of
IoFilterChains. It also makes it possible to have a setFilterChain() method
on SocketAcceptor and other SessionManagers which make these more
DI-friendly. When using setFilterChain() on an IoAcceptor the acceptor will
call setIoProcessor() on the filter chain.

When you call bind(address, chain, handler) the setIoProcessor() method of
the chain will be called to wire it up with the root filter chain owned by
the IoAcceptor/IoConnector.

I don't think it will require any refactoring of existing filters. It will
also eliminate a couple of interface/classes (like the
SessionManagerFilterChains).

Let me know what you think.

/Niklas

-----Original Message-----
From: Irving, Dave [mailto:dave.irving@...]
Sent: den 14 november 2005 14:18
To: Apache Directory Developers List
Subject: [mina] IoFilters: DIRMINA-121 / 122

Hi,

It looks like Im going to need to get DIRMINA-121 completed before I can
tackle DIRMINA-116.
It also looks like 121 is closely related to 122. Is anyone working on
these at the moment?
If not, I don't mind picking them up - I can probably spend a few hours
beginning to look at it tonight after work.

If that's agreed on - has the factory vs builder route been decided on
yet? (The builder approach looks cleaner to me?).

Any comments / preferences / etc appreciated....

Dave



This e-mail and any attachment is for authorised use by the intended
recipient(s) only. It may contain proprietary material, confidential
information and/or be subject to legal privilege. It should not be copied,
disclosed to, retained or used by, any other party. If you are not an
intended recipient then please promptly delete this e-mail and any
attachment and all copies and inform the sender. Thank you.

[Niklas Therning]

I’m totally ok with that! J I totally understand Jose’s approach and why it’s needed. I consider it a different use case. If the filter chain handling is changed the way I suggested in the previous mail I think both approaches will be trivial.

 

/Niklas

 

---

From: Trustin Lee [mailto:trustin@...]
Sent: den 14 november 2005 15:26
To: Apache Directory Developers List
Subject: Re: [mina] IoFilters: DIRMINA-121 / 122

 

Hi,

2005/11/14, Irving, Dave <dave.irving@...>:

It looks like Im going to need to get DIRMINA-121 completed before I can
tackle DIRMINA-116.
It also looks like 121 is closely related to 122. Is anyone working on
these at the moment?
If not, I don't mind picking them up - I can probably spend a few hours
beginning to look at it tonight after work.

Great.  You can resolve in this order: 121, 116, and 122.

 

If that's agreed on - has the factory vs builder route been decided on
yet? (The builder approach looks cleaner to me?).

Right both approach looks great.  Actually Jose's Builder approach takes less energy to implement.  You can start implementing the builder approach first.  We're working at a unstable stream, so we can change it later.  I hope Niklas is OK with this.

Cheers,
Trustin

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

[Irving, Dave]

Hi,

Im just starting to have a look at this now - and I have a couple of
questions.
If you could help me in clearing them up that would be greatly
appreciated:

> Today we have SocketSessionManagerFilterChain,
DatagramSessionManagerFilterChain, and so on.
> This makes it hard to combine arbitrary IoFilterChains into longer
chains which is
> required for DIRMINA-121.

Agreed

> Add a new interface which is the counter part of IoHandler.
> I don't have a good name for it, let's call it IoProcessor for now.
> IoProcessor looks something like this:

> public interface IoProcessor {
>    void write( IoSession session, WriteRequest writeRequest );
>    void close( IoSession session, CloseFuture closeFuture ); }

The way I understand this is that the concrete "do a write / do a close"
behaviour used to be embedded in the concrete filter chain
implementations. E.g: SocketSessionManagerFilterChain knows how to do a
"real" write / close for a socket session. What we are doing here is
extracting this behaviour in to a sort of "strategy" interface which we
can plug in to any chain. Correct?

> Add some methods to the IoFilterChain interface:

> void setIoProcessor( IoProcessor p );

Makes sense if Im correct above. I.e: plug in the "processor" strategy
for the chain.

> void sessionOpened( IoSession session, IoHandler h ) throws Exception;

> void sessionClosed( IoSession session, IoHandler h ) throws Exception;

> void messageReceived( IoSession session, Object message, IoHandler h )
throws Exception; ...
> (I think you get the point)

Replacing the sessionOpened(IoSession) / sessionClosed(IoSession) et al
methods in AbstractFilterChain?

> These are exactly the same as the methods in IoHandler except that the
extra IoHandler argument has been added. > > The idea is that this
should be propagated through the chain (embedded in the NextFilter
class)

This is where my real question comes in: You say later that we shouldn't
need to refactor existing filter implementations.
How are we going to pass the handler through transparently without
changing the IoFilter / NextFilter interface?
I think this is the bit Im missing at the moment :o)

> When you call bind(address, chain, handler) the setIoProcessor()
method of the chain will be
> called to wire it up with the root filter chain owned by the
IoAcceptor/IoConnector.

Yep - plugging in its own strategy to the chain. Sounds good.

> I don't think it will require any refactoring of existing filters.

See question above

> It will also eliminate a couple of interface/classes (like the
SessionManagerFilterChains).

Cool! I love removing classes :o)

Cheers for your help,

Dave



This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.

[Niklas Therning]

Irving, Dave wrote:

>Hi,
>
>Im just starting to have a look at this now - and I have a couple of
>questions.
>If you could help me in clearing them up that would be greatly
>appreciated:
>
>  
>
>>Today we have SocketSessionManagerFilterChain,
>>    
>>
>DatagramSessionManagerFilterChain, and so on.
>  
>
>>This makes it hard to combine arbitrary IoFilterChains into longer
>>    
>>
>chains which is
>  
>
>>required for DIRMINA-121.
>>    
>>
>
>Agreed
>
>  
>
>>Add a new interface which is the counter part of IoHandler.
>>I don't have a good name for it, let's call it IoProcessor for now.
>>IoProcessor looks something like this:
>>    
>>
>
>  
>
>>public interface IoProcessor {
>>   void write( IoSession session, WriteRequest writeRequest );
>>   void close( IoSession session, CloseFuture closeFuture ); }
>>    
>>
>
>The way I understand this is that the concrete "do a write / do a close"
>behaviour used to be embedded in the concrete filter chain
>implementations. E.g: SocketSessionManagerFilterChain knows how to do a
>"real" write / close for a socket session. What we are doing here is
>extracting this behaviour in to a sort of "strategy" interface which we
>can plug in to any chain. Correct?
>
>  
>

Absolutely correct!

>>Add some methods to the IoFilterChain interface:
>>    
>>
>
>  
>
>>void setIoProcessor( IoProcessor p );
>>    
>>
>
>Makes sense if Im correct above. I.e: plug in the "processor" strategy
>for the chain.
>
>  
>
>>void sessionOpened( IoSession session, IoHandler h ) throws Exception;
>>    
>>
>
>  
>
>>void sessionClosed( IoSession session, IoHandler h ) throws Exception;
>>    
>>
>
>  
>
>>void messageReceived( IoSession session, Object message, IoHandler h )
>>    
>>
>throws Exception; ...
>  
>
>>(I think you get the point)
>>    
>>
>
>Replacing the sessionOpened(IoSession) / sessionClosed(IoSession) et al
>methods in AbstractFilterChain?
>  
>

Yes.

>  
>
>>These are exactly the same as the methods in IoHandler except that the
>>    
>>
>extra IoHandler argument has been added. > > The idea is that this
>should be propagated through the chain (embedded in the NextFilter
>class)
>
>This is where my real question comes in: You say later that we shouldn't
>need to refactor existing filter implementations.
>How are we going to pass the handler through transparently without
>changing the IoFilter / NextFilter interface?
>I think this is the bit Im missing at the moment :o)
>
>  
>

And that's where things get really tricky! ;-)    Actually when I have a
look at the code now I see that the concrete NextFilter instances are
created only once and then used all the time. If my idea is going to
work the NextFilter objects created by AbstractIoFilterChain will have
to be created on each filter invocation. We need a new NextFilter
implementation. Here's an example:

    public static class UpstreamNextFilter implements NextFilter {
        Entry head;
        IoHandler handler;
        public UpstreamNextFilter(Entry head, IoHandler handler) {
            this.head = head;
            this.handler = handler;
        }
        public void sessionCreated( IoSession session )
        {
            if (head != null) {
                head.filter.sessionCreated(new
UpstreamNextFilter(head.nextEntry, handler), session);
            } else {
                handler.sessionCreated(session);
            }
        }
        public void sessionOpened( IoSession session )
        {
            if (head != null) {
                head.filter.sessionOpened(new
UpstreamNextFilter(head.nextEntry, handler), session);
            } else {
                handler.sessionOpened(session);
            }
        }
        public void sessionClosed( IoSession session )
        {
            if (head != null) {
                head.filter.sessionClosed(new
UpstreamNextFilter(head.nextEntry, handler), session);
            } else {
                handler.sessionClosed(session);
            }
        }
        ... (I think you get the point)

In AbstractIoFilterChain sessionCreated() would look something like:

    public void sessionCreated( IoSession session, IoHandler handler )
    {
        Entry head = this.head;
        new UpstreamNextFilter(head, handler).sessionCreated(session);
    }

There would also be a corresponding DownstreamNextFilter implementation
(perhaps Upstream and Downstream can be combined into one) which takes
an IoProcessor in its constructor instead of an IoHandler.

One thing that strikes me now is that the NextFilter interface is almost
the union of IoHandler and IoProcessor. Except that the methods of
NextFilter aren't allowed to throw Exception. Maybe that fact could be
used to simplify the API?

I hope this made any sense. It works in my head but that doesn't have to
mean anything! ;-)

/Niklas

[Irving, Dave]

Hi Niklas,

Thanks for your reply. I just wanted to make sure we were on the same
wavelength before I started thrashing out code :o)

I ** was ** thinking earlier that the only way we'd be able to get the
required propagation (cleanly) would be to have a new "next handler" per
chain traversal - so Im glad I didn't miss anything silly :o)
Makes much more sense now.

I think there's just one more thing to discuss / clear up about the
approach.
At first I was wondering why you wanted to pass the handler through at
all (IoSession already has a "getHandler" method).
However, this seemed to hint at the reason:

> The tail filter in a chain will always get the IoHandler from the
NextFilter and call it.
> When combining IoFilterChains A and B, B is the IoHandler of A while
the real
> IoHandler is the IoHandler of B.

Is the reason to enable a chain to masquerade as a handler, and have
transparent chain "chaining" ?).
If so, Im wondering if maybe there is a simpler way to "chain
chains"....? (E.g, if we're jumping through hoops to chain them, maybe
the "special" head and tail filters don't really belong in the chain
(per se) at all...?
That way, you'd just have something like
IoFilterChain#chain(IoFilterChain next). Any special meaning to the tail
operations would be at a higher level.
(Again, clear in my head - but that doesn't mean anything :o)

I think that's the only area that I'd like to discuss further before
knocking out the code.

Thanks again for your reply - its been really useful,

Dave


This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.

[Niklas Therning]

Irving, Dave wrote:

>Hi Niklas,
>
>Thanks for your reply. I just wanted to make sure we were on the same
>wavelength before I started thrashing out code :o)
>
>I ** was ** thinking earlier that the only way we'd be able to get the
>required propagation (cleanly) would be to have a new "next handler" per
>chain traversal - so Im glad I didn't miss anything silly :o)
>Makes much more sense now.
>
>I think there's just one more thing to discuss / clear up about the
>approach.
>At first I was wondering why you wanted to pass the handler through at
>all (IoSession already has a "getHandler" method).
>However, this seemed to hint at the reason:
>
>  
>
>>The tail filter in a chain will always get the IoHandler from the
>>    
>>
>NextFilter and call it.
>  
>
>>When combining IoFilterChains A and B, B is the IoHandler of A while
>>    
>>
>the real
>  
>
>>IoHandler is the IoHandler of B.
>>    
>>
>
>Is the reason to enable a chain to masquerade as a handler, and have
>transparent chain "chaining" ?).
>  
>

Yes that's the reason. But I'm not sure exactly what happens when the
last filter of a filter chain has been executed and control should be
transferred to a new chain. The thing is that the code starting the
filtering through a chain also has to know what should happen after the
chain has completed and create/use the appropriate handler.

One possiblity would be to create chains of chains by wrapping them two
by two. Let's pretend we have this special IoFilterChain implementation
which takes two IoFilterChains (X, Y) and filters through them in order.
We can use this implementation to combine an arbitrary number of chains,
(((A, B), C), D). Oh, how I love recursion! :)

>If so, Im wondering if maybe there is a simpler way to "chain
>chains"....? (E.g, if we're jumping through hoops to chain them, maybe
>the "special" head and tail filters don't really belong in the chain
>(per se) at all...?
>  
>
The head and tail should not be needed anymore. The
Upstream/DownstreamNextFilters will know what to do when there are no
more filters.

>That way, you'd just have something like
>IoFilterChain#chain(IoFilterChain next). Any special meaning to the tail
>operations would be at a higher level.
>(Again, clear in my head - but that doesn't mean anything :o)
>  
>
That doesn't make sense in my head! :) Though I am a bit sleepy at the
moment. Please elaborate.

/Niklas

[Irving, Dave]

Niklas Therning wrote:

> Yes that's the reason. But I'm not sure exactly what happens when
> the last filter of a filter chain has been executed and control should

> be transferred to a new chain. The thing is that the code starting the

> filtering through a chain also has to know what should happen after
the
> chain has completed and create/use the appropriate handler.

> One possiblity would be to create chains of chains by wrapping them
two by two.
> Let's pretend we have this special IoFilterChain implementation which
takes
> two IoFilterChains (X, Y) and filters through them in order.
>We can use this implementation to combine an arbitrary number of
> chains, (((A, B), C), D). Oh, how I love recursion! :)

This is what I was getting at in the last bit of my post (sorry - I read
it back and it didn't make much sense to me either - really shouldn't do
these 14 hour days :o)

I was thinking that chaining chains directly (as you describe above)
might be cleaner than having a chain masquerade as a handler.

So Im thinking that maybe the "ChainedFilterChain" or what-ever might be
a good way.


> The head and tail should not be needed anymore.
> The Upstream/DownstreamNextFilters will know what to do when there are
no more filters.

As I understand it though, they will currently either delegate to the
IoHandler or to the IoProcessor.
And I think that's why you passed the handler through (so the next chain
can masquerade as a handler).
But I think we're maybe agreeing that a "ChainedFilterChain" is maybe a
cleaner approach (presumably to chain the per acceptor / connector chain
with the per session chain?).
So, given that:

>> That way, you'd just have something like
>> IoFilterChain#chain(IoFilterChain next). Any special meaning to the
>> tail operations would be at a higher level.
>> (Again, clear in my head - but that doesn't mean anything :o)
>>

> That doesn't make sense in my head! :) Though I am a bit sleepy at the
moment. Please elaborate.

That's my dumb way of saying that Im wondering whether the chain
implementation ** itself ** should even know or care about the "final
tasks" (i.e: Performing operations on the IoHandler or the IoProcessor).
If the chain just invokes filters until there are no more filters, then
the chain interface (and implementation) is pretty simple - and it
becomes very easy to "chain chains" using the "ChainedFilterChain" I
alluded to and you described.

Then there are endless possibilities to how filters and chains could be
attached together.
For example, you could even have IoFilterChain implement (directly, or
via adaptor) IoFilter.
Then you might have something real simple like this:

--------------------------------------------------
BasicFilterChain chain = ... // some basic impl - like
AbstractFilterChain is now
IoFilter sessionManagerFilter = ... // really a filter chain!
IoFilter sessionFilter = .... // really a filter chain!
chain.addLast(sessionManagerFilter);
chain.addLast(sessionFilter);

// Dummy name: This is the filter that delegates to IoHandler,
IoProcessor
EndOfTheLineFilter endOfTheLineFilter = ...

chain.addLast(endOfTheLineFilter);
--------------------------------------------------

This should also fit in with the DI stuff too.
I hope this makes a little more sense. But if it doesn't - I can
certainly discuss further when Im more awake :o)
(Been working since 7am, and its 10pm now - so Im more than a little
drained :o)

Any comments would be greatly appreciated....

> /Niklas

Regards,

Dave







This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.

[Niklas Therning]

Irving, Dave wrote:

>Niklas Therning wrote:
>
>  
>
>>Yes that's the reason. But I'm not sure exactly what happens when
>>the last filter of a filter chain has been executed and control should
>>    
>>
>
>  
>
>>be transferred to a new chain. The thing is that the code starting the
>>    
>>
>
>  
>
>>filtering through a chain also has to know what should happen after
>>    
>>
>the
>  
>
>>chain has completed and create/use the appropriate handler.
>>    
>>
>
>  
>
>>One possiblity would be to create chains of chains by wrapping them
>>    
>>
>two by two.
>  
>
>>Let's pretend we have this special IoFilterChain implementation which
>>    
>>
>takes
>  
>
>>two IoFilterChains (X, Y) and filters through them in order.
>>We can use this implementation to combine an arbitrary number of
>>chains, (((A, B), C), D). Oh, how I love recursion! :)
>>    
>>
>
>This is what I was getting at in the last bit of my post (sorry - I read
>it back and it didn't make much sense to me either - really shouldn't do
>these 14 hour days :o)
>
>I was thinking that chaining chains directly (as you describe above)
>might be cleaner than having a chain masquerade as a handler.
>
>So Im thinking that maybe the "ChainedFilterChain" or what-ever might be
>a good way.
>
>
>  
>
>>The head and tail should not be needed anymore.
>>The Upstream/DownstreamNextFilters will know what to do when there are
>>    
>>
>no more filters.
>
>As I understand it though, they will currently either delegate to the
>IoHandler or to the IoProcessor.
>And I think that's why you passed the handler through (so the next chain
>can masquerade as a handler).
>But I think we're maybe agreeing that a "ChainedFilterChain" is maybe a
>cleaner approach (presumably to chain the per acceptor / connector chain
>with the per session chain?).
>So, given that:
>
>  
>
>>>That way, you'd just have something like
>>>IoFilterChain#chain(IoFilterChain next). Any special meaning to the
>>>tail operations would be at a higher level.
>>>(Again, clear in my head - but that doesn't mean anything :o)
>>>
>>>      
>>>
>
>  
>
>>That doesn't make sense in my head! :) Though I am a bit sleepy at the
>>    
>>
>moment. Please elaborate.
>
>That's my dumb way of saying that Im wondering whether the chain
>implementation ** itself ** should even know or care about the "final
>tasks" (i.e: Performing operations on the IoHandler or the IoProcessor).
>If the chain just invokes filters until there are no more filters, then
>the chain interface (and implementation) is pretty simple - and it
>becomes very easy to "chain chains" using the "ChainedFilterChain" I
>alluded to and you described.
>
>Then there are endless possibilities to how filters and chains could be
>attached together.
>For example, you could even have IoFilterChain implement (directly, or
>via adaptor) IoFilter.
>Then you might have something real simple like this:
>
>--------------------------------------------------
>BasicFilterChain chain = ... // some basic impl - like
>AbstractFilterChain is now
>IoFilter sessionManagerFilter = ... // really a filter chain!
>IoFilter sessionFilter = .... // really a filter chain!
>chain.addLast(sessionManagerFilter);
>chain.addLast(sessionFilter);
>
>// Dummy name: This is the filter that delegates to IoHandler,
>IoProcessor
>EndOfTheLineFilter endOfTheLineFilter = ...
>
>chain.addLast(endOfTheLineFilter);
>--------------------------------------------------
>
>This should also fit in with the DI stuff too.
>I hope this makes a little more sense. But if it doesn't - I can
>certainly discuss further when Im more awake :o)
>(Been working since 7am, and its 10pm now - so Im more than a little
>drained :o)
>
>Any comments would be greatly appreciated....
>
>  
>

I really like your way of thinking! I will have to get back to you on
this tomorrow when my brain is functioning properly again. At the
moment, all I can say is that all this looks very nice to me.

/Niklas

[Trustin Lee]

Hi all,

Wow, there was a lot of conversation. :)

2005/11/15, Irving, Dave <dave.irving@...>:

> Add a new interface which is the counter part of IoHandler.
> I don't have a good name for it, let's call it IoProcessor for now.
> IoProcessor looks something like this:

> public interface IoProcessor {
>    void write( IoSession session, WriteRequest writeRequest );
>    void close( IoSession session, CloseFuture closeFuture ); }

> Add some methods to the IoFilterChain interface:

> void setIoProcessor( IoProcessor p );

This breaks OOP abstraction principal.  What if a user calls setIoProcessor innocently?  It will break the whole MINA application.  We shouldn't expose this method in IoFilterChain.  We have to use some internal class.  So I suggest you to mosify IoSessionManagerFilterChain to provide an IoProcessor as a constructor parameter.  This is the safer way.

Cheers,
Trustin

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

[Trustin Lee]

Hi Niklas,

2005/11/14, Niklas Therning <niklas@...>:

void sessionOpened( IoSession session, IoHandler h ) throws Exception;
void sessionClosed( IoSession session, IoHandler h ) throws Exception;
void messageReceived( IoSession session, Object message, IoHandler h )
throws Exception;
... (I think you get the point)

These are exactly the same as the methods in IoHandler except that the extra
IoHandler argument has been added. The idea is that this should be
propagated through the chain (embedded in the NextFilter class). The tail
filter in a chain will always get the IoHandler from the NextFilter and call
it. When combining IoFilterChains A and B, B is the IoHandler of A while the
real IoHandler is the IoHandler of B.

The nice thing about this is that you can combine an arbitrary number of
IoFilterChains.

This is really interesting.  But wouldn't there be any confusion with IoSession.getHandler() method? 

It also makes it possible to have a setFilterChain() method
on SocketAcceptor and other SessionManagers which make these more
DI-friendly. When using setFilterChain() on an IoAcceptor the acceptor will
call setIoProcessor() on the filter chain.

When you call bind(address, chain, handler) the setIoProcessor() method of
the chain will be called to wire it up with the root filter chain owned by
the IoAcceptor/IoConnector.

I'd rather wrap the IoFilterChain user provided and provide getIoProcessor in the wrapper to make it look more like OOP.  I'm good with setFilterChain() BTW.

Let me know what you think.

The question is how many filter chains we need per session.  Do we need three (IoSessionManager, port, and IoSession) now?  Wouldn't it be too complex?  Actually I was thinking only 2 (port and IoSession).  What is the advantage of IoSessionManager-level filters? 

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

[Trustin Lee]

2005/11/15, Niklas Therning <niklas@...>:

And that's where things get really tricky! ;-)    Actually when I have a
look at the code now I see that the concrete NextFilter instances are
created only once and then used all the time. If my idea is going to
work the NextFilter objects created by AbstractIoFilterChain will have
to be created on each filter invocation. We need a new NextFilter
implementation. Here's an example:

    public static class UpstreamNextFilter implements NextFilter {
        Entry head;
        IoHandler handler;
        public UpstreamNextFilter(Entry head, IoHandler handler) {
            this.head = head;
            this.handler = handler;
        }
        public void sessionCreated( IoSession session )
        {
            if (head != null) {
                head.filter.sessionCreated(new
UpstreamNextFilter(head.nextEntry, handler), session);
            } else {
                handler.sessionCreated (session);
            }
        }
        public void sessionOpened( IoSession session )
        {
            if (head != null) {
                head.filter.sessionOpened(new
UpstreamNextFilter(head.nextEntry , handler), session);
            } else {
                handler.sessionOpened(session);
            }
        }
        public void sessionClosed( IoSession session )
        {
            if (head != null) {
                head.filter.sessionClosed(new
UpstreamNextFilter(head.nextEntry, handler), session);
            } else {
                handler.sessionClosed(session);
            }
        }
        ... (I think you get the point)

In AbstractIoFilterChain sessionCreated() would look something like:

    public void sessionCreated( IoSession session, IoHandler handler )
    {
        Entry head = this.head;
        new UpstreamNextFilter(head, handler).sessionCreated(session);
    }

There would also be a corresponding DownstreamNextFilter implementation
(perhaps Upstream and Downstream can be combined into one) which takes
an IoProcessor in its constructor instead of an IoHandler.

One thing that strikes me now is that the NextFilter interface is almost
the union of IoHandler and IoProcessor. Except that the methods of
NextFilter aren't allowed to throw Exception. Maybe that fact could be
used to simplify the API?

Hmm.. you're right.  But I think it is essentially same with what we're doing in AbstractIoFilterChain.  It provides a default implementation of head/tail filter and you can override it by overriding createHeadFilter and createTailFilter.

Providing various nextfilter implementation will drive you into some trouble because the order of the filters can change at any time.  So you'll have to switch the nextFilter property of Entry and it might not be thread-safe.  That's why I used head/tail filter instead, and you can still override the implementation. :)

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

[Jose Alberto Fernandez-2]

Recreating the chains on every invocation sounds like a very expensive
price to pay on every call. What is exactly the benefit we will be
getting out of it?

> -----Original Message-----
> From: Niklas Therning [mailto:niklas@...]
> Sent: 14 November 2005 20:42
> To: Apache Directory Developers List
> Subject: Re: [mina] IoFilters: DIRMINA-121 / 122
>
> Irving, Dave wrote:
>
> >Hi,
> >
> >Im just starting to have a look at this now - and I have a couple of
> >questions.
> >If you could help me in clearing them up that would be greatly
> >appreciated:
> >
> >
> >
> >>Today we have SocketSessionManagerFilterChain,
> >>
> >>
> >DatagramSessionManagerFilterChain, and so on.
> >
> >
> >>This makes it hard to combine arbitrary IoFilterChains into longer
> >>
> >>
> >chains which is
> >
> >
> >>required for DIRMINA-121.
> >>
> >>
> >
> >Agreed
> >
> >
> >
> >>Add a new interface which is the counter part of IoHandler.
> >>I don't have a good name for it, let's call it IoProcessor for now.
> >>IoProcessor looks something like this:
> >>
> >>
> >
> >
> >
> >>public interface IoProcessor {
> >>   void write( IoSession session, WriteRequest writeRequest );
> >>   void close( IoSession session, CloseFuture closeFuture ); }
> >>
> >>
> >
> >The way I understand this is that the concrete "do a write / do a

close"
> >behaviour used to be embedded in the concrete filter chain
> >implementations. E.g: SocketSessionManagerFilterChain knows how to do
a
> >"real" write / close for a socket session. What we are doing here is
> >extracting this behaviour in to a sort of "strategy" interface which
we

> >can plug in to any chain. Correct?
> >
> >
> >
> Absolutely correct!
>
> >>Add some methods to the IoFilterChain interface:
> >>
> >>
> >
> >
> >
> >>void setIoProcessor( IoProcessor p );
> >>
> >>
> >
> >Makes sense if Im correct above. I.e: plug in the "processor"

strategy
> >for the chain.
> >
> >
> >
> >>void sessionOpened( IoSession session, IoHandler h ) throws
Exception;
> >>
> >>
> >
> >
> >
> >>void sessionClosed( IoSession session, IoHandler h ) throws
Exception;
> >>
> >>
> >
> >
> >
> >>void messageReceived( IoSession session, Object message, IoHandler h
)

> >>
> >>
> >throws Exception; ...
> >
> >
> >>(I think you get the point)
> >>
> >>
> >
> >Replacing the sessionOpened(IoSession) / sessionClosed(IoSession) et

al
> >methods in AbstractFilterChain?
> >
> >
> Yes.
>
> >
> >
> >>These are exactly the same as the methods in IoHandler except that
the
> >>
> >>
> >extra IoHandler argument has been added. > > The idea is that this
> >should be propagated through the chain (embedded in the NextFilter
> >class)
> >
> >This is where my real question comes in: You say later that we
shouldn't
> >need to refactor existing filter implementations.
> >How are we going to pass the handler through transparently without
> >changing the IoFilter / NextFilter interface?
> >I think this is the bit Im missing at the moment :o)
> >
> >
> >
> And that's where things get really tricky! ;-)    Actually when I have
a

> look at the code now I see that the concrete NextFilter instances are
> created only once and then used all the time. If my idea is going to
> work the NextFilter objects created by AbstractIoFilterChain will have
> to be created on each filter invocation. We need a new NextFilter
> implementation. Here's an example:
>
>     public static class UpstreamNextFilter implements NextFilter {
>         Entry head;
>         IoHandler handler;
>         public UpstreamNextFilter(Entry head, IoHandler handler) {
>             this.head = head;
>             this.handler = handler;
>         }
>         public void sessionCreated( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionCreated(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionCreated(session);
>             }
>         }
>         public void sessionOpened( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionOpened(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionOpened(session);
>             }
>         }
>         public void sessionClosed( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionClosed(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionClosed(session);
>             }
>         }
>         ... (I think you get the point)
>
> In AbstractIoFilterChain sessionCreated() would look something like:
>
>     public void sessionCreated( IoSession session, IoHandler handler )
>     {
>         Entry head = this.head;
>         new UpstreamNextFilter(head, handler).sessionCreated(session);
>     }
>
> There would also be a corresponding DownstreamNextFilter

implementation
> (perhaps Upstream and Downstream can be combined into one) which takes
> an IoProcessor in its constructor instead of an IoHandler.
>
> One thing that strikes me now is that the NextFilter interface is
almost
> the union of IoHandler and IoProcessor. Except that the methods of
> NextFilter aren't allowed to throw Exception. Maybe that fact could be
> used to simplify the API?
>
> I hope this made any sense. It works in my head but that doesn't have
to
> mean anything! ;-)
>
> /Niklas

[Irving, Dave]

> Recreating the chains on every invocation sounds like a very expensive
price to pay on every call. What is exactly the benefit we will be
getting out of it?

I don't think we will be recreating the chains on every invocation.
It'll go like this:

1) User gets to configure a per session manager chain. Created once per
session manager (if used) using builder approach
2) User gets to configure a per port chain. Created once per port (if
used) using builder approach
3) User gets to configure a per session (connection) chain. Created once
per session (if used) using builder approach

When a new session is created, the chains (1) (2) (3) which apply to the
connection are tied together in to a "ConnectionChain" (behind the
scnenes - not visible to the user) by the mechanisms discussed in my
recent posts. Individual filters are not cloned or recreated to achieve
this.
The user never sees the ConnectionChain.

Each filter call for a connection is simply a call in to the connection
chain.

Does this address your concerns?

Thanks,

Dave
 

-----Original Message-----
From: Jose Alberto Fernandez [mailto:jalberto@...]
Sent: 15 November 2005 12:50
To: Apache Directory Developers List
Subject: RE: [mina] IoFilters: DIRMINA-121 / 122

Recreating the chains on every invocation sounds like a very expensive
price to pay on every call. What is exactly the benefit we will be
getting out of it?

> -----Original Message-----
> From: Niklas Therning [mailto:niklas@...]
> Sent: 14 November 2005 20:42
> To: Apache Directory Developers List
> Subject: Re: [mina] IoFilters: DIRMINA-121 / 122
>
> Irving, Dave wrote:
>
> >Hi,
> >
> >Im just starting to have a look at this now - and I have a couple of
> >questions.
> >If you could help me in clearing them up that would be greatly
> >appreciated:
> >
> >
> >
> >>Today we have SocketSessionManagerFilterChain,
> >>
> >>
> >DatagramSessionManagerFilterChain, and so on.
> >
> >
> >>This makes it hard to combine arbitrary IoFilterChains into longer
> >>
> >>
> >chains which is
> >
> >
> >>required for DIRMINA-121.
> >>
> >>
> >
> >Agreed
> >
> >
> >
> >>Add a new interface which is the counter part of IoHandler.
> >>I don't have a good name for it, let's call it IoProcessor for now.
> >>IoProcessor looks something like this:
> >>
> >>
> >
> >
> >
> >>public interface IoProcessor {
> >>   void write( IoSession session, WriteRequest writeRequest );
> >>   void close( IoSession session, CloseFuture closeFuture ); }
> >>
> >>
> >
> >The way I understand this is that the concrete "do a write / do a

close"
> >behaviour used to be embedded in the concrete filter chain
> >implementations. E.g: SocketSessionManagerFilterChain knows how to do
a
> >"real" write / close for a socket session. What we are doing here is
> >extracting this behaviour in to a sort of "strategy" interface which
we

> >can plug in to any chain. Correct?
> >
> >
> >
> Absolutely correct!
>
> >>Add some methods to the IoFilterChain interface:
> >>
> >>
> >
> >
> >
> >>void setIoProcessor( IoProcessor p );
> >>
> >>
> >
> >Makes sense if Im correct above. I.e: plug in the "processor"

strategy
> >for the chain.
> >
> >
> >
> >>void sessionOpened( IoSession session, IoHandler h ) throws
Exception;
> >>
> >>
> >
> >
> >
> >>void sessionClosed( IoSession session, IoHandler h ) throws
Exception;
> >>
> >>
> >
> >
> >
> >>void messageReceived( IoSession session, Object message, IoHandler h
)

> >>
> >>
> >throws Exception; ...
> >
> >
> >>(I think you get the point)
> >>
> >>
> >
> >Replacing the sessionOpened(IoSession) / sessionClosed(IoSession) et

al
> >methods in AbstractFilterChain?
> >
> >
> Yes.
>
> >
> >
> >>These are exactly the same as the methods in IoHandler except that
the
> >>
> >>
> >extra IoHandler argument has been added. > > The idea is that this
> >should be propagated through the chain (embedded in the NextFilter
> >class)
> >
> >This is where my real question comes in: You say later that we
shouldn't
> >need to refactor existing filter implementations.
> >How are we going to pass the handler through transparently without
> >changing the IoFilter / NextFilter interface?
> >I think this is the bit Im missing at the moment :o)
> >
> >
> >
> And that's where things get really tricky! ;-)    Actually when I have
a
> look at the code now I see that the concrete NextFilter instances are
> created only once and then used all the time. If my idea is going to
> work the NextFilter objects created by AbstractIoFilterChain will have

> to be created on each filter invocation. We need a new NextFilter
> implementation. Here's an example:
>
>     public static class UpstreamNextFilter implements NextFilter {
>         Entry head;
>         IoHandler handler;
>         public UpstreamNextFilter(Entry head, IoHandler handler) {
>             this.head = head;
>             this.handler = handler;
>         }
>         public void sessionCreated( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionCreated(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionCreated(session);
>             }
>         }
>         public void sessionOpened( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionOpened(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionOpened(session);
>             }
>         }
>         public void sessionClosed( IoSession session )
>         {
>             if (head != null) {
>                 head.filter.sessionClosed(new
> UpstreamNextFilter(head.nextEntry, handler), session);
>             } else {
>                 handler.sessionClosed(session);
>             }
>         }
>         ... (I think you get the point)
>
> In AbstractIoFilterChain sessionCreated() would look something like:
>
>     public void sessionCreated( IoSession session, IoHandler handler )
>     {
>         Entry head = this.head;
>         new UpstreamNextFilter(head, handler).sessionCreated(session);
>     }
>
> There would also be a corresponding DownstreamNextFilter

implementation
> (perhaps Upstream and Downstream can be combined into one) which takes

> an IoProcessor in its constructor instead of an IoHandler.
>
> One thing that strikes me now is that the NextFilter interface is
almost
> the union of IoHandler and IoProcessor. Except that the methods of
> NextFilter aren't allowed to throw Exception. Maybe that fact could be

> used to simplify the API?
>
> I hope this made any sense. It works in my head but that doesn't have
to
> mean anything! ;-)
>
> /Niklas



This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender. Thank you.