You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Trustin Lee <tr...@gmail.com> on 2007/08/17 09:47:30 UTC

Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Hi all,

Currently, ProtocolEncoder and ProtocolDecoder uses IoSession
user-defined attributes to store their state information.

If what ProtocolEncoder and ProtocolDecoder do with IoSession is just
to access user defined attributes, we could simply remove the coupling
with IoSession and replace it with java.util.Map (or ConcurrentMap).
By doing this, we can make our codecs reusable across various
applications, even that don't use MINA as a network layer.

Please let me know if there's a use case that you access other
properties or operations of IoSession in your codec implementation and
why it's mandatory.

Thanks in advance,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Niklas Therning <ni...@trillian.se> wrote:
> Trustin Lee wrote:
> > On 8/17/07, Niklas Therning <ni...@trillian.se> wrote:
> >
> >> Trustin Lee wrote:
> >>
> >>> On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> >>>
> >>>
> >>>> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
> >>>>
> >>>>
> >>>>> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> >>>>>
> >>>>>
> >>>>>> Hi,
> >>>>>> It think the ProtocolDecoder implementer can encapsulate his logic
> >>>>>> under some other class if it doesn't want to depend on MINA, but with
> >>>>>> ByteBuffer, and all the point Maaarten added (like
> >>>>>> ProtocolDecoderOutput) I think it won't be easly doable without
> >>>>>> breaking the codec API and sacrifice some of it simplicity.
> >>>>>>
> >>>>>>
> >>>>> Yeah, you are right.  I forgot the ByteBuffer! :)
> >>>>>
> >>>>> But the point here is to decouple a codec from IoSession so the codec
> >>>>> can be easily adaptable into other application component.  ByteBuffer
> >>>>> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
> >>>>> because they are very simple, but IoSession is a different beast.
> >>>>>
> >>>>> We could provide a mock implementation of IoSession, but I guess it
> >>>>> will not look nice (i.e. it violates OO principle).
> >>>>>
> >>>>>
> >>>> Ok, back to your original suggestion :-)
> >>>>
> >>>>
> >>> Good to see you back to the track. :D
> >>>
> >>>
> >>>
> >>>> I think it's a really good idea to remove the IoSession from the signature
> >>>> of the encoder/decoder.
> >>>> Testing encoders/decoders will also be asier since  one won't need a Dummy
> >>>> IoSession anymore.
> >>>>
> >>>>
> >>> Exactly.  That's why I posted the first message in this thread.  :)
> >>> However, I wanted to make sure if there's any special use case I might
> >>> have missed.
> >>>
> >>> Hey MINA community, your participation is appreciated!
> >>>
> >>> Thanks in advance,
> >>> Trustin
> >>>
> >>>
> >> I'd say that the current ProtocolEncoder and ProtocolDecoder are fine. I
> >> see no problem with having them depend on IoSession. Instead we could
> >> create base classes for our codecs which aren't dependent on IoSession
> >> (use Map only) and don't implement ProtocolDecoder/Encoder at all. Then
> >> the actual ProtocolDecoder/Encoder implementations would simply extend
> >> these base classes and call the base class with
> >> IoSession.getAttributesMap() as argument (provided that there is such a
> >> method, I don't remember at the moment).
> >>
> >> This way the codec could be even more independent from MINA. They don't
> >> even implement ProtocolDecoder/Encoder and the child classes would
> >> probably be trivial in most cases.
> >>
> >> WDYT?
> >>
> >
> > It sounds like an interesting idea.  We could probably provide a
> > separate package that contains the classes you mentioned and
> > ByteBuffer (I think we still need coupling with MINA ByteBuffer
> > because it's very convenient.)
> >
> > Could you provide an example code of the generic codec you are
> > suggesting to help our understanding?
> >
> > Trustin
> >
> Well, consider the HttpRequestDecoder in examples. Lets break out the
> methods
>
> boolean messageComplete(ByteBuffer in)
> HttpRequestMessage decodeBody(ByteBuffer in)
> Map parseRequest(Reader is)
>
> into a base class and change the methods' scope to protected:
>
> public class BaseHttpRequestDecoder {
>     private static final byte[] CONTENT_LENGTH = new
> String("Content-Length:")
>             .getBytes();
>
>     private final CharsetDecoder decoder = Charset.defaultCharset()
>             .newDecoder();
>
>     protected boolean messageComplete(ByteBuffer in) { ... }
>     protected HttpRequestMessage decodeBody(ByteBuffer in) { ... }
>     protected Map parseRequest(Reader is) { ... }
> }
>
> The only dependency on MINA here is (I hope!) ByteBuffer.
>
> The new HttpRequestDecoder would simply become
>
> public class HttpRequestDecoder extends BaseHttpRequestDecoder {
>
>     public MessageDecoderResult decodable(IoSession session, ByteBuffer
> in) {
>         // Return NEED_DATA if the whole header is not read yet.
>         try {
>             return messageComplete(in) ? MessageDecoderResult.OK
>                     : MessageDecoderResult.NEED_DATA;
>         } catch (Exception ex) {
>             ex.printStackTrace();
>         }
>
>         return MessageDecoderResult.NOT_OK;
>     }
>
>     public MessageDecoderResult decode(IoSession session, ByteBuffer in,
>             ProtocolDecoderOutput out) throws Exception {
>         // Try to decode body
>         HttpRequestMessage m = decodeBody(in);
>
>         // Return NEED_DATA if the body is not fully read.
>         if (m == null) {
>             return MessageDecoderResult.NEED_DATA;
>         }
>
>         out.write(m);
>
>         return MessageDecoderResult.OK;
>     }
>
>     public void finishDecode(IoSession session, ProtocolDecoderOutput out)
>             throws Exception {
>     }
> }
>
> I think this would work in this example at least. :-) Maybe delegation
> would be better than subclassing in this case since HttpRequestDecoder
> cannot extend MessageDecoderAdapter any longer.

Hmm.. my idea was to provide a kind of standard interface for codec,
and I find your idea comes from a different land. :)   I agree with
you that delegation is a better choice in this case, and in general, a
user could extract existing MINA codec implementation to POJO, and
implement MINA codec classes to wrap the extracted POJO codec
implementation, if the user needs reusable codec.

