Provide a way to set the root context path #4802

bkkanq · 2023-04-04T02:28:12Z

Motivation:

Described in #3591

Modifications:

Add ContextServiceBuilder to bind a root context path to VirtualHost

Result:

User can set the root context path
TODO: Support to set contextPath in Armeria Spring integration
Closes Provide a way to set the root context path #3591

CLAassistant · 2023-04-04T02:28:17Z

All committers have signed the CLA.

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

ikhoon · 2023-04-04T06:23:58Z

Had an internal discussion about the API design of this PR. We were worried that the server and service builder become too complex by adding ContextServiceBuilder.

It should be nicer to reuse the existing API to build the services rather than reinventing the wheel. On second thought, I think, we can add a new concept of virtual hosting that serves all services under a certain path like ServerPath of Virtualhost in Apache HTTP server.

For example:

Server
  .builder()
  // return a path-based `VirtualHost` prefixed with `/api/v0`
  .virtualHostWithPath("/api/v0")
    .service(...)
    .decorator(...)
    .and()
  // return a path-based `VirtualHost` prefixed with `/api/v1`
  .virtualHostWithPath("/api/v1")
    .service(...)
    .decorator(...)
    .and()

To implement a path-based virtual host, you may want to check out the following points.

Add a new builder method to ServerBuilder.

If a predefined path exists, the same virtual host should be returned as port-based virtual hosting.

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

Lines 1419 to 1434 in b16bc8d

    
           public VirtualHostBuilder virtualHost(int port) { 
        
               checkArgument(port >= 1 && port <= 65535, "port: %s (expected: 1-65535)", port); 
        
               // Look for a virtual host that has already been made and reuse it. 
        
               final Optional<VirtualHostBuilder> vhost = 
        
                       virtualHostBuilders.stream() 
        
                                          .filter(v -> v.port() == port && v.defaultVirtualHost()) 
        
                                          .findFirst(); 
        
               if (vhost.isPresent()) { 
        
                   return vhost.get(); 
        
               } 
        
               final VirtualHostBuilder virtualHostBuilder = new VirtualHostBuilder(this, port); 
        
               virtualHostBuilders.add(virtualHostBuilder); 
        
               return virtualHostBuilder; 
        
           }

Fix buildDomainAndPortMapping to take the context path into account.

The return type of buildDomainAndPortMapping could be:

- Int2ObjectMap<Mapping<String, VirtualHost>>
+ Int2ObjectMap<Mapping<String, List<VirtualHost>>>

The size of List will be 1 if no path-based VirtualHost is configured. The type looks complex. If you have a good idea, feel free to change it.

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

