Skip to content

Commit 5ddda8b

Browse files
committed
Align target creation logic with existing logic
1 parent 7f505c2 commit 5ddda8b

2 files changed

Lines changed: 14 additions & 23 deletions

File tree

src/main/java/emissary/grpc/channel/ChannelManager.java

Lines changed: 10 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import io.grpc.ChannelCredentials;
66
import io.grpc.Grpc;
77
import io.grpc.ManagedChannel;
8+
import org.apache.commons.lang3.StringUtils;
89
import org.slf4j.Logger;
910
import org.slf4j.LoggerFactory;
1011

@@ -33,8 +34,6 @@ public abstract class ChannelManager implements AutoCloseable {
3334
public static final String MAX_INBOUND_MESSAGE_BYTE_SIZE = GRPC_CHANNEL_PREFIX + "MAX_INBOUND_MESSAGE_BYTE_SIZE";
3435
public static final String MAX_INBOUND_METADATA_BYTE_SIZE = GRPC_CHANNEL_PREFIX + "MAX_INBOUND_METADATA_BYTE_SIZE";
3536

36-
protected static final String LOCALHOST = "localhost";
37-
protected static final String DNS_PREFIX = "dns:///";
3837
protected static final int MAX_PORT_NUMBER = 0xFFFF;
3938

4039
protected final Logger logger;
@@ -65,9 +64,9 @@ public abstract class ChannelManager implements AutoCloseable {
6564
protected ChannelManager(String host, int port, Configurator configG, ChannelCredentials credentials) {
6665
this.logger = LoggerFactory.getLogger(this.getClass().getName());
6766

68-
this.host = validateHostName(host);
69-
this.port = validatePortNumber(port);
70-
this.target = createTarget(host, port);
67+
this.host = host;
68+
this.port = port;
69+
this.target = createTarget();
7170

7271
// How often (in milliseconds) to send pings when the connection is idle
7372
this.keepAliveMillis = configG.findLongEntry(KEEP_ALIVE_MILLIS, 60000L);
@@ -91,23 +90,14 @@ protected ChannelManager(String host, int port, Configurator configG, ChannelCre
9190
this.channelCredentials = credentials;
9291
}
9392

94-
protected String validateHostName(String host) {
95-
if (host.equals(LOCALHOST) || host.startsWith(DNS_PREFIX)) {
96-
return host;
93+
private String createTarget() {
94+
if (StringUtils.isEmpty(host)) {
95+
throw new IllegalArgumentException("Missing required gRPC host configuration");
9796
}
98-
throw new IllegalArgumentException(
99-
String.format("Expected \"%s\" or DNS URI prefix \"%s\" but got \"%s\"", LOCALHOST, DNS_PREFIX, host));
100-
}
101-
102-
protected int validatePortNumber(int port) {
103-
if (port > 0 && port <= MAX_PORT_NUMBER) {
104-
return port;
97+
if (port <= 0 || port > MAX_PORT_NUMBER) {
98+
throw new IllegalArgumentException(
99+
String.format("Port \"%d\" is outside valid range [1, %d]", port, MAX_PORT_NUMBER));
105100
}
106-
throw new IllegalArgumentException(
107-
String.format("Port \"%d\" is outside valid range [1, %d]", port, MAX_PORT_NUMBER));
108-
}
109-
110-
protected String createTarget(String host, int port) {
111101
return host + ":" + port;
112102
}
113103

src/test/java/emissary/grpc/channel/ChannelManagerTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import io.grpc.InsecureChannelCredentials;
77
import io.grpc.ManagedChannel;
88
import org.junit.jupiter.params.ParameterizedTest;
9+
import org.junit.jupiter.params.provider.NullAndEmptySource;
910
import org.junit.jupiter.params.provider.ValueSource;
1011

1112
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
@@ -14,18 +15,18 @@
1415

1516
class ChannelManagerTest extends UnitTest {
1617
@ParameterizedTest
17-
@ValueSource(strings = {"localhost", "dns:///foo.bar"})
18+
@ValueSource(strings = {"localhost", "dns:///foo.bar", "foo.bar"})
1819
void testGoodHostName(String host) {
1920
Runnable invocation = () -> new TestChannelManager(host, 1).close();
2021
assertDoesNotThrow(invocation::run);
2122
}
2223

2324
@ParameterizedTest
24-
@ValueSource(strings = {"dns:foo.bar", "dns:/foo.bar", "dns://foo.bar", "http:///foo.bar", "https:///foo.bar", "foo.bar"})
25+
@NullAndEmptySource
2526
void testBadHostName(String host) {
2627
Runnable invocation = () -> new TestChannelManager(host, 1).close();
2728
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, invocation::run);
28-
assertEquals(String.format("Expected \"localhost\" or DNS URI prefix \"dns:///\" but got \"%s\"", host), e.getMessage());
29+
assertEquals("Missing required gRPC host configuration", e.getMessage());
2930
}
3031

3132
@ParameterizedTest

0 commit comments

Comments
 (0)