However, at least for decoding, we already proved that our
ProtocolDecoder + ProtocolDecoderOutput pattern fits perfectly for
decoding in all kinds of non-blocking environment.

As for encoding part, it's somewhat of dubious value, but having
standard interface makes integration of various codecs (i.e. creating
a new codec using existing codec implementations).

So.. what about providing them as more generic package  along with ByteBuffer?

Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Niklas Therning <ni...@trillian.se>.
Trustin Lee wrote:
> On 8/17/07, Niklas Therning <ni...@trillian.se> wrote:
>   
>> Trustin Lee wrote:
>>     
>>> On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
>>>
>>>       
>>>> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
>>>>
>>>>         
>>>>> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
>>>>>
>>>>>           
>>>>>> Hi,
>>>>>> It think the ProtocolDecoder implementer can encapsulate his logic
>>>>>> under some other class if it doesn't want to depend on MINA, but with
>>>>>> ByteBuffer, and all the point Maaarten added (like
>>>>>> ProtocolDecoderOutput) I think it won't be easly doable without
>>>>>> breaking the codec API and sacrifice some of it simplicity.
>>>>>>
>>>>>>             
>>>>> Yeah, you are right.  I forgot the ByteBuffer! :)
>>>>>
>>>>> But the point here is to decouple a codec from IoSession so the codec
>>>>> can be easily adaptable into other application component.  ByteBuffer
>>>>> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
>>>>> because they are very simple, but IoSession is a different beast.
>>>>>
>>>>> We could provide a mock implementation of IoSession, but I guess it
>>>>> will not look nice (i.e. it violates OO principle).
>>>>>
>>>>>           
>>>> Ok, back to your original suggestion :-)
>>>>
>>>>         
>>> Good to see you back to the track. :D
>>>
>>>
>>>       
>>>> I think it's a really good idea to remove the IoSession from the signature
>>>> of the encoder/decoder.
>>>> Testing encoders/decoders will also be asier since  one won't need a Dummy
>>>> IoSession anymore.
>>>>
>>>>         
>>> Exactly.  That's why I posted the first message in this thread.  :)
>>> However, I wanted to make sure if there's any special use case I might
>>> have missed.
>>>
>>> Hey MINA community, your participation is appreciated!
>>>
>>> Thanks in advance,
>>> Trustin
>>>
>>>       
>> I'd say that the current ProtocolEncoder and ProtocolDecoder are fine. I
>> see no problem with having them depend on IoSession. Instead we could
>> create base classes for our codecs which aren't dependent on IoSession
>> (use Map only) and don't implement ProtocolDecoder/Encoder at all. Then
>> the actual ProtocolDecoder/Encoder implementations would simply extend
>> these base classes and call the base class with
>> IoSession.getAttributesMap() as argument (provided that there is such a
>> method, I don't remember at the moment).
>>
>> This way the codec could be even more independent from MINA. They don't
>> even implement ProtocolDecoder/Encoder and the child classes would
>> probably be trivial in most cases.
>>
>> WDYT?
>>     
>
> It sounds like an interesting idea.  We could probably provide a
> separate package that contains the classes you mentioned and
> ByteBuffer (I think we still need coupling with MINA ByteBuffer
> because it's very convenient.)
>
> Could you provide an example code of the generic codec you are
> suggesting to help our understanding?
>
> Trustin
>   
Well, consider the HttpRequestDecoder in examples. Lets break out the
methods

boolean messageComplete(ByteBuffer in)
HttpRequestMessage decodeBody(ByteBuffer in)
Map parseRequest(Reader is)

into a base class and change the methods' scope to protected:

public class BaseHttpRequestDecoder {
    private static final byte[] CONTENT_LENGTH = new
String("Content-Length:")
            .getBytes();

    private final CharsetDecoder decoder = Charset.defaultCharset()
            .newDecoder();

    protected boolean messageComplete(ByteBuffer in) { ... }
    protected HttpRequestMessage decodeBody(ByteBuffer in) { ... }
    protected Map parseRequest(Reader is) { ... }
}

The only dependency on MINA here is (I hope!) ByteBuffer.

The new HttpRequestDecoder would simply become

public class HttpRequestDecoder extends BaseHttpRequestDecoder {

    public MessageDecoderResult decodable(IoSession session, ByteBuffer
in) {
        // Return NEED_DATA if the whole header is not read yet.
        try {
            return messageComplete(in) ? MessageDecoderResult.OK
                    : MessageDecoderResult.NEED_DATA;
        } catch (Exception ex) {
            ex.printStackTrace();
        }

        return MessageDecoderResult.NOT_OK;
    }

    public MessageDecoderResult decode(IoSession session, ByteBuffer in,
            ProtocolDecoderOutput out) throws Exception {
        // Try to decode body
        HttpRequestMessage m = decodeBody(in);

        // Return NEED_DATA if the body is not fully read.
        if (m == null) {
            return MessageDecoderResult.NEED_DATA;
        }

        out.write(m);

        return MessageDecoderResult.OK;
    }

    public void finishDecode(IoSession session, ProtocolDecoderOutput out)
            throws Exception {
    }
}

I think this would work in this example at least. :-) Maybe delegation
would be better than subclassing in this case since HttpRequestDecoder
cannot extend MessageDecoderAdapter any longer.

Disclaimer: I have no idea whether this will work for any decoder/encoder.

-- 
Niklas Therning
www.spamdrain.net


Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Niklas Therning <ni...@trillian.se> wrote:
> Trustin Lee wrote:
> > On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> >
> >> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
> >>
> >>> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> >>>
> >>>> Hi,
> >>>> It think the ProtocolDecoder implementer can encapsulate his logic
> >>>> under some other class if it doesn't want to depend on MINA, but with
> >>>> ByteBuffer, and all the point Maaarten added (like
> >>>> ProtocolDecoderOutput) I think it won't be easly doable without
> >>>> breaking the codec API and sacrifice some of it simplicity.
> >>>>
> >>> Yeah, you are right.  I forgot the ByteBuffer! :)
> >>>
> >>> But the point here is to decouple a codec from IoSession so the codec
> >>> can be easily adaptable into other application component.  ByteBuffer
> >>> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
> >>> because they are very simple, but IoSession is a different beast.
> >>>
> >>> We could provide a mock implementation of IoSession, but I guess it
> >>> will not look nice (i.e. it violates OO principle).
> >>>
> >> Ok, back to your original suggestion :-)
> >>
> >
> > Good to see you back to the track. :D
> >
> >
> >> I think it's a really good idea to remove the IoSession from the signature
> >> of the encoder/decoder.
> >> Testing encoders/decoders will also be asier since  one won't need a Dummy
> >> IoSession anymore.
> >>
> >
> > Exactly.  That's why I posted the first message in this thread.  :)
> > However, I wanted to make sure if there's any special use case I might
> > have missed.
> >
> > Hey MINA community, your participation is appreciated!
> >
> > Thanks in advance,
> > Trustin
> >
> I'd say that the current ProtocolEncoder and ProtocolDecoder are fine. I
> see no problem with having them depend on IoSession. Instead we could
> create base classes for our codecs which aren't dependent on IoSession
> (use Map only) and don't implement ProtocolDecoder/Encoder at all. Then
> the actual ProtocolDecoder/Encoder implementations would simply extend
> these base classes and call the base class with
> IoSession.getAttributesMap() as argument (provided that there is such a
> method, I don't remember at the moment).
>
> This way the codec could be even more independent from MINA. They don't
> even implement ProtocolDecoder/Encoder and the child classes would
> probably be trivial in most cases.
>
> WDYT?

It sounds like an interesting idea.  We could probably provide a
separate package that contains the classes you mentioned and
ByteBuffer (I think we still need coupling with MINA ByteBuffer
because it's very convenient.)

Could you provide an example code of the generic codec you are
suggesting to help our understanding?

Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Niklas Therning <ni...@trillian.se>.
Trustin Lee wrote:
> On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
>   
>> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
>>     
>>> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
>>>       
>>>> Hi,
>>>> It think the ProtocolDecoder implementer can encapsulate his logic
>>>> under some other class if it doesn't want to depend on MINA, but with
>>>> ByteBuffer, and all the point Maaarten added (like
>>>> ProtocolDecoderOutput) I think it won't be easly doable without
>>>> breaking the codec API and sacrifice some of it simplicity.
>>>>         
>>> Yeah, you are right.  I forgot the ByteBuffer! :)
>>>
>>> But the point here is to decouple a codec from IoSession so the codec
>>> can be easily adaptable into other application component.  ByteBuffer
>>> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
>>> because they are very simple, but IoSession is a different beast.
>>>
>>> We could provide a mock implementation of IoSession, but I guess it
>>> will not look nice (i.e. it violates OO principle).
>>>       
>> Ok, back to your original suggestion :-)
>>     
>
> Good to see you back to the track. :D
>
>   
>> I think it's a really good idea to remove the IoSession from the signature
>> of the encoder/decoder.
>> Testing encoders/decoders will also be asier since  one won't need a Dummy
>> IoSession anymore.
>>     
>
> Exactly.  That's why I posted the first message in this thread.  :)
> However, I wanted to make sure if there's any special use case I might
> have missed.
>
> Hey MINA community, your participation is appreciated!
>
> Thanks in advance,
> Trustin
>   
I'd say that the current ProtocolEncoder and ProtocolDecoder are fine. I
see no problem with having them depend on IoSession. Instead we could
create base classes for our codecs which aren't dependent on IoSession
(use Map only) and don't implement ProtocolDecoder/Encoder at all. Then
the actual ProtocolDecoder/Encoder implementations would simply extend
these base classes and call the base class with
IoSession.getAttributesMap() as argument (provided that there is such a
method, I don't remember at the moment).

This way the codec could be even more independent from MINA. They don't
even implement ProtocolDecoder/Encoder and the child classes would
probably be trivial in most cases.

WDYT?

-- 
Niklas Therning
www.spamdrain.net


Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
> >
> > On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> > > Hi,
> > > It think the ProtocolDecoder implementer can encapsulate his logic
> > > under some other class if it doesn't want to depend on MINA, but with
> > > ByteBuffer, and all the point Maaarten added (like
> > > ProtocolDecoderOutput) I think it won't be easly doable without
> > > breaking the codec API and sacrifice some of it simplicity.
> >
> > Yeah, you are right.  I forgot the ByteBuffer! :)
> >
> > But the point here is to decouple a codec from IoSession so the codec
> > can be easily adaptable into other application component.  ByteBuffer
> > and ProtocolEncoder/DecoderOutput is very easy to convert as you know
> > because they are very simple, but IoSession is a different beast.
> >
> > We could provide a mock implementation of IoSession, but I guess it
> > will not look nice (i.e. it violates OO principle).
>
> Ok, back to your original suggestion :-)

