You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2018/12/04 10:50:13 UTC

[servicecomb-java-chassis] branch master updated (f4b9d7b -> e0e8df7)

This is an automated email from the ASF dual-hosted git repository.

liubao pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git.


    from f4b9d7b  [SCB-1063]revert concurrent change which may cause compile problem
     new 5375d84  [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx
     new b2059c0  [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: fix as reviewed
     new e0e8df7  [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: modify details

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../consumer/TestConsumerProviderManager.java      |  4 +-
 .../edge/core/TestCompatiblePathVersionMapper.java |  4 +-
 .../edge/core/TestDefaultEdgeDispatcher.java       |  2 +-
 .../api/registry/MicroserviceFactory.java          |  8 ++-
 .../serviceregistry/version/Version.java           | 26 ++++++---
 .../api/registry/TestMicroserviceFactory.java      | 28 +++++++++
 .../consumer/StaticMicroserviceVersionsTest.java   |  2 +-
 .../serviceregistry/consumer/TestAppManager.java   |  2 +-
 .../TestDefaultMicroserviceVersionFactory.java     |  2 +-
 .../consumer/TestMicroserviceManager.java          |  4 +-
 .../consumer/TestMicroserviceVersion.java          |  2 +-
 .../consumer/TestMicroserviceVersionRule.java      |  2 +-
 .../registry/TestRemoteServiceRegistry.java        | 17 +++++-
 .../serviceregistry/version/TestVersion.java       | 68 ++++++++++++++++------
 .../version/TestVersionRuleFixedParser.java        |  2 +-
 .../version/TestVersionRuleRangeParser.java        |  2 +-
 .../version/TestVersionRuleStartFromParser.java    |  2 +-
 .../serviceregistry/version/TestVersionUtils.java  |  2 +-
 .../serviceregistry/version/VersionConst.java      |  4 +-
 19 files changed, 135 insertions(+), 48 deletions(-)


[servicecomb-java-chassis] 01/03: [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx

Posted by li...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit 5375d840f4c10b3d3c3be9878217621311ac7187
Author: heyile <25...@qq.com>
AuthorDate: Fri Nov 30 14:32:25 2018 +0800

    [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx
---
 .../api/registry/MicroserviceFactory.java          |  8 +-
 .../serviceregistry/version/Version.java           | 42 ++++++++--
 .../serviceregistry/version/VersionRuleUtils.java  | 12 ++-
 .../api/registry/TestMicroserviceFactory.java      | 28 +++++++
 .../registry/TestRemoteServiceRegistry.java        |  8 ++
 .../serviceregistry/version/TestVersion.java       | 93 ++++++++++++++++++----
 .../serviceregistry/version/VersionConst.java      |  4 +-
 7 files changed, 167 insertions(+), 28 deletions(-)

diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
index 97b2df4..44b1917 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
@@ -34,6 +34,7 @@ import org.apache.commons.configuration.Configuration;
 import org.apache.servicecomb.serviceregistry.config.ConfigurePropertyUtils;
 import org.apache.servicecomb.serviceregistry.config.MicroservicePropertiesLoader;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
+import org.apache.servicecomb.serviceregistry.version.VersionRuleUtils;
 
 public class MicroserviceFactory {
   public Microservice create(String appId, String microserviceName) {
@@ -53,8 +54,11 @@ public class MicroserviceFactory {
     microservice.setServiceName(configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_NAME_KEY,
         DEFAULT_MICROSERVICE_NAME));
     microservice.setAppId(configuration.getString(CONFIG_APPLICATION_ID_KEY, DEFAULT_APPLICATION_ID));
-    microservice.setVersion(configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
-        DEFAULT_MICROSERVICE_VERSION));
+    String version = configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
+        DEFAULT_MICROSERVICE_VERSION);
+    //check version format
+    VersionRuleUtils.checkVersionFormat(version);
+    microservice.setVersion(version);
     setDescription(configuration, microservice);
     microservice.setLevel(configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_ROLE_KEY, "FRONT"));
     microservice.setPaths(ConfigurePropertyUtils.getMicroservicePaths(configuration));
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
index 6e5a954..69b8df0 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
@@ -23,7 +23,7 @@ import org.apache.commons.lang.ArrayUtils;
 
 // short version is enough
 public class Version implements Comparable<Version> {
-  private static final String[] ZERO = new String[] {"0", "0", "0"};
+  private static final String[] ZERO = new String[] {"0", "0", "0", "0"};
 
   private final short major;
 
@@ -31,6 +31,10 @@ public class Version implements Comparable<Version> {
 
   private final short patch;
 
+  private final short build;
+
+  private final boolean versionFour;
+
   private final String version;
 
   private final long numberVersion;
@@ -38,20 +42,25 @@ public class Version implements Comparable<Version> {
   // 1
   // 1.0
   // 1.0.0
+  // 1.0.0.0
   public Version(String version) {
     Objects.requireNonNull(version);
 
     String[] versions = version.split("\\.", -1);
-    if (versions.length > 3) {
+    if (versions.length > 4) {
       throw new IllegalStateException(String.format("Invalid version \"%s\".", version));
     }
 
-    if (versions.length < 3) {
+    if (versions.length == 4) {
+      versionFour = true;
+    } else {
+      versionFour = false;
       versions = (String[]) ArrayUtils.addAll(versions, ZERO);
     }
     this.major = parseNumber("major", version, versions[0]);
     this.minor = parseNumber("minor", version, versions[1]);
     this.patch = parseNumber("patch", version, versions[2]);
+    this.build = parseNumber("build", version, versions[3]);
 
     this.version = combineStringVersion();
     this.numberVersion = combineVersion();
@@ -74,20 +83,33 @@ public class Version implements Comparable<Version> {
     return value;
   }
 
-  public Version(short major, short minor, short patch) {
+  public Version(short major, short minor, short patch, short build, boolean versionFour) {
     this.major = major;
     this.minor = minor;
     this.patch = patch;
+    this.build = build;
+    this.versionFour = versionFour;
     this.version = combineStringVersion();
     this.numberVersion = combineVersion();
   }
 
   private String combineStringVersion() {
-    return major + "." + minor + "." + patch;
+    StringBuilder stringBuilder = new StringBuilder();
+    stringBuilder.append(major)
+        .append(".")
+        .append(minor)
+        .append(".")
+        .append(patch);
+    if (versionFour) {
+      stringBuilder.append(".")
+          .append(build);
+    }
+    return stringBuilder.toString();
   }
 
+  // 1.0.0 equals 1.0.0.0
   private long combineVersion() {
-    return (long) major << 32 | (long) minor << 16 | (long) patch;
+    return (long) major << 48 | (long) minor << 32 | (long) patch << 16 | (long) build;
   }
 
   public short getMajor() {
@@ -102,6 +124,14 @@ public class Version implements Comparable<Version> {
     return patch;
   }
 
+  public short getBuild() {
+    return build;
+  }
+
+  public boolean isVersionFour() {
+    return versionFour;
+  }
+
   public String getVersion() {
     return version;
   }
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
index f8d512e..5edb5af 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
@@ -54,7 +54,17 @@ public final class VersionRuleUtils {
         return versionRule;
       }
     }
+    throw new IllegalStateException("config service_description.version is invalid ");
+  }
 
-    throw new IllegalStateException("never run to here");
+  public static void checkVersionFormat(String strVersionRule) {
+    strVersionRule = strVersionRule.trim();
+    for (VersionRuleParser parser : parsers) {
+      VersionRule versionRule = parser.parse(strVersionRule);
+      if (versionRule != null) {
+        return;
+      }
+    }
+    throw new IllegalStateException("config service_description.version is invalid ");
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
index d3f69b0..508833a 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
@@ -18,7 +18,9 @@
 package org.apache.servicecomb.serviceregistry.api.registry;
 
 import static org.apache.servicecomb.foundation.common.base.ServiceCombConstants.CONFIG_QUALIFIED_MICROSERVICE_DESCRIPTION_KEY;
+import static org.apache.servicecomb.foundation.common.base.ServiceCombConstants.CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY;
 import static org.apache.servicecomb.serviceregistry.definition.DefinitionConst.CONFIG_ALLOW_CROSS_APP_KEY;
+import static org.apache.servicecomb.serviceregistry.definition.DefinitionConst.DEFAULT_MICROSERVICE_VERSION;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -27,12 +29,19 @@ import org.apache.commons.configuration.Configuration;
 import org.apache.servicecomb.config.archaius.sources.MicroserviceConfigLoader;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 import org.mockito.Mockito;
 
 import mockit.Deencapsulation;
+import mockit.Expectations;
+import mockit.Mocked;
 
 public class TestMicroserviceFactory {
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
   @Test
   public void testAllowCrossApp() {
     MicroserviceFactory factory = new MicroserviceFactory();
@@ -125,4 +134,23 @@ public class TestMicroserviceFactory {
 
     Assert.assertEquals(" , ", microservice.getDescription());
   }
+
+  @Test
+  public void testCreateMicroserviceFromDefinitionWithInvalidVersion(@Mocked Configuration configuration,
+      @Mocked MicroserviceDefinition microserviceDefinition) {
+
+    new Expectations() {
+      {
+        configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
+            DEFAULT_MICROSERVICE_VERSION);
+        result = "x.y.x.1";
+        microserviceDefinition.getConfiguration();
+        result = configuration;
+      }
+    };
+    expectedException.equals(IllegalStateException.class);
+    expectedException.expectMessage("Invalid major \"x\", version \"x.y.x.1\"");
+    MicroserviceFactory microserviceFactory = new MicroserviceFactory();
+    microserviceFactory.create(microserviceDefinition);
+  }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
index 73483fa..3ac72c6 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
@@ -35,6 +35,7 @@ import org.apache.servicecomb.serviceregistry.consumer.MicroserviceVersions;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
 import org.apache.servicecomb.serviceregistry.task.event.PullMicroserviceVersionsInstancesEvent;
 import org.apache.servicecomb.serviceregistry.task.event.ShutdownEvent;
+import org.apache.servicecomb.serviceregistry.version.VersionRuleUtils;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Rule;
@@ -141,6 +142,13 @@ public class TestRemoteServiceRegistry {
       }
     }.getMockInstance();
 
+    new MockUp<VersionRuleUtils>() {
+      @Mock
+      void checkVersionFormat(String strVersionRule) {
+        return;
+      }
+    };
+
     expectedException.expect(Error.class);
     expectedException.expectMessage(Matchers.is("ok"));
 
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
index 2952d3e..c79b30c 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
@@ -30,6 +30,8 @@ public class TestVersion {
 
   short s2 = 2;
 
+  short s0 = 0;
+
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
@@ -40,6 +42,8 @@ public class TestVersion {
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(0, version.getMinor());
     Assert.assertEquals(0, version.getPatch());
+    Assert.assertEquals(0, version.getBuild());
+    Assert.assertFalse(version.isVersionFour());
   }
 
   @Test
@@ -49,15 +53,30 @@ public class TestVersion {
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(0, version.getPatch());
+    Assert.assertEquals(0, version.getBuild());
+    Assert.assertFalse(version.isVersionFour());
   }
 
   @Test
-  public void constructFromStringNormalAll() {
+  public void constructFromStringOnlyMajorMinorPatch() {
     version = new Version("1.1.1");
     Assert.assertEquals("1.1.1", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(1, version.getPatch());
+    Assert.assertEquals(0, version.getBuild());
+    Assert.assertFalse(version.isVersionFour());
+  }
+
+  @Test
+  public void constructFromStringNormal() {
+    version = new Version("1.1.1.1");
+    Assert.assertEquals("1.1.1.1", version.getVersion());
+    Assert.assertEquals(1, version.getMajor());
+    Assert.assertEquals(1, version.getMinor());
+    Assert.assertEquals(1, version.getPatch());
+    Assert.assertEquals(1, version.getBuild());
+    Assert.assertTrue(version.isVersionFour());
   }
 
   @Test
@@ -119,54 +138,94 @@ public class TestVersion {
   }
 
   @Test
-  public void constructFromStringInvalidTooManyPart() {
+  public void constructFromStringInvalidPatchDot() {
     expectedException.expect(IllegalStateException.class);
-    expectedException.expectMessage(Matchers.is("Invalid version \"1.1.1.\"."));
+    expectedException.expectMessage(Matchers.is("Invalid build \"\", version \"1.1.1.\"."));
+    expectedException.expectCause(Matchers.instanceOf(NumberFormatException.class));
 
     version = new Version("1.1.1.");
   }
 
   @Test
+  public void constructFromStringInvalidBuildNegative() {
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectMessage(Matchers.is("build \"-1\" can not be negative, version \"1.1.1.-1\"."));
+
+    version = new Version("1.1.1.-1");
+  }
+
+  @Test
+  public void constructFromStringInvalidTooManyPart() {
+    expectedException.expect(IllegalStateException.class);
+    expectedException.expectMessage(Matchers.is("Invalid version \"1.1.1.1.\"."));
+
+    version = new Version("1.1.1.1.");
+  }
+
+  @Test
   public void constructFromNumber() {
-    version = new Version(s1, s1, s1);
+    version = new Version(s1, s1, s1, s0, false);
     Assert.assertEquals("1.1.1", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(1, version.getPatch());
+    Assert.assertEquals(0, version.getBuild());
+    version = new Version(s1, s1, s1, s1, true);
+    Assert.assertEquals("1.1.1.1", version.getVersion());
+    Assert.assertEquals(1, version.getMajor());
+    Assert.assertEquals(1, version.getMinor());
+    Assert.assertEquals(1, version.getPatch());
+    Assert.assertEquals(1, version.getPatch());
   }
 
   @Test
   public void testToString() {
-    version = new Version(s1, s1, s1);
+    version = new Version(s1, s1, s1, s0, false);
     Assert.assertEquals("1.1.1", version.toString());
+    version = new Version(s1, s1, s1, s0, true);
+    Assert.assertEquals("1.1.1.0", version.toString());
   }
 
   @Test
   public void testHashCode() {
-    version = new Version(s1, s1, s1);
+    version = new Version(s1, s1, s1, s0, false);
     Assert.assertEquals(version.getVersion().hashCode(), version.hashCode());
+    Version version2 = new Version(s1, s1, s1, s0, true);
+    Assert.assertNotEquals(version.getVersion().hashCode(), version2.hashCode());
   }
 
   @Test
   public void testEquals() {
-    version = new Version(s1, s1, s1);
+    version = new Version(s1, s1, s1, s0, false);
 
     Assert.assertTrue(version.equals(version));
-    Assert.assertTrue(version.equals(new Version(s1, s1, s1)));
+
+    Assert.assertTrue(version.equals(new Version(s1, s1, s1, s0, false)));
+    Assert.assertTrue(version.equals(new Version(s1, s1, s1, s0, true)));
+
     Assert.assertFalse(version.equals(null));
   }
 
   @Test
   public void compareTo() {
-    version = new Version(s1, s1, s1);
-
+    version = new Version(s1, s1, s1, s0, false);
     Assert.assertEquals(0, version.compareTo(version));
-    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1)));
-
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1)));
-
-    Assert.assertEquals(1, version.compareTo(new Version((short) 0, Short.MAX_VALUE, Short.MAX_VALUE)));
+    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1, s0, false)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2, s0, false)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1, s0, false)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1, s0, false)));
+    Assert.assertEquals(1, version.compareTo(new Version((short) 0, Short.MAX_VALUE, Short.MAX_VALUE, s0, false)));
+    Assert.assertEquals(1,version.compareTo(new Version(s1, s1, s0, s0, true)));
+
+    version = new Version(s1, s1, s1, s0, true);
+    Assert.assertEquals(0, version.compareTo(version));
+    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1, s0, true)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2, s0, true)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1, s0, true)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1, s0, true)));
+    Assert.assertEquals(1, version.compareTo(new Version((short) 0, Short.MAX_VALUE, Short.MAX_VALUE, s0, true)));
+    Assert.assertEquals(1,version.compareTo(new Version(s1, s1, s0, s0, false)));
+
+    Assert.assertEquals(0,version.compareTo(new Version(s1, s1, s1, s0, false)));
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
index 9e53aab..f08493f 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
@@ -18,11 +18,11 @@
 package org.apache.servicecomb.serviceregistry.version;
 
 public interface VersionConst {
-  Version v0 = new Version((short) 0, (short) 0, (short) 0);
+  Version v0 = new Version((short) 0, (short) 0, (short) 0, (short) 0, false);
 
   Version v1 = new Version("1");
 
-  Version v1Max = new Version((short) 1, Short.MAX_VALUE, Short.MAX_VALUE);
+  Version v1Max = new Version((short) 1, Short.MAX_VALUE, Short.MAX_VALUE, Short.MAX_VALUE, true);
 
   Version v2 = new Version("2");
 }


