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. Modifications: - Add http() and https() which supercede most port() methods. - Add tls() which supercede sslContext() methods. - Deprecate the old methods. - Update documentation. - Miscellaneous: - Remove redundant `port(0, HTTP)` calls - Add `@Nullable` where necessary Result: - Fixes #1050 - More user-friendliness
line · Mar 6, 2018 · f4c4578 · f4c4578
1 parent f96772f
commit f4c4578
Show file tree

Hide file tree

Showing 29 changed files with 231 additions and 106 deletions.
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/ServerBuilder.java b/core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java
@@ -17,6 +17,7 @@
 package com.linecorp.armeria.server;
 
 import static com.linecorp.armeria.common.SessionProtocol.HTTP;
+import static com.linecorp.armeria.common.SessionProtocol.HTTPS;
 import static com.linecorp.armeria.server.ServerConfig.validateDefaultMaxRequestLength;
 import static com.linecorp.armeria.server.ServerConfig.validateDefaultRequestTimeoutMillis;
 import static com.linecorp.armeria.server.ServerConfig.validateNonNegative;
@@ -33,6 +34,7 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import javax.annotation.Nullable;
 import javax.net.ssl.SSLException;
 
 import com.google.common.collect.ImmutableList;
@@ -60,7 +62,7 @@
  * <pre>{@code
  * ServerBuilder sb = new ServerBuilder();
  * // Add a port to listen
- * sb.port(8080, SessionProtocol.HTTP);
+ * sb.http(8080);
  * // Build and add a virtual host.
  * sb.virtualHost(new VirtualHostBuilder("*.foo.com").service(...).build());
  * // Add services to the default virtual host.
@@ -74,15 +76,16 @@
  * <pre>{@code
  * ServerBuilder sb = new ServerBuilder();
  * Server server =
- *      sb.port(8080, SessionProtocol.HTTP) // Add a port to listen
- *      .withDefaultVirtualHost() // Add services to the default virtual host.
- *          .service(...)
- *          .serviceUnder(...)
- *      .and().withVirtualHost("*.foo.com") // Add a another virtual host.
- *          .service(...)
- *          .serviceUnder(...)
- *      .and().build(); // Build a server.
+ *     sb.http(8080) // Add a port to listen
+ *       .withDefaultVirtualHost() // Add services to the default virtual host.
+ *           .service(...)
+ *           .serviceUnder(...)
+ *       .and().withVirtualHost("*.foo.com") // Add a another virtual host.
+ *           .service(...)
+ *           .serviceUnder(...)
+ *       .and().build(); // Build a server.
  * }</pre>
+ *
  * @see VirtualHostBuilder
  */
 public final class ServerBuilder {
@@ -102,6 +105,7 @@ public final class ServerBuilder {
     private final ChainedVirtualHostBuilder defaultVirtualHostBuilder = new ChainedVirtualHostBuilder(this);
     private boolean updatedDefaultVirtualHostBuilder;
 
+    @Nullable
     private VirtualHost defaultVirtualHost;
     private EventLoopGroup workerGroup = CommonPools.workerGroup();
     private boolean shutdownWorkerGroupOnStop;
@@ -119,13 +123,71 @@ public final class ServerBuilder {
     private String serviceLoggerPrefix = DEFAULT_SERVICE_LOGGER_PREFIX;
     private Consumer<RequestLog> accessLogWriter = AccessLogWriters.disabled();
 
+    @Nullable
     private Function<Service<HttpRequest, HttpResponse>, Service<HttpRequest, HttpResponse>> decorator;
 
+    /**
+     * Adds an HTTP port that listens on all available network interfaces. If no HTTP(S) port is added
+     * (i.e. neither {@code http()} nor {@code https()} method is called), a default of {@code 0}
+     * (randomly-assigned port) will be used.
+     *
+     * @param port the HTTP port number.
+     *
+     * @see #http(InetSocketAddress)
+     */
+    public ServerBuilder http(int port) {
+        ports.add(new ServerPort(port, HTTP));
+        return this;
+    }
+
+    /**
+     * Adds an HTTP port that listens to the specified {@code localAddress}. If no HTTP(S) port is added
+     * (i.e. neither {@code http()} nor {@code https()} method is called), a default of {@code 0}
+     * (randomly-assigned port) will be used.
+     *
+     * @param localAddress the local address to bind
+     *
+     * @see #http(int)
+     */
+    public ServerBuilder http(InetSocketAddress localAddress) {
+        ports.add(new ServerPort(requireNonNull(localAddress, "localAddress"), HTTP));
+        return this;
+    }
+
+    /**
+     * Adds an HTTPS port that listens on all available network interfaces. If no HTTPS port is added,
+     * (i.e. no {@code https()} method is called), HTTPS support is disabled.
+     *
+     * @param port the HTTPS port number.
+     *
+     * @see #https(InetSocketAddress)
+     */
+    public ServerBuilder https(int port) {
+        ports.add(new ServerPort(port, HTTPS));
+        return this;
+    }
+
+    /**
+     * Adds an HTTPS port that listens to the specified {@code localAddress}. If no HTTPS port is added
+     * (i.e. no {@code https()} method is called), HTTPS support is disabled.
+     *
+     * @param localAddress the local address to bind
+     *
+     * @see #http(int)
+     */
+    public ServerBuilder https(InetSocketAddress localAddress) {
+        ports.add(new ServerPort(requireNonNull(localAddress, "localAddress"), HTTPS));
+        return this;
+    }
+
     /**
      * Adds a new {@link ServerPort} that listens to the specified {@code port} of all available network
      * interfaces using the specified protocol. If no port is added (i.e. no {@code port()} method is called),
      * a default of {@code 0} (randomly-assigned port) and {@code "http"} will be used.
+     *
+     * @deprecated Use {@link #http(int)} or {@link #https(int)}.
      */
+    @Deprecated
     public ServerBuilder port(int port, String protocol) {
         return port(port, SessionProtocol.of(requireNonNull(protocol, "protocol")));
     }
@@ -134,7 +196,10 @@ public ServerBuilder port(int port, String protocol) {
      * Adds a new {@link ServerPort} that listens to the specified {@code port} of all available network
      * interfaces using the specified {@link SessionProtocol}. If no port is added (i.e. no {@code port()}
      * method is called), a default of {@code 0} (randomly-assigned port) and {@code "http"} will be used.
+     *
+     * @deprecated Use {@link #http(int)} or {@link #https(int)}.
      */
+    @Deprecated
     public ServerBuilder port(int port, SessionProtocol protocol) {
         ports.add(new ServerPort(port, protocol));
         return this;
@@ -144,7 +209,10 @@ public ServerBuilder port(int port, SessionProtocol protocol) {
      * Adds a new {@link ServerPort} that listens to the specified {@code localAddress} using the specified
      * protocol. If no port is added (i.e. no {@code port()} method is called), a default of {@code 0}
      * (randomly-assigned port) and {@code "http"} will be used.
+     *
+     * @deprecated Use {@link #http(InetSocketAddress)} or {@link #https(InetSocketAddress)}.
      */
+    @Deprecated
     public ServerBuilder port(InetSocketAddress localAddress, String protocol) {
         return port(localAddress, SessionProtocol.of(requireNonNull(protocol, "protocol")));
     }
@@ -153,7 +221,10 @@ public ServerBuilder port(InetSocketAddress localAddress, String protocol) {
      * Adds a new {@link ServerPort} that listens to the specified {@code localAddress} using the specified
      * {@link SessionProtocol}. If no port is added (i.e. no {@code port()} method is called), a default of
      * {@code 0} (randomly-assigned port) and {@code "http"} will be used.
+     *
+     * @deprecated Use {@link #http(InetSocketAddress)} or {@link #https(InetSocketAddress)}.
      */
+    @Deprecated
     public ServerBuilder port(InetSocketAddress localAddress, SessionProtocol protocol) {
         ports.add(new ServerPort(localAddress, protocol));
         return this;
@@ -369,19 +440,65 @@ public ServerBuilder accessLogWriter(Consumer<? super RequestLog> accessLogWrite
      * @throws IllegalStateException if the default {@link VirtualHost} has been set via
      *                               {@link #defaultVirtualHost(VirtualHost)} already
      */
+    public ServerBuilder tls(SslContext sslContext) {
+        defaultVirtualHostBuilderUpdated();
+        defaultVirtualHostBuilder.tls(sslContext);
+        return this;
+    }
+
+    /**
+     * Sets the {@link SslContext} of the default {@link VirtualHost} from the specified
+     * {@code keyCertChainFile} and cleartext {@code keyFile}.
+     *
+     * @throws IllegalStateException if the default {@link VirtualHost} has been set via
+     *                               {@link #defaultVirtualHost(VirtualHost)} already
+     */
+    public ServerBuilder tls(File keyCertChainFile, File keyFile) throws SSLException {
+        defaultVirtualHostBuilderUpdated();
+        defaultVirtualHostBuilder.tls(keyCertChainFile, keyFile);
+        return this;
+    }
+
+    /**
+     * Sets the {@link SslContext} of the default {@link VirtualHost} from the specified
+     * {@code keyCertChainFile}, {@code keyFile} and {@code keyPassword}.
+     *
+     * @throws IllegalStateException if the default {@link VirtualHost} has been set via
+     *                               {@link #defaultVirtualHost(VirtualHost)} already
+     */
+    public ServerBuilder tls(
+            File keyCertChainFile, File keyFile, @Nullable String keyPassword) throws SSLException {
+
+        defaultVirtualHostBuilderUpdated();
+        defaultVirtualHostBuilder.tls(keyCertChainFile, keyFile, keyPassword);
+        return this;
+    }
+
+    /**
+     * Sets the {@link SslContext} of the default {@link VirtualHost}.
+     *
+     * @deprecated Use {@link #tls(SslContext)}.
+     *
+     * @throws IllegalStateException if the default {@link VirtualHost} has been set via
+     *                               {@link #defaultVirtualHost(VirtualHost)} already
+     */
+    @Deprecated
     public ServerBuilder sslContext(SslContext sslContext) {
         defaultVirtualHostBuilderUpdated();
-        defaultVirtualHostBuilder.sslContext(sslContext);
+        defaultVirtualHostBuilder.tls(sslContext);
         return this;
     }
 
     /**
      * Sets the {@link SslContext} of the default {@link VirtualHost} from the specified
      * {@link SessionProtocol}, {@code keyCertChainFile} and cleartext {@code keyFile}.
      *
+     * @deprecated Use {@link #tls(File, File)}.
+     *
      * @throws IllegalStateException if the default {@link VirtualHost} has been set via
      *                               {@link #defaultVirtualHost(VirtualHost)} already
      */
+    @Deprecated
     public ServerBuilder sslContext(
             SessionProtocol protocol, File keyCertChainFile, File keyFile) throws SSLException {
         defaultVirtualHostBuilderUpdated();
@@ -393,9 +510,12 @@ public ServerBuilder sslContext(
      * Sets the {@link SslContext} of the default {@link VirtualHost} from the specified
      * {@link SessionProtocol}, {@code keyCertChainFile}, {@code keyFile} and {@code keyPassword}.
      *
+     * @deprecated Use {@link #tls(File, File, String)}.
+     *
      * @throws IllegalStateException if the default {@link VirtualHost} has been set via
      *                               {@link #defaultVirtualHost(VirtualHost)} already
      */
+    @Deprecated
     public ServerBuilder sslContext(
             SessionProtocol protocol,
             File keyCertChainFile, File keyFile, String keyPassword) throws SSLException {
@@ -608,7 +728,7 @@ private void defaultVirtualHostBuilderUpdated() {
      * @throws IllegalStateException
      *     if other default {@link VirtualHost} builder methods have been invoked already, including:
      *     <ul>
-     *       <li>{@link #sslContext(SslContext)}</li>
+     *       <li>{@link #tls(SslContext)}</li>
      *       <li>{@link #service(String, Service)}</li>
      *       <li>{@link #serviceUnder(String, Service)}</li>
      *       <li>{@link #service(PathMapping, Service)}</li>