Good to see you back to the track. :D

> I think it's a really good idea to remove the IoSession from the signature
> of the encoder/decoder.
> Testing encoders/decoders will also be asier since  one won't need a Dummy
> IoSession anymore.

Exactly.  That's why I posted the first message in this thread.  :)
However, I wanted to make sure if there's any special use case I might
have missed.

Hey MINA community, your participation is appreciated!

Thanks in advance,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Maarten Bosteels <mb...@gmail.com>.
On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
>
> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> > Hi,
> > It think the ProtocolDecoder implementer can encapsulate his logic
> > under some other class if it doesn't want to depend on MINA, but with
> > ByteBuffer, and all the point Maaarten added (like
> > ProtocolDecoderOutput) I think it won't be easly doable without
> > breaking the codec API and sacrifice some of it simplicity.
>
> Yeah, you are right.  I forgot the ByteBuffer! :)
>
> But the point here is to decouple a codec from IoSession so the codec
> can be easily adaptable into other application component.  ByteBuffer
> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
> because they are very simple, but IoSession is a different beast.
>
> We could provide a mock implementation of IoSession, but I guess it
> will not look nice (i.e. it violates OO principle).



Ok, back to your original suggestion :-)
I think it's a really good idea to remove the IoSession from the signature
of the encoder/decoder.
Testing encoders/decoders will also be asier since  one won't need a Dummy
IoSession anymore.

