http protocol detector #12452
http protocol detector #12452
Conversation
|
@chickenlj rest protocol is integrated into PortUnification |
Codecov Report
@@ Coverage Diff @@
## 3.3 #12452 +/- ##
======================================
Coverage ? 68.28%
Complexity ? 5
======================================
Files ? 1644
Lines ? 67735
Branches ? 9872
======================================
Hits ? 46254
Misses ? 17007
Partials ? 4474 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@EarthChen @CrazyHZM PTAL |
| /** | ||
| * for http methods | ||
| */ | ||
| public class HttpUtils { |
There was a problem hiding this comment.
This class is not suitable for the dubco-common module in terms of function and methods. There is no current intersection between qos and http handling except HttpMethod, and the methods in HttpUtils are not suitable for all modules.
There was a problem hiding this comment.
ok , I will change the position
| return Result.UNRECOGNIZED; | ||
| } | ||
|
|
||
| default Result detect(ChannelBuffer in, URL url) { |
There was a problem hiding this comment.
I don't see a use for url, and it shouldn't be in this interface definition.
There was a problem hiding this comment.
1.qos和rest 在http检测这块是有冲突的， 当Qos 和rest同时开启时， 需要进行 requestUrl 判断 ， service经过 RestProtocol 进行export时才认为 rest开启，不能通过RestProtocol的存在与否判断，这个rest开启标志通过url进行传递
2.希望detector添加 开关，通过url属性进行判断
所以就增加了这个参数
There was a problem hiding this comment.
- 我在想qos有关http的是不是还是需要依赖http的模块，这样才能保证所有http的实现都在一个地方，它的层级应该跟triple是一样的，应该在抽象的http之上，qos的命令应该只是rest服务接口中的一部分，但是qos命令受开关影响，举个例子，有一个AbstractHttpProtocolDetector。
- 这个判断应该是有export的时候就应该把开关传递过来，就像QosEnableFlag，但我觉得这个开关的确可以被抽象在ProtocolDetector中。
There was a problem hiding this comment.
AbstractHttpProtocolDetector 这个抽象可以的有qos和http两个实现，ProtocolDetector 直接进行enable的set,因为QosWireProtocol 这个是单例的，不同url之间开关会不会有影响？增加url参数也是想动态的进行开关，还有就是如果用户想显式的关闭某种协议就不能单单通过export判断了
| */ | ||
| public interface ProtocolDetector { | ||
| int empty = ' '; | ||
|
|
||
| default Result detect(ChannelBuffer in) { |
There was a problem hiding this comment.
There is no need to change this to default.
…tocol_detector # Conflicts: # dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/pu/QosDetector.java # dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/pu/QosHTTP1Detector.java # dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/ProtocolDetector.java # dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyConfigOperator.java
|
Kudos, SonarCloud Quality Gate passed! |
What is the purpose of the change
add http protocol detector for support http PortUnification
Brief changelog
Verifying this change
@AlbumenJ
Checklist