On 8/17/07, Niklas Therning <[EMAIL PROTECTED]> wrote:
> Trustin Lee wrote:
> > On 8/17/07, Niklas Therning <[EMAIL PROTECTED]> wrote:
> >
> >> Trustin Lee wrote:
> >>
> >>> On 8/17/07, Maarten Bosteels <[EMAIL PROTECTED]> wrote:
> >>>
> >>>
> >>>> On 8/17/07, Trustin Lee <[EMAIL PROTECTED]> wrote:
> >>>>
> >>>>
> >>>>> On 8/17/07, Julien Vermillard <[EMAIL PROTECTED]> 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
