Reuse VirtualHostBuilder on the same hostnamePattern

core/src/main/java/com/linecorp/armeria/server/ServerBuilder.java

@@ -27,6 +27,8 @@
 import static com.linecorp.armeria.server.DefaultServerConfig.validateIdleTimeoutMillis;
 import static com.linecorp.armeria.server.DefaultServerConfig.validateMaxNumConnections;
 import static com.linecorp.armeria.server.DefaultServerConfig.validateNonNegative;
+import static com.linecorp.armeria.server.VirtualHost.normalizeHostnamePattern;
+import static com.linecorp.armeria.server.VirtualHost.validateHostnamePattern;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_FRAME_SIZE_LOWER_BOUND;
 import static io.netty.handler.codec.http2.Http2CodecUtil.MAX_FRAME_SIZE_UPPER_BOUND;
 import static java.util.Objects.requireNonNull;
@@ -1556,15 +1558,25 @@ public ServerBuilder withVirtualHost(Consumer<? super VirtualHostBuilder> custom
         return this;
     }
 
+    /**
+     * Configures a {@link VirtualHost} with the {@code customizer}.
+     */
+    public ServerBuilder withVirtualHost(String hostnamePattern,
+                                         Consumer<? super VirtualHostBuilder> customizer) {
+        final VirtualHostBuilder virtualHostBuilder = findOrCreateVirtualHostBuilder(hostnamePattern);
+        customizer.accept(virtualHostBuilder);
+        virtualHostBuilders.add(virtualHostBuilder);
+        return this;
+    }
+
     /**
      * Adds the <a href="https://en.wikipedia.org/wiki/Virtual_hosting#Name-based">name-based virtual host</a>.
      *
      * @param hostnamePattern virtual host name regular expression
      * @return {@link VirtualHostBuilder} for building the virtual host
      */
     public VirtualHostBuilder virtualHost(String hostnamePattern) {
-        final VirtualHostBuilder virtualHostBuilder =
-                new VirtualHostBuilder(this, false).hostnamePattern(hostnamePattern);
+        final VirtualHostBuilder virtualHostBuilder = findOrCreateVirtualHostBuilder(hostnamePattern);
         virtualHostBuilders.add(virtualHostBuilder);
         return virtualHostBuilder;
     }
