mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject Re: onPreAdd, PostAdd, remove : correction no---signature
Date Mon, 10 Nov 2008 09:34:16 GMT
Steve Ulrich wrote:
>> Emmanuel Lecharny [mailto:elecharny@gmail.com] wrote
>> Hi,
> Hi!
> I have already used them for some session specific initialization.
Makes sense.  Seems that pre-add and post-remove can be useful, as you 
say later in your reply.

>> but SslFilter does some intrigating things, which seems to me deserves
>> to be found in a init() method.
> An init() method doesn't know anything about the session so far.
I was specifically thinking if a init(session) method. We also need a 
destroy(session) method. Eh, call them preAdd and postRemove ;)

Seriously, you make a good point here, and enlighted the reason we have 
these methods. And after a day looking at those preAdd and postRemove 
(well, not a full day !), I think it's better to have those two guys 
called directly when you do a add() and remove() than to have an 
explicit init() method, generally speaking.  But there is another problem :

If you look at the existing javadoc :

     * Invoked by {@link ReferenceCountingFilter} when this filter
     * is added to a {@link IoFilterChain} at the first time, so you can
     * initialize shared resources.  Please note that this method is never
     * called if you don't wrap a filter with {@link 
    void init() throws Exception;

     * Invoked before this filter is added to the specified <tt>parent</tt>.
     * Please note that this method can be invoked more than once if
     * this filter is added to more than one parents.  This method is not
     * invoked before {@link #init()} is invoked.
    void onPreAdd(IoFilterChain parent, String name, NextFilter nextFilter)
            throws Exception;

You can see that the init is used in a very specific case : when you 
wrap the filter with ReferenceCountingFilter (seems like an abusive 
usage to me), and that the onPreAdd may be called more than once, which 
seems also abusive when you know that we will only have one instance of 
this filter, leading potentially to many mistakes if the initialization 
done during this phase is done more than once.
so : I think that we need to rethink about what those two methods do, in 
order to protect the user against dangerous hidden mistakes...

I'm going to propose something slightly better for those methods :
- remove the postAdd and preRemove methods
- rename the preAdd/postRemove to init(session) and destroy(session), 
and call them explicitly _before_ we remove or add this filter from the 
session chain
- if we don't need those initialization/destruction, then default to a 
flag being set indication the filter's status (initialized/removed)
- throw an exception if the status is incorrect when he use the filter 
(protecting the user if he forgets to initialize the filter).

I'm not completely convinced that I have the best possible idea in this 
area, I'm just trying to be sure that we don't miss something important 
here. feel free to comment and tell if I'm wrong !

Thanks !

cordialement, regards,
Emmanuel L├ęcharny

View raw message