Maarten

Thanks,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Julien Vermillard <jv...@archean.fr>.
On Fri, 17 Aug 2007 20:42:23 +0900
"Trustin Lee" <tr...@gmail.com> wrote:

> On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> > Hi,
> > It think the ProtocolDecoder implementer can encapsulate his logic
> > under some other class if it doesn't want to depend on MINA, but
> > with ByteBuffer, and all the point Maaarten added (like
> > ProtocolDecoderOutput) I think it won't be easly doable without
> > breaking the codec API and sacrifice some of it simplicity.
> 
> Yeah, you are right.  I forgot the ByteBuffer! :)
> 
> But the point here is to decouple a codec from IoSession so the codec
> can be easily adaptable into other application component.  ByteBuffer
> and ProtocolEncoder/DecoderOutput is very easy to convert as you know
> because they are very simple, but IoSession is a different beast.
> 
> We could provide a mock implementation of IoSession, but I guess it
> will not look nice (i.e. it violates OO principle).
> 
> Thanks,
> Trustin
A good use for a Codec outside of the ProtocolFilter could be (at least
for me) serializing to file a stream of object/messages.

HTH
Julien

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Julien Vermillard <jv...@archean.fr> wrote:
> Hi,
> It think the ProtocolDecoder implementer can encapsulate his logic
> under some other class if it doesn't want to depend on MINA, but with
> ByteBuffer, and all the point Maaarten added (like
> ProtocolDecoderOutput) I think it won't be easly doable without
> breaking the codec API and sacrifice some of it simplicity.

