fix empty protocol in consul registry (#4354)

fixes #4294
apache · Jun 26, 2019 · 6b7d118 · 6b7d118
1 parent e3c35f2
commit 6b7d118
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 21 deletions.
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java b/dubbo-common/src/main/java/org/apache/dubbo/common/beanutil/JavaBeanSerializeUtil.java
@@ -237,12 +237,12 @@ private static void deserializeInternal(Object result, JavaBeanDescriptor beanDe
             for (Map.Entry<Object, Object> entry : beanDescriptor) {
                 Object key = entry.getKey();
                 Object value = entry.getValue();
-                if (key != null && key instanceof JavaBeanDescriptor) {
+                if (key instanceof JavaBeanDescriptor) {
                     JavaBeanDescriptor keyDescriptor = (JavaBeanDescriptor) entry.getKey();
                     key = instantiateForDeserialize(keyDescriptor, loader, cache);
                     deserializeInternal(key, keyDescriptor, loader, cache);
                 }
-                if (value != null && value instanceof JavaBeanDescriptor) {
+                if (value instanceof JavaBeanDescriptor) {
                     JavaBeanDescriptor valueDescriptor = (JavaBeanDescriptor) entry.getValue();
                     value = instantiateForDeserialize(valueDescriptor, loader, cache);
                     deserializeInternal(value, valueDescriptor, loader, cache);

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/json/JSONArray.java b/dubbo-common/src/main/java/org/apache/dubbo/common/json/JSONArray.java
@@ -47,7 +47,7 @@ public Object get(int index) {
      */
     public boolean getBoolean(int index, boolean def) {
         Object tmp = mArray.get(index);
-        return tmp != null && tmp instanceof Boolean ? ((Boolean) tmp).booleanValue() : def;
+        return tmp instanceof Boolean ? ((Boolean) tmp).booleanValue() : def;
     }
 
     /**
@@ -59,7 +59,7 @@ public boolean getBoolean(int index, boolean def) {
      */
     public int getInt(int index, int def) {
         Object tmp = mArray.get(index);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).intValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).intValue() : def;
     }
 
     /**
@@ -71,7 +71,7 @@ public int getInt(int index, int def) {
      */
     public long getLong(int index, long def) {
         Object tmp = mArray.get(index);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).longValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).longValue() : def;
     }
 
     /**
@@ -83,7 +83,7 @@ public long getLong(int index, long def) {
      */
     public float getFloat(int index, float def) {
         Object tmp = mArray.get(index);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).floatValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).floatValue() : def;
     }
 
     /**
@@ -95,7 +95,7 @@ public float getFloat(int index, float def) {
      */
     public double getDouble(int index, double def) {
         Object tmp = mArray.get(index);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).doubleValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).doubleValue() : def;
     }
 
     /**

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/json/JSONObject.java b/dubbo-common/src/main/java/org/apache/dubbo/common/json/JSONObject.java
@@ -47,7 +47,7 @@ public Object get(String key) {
      */
     public boolean getBoolean(String key, boolean def) {
         Object tmp = mMap.get(key);
-        return tmp != null && tmp instanceof Boolean ? (Boolean) tmp : def;
+        return tmp instanceof Boolean ? (Boolean) tmp : def;
     }
 
     /**
@@ -59,7 +59,7 @@ public boolean getBoolean(String key, boolean def) {
      */
     public int getInt(String key, int def) {
         Object tmp = mMap.get(key);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).intValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).intValue() : def;
     }
 
     /**
@@ -71,7 +71,7 @@ public int getInt(String key, int def) {
      */
     public long getLong(String key, long def) {
         Object tmp = mMap.get(key);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).longValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).longValue() : def;
     }
 
     /**
@@ -83,7 +83,7 @@ public long getLong(String key, long def) {
      */
     public float getFloat(String key, float def) {
         Object tmp = mMap.get(key);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).floatValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).floatValue() : def;
     }
 
     /**
@@ -95,7 +95,7 @@ public float getFloat(String key, float def) {
      */
     public double getDouble(String key, double def) {
         Object tmp = mMap.get(key);
-        return tmp != null && tmp instanceof Number ? ((Number) tmp).doubleValue() : def;
+        return tmp instanceof Number ? ((Number) tmp).doubleValue() : def;
     }
 
     /**

diff --git a/dubbo-demo/dubbo-demo-annotation/dubbo-demo-annotation-consumer/pom.xml b/dubbo-demo/dubbo-demo-annotation/dubbo-demo-annotation-consumer/pom.xml
@@ -42,6 +42,14 @@
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-registry-multicast</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-registry-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-configcenter-zookeeper</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-rpc-dubbo</artifactId>

diff --git a/dubbo-demo/dubbo-demo-annotation/dubbo-demo-annotation-provider/pom.xml b/dubbo-demo/dubbo-demo-annotation/dubbo-demo-annotation-provider/pom.xml
@@ -43,6 +43,14 @@
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-registry-multicast</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-registry-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-configcenter-zookeeper</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-rpc-dubbo</artifactId>

diff --git a/dubbo-demo/dubbo-demo-xml/dubbo-demo-xml-consumer/pom.xml b/dubbo-demo/dubbo-demo-xml/dubbo-demo-xml-consumer/pom.xml
@@ -38,6 +38,14 @@
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-registry-multicast</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-registry-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-configcenter-zookeeper</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-config-spring</artifactId>

diff --git a/dubbo-demo/dubbo-demo-xml/dubbo-demo-xml-provider/pom.xml b/dubbo-demo/dubbo-demo-xml/dubbo-demo-xml-provider/pom.xml
@@ -40,6 +40,14 @@
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-registry-multicast</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-registry-zookeeper</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.dubbo</groupId>
+            <artifactId>dubbo-configcenter-zookeeper</artifactId>
+        </dependency>
         <dependency>
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-rpc-dubbo</artifactId>

diff --git a/...-registry-api/src/main/java/org/apache/dubbo/registry/support/ProviderInvokerWrapper.java b/...-registry-api/src/main/java/org/apache/dubbo/registry/support/ProviderInvokerWrapper.java
@@ -90,7 +90,7 @@ public void setReg(boolean reg) {
 
     @Override
     public boolean equals(Object o) {
-        if (o == null || !(o instanceof ProviderInvokerWrapper)) {
+        if (!(o instanceof ProviderInvokerWrapper)) {
             return false;
         }
         ProviderInvokerWrapper other = (ProviderInvokerWrapper) o;

diff --git a/.../dubbo-registry-consul/src/main/java/org/apache/dubbo/registry/consul/ConsulRegistry.java b/.../dubbo-registry-consul/src/main/java/org/apache/dubbo/registry/consul/ConsulRegistry.java
@@ -18,8 +18,10 @@
 package org.apache.dubbo.registry.consul;
 
 import org.apache.dubbo.common.URL;
+import org.apache.dubbo.common.URLBuilder;
 import org.apache.dubbo.common.logger.Logger;
 import org.apache.dubbo.common.logger.LoggerFactory;
+import org.apache.dubbo.common.utils.CollectionUtils;
 import org.apache.dubbo.common.utils.NamedThreadFactory;
 import org.apache.dubbo.registry.NotifyListener;
 import org.apache.dubbo.registry.support.FailbackRegistry;
@@ -49,6 +51,8 @@
 
 import static java.util.concurrent.Executors.newCachedThreadPool;
 import static org.apache.dubbo.common.constants.CommonConstants.ANY_VALUE;
+import static org.apache.dubbo.common.constants.RegistryConstants.CATEGORY_KEY;
+import static org.apache.dubbo.common.constants.RegistryConstants.EMPTY_PROTOCOL;
 import static org.apache.dubbo.registry.Constants.CONSUMER_PROTOCOL;
 import static org.apache.dubbo.registry.Constants.PROVIDER_PROTOCOL;
 
@@ -136,12 +140,12 @@ public void doSubscribe(URL url, NotifyListener listener) {
             Response<Map<String, List<String>>> response = getAllServices(-1, buildWatchTimeout(url));
             index = response.getConsulIndex();
             List<HealthService> services = getHealthServices(response.getValue());
-            urls = convert(services);
+            urls = convert(services, url);
         } else {
             String service = url.getServiceKey();
             Response<List<HealthService>> response = getHealthServices(service, -1, buildWatchTimeout(url));
             index = response.getConsulIndex();
-            urls = convert(response.getValue());
+            urls = convert(response.getValue(), url);
         }
 
         notify(url, listener, urls);
@@ -175,7 +179,7 @@ public List<URL> lookup(URL url) {
             if (result == null || result.getValue() == null || result.getValue().isEmpty()) {
                 return new ArrayList<>();
             } else {
-                return  convert(result.getValue());
+                return convert(result.getValue(), url);
             }
         } catch (Throwable e) {
             throw new RpcException("Failed to lookup " + url + " from consul " + getUrl() + ", cause: " + e.getMessage(), e);
@@ -241,7 +245,10 @@ private boolean isProviderSide(URL url) {
         return url.getProtocol().equals(PROVIDER_PROTOCOL);
     }
 
-    private List<URL> convert(List<HealthService> services) {
+    private List<URL> convert(List<HealthService> services, URL consumerURL) {
+        if (CollectionUtils.isEmpty(services)) {
+            return emptyURL(consumerURL);
+        }
         return services.stream()
                 .map(HealthService::getService)
                 .filter(Objects::nonNull)
@@ -252,6 +259,17 @@ private List<URL> convert(List<HealthService> services) {
                 .collect(Collectors.toList());
     }
 
+    private List<URL> emptyURL(URL consumerURL) {
+        // No Category Parameter
+        URL empty = URLBuilder.from(consumerURL)
+                .setProtocol(EMPTY_PROTOCOL)
+                .removeParameter(CATEGORY_KEY)
+                .build();
+        List<URL> result = new ArrayList<URL>();
+        result.add(empty);
+        return result;
+    }
+
     private NewService buildService(URL url) {
         NewService service = new NewService();
         service.setAddress(url.getHost());
@@ -318,7 +336,7 @@ private void processService() {
             if (currentIndex != null && currentIndex > consulIndex) {
                 consulIndex = currentIndex;
                 List<HealthService> services = response.getValue();
-                List<URL> urls = convert(services);
+                List<URL> urls = convert(services, url);
                 for (NotifyListener listener : getSubscribed().get(url)) {
                     doNotify(url, listener, urls);
                 }
@@ -331,7 +349,7 @@ private void processServices() {
             if (currentIndex != null && currentIndex > consulIndex) {
                 consulIndex = currentIndex;
                 List<HealthService> services = getHealthServices(response.getValue());
-                List<URL> urls = convert(services);
+                List<URL> urls = convert(services, url);
                 for (NotifyListener listener : getSubscribed().get(url)) {
                     doNotify(url, listener, urls);
                 }

diff --git a/...o-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyHelper.java b/...o-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyHelper.java
@@ -27,7 +27,7 @@ final class NettyHelper {
 
     public static void setNettyLoggerFactory() {
         InternalLoggerFactory factory = InternalLoggerFactory.getDefaultFactory();
-        if (factory == null || !(factory instanceof DubboLoggerFactory)) {
+        if (!(factory instanceof DubboLoggerFactory)) {
             InternalLoggerFactory.setDefaultFactory(new DubboLoggerFactory());
         }
     }

diff --git a/...dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/decode/DubboTelnetDecodeTest.java b/...dubbo/src/test/java/org/apache/dubbo/rpc/protocol/dubbo/decode/DubboTelnetDecodeTest.java
@@ -452,7 +452,7 @@ private ByteBuf createDubboByteBuf() throws IOException {
     }
 
     private static boolean checkTelnetDecoded(Object msg) {
-        if (msg != null && msg instanceof String && !msg.toString().contains("Unsupported command:")) {
+        if (msg instanceof String && !msg.toString().contains("Unsupported command:")) {
             return true;
         }
         return false;