[SCB-2064]Fix RegistryUtils API not using version rule parameter problem (#1916)

liubao68 · web-flow · commit e84f4c8c1b94 · 2020-08-12T08:58:36.000+08:00
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/RegistryUtils.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/RegistryUtils.java
@@ -154,7 +154,9 @@ public static List<MicroserviceInstance> findServiceInstance(String appId, Strin
       String versionRule) {
     MicroserviceCache serviceCache = aggregateServiceRegistryCache.findServiceCache(
         MicroserviceCacheKey.builder()
-            .appId(appId).serviceName(serviceName).env(getMicroservice().getEnvironment())
+            .appId(appId).serviceName(serviceName)
+            .env(getMicroservice().getEnvironment())
+            .versionRule(versionRule)
             .build()
     );
     return MicroserviceCacheStatus.SERVICE_NOT_FOUND.equals(serviceCache.getStatus()) ?
@@ -176,9 +178,13 @@ public static Microservice getMicroservice(String microserviceId) {
   }
 
   public static MicroserviceInstances findServiceInstances(String appId, String serviceName,
-      String versionRule, String revision) {
+      String versionRule) {
     MicroserviceCache serviceCache = aggregateServiceRegistryCache.findServiceCache(
-        MicroserviceCacheKey.builder().appId(appId).serviceName(serviceName).env(getMicroservice().getEnvironment())
+        MicroserviceCacheKey.builder()
+            .appId(appId)
+            .serviceName(serviceName)
+            .env(getMicroservice().getEnvironment())
+            .versionRule(versionRule)
             .build());
     return convertCacheToMicroserviceInstances(serviceCache);
   }
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/ServiceCenterDiscovery.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/ServiceCenterDiscovery.java
@@ -79,8 +79,7 @@ public MicroserviceInstance getMicroserviceInstance(String serviceId, String ins
   public MicroserviceInstances findServiceInstances(String appId, String serviceName, String versionRule) {
     return RegistryUtils.findServiceInstances(appId,
         serviceName,
-        versionRule,
-        revision);
+        versionRule);
   }
 
   @Override
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/ServiceRegistry.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/ServiceRegistry.java
@@ -77,7 +77,7 @@ List<MicroserviceInstance> findServiceInstance(String appId, String microservice
       String microserviceVersionRule);
 
   MicroserviceInstances findServiceInstances(String appId, String microserviceName,
-      String microserviceVersionRule, String revision);
+      String microserviceVersionRule);
 
   MicroserviceCache findMicroserviceCache(MicroserviceCacheKey microserviceCacheKey);
 
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/InstanceCacheChecker.java
@@ -87,8 +87,7 @@ protected InstanceCacheResult check(MicroserviceVersions microserviceVersions) {
     MicroserviceInstances microserviceInstances = RegistryUtils
         .findServiceInstances(microserviceVersions.getAppId(),
             microserviceVersions.getMicroserviceName(),
-            DefinitionConst.VERSION_RULE_ALL,
-            null);
+            DefinitionConst.VERSION_RULE_ALL);
     if (microserviceInstances == null) {
       instanceCacheResult.setStatus(Status.UNKNOWN);
       instanceCacheResult.setDetail("failed to find instances from service center");
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/registry/AbstractServiceRegistry.java
@@ -19,8 +19,6 @@
 import static org.apache.servicecomb.foundation.common.base.ServiceCombConstants.CONFIG_DEFAULT_REGISTER_BY;
 import static org.apache.servicecomb.foundation.common.base.ServiceCombConstants.CONFIG_FRAMEWORK_DEFAULT_NAME;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -40,10 +38,7 @@
 import org.apache.servicecomb.registry.api.registry.MicroserviceFactory;
 import org.apache.servicecomb.registry.api.registry.MicroserviceInstance;
 import org.apache.servicecomb.registry.api.registry.MicroserviceInstances;
-import org.apache.servicecomb.registry.consumer.MicroserviceManager;
-import org.apache.servicecomb.registry.consumer.StaticMicroserviceVersions;
 import org.apache.servicecomb.registry.definition.MicroserviceDefinition;
-import org.apache.servicecomb.registry.definition.MicroserviceNameParser;
 import org.apache.servicecomb.serviceregistry.RegistryUtils;
 import org.apache.servicecomb.serviceregistry.ServiceRegistry;
 import org.apache.servicecomb.serviceregistry.api.Const;
@@ -196,20 +191,25 @@ public boolean unregisterInstance() {
     return true;
   }
 
+  @Override
   public List<MicroserviceInstance> findServiceInstance(String appId, String serviceName,
       String versionRule) {
-    MicroserviceInstances instances = findServiceInstances(appId, serviceName, versionRule, null);
+    MicroserviceInstances instances = findServiceInstances(appId, serviceName, versionRule);
     if (instances == null || instances.isMicroserviceNotExist()) {
       return null;
     }
     return instances.getInstancesResponse().getInstances();
   }
 
+  @Override
   public MicroserviceInstances findServiceInstances(String appId, String serviceName,
-      String versionRule, String revision) {
+      String versionRule) {
     MicroserviceCache microserviceCache = serviceRegistryCache
         .findServiceCache(MicroserviceCacheKey.builder()
-            .serviceName(serviceName).appId(appId).env(microservice.getEnvironment()).build());
+            .serviceName(serviceName).appId(appId)
+            .env(microservice.getEnvironment())
+            .versionRule(versionRule)
+            .build());
     return RegistryUtils.convertCacheToMicroserviceInstances(microserviceCache);
   }
 
@@ -228,6 +228,7 @@ public boolean updateMicroserviceProperties(Map<String, String> properties) {
     return success;
   }
 
+  @Override
   public boolean updateInstanceProperties(Map<String, String> instanceProperties) {
     MicroserviceInstance microserviceInstance = microservice.getInstance();
     boolean success = srClient.updateInstanceProperties(microserviceInstance.getServiceId(),
diff --git a/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/registry/cache/MicroserviceCacheKey.java b/service-registry/registry-service-center/src/main/java/org/apache/servicecomb/serviceregistry/registry/cache/MicroserviceCacheKey.java
@@ -29,7 +29,7 @@ public class MicroserviceCacheKey {
 
   private String serviceName;
 
-  private static final String VERSION_RULE = DefinitionConst.VERSION_RULE_ALL;
+  private String versionRule = DefinitionConst.VERSION_RULE_ALL;
 
   public static MicroserviceCacheKeyBuilder builder() {
     return new MicroserviceCacheKeyBuilder();
@@ -42,6 +42,7 @@ public void validate() {
     Objects.requireNonNull(this.env, "microserviceCacheKey.env is null");
     Objects.requireNonNull(this.appId, "microserviceCacheKey.appId is null");
     Objects.requireNonNull(this.serviceName, "microserviceCacheKey.serviceName is null");
+    Objects.requireNonNull(this.versionRule, "microserviceCacheKey.versionRule is null");
   }
 
   public String getEnv() {
@@ -57,11 +58,11 @@ public String getServiceName() {
   }
 
   public String getVersionRule() {
-    return VERSION_RULE;
+    return versionRule;
   }
 
   public String plainKey() {
-    return serviceName + "@" + appId + "@" + env;
+    return serviceName + "@" + appId + "@" + env + "@" + versionRule;
   }
 
   @Override
@@ -75,12 +76,13 @@ public boolean equals(Object o) {
     MicroserviceCacheKey that = (MicroserviceCacheKey) o;
     return Objects.equals(env, that.env) &&
         Objects.equals(appId, that.appId) &&
-        Objects.equals(serviceName, that.serviceName);
+        Objects.equals(serviceName, that.serviceName) &&
+        Objects.equals(versionRule, that.versionRule);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(env, appId, serviceName);
+    return Objects.hash(env, appId, serviceName, versionRule);
   }
 
   @Override
@@ -115,6 +117,11 @@ public MicroserviceCacheKeyBuilder serviceName(String serviceName) {
       return this;
     }
 
+    public MicroserviceCacheKeyBuilder versionRule(String versionRule) {
+      microserviceCacheKey.versionRule = versionRule;
+      return this;
+    }
+
     MicroserviceCacheKeyBuilder() {
       microserviceCacheKey = new MicroserviceCacheKey();
     }
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/TestRegistry.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/TestRegistry.java
@@ -102,7 +102,7 @@ public void testDelegate() {
     Assert.assertNull(instanceList);
 
     MicroserviceInstances microserviceInstances =
-        RegistryUtils.findServiceInstances("default", "default", "0.0.1", "0");
+        RegistryUtils.findServiceInstances("default", "default", "0.0.1");
     List<MicroserviceInstance> instanceLists = microserviceInstances.getInstancesResponse().getInstances();
     Assert.assertEquals(1, instanceLists.size());
     Assert.assertEquals(RegistryUtils.getMicroservice().getServiceId(), instanceLists.get(0).getServiceId());
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheCheckerMock.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/diagnosis/instance/TestInstanceCacheCheckerMock.java
@@ -104,7 +104,7 @@ public void check_findInstances_failed() {
     new MockUp<RegistryUtils>() {
       @Mock
       MicroserviceInstances findServiceInstances(String appId, String serviceName,
-          String versionRule, String revision) {
+          String versionRule) {
         return findHolder.value;
       }
     };
@@ -141,7 +141,7 @@ public void check_findInstances_serviceNotExist() {
     new MockUp<RegistryUtils>() {
       @Mock
       MicroserviceInstances findServiceInstances(String appId, String serviceName,
-          String versionRule, String revision) {
+          String versionRule) {
         return findHolder.value;
       }
     };
@@ -178,7 +178,7 @@ public void check_findInstances_revisionNotMatch() {
     new MockUp<RegistryUtils>() {
       @Mock
       MicroserviceInstances findServiceInstances(String appId, String serviceName,
-          String versionRule, String revision) {
+          String versionRule) {
         return findHolder.value;
       }
     };
@@ -211,7 +211,7 @@ public void check_findInstances_cacheNotMatch() {
     new MockUp<RegistryUtils>() {
       @Mock
       MicroserviceInstances findServiceInstances(String appId, String serviceName,
-          String versionRule, String revision) {
+          String versionRule) {
         return findHolder.value;
       }
     };
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/registry/EmptyMockServiceRegistry.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/registry/EmptyMockServiceRegistry.java
@@ -95,7 +95,7 @@ public List<MicroserviceInstance> findServiceInstance(String appId, String micro
 
   @Override
   public MicroserviceInstances findServiceInstances(String appId, String microserviceName,
-      String microserviceVersionRule, String revision) {
+      String microserviceVersionRule) {
     return null;
   }
 
diff --git a/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/registry/cache/MicroserviceCacheKeyTest.java b/service-registry/registry-service-center/src/test/java/org/apache/servicecomb/serviceregistry/registry/cache/MicroserviceCacheKeyTest.java
@@ -35,7 +35,7 @@ public void constructors() {
     Assert.assertEquals("svc", microserviceCacheKey.getServiceName());
     Assert.assertEquals("app", microserviceCacheKey.getAppId());
     Assert.assertEquals("env", microserviceCacheKey.getEnv());
-    Assert.assertEquals("svc@app@env", microserviceCacheKey.toString());
+    Assert.assertEquals("svc@app@env@0.0.0.0+", microserviceCacheKey.toString());
 
     microserviceCacheKey =
         MicroserviceCacheKey.builder().serviceName("app:svc").appId("app").env("env").build();
@@ -83,6 +83,6 @@ public void equals_and_hashcode() {
   public void plainKey() {
     MicroserviceCacheKey microserviceCacheKey =
         MicroserviceCacheKey.builder().env("env").appId("app").serviceName("svc").build();
-    Assert.assertEquals("svc@app@env", microserviceCacheKey.plainKey());
+    Assert.assertEquals("svc@app@env@0.0.0.0+", microserviceCacheKey.plainKey());
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -79,8 +79,7 @@ public MicroserviceInstance getMicroserviceInstance(String serviceId, String ins`
`79`	`79`	`public MicroserviceInstances findServiceInstances(String appId, String serviceName, String versionRule) {`
`80`	`80`	`return RegistryUtils.findServiceInstances(appId,`
`81`	`81`	`serviceName,`
`82`		`- versionRule,`
`83`		`- revision);`
	`82`	`+ versionRule);`
`84`	`83`	`}`
`85`	`84`
`86`	`85`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -95,7 +95,7 @@ public List<MicroserviceInstance> findServiceInstance(String appId, String micro`
`95`	`95`
`96`	`96`	`@Override`
`97`	`97`	`public MicroserviceInstances findServiceInstances(String appId, String microserviceName,`
`98`		`- String microserviceVersionRule, String revision) {`
	`98`	`+ String microserviceVersionRule) {`
`99`	`99`	`return null;`
`100`	`100`	`}`
`101`	`101`