Lines 262 to 287 in b16bc8d

    
           private static Int2ObjectMap<Mapping<String, VirtualHost>> buildDomainAndPortMapping( 
        
                   VirtualHost defaultVirtualHost, List<VirtualHost> virtualHosts) { 
        
               final List<VirtualHost> portMappingVhosts = virtualHosts.stream() 
        
                                                                       .filter(v -> v.port() > 0) 
        
                                                                       .collect(toImmutableList()); 
        
               final Map<Integer, VirtualHost> portMappingDefaultVhosts = 
        
                       portMappingVhosts.stream() 
        
                                        .filter(v -> v.hostnamePattern().startsWith("*:")) 
        
                                        .collect(toImmutableMap(VirtualHost::port, Function.identity())); 
        
               final Map<Integer, DomainMappingBuilder<VirtualHost>> mappingsBuilder = new HashMap<>(); 
        
               for (VirtualHost virtualHost : portMappingVhosts) { 
        
                   final int port = virtualHost.port(); 
        
                   // The default virtual host should be either '*' or '*:<port>'. 
        
                   final VirtualHost defaultVhost = 
        
                           firstNonNull(portMappingDefaultVhosts.get(port), defaultVirtualHost); 
        
                   // Builds a 'DomainMappingBuilder' with 'defaultVhost' for the port if absent. 
        
                   final DomainMappingBuilder<VirtualHost> mappingBuilder = 
        
                           mappingsBuilder.computeIfAbsent(port, key -> new DomainMappingBuilder<>(defaultVhost)); 
        
                   if (defaultVhost == virtualHost) { 
        
                       // The 'virtualHost' was added already as a default value when creating 'DomainMappingBuilder'. 
        
                   } else { 
        
                       mappingBuilder.add(virtualHost.hostnamePattern(), virtualHost); 
        
                   }

Use a request path to find a matched virtual host.
We can add a path as the last parameter of ServerConfing.findVirtualHost().

- public VirtualHost findVirtualHost(String hostname, int port) {
+ public VirtualHost findVirtualHost(String hostname, int port, String path) {

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

Lines 403 to 421 in b16bc8d

    
           @Override 
        
           public VirtualHost findVirtualHost(String hostname, int port) { 
        
               if (virtualHostAndPortMapping == null) { 
        
                   return defaultVirtualHost; 
        
               } 
        
               final Mapping<String, VirtualHost> virtualHostMapping = virtualHostAndPortMapping.get(port); 
        
               if (virtualHostMapping != null) { 
        
                   final VirtualHost virtualHost = virtualHostMapping.map(hostname + ':' + port); 
        
                   // Exclude the default virtual host from port-based virtual hosts. 
        
                   if (virtualHost != defaultVirtualHost) { 
        
                       return virtualHost; 
        
                   } 
        
               } 
        
               // No port-based virtual host is configured. Look for named-based virtual host. 
        
               final Mapping<String, VirtualHost> nameBasedMapping = virtualHostAndPortMapping.get(-1); 
        
               assert nameBasedMapping != null; 
        
               return nameBasedMapping.map(hostname); 
        
           }

bkkanq · 2023-04-10T03:27:16Z

Thanks for the detailed explanation. let me update this draft PR soon. 🙇

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

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

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

jrhee17

Left some minor comments, but I think the overall direction looks good and the PR should be almost done soon.

I think if this is handled quickly, hopefully it can be released in this iteration 🤞

jrhee17 · 2023-07-19T07:19:24Z

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

@@ -77,6 +77,8 @@ public final class VirtualHost {
    private final String defaultHostname;
    private final String hostnamePattern;
    private final int port;
+    private final String contextPath;


I don't think we need to add contextPath to VirtualHost for the new direction.
What do you think of reverting this file?

jrhee17 · 2023-07-19T07:22:29Z

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

@@ -181,6 +184,12 @@ static String normalizeDefaultHostname(String defaultHostname) {
        return Ascii.toLowerCase(defaultHostname);
    }

+    static String normalizeContextPath(String contextPath) {
+        requireNonNull(contextPath, "contextPath");
+        // TODO Implement this


I think we can just do something like the following.

RouteUtil.ensureAbsolutePath(contextPath, "contextPath");

We probably don't even need to define a dedicated method for this

aha, thank you. let me address it.

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

jrhee17 · 2023-07-19T07:28:50Z

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

@@ -478,7 +501,8 @@ public VirtualHostBuilder serviceUnder(String pathPrefix, HttpService service) {
     * @throws IllegalArgumentException if the specified path pattern is invalid
     */
    public VirtualHostBuilder service(String pathPattern, HttpService service) {
-        service(Route.builder().path(pathPattern).build(), service);
+        final String prefixed = contextPath == null ? pathPattern : contextPath + pathPattern;
+        service(Route.builder().path(prefixed).build(), service);


Instead of adding the prefix here, what do you think of adding the prefix at VirtualHostBuilder#build?

If we add routes here there are several problems:

Some builders like VirtualHostBuilder#route(), or VirtualHostBuilder#annotatedService, VirtualHostBuilder#serviceUnder won't be applied

If the contextPath value changes, then the applied prefix can be inconsistent

thanks. understood!

jrhee17 · 2023-07-19T07:29:17Z

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

@@ -677,9 +701,29 @@ private List<ServiceConfigSetters> getServiceConfigSetters(
        } else {
            serviceConfigSetters = ImmutableList.copyOf(this.serviceConfigSetters);
        }
+
+        if (defaultVirtualHostBuilder.contextPath() != null) {


Ditto, I propose that we add prefixes at VirtualHostBuilder#build.

In detail, I propose the following change so that ServiceConfigBuilder accepts the prefix before creating the actual ServiceConfig.

+++ b/core/src/main/java/com/linecorp/armeria/server/ServiceConfigBuilder.java @@ -293,7 +293,8 @@ final class ServiceConfigBuilder implements ServiceConfigSetters { HttpHeaders virtualHostDefaultHeaders, Function<? super RoutingContext, ? extends RequestId> defaultRequestIdGenerator, ServiceErrorHandler defaultServiceErrorHandler, - @Nullable UnhandledExceptionsReporter unhandledExceptionsReporter) { + @Nullable UnhandledExceptionsReporter unhandledExceptionsReporter, + String contextPath) { ServiceErrorHandler errorHandler = serviceErrorHandler != null ? serviceErrorHandler.orElse(defaultServiceErrorHandler) : defaultServiceErrorHandler; @@ -303,7 +304,7 @@ final class ServiceConfigBuilder implements ServiceConfigSetters { } return new ServiceConfig( - route, mappedRoute == null ? route : mappedRoute, + route.withPrefix(contextPath), mappedRoute == null ? route : mappedRoute,

This can be passed from VirtualHostBuilder like the following:

+++ b/core/src/main/java/com/linecorp/armeria/server/VirtualHostBuilder.java @@ -1338,7 +1337,7 @@ public final class VirtualHostBuilder implements TlsSetters { successFunction, requestAutoAbortDelayMillis, multipartUploadsLocation, defaultHeaders, requestIdGenerator, defaultErrorHandler, - unhandledExceptionsReporter); + unhandledExceptionsReporter, contextPath); }).collect(toImmutableList()); final ServiceConfig fallbackServiceConfig = @@ -1347,7 +1346,7 @@ public final class VirtualHostBuilder implements TlsSetters { accessLogWriter, blockingTaskExecutor, successFunction, requestAutoAbortDelayMillis, multipartUploadsLocation, defaultHeaders, requestIdGenerator, - defaultErrorHandler, unhandledExceptionsReporter); + defaultErrorHandler, unhandledExceptionsReporter, "/");

Note that fallbackServiceConfig should still catch all requests, so the prefix isn't applied at line 1346.

jrhee17 · 2023-07-19T07:30:51Z

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

@@ -320,6 +320,10 @@ ServiceConfig build(ServiceNaming defaultServiceNaming,
                requestIdGenerator != null ? requestIdGenerator : defaultRequestIdGenerator, errorHandler);
    }

+    public ServiceConfigBuilder getPrefixedServiceConfigBuilder(String contextPath) {


I don't think this method is necessary if we can set the contextPath when ServiceConfigBuilder#build is invoked.

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

jrhee17 · 2023-07-19T07:33:11Z

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

@@ -122,6 +123,8 @@ public final class VirtualHostBuilder implements TlsSetters {
    private String hostnamePattern;
    private int port = -1;
    @Nullable
+    private String contextPath;


Can we just set to / by default and remove the @Nullable contract?

jrhee17 · 2023-07-19T07:47:04Z

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

+    @Test
+    void virtualHostContextPath() {
+        final Server server = Server
+                .builder()


I think we should also consider decorators:

Server.builder() .decorator("/asdf", (delegate, ctx, req) -> HttpResponse.of(200)) ...

We probably need to apply the contextPath like the following:

@@ -737,7 +737,7 @@ public final class VirtualHostBuilder implements TlsSetters { @Nullable private Function<? super HttpService, ? extends HttpService> getRouteDecoratingService( - @Nullable VirtualHostBuilder defaultVirtualHostBuilder) { + @Nullable VirtualHostBuilder defaultVirtualHostBuilder, String contextPath) { final List<RouteDecoratingService> routeDecoratingServices; if (defaultVirtualHostBuilder != null) { routeDecoratingServices = ImmutableList.<RouteDecoratingService>builder() @@ -749,8 +749,11 @@ public final class VirtualHostBuilder implements TlsSetters { } if (!routeDecoratingServices.isEmpty()) { - return RouteDecoratingService.newDecorator( - Routers.ofRouteDecoratingService(routeDecoratingServices)); + final List<RouteDecoratingService> prefixed = + routeDecoratingServices.stream() + .map(service -> service.withRoutePrefix(contextPath)) + .collect(Collectors.toList()); + return RouteDecoratingService.newDecorator(Routers.ofRouteDecoratingService(prefixed)); } else { return null; }

where the routes can be overridden using delegation

+++ b/core/src/main/java/com/linecorp/armeria/internal/server/RouteDecoratingService.java @@ -91,6 +91,15 @@ public final class RouteDecoratingService implements HttpService { decorator = requireNonNull(decoratorFunction, "decoratorFunction").apply(this); } + public RouteDecoratingService(Route route, HttpService decorator) { + this.route = requireNonNull(route, "route"); + this.decorator = requireNonNull(decorator, "decorator"); + } + + public RouteDecoratingService withRoutePrefix(String prefix) { + return new RouteDecoratingService(route.withPrefix(prefix), decorator); + } +

Ah... decorator. thanks.

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

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

bkkanq · 2023-07-21T00:38:53Z

@jrhee17 nim, Thanks for the review. I'll address your comments.
let me change the status from draft to ready after adding some details to it!

- address comments

- consider decorator - rename API to baseContextPath - handle VirtualHost separatly - add prefixes at VirtualHostBuilder#build.

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

github-actions · 2023-08-24T01:42:18Z

🔍 Build Scan® (commit: `505d1f5`)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/hwssro43xwj52
build-self-hosted-jdk-8	✅	https://ge.armeria.dev/s/xt7lxmbpbewoi
build-self-hosted-jdk-19-snapshot-blockhound	✅	https://ge.armeria.dev/s/sl2qddj4c7psg
build-self-hosted-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/bwdrrukdiondg
build-self-hosted-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/7ggnmjafrww5q
build-self-hosted-jdk-17-leak	✅	https://ge.armeria.dev/s/mq4ua6dbniuto
build-self-hosted-jdk-11	✅	https://ge.armeria.dev/s/fgtecjapkzuee
build-macos-12-jdk-19	✅	https://ge.armeria.dev/s/tidzjomxf32tc

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

codecov · 2023-08-24T03:06:37Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (026351d) 74.16% compared to head (505d1f5) 74.17%.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #4802      +/-   ##
============================================
+ Coverage     74.16%   74.17%   +0.01%     
- Complexity    19891    19895       +4     
============================================
  Files          1707     1707              
  Lines         73325    73338      +13     
  Branches       9364     9364              
============================================
+ Hits          54381    54398      +17     
+ Misses        14511    14507       -4     
  Partials       4433     4433

Files	Coverage Δ
...rmeria/internal/server/RouteDecoratingService.java	`78.00% <100.00%> (+2.44%)`	⬆️
...ava/com/linecorp/armeria/server/ServerBuilder.java	`83.05% <100.00%> (+0.06%)`	⬆️
.../linecorp/armeria/server/ServiceConfigBuilder.java	`58.46% <100.00%> (+0.32%)`	⬆️
...om/linecorp/armeria/server/VirtualHostBuilder.java	`82.27% <100.00%> (+0.23%)`	⬆️

... and 16 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jrhee17

Looks almost done! Only minor comments for me 🙇

jrhee17 · 2023-08-24T14:07:08Z

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

@@ -335,7 +336,7 @@ ServiceConfig build(ServiceNaming defaultServiceNaming,
        }

        return new ServiceConfig(
-                route, mappedRoute == null ? route : mappedRoute,
+                route.withPrefix(contextPath), mappedRoute == null ? route : mappedRoute,


Following the contract of mappedRoute:

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

Lines 250 to 251 in 5f9c1b6

* If the service is not an {@link HttpServiceWithRoutes}, the {@link Route} is the same as

* {@link #route()}.

We can probably also dedup the route.withPrefix(contextPath)

Suggested change

route.withPrefix(contextPath), mappedRoute == null ? route : mappedRoute,

route.withPrefix(contextPath), mappedRoute == null ? route.withPrefix(contextPath) : mappedRoute,

the {@link Route} is the same as {@link #route()}.

Understood... thanks for catching it out

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

core/src/main/java/com/linecorp/armeria/internal/server/RouteDecoratingService.java

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

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

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

jrhee17 · 2023-08-24T15:28:18Z

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

+        final Server server = Server
+                .builder()
+                // 1
+                .port(8888, SessionProtocol.HTTP)


Setting a fixed port can be a cause of flakiness. Can you modify so that the port is set as random (same as the other tests in this class?)

Sorry, Is there some way to test virtualHost with port (e.g. .virtualHost("*.foo.com:8888")) without setting a fixed port? 😢

When I set the port as 0 ( to use random port) I've got the following error.

virtual host port: 8888 (expected: one of []) java.lang.IllegalStateException: virtual host port: 8888 (expected: one of []) at com.google.common.base.Preconditions.checkState(Preconditions.java:715) at com.linecorp.armeria.server.ServerBuilder.buildServerConfig(ServerBuilder.java:2056) at com.linecorp.armeria.server.ServerBuilder.build(ServerBuilder.java:2002) at com.linecorp.armeria.server.ServerBuilderTest.baseContextPathApplied(ServerBuilderTest.java:740) at java.lang.reflect.Method.invoke(Method.java:498)

So I guessed I should set a fixed port to test it.

I've investigated test codes in this class, there's a similar test to follow.

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

Line 236 in 863e27c

.virtualHost("*.example2.com:8080")

and it's using http()

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

Line 230 in 863e27c

.http(8080)

Did you mean to set the port using .http().. ?.?

I see 😅 If you would like to test virtual hosts with ports, I think the following reference can help:

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

Lines 48 to 56 in b604f3f

try (ServerSocket ss = new ServerSocket(0)) {

normalServerPort = ss.getLocalPort();

}

try (ServerSocket ss = new ServerSocket(0)) {

virtualHostPort = ss.getLocalPort();

}

try (ServerSocket ss = new ServerSocket(0)) {

fooHostPort = ss.getLocalPort();

}

Optionally since the test class is convoluted (so it is difficult to use ServerExtension), I wouldn't be against just creating a separate test class (either way is fine)

thanks! let me handle it.

Co-authored-by: jrhee17 <guins_j@guins.org>

jrhee17

Still looks good 👍 I think this PR is ready to go once my comment on testing is resolved

jrhee17 · 2023-09-04T09:21:05Z

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

+        final Server server = Server
+                .builder()
+                // 1
+                .port(8888, SessionProtocol.HTTP)


I see 😅 If you would like to test virtual hosts with ports, I think the following reference can help:

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

Lines 48 to 56 in b604f3f

try (ServerSocket ss = new ServerSocket(0)) {

normalServerPort = ss.getLocalPort();

}

try (ServerSocket ss = new ServerSocket(0)) {

virtualHostPort = ss.getLocalPort();

}

try (ServerSocket ss = new ServerSocket(0)) {

fooHostPort = ss.getLocalPort();

}

Optionally since the test class is convoluted (so it is difficult to use ServerExtension), I wouldn't be against just creating a separate test class (either way is fine)

- seperate test class

jrhee17

Changes look good to me! Thanks for your patience @bkkanq 🙇 🚀 🙇

Note that I've pushed a small commit which renames contextPath -> baseContextPath

minwoox

Looks great!
Thanks, @bkkanq!
Also thanks, @jrhee17 for taking care of it. 😉

minwoox · 2023-09-25T08:16:55Z

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

@@ -335,7 +336,8 @@ ServiceConfig build(ServiceNaming defaultServiceNaming,
        }

        return new ServiceConfig(
-                route, mappedRoute == null ? route : mappedRoute,
+                route.withPrefix(baseContextPath),


nit; we can extract the route:

final Route withPrefix = route.withPrefix(baseContextPath); return new ServiceConfig( withPrefix, mappedRoute == null ? withPrefix : mappedRoute,

minwoox · 2023-09-25T08:19:29Z

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

-                    Routers.ofRouteDecoratingService(routeDecoratingServices));
+            final List<RouteDecoratingService> prefixed = routeDecoratingServices.stream()
+                    .map(service -> service.withRoutePrefix(baseContextPath))
+                    .collect(Collectors.toList());


We usually use:

Suggested change

.collect(Collectors.toList());

.collect(toImmutableList());

minwoox · 2023-09-25T08:21:17Z

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

+import com.linecorp.armeria.internal.testing.MockAddressResolverGroup;
+import com.linecorp.armeria.testing.junit5.server.ServerExtension;
+
+public class BaseContextPathTest {


We can remove public modifiers in this class.
Let's also add @FlakyTest because this can be flaky.

ikhoon

Thanks a lot, @bkkanq and @jrhee17. It is a long-awaited feature. Many users will be happy with this. 🙇‍♂️🙇‍♂️

Motivation: Described in line#3591 Modifications: - Add `ContextServiceBuilder` to bind a root context path to `VirtualHost` Result: - User can set the root context path - TODO: Support to set `contextPath` in Armeria Spring integration - Closes line#3591 Co-authored-by: jrhee17 <guins_j@guins.org> Co-authored-by: Ikhun Um <ih.pert@gmail.com>

bkkanq force-pushed the 3591-provide-context-path branch from c3d9c46 to abe193b Compare April 4, 2023 03:03

bkkanq commented Apr 4, 2023

View reviewed changes

core/src/test/java/com/linecorp/armeria/server/ServerBuilderTest.java Outdated Show resolved Hide resolved

bkkanq force-pushed the 3591-provide-context-path branch from dffeac3 to fd58e5f Compare April 11, 2023 02:59

jrhee17 added this to the 1.25.0 milestone Jul 6, 2023

bkkanq force-pushed the 3591-provide-context-path branch 2 times, most recently from db59dcf to 9fb9f74 Compare July 13, 2023 14:56

bkkanq commented Jul 13, 2023

View reviewed changes

jrhee17 reviewed Jul 19, 2023

View reviewed changes

core/src/test/java/com/linecorp/armeria/server/ServerBuilderTest.java Outdated Show resolved Hide resolved

jrhee17 modified the milestones: 1.25.0, 1.26.0 Jul 30, 2023

bkkanq added 3 commits August 20, 2023 21:14

Provide a way to set the root context path

ae73e94

- address comments

Add a test for defaultContextPath()

2019aaa

Address comments

e17c6bb

- consider decorator - rename API to baseContextPath - handle VirtualHost separatly - add prefixes at VirtualHostBuilder#build.

bkkanq force-pushed the 3591-provide-context-path branch from 9fb9f74 to f452ee4 Compare August 20, 2023 15:29

Add a test to RouteDecoratingService

49f37e7

bkkanq force-pushed the 3591-provide-context-path branch from f452ee4 to 49f37e7 Compare August 20, 2023 15:32

bkkanq commented Aug 20, 2023

View reviewed changes

core/src/test/java/com/linecorp/armeria/server/RouteDecoratingServiceTest.java Outdated Show resolved Hide resolved

core/src/test/java/com/linecorp/armeria/server/RouteDecoratingServiceTest.java Outdated Show resolved Hide resolved

Add a test to RouteDecoratingService

d41dc30

bkkanq force-pushed the 3591-provide-context-path branch from d6d4c51 to d41dc30 Compare August 24, 2023 01:00

Fix test

4542060

bkkanq commented Aug 24, 2023

View reviewed changes

core/src/test/java/com/linecorp/armeria/server/VirtualHostBuilderTest.java Outdated Show resolved Hide resolved

bkkanq marked this pull request as ready for review August 24, 2023 04:08

bkkanq requested a review from ikhoon as a code owner August 24, 2023 04:08

bkkanq requested review from minwoox and trustin as code owners August 24, 2023 04:08

Merge branch 'main' into 3591-provide-context-path

23c0ad7

jrhee17 reviewed Aug 24, 2023

View reviewed changes

bkkanq and others added 2 commits September 1, 2023 00:06

Apply suggestions from code review

cc6e6ab

Co-authored-by: jrhee17 <guins_j@guins.org>

Address comments

d726a83

jrhee17 reviewed Sep 4, 2023

View reviewed changes

jrhee17 added the new feature label Sep 4, 2023

bkkanq and others added 3 commits September 8, 2023 14:22

Address comment

3cda80c

- seperate test class

rename context path to base context path

46b9220

lint failure

cfc9948

jrhee17 approved these changes Sep 18, 2023

View reviewed changes

minwoox approved these changes Sep 25, 2023

View reviewed changes

jrhee17 and others added 2 commits October 13, 2023 10:54

address comments by @minwoox

04c15be

Merge branch 'main' into pr-4802-bkkanq-3591-provide-context-path

505d1f5

ikhoon approved these changes Oct 13, 2023

View reviewed changes

ikhoon merged commit d9b6142 into line:main Oct 13, 2023
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide a way to set the root context path #4802

Provide a way to set the root context path #4802

bkkanq commented Apr 4, 2023

CLAassistant commented Apr 4, 2023 •

edited

ikhoon commented Apr 4, 2023 •

edited

bkkanq commented Apr 10, 2023

jrhee17 left a comment

jrhee17 Jul 19, 2023

jrhee17 Jul 19, 2023

bkkanq Jul 21, 2023

jrhee17 Jul 19, 2023

bkkanq Jul 21, 2023

jrhee17 Jul 19, 2023

jrhee17 Jul 19, 2023

jrhee17 Jul 19, 2023

jrhee17 Jul 19, 2023

bkkanq Aug 20, 2023

bkkanq commented Jul 21, 2023

github-actions bot commented Aug 24, 2023 •

edited

codecov bot commented Aug 24, 2023 •

edited

jrhee17 left a comment

jrhee17 Aug 24, 2023

bkkanq Aug 31, 2023

jrhee17 Aug 24, 2023

bkkanq Aug 31, 2023 •

edited

jrhee17 Sep 4, 2023

bkkanq Sep 8, 2023

jrhee17 left a comment

jrhee17 Sep 4, 2023

jrhee17 left a comment

minwoox left a comment

minwoox Sep 25, 2023

minwoox Sep 25, 2023

minwoox Sep 25, 2023

ikhoon left a comment

	* If the service is not an {@link HttpServiceWithRoutes}, the {@link Route} is the same as
	* {@link #route()}.

	route.withPrefix(contextPath), mappedRoute == null ? route : mappedRoute,
	route.withPrefix(contextPath), mappedRoute == null ? route.withPrefix(contextPath) : mappedRoute,

	try (ServerSocket ss = new ServerSocket(0)) {
	normalServerPort = ss.getLocalPort();
	}
	try (ServerSocket ss = new ServerSocket(0)) {
	virtualHostPort = ss.getLocalPort();
	}
	try (ServerSocket ss = new ServerSocket(0)) {
	fooHostPort = ss.getLocalPort();
	}

Provide a way to set the root context path #4802

Provide a way to set the root context path #4802

Conversation

bkkanq commented Apr 4, 2023

CLAassistant commented Apr 4, 2023 • edited

ikhoon commented Apr 4, 2023 • edited

bkkanq commented Apr 10, 2023

jrhee17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkkanq commented Jul 21, 2023

github-actions bot commented Aug 24, 2023 • edited

🔍 Build Scan® (commit: 505d1f5)

codecov bot commented Aug 24, 2023 • edited

Codecov Report

jrhee17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkkanq Aug 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrhee17 left a comment

Choose a reason for hiding this comment

minwoox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ikhoon left a comment

Choose a reason for hiding this comment

CLAassistant commented Apr 4, 2023 •

edited

ikhoon commented Apr 4, 2023 •

edited

github-actions bot commented Aug 24, 2023 •

edited

🔍 Build Scan® (commit: `505d1f5`)

codecov bot commented Aug 24, 2023 •

edited

bkkanq Aug 31, 2023 •

edited