@@ -1575,7 +1587,10 @@ public VirtualHostBuilder virtualHost(String hostnamePattern) {
      * @param defaultHostname default hostname of this virtual host
      * @param hostnamePattern virtual host name regular expression
      * @return {@link VirtualHostBuilder} for building the virtual host
+     *
+     * @deprecated prefer {@link #virtualHost(String)} with {@link VirtualHostBuilder#defaultHostname(String)}.
      */
+    @Deprecated
     public VirtualHostBuilder virtualHost(String defaultHostname, String hostnamePattern) {
         final VirtualHostBuilder virtualHostBuilder = new VirtualHostBuilder(this, false)
                 .defaultHostname(defaultHostname)
@@ -1612,6 +1627,22 @@ public VirtualHostBuilder virtualHost(int port) {
         return virtualHostBuilder;
     }
 
+    private VirtualHostBuilder findOrCreateVirtualHostBuilder(String hostnamePattern) {
+        requireNonNull(hostnamePattern, "hostnamePattern");
+        final HostAndPort hostAndPort = HostAndPort.fromString(hostnamePattern);
+        validateHostnamePattern(hostAndPort.getHost());
+
+        final String normalizedHostnamePattern = normalizeHostnamePattern(hostAndPort.getHost());
+        final int port = hostAndPort.getPortOrDefault(-1);
+        for (VirtualHostBuilder virtualHostBuilder : virtualHostBuilders) {
+            if (!virtualHostBuilder.defaultVirtualHost() &&
+                virtualHostBuilder.equalsHostnamePattern(normalizedHostnamePattern, port)) {
+                return virtualHostBuilder;
+            }
+        }
+        return new VirtualHostBuilder(this, false).hostnamePattern(normalizedHostnamePattern, port);
+    }
+
     /**
      * Decorates all {@link HttpService}s with the specified {@code decorator}.
      * The specified decorator(s) is/are executed in reverse order of the insertion.

core/src/main/java/com/linecorp/armeria/server/VirtualHost.java

@@ -15,6 +15,7 @@
  */
 package com.linecorp.armeria.server;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static java.util.Objects.requireNonNull;
@@ -205,6 +206,21 @@ static String normalizeHostnamePattern(String hostnamePattern) {
         return Ascii.toLowerCase(hostnamePattern);
     }
 
+    static void validateHostnamePattern(String hostnamePattern) {
+        final boolean validHostnamePattern;
+        if (hostnamePattern.charAt(0) == '*') {
+            validHostnamePattern =
+                    hostnamePattern.length() >= 3 &&
+                    hostnamePattern.charAt(1) == '.' &&
+                    HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern.substring(2)).matches();
+        } else {
+            validHostnamePattern = HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern).matches();
+        }
+
+        checkArgument(validHostnamePattern,
+                      "hostnamePattern: %s (expected: *.<hostname> or <hostname>)", hostnamePattern);
+    }
+
     /**
      * Ensure that 'hostnamePattern' matches 'defaultHostname'.
      */

core/src/main/java/com/linecorp/armeria/server/VirtualHostBuilder.java

@@ -25,10 +25,10 @@
 import static com.linecorp.armeria.server.ServerSslContextUtil.validateSslContext;
 import static com.linecorp.armeria.server.ServiceConfig.validateMaxRequestLength;
 import static com.linecorp.armeria.server.ServiceConfig.validateRequestTimeoutMillis;
-import static com.linecorp.armeria.server.VirtualHost.HOSTNAME_WITH_NO_PORT_PATTERN;
 import static com.linecorp.armeria.server.VirtualHost.ensureHostnamePatternMatchesDefaultHostname;
 import static com.linecorp.armeria.server.VirtualHost.normalizeDefaultHostname;
 import static com.linecorp.armeria.server.VirtualHost.normalizeHostnamePattern;
+import static com.linecorp.armeria.server.VirtualHost.validateHostnamePattern;
 import static io.netty.handler.codec.http2.Http2Headers.PseudoHeaderName.isPseudoHeader;
 import static java.util.Objects.requireNonNull;
 
@@ -232,7 +232,11 @@
      * will be bound to the {@code 8080} port. Otherwise, the virtual host will allow all active ports.
      *
      * @throws UnsupportedOperationException if this is the default {@link VirtualHostBuilder}
+     *
+     * @deprecated prefer specifying the hostnamePattern using {@link ServerBuilder#virtualHost(String)}
+     *             or {@link ServerBuilder#withVirtualHost(String, Consumer)}
      */
+    @Deprecated
     public VirtualHostBuilder hostnamePattern(String hostnamePattern) {
         if (defaultVirtualHost) {
             throw new UnsupportedOperationException(
@@ -248,23 +252,25 @@
             hostnamePattern = hostAndPort.getHost();
         }
 
-        final boolean validHostnamePattern;
-        if (hostnamePattern.charAt(0) == '*') {
-            validHostnamePattern =
-                    hostnamePattern.length() >= 3 &&
-                    hostnamePattern.charAt(1) == '.' &&
-                    HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern.substring(2)).matches();
-        } else {
-            validHostnamePattern = HOSTNAME_WITH_NO_PORT_PATTERN.matcher(hostnamePattern).matches();
-        }
-
-        checkArgument(validHostnamePattern,
-                      "hostnamePattern: %s (expected: *.<hostname> or <hostname>)", hostnamePattern);
+        validateHostnamePattern(hostnamePattern);
 
         this.hostnamePattern = normalizeHostnamePattern(hostnamePattern);
         return this;
     }
 
+    VirtualHostBuilder hostnamePattern(String hostnamePattern, int port) {
+        if (defaultVirtualHost) {
+            throw new UnsupportedOperationException(
+                    "Cannot set hostnamePattern for the default virtual host builder");
+        }
+
+        this.hostnamePattern = hostnamePattern;
+        if (port >= 1 && port <= 65535) {
+            this.port = port;
+        }
+        return this;
+    }
+
     @Override
     public VirtualHostBuilder tls(File keyCertChainFile, File keyFile) {
         return (VirtualHostBuilder) TlsSetters.super.tls(keyCertChainFile, keyFile);
@@ -1525,6 +1531,15 @@
         return selfSignedCertificate;
     }
 
+    boolean equalsHostnamePattern(String validHostnamePattern, int port) {
+        checkArgument(!validHostnamePattern.isEmpty(), "hostnamePattern is empty.");
+
+        if (this.port != port) {
+            return false;
+        }
+        return validHostnamePattern.equals(hostnamePattern);
+    }
+
     int port() {
         return port;
     }

core/src/test/java/com/linecorp/armeria/server/HostnameBasedVirtualHostTest.java

@@ -0,0 +1,99 @@
+/*
+ * Copyright 2024 LINE Corporation
+ *
+ * LINE Corporation licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.linecorp.armeria.server;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.net.ServerSocket;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import com.linecorp.armeria.client.ClientFactory;
+import com.linecorp.armeria.client.WebClient;
+import com.linecorp.armeria.common.AggregatedHttpResponse;
+import com.linecorp.armeria.common.HttpResponse;
+import com.linecorp.armeria.internal.testing.MockAddressResolverGroup;
+import com.linecorp.armeria.testing.junit5.server.ServerExtension;
+
+class HostnameBasedVirtualHostTest {
+
+    private static int fooHostPort;
+
+    @RegisterExtension
+    static ServerExtension serverWithPortMapping = new ServerExtension() {
+        @Override
+        protected void configure(ServerBuilder sb) throws Exception {
+
+            try (ServerSocket ss = new ServerSocket(0)) {
+                fooHostPort = ss.getLocalPort();
+            }
+
+            sb.http(fooHostPort)
+              .virtualHost("foo.com:" + fooHostPort)
+              .service("/foo", (ctx, req) -> HttpResponse.of("foo with port"))
+              .and()
+              .virtualHost("foo.com:" + fooHostPort)
+              .service("/bar", (ctx, req) -> HttpResponse.of("bar with port"))
+              .and()
+              .virtualHost("foo.bar.com")
+              .service("/foo-bar", (ctx, req) -> HttpResponse.of("foo bar"))
+              .and()
+              .build();
+        }
+    };
+
+    @Test
+    void testHostnamePattern() {
+        try (ClientFactory factory = ClientFactory.builder()
+                                                  .addressResolverGroupFactory(
+                                                          unused -> MockAddressResolverGroup.localhost())
+                                                  .build()) {
+
+            final WebClient client = WebClient.builder("http://foo.com:" + fooHostPort)
+                                              .factory(factory)
+                                              .build();
+            AggregatedHttpResponse response = client.get("/foo").aggregate().join();
+            assertThat(response.contentUtf8()).isEqualTo("foo with port");
+
+            response = client.get("/bar").aggregate().join();
+            assertThat(response.contentUtf8()).isEqualTo("bar with port");
+        }
+    }
+
+    @Test
+    void shouldReturnSameInstanceForHostnameBasedVirtualHost() {
+        final ServerBuilder serverBuilder = Server.builder();
+        final VirtualHostBuilder virtualHost1 = serverBuilder.virtualHost("foo.com");
+        final VirtualHostBuilder virtualHost2 = serverBuilder.virtualHost("foo.com");
+        assertThat(virtualHost1).isSameAs(virtualHost2);
+        final VirtualHostBuilder virtualHost3 = serverBuilder.virtualHost("foo.com:18080");
+        assertThat(virtualHost2).isNotSameAs(virtualHost3);
+        final VirtualHostBuilder virtualHost4 = serverBuilder.virtualHost("bar.com");
+        assertThat(virtualHost2).isNotSameAs(virtualHost4);
+    }
+
+    @Test
+    void shouldReturnSameInstanceForHostnameBasedVirtualHostWithPort() {
+        final ServerBuilder serverBuilder = Server.builder();
+        final VirtualHostBuilder virtualHost1 = serverBuilder.virtualHost("foo.com:18080");
+        final VirtualHostBuilder virtualHost2 = serverBuilder.virtualHost("foo.com:18080");
+        assertThat(virtualHost1).isSameAs(virtualHost2);
+        final VirtualHostBuilder virtualHost3 = serverBuilder.virtualHost("foo.com:18081");
+        assertThat(virtualHost2).isNotSameAs(virtualHost3);
+    }
+}

core/src/test/java/com/linecorp/armeria/server/PortBasedVirtualHostTest.java

@@ -227,4 +227,12 @@ void portBasedVirtualHostWithTls() {
         assertThat(server.config().virtualHosts().stream().map(VirtualHost::originalHostnamePattern))
                 .containsExactly("*", "foo.com", "*");
     }
+
+    @Test
+    void notAllowSettingHostnameWhenDefaultVirtualHost() {
+        final VirtualHostBuilder virtualHostBuilder = Server.builder()
+                                                            .virtualHost(8080);
+        assertThatThrownBy(() -> virtualHostBuilder.hostnamePattern("foo.com"))
+                .isInstanceOf(UnsupportedOperationException.class);
+    }
 }

core/src/test/java/com/linecorp/armeria/server/ServerBuilderTest.java

@@ -361,8 +361,8 @@ void serviceConfigurationPriority() {
                                                           Duration.ofMillis(250),
                                                           ctx.eventLoop())))
                                     .withVirtualHost(
-                                            h -> h.hostnamePattern("foo.com")
-                                                  .service("/custom_virtual_host",
+                                            "foo.com",
+                                            h -> h.service("/custom_virtual_host",
                                                            (ctx, req) -> HttpResponse.delayed(
                                                                    HttpResponse.of(HttpStatus.OK),
                                                                    Duration.ofMillis(150),

core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java

@@ -164,38 +164,39 @@ void defaultVirtualHostMixedStyle() {
 
     @Test
     void virtualHostWithoutPattern() {
-        final VirtualHost h = new VirtualHostBuilder(Server.builder(), false)
-                .defaultHostname("foo.com")
-                .hostnamePattern("foo.com")
-                .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
+        final VirtualHost h =
+                Server.builder()
+                      .virtualHost("foo.com")
+                      .defaultHostname("foo.com")
+                      .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         assertThat(h.hostnamePattern()).isEqualTo("foo.com");
         assertThat(h.defaultHostname()).isEqualTo("foo.com");
     }
 
     @Test
     void virtualHostWithPattern() {
-        final VirtualHost h = new VirtualHostBuilder(Server.builder(), false)
-                .defaultHostname("bar.foo.com")
-                .hostnamePattern("*.foo.com")
-                .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
+        final VirtualHost h =
+                Server.builder().virtualHost("*.foo.com")
+                      .defaultHostname("bar.foo.com")
+                      .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         assertThat(h.hostnamePattern()).isEqualTo("*.foo.com");
         assertThat(h.defaultHostname()).isEqualTo("bar.foo.com");
     }
 
     @Test
     void accessLoggerCustomization() {
-        final VirtualHost h1 = new VirtualHostBuilder(Server.builder(), false)
-                .defaultHostname("bar.foo.com")
-                .hostnamePattern("*.foo.com")
-                .accessLogger(host -> LoggerFactory.getLogger("customize.test"))
-                .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
+        final VirtualHost h1 =
+                Server.builder().virtualHost("*.foo.com")
+                      .defaultHostname("bar.foo.com")
+                      .accessLogger(host -> LoggerFactory.getLogger("customize.test"))
+                      .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         assertThat(h1.accessLogger().getName()).isEqualTo("customize.test");
 
-        final VirtualHost h2 = new VirtualHostBuilder(Server.builder(), false)
-                .defaultHostname("bar.foo.com")
-                .hostnamePattern("*.foo.com")
-                .accessLogger(LoggerFactory.getLogger("com.foo.test"))
-                .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
+        final VirtualHost h2 =
+                Server.builder().virtualHost("*.foo.com")
+                      .defaultHostname("bar.foo.com")
+                      .accessLogger(LoggerFactory.getLogger("com.foo.test"))
+                      .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         assertThat(h2.accessLogger().getName()).isEqualTo("com.foo.test");
     }
 
@@ -301,19 +302,17 @@ void virtualHostWithNull3() {
     @Test
     void virtualHostWithMismatch() {
         assertThatThrownBy(() -> {
-            new VirtualHostBuilder(Server.builder(), false)
-                    .defaultHostname("bar.com")
-                    .hostnamePattern("foo.com")
-                    .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
+            Server.builder().virtualHost("foo.com")
+                  .defaultHostname("bar.com")
+                  .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         }).isInstanceOf(IllegalArgumentException.class);
     }
 
     @Test
     void virtualHostWithMismatch2() {
         assertThatThrownBy(() -> {
-            new VirtualHostBuilder(Server.builder(), false)
+            Server.builder().virtualHost("*.foo.com")
                     .defaultHostname("bar.com")
-                    .hostnamePattern("*.foo.com")
                     .build(template, noopDependencyInjector, null, ServerErrorHandler.ofDefault());
         }).isInstanceOf(IllegalArgumentException.class);
     }