More concise port and TLS configuration

Motivation: - A user currently has to specify SessionProtocol when adding a port, which increases the chance of specifying incorrect SessionProtocol. - sslContext() builder methods require SessionProtocol which is practically unncessary. - It would be confusing if Server serves plaintext HTTP when a user configured TLS even if he or she did not specify a port. More sensible behavior would be serving HTTPS when no port is specified in this case. Modifications: - Add http() and https() which supercede most port() methods. - Add tls() which supercede sslContext() methods. - Serve HTTPS by default when a user called tls() and no port was specified. - Add log messages that indicates the protocol and local address. - Deprecate the old methods. - Update documentation. - Miscellaneous: - Remove redundant `port(0, HTTP)` calls - Add `@Nullable` where necessary Result: - Fixes #1050 - More user-friendliness
line · trustin · Mar 6, 2018 · Mar 6, 2018 · Mar 6, 2018 · 8df57d6d3f04d5020cd6d622743df0a3767e1b88
commit 8df57d6d3f04d5020cd6d622743df0a3767e1b88
diff --git a/benchmarks/src/jmh/java/com/linecorp/armeria/core/HttpServerBenchmark.java b/benchmarks/src/jmh/java/com/linecorp/armeria/core/HttpServerBenchmark.java
@@ -67,7 +67,6 @@ String uriText() {
     @Setup
     public void startServer() throws Exception {
         server = new ServerBuilder()
-                .port(0, HTTP)
                 .service("/empty", ((ctx, req) -> HttpResponse.of(HttpStatus.OK)))
                 .defaultRequestTimeout(Duration.ZERO)
                 .meterRegistry(NoopMeterRegistry.get())

diff --git a/benchmarks/src/jmh/java/com/linecorp/armeria/grpc/downstream/DownstreamSimpleBenchmark.java b/benchmarks/src/jmh/java/com/linecorp/armeria/grpc/downstream/DownstreamSimpleBenchmark.java
@@ -59,7 +59,6 @@ protected GithubServiceFutureStub normalFutureClient() {
     @Override
     protected void setUp() throws Exception {
         server = new ServerBuilder()
-                .port(0, HTTP)
                 .serviceUnder("/", new GrpcServiceBuilder().addService(new GithubApiService()).build())
                 .build();
         server.start().join();

diff --git a/benchmarks/src/jmh/java/com/linecorp/armeria/grpc/downstream/LargePayloadBenchmark.java b/benchmarks/src/jmh/java/com/linecorp/armeria/grpc/downstream/LargePayloadBenchmark.java
@@ -67,7 +67,6 @@ public class LargePayloadBenchmark {
     @Setup
     public void setUp() {
         server = new ServerBuilder()
-                .port(0, HTTP)
                 .serviceUnder("/", new GrpcServiceBuilder().addService(
                         new BinaryProxyImplBase() {
                             @Override

diff --git a/benchmarks/src/jmh/java/com/linecorp/armeria/thrift/PooledResponseBufferBenchmark.java b/benchmarks/src/jmh/java/com/linecorp/armeria/thrift/PooledResponseBufferBenchmark.java
@@ -141,7 +141,6 @@ public void onComplete() {
     @Setup
     public void startServer() throws Exception {
         ServerBuilder sb = new ServerBuilder()
-                .port(0, HTTP)
                 .service("/a", THttpService.of(
                         (AsyncIface) (name, resultHandler) ->
                                 resultHandler.onComplete(RESPONSE))

diff --git a/core/src/main/java/com/linecorp/armeria/server/AbstractVirtualHostBuilder.java b/core/src/main/java/com/linecorp/armeria/server/AbstractVirtualHostBuilder.java
@@ -29,6 +29,7 @@
 import java.util.List;
 import java.util.function.Function;
 
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLException;
 
 import org.slf4j.Logger;
@@ -119,7 +120,9 @@ abstract class AbstractVirtualHostBuilder<B extends AbstractVirtualHostBuilder>
     private final String defaultHostname;
     private final String hostnamePattern;
     private final List<ServiceConfig> services = new ArrayList<>();
+    @Nullable
     private SslContext sslContext;
+    @Nullable
     private Function<Service<HttpRequest, HttpResponse>, Service<HttpRequest, HttpResponse>> decorator;
 
     /**
@@ -162,17 +165,54 @@ abstract class AbstractVirtualHostBuilder<B extends AbstractVirtualHostBuilder>
     }
 
     /**
-     * Sets the {@link SslContext} of this {@link VirtualHost}.
+     * Configures SSL or TLS of this {@link VirtualHost} with the specified {@link SslContext}.
      */
-    public B sslContext(SslContext sslContext) {
+    public B tls(SslContext sslContext) {
         this.sslContext = VirtualHost.validateSslContext(requireNonNull(sslContext, "sslContext"));
         return self();
     }
 
+    /**
+     * Configures SSL or TLS of this {@link VirtualHost} with the specified {@code keyCertChainFile}
+     * and cleartext {@code keyFile}.
+     */
+    public B tls(File keyCertChainFile, File keyFile) throws SSLException {
+        tls(keyCertChainFile, keyFile, null);
+        return self();
+    }
+
+    /**
+     * Configures SSL or TLS of this {@link VirtualHost} with the specified {@code keyCertChainFile},
+     * {@code keyFile} and {@code keyPassword}.
+     */
+    public B tls(File keyCertChainFile, File keyFile, @Nullable String keyPassword) throws SSLException {
+        final SslContextBuilder builder = SslContextBuilder.forServer(keyCertChainFile, keyFile, keyPassword);
+
+        builder.sslProvider(Flags.useOpenSsl() ? SslProvider.OPENSSL : SslProvider.JDK);
+        builder.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE);
+        builder.applicationProtocolConfig(HTTPS_ALPN_CFG);
+
+        tls(builder.build());
+        return self();
+    }
+
+    /**
+     * Sets the {@link SslContext} of this {@link VirtualHost}.
+     *
+     * @deprecated Use {@link #tls(SslContext)}.
+     */
+    @Deprecated
+    public B sslContext(SslContext sslContext) {
+        return tls(sslContext);
+    }
+
     /**
      * Sets the {@link SslContext} of this {@link VirtualHost} from the specified {@link SessionProtocol},
      * {@code keyCertChainFile} and cleartext {@code keyFile}.
+     *
+     * @deprecated Use {@link #tls(File, File)}.
      */
+    @Deprecated
     public B sslContext(
             SessionProtocol protocol, File keyCertChainFile, File keyFile) throws SSLException {
         sslContext(protocol, keyCertChainFile, keyFile, null);
@@ -182,23 +222,19 @@ public B sslContext(
     /**
      * Sets the {@link SslContext} of this {@link VirtualHost} from the specified {@link SessionProtocol},
      * {@code keyCertChainFile}, {@code keyFile} and {@code keyPassword}.
+     *
+     * @deprecated Use {@link #tls(File, File, String)}.
      */
+    @Deprecated
     public B sslContext(
             SessionProtocol protocol,
-            File keyCertChainFile, File keyFile, String keyPassword) throws SSLException {
+            File keyCertChainFile, File keyFile, @Nullable String keyPassword) throws SSLException {
 
         if (requireNonNull(protocol, "protocol") != SessionProtocol.HTTPS) {
             throw new IllegalArgumentException("unsupported protocol: " + protocol);
         }
 
-        final SslContextBuilder builder = SslContextBuilder.forServer(keyCertChainFile, keyFile, keyPassword);
-
-        builder.sslProvider(Flags.useOpenSsl() ? SslProvider.OPENSSL : SslProvider.JDK);
-        builder.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE);
-        builder.applicationProtocolConfig(HTTPS_ALPN_CFG);
-
-        sslContext(builder.build());
-        return self();
+        return tls(keyCertChainFile, keyFile, keyPassword);
     }
 
     /**

diff --git a/core/src/main/java/com/linecorp/armeria/server/Server.java b/core/src/main/java/com/linecorp/armeria/server/Server.java
@@ -64,7 +64,6 @@
 import io.netty.channel.EventLoopGroup;
 import io.netty.handler.ssl.SslContext;
 import io.netty.util.DomainNameMapping;
-import io.netty.util.DomainNameMappingBuilder;
 import io.netty.util.concurrent.FastThreadLocalThread;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.GlobalEventExecutor;
@@ -80,6 +79,7 @@ public final class Server implements AutoCloseable {
     private static final Logger logger = LoggerFactory.getLogger(Server.class);
 
     private final ServerConfig config;
+    @Nullable
     private final DomainNameMapping<SslContext> sslContexts;
 
     private final StateManager stateManager = new StateManager();
@@ -92,6 +92,7 @@ public final class Server implements AutoCloseable {
 
     private final ConnectionLimitingHandler connectionLimitingHandler;
 
+    @Nullable
     private volatile ServerPort primaryActivePort;
 
     /**
@@ -100,34 +101,11 @@ public final class Server implements AutoCloseable {
      */
     private volatile GracefulShutdownSupport gracefulShutdownSupport = GracefulShutdownSupport.disabled();
 
-    Server(ServerConfig config) {
+    Server(ServerConfig config, @Nullable DomainNameMapping<SslContext> sslContexts) {
         this.config = requireNonNull(config, "config");
-        config.setServer(this);
-
-        // Pre-populate the domain name mapping for later matching.
-        SslContext lastSslContext = null;
-        for (VirtualHost h: config.virtualHosts()) {
-            lastSslContext = h.sslContext();
-        }
+        this.sslContexts = sslContexts;
 
-        if (lastSslContext == null) {
-            sslContexts = null;
-            for (ServerPort p: config.ports()) {
-                if (p.protocol().isTls()) {
-                    throw new IllegalArgumentException("no SSL context specified");
-                }
-            }
-        } else {
-            final DomainNameMappingBuilder<SslContext>
-                    mappingBuilder = new DomainNameMappingBuilder<>(lastSslContext);
-            for (VirtualHost h : config.virtualHosts()) {
-                final SslContext sslCtx = h.sslContext();
-                if (sslCtx != null) {
-                    mappingBuilder.add(h.hostnamePattern(), sslCtx);
-                }
-            }
-            sslContexts = mappingBuilder.build();
-        }
+        config.setServer(this);
 
         // Invoke the serviceAdded() method in Service so that it can keep the reference to this Server or
         // add a listener to it.
@@ -489,7 +467,7 @@ static final class State {
         final StateType type;
         final CompletableFuture<Void> future;
 
-        State(StateType type, CompletableFuture<Void> future) {
+        State(StateType type, @Nullable CompletableFuture<Void> future) {
             this.type = type;
             this.future = future;
         }
@@ -653,6 +631,15 @@ public void operationComplete(ChannelFuture f) throws Exception {
                     primaryActivePort = actualPort;
                 }
 
+                if (localAddress.getAddress().isAnyLocalAddress() ||
+                    localAddress.getAddress().isLoopbackAddress()) {
+                    logger.info("Serving {} at {} - {}://127.0.0.1:{}/",
+                                port.protocol().name(), localAddress,
+                                port.protocol().uriText(), localAddress.getPort());
+                } else {
+                    logger.info("Serving {} at {}", port.protocol().name(), localAddress);
+                }
+
                 if (remainingPorts.decrementAndGet() == 0) {
                     completeFuture(startFuture);
                 }