Skip to content

JAV-282 Auto choose same name transport #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 9, 2017
Merged

JAV-282 Auto choose same name transport #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b0eadcb
JAV-282 transport manager can choose a better one when same transport…
wujimin Aug 8, 2017
577002e
make servlet rest listen config rule equal other transport
wujimin Aug 8, 2017
569e99b
JAV-282 create a util function to check if a address can listen with …
wujimin Aug 8, 2017
4238b43
JAV-282 servlet and vertx rest transport can be depended in the same …
wujimin Aug 8, 2017
f1f9b22
JAV-282 modify demo dependence
wujimin Aug 8, 2017
dd17867
Merge branch 'master' into auto-choose-same-name-transport
wujimin Aug 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion core/src/main/java/io/servicecomb/core/Transport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
public interface Transport {
String getName();

default int getOrder() {
return 0;
}

default boolean canInit() {
return true;
}

boolean init() throws Exception;

/**
Expand All @@ -34,7 +42,6 @@ public interface Transport {
*/
Endpoint getEndpoint() throws Exception;


/**
* 用于上报到服务中心，要求是其他节点可访问的地址
*/
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/io/servicecomb/core/transport/AbstractTransport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public Endpoint getPublishEndpoint() {
}

@Override
public Endpoint getEndpoint() throws Exception {
public Endpoint getEndpoint() {
return endpoint;
}

Expand Down Expand Up @@ -99,7 +99,7 @@ protected void setListenAddressWithoutSchema(String addressWithoutSchema,
}
this.endpoint = new Endpoint(this, NetUtils.getRealListenAddress(getName(), addressWithoutSchema));
if (this.endpoint.getEndpoint() != null) {
this.publishEndpoint = new Endpoint(this, RegistryUtils.getPublishAddress(getName(),
this.publishEndpoint = new Endpoint(this, RegistryUtils.getPublishAddress(getName(),
addressWithoutSchema));
} else {
this.publishEndpoint = null;
Expand Down
68 changes: 66 additions & 2 deletions core/src/main/java/io/servicecomb/core/transport/TransportManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@

package io.servicecomb.core.transport;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;

import javax.inject.Inject;

Expand All @@ -28,6 +30,7 @@

import io.servicecomb.core.Endpoint;
import io.servicecomb.core.Transport;
import io.servicecomb.foundation.common.exceptions.ServiceCombException;
import io.servicecomb.serviceregistry.RegistryUtils;
import io.servicecomb.serviceregistry.api.registry.Microservice;

Expand All @@ -36,12 +39,18 @@ public class TransportManager {
private static final Logger LOGGER = LoggerFactory.getLogger(TransportManager.class);

@Inject
private List<Transport> transportList;
private List<Transport> transports;

private Map<String, Transport> transportMap = new HashMap<>();

public void setTransports(List<Transport> transports) {
this.transports = transports;
}

public void init() throws Exception {
for (Transport transport : transportList) {
buildTransportMap();

for (Transport transport : transportMap.values()) {
transportMap.put(transport.getName(), transport);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

多了一条语句

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes
unused code, and will not cause problem
when i fetch and rebase, delete this line, then i can not push, git said that: rejected ..... (stale info)

i will delete this line in next PR.


if (transport.init()) {
Expand All @@ -56,6 +65,61 @@ public void init() throws Exception {
}
}

protected void buildTransportMap() {
Map<String, List<Transport>> groups = groupByName();

for (Entry<String, List<Transport>> entry : groups.entrySet()) {
List<Transport> group = entry.getValue();

checkTransportGroup(group);
Transport transport = chooseOneTransport(group);
if (transport != null) {
transportMap.put(transport.getName(), transport);
}
}
}

protected Transport chooseOneTransport(List<Transport> group) {
group.sort((t1, t2) -> {
return t1.getOrder() - t2.getOrder();
});

for (Transport transport : group) {
if (transport.canInit()) {
LOGGER.info("choose {} for {}.", transport.getClass().getName(), transport.getName());
return transport;
}
}

LOGGER.info("all transport named {} refused to init.", group.get(0).getName());
return null;
}

protected void checkTransportGroup(List<Transport> group) {
// order value must be different, otherwise, maybe will choose a random transport
Map<Integer, Transport> orderMap = new HashMap<>();
for (Transport transport : group) {
Transport existTransport = orderMap.putIfAbsent(transport.getOrder(), transport);
if (existTransport != null) {
throw new ServiceCombException(String.format("%s and %s have the same order %d",
existTransport.getClass().getName(),
transport.getClass().getName(),
transport.getOrder()));
}
}
}

protected Map<String, List<Transport>> groupByName() {
Map<String, List<Transport>> groups = new HashMap<>();
for (Transport transport : transports) {
List<Transport> list = groups.computeIfAbsent(transport.getName(), name -> {
return new ArrayList<>();
});
list.add(transport);
}
return groups;
}

public Transport findTransport(String transportName) {
return transportMap.get(transportName);
}
Expand Down
176 changes: 168 additions & 8 deletions core/src/test/java/io/servicecomb/core/transport/TestTransportManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,18 @@

package io.servicecomb.core.transport;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

import org.junit.Assert;
import org.junit.Test;

import io.servicecomb.core.Endpoint;
import io.servicecomb.core.Transport;
import io.servicecomb.foundation.common.exceptions.ServiceCombException;
import io.servicecomb.serviceregistry.RegistryUtils;
import io.servicecomb.serviceregistry.api.registry.MicroserviceInstance;
import mockit.Deencapsulation;
import mockit.Expectations;
import mockit.Injectable;
import mockit.Mocked;
Expand All @@ -40,13 +41,14 @@ public void testTransportManagerInitFail(@Injectable Transport transport) throws
result = "test";
transport.init();
result = false;
transport.canInit();
result = true;
}
};
List<Transport> transports = new ArrayList<>();
transports.add(transport);
List<Transport> transports = Arrays.asList(transport);

TransportManager manager = new TransportManager();
Deencapsulation.setField(manager, "transportList", transports);
manager.setTransports(transports);

manager.init();
Assert.assertEquals(manager.findTransport("test"), transport);
Expand All @@ -59,19 +61,177 @@ public void testTransportManagerInitSucc(@Injectable Transport transport, @Injec
{
transport.getName();
result = "test";
transport.canInit();
result = true;
transport.init();
result = true;
transport.getPublishEndpoint();
result = endpoint;
}
};
List<Transport> transports = new ArrayList<>();
transports.add(transport);
List<Transport> transports = Arrays.asList(transport);

TransportManager manager = new TransportManager();
Deencapsulation.setField(manager, "transportList", transports);
manager.setTransports(transports);

manager.init();
Assert.assertEquals(manager.findTransport("test"), transport);
}

@Test
public void testGroupByName(@Mocked Transport t1, @Mocked Transport t2_1, @Mocked Transport t2_2) {
new Expectations() {
{
t1.getName();
result = "t1";

t2_1.getName();
result = "t2";
t2_2.getName();
result = "t2";
}
};

TransportManager manager = new TransportManager();
manager.setTransports(Arrays.asList(t1, t2_1, t2_2));

Map<String, List<Transport>> groups = manager.groupByName();
Assert.assertEquals(2, groups.size());
Assert.assertEquals(1, groups.get("t1").size());
Assert.assertEquals(t1, groups.get("t1").get(0));
Assert.assertEquals(2, groups.get("t2").size());
Assert.assertEquals(t2_1, groups.get("t2").get(0));
Assert.assertEquals(t2_2, groups.get("t2").get(1));
}

@Test
public void testCheckTransportGroupInvalid(@Mocked Transport t1, @Mocked Transport t2) {
new Expectations() {
{
t1.getOrder();
result = 1;

t2.getOrder();
result = 1;
}
};

TransportManager manager = new TransportManager();
List<Transport> group = Arrays.asList(t1, t2);

try {
manager.checkTransportGroup(group);
Assert.fail("must throw exception");
} catch (ServiceCombException e) {
Assert.assertEquals(
"io.servicecomb.core.$Impl_Transport and io.servicecomb.core.$Impl_Transport have the same order 1",
e.getMessage());
}
}

@Test
public void testCheckTransportGroupValid(@Mocked Transport t1, @Mocked Transport t2) {
new Expectations() {
{
t1.getOrder();
result = 1;

t2.getOrder();
result = 2;
}
};

TransportManager manager = new TransportManager();
List<Transport> group = Arrays.asList(t1, t2);

try {
manager.checkTransportGroup(group);
} catch (ServiceCombException e) {
Assert.fail("must not throw exception: " + e.getMessage());
}
}

@Test
public void testChooseOneTransportFirst(@Mocked Transport t1, @Mocked Transport t2) {
new Expectations() {
{
t1.getOrder();
result = 1;
t1.canInit();
result = true;

t2.getOrder();
result = 2;
}
};

TransportManager manager = new TransportManager();
List<Transport> group = Arrays.asList(t1, t2);

Assert.assertEquals(t1, manager.chooseOneTransport(group));
}

@Test
public void testChooseOneTransportSecond(@Mocked Transport t1, @Mocked Transport t2) {
new Expectations() {
{
t1.getOrder();
result = 1;
t1.canInit();
result = false;

t2.getOrder();
result = 2;
t2.canInit();
result = true;
}
};

TransportManager manager = new TransportManager();
List<Transport> group = Arrays.asList(t1, t2);

Assert.assertEquals(t2, manager.chooseOneTransport(group));
}

@Test
public void testChooseOneTransportNone(@Mocked Transport t1, @Mocked Transport t2) {
new Expectations() {
{
t1.getName();
result = "t";
t1.getOrder();
result = 1;
t1.canInit();
result = false;

t2.getOrder();
result = 2;
t2.canInit();
result = false;
}
};

TransportManager manager = new TransportManager();
List<Transport> group = Arrays.asList(t1, t2);

Assert.assertNull(manager.chooseOneTransport(group));
}

@Test
public void testBuildTransportMapChooseNull(@Mocked Transport t1) {
new Expectations() {
{
t1.getName();
result = "t1";
t1.canInit();
result = false;
}
};

TransportManager manager = new TransportManager();
manager.setTransports(Arrays.asList(t1));

manager.buildTransportMap();
Assert.assertEquals(null, manager.findTransport("t1"));
}
}
4 changes: 4 additions & 0 deletions demo/demo-schema/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@
<groupId>io.servicecomb</groupId>
<artifactId>transport-rest-vertx</artifactId>
</dependency>
<dependency>
<groupId>io.servicecomb</groupId>
<artifactId>transport-rest-servlet</artifactId>
</dependency>
<dependency>
<groupId>javax.servlet</groupId>
<artifactId>javax.servlet-api</artifactId>
Expand Down
11 changes: 0 additions & 11 deletions demo/demo-server-servlet/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,9 @@
<packaging>war</packaging>

<dependencies>
<dependency>
<groupId>io.servicecomb</groupId>
<artifactId>transport-rest-servlet</artifactId>
</dependency>

<dependency>
<groupId>io.servicecomb.demo</groupId>
<artifactId>pojo-server</artifactId>
<exclusions>
<exclusion>
<groupId>io.servicecomb</groupId>
<artifactId>transport-rest-vertx</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

Expand Down
Loading