[servicecomb-java-chassis] 03/03: [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: modify details

Posted by li...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit e0e8df7059603fc740fa84d47f2f76807bc75ed3
Author: heyile <25...@qq.com>
AuthorDate: Tue Dec 4 15:59:39 2018 +0800

    [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: modify details
---
 .../api/registry/MicroserviceFactory.java          |  6 ++---
 .../serviceregistry/version/Version.java           | 30 +++++-----------------
 .../serviceregistry/version/VersionRuleUtils.java  |  3 ++-
 .../api/registry/TestMicroserviceFactory.java      |  2 +-
 .../registry/TestRemoteServiceRegistry.java        | 19 +++++++++-----
 .../serviceregistry/version/TestVersion.java       | 24 -----------------
 6 files changed, 24 insertions(+), 60 deletions(-)

diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
index b25baf7..2ace55f 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
@@ -56,10 +56,8 @@ public class MicroserviceFactory {
     microservice.setAppId(configuration.getString(CONFIG_APPLICATION_ID_KEY, DEFAULT_APPLICATION_ID));
     String version = configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
         DEFAULT_MICROSERVICE_VERSION);
-    //check version format
-    if (!Version.checkVersion(version)) {
-      throw new IllegalStateException(String.format("config service_description.version %s is invalid", version));
-    }
+    // just check version format
+    new Version(version);
     microservice.setVersion(version);
     setDescription(configuration, microservice);
     microservice.setLevel(configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_ROLE_KEY, "FRONT"));
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
index b4a82a2..3ec9753 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
@@ -21,8 +21,6 @@ import java.util.Objects;
 
 import org.apache.commons.lang.ArrayUtils;
 
-import com.google.common.annotations.VisibleForTesting;
-
 // short version is enough
 public class Version implements Comparable<Version> {
   private static final String[] ZERO = new String[] {"0", "0", "0", "0"};
@@ -51,7 +49,9 @@ public class Version implements Comparable<Version> {
       throw new IllegalStateException(String.format("Invalid version \"%s\".", version));
     }
 
-    versions = (String[]) ArrayUtils.addAll(versions, ZERO);
+    if (versions.length < 4) {
+      versions = (String[]) ArrayUtils.addAll(versions, ZERO);
+    }
     this.major = parseNumber("major", version, versions[0]);
     this.minor = parseNumber("minor", version, versions[1]);
     this.patch = parseNumber("patch", version, versions[2]);
@@ -66,8 +66,8 @@ public class Version implements Comparable<Version> {
     try {
       value = Short.parseShort(version);
     } catch (NumberFormatException e) {
-      throw new IllegalStateException(String.format("Invalid %s \"%s\", version \"%s\".", name, version, allVersion),
-          e);
+      throw new IllegalStateException(
+          String.format("Invalid %s \"%s\", version \"%s\".", name, version, allVersion), e);
     }
 
     if (value < 0) {
@@ -88,15 +88,7 @@ public class Version implements Comparable<Version> {
   }
 
   private String combineStringVersion() {
-    StringBuilder stringBuilder = new StringBuilder();
-    stringBuilder.append(major)
-        .append(".")
-        .append(minor)
-        .append(".")
-        .append(patch)
-        .append(".")
-        .append(build);
-    return stringBuilder.toString();
+    return major + "." + minor + "." + patch + "." + build;
   }
 
   // 1.0.0 equals 1.0.0.0
@@ -104,22 +96,18 @@ public class Version implements Comparable<Version> {
     return (long) major << 48 | (long) minor << 32 | (long) patch << 16 | (long) build;
   }
 
-  @VisibleForTesting
   public short getMajor() {
     return major;
   }
 
-  @VisibleForTesting
   public short getMinor() {
     return minor;
   }
 
-  @VisibleForTesting
   public short getPatch() {
     return patch;
   }
 
-  @VisibleForTesting
   public short getBuild() {
     return build;
   }
@@ -155,10 +143,4 @@ public class Version implements Comparable<Version> {
   public int compareTo(Version other) {
     return Long.compare(numberVersion, other.numberVersion);
   }
-
-  //check version. maybe not contains the range of short. but it's just ok
-  public static boolean checkVersion(String version) {
-    Objects.requireNonNull(version);
-    return version.matches("\\d+(.\\d+){0,3}");
-  }
 }
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
index c36f9fb..f8d512e 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
@@ -54,6 +54,7 @@ public final class VersionRuleUtils {
         return versionRule;
       }
     }
-    throw new IllegalStateException("config service_description.version is invalid ");
+
+    throw new IllegalStateException("never run to here");
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
index 108bab5..c735f9b 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
@@ -149,7 +149,7 @@ public class TestMicroserviceFactory {
       }
     };
     expectedException.equals(IllegalStateException.class);
-    expectedException.expectMessage("config service_description.version x.y.x.1 is invalid");
+    expectedException.expectMessage("Invalid major \"x\", version \"x.y.x.1\".");
     MicroserviceFactory microserviceFactory = new MicroserviceFactory();
     microserviceFactory.create(microserviceDefinition);
   }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
index 6fa1a9d..718e649 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
@@ -16,6 +16,9 @@
  */
 package org.apache.servicecomb.serviceregistry.registry;
 
+import static org.apache.servicecomb.foundation.common.base.ServiceCombConstants.CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY;
+import static org.apache.servicecomb.serviceregistry.definition.DefinitionConst.DEFAULT_MICROSERVICE_VERSION;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.concurrent.CountDownLatch;
@@ -23,6 +26,7 @@ import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.commons.configuration.Configuration;
 import org.apache.servicecomb.config.ConfigUtil;
 import org.apache.servicecomb.foundation.common.net.IpPort;
 import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils;
@@ -35,7 +39,6 @@ import org.apache.servicecomb.serviceregistry.consumer.MicroserviceVersions;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
 import org.apache.servicecomb.serviceregistry.task.event.PullMicroserviceVersionsInstancesEvent;
 import org.apache.servicecomb.serviceregistry.task.event.ShutdownEvent;
-import org.apache.servicecomb.serviceregistry.version.Version;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Rule;
@@ -129,7 +132,8 @@ public class TestRemoteServiceRegistry {
 
   @Test
   public void onPullMicroserviceVersionsInstancesEvent(@Injectable ServiceRegistryConfig config,
-      @Injectable MicroserviceDefinition definition, @Mocked MicroserviceVersions microserviceVersions) {
+      @Injectable MicroserviceDefinition definition, @Mocked MicroserviceVersions microserviceVersions,
+      @Mocked Configuration configuration) {
     PullMicroserviceVersionsInstancesEvent event = new PullMicroserviceVersionsInstancesEvent(microserviceVersions, 1);
 
     ScheduledThreadPoolExecutor taskPool = new MockUp<ScheduledThreadPoolExecutor>() {
@@ -142,10 +146,13 @@ public class TestRemoteServiceRegistry {
       }
     }.getMockInstance();
 
-    new MockUp<Version>() {
-      @Mock
-      boolean checkVersion(String version){
-        return true;
+    new Expectations() {
+      {
+        definition.getConfiguration();
+        result = configuration;
+        configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
+            DEFAULT_MICROSERVICE_VERSION);
+        result = "1.0.0";
       }
     };
 
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
index 8b01375..63025de 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
@@ -199,28 +199,4 @@ public class TestVersion {
     Assert.assertEquals(1, version.compareTo(new Version((short) 0,
         Short.MAX_VALUE, Short.MAX_VALUE, Short.MAX_VALUE)));
   }
-
-  @Test
-  public void testCheckVersion() {
-    Assert.assertFalse(Version.checkVersion("-1"));
-    Assert.assertFalse(Version.checkVersion("1."));
-    Assert.assertFalse(Version.checkVersion("1.-1"));
-    Assert.assertFalse(Version.checkVersion("1.1."));
-    Assert.assertFalse(Version.checkVersion("1.1.-1"));
-    Assert.assertFalse(Version.checkVersion("1.1.1."));
-    Assert.assertFalse(Version.checkVersion("1.1.1.-1"));
-    Assert.assertFalse(Version.checkVersion("1.1.1.1."));
-    Assert.assertFalse(Version.checkVersion("1.1.1.1.1"));
-    Assert.assertFalse(Version.checkVersion("a"));
-    Assert.assertFalse(Version.checkVersion("a."));
-    Assert.assertFalse(Version.checkVersion("1.a"));
-    Assert.assertFalse(Version.checkVersion("1.1.a"));
-    Assert.assertFalse(Version.checkVersion("1.1.1.a"));
-    Assert.assertFalse(Version.checkVersion("1.1.1.1.a"));
-
-    Assert.assertTrue(Version.checkVersion("1"));
-    Assert.assertTrue(Version.checkVersion("1.1"));
-    Assert.assertTrue(Version.checkVersion("1.1.1"));
-    Assert.assertTrue(Version.checkVersion("1.1.1.1"));
-  }
 }


[servicecomb-java-chassis] 02/03: [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: fix as reviewed

Posted by li...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git

commit b2059c0712175e2ce94ed4d7062be84542e3bcf1
Author: heyile <25...@qq.com>
AuthorDate: Tue Dec 4 13:48:30 2018 +0800

    [SCB-1047]microservice.yaml service_description.version support format xxx.xx.xxx.xxx: fix as reviewed
---
 .../consumer/TestConsumerProviderManager.java      |  4 +-
 .../edge/core/TestCompatiblePathVersionMapper.java |  4 +-
 .../edge/core/TestDefaultEdgeDispatcher.java       |  2 +-
 .../api/registry/MicroserviceFactory.java          |  6 +-
 .../serviceregistry/version/Version.java           | 36 ++++-----
 .../serviceregistry/version/VersionRuleUtils.java  | 11 ---
 .../api/registry/TestMicroserviceFactory.java      |  2 +-
 .../consumer/StaticMicroserviceVersionsTest.java   |  2 +-
 .../serviceregistry/consumer/TestAppManager.java   |  2 +-
 .../TestDefaultMicroserviceVersionFactory.java     |  2 +-
 .../consumer/TestMicroserviceManager.java          |  4 +-
 .../consumer/TestMicroserviceVersion.java          |  2 +-
 .../consumer/TestMicroserviceVersionRule.java      |  2 +-
 .../registry/TestRemoteServiceRegistry.java        |  8 +-
 .../serviceregistry/version/TestVersion.java       | 93 ++++++++++------------
 .../version/TestVersionRuleFixedParser.java        |  2 +-
 .../version/TestVersionRuleRangeParser.java        |  2 +-
 .../version/TestVersionRuleStartFromParser.java    |  2 +-
 .../serviceregistry/version/TestVersionUtils.java  |  2 +-
 .../serviceregistry/version/VersionConst.java      |  4 +-
 20 files changed, 88 insertions(+), 104 deletions(-)

diff --git a/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestConsumerProviderManager.java b/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestConsumerProviderManager.java
index 7cee5e3..56bfe38 100644
--- a/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestConsumerProviderManager.java
+++ b/core/src/test/java/org/apache/servicecomb/core/provider/consumer/TestConsumerProviderManager.java
@@ -94,7 +94,7 @@ public class TestConsumerProviderManager {
 
     Assert.assertEquals("app", referenceConfig.getMicroserviceVersionRule().getAppId());
     Assert.assertEquals("app:ms", referenceConfig.getMicroserviceVersionRule().getMicroserviceName());
-    Assert.assertEquals("0.0.0+", referenceConfig.getMicroserviceVersionRule().getVersionRule().getVersionRule());
+    Assert.assertEquals("0.0.0.0+", referenceConfig.getMicroserviceVersionRule().getVersionRule().getVersionRule());
     Assert.assertEquals(Const.ANY_TRANSPORT, referenceConfig.getTransport());
   }
 
@@ -107,7 +107,7 @@ public class TestConsumerProviderManager {
 
     Assert.assertEquals("app", referenceConfig.getMicroserviceVersionRule().getAppId());
     Assert.assertEquals("app:ms", referenceConfig.getMicroserviceVersionRule().getMicroserviceName());
-    Assert.assertEquals("1.0.0+", referenceConfig.getMicroserviceVersionRule().getVersionRule().getVersionRule());
+    Assert.assertEquals("1.0.0.0+", referenceConfig.getMicroserviceVersionRule().getVersionRule().getVersionRule());
     Assert.assertEquals(Const.RESTFUL, referenceConfig.getTransport());
   }
 
diff --git a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestCompatiblePathVersionMapper.java b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestCompatiblePathVersionMapper.java
index 2cfb9fc..871c25d 100644
--- a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestCompatiblePathVersionMapper.java
+++ b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestCompatiblePathVersionMapper.java
@@ -35,7 +35,7 @@ public class TestCompatiblePathVersionMapper {
   public void getOrCreate() {
     VersionRule versionRule = mapper.getOrCreate("v1");
 
-    Assert.assertEquals("1.0.0-2.0.0", versionRule.getVersionRule());
+    Assert.assertEquals("1.0.0.0-2.0.0.0", versionRule.getVersionRule());
   }
 
   @Test
@@ -82,6 +82,6 @@ public class TestCompatiblePathVersionMapper {
   public void createVersionRule_32767() {
     VersionRule versionRule = mapper.getOrCreate("v32767");
 
-    Assert.assertEquals("32767.0.0+", versionRule.getVersionRule());
+    Assert.assertEquals("32767.0.0.0+", versionRule.getVersionRule());
   }
 }
diff --git a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestDefaultEdgeDispatcher.java b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestDefaultEdgeDispatcher.java
index 518740a..e74f13a 100644
--- a/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestDefaultEdgeDispatcher.java
+++ b/edge/edge-core/src/test/java/org/apache/servicecomb/edge/core/TestDefaultEdgeDispatcher.java
@@ -71,7 +71,7 @@ public class TestDefaultEdgeDispatcher {
         result = requst;
         requst.path();
         result = "/api/testService/v1/hello";
-        invocation.setVersionRule("1.0.0-2.0.0");
+        invocation.setVersionRule("1.0.0.0-2.0.0.0");
         invocation.init("testService", context, "/testService/v1/hello",
             Deencapsulation.getField(dispatcher, "httpServerFilters"));
         invocation.edgeInvoke();
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
index 44b1917..b25baf7 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/api/registry/MicroserviceFactory.java
@@ -34,7 +34,7 @@ import org.apache.commons.configuration.Configuration;
 import org.apache.servicecomb.serviceregistry.config.ConfigurePropertyUtils;
 import org.apache.servicecomb.serviceregistry.config.MicroservicePropertiesLoader;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
-import org.apache.servicecomb.serviceregistry.version.VersionRuleUtils;
+import org.apache.servicecomb.serviceregistry.version.Version;
 
 public class MicroserviceFactory {
   public Microservice create(String appId, String microserviceName) {
@@ -57,7 +57,9 @@ public class MicroserviceFactory {
     String version = configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_VERSION_KEY,
         DEFAULT_MICROSERVICE_VERSION);
     //check version format
-    VersionRuleUtils.checkVersionFormat(version);
+    if (!Version.checkVersion(version)) {
+      throw new IllegalStateException(String.format("config service_description.version %s is invalid", version));
+    }
     microservice.setVersion(version);
     setDescription(configuration, microservice);
     microservice.setLevel(configuration.getString(CONFIG_QUALIFIED_MICROSERVICE_ROLE_KEY, "FRONT"));
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
index 69b8df0..b4a82a2 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/Version.java
@@ -21,6 +21,8 @@ import java.util.Objects;
 
 import org.apache.commons.lang.ArrayUtils;
 
+import com.google.common.annotations.VisibleForTesting;
+
 // short version is enough
 public class Version implements Comparable<Version> {
   private static final String[] ZERO = new String[] {"0", "0", "0", "0"};
@@ -33,8 +35,6 @@ public class Version implements Comparable<Version> {
 
   private final short build;
 
-  private final boolean versionFour;
-
   private final String version;
 
   private final long numberVersion;
@@ -51,12 +51,7 @@ public class Version implements Comparable<Version> {
       throw new IllegalStateException(String.format("Invalid version \"%s\".", version));
     }
 
-    if (versions.length == 4) {
-      versionFour = true;
-    } else {
-      versionFour = false;
-      versions = (String[]) ArrayUtils.addAll(versions, ZERO);
-    }
+    versions = (String[]) ArrayUtils.addAll(versions, ZERO);
     this.major = parseNumber("major", version, versions[0]);
     this.minor = parseNumber("minor", version, versions[1]);
     this.patch = parseNumber("patch", version, versions[2]);
@@ -83,12 +78,11 @@ public class Version implements Comparable<Version> {
     return value;
   }
 
-  public Version(short major, short minor, short patch, short build, boolean versionFour) {
+  public Version(short major, short minor, short patch, short build) {
     this.major = major;
     this.minor = minor;
     this.patch = patch;
     this.build = build;
-    this.versionFour = versionFour;
     this.version = combineStringVersion();
     this.numberVersion = combineVersion();
   }
@@ -99,11 +93,9 @@ public class Version implements Comparable<Version> {
         .append(".")
         .append(minor)
         .append(".")
-        .append(patch);
-    if (versionFour) {
-      stringBuilder.append(".")
-          .append(build);
-    }
+        .append(patch)
+        .append(".")
+        .append(build);
     return stringBuilder.toString();
   }
 
@@ -112,26 +104,26 @@ public class Version implements Comparable<Version> {
     return (long) major << 48 | (long) minor << 32 | (long) patch << 16 | (long) build;
   }
 
+  @VisibleForTesting
   public short getMajor() {
     return major;
   }
 
+  @VisibleForTesting
   public short getMinor() {
     return minor;
   }
 
+  @VisibleForTesting
   public short getPatch() {
     return patch;
   }
 
+  @VisibleForTesting
   public short getBuild() {
     return build;
   }
 
-  public boolean isVersionFour() {
-    return versionFour;
-  }
-
   public String getVersion() {
     return version;
   }
@@ -163,4 +155,10 @@ public class Version implements Comparable<Version> {
   public int compareTo(Version other) {
     return Long.compare(numberVersion, other.numberVersion);
   }
+
+  //check version. maybe not contains the range of short. but it's just ok
+  public static boolean checkVersion(String version) {
+    Objects.requireNonNull(version);
+    return version.matches("\\d+(.\\d+){0,3}");
+  }
 }
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
index 5edb5af..c36f9fb 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/version/VersionRuleUtils.java
@@ -56,15 +56,4 @@ public final class VersionRuleUtils {
     }
     throw new IllegalStateException("config service_description.version is invalid ");
   }
-
-  public static void checkVersionFormat(String strVersionRule) {
-    strVersionRule = strVersionRule.trim();
-    for (VersionRuleParser parser : parsers) {
-      VersionRule versionRule = parser.parse(strVersionRule);
-      if (versionRule != null) {
-        return;
-      }
-    }
-    throw new IllegalStateException("config service_description.version is invalid ");
-  }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
index 508833a..108bab5 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/api/registry/TestMicroserviceFactory.java
@@ -149,7 +149,7 @@ public class TestMicroserviceFactory {
       }
     };
     expectedException.equals(IllegalStateException.class);
-    expectedException.expectMessage("Invalid major \"x\", version \"x.y.x.1\"");
+    expectedException.expectMessage("config service_description.version x.y.x.1 is invalid");
     MicroserviceFactory microserviceFactory = new MicroserviceFactory();
     microserviceFactory.create(microserviceDefinition);
   }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/StaticMicroserviceVersionsTest.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/StaticMicroserviceVersionsTest.java
index ca3c3b3..a331749 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/StaticMicroserviceVersionsTest.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/StaticMicroserviceVersionsTest.java
@@ -66,7 +66,7 @@ public class StaticMicroserviceVersionsTest {
     StaticMicroserviceVersions staticMicroserviceVersions = createStaticMicroserviceVersions();
 
     MicroserviceInstance instance = new MicroserviceInstance();
-    String serviceVersion = "1.2.1";
+    String serviceVersion = "1.2.1.0";
     staticMicroserviceVersions.addInstances(serviceVersion, Collections.singletonList(instance));
 
     MicroserviceVersionRule versionRule = staticMicroserviceVersions.getOrCreateMicroserviceVersionRule(serviceVersion);
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestAppManager.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestAppManager.java
index e76d23b..17b5da9 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestAppManager.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestAppManager.java
@@ -78,7 +78,7 @@ public class TestAppManager {
 
     MicroserviceVersionRule microserviceVersionRule =
         appManager.getOrCreateMicroserviceVersionRule(appId, serviceName, versionRule);
-    Assert.assertEquals("0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
+    Assert.assertEquals("0.0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
     Assert.assertNull(microserviceVersionRule.getLatestMicroserviceVersion());
   }
 
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestDefaultMicroserviceVersionFactory.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestDefaultMicroserviceVersionFactory.java
index 3d9056c..5dfe428 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestDefaultMicroserviceVersionFactory.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestDefaultMicroserviceVersionFactory.java
@@ -43,6 +43,6 @@ public class TestDefaultMicroserviceVersionFactory {
 
     MicroserviceVersion microserviceVersion = new DefaultMicroserviceVersionFactory().create("", microserviceId);
     Assert.assertSame(microservice, microserviceVersion.getMicroservice());
-    Assert.assertEquals("1.0.0", microserviceVersion.getVersion().getVersion());
+    Assert.assertEquals("1.0.0.0", microserviceVersion.getVersion().getVersion());
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
index 6812721..9fbdf0f 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
@@ -87,7 +87,7 @@ public class TestMicroserviceManager {
 
     MicroserviceVersionRule microserviceVersionRule =
         microserviceManager.getOrCreateMicroserviceVersionRule(serviceName, versionRule);
-    Assert.assertEquals("0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
+    Assert.assertEquals("0.0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
     Assert.assertNull(microserviceVersionRule.getLatestMicroserviceVersion());
     Assert.assertEquals(1, cachedVersions.size());
   }
@@ -111,7 +111,7 @@ public class TestMicroserviceManager {
 
     MicroserviceVersionRule microserviceVersionRule =
         microserviceManager.getOrCreateMicroserviceVersionRule(serviceName, versionRule);
-    Assert.assertEquals("0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
+    Assert.assertEquals("0.0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
     Assert.assertNull(microserviceVersionRule.getLatestMicroserviceVersion());
     Assert.assertEquals(0, cachedVersions.size());
 
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersion.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersion.java
index b6b036d..279743a 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersion.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersion.java
@@ -56,7 +56,7 @@ public class TestMicroserviceVersion {
     MicroserviceVersion microserviceVersion = MicroserviceVersionTestUtils
         .createMicroserviceVersion("1", "1.0.0", serviceRegistry);
     Assert.assertEquals("1", microserviceVersion.getMicroservice().getServiceId());
-    Assert.assertEquals("1.0.0", microserviceVersion.getVersion().getVersion());
+    Assert.assertEquals("1.0.0.0", microserviceVersion.getVersion().getVersion());
   }
 
   @Test
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersionRule.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersionRule.java
index d78bf18..996a865 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersionRule.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceVersionRule.java
@@ -33,7 +33,7 @@ public class TestMicroserviceVersionRule {
 
   @Test
   public void getVersionRule() {
-    Assert.assertEquals("1.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
+    Assert.assertEquals("1.0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
   }
 
   @Test
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
index 3ac72c6..6fa1a9d 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/registry/TestRemoteServiceRegistry.java
@@ -35,7 +35,7 @@ import org.apache.servicecomb.serviceregistry.consumer.MicroserviceVersions;
 import org.apache.servicecomb.serviceregistry.definition.MicroserviceDefinition;
 import org.apache.servicecomb.serviceregistry.task.event.PullMicroserviceVersionsInstancesEvent;
 import org.apache.servicecomb.serviceregistry.task.event.ShutdownEvent;
-import org.apache.servicecomb.serviceregistry.version.VersionRuleUtils;
+import org.apache.servicecomb.serviceregistry.version.Version;
 import org.hamcrest.Matchers;
 import org.junit.Assert;
 import org.junit.Rule;
@@ -142,10 +142,10 @@ public class TestRemoteServiceRegistry {
       }
     }.getMockInstance();
 
-    new MockUp<VersionRuleUtils>() {
+    new MockUp<Version>() {
       @Mock
-      void checkVersionFormat(String strVersionRule) {
-        return;
+      boolean checkVersion(String version){
+        return true;
       }
     };
 
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
index c79b30c..8b01375 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersion.java
@@ -30,42 +30,35 @@ public class TestVersion {
 
   short s2 = 2;
 
-  short s0 = 0;
-
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
   @Test
   public void constructFromStringNormalOnlyMajor() {
     version = new Version("1");
-    Assert.assertEquals("1.0.0", version.getVersion());
+    Assert.assertEquals("1.0.0.0", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(0, version.getMinor());
     Assert.assertEquals(0, version.getPatch());
-    Assert.assertEquals(0, version.getBuild());
-    Assert.assertFalse(version.isVersionFour());
   }
 
   @Test
   public void constructFromStringNormalOnlyMajorMinor() {
     version = new Version("1.1");
-    Assert.assertEquals("1.1.0", version.getVersion());
+    Assert.assertEquals("1.1.0.0", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(0, version.getPatch());
-    Assert.assertEquals(0, version.getBuild());
-    Assert.assertFalse(version.isVersionFour());
   }
 
   @Test
   public void constructFromStringOnlyMajorMinorPatch() {
     version = new Version("1.1.1");
-    Assert.assertEquals("1.1.1", version.getVersion());
+    Assert.assertEquals("1.1.1.0", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(1, version.getPatch());
     Assert.assertEquals(0, version.getBuild());
-    Assert.assertFalse(version.isVersionFour());
   }
 
   @Test
@@ -76,9 +69,9 @@ public class TestVersion {
     Assert.assertEquals(1, version.getMinor());
     Assert.assertEquals(1, version.getPatch());
     Assert.assertEquals(1, version.getBuild());
-    Assert.assertTrue(version.isVersionFour());
   }
 
+
   @Test
   public void constructFromStringInvalidNull() {
     expectedException.expect(NullPointerException.class);
@@ -158,19 +151,12 @@ public class TestVersion {
   public void constructFromStringInvalidTooManyPart() {
     expectedException.expect(IllegalStateException.class);
     expectedException.expectMessage(Matchers.is("Invalid version \"1.1.1.1.\"."));
-
     version = new Version("1.1.1.1.");
   }
 
   @Test
   public void constructFromNumber() {
-    version = new Version(s1, s1, s1, s0, false);
-    Assert.assertEquals("1.1.1", version.getVersion());
-    Assert.assertEquals(1, version.getMajor());
-    Assert.assertEquals(1, version.getMinor());
-    Assert.assertEquals(1, version.getPatch());
-    Assert.assertEquals(0, version.getBuild());
-    version = new Version(s1, s1, s1, s1, true);
+    version = new Version(s1, s1, s1, s1);
     Assert.assertEquals("1.1.1.1", version.getVersion());
     Assert.assertEquals(1, version.getMajor());
     Assert.assertEquals(1, version.getMinor());
@@ -180,52 +166,61 @@ public class TestVersion {
 
   @Test
   public void testToString() {
-    version = new Version(s1, s1, s1, s0, false);
-    Assert.assertEquals("1.1.1", version.toString());
-    version = new Version(s1, s1, s1, s0, true);
-    Assert.assertEquals("1.1.1.0", version.toString());
+    version = new Version(s1, s1, s1, s1);
+    Assert.assertEquals("1.1.1.1", version.toString());
   }
 
   @Test
   public void testHashCode() {
-    version = new Version(s1, s1, s1, s0, false);
+    version = new Version(s1, s1, s1, s1);
     Assert.assertEquals(version.getVersion().hashCode(), version.hashCode());
-    Version version2 = new Version(s1, s1, s1, s0, true);
-    Assert.assertNotEquals(version.getVersion().hashCode(), version2.hashCode());
   }
 
   @Test
   public void testEquals() {
-    version = new Version(s1, s1, s1, s0, false);
+    version = new Version(s1, s1, s1, s1);
 
     Assert.assertTrue(version.equals(version));
-
-    Assert.assertTrue(version.equals(new Version(s1, s1, s1, s0, false)));
-    Assert.assertTrue(version.equals(new Version(s1, s1, s1, s0, true)));
-
+    Assert.assertTrue(version.equals(new Version(s1, s1, s1, s1)));
     Assert.assertFalse(version.equals(null));
   }
 
   @Test
   public void compareTo() {
-    version = new Version(s1, s1, s1, s0, false);
-    Assert.assertEquals(0, version.compareTo(version));
-    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1, s0, false)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2, s0, false)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1, s0, false)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1, s0, false)));
-    Assert.assertEquals(1, version.compareTo(new Version((short) 0, Short.MAX_VALUE, Short.MAX_VALUE, s0, false)));
-    Assert.assertEquals(1,version.compareTo(new Version(s1, s1, s0, s0, true)));
-
-    version = new Version(s1, s1, s1, s0, true);
+    version = new Version(s1, s1, s1, s1);
+
     Assert.assertEquals(0, version.compareTo(version));
-    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1, s0, true)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2, s0, true)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1, s0, true)));
-    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1, s0, true)));
-    Assert.assertEquals(1, version.compareTo(new Version((short) 0, Short.MAX_VALUE, Short.MAX_VALUE, s0, true)));
-    Assert.assertEquals(1,version.compareTo(new Version(s1, s1, s0, s0, false)));
-
-    Assert.assertEquals(0,version.compareTo(new Version(s1, s1, s1, s0, false)));
+    Assert.assertEquals(0, version.compareTo(new Version(s1, s1, s1, s1)));
+
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s1, s2, s1)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s1, s2, s1, s1)));
+    Assert.assertEquals(-1, version.compareTo(new Version(s2, s1, s1, s1)));
+
+    Assert.assertEquals(1, version.compareTo(new Version((short) 0,
+        Short.MAX_VALUE, Short.MAX_VALUE, Short.MAX_VALUE)));
+  }
+
+  @Test
+  public void testCheckVersion() {
+    Assert.assertFalse(Version.checkVersion("-1"));
+    Assert.assertFalse(Version.checkVersion("1."));
+    Assert.assertFalse(Version.checkVersion("1.-1"));
+    Assert.assertFalse(Version.checkVersion("1.1."));
+    Assert.assertFalse(Version.checkVersion("1.1.-1"));
+    Assert.assertFalse(Version.checkVersion("1.1.1."));
+    Assert.assertFalse(Version.checkVersion("1.1.1.-1"));
+    Assert.assertFalse(Version.checkVersion("1.1.1.1."));
+    Assert.assertFalse(Version.checkVersion("1.1.1.1.1"));
+    Assert.assertFalse(Version.checkVersion("a"));
+    Assert.assertFalse(Version.checkVersion("a."));
+    Assert.assertFalse(Version.checkVersion("1.a"));
+    Assert.assertFalse(Version.checkVersion("1.1.a"));
+    Assert.assertFalse(Version.checkVersion("1.1.1.a"));
+    Assert.assertFalse(Version.checkVersion("1.1.1.1.a"));
+
+    Assert.assertTrue(Version.checkVersion("1"));
+    Assert.assertTrue(Version.checkVersion("1.1"));
+    Assert.assertTrue(Version.checkVersion("1.1.1"));
+    Assert.assertTrue(Version.checkVersion("1.1.1.1"));
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleFixedParser.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleFixedParser.java
index 99ba301..3759605 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleFixedParser.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleFixedParser.java
@@ -28,7 +28,7 @@ public class TestVersionRuleFixedParser {
   @Test
   public void parseNormal() {
     versionRule = parser.parse("1");
-    Assert.assertEquals("1.0.0", versionRule.getVersionRule());
+    Assert.assertEquals("1.0.0.0", versionRule.getVersionRule());
   }
 
   @Test
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleRangeParser.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleRangeParser.java
index cd2da7e..a44a23b 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleRangeParser.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleRangeParser.java
@@ -34,7 +34,7 @@ public class TestVersionRuleRangeParser {
 
   @Test
   public void parseNormal() {
-    Assert.assertEquals("1.0.0-2.0.0", versionRule.getVersionRule());
+    Assert.assertEquals("1.0.0.0-2.0.0.0", versionRule.getVersionRule());
   }
 
   @Test
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleStartFromParser.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleStartFromParser.java
index 037feed..a99f39f 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleStartFromParser.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionRuleStartFromParser.java
@@ -34,7 +34,7 @@ public class TestVersionRuleStartFromParser {
 
   @Test
   public void parseNormal() {
-    Assert.assertEquals("1.0.0+", versionRule.getVersionRule());
+    Assert.assertEquals("1.0.0.0+", versionRule.getVersionRule());
   }
 
   @Test
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionUtils.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionUtils.java
index a7ffcf4..cd8a460 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionUtils.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/TestVersionUtils.java
@@ -41,7 +41,7 @@ public class TestVersionUtils {
   public void getOrCreate() {
     Version v = VersionUtils.getOrCreate("1.0.0");
 
-    Assert.assertEquals("1.0.0", v.getVersion());
+    Assert.assertEquals("1.0.0.0", v.getVersion());
     Assert.assertSame(v, VersionUtils.getOrCreate("1.0.0"));
   }
 }
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
index f08493f..d92f03e 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/version/VersionConst.java
@@ -18,11 +18,11 @@
 package org.apache.servicecomb.serviceregistry.version;
 
 public interface VersionConst {
-  Version v0 = new Version((short) 0, (short) 0, (short) 0, (short) 0, false);
+  Version v0 = new Version((short) 0, (short) 0, (short) 0, (short) 0);
 
   Version v1 = new Version("1");
 
-  Version v1Max = new Version((short) 1, Short.MAX_VALUE, Short.MAX_VALUE, Short.MAX_VALUE, true);
+  Version v1Max = new Version((short) 1, Short.MAX_VALUE, Short.MAX_VALUE, Short.MAX_VALUE);
 
   Version v2 = new Version("2");
 }