diff --git a/core/src/main/java/io/confluent/rest/Application.java b/core/src/main/java/io/confluent/rest/Application.java index 302a463a91..9a49bc61c8 100644 --- a/core/src/main/java/io/confluent/rest/Application.java +++ b/core/src/main/java/io/confluent/rest/Application.java @@ -19,6 +19,8 @@ import com.fasterxml.jackson.jaxrs.base.JsonParseExceptionMapper; import io.confluent.common.config.ConfigException; + +import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.NetworkTrafficServerConnector; import org.eclipse.jetty.server.Server; @@ -121,6 +123,12 @@ public Server createServer() throws RestConfigException { ServletContainer servletContainer = new ServletContainer(resourceConfig); ServletHolder servletHolder = new ServletHolder(servletContainer); server = new Server() { + @Override + protected void doStart() throws Exception { + super.doStart(); + Application.this.onStarted(); + } + @Override protected void doStop() throws Exception { super.doStop(); @@ -239,6 +247,26 @@ protected void doStop() throws Exception { return server; } + /** + * Get the local ports that were bound for the listeners. + * @return a List containing the local ports + */ + public List localPorts() { + List ports = new ArrayList<>(); + for(Connector connector : server.getConnectors()) { + if (!(connector instanceof NetworkTrafficServerConnector)) { + throw new RuntimeException( + String.format( + "Expected NetworkTrafficServerConnector for listener but found %s", + connector.getClass() + ) + ); + } + ports.add(((NetworkTrafficServerConnector) connector).getLocalPort()); + } + return ports; + } + // TODO: delete deprecatedPort parameter when `PORT_CONFIG` is deprecated. It's only used to support the deprecated // configuration. public static List parseListeners(List listenersConfig, int deprecatedPort, @@ -352,6 +380,10 @@ public void stop() throws Exception { server.stop(); } + public void onStarted() { + // Intentionally left blank + } + /** * Shutdown hook that is invoked after the Jetty server has processed the shutdown request, * stopped accepting new connections, and tried to gracefully finish existing requests. At this diff --git a/core/src/test/java/io/confluent/rest/ApplicationTest.java b/core/src/test/java/io/confluent/rest/ApplicationTest.java index a8e1e27b53..51466711ff 100644 --- a/core/src/test/java/io/confluent/rest/ApplicationTest.java +++ b/core/src/test/java/io/confluent/rest/ApplicationTest.java @@ -16,15 +16,21 @@ package io.confluent.rest; +import io.confluent.common.config.ConfigDef; import io.confluent.common.config.ConfigException; import org.junit.Test; import java.net.URI; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import java.util.Map; + +import javax.ws.rs.core.Configurable; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class ApplicationTest { @Test @@ -79,6 +85,52 @@ public void testParseListenersNoPort() { List listeners = Application.parseListeners(listenersConfig, -1, Arrays.asList("http", "https"), "http"); } + @Test + public void zeroForPortShouldHaveNonZeroLocalPort() throws Exception { + TestRestConfig config = new TestRestConfig( + Collections.singletonMap(RestConfig.PORT_CONFIG, "0") + ); + TestApplication testApp = new TestApplication(config); + testApp.start(); + List localPorts = testApp.localPorts(); + assertEquals(1, localPorts.size()); + // Validate not only that it isn't zero, but also a valid value + assertTrue("Should have a valid local port value greater than 0", localPorts.get(0) > 0); + assertTrue( + "Should have a valid local port value less than or equal to 65535", + localPorts.get(0) <= 0xFFFF + ); + testApp.stop(); + testApp.join(); + } + + private class TestApplication extends Application { + TestApplication(TestRestConfig config) { + super(config); + } + + @Override + public void setupResources(Configurable config, TestRestConfig appConfig) { + // intentionally left blank + } + } + + private static class TestRestConfig extends RestConfig { + private static ConfigDef config; + + static { + config = baseConfigDef(); + } + + public TestRestConfig() { + super(config); + } + + public TestRestConfig(Map props) { + super(config, props); + } + } + private void assertExpectedUri(URI uri, String scheme, String host, int port) { assertEquals("Scheme should be " + scheme, scheme, uri.getScheme()); assertEquals("Host should be " + host, host, uri.getHost()); diff --git a/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java b/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java index cf8f3c4bc7..8165b2df48 100644 --- a/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java +++ b/core/src/test/java/io/confluent/rest/ExceptionHandlingTest.java @@ -51,6 +51,9 @@ public class ExceptionHandlingTest { public void setUp() throws Exception { Properties props = new Properties(); props.setProperty("debug", "false"); + // Override normal port to 0 for unit tests to use a random, available, ephemeral port that + // won't conflict with locally running services or parallel tests + props.setProperty(RestConfig.PORT_CONFIG, "0"); config = new TestRestConfig(props); app = new ExceptionApplication(config); app.start(); @@ -65,7 +68,7 @@ public void tearDown() throws Exception { private void testGetException(String path, int expectedStatus, int expectedErrorCode, String expectedMessage) { Response response = ClientBuilder.newClient(app.resourceConfig.getConfiguration()) - .target("http://localhost:" + config.getInt(RestConfig.PORT_CONFIG)) + .target("http://localhost:" + app.localPorts().get(0)) .path(path) .request() .get(); @@ -79,7 +82,7 @@ private void testGetException(String path, int expectedStatus, int expectedError private void testPostException(String path, Entity entity, int expectedStatus, int expectedErrorCode, String expectedMessage) { Response response = ClientBuilder.newClient(app.resourceConfig.getConfiguration()) - .target("http://localhost:" + config.getInt(RestConfig.PORT_CONFIG)) + .target("http://localhost:" + app.localPorts().get(0)) .path(path) .request() .post(entity); diff --git a/core/src/test/java/io/confluent/rest/ShutdownTest.java b/core/src/test/java/io/confluent/rest/ShutdownTest.java index 5edfcb0a13..9c878cf356 100644 --- a/core/src/test/java/io/confluent/rest/ShutdownTest.java +++ b/core/src/test/java/io/confluent/rest/ShutdownTest.java @@ -35,6 +35,9 @@ public class ShutdownTest { public void testShutdownHook() throws Exception { Properties props = new Properties(); props.put("shutdown.graceful.ms", "50"); + // Override normal port to 0 for unit tests to use a random, available, ephemeral port that + // won't conflict with locally running services or parallel tests + props.put(RestConfig.PORT_CONFIG, "0"); ShutdownApplication app = new ShutdownApplication(new TestRestConfig(props)); app.start(); @@ -49,11 +52,14 @@ public void testShutdownHook() throws Exception { public void testGracefulShutdown() throws Exception { Properties props = new Properties(); props.put("shutdown.graceful.ms", "50"); + // Override normal port to 0 for unit tests to use a random, available, ephemeral port that + // won't conflict with locally running services or parallel tests + props.put(RestConfig.PORT_CONFIG, "0"); final TestRestConfig config = new TestRestConfig(props); ShutdownApplication app = new ShutdownApplication(config); app.start(); - RequestThread req = new RequestThread(config); + RequestThread req = new RequestThread(app.localPorts().get(0)); req.start(); app.resource.requestProcessingStarted.await(); @@ -115,12 +121,12 @@ public void run() { }; private static class RequestThread extends Thread { - RestConfig config; + int port; boolean finished = false; String response = null; - RequestThread(RestConfig config) { - this.config = config; + RequestThread(int port) { + this.port = port; } @Override public void run() { @@ -131,7 +137,7 @@ public void run() { try { Client client = ClientBuilder.newClient(); response = client - .target("http://localhost:" + config.getInt(RestConfig.PORT_CONFIG)) + .target("http://localhost:" + port) .path("/") .request() .get(String.class);