Yeah, you are right.  I forgot the ByteBuffer! :)

But the point here is to decouple a codec from IoSession so the codec
can be easily adaptable into other application component.  ByteBuffer
and ProtocolEncoder/DecoderOutput is very easy to convert as you know
because they are very simple, but IoSession is a different beast.

We could provide a mock implementation of IoSession, but I guess it
will not look nice (i.e. it violates OO principle).

Thanks,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Julien Vermillard <jv...@archean.fr>.
Hi,
It think the ProtocolDecoder implementer can encapsulate his logic
under some other class if it doesn't want to depend on MINA, but with
ByteBuffer, and all the point Maaarten added (like
ProtocolDecoderOutput) I think it won't be easly doable without
breaking the codec API and sacrifice some of it simplicity.

Julien

On Fri, 17 Aug 2007 11:03:07 +0200
"Maarten Bosteels" <mb...@gmail.com> wrote:

> Trustin.
> 
> I like the idea, but I guess some people (like me) use the IoSession
> for logging the remote address.
> And I guess codec implementations will still depend on mina because of
> ProtocolDecoderOutput, right ?
> 
> About logging, I guess slf4j does not have something as log4j's NDC
> concept ?
> I have an IoFilter that pushes the remote address in the NDC before
> propagating the event and pops it afterwards.
> Very handy, because all logging events down the call stack will
> contain the remote address, without depending on mina's
> SessionLog.
> 
> I guess we cannot add this IoFilter to mina since it depends on
> log4j ? maybe I should write a wiki page about it ?
> 
> Maarten
> 
> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
> >
> > Hi all,
> >
> > Currently, ProtocolEncoder and ProtocolDecoder uses IoSession
> > user-defined attributes to store their state information.
> >
> > If what ProtocolEncoder and ProtocolDecoder do with IoSession is
> > just to access user defined attributes, we could simply remove the
> > coupling with IoSession and replace it with java.util.Map (or
> > ConcurrentMap). By doing this, we can make our codecs reusable
> > across various applications, even that don't use MINA as a network
> > layer.
> >
> > Please let me know if there's a use case that you access other
> > properties or operations of IoSession in your codec implementation
> > and why it's mandatory.
> >
> > Thanks in advance,
> > Trustin
> > --
> > what we call human nature is actually human habit
> > --
> > http://gleamynode.net/
> > --
> > PGP Key ID: 0x0255ECA6
> >

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
> >
> > On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > > Trustin.
> > >
> > > I like the idea, but I guess some people (like me) use the IoSession for
> > > logging the remote address.
> > > And I guess codec implementations will still depend on mina because of
> > > ProtocolDecoderOutput, right ?
> >
> > Yeah, I agree with you and that's why I am asking if there's any use
> > cases. :)
> >
> > ProtocolDecoderOutput and ProtocolEncoderOutput also doesn't have any
> > dependency on IoSession or any other MINA interfaces, so protocol
> > implementors will be enough with ProtocolEncoder, ProtocolDecoder,
> > ProtocolEncoderOutput and ProtocolDecoderOutput.
> >
> > > About logging, I guess slf4j does not have something as log4j's NDC
> > concept
> > > ?
> > > I have an IoFilter that pushes the remote address in the NDC before
> > > propagating the event and pops it afterwards.
> > > Very handy, because all logging events down the call stack will contain
> > the
> > > remote address, without depending on mina's
> > > SessionLog.
> >
> > SLF4J doesn't support NDC, but at least it supports MDC:
> >
> > http://www.slf4j.org/api/org/slf4j/MDC.html
> >
> > IIUC, MDC or NDC should be used for purely diagnostic purpose, so I
> > guess we need to provide a Map to the codec anyway. WDYT?
>
>
> I  am not following you here.
> Do you mean the map with session attributes that would replace the IoSession
> in the decode method ?
> What has it got to do with an MdcLoggingFilter ?
>
> Oh wait, you thought I was suggesting to replace the IoSession parameter
> with an NDC or MDC ?
> No, no, of course, MDC/NDC is just for logging.
>
> It 's just that I was thinking that some people would use the IoSession in
> their decoder to be able to log
> for which session they are decoding. And that's when I remembered my
> NdcLoggingFilter.
> Sorry for the confusion.

