fix router match condition (#12491)

* fix router match condition * fix ut compilation * add null check
apache · Jun 12, 2023 · dd94c15 · dd94c15
1 parent bde608e
commit dd94c15
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 28 deletions.
diff --git a/...cluster/src/main/java/org/apache/dubbo/rpc/cluster/configurator/AbstractConfigurator.java b/...cluster/src/main/java/org/apache/dubbo/rpc/cluster/configurator/AbstractConfigurator.java
@@ -128,7 +128,7 @@ private URL configureIfMatch(String host, URL url) {
                 if (apiVersion != null && apiVersion.startsWith(RULE_VERSION_V30)) {
                     ConditionMatch matcher = (ConditionMatch) configuratorUrl.getAttribute(MATCH_CONDITION);
                     if (matcher != null) {
-                        if (matcher.isMatch(url)) {
+                        if (matcher.isMatch(host, url)) {
                             return doConfigure(url, configuratorUrl.removeParameters(conditionKeys));
                         } else {
                             logger.debug("Cannot apply configurator rule, param mismatch, current params are " + url + ", params in rule is " + matcher);

diff --git a/.../src/main/java/org/apache/dubbo/rpc/cluster/configurator/parser/model/ConditionMatch.java b/.../src/main/java/org/apache/dubbo/rpc/cluster/configurator/parser/model/ConditionMatch.java
@@ -26,6 +26,7 @@
 
 public class ConditionMatch {
     private AddressMatch address;
+    private AddressMatch providerAddress;
     private ListStringMatch service;
     private ListStringMatch app;
     private List<ParamMatch> param;
@@ -38,6 +39,14 @@ public void setAddress(AddressMatch address) {
         this.address = address;
     }
 
+    public AddressMatch getProviderAddress() {
+        return providerAddress;
+    }
+
+    public void setProviderAddress(AddressMatch providerAddress) {
+        this.providerAddress = providerAddress;
+    }
+
     public ListStringMatch getService() {
         return service;
     }
@@ -62,8 +71,12 @@ public void setParam(List<ParamMatch> param) {
         this.param = param;
     }
 
-    public boolean isMatch(URL url) {
-        if (getAddress() != null && !getAddress().isMatch(url.getAddress())) {
+    public boolean isMatch(String host, URL url) {
+        if (getAddress() != null && !getAddress().isMatch(host)) {
+            return false;
+        }
+
+        if (getProviderAddress() != null && !getProviderAddress().isMatch(url.getAddress())) {
             return false;
         }
 
@@ -90,6 +103,7 @@ public boolean isMatch(URL url) {
     public String toString() {
         return "ConditionMatch{" +
             "address='" + address + '\'' +
+            "providerAddress='" + providerAddress + '\'' +
             ", service='" + service + '\'' +
             ", app='" + app + '\'' +
             ", param='" + param + '\'' +

diff --git a/...ster/src/main/java/org/apache/dubbo/rpc/cluster/configurator/parser/model/ParamMatch.java b/...ster/src/main/java/org/apache/dubbo/rpc/cluster/configurator/parser/model/ParamMatch.java
@@ -40,12 +40,12 @@ public void setValue(StringMatch value) {
     }
 
     public boolean isMatch(URL url) {
-        if (key == null) {
+        if (key == null || value == null) {
             return false;
         }
 
         String input = url.getParameter(key);
-        return input != null && value.isMatch(input);
+        return value.isMatch(input);
     }
 
     @Override

diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/model/ParamMatch.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/model/ParamMatch.java
@@ -39,7 +39,7 @@ public void setValue(StringMatch value) {
     }
 
     public boolean isMatch(String input) {
-        if (getValue() != null && input != null) {
+        if (getValue() != null) {
             return getValue().isMatch(input);
         }
         return false;

diff --git a/...ster/src/test/java/org/apache/dubbo/rpc/cluster/configurator/parser/ConfigParserTest.java b/...ster/src/test/java/org/apache/dubbo/rpc/cluster/configurator/parser/ConfigParserTest.java
@@ -187,11 +187,11 @@ void parseProviderConfigurationV3() throws IOException {
             URL notMatchURL3 = URL.valueOf("dubbo://10.0.0.1:20880/DemoService?match_key1=value_not_match");// key not match
 
             ConditionMatch matcher = (ConditionMatch) url.getAttribute(MATCH_CONDITION);
-            Assertions.assertTrue(matcher.isMatch(matchURL1));
-            Assertions.assertTrue(matcher.isMatch(matchURL2));
-            Assertions.assertFalse(matcher.isMatch(notMatchURL1));
-            Assertions.assertFalse(matcher.isMatch(notMatchURL2));
-            Assertions.assertFalse(matcher.isMatch(notMatchURL3));
+            Assertions.assertTrue(matcher.isMatch(matchURL1.getAddress(), matchURL1));
+            Assertions.assertTrue(matcher.isMatch(matchURL2.getAddress(), matchURL2));
+            Assertions.assertFalse(matcher.isMatch(notMatchURL1.getAddress(), notMatchURL1));
+            Assertions.assertFalse(matcher.isMatch(notMatchURL2.getAddress(), notMatchURL2));
+            Assertions.assertFalse(matcher.isMatch(notMatchURL3.getAddress(), notMatchURL3));
         }
     }
 
@@ -211,8 +211,8 @@ void parseProviderConfigurationV3Compatibility() throws IOException {
             URL notMatchURL = URL.valueOf("dubbo://10.0.0.1:20880/DemoService?match_key1=value_not_match");// key not match
 
             ConditionMatch matcher = (ConditionMatch) url.getAttribute(MATCH_CONDITION);
-            Assertions.assertTrue(matcher.isMatch(matchURL));
-            Assertions.assertFalse(matcher.isMatch(notMatchURL));
+            Assertions.assertTrue(matcher.isMatch(matchURL.getAddress(), matchURL));
+            Assertions.assertFalse(matcher.isMatch(notMatchURL.getAddress(), notMatchURL));
         }
     }
 
@@ -232,8 +232,8 @@ void parseProviderConfigurationV3Conflict() throws IOException {
             URL notMatchURL = URL.valueOf("dubbo://10.0.0.1:20880/DemoService?match_key1=value_not_match");// key not match
 
             ConditionMatch matcher = (ConditionMatch) url.getAttribute(MATCH_CONDITION);
-            Assertions.assertTrue(matcher.isMatch(matchURL));
-            Assertions.assertFalse(matcher.isMatch(notMatchURL));
+            Assertions.assertTrue(matcher.isMatch(matchURL.getAddress(), matchURL));
+            Assertions.assertFalse(matcher.isMatch(notMatchURL.getAddress(), notMatchURL));
         }
     }
 

diff --git a/...ubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/AbstractServiceNameMapping.java b/...ubbo-metadata-api/src/main/java/org/apache/dubbo/metadata/AbstractServiceNameMapping.java
@@ -124,18 +124,20 @@ public Set<String> getAndListen(URL registryURL, URL subscribedURL, MappingListe
     @Override
     public MappingListener stopListen(URL subscribeURL, MappingListener listener) {
         synchronized (mappingListeners) {
-            String mappingKey = ServiceNameMapping.buildMappingKey(subscribeURL);
-            Set<MappingListener> listeners = mappingListeners.get(mappingKey);
-            //todo, remove listener from remote metadata center
-            if (CollectionUtils.isNotEmpty(listeners)) {
-                listeners.remove(listener);
-                listener.stop();
-                removeListener(subscribeURL, listener);
-            }
-            if (CollectionUtils.isEmpty(listeners)) {
-                mappingListeners.remove(mappingKey);
-                removeCachedMapping(mappingKey);
-                removeMappingLock(mappingKey);
+            if (listener != null) {
+                String mappingKey = ServiceNameMapping.buildMappingKey(subscribeURL);
+                Set<MappingListener> listeners = mappingListeners.get(mappingKey);
+                //todo, remove listener from remote metadata center
+                if (CollectionUtils.isNotEmpty(listeners)) {
+                    listeners.remove(listener);
+                    listener.stop();
+                    removeListener(subscribeURL, listener);
+                }
+                if (CollectionUtils.isEmpty(listeners)) {
+                    mappingListeners.remove(mappingKey);
+                    removeCachedMapping(mappingKey);
+                    removeMappingLock(mappingKey);
+                }
             }
             return listener;
         }

diff --git a/...registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java b/...registry-api/src/main/java/org/apache/dubbo/registry/client/ServiceDiscoveryRegistry.java
@@ -248,7 +248,9 @@ public void doUnsubscribe(URL url, NotifyListener listener) {
         serviceDiscovery.unsubscribe(url, listener);
         String protocolServiceKey = url.getProtocolServiceKey();
         Set<String> serviceNames = serviceNameMapping.getMapping(url);
-        serviceNameMapping.stopListen(url, mappingListeners.remove(protocolServiceKey));
+        if (mappingListeners.get(protocolServiceKey) != null) {
+            serviceNameMapping.stopListen(url, mappingListeners.remove(protocolServiceKey));
+        }
         if (CollectionUtils.isNotEmpty(serviceNames)) {
             String serviceNamesKey = toStringKeys(serviceNames);
             Lock appSubscriptionLock = getAppSubscription(serviceNamesKey);