oxsean commented on code in PR #13786:
URL: https://github.com/apache/dubbo/pull/13786#discussion_r1501372297
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/codec/JsonCodecFactory.java:
##########
@@ -27,9 +27,11 @@
@Activate
public final class JsonCodecFactory implements HttpMessageEncoderFactory,
HttpMessageDecoderFactory {
+ public static final JsonCodec instance = new JsonCodec();
Review Comment:
I think this modification is unrelated to the PR, and it's not reasonable as
an instance of factory.
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/h12/grpc/GrpcCompositeCodec.java:
##########
@@ -16,123 +16,103 @@
*/
package org.apache.dubbo.rpc.protocol.tri.h12.grpc;
+import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.config.ConfigurationUtils;
+import org.apache.dubbo.common.io.StreamUtils;
+import org.apache.dubbo.common.utils.ArrayUtils;
import org.apache.dubbo.remoting.http12.exception.DecodeException;
import org.apache.dubbo.remoting.http12.exception.EncodeException;
import org.apache.dubbo.remoting.http12.message.HttpMessageCodec;
import org.apache.dubbo.remoting.http12.message.MediaType;
+import org.apache.dubbo.rpc.model.FrameworkModel;
+import org.apache.dubbo.rpc.model.MethodDescriptor;
+import org.apache.dubbo.rpc.model.PackableMethod;
+import org.apache.dubbo.rpc.model.PackableMethodFactory;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.charset.Charset;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
-import com.google.protobuf.Message;
-
-import static
org.apache.dubbo.common.constants.CommonConstants.PROTOBUF_MESSAGE_CLASS_NAME;
+import static org.apache.dubbo.common.constants.CommonConstants.DEFAULT_KEY;
+import static
org.apache.dubbo.common.constants.CommonConstants.DUBBO_PACKABLE_METHOD_FACTORY;
public class GrpcCompositeCodec implements HttpMessageCodec {
Review Comment:
@icodening Could you please review the modification of GrpcCompositeCodec?
I'm not very familiar this part.
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/RequestMetadata.java:
##########
@@ -57,7 +57,8 @@ public DefaultHttp2Headers toHeaders() {
.method(HttpMethod.POST.asciiName())
.path("/" + service + "/" + method.getMethodName())
.set(TripleHeaderEnum.CONTENT_TYPE_KEY.getHeader(),
MediaType.APPLICATION_GRPC_PROTO.getName())
- .set(HttpHeaderNames.TE, HttpHeaderValues.TRAILERS);
+ .set(HttpHeaderNames.TE, HttpHeaderValues.TRAILERS)
+ .set(TripleHeaderEnum.TRI_PARAM_DESC.getHeader(),
method.getParamDesc());
Review Comment:
In the triple wrapper, the content already include the method description,
and it seems that this parsing is not complicated. Adding a head makes the
protocol more complex. Does the benefit outweigh the increase in complexity?
@AlbumenJ What's your opinion?
##########
dubbo-remoting/dubbo-remoting-http12/src/main/java/org/apache/dubbo/remoting/http12/message/LengthFieldStreamingDecoder.java:
##########
@@ -46,8 +45,6 @@ public class LengthFieldStreamingDecoder implements
StreamingDecoder {
private int requiredLength;
- private InputStream dataHeader = new ByteArrayInputStream(new byte[0]);
Review Comment:
@icodening Can you review this modification? It seems that LengthField and
accumulate are not necessary, can we simplify the logic here?
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/route/DefaultRequestRouter.java:
##########
@@ -45,8 +45,7 @@ public RpcInvocationBuildContext route(URL url,
RequestMetadata metadata, HttpCh
HttpRequest request = httpMessageAdapterFactory.adaptRequest(metadata,
httpChannel);
HttpResponse response =
httpMessageAdapterFactory.adaptResponse(request, metadata);
- for (int i = 0, size = requestHandlerMappings.size(); i < size; i++) {
- RequestHandlerMapping mapping = requestHandlerMappings.get(i);
+ for (RequestHandlerMapping mapping : requestHandlerMappings) {
Review Comment:
The enhanced index loop can avoid creating iterator object. Although it's a
minor optimization, I think it's unnecessary to switch to an iterator loop.
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