NP. Let's start a new thread about MDCLoggingFilter.

> > Actually I have never tried this kind of feature and that's why I was
> > trying to create a new Logger class that decorates message output.  If
> > we can utilize MDC, we might not need log message decoration at all.
> > What was your experience?
>
>
> Very good.  I first implemented it using mina 0.8 : it was very simple and
> didn't have any problems.
>
> When I switched to 2.x it was a bit more tricky because of the
> ExecutorFilter in the chain:
> The loggingfilter should be after the executorfilter, but then my
> encoders/decoders weren't decorated.
> (I had a quick and dirty fix for that, but will try to find something less
> dirty ;-)

Right.  ThreadLocal doesn't work very well with MINA, so MDC might not
be a good solution because it can cause very frequent creation and
destruction of ThreadLocal entries.

> But the nice thing is that all logging events, even those generated by other
> libs like spring and hibernate
> all have the remote address info. IMHO that's a big advantage over a
> decorator like SessionLogger.
>
> You could grep on a specific ip address + port in the logfile and see the
> complete flow for just that client.
>
> Let me have a look at the MDC (I think it's even more convenient than NDC)
> and I try to build
> an MdcLoggingFilter this weekend, using SLF4J.

Great!

Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Maarten Bosteels <mb...@gmail.com>.
On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
>
> On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> > Trustin.
> >
> > I like the idea, but I guess some people (like me) use the IoSession for
> > logging the remote address.
> > And I guess codec implementations will still depend on mina because of
> > ProtocolDecoderOutput, right ?
>
> Yeah, I agree with you and that's why I am asking if there's any use
> cases. :)
>
> ProtocolDecoderOutput and ProtocolEncoderOutput also doesn't have any
> dependency on IoSession or any other MINA interfaces, so protocol
> implementors will be enough with ProtocolEncoder, ProtocolDecoder,
> ProtocolEncoderOutput and ProtocolDecoderOutput.
>
> > About logging, I guess slf4j does not have something as log4j's NDC
> concept
> > ?
> > I have an IoFilter that pushes the remote address in the NDC before
> > propagating the event and pops it afterwards.
> > Very handy, because all logging events down the call stack will contain
> the
> > remote address, without depending on mina's
> > SessionLog.
>
> SLF4J doesn't support NDC, but at least it supports MDC:
>
> http://www.slf4j.org/api/org/slf4j/MDC.html
>
> IIUC, MDC or NDC should be used for purely diagnostic purpose, so I
> guess we need to provide a Map to the codec anyway. WDYT?


