[ https://issues.apache.org/jira/browse/SCB-860?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16597026#comment-16597026 ]
ASF GitHub Bot commented on SCB-860: ------------------------------------ wujimin closed pull request #880: [SCB-860] Optimize microservice register process, abort registry when query microservice id fails URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/880 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java index 196cd24d3..cb59717ff 100644 --- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java +++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImpl.java @@ -163,10 +163,15 @@ public void init() { } @Override - public String getMicroserviceId(String appId, String microserviceName, String strVersionRule, String environment) { + public Holder<String> getMicroserviceId(String appId, String microserviceName, String strVersionRule, + String environment) { VersionRule versionRule = VersionRuleUtils.getOrCreate(strVersionRule); Microservice latest = findLatest(appId, microserviceName, versionRule); - return latest != null ? latest.getServiceId() : null; + Holder<String> serviceIdHolder = new Holder<>(); + serviceIdHolder + .setStatusCode(Status.OK.getStatusCode()) + .setValue(latest != null ? latest.getServiceId() : null); + return serviceIdHolder; } @Override @@ -275,7 +280,7 @@ protected Microservice findLatest(String appId, String serviceName, VersionRule String strVersionRule) { MicroserviceInstances instances = findServiceInstances(selfMicroserviceId, appId, serviceName, strVersionRule, null); - if(instances.isMicroserviceNotExist()) { + if (instances.isMicroserviceNotExist()) { return null; } return instances.getInstancesResponse().getInstances(); diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/ServiceRegistryClient.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/ServiceRegistryClient.java index 4af6d7087..dc41ab71b 100644 --- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/ServiceRegistryClient.java +++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/ServiceRegistryClient.java @@ -42,7 +42,7 @@ * * 获取微服务唯一标识 */ - String getMicroserviceId(String appId, String microserviceName, String versionRule, String environment); + Holder<String> getMicroserviceId(String appId, String microserviceName, String versionRule, String environment); /** * diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/Holder.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/Holder.java index e63911a93..41dd4ecdc 100644 --- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/Holder.java +++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/Holder.java @@ -17,17 +17,40 @@ package org.apache.servicecomb.serviceregistry.client.http; +import io.vertx.core.buffer.Buffer; + /** * To carry the rest response information. * @param <T> Type of response body */ public class Holder<T> { + /** + * The deserialized response body + */ T value; int statusCode; Throwable throwable; + /** + * In error situation, response body cannot be deserialized to expected response type, + * so we record raw body. + */ + Buffer rawResponseBody; + + /** + * Copy inner state from another holder object, except for {@link #value} + * @param fromHolder copy from {@code fromHolder} + * @return reference of this holder itself + */ + public Holder<T> copyFrom(Holder<?> fromHolder) { + this.statusCode = fromHolder.statusCode; + this.throwable = fromHolder.throwable; + this.rawResponseBody = fromHolder.rawResponseBody; + return this; + } + public T getValue() { return value; } @@ -55,12 +78,22 @@ public Throwable getThrowable() { return this; } + public Buffer getRawResponseBody() { + return rawResponseBody; + } + + public Holder<T> setRawResponseBody(Buffer rawResponseBody) { + this.rawResponseBody = rawResponseBody; + return this; + } + @Override public String toString() { final StringBuilder sb = new StringBuilder("Holder{"); sb.append("value=").append(value); sb.append(", statusCode=").append(statusCode); sb.append(", throwable=").append(throwable); + sb.append(", rawResponseBody=").append(rawResponseBody); sb.append('}'); return sb.toString(); } diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java index b5e24368d..0d62f88d7 100644 --- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java +++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/client/http/ServiceRegistryClientImpl.java @@ -65,10 +65,18 @@ import io.vertx.core.Handler; import io.vertx.core.buffer.Buffer; import io.vertx.core.http.HttpClientResponse; +import io.vertx.core.json.JsonObject; public final class ServiceRegistryClientImpl implements ServiceRegistryClient { + private static final Logger LOGGER = LoggerFactory.getLogger(ServiceRegistryClientImpl.class); + public static final int MICROSERVICE_NOT_FOUND_STATUS_CODE = 400; + + public static final String MICROSERVICE_NOT_FOUND_ERROR_CODE = "400012"; + + public static final String SC_RESPONSE_KEY_ERROR_CODE = "errorCode"; + private IpPortManager ipPortManager; // key是本进程的微服务id和服务管理中心的id @@ -102,6 +110,7 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response if (requestContext.getRetryTimes() <= ipPortManager.getMaxRetryTimes()) { retry(requestContext, syncHandler(countDownLatch, cls, holder)); } else { + holder.setStatusCode(0); countDownLatch.countDown(); } return; @@ -109,6 +118,7 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response holder.setStatusCode(response.statusCode()); response.bodyHandler( bodyBuffer -> { + holder.setRawResponseBody(bodyBuffer); if (cls.getName().equals(HttpClientResponse.class.getName())) { holder.value = (T) response; countDownLatch.countDown(); @@ -206,7 +216,7 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response case 400: { @SuppressWarnings("unchecked") Map<String, Object> error = JsonUtils.readValue(bodyBuffer.getBytes(), Map.class); - if ("400012".equals(error.get("errorCode"))) { + if (MICROSERVICE_NOT_FOUND_ERROR_CODE.equals(error.get(SC_RESPONSE_KEY_ERROR_CODE))) { mInstances.setMicroserviceNotExist(true); mInstances.setNeedRefresh(false); } @@ -247,7 +257,8 @@ private void retry(RequestContext requestContext, Handler<RestResponse> response } @Override - public String getMicroserviceId(String appId, String microserviceName, String versionRule, String environment) { + public Holder<String> getMicroserviceId(String appId, String microserviceName, String versionRule, + String environment) { Holder<GetExistenceResponse> holder = new Holder<>(); IpPort ipPort = ipPortManager.getAvailableAddress(); @@ -260,19 +271,41 @@ public String getMicroserviceId(String appId, String microserviceName, String ve .addQueryParam("version", versionRule) .addQueryParam("env", environment), syncHandler(countDownLatch, GetExistenceResponse.class, holder)); + + Holder<String> responseHolder = new Holder<>(); try { countDownLatch.await(); - if (holder.value != null) { - return holder.value.getServiceId(); + responseHolder.copyFrom(holder) + .setValue(null == holder.value ? null : holder.value.getServiceId()); + if (null == responseHolder.getValue() && serviceIdNotFoundInSC(responseHolder)) { + // In fact this query does not fail, so we set it's status code to 200 + responseHolder.setStatusCode(Status.OK.getStatusCode()); } + return responseHolder; } catch (Exception e) { LOGGER.error("query microservice id {}/{}/{} fail", appId, microserviceName, versionRule, e); + responseHolder.setThrowable(e); } - return null; + return responseHolder; + } + + /** + * @return true if the query success and the response indicates that the microservice is not registered yet; + * otherwise false. + */ + private boolean serviceIdNotFoundInSC(Holder<String> serviceIdHolder) { + if (null == serviceIdHolder.getRawResponseBody()) { + return false; + } + + JsonObject responseJson = new JsonObject(serviceIdHolder.getRawResponseBody()); + // HttpStatusCode is 400 and errorCode is 400012, which means the microservice hasn't been registered before. + return MICROSERVICE_NOT_FOUND_STATUS_CODE == serviceIdHolder.getStatusCode() + && MICROSERVICE_NOT_FOUND_ERROR_CODE.equals(responseJson.getString(SC_RESPONSE_KEY_ERROR_CODE)); } @Override @@ -388,7 +421,7 @@ public String getSchema(String microserviceId, String schemaId) { microserviceId, e); } - resultHolder.setStatusCode(holder.getStatusCode()).setThrowable(holder.getThrowable()); + resultHolder.copyFrom(holder); if (holder.value != null) { return resultHolder.setValue( holder.value.getSchema() != null ? @@ -593,7 +626,7 @@ public void watch(String selfMicroserviceId, AsyncResultCallback<MicroserviceIns callback); onClose.success(null); }, bodyBuffer -> { - MicroserviceInstanceChangedEvent response = null; + MicroserviceInstanceChangedEvent response; try { response = JsonUtils.readValue(bodyBuffer.getBytes(), MicroserviceInstanceChangedEvent.class); @@ -613,9 +646,7 @@ public void watch(String selfMicroserviceId, AsyncResultCallback<MicroserviceIns }, e -> { watchErrorHandler(e, selfMicroserviceId, callback); onClose.success(null); - }, f -> { - watchErrorHandler(f, selfMicroserviceId, callback); - }); + }, f -> watchErrorHandler(f, selfMicroserviceId, callback)); } } } diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/task/MicroserviceRegisterTask.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/task/MicroserviceRegisterTask.java index 35ee1e636..6db3721d9 100644 --- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/task/MicroserviceRegisterTask.java +++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/task/MicroserviceRegisterTask.java @@ -25,6 +25,7 @@ import javax.ws.rs.core.Response.Status; +import org.apache.commons.lang3.StringUtils; import org.apache.servicecomb.foundation.common.base.ServiceCombConstants; import org.apache.servicecomb.serviceregistry.RegistryUtils; import org.apache.servicecomb.serviceregistry.api.registry.Microservice; @@ -36,7 +37,6 @@ import com.google.common.eventbus.EventBus; import com.google.common.eventbus.Subscribe; -import org.apache.commons.lang3.StringUtils; public class MicroserviceRegisterTask extends AbstractRegisterTask { private static final Logger LOGGER = LoggerFactory.getLogger(MicroserviceRegisterTask.class); @@ -73,10 +73,15 @@ public void onInstanceRegistryFailed(MicroserviceInstanceRegisterTask task) { @Override protected boolean doRegister() { LOGGER.info("running microservice register task."); - String serviceId = srClient.getMicroserviceId(microservice.getAppId(), + Holder<String> serviceIdHolder = srClient.getMicroserviceId(microservice.getAppId(), microservice.getServiceName(), microservice.getVersion(), microservice.getEnvironment()); + if (Status.OK.getStatusCode() != serviceIdHolder.getStatusCode()) { + LOGGER.error("failed to query serviceId, status code is [{}]", serviceIdHolder.getStatusCode()); + return false; + } + String serviceId = serviceIdHolder.getValue(); if (!StringUtils.isEmpty(serviceId)) { // This microservice has been registered, so we just use the serviceId gotten from service center microservice.setServiceId(serviceId); @@ -325,7 +330,7 @@ private void checkRemainingSchema(Map<String, GetSchemaResponse> scSchemaMap) { // Currently nothing to do but print a warning LOGGER.warn("There are schemas only existing in service center: {}, which means there are interfaces changed. " - + "It's recommended to increment microservice version before deploying.", + + "It's recommended to increment microservice version before deploying.", scSchemaMap.keySet()); LOGGER.warn("ATTENTION: The schemas in new version are less than the old version, " + "which may cause compatibility problems."); diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java index e4fdc7dcf..6a3e0842e 100644 --- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java +++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/LocalServiceRegistryClientImplTest.java @@ -81,20 +81,23 @@ private Microservice mockRegisterMicroservice(String appId, String name, String public void getMicroserviceId_appNotMatch() { mockRegisterMicroservice("otherApp", microserviceName, "1.0.0"); - Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "")); + Assert.assertEquals(200, registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "").getStatusCode()); + Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "").getValue()); } @Test public void getMicroserviceId_nameNotMatch() { mockRegisterMicroservice(appId, "otherName", "1.0.0"); - Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "")); + Assert.assertEquals(200, registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "").getStatusCode()); + Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "").getValue()); } @Test public void getMicroserviceId_versionNotMatch() { mockRegisterMicroservice(appId, microserviceName, "1.0.0"); - Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "2.0.0", "")); + Assert.assertEquals(200, registryClient.getMicroserviceId(appId, microserviceName, "2.0.0", "").getStatusCode()); + Assert.assertNull(registryClient.getMicroserviceId(appId, microserviceName, "2.0.0", "").getValue()); } @Test @@ -102,9 +105,10 @@ public void getMicroserviceId_latest() { Microservice v2 = mockRegisterMicroservice(appId, microserviceName, "2.0.0"); mockRegisterMicroservice(appId, microserviceName, "1.0.0"); - String serviceId = - registryClient.getMicroserviceId(appId, microserviceName, DefinitionConst.VERSION_RULE_LATEST, ""); - Assert.assertEquals(v2.getServiceId(), serviceId); + Holder<String> serviceIdHolder = registryClient + .getMicroserviceId(appId, microserviceName, DefinitionConst.VERSION_RULE_LATEST, ""); + Assert.assertEquals(200, serviceIdHolder.getStatusCode()); + Assert.assertEquals(v2.getServiceId(), serviceIdHolder.getValue()); } @Test @@ -112,7 +116,7 @@ public void getMicroserviceId_fixVersion() { Microservice v1 = mockRegisterMicroservice(appId, microserviceName, "1.0.0"); mockRegisterMicroservice(appId, microserviceName, "2.0.0"); - String serviceId = registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", ""); + String serviceId = registryClient.getMicroserviceId(appId, microserviceName, "1.0.0", "").getValue(); Assert.assertEquals(v1.getServiceId(), serviceId); } diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java index 96f2daf3e..0b97408b8 100644 --- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java +++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestClientHttp.java @@ -93,7 +93,7 @@ void open(IpPort ipPort, String url, Handler<Void> onOpen, Handler<Void> onClose oClient.getMicroserviceId(microservice.getAppId(), microservice.getServiceName(), microservice.getVersion(), - microservice.getEnvironment())); + microservice.getEnvironment()).getValue()); Assert.assertEquals(null, oClient.heartbeat(microservice.getServiceId(), microservice.getInstance().getInstanceId())); diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java index 1c238dc85..4fc0e87f8 100644 --- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java +++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/client/http/TestServiceRegistryClientImpl.java @@ -105,25 +105,23 @@ public void testPrivateMethodCreateHttpClientOptions() { public void testException() { MicroserviceFactory microserviceFactory = new MicroserviceFactory(); Microservice microservice = microserviceFactory.create("app", "ms"); - Assert.assertEquals(null, oClient.registerMicroservice(microservice)); - Assert.assertEquals(null, oClient.registerMicroserviceInstance(microservice.getInstance())); + Assert.assertNull(oClient.registerMicroservice(microservice)); + Assert.assertNull(oClient.registerMicroserviceInstance(microservice.getInstance())); oClient.init(); - Assert.assertEquals(null, - oClient.getMicroserviceId(microservice.getAppId(), - microservice.getServiceName(), - microservice.getVersion(), - microservice.getEnvironment())); + Holder<String> microserviceIdHolder = oClient.getMicroserviceId(microservice.getAppId(), + microservice.getServiceName(), + microservice.getVersion(), + microservice.getEnvironment()); + Assert.assertNull(microserviceIdHolder.getValue()); + Assert.assertEquals(0, microserviceIdHolder.getStatusCode()); Assert.assertThat(oClient.getAllMicroservices().isEmpty(), is(true)); - Assert.assertEquals(null, oClient.registerMicroservice(microservice)); - Assert.assertEquals(null, oClient.getMicroservice("microserviceId")); - Assert.assertEquals(null, oClient.getMicroserviceInstance("consumerId", "providerId")); - Assert.assertEquals(false, - oClient.unregisterMicroserviceInstance("microserviceId", "microserviceInstanceId")); - Assert.assertEquals(null, oClient.heartbeat("microserviceId", "microserviceInstanceId")); - Assert.assertEquals(null, - oClient.findServiceInstance("selfMicroserviceId", "appId", "serviceName", "versionRule")); - Assert.assertEquals(null, - oClient.findServiceInstances("selfMicroserviceId", "appId", "serviceName", "versionRule", "0")); + Assert.assertNull(oClient.registerMicroservice(microservice)); + Assert.assertNull(oClient.getMicroservice("microserviceId")); + Assert.assertNull(oClient.getMicroserviceInstance("consumerId", "providerId")); + Assert.assertFalse(oClient.unregisterMicroserviceInstance("microserviceId", "microserviceInstanceId")); + Assert.assertNull(oClient.heartbeat("microserviceId", "microserviceInstanceId")); + Assert.assertNull(oClient.findServiceInstance("selfMicroserviceId", "appId", "serviceName", "versionRule")); + Assert.assertNull(oClient.findServiceInstances("selfMicroserviceId", "appId", "serviceName", "versionRule", "0")); Assert.assertEquals("a", new ClientException("a").getMessage()); } @@ -281,6 +279,7 @@ HttpClientResponse bodyHandler(Handler<Buffer> bodyHandler) { bodyHandlerHolder.value.handle(bodyBuffer); Assert.assertNull(holder.value); + Assert.assertEquals(400, holder.getStatusCode()); } @Test @@ -403,6 +402,7 @@ void httpDo(RequestContext requestContext, Handler<RestResponse> responseHandler MicroserviceInstances microserviceInstances = oClient .findServiceInstances("consumerId", "appId", "serviceName", DefinitionConst.VERSION_RULE_ALL, null); + Assert.assertNotNull(microserviceInstances); Assert.assertTrue(microserviceInstances.isMicroserviceNotExist()); Assert.assertFalse(microserviceInstances.isNeedRefresh()); } @@ -424,6 +424,7 @@ void httpDo(RequestContext requestContext, Handler<RestResponse> responseHandler } }; ServiceCenterInfo info = oClient.getServiceCenterInfo(); + Assert.assertNotNull(info); Assert.assertEquals("x.x.x", info.getVersion()); Assert.assertEquals("xxx", info.getBuildTag()); Assert.assertEquals("dev", info.getRunMode()); @@ -450,4 +451,106 @@ void doRun(java.util.List<LoggingEvent> events) { } }.run(); } + + @Test + public void getMicroserviceId() { + RestResponse restResponse = Mockito.mock(RestResponse.class); + Holder<HttpClientResponse> httpClientResponseHolder = new Holder<>(); + HttpClientResponse httpClientResponse = new MockUp<HttpClientResponse>() { + @Mock + HttpClientResponse bodyHandler(io.vertx.core.Handler<io.vertx.core.buffer.Buffer> bodyHandler) { + bodyHandler.handle(Buffer.buffer("{\"serviceId\":\"testServiceId\",\"schemaId\":\"testSchemaId\"}")); + return httpClientResponseHolder.getValue(); + } + + @Mock + int statusCode() { + return 200; + } + }.getMockInstance(); + httpClientResponseHolder.setValue(httpClientResponse); + Mockito.when(restResponse.getResponse()).thenReturn(httpClientResponse); + new MockUp<RestUtils>() { + @Mock + void get(IpPort ipPort, String uri, RequestParam requestParam, Handler<RestResponse> responseHandler) { + Assert.assertEquals("microservice", requestParam.getQueryParamsMap().get("type")[0]); + Assert.assertEquals("appId00", requestParam.getQueryParamsMap().get("appId")[0]); + Assert.assertEquals("testService00", requestParam.getQueryParamsMap().get("serviceName")[0]); + Assert.assertEquals("0.0.1+", requestParam.getQueryParamsMap().get("version")[0]); + Assert.assertEquals("", requestParam.getQueryParamsMap().get("env")[0]); + responseHandler.handle(restResponse); + } + }; + + Holder<String> microserviceIdHolder = oClient.getMicroserviceId("appId00", "testService00", "0.0.1+", ""); + Assert.assertEquals(200, microserviceIdHolder.getStatusCode()); + Assert.assertEquals("testServiceId", microserviceIdHolder.getValue()); + } + + @Test + public void getMicroserviceId_ServiceNotFound() { + RestResponse restResponse = Mockito.mock(RestResponse.class); + Holder<HttpClientResponse> httpClientResponseHolder = new Holder<>(); + HttpClientResponse httpClientResponse = new MockUp<HttpClientResponse>() { + @Mock + HttpClientResponse bodyHandler(io.vertx.core.Handler<io.vertx.core.buffer.Buffer> bodyHandler) { + bodyHandler.handle(Buffer.buffer("{\"errorCode\":\"400012\",\"errorMessage\":\"Micro-service does not exist\"," + + "\"detail\":\"service does not exist.\"}")); + return httpClientResponseHolder.getValue(); + } + + @Mock + int statusCode() { + return 400; + } + }.getMockInstance(); + httpClientResponseHolder.setValue(httpClientResponse); + Mockito.when(restResponse.getResponse()).thenReturn(httpClientResponse); + new MockUp<RestUtils>() { + @Mock + void get(IpPort ipPort, String uri, RequestParam requestParam, Handler<RestResponse> responseHandler) { + responseHandler.handle(restResponse); + } + }; + + Holder<String> microserviceIdHolder = oClient.getMicroserviceId("appId00", "testService00", "0.0.1+", ""); + Assert.assertEquals(200, microserviceIdHolder.getStatusCode()); + Assert.assertNull(microserviceIdHolder.getValue()); + Assert.assertEquals("{\"errorCode\":\"400012\",\"errorMessage\":\"Micro-service does not exist\"," + + "\"detail\":\"service does not exist.\"}", + microserviceIdHolder.getRawResponseBody().toString()); + } + + @Test + public void getMicroserviceId_QueryResponseError() { + RestResponse restResponse = Mockito.mock(RestResponse.class); + Holder<HttpClientResponse> httpClientResponseHolder = new Holder<>(); + HttpClientResponse httpClientResponse = new MockUp<HttpClientResponse>() { + @Mock + HttpClientResponse bodyHandler(io.vertx.core.Handler<io.vertx.core.buffer.Buffer> bodyHandler) { + bodyHandler.handle(Buffer.buffer("{\"errorCode\":\"xxxxxx\",\"errorMessage\":\"test Error\"," + + "\"detail\":\"test Error.\"}")); + return httpClientResponseHolder.getValue(); + } + + @Mock + int statusCode() { + return 401; + } + }.getMockInstance(); + httpClientResponseHolder.setValue(httpClientResponse); + Mockito.when(restResponse.getResponse()).thenReturn(httpClientResponse); + new MockUp<RestUtils>() { + @Mock + void get(IpPort ipPort, String uri, RequestParam requestParam, Handler<RestResponse> responseHandler) { + responseHandler.handle(restResponse); + } + }; + + Holder<String> microserviceIdHolder = oClient.getMicroserviceId("appId00", "testService00", "0.0.1+", ""); + Assert.assertEquals(401, microserviceIdHolder.getStatusCode()); + Assert.assertNull(microserviceIdHolder.getValue()); + Assert.assertEquals("{\"errorCode\":\"xxxxxx\",\"errorMessage\":\"test Error\",\"detail\":\"test Error.\"}", + microserviceIdHolder.getRawResponseBody().toString()); + } } diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/task/TestMicroserviceRegisterTask.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/task/TestMicroserviceRegisterTask.java index 5c44312c9..93d5648ed 100644 --- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/task/TestMicroserviceRegisterTask.java +++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/task/TestMicroserviceRegisterTask.java @@ -69,18 +69,16 @@ public void onEvent(MicroserviceRegisterTask task) { } @After - public void teardown() { + public void tearDown() { collector.teardown(); } @Test - public void testNewRegisterFailed(@Mocked ServiceRegistryClient srClient) { + public void testNewRegisterGetServiceIdFailed(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = null; - srClient.registerMicroservice((Microservice) any); - result = null; + result = new Holder<String>(); } }; @@ -91,16 +89,19 @@ public void testNewRegisterFailed(@Mocked ServiceRegistryClient srClient) { Assert.assertEquals(false, registerTask.isSchemaIdSetMatch()); Assert.assertEquals(null, microservice.getServiceId()); Assert.assertEquals(1, taskList.size()); + Assert.assertSame(registerTask, taskList.get(0)); } @Test public void testNewRegisterSuccess(@Mocked ServiceRegistryClient srClient) { Holder<List<GetSchemaResponse>> onlineSchemasHolder = new Holder<>(); onlineSchemasHolder.setStatusCode(200); + Holder<String> serviceIdHolder = new Holder<>(); + serviceIdHolder.setValue(null).setStatusCode(200); new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = null; + result = serviceIdHolder; srClient.registerMicroservice((Microservice) any); result = "serviceId"; srClient.getSchemas("serviceId"); @@ -141,7 +142,7 @@ public void testRegisterSchemaFailed(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = null; + result = new Holder<String>().setStatusCode(200); srClient.registerMicroservice((Microservice) any); result = "serviceId"; srClient.registerSchema(anyString, anyString, anyString); @@ -177,7 +178,7 @@ public void testRegisterSchemaSuccess(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = null; + result = new Holder<String>().setStatusCode(200).setValue(null); srClient.registerMicroservice((Microservice) any); result = "serviceId"; srClient.getSchema("serviceId", "s1"); @@ -204,7 +205,7 @@ public void testAlreadyRegisteredSchemaIdSetMatch(@Mocked ServiceRegistryClient new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = microservice; srClient.getSchemas("serviceId"); @@ -240,7 +241,7 @@ public void testAlreadyRegisteredSchemaIdSetNotMatch(@Mocked ServiceRegistryClie new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas("serviceId"); @@ -262,7 +263,7 @@ public void testAlreadyRegisteredGetSchemaIdSetFailed(@Mocked ServiceRegistryCli new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = null; } @@ -297,7 +298,7 @@ public void testReRegisteredSetForDev(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -338,7 +339,7 @@ public void testFirstRegisterForProd(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -382,7 +383,7 @@ public void testReRegisteredSetForProd(@Mocked ServiceRegistryClient srClient) { new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -420,7 +421,7 @@ public void testReRegisterForProductAndLocalSchemasAreLess(@Mocked ServiceRegist new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -461,7 +462,7 @@ public void testReRegisterForDevAndLocalSchemasAreLess(@Mocked ServiceRegistryCl new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -506,7 +507,7 @@ public void testLocalSchemaAndServiceCenterSchemaDiff(@Mocked ServiceRegistryCli new Expectations() { { srClient.getMicroserviceId(anyString, anyString, anyString, anyString); - result = "serviceId"; + result = new Holder<String>().setStatusCode(200).setValue("serviceId"); srClient.getMicroservice(anyString); result = otherMicroservice; srClient.getSchemas(anyString); @@ -700,7 +701,6 @@ public void testLocalSchemaAndServiceCenterSchemaDiff(@Mocked ServiceRegistryCli " format: \"int32\"\n" + " maximum: 20\n" + " x-java-class: \"org.apache.servicecomb.demo.validator.Student\"]", events.get(5).getMessage()); - } Assert.assertEquals(true, isIlleagalException); ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Optimize microservice register process, abort registry when query > microservice id fails > --------------------------------------------------------------------------------------- > > Key: SCB-860 > URL: https://issues.apache.org/jira/browse/SCB-860 > Project: Apache ServiceComb > Issue Type: Improvement > Reporter: YaoHaishi > Assignee: YaoHaishi > Priority: Minor > > Currently if the microservice id query fails or microservice does not exist, > the sc client will return null, and MicroserviceRegisterTask cannot > distinguish these situation. As a result, even though query fails, > MicroserviceRegisterTask will go on to the next logic, which will cause > unnecessary net work load. > We need to make sc client return more detailed microservice id query result, > and abort registry process when query fails. -- This message was sent by Atlassian JIRA (v7.6.3#76005)