pandaapo commented on code in PR #4702:
URL: https://github.com/apache/eventmesh/pull/4702#discussion_r1444335384
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/codec/Codec.java:
##########
@@ -223,7 +223,9 @@ private static Object deserializeBody(String
bodyJsonString, Header header) thro
case REDIRECT_TO_CLIENT:
return JsonUtils.parseObject(bodyJsonString,
RedirectInfo.class);
default:
- LogUtils.warn(log, "Invalidate TCP command: {}", command);
+ if (log.isWarnEnabled()) {
+ log.warn("Invalidate TCP command: {}", command);
Review Comment:
The same situation as mentioned earlier:
https://github.com/apache/eventmesh/pull/4702#discussion_r1443969015
和前面同样的情况：https://github.com/apache/eventmesh/pull/4702#discussion_r1443969015
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/codec/Codec.java:
##########
@@ -171,7 +171,7 @@ private Object parseBody(ByteBuf in, Header header, int
bodyLength) throws JsonP
}
final byte[] bodyData = new byte[bodyLength];
in.readBytes(bodyData);
- LogUtils.debug(log, "Decode bodyJson={}",
deserializeBytes(bodyData));
+ LogUtils.debug(log, "Decode headerJson={}",
deserializeBytes(bodyData));
Review Comment:
It is `bodyJson` here instead of `headerJson`.
##########
eventmesh-common/src/main/java/org/apache/eventmesh/common/protocol/tcp/codec/Codec.java:
##########
@@ -116,31 +102,45 @@ public void encode(ChannelHandlerContext ctx, Package
pkg, ByteBuf out) throws E
}
}
- public static class Decoder extends ReplayingDecoder<Package> {
+ public static class Decoder extends LengthFieldBasedFrameDecoder {
+
+ public Decoder() {
+ super(FRAME_MAX_LENGTH, PREFIX_LENGTH, Integer.BYTES, -9, 0);
Review Comment:
I believe `Integer.BYTES` and `-9` are still not clear enough for
developers. `Integer.BYTES`: The lengths of version field, package length
field, and header length field are all `Integer.BYTES`. `-9`: Hard-coded.
If you disagree with replacing them with expressions composed of more
descriptive constant or variable names, as mentioned earlier, you can provide
detailed comments to explain the usage of `Integer.BYTES` and `-9` here.
我认为`Integer.BYTES`和`-9`对开发者来说依旧不够清晰。`Integer.BYTES`：Version字段, package
length字段, header length字段的长度都是`Integer.BYTES`。`-9`：硬编码。
如果您不赞同将它们换成有助理解的常量名或变量名组成的表达式，正如我之前所说，您可以用详细的注释来说明这里的`Integer.BYTES`和`-9`。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
