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 <2513931...@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")); - } }