CrazyHZM commented on code in PR #10602:
URL: https://github.com/apache/dubbo/pull/10602#discussion_r969217405
##########
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java:
##########
@@ -580,6 +581,24 @@ private void exportUrl(URL url, List<URL> registryURLs) {
// export to remote if the config is not local (export to local
only when config is local)
if (!SCOPE_LOCAL.equalsIgnoreCase(scope)) {
+ // export to extra protocol is used in remote export
+ String extProtocol = url.getParameter("ext.protocol", "");
Review Comment:
Where do you get the value of `ext.protocol` from?
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/WireProtocol.java:
##########
@@ -34,4 +34,6 @@ public interface WireProtocol {
void configClientPipeline(URL url, ChannelPipeline pipeline, SslContext
sslContext);
void close();
+
+ String protocolName();
Review Comment:
`ExtensionLoader.getExtensionName()` can solve this problem.
##########
dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DubboProtocol.java:
##########
@@ -364,6 +367,9 @@ private ProtocolServer createServer(URL url) {
// enable heartbeat by default
.addParameterIfAbsent(HEARTBEAT_KEY,
String.valueOf(DEFAULT_HEARTBEAT))
.addParameter(CODEC_KEY, DubboCodec.NAME)
+ //todo
+ // enable pu server by default, this config should be closed when
merging
+ .addParameter(IS_PU_SERVER_KEY, Boolean.TRUE.toString())
Review Comment:
Why is it enabled by default for now?
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java:
##########
@@ -23,10 +23,16 @@
import org.apache.dubbo.remoting.transport.AbstractServer;
import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
public abstract class AbstractPortUnificationServer extends AbstractServer {
private final List<WireProtocol> protocols;
+ private final Map<String, URL> urlMapper = new ConcurrentHashMap<>();
+
+ private final Map<String, ChannelHandler> handlerMapper = new
ConcurrentHashMap<>();
Review Comment:
Naming needs to be enhanced for readability, such as `supportedUrls`, and be
annotated to describe the use.
##########
dubbo-common/src/main/java/org/apache/dubbo/config/ProtocolConfig.java:
##########
@@ -200,6 +200,11 @@ public class ProtocolConfig extends AbstractConfig {
private Boolean sslEnabled;
+ /*
+ * Extra Protocol for this service, using Port Unification Server
+ */
+ private String extProtocol;
Review Comment:
Should support multiple protocols.
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java:
##########
@@ -36,4 +42,18 @@ public List<WireProtocol> getProtocols() {
return protocols;
}
+ public void addNewURL(URL url, ChannelHandler handler) {
+ this.urlMapper.put(url.getProtocol(), url);
+ this.handlerMapper.put(url.getProtocol(), handler);
+ }
+
+ protected Map<String, URL> getUrlMapper() {
+ // this getter is just used by implementation of this class
+ return urlMapper;
+ }
+
+ public Map<String, ChannelHandler> getHandlerMapper() {
+ // this getter is just used by implementation of this class
+ return handlerMapper;
+ }
Review Comment:
Naming needs to be enhanced for readability.
--
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]
