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.
Disclaimer: I have no idea whether this will work for any decoder/encoder.
--
Niklas Therning
www.spamdrain.net
