On 5/13/07, shumbola <[EMAIL PROTECTED]> wrote:
Hello,
Today I've got my feets wet with mina and wrote some code. I've studied
examples before that. But I'm not sure if I'm doing things correct, and I
kindly ask to review my code. If it is not the way how mina works, point me
to the right direction. BTW, I'm using mina 1.1.0. I've wrote a test server
and a multithreaded client in java, and was able to communicate in both
directions.
I'm going to connect to my mina server from the C clients over plain socket
calls. From C side, I'll be sending message type, body length and a body as
a hexadecimal string (which itself is a complex structure). And expect the
same format from server side.
In the future I may differentiate Request and Response messages, thats why
I've created two distinct classes.
So, I've come up with following for now:
-------
public class RequestMessage implements Serializable {
private short type;
private String body;
}
------
public class ResponseMessage implements Serializable {
private short type;
private String body;
}
------
public class PaynetPosProtocolCodecFactory extends
DemuxingProtocolCodecFactory {
public PaynetPosProtocolCodecFactory() {
super.register(PosRequestDecoder.class);
super.register(PosResponseEncoder.class);
}
}
-----
public class ResponseEncoder implements MessageEncoder {
private static final Set<Class> TYPES;
static
{
Set<Class> types = new HashSet<Class>();
types.add( ResponseMessage.class );
TYPES = Collections.unmodifiableSet( types );
}
public Set<Class> getMessageTypes() {
return TYPES;
}
public void encode(IoSession ioSession, Object message,
ProtocolEncoderOutput out) throws Exception {
ResponseMessage msg = ( ResponseMessage ) message;
ByteBuffer buf = ByteBuffer.allocate( 256 );
// Enable auto-expand for easier encoding
buf.setAutoExpand( true );
try
{
// output all headers except the content length
CharsetEncoder encoder = Charset.defaultCharset()
.newEncoder();
buf.putShort(msg.getType());
buf.putShort((short)msg.getBody().length());
buf.putString(msg.getBody(), encoder);
}
catch( CharacterCodingException ex )
{
ex.printStackTrace();
}
buf.flip();
out.write( buf );
}
}
The encoder implementation looks good except that the length field in
your message can be inaccurate if the body contains non-ascii
characters. I'd suggest you to use ByteBuffer.putPrefixedString()
method:
http://mina.apache.org/report/1.0/apidocs/org/apache/mina/common/ByteBuffer.html#putPrefixedString(java.lang.CharSequence,%20java.nio.charset.CharsetEncoder)
-----
public class RequestDecoder extends MessageDecoderAdapter {
private short type;
private boolean readHeader;
public MessageDecoderResult decodable(IoSession ioSession, ByteBuffer
in) {
// Return NEED_DATA if the whole header is not read yet.
if( in.remaining() < Constants.HEADER_LEN )
{
return MessageDecoderResult.NEED_DATA;
}
type = in.getShort();
if(type >= Constants.TYPE_MIN && type <= Constants.TYPE_MAX)
{
return MessageDecoderResult.OK;
}
// Return NOT_OK if not matches.
return MessageDecoderResult.NOT_OK;
}
decodable() looks OK, too. You've done very good so far! I'd set
type property in decode() though to make the role of decodable()
clearer and close to its original intention.
public MessageDecoderResult decode(IoSession ioSession, ByteBuffer in,
ProtocolDecoderOutput out) throws Exception {
RequestMessage message = new RequestMessage();
message.setType(type);
if(!readHeader)
{
in.getShort();
readHeader = true;
}
if(in.remaining() < 2)
{
return MessageDecoderResult.NEED_DATA;
}
short length = in.getShort();
if(in.remaining() < length)
{
return MessageDecoderResult.NEED_DATA;
}
byte [] buffer = new byte[length];
in.get(buffer);
String str = new String(buffer);
message.setBody(str);
out.write(message);
readHeader = false;
return MessageDecoderResult.OK;
}
}
This implementation will not work because length field will always be
read even if it's read already. You did well with the header (type)
field, but you didn't do anything for the length field. I'd add an
additional 'readLength' flag. Otherwise, I'd implement in a simpler
way:
if (in.remaining() < 4) {
return NEED_DATA;
}
in.mark();
short type = in.getShort();
short length = in.getShort();
if (in.remaining() < length) {
in.reset();
return NEED_DATA;
}
RequestMessage m = ...;
// TODO read body and set fields...
out.write(m);
HTH,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