I  am not following you here.
Do you mean the map with session attributes that would replace the IoSession
in the decode method ?
What has it got to do with an MdcLoggingFilter ?

Oh wait, you thought I was suggesting to replace the IoSession parameter
with an NDC or MDC ?
No, no, of course, MDC/NDC is just for logging.

It 's just that I was thinking that some people would use the IoSession in
their decoder to be able to log
for which session they are decoding. And that's when I remembered my
NdcLoggingFilter.
Sorry for the confusion.

Actually I have never tried this kind of feature and that's why I was
> trying to create a new Logger class that decorates message output.  If
> we can utilize MDC, we might not need log message decoration at all.
> What was your experience?


Very good.  I first implemented it using mina 0.8 : it was very simple and
didn't have any problems.

When I switched to 2.x it was a bit more tricky because of the
ExecutorFilter in the chain:
The loggingfilter should be after the executorfilter, but then my
encoders/decoders weren't decorated.
(I had a quick and dirty fix for that, but will try to find something less
dirty ;-)

But the nice thing is that all logging events, even those generated by other
libs like spring and hibernate
all have the remote address info. IMHO that's a big advantage over a
decorator like SessionLogger.

You could grep on a specific ip address + port in the logfile and see the
complete flow for just that client.

Let me have a look at the MDC (I think it's even more convenient than NDC)
and I try to build
an MdcLoggingFilter this weekend, using SLF4J.

Maarten

Thanks,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Trustin Lee <tr...@gmail.com>.
On 8/17/07, Maarten Bosteels <mb...@gmail.com> wrote:
> Trustin.
>
> I like the idea, but I guess some people (like me) use the IoSession for
> logging the remote address.
> And I guess codec implementations will still depend on mina because of
> ProtocolDecoderOutput, right ?

Yeah, I agree with you and that's why I am asking if there's any use cases. :)

