[
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)
