oxsean commented on code in PR #14026:
URL: https://github.com/apache/dubbo/pull/14026#discussion_r1583229862
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2Protocol.java:
##########
@@ -143,31 +149,54 @@ public void configServerProtocolHandler(URL url,
ChannelOperator operator) {
}
private void configurerHttp1Handlers(URL url, List<ChannelHandler>
handlers) {
- handlers.add(new ChannelHandlerPretender(new HttpServerCodec()));
+ final HttpServerCodec sourceCodec = new HttpServerCodec();
+ handlers.add(new ChannelHandlerPretender(sourceCodec));
+ // Triple protocol http1 upgrade support
+ handlers.add(new ChannelHandlerPretender(new HttpServerUpgradeHandler(
+ sourceCodec,
+ protocol -> {
+ if
(!AsciiString.contentEquals(Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME,
protocol)) {
Review Comment:
Generally, non-inverted if is easier to understand:
```
if (AsciiString.contentEquals(Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME,
protocol)) {
return buildHttp2ServerUpgradeCodec(url);
}
return null;
```
##########
dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationServerHandler.java:
##########
@@ -181,4 +153,48 @@ private boolean isSsl(ByteBuf buf) {
}
return false;
}
+
+ private void invokeProtocol(ChannelHandlerContext ctx, URL url,
NettyChannel channel, ByteBuf in) {
Review Comment:
detectProtocol is more appropriate
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2Protocol.java:
##########
@@ -143,31 +149,54 @@ public void configServerProtocolHandler(URL url,
ChannelOperator operator) {
}
private void configurerHttp1Handlers(URL url, List<ChannelHandler>
handlers) {
- handlers.add(new ChannelHandlerPretender(new HttpServerCodec()));
+ final HttpServerCodec sourceCodec = new HttpServerCodec();
+ handlers.add(new ChannelHandlerPretender(sourceCodec));
+ // Triple protocol http1 upgrade support
+ handlers.add(new ChannelHandlerPretender(new HttpServerUpgradeHandler(
+ sourceCodec,
+ protocol -> {
+ if
(!AsciiString.contentEquals(Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME,
protocol)) {
+ // Not upgrade request
+ return null;
+ }
+ return buildHttp2ServerUpgradeCodec(url);
+ },
+ Integer.MAX_VALUE)));
+ // If the upgrade was successful, remove the message from the output
list
+ // so that it's not propagated to the next handler. This request will
+ // be propagated as a user event instead.
handlers.add(new ChannelHandlerPretender(new
HttpObjectAggregator(Integer.MAX_VALUE)));
handlers.add(new ChannelHandlerPretender(new NettyHttp1Codec()));
handlers.add(new ChannelHandlerPretender(new
NettyHttp1ConnectionHandler(
url, frameworkModel,
DefaultHttp11ServerTransportListenerFactory.INSTANCE)));
}
+ private Http2ServerUpgradeCodec buildHttp2ServerUpgradeCodec(URL url) {
+ Configuration config =
ConfigurationUtils.getGlobalConfiguration(url.getOrDefaultApplicationModel());
+ final Http2FrameCodec codec = buildHttp2FrameCodec(config,
url.getOrDefaultApplicationModel());
+ final Http2MultiplexHandler handler = new Http2MultiplexHandler(new
ChannelInitializer<Http2StreamChannel>() {
Review Comment:
Also can extract a build method:
```
return new Http2ServerUpgradeCodec(
buildHttp2FrameCodec(config, url),
new HttpServerAfterUpgradeHandler(),
new HttpWriteQueueHandler(),
new FlushConsolidationHandler(64, true),
new TripleServerConnectionHandler(),
buildHttp2MultiplexHandler(url, frameworkmodel),
new TripleTailHandler());
```
##########
dubbo-rpc/dubbo-rpc-triple/src/main/java/org/apache/dubbo/rpc/protocol/tri/TripleHttp2Protocol.java:
##########
@@ -143,31 +149,54 @@ public void configServerProtocolHandler(URL url,
ChannelOperator operator) {
}
private void configurerHttp1Handlers(URL url, List<ChannelHandler>
handlers) {
- handlers.add(new ChannelHandlerPretender(new HttpServerCodec()));
+ final HttpServerCodec sourceCodec = new HttpServerCodec();
+ handlers.add(new ChannelHandlerPretender(sourceCodec));
+ // Triple protocol http1 upgrade support
+ handlers.add(new ChannelHandlerPretender(new HttpServerUpgradeHandler(
+ sourceCodec,
+ protocol -> {
+ if
(!AsciiString.contentEquals(Http2CodecUtil.HTTP_UPGRADE_PROTOCOL_NAME,
protocol)) {
+ // Not upgrade request
+ return null;
+ }
+ return buildHttp2ServerUpgradeCodec(url);
+ },
+ Integer.MAX_VALUE)));
+ // If the upgrade was successful, remove the message from the output
list
+ // so that it's not propagated to the next handler. This request will
+ // be propagated as a user event instead.
handlers.add(new ChannelHandlerPretender(new
HttpObjectAggregator(Integer.MAX_VALUE)));
handlers.add(new ChannelHandlerPretender(new NettyHttp1Codec()));
handlers.add(new ChannelHandlerPretender(new
NettyHttp1ConnectionHandler(
url, frameworkModel,
DefaultHttp11ServerTransportListenerFactory.INSTANCE)));
}
+ private Http2ServerUpgradeCodec buildHttp2ServerUpgradeCodec(URL url) {
+ Configuration config =
ConfigurationUtils.getGlobalConfiguration(url.getOrDefaultApplicationModel());
Review Comment:
Move into buildHttp2FrameCodec
--
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]