This is an automated email from the ASF dual-hosted git repository. yaohaishi pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit 0b507b459e0a512e74e368e19063a3c23196b16e Author: yhs0092 <[email protected]> AuthorDate: Sat Dec 28 17:51:43 2019 +0800 [SCB-1691] modify instance status change interface --- .../client/LocalServiceRegistryClientImpl.java | 30 +++--- .../client/ServiceRegistryClient.java | 23 ++++- .../client/http/ServiceRegistryClientImpl.java | 34 +++---- .../client/LocalServiceRegistryClientImplTest.java | 66 ++++++++++++++ .../client/http/TestServiceRegistryClientImpl.java | 101 ++++++++++++++++----- 5 files changed, 199 insertions(+), 55 deletions(-) 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 4b1b6a6..cf8963f 100755 --- 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 @@ -434,30 +434,26 @@ public class LocalServiceRegistryClientImpl implements ServiceRegistryClient { } @Override - public boolean undateMicroserviceInstanceStatus(String microserviceId, String microserviceInstanceId, String status) { + public boolean updateMicroserviceInstanceStatus(String microserviceId, String instanceId, + MicroserviceInstanceStatus status) { + if (null == status) { + throw new IllegalArgumentException("null status is now allowed"); + } + Map<String, MicroserviceInstance> instanceMap = microserviceInstanceMap.get(microserviceId); if (instanceMap == null) { throw new IllegalArgumentException("Invalid serviceId, serviceId=" + microserviceId); } - MicroserviceInstance microserviceInstance = instanceMap.get(microserviceInstanceId); + MicroserviceInstance microserviceInstance = instanceMap.get(instanceId); if (microserviceInstance == null) { throw new IllegalArgumentException( - String.format("Invalid argument. microserviceId=%s, microserviceInstanceId=%s.", - microserviceId, - microserviceInstanceId)); - } - MicroserviceInstanceStatus instanceStatus; - try { - instanceStatus = MicroserviceInstanceStatus.valueOf(status); - } - catch (IllegalArgumentException e){ - throw new IllegalArgumentException("Invalid status."); - } - if (null != instanceStatus) { - microserviceInstance.setStatus(instanceStatus); - return true; + String.format("Invalid argument. microserviceId=%s, instanceId=%s.", + microserviceId, + instanceId)); } - return false; + + microserviceInstance.setStatus(status); + return true; } } 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 81f61bf..b093c0c 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 @@ -179,6 +179,27 @@ public interface ServiceRegistryClient { * @param microserviceId * @param microserviceInstanceId * @return + * @deprecated use {@link #updateMicroserviceInstanceStatus(String, String, MicroserviceInstanceStatus)} instead */ - boolean undateMicroserviceInstanceStatus(String microserviceId, String microserviceInstanceId, String status); + @Deprecated + default boolean undateMicroserviceInstanceStatus(String microserviceId, String microserviceInstanceId, + String status) { + MicroserviceInstanceStatus instanceStatus; + try { + instanceStatus = MicroserviceInstanceStatus.valueOf(status); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid status: " + status); + } + + return updateMicroserviceInstanceStatus(microserviceId, microserviceInstanceId, instanceStatus); + } + + /** + * Update the instance status registered in service center. + * @param microserviceId the microserviceId of the instance + * @param instanceId the instanceId of the instance + * @param status update to this status + * @return whether this operation success + */ + boolean updateMicroserviceInstanceStatus(String microserviceId, String instanceId, MicroserviceInstanceStatus status); } 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 3c5930f..ed2e369 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 @@ -39,6 +39,7 @@ import org.apache.servicecomb.serviceregistry.RegistryUtils; import org.apache.servicecomb.serviceregistry.api.Const; import org.apache.servicecomb.serviceregistry.api.registry.Microservice; import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance; +import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus; import org.apache.servicecomb.serviceregistry.api.registry.ServiceCenterInfo; import org.apache.servicecomb.serviceregistry.api.request.CreateSchemaRequest; import org.apache.servicecomb.serviceregistry.api.request.CreateServiceRequest; @@ -62,7 +63,6 @@ import org.apache.servicecomb.serviceregistry.client.ServiceRegistryClient; import org.apache.servicecomb.serviceregistry.config.ServiceRegistryConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import org.springframework.util.StringUtils; import com.google.common.annotations.VisibleForTesting; import com.google.common.cache.CacheBuilder; @@ -78,7 +78,9 @@ public final class ServiceRegistryClientImpl implements ServiceRegistryClient { private static final Logger LOGGER = LoggerFactory.getLogger(ServiceRegistryClientImpl.class); private static final String ERROR_CODE = "errorCode"; + private static final String ERR_SERVICE_NOT_EXISTS = "400012"; + private static final String ERR_SCHEMA_NOT_EXISTS = "400016"; private IpPortManager ipPortManager; @@ -691,7 +693,7 @@ public final class ServiceRegistryClientImpl implements ServiceRegistryClient { callback); onClose.success(null); }, bodyBuffer -> { - MicroserviceInstanceChangedEvent response = null; + MicroserviceInstanceChangedEvent response; try { response = JsonUtils.readValue(bodyBuffer.getBytes(), MicroserviceInstanceChangedEvent.class); @@ -904,22 +906,22 @@ public final class ServiceRegistryClientImpl implements ServiceRegistryClient { } @Override - public boolean undateMicroserviceInstanceStatus(String microserviceId, String microserviceInstanceId, String status) { + public boolean updateMicroserviceInstanceStatus(String microserviceId, String instanceId, + MicroserviceInstanceStatus status) { + if (null == status) { + throw new IllegalArgumentException("null status is now allowed"); + } + Holder<HttpClientResponse> holder = new Holder<>(); IpPort ipPort = ipPortManager.getAvailableAddress(); try { - if (LOGGER.isDebugEnabled()) { - LOGGER.debug("update status of microservice instance: {}", status); - } - String url = String.format(Const.REGISTRY_API.MICROSERVICE_INSTANCE_STATUS, microserviceId, microserviceInstanceId); - if (StringUtils.isEmpty(status)) { - LOGGER.debug("empty status"); - return false; - } + LOGGER.debug("update status of microservice instance: {}", status); + String url = String.format(Const.REGISTRY_API.MICROSERVICE_INSTANCE_STATUS, microserviceId, instanceId); Map<String, String[]> queryParams = new HashMap<>(); - queryParams.put("value", new String[] {status}); + queryParams.put("value", new String[] {status.toString()}); CountDownLatch countDownLatch = new CountDownLatch(1); - RestUtils.put(ipPort, url, new RequestParam().setQueryParams(queryParams), syncHandler(countDownLatch, HttpClientResponse.class, holder)); + RestUtils.put(ipPort, url, new RequestParam().setQueryParams(queryParams), + syncHandler(countDownLatch, HttpClientResponse.class, holder)); countDownLatch.await(); if (holder.value != null) { if (holder.value.statusCode() == Status.OK.getStatusCode()) { @@ -929,9 +931,9 @@ public final class ServiceRegistryClientImpl implements ServiceRegistryClient { } } catch (Exception e) { LOGGER.error("update status of microservice instance {}/{} failed", - microserviceId, - microserviceInstanceId, - e); + microserviceId, + instanceId, + e); } return false; } 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 a1f0f03..375f299 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 @@ -17,11 +17,14 @@ package org.apache.servicecomb.serviceregistry.client; +import static org.junit.Assert.fail; + import java.io.InputStream; import java.util.List; import org.apache.servicecomb.serviceregistry.api.registry.Microservice; import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstance; +import org.apache.servicecomb.serviceregistry.api.registry.MicroserviceInstanceStatus; import org.apache.servicecomb.serviceregistry.api.registry.ServiceCenterInfo; import org.apache.servicecomb.serviceregistry.api.response.GetSchemaResponse; import org.apache.servicecomb.serviceregistry.client.http.Holder; @@ -196,5 +199,68 @@ public class LocalServiceRegistryClientImplTest { Assert.assertThat(microservice.getSchemas().size(), Is.is(1)); Assert.assertTrue(microservice.getSchemas().contains("hello")); } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus() { + List<MicroserviceInstance> m = registryClient + .findServiceInstance("", "default", "ms2", DefinitionConst.VERSION_RULE_ALL); + MicroserviceInstance instance = m.get(0); + Assert.assertEquals(MicroserviceInstanceStatus.UP, instance.getStatus()); + + boolean updateOperationResult = registryClient + .undateMicroserviceInstanceStatus(instance.getServiceId(), instance.getInstanceId(), "TESTING"); + Assert.assertTrue(updateOperationResult); + + m = registryClient + .findServiceInstance("", "default", "ms2", DefinitionConst.VERSION_RULE_ALL); + instance = m.get(0); + Assert.assertEquals(MicroserviceInstanceStatus.TESTING, instance.getStatus()); + } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus_instance_not_found() { + try { + registryClient.undateMicroserviceInstanceStatus("msIdNotExist", "", "UP"); + shouldThrowException(); + } catch (IllegalArgumentException e) { + Assert.assertEquals("Invalid serviceId, serviceId=msIdNotExist", e.getMessage()); + } + + try { + registryClient.undateMicroserviceInstanceStatus("002", "instanceIdNotExist", "UP"); + shouldThrowException(); + } catch (IllegalArgumentException e) { + Assert.assertEquals("Invalid argument. microserviceId=002, instanceId=instanceIdNotExist.", + e.getMessage()); + } + } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus_illegal_status() { + List<MicroserviceInstance> m = registryClient + .findServiceInstance("", "default", "ms2", DefinitionConst.VERSION_RULE_ALL); + MicroserviceInstance instance = m.get(0); + + try { + registryClient.undateMicroserviceInstanceStatus(instance.getServiceId(), instance.getInstanceId(), null); + shouldThrowException(); + } catch (NullPointerException e) { + Assert.assertEquals("Name is null", e.getMessage()); + } + try { + registryClient + .undateMicroserviceInstanceStatus(instance.getServiceId(), instance.getInstanceId(), "IllegalStatus"); + shouldThrowException(); + } catch (IllegalArgumentException e) { + Assert.assertEquals("Invalid status: IllegalStatus", e.getMessage()); + } + } + + private void shouldThrowException() { + fail("an exception is expected"); + } } 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 d1c855b..3a7662b 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 @@ -18,6 +18,7 @@ package org.apache.servicecomb.serviceregistry.client.http; import static org.hamcrest.core.Is.is; +import static org.junit.Assert.fail; import java.util.ArrayList; import java.util.HashMap; @@ -124,25 +125,21 @@ public class TestServiceRegistryClientImpl { 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())); + Assert.assertNull(oClient.getMicroserviceId(microservice.getAppId(), + microservice.getServiceName(), + microservice.getVersion(), + microservice.getEnvironment())); 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()); } @@ -324,14 +321,13 @@ public class TestServiceRegistryClientImpl { @Mock void httpDo(RequestContext requestContext, Handler<RestResponse> responseHandler) { Holder<GetServiceResponse> holder = Deencapsulation.getField(responseHandler, "arg$4"); - GetServiceResponse serviceResp = Json + holder.value = Json .decodeValue( "{\"service\":{\"serviceId\":\"serviceId\",\"framework\":null" + ",\"registerBy\":null,\"environment\":null,\"appId\":\"appId\",\"serviceName\":null," + "\"alias\":null,\"version\":null,\"description\":null,\"level\":null,\"schemas\":[]," + "\"paths\":[],\"status\":\"UP\",\"properties\":{},\"intance\":null}}", GetServiceResponse.class); - holder.value = serviceResp; RequestParam requestParam = requestContext.getParams(); Assert.assertEquals("global=true", requestParam.getQueryParams()); } @@ -351,11 +347,10 @@ public class TestServiceRegistryClientImpl { @Mock void httpDo(RequestContext requestContext, Handler<RestResponse> responseHandler) { Holder<GetSchemaResponse> holder = Deencapsulation.getField(responseHandler, "arg$4"); - GetSchemaResponse schemasResp = Json + holder.value = Json .decodeValue( "{ \"schema\": \"schema\", \"schemaId\":\"metricsEndpoint\",\"summary\":\"c1188d709631a9038874f9efc6eb894f\"}", GetSchemaResponse.class); - holder.value = schemasResp; RequestParam requestParam = requestContext.getParams(); Assert.assertEquals("global=true", requestParam.getQueryParams()); } @@ -545,4 +540,68 @@ public class TestServiceRegistryClientImpl { } }.run(); } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus() { + HttpClientResponse httpClientResponse = new MockUp<HttpClientResponse>() { + @Mock + int statusCode() { + return 200; + } + }.getMockInstance(); + new MockUp<RestUtils>() { + @Mock + void put(IpPort ipPort, String uri, RequestParam requestParam, Handler<RestResponse> responseHandler) { + Holder<HttpClientResponse> holder = Deencapsulation.getField(responseHandler, "arg$4"); + holder.value = httpClientResponse; + } + }; + + boolean result = oClient.undateMicroserviceInstanceStatus("svcId", "instanceId", "UP"); + Assert.assertTrue(result); + } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus_response_failure() { + HttpClientResponse httpClientResponse = new MockUp<HttpClientResponse>() { + @Mock + int statusCode() { + return 400; + } + }.getMockInstance(); + new MockUp<RestUtils>() { + @Mock + void put(IpPort ipPort, String uri, RequestParam requestParam, Handler<RestResponse> responseHandler) { + Holder<HttpClientResponse> holder = Deencapsulation.getField(responseHandler, "arg$4"); + holder.value = httpClientResponse; + } + }; + + boolean result = oClient.undateMicroserviceInstanceStatus("svcId", "instanceId", "UP"); + Assert.assertFalse(result); + } + + @SuppressWarnings("deprecation") + @Test + public void undateMicroserviceInstanceStatus_illegal_status() { + try { + oClient.undateMicroserviceInstanceStatus("svcId", "instanceId", null); + shouldThrowException(); + } catch (NullPointerException e) { + Assert.assertEquals("Name is null", e.getMessage()); + } + try { + oClient + .undateMicroserviceInstanceStatus("svcId", "instanceId", "IllegalStatus"); + shouldThrowException(); + } catch (IllegalArgumentException e) { + Assert.assertEquals("Invalid status: IllegalStatus", e.getMessage()); + } + } + + private void shouldThrowException() { + fail("an exception is expected"); + } }
