You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2020/06/02 03:39:44 UTC
[servicecomb-java-chassis] 02/05: [SCB-1971] Address PR comments
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 8343d11e1e11a7422051b2e968227341743acd16
Author: Jun Gan <ju...@gmail.com>
AuthorDate: Sun May 31 23:12:32 2020 -0400
[SCB-1971] Address PR comments
---
.../zeroconfig/ZeroConfigDiscovery.java | 5 ++-
.../zeroconfig/ZeroConfigRegistration.java | 39 ++++++++++++++-----
.../zeroconfig/ZeroConfigRegistryConstants.java | 2 +
.../zeroconfig/client/ZeroConfigClient.java | 44 +++++++++-------------
.../zeroconfig/client/TestZeroConfigClient.java | 11 +-----
5 files changed, 55 insertions(+), 46 deletions(-)
diff --git a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigDiscovery.java b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigDiscovery.java
index 3ce2871..ae7edff 100644
--- a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigDiscovery.java
+++ b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigDiscovery.java
@@ -17,6 +17,7 @@
package org.apache.servicecomb.zeroconfig;
import com.netflix.config.DynamicPropertyFactory;
+import com.sun.xml.internal.bind.v2.TODO;
import java.util.Collection;
import java.util.List;
import org.apache.servicecomb.registry.api.Discovery;
@@ -25,10 +26,11 @@ import org.apache.servicecomb.registry.api.registry.MicroserviceInstance;
import org.apache.servicecomb.registry.api.registry.MicroserviceInstances;
import org.apache.servicecomb.zeroconfig.client.ZeroConfigClient;
+import static org.apache.servicecomb.zeroconfig.ZeroConfigRegistryConstants.ENABLED;
+
public class ZeroConfigDiscovery implements Discovery {
private static final String NAME = "zero-config discovery";
- private static final String ENABLED = "servicecomb.zeroconfig.registry.discovery.enabled";
private ZeroConfigClient zeroConfigClient = ZeroConfigClient.INSTANCE;
private String revision;
@@ -68,6 +70,7 @@ public class ZeroConfigDiscovery implements Discovery {
return zeroConfigClient.getAllMicroservices();
}
+ //TODO
@Override
public String getSchema(String microserviceId, Collection<MicroserviceInstance> instances,
String schemaId) {
diff --git a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistration.java b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistration.java
index 06c0849..745f43f 100644
--- a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistration.java
+++ b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistration.java
@@ -16,8 +16,10 @@
*/
package org.apache.servicecomb.zeroconfig;
+import com.google.common.annotations.VisibleForTesting;
import com.netflix.config.DynamicPropertyFactory;
import java.util.Collection;
+import org.apache.commons.lang.StringUtils;
import org.apache.servicecomb.config.ConfigUtil;
import org.apache.servicecomb.config.archaius.sources.MicroserviceConfigLoader;
import org.apache.servicecomb.registry.api.Registration;
@@ -33,6 +35,8 @@ import org.apache.servicecomb.zeroconfig.server.ServerUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.servicecomb.zeroconfig.ZeroConfigRegistryConstants.ENABLED;
+
public class ZeroConfigRegistration implements Registration {
private static final Logger LOGGER = LoggerFactory.getLogger(ZeroConfigRegistration.class);
@@ -40,7 +44,6 @@ public class ZeroConfigRegistration implements Registration {
public static ZeroConfigRegistration INSTANCE = new ZeroConfigRegistration();
private static final String NAME = "zero-config registration";
- private static final String ENABLED = "servicecomb.zeroconfig.registry.registration.enabled";
private ZeroConfigClient zeroConfigClient = ZeroConfigClient.INSTANCE;
@@ -63,10 +66,27 @@ public class ZeroConfigRegistration implements Registration {
this.selfMicroservice = microserviceFactory.create(microserviceDefinition);
this.selfMicroserviceInstance = selfMicroservice.getInstance();
+ setServiceIdAndInstanceId();
+
ServerUtil.init();
ClientUtil.init();
}
+ private void setServiceIdAndInstanceId(){
+ // set serviceId
+ if (StringUtils.isEmpty(selfMicroservice.getServiceId())) {
+ String serviceId = ClientUtil.generateServiceId(selfMicroservice);
+ selfMicroservice.setServiceId(serviceId);
+ selfMicroserviceInstance.setServiceId(serviceId);
+ }
+
+ // set instanceId
+ if (StringUtils.isEmpty(selfMicroserviceInstance.getInstanceId())) {
+ String instanceId = ClientUtil.generateServiceInstanceId();
+ selfMicroserviceInstance.setInstanceId(instanceId);
+ }
+ }
+
@Override
public void run() {
// register service instance
@@ -134,23 +154,24 @@ public class ZeroConfigRegistration implements Registration {
}
// setter/getter
-
- public void setSelfMicroservice(
- Microservice selfMicroservice) {
- this.selfMicroservice = selfMicroservice;
- }
-
public Microservice getSelfMicroservice() {
return this.selfMicroservice;
}
+ public MicroserviceInstance getSelfMicroserviceInstance() {
+ return this.selfMicroserviceInstance;
+ }
+
+ @VisibleForTesting
public void setSelfMicroserviceInstance(
MicroserviceInstance selfMicroserviceInstance) {
this.selfMicroserviceInstance = selfMicroserviceInstance;
}
- public MicroserviceInstance getSelfMicroserviceInstance() {
- return this.selfMicroserviceInstance;
+ @VisibleForTesting
+ public void setSelfMicroservice(
+ Microservice selfMicroservice) {
+ this.selfMicroservice = selfMicroservice;
}
}
diff --git a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistryConstants.java b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistryConstants.java
index 417b958..412912f 100644
--- a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistryConstants.java
+++ b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/ZeroConfigRegistryConstants.java
@@ -18,6 +18,8 @@ package org.apache.servicecomb.zeroconfig;
public interface ZeroConfigRegistryConstants {
+ String ENABLED = "servicecomb.zeroconfig.registry.enabled";
+
// MulticastSocket related
String GROUP = "225.0.0.0";
Integer PORT = 6666;
diff --git a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/client/ZeroConfigClient.java b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/client/ZeroConfigClient.java
index 23e290a..83c5550 100644
--- a/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/client/ZeroConfigClient.java
+++ b/service-registry/registry-zero-config/src/main/java/org/apache/servicecomb/zeroconfig/client/ZeroConfigClient.java
@@ -16,6 +16,7 @@
*/
package org.apache.servicecomb.zeroconfig.client;
+import com.google.common.annotations.VisibleForTesting;
import java.io.IOException;
import java.net.DatagramPacket;
import java.net.InetAddress;
@@ -56,13 +57,24 @@ public class ZeroConfigClient {
private MulticastSocket multicastSocket;
// Constructor
- public ZeroConfigClient(ZeroConfigRegistryService zeroConfigRegistryService,
+
+ private ZeroConfigClient(ZeroConfigRegistryService zeroConfigRegistryService,
MulticastSocket multicastSocket, RestTemplate restTemplate) {
this.zeroConfigRegistryService = zeroConfigRegistryService;
this.restTemplate = restTemplate;
this.multicastSocket = multicastSocket;
}
+ @VisibleForTesting
+ public ZeroConfigClient initZeroConfigClientWithMocked(
+ ZeroConfigRegistryService zeroConfigRegistryService,
+ MulticastSocket multicastSocket, RestTemplate restTemplate) {
+ this.zeroConfigRegistryService = zeroConfigRegistryService;
+ this.multicastSocket = multicastSocket;
+ this.restTemplate = restTemplate;
+ return this;
+ }
+
// builder method
private static ZeroConfigClient buildZeroConfigClient() {
MulticastSocket multicastSocket = null;
@@ -263,34 +275,14 @@ public class ZeroConfigClient {
}
private Map<String, String> prepareRegisterData() {
- // set serviceId
- Microservice selfMicroservice = ZeroConfigRegistration.INSTANCE.getSelfMicroservice();
- MicroserviceInstance selfMicroserviceInstance = ZeroConfigRegistration.INSTANCE
+ // Convert to Multicast data format
+ Microservice selfService = ZeroConfigRegistration.INSTANCE.getSelfMicroservice();
+ MicroserviceInstance selfInstance = ZeroConfigRegistration.INSTANCE
.getSelfMicroserviceInstance();
- if (selfMicroservice == null || selfMicroserviceInstance == null) {
- return null;
- }
-
- String serviceId = selfMicroservice.getServiceId();
- if (StringUtils.isEmpty(serviceId)) {
- serviceId = ClientUtil.generateServiceId(selfMicroservice);
- selfMicroservice.setServiceId(serviceId);
- selfMicroserviceInstance.setServiceId(serviceId);
- }
-
- // set instanceId
- String instanceId = selfMicroserviceInstance
- .getInstanceId(); // allow client to set the instanceId
- if (StringUtils.isEmpty(instanceId)) {
- instanceId = ClientUtil.generateServiceInstanceId();
- selfMicroserviceInstance.setInstanceId(instanceId);
- }
-
- // Convert to Multicast data format
Optional<Map<String, String>> optionalDataMap = ClientUtil
- .convertToRegisterDataModel(serviceId, instanceId, selfMicroserviceInstance,
- selfMicroservice);
+ .convertToRegisterDataModel(selfService.getServiceId(), selfInstance.getInstanceId(),
+ selfInstance, selfService);
return optionalDataMap.orElse(null);
}
diff --git a/service-registry/registry-zero-config/src/test/java/org/apache/servicecomb/zeroconfig/client/TestZeroConfigClient.java b/service-registry/registry-zero-config/src/test/java/org/apache/servicecomb/zeroconfig/client/TestZeroConfigClient.java
index e77e715..025fcef 100644
--- a/service-registry/registry-zero-config/src/test/java/org/apache/servicecomb/zeroconfig/client/TestZeroConfigClient.java
+++ b/service-registry/registry-zero-config/src/test/java/org/apache/servicecomb/zeroconfig/client/TestZeroConfigClient.java
@@ -72,7 +72,7 @@ public class TestZeroConfigClient {
@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
- target = new ZeroConfigClient(zeroConfigRegistryService, multicastSocket, restTemplate);
+ target = ZeroConfigClient.INSTANCE.initZeroConfigClientWithMocked(zeroConfigRegistryService, multicastSocket, restTemplate);
prepareSelfMicroserviceAndInstance();
}
@@ -137,15 +137,6 @@ public class TestZeroConfigClient {
}
@Test
- public void test_register_withWrongData_RegisterShouldFail() {
- ZeroConfigRegistration.INSTANCE.setSelfMicroservice(null);
-
- boolean returnedResult = target.register();
-
- Assert.assertFalse(returnedResult);
- }
-
- @Test
public void test_register_MulticastThrowException_RegisterShouldFail() throws IOException {
doThrow(IOException.class).when(multicastSocket).send(anyObject());