ProtocolDecoderOutput and ProtocolEncoderOutput also doesn't have any
dependency on IoSession or any other MINA interfaces, so protocol
implementors will be enough with ProtocolEncoder, ProtocolDecoder,
ProtocolEncoderOutput and ProtocolDecoderOutput.

> About logging, I guess slf4j does not have something as log4j's NDC concept
> ?
> I have an IoFilter that pushes the remote address in the NDC before
> propagating the event and pops it afterwards.
> Very handy, because all logging events down the call stack will contain the
> remote address, without depending on mina's
> SessionLog.

SLF4J doesn't support NDC, but at least it supports MDC:

http://www.slf4j.org/api/org/slf4j/MDC.html

IIUC, MDC or NDC should be used for purely diagnostic purpose, so I
guess we need to provide a Map to the codec anyway. WDYT?

Actually I have never tried this kind of feature and that's why I was
trying to create a new Logger class that decorates message output.  If
we can utilize MDC, we might not need log message decoration at all.
What was your experience?

Thanks,
Trustin
-- 
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6

Re: Making ProtocolEncoder and ProtocolDecoder independent from IoSession

Posted by Maarten Bosteels <mb...@gmail.com>.
Trustin.

I like the idea, but I guess some people (like me) use the IoSession for
logging the remote address.
And I guess codec implementations will still depend on mina because of
ProtocolDecoderOutput, right ?

About logging, I guess slf4j does not have something as log4j's NDC concept
?
I have an IoFilter that pushes the remote address in the NDC before
propagating the event and pops it afterwards.
Very handy, because all logging events down the call stack will contain the
remote address, without depending on mina's
SessionLog.

I guess we cannot add this IoFilter to mina since it depends on log4j ?
maybe I should write a wiki page about it ?

Maarten

On 8/17/07, Trustin Lee <tr...@gmail.com> wrote:
>
> Hi all,
>
> Currently, ProtocolEncoder and ProtocolDecoder uses IoSession
> user-defined attributes to store their state information.
>
> If what ProtocolEncoder and ProtocolDecoder do with IoSession is just
> to access user defined attributes, we could simply remove the coupling
> with IoSession and replace it with java.util.Map (or ConcurrentMap).
> By doing this, we can make our codecs reusable across various
> applications, even that don't use MINA as a network layer.
>
> Please let me know if there's a use case that you access other
> properties or operations of IoSession in your codec implementation and
> why it's mandatory.
>
> Thanks in advance,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>