Skip to content

Commit 623ee06

Browse files
Make server build fail when duplicate route and fail handler. (#6499)
### Motivation: `RejectRouterHandler.FAIL` does not work as intended. ### Modifications: - The call site that calls `rejectionHandler.handleDuplicateRoute(virtualHost, route, existingRoute)` catch`IllegalStateException` and throw it. - Fix false alert in `ContextPathTest`. (`ContextPathTest` already has duplicated route with `RejectRouterHandler.FAIL`, but it just warn not fail. ) ### Result: - If there are duplicated routes and `RejectRouterHandler.FAIL` is configured, Server build will be failed. - Close #6498
1 parent a704e69 commit 623ee06

File tree

5 files changed

+166
-10
lines changed

5 files changed

+166
-10
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright 2025 LINE Corporation
3+
*
4+
* LINE Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
17+
package com.linecorp.armeria.server;
18+
19+
import com.linecorp.armeria.common.annotation.UnstableApi;
20+
21+
/**
22+
* A {@link RuntimeException} thrown when attempting to register a route
23+
* that conflicts with an existing route for the same path.
24+
*
25+
* <p>This exception is raised when multiple routes share an identical path
26+
* and thus cannot be uniquely distinguished.</p>
27+
*/
28+
@UnstableApi
29+
public final class DuplicateRouteException extends RuntimeException {
30+
31+
private static final long serialVersionUID = 4679512839761213302L;
32+
33+
/**
34+
* Creates a new instance with the error message.
35+
*/
36+
public DuplicateRouteException(String errorMsg) {
37+
super(errorMsg);
38+
}
39+
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,18 +60,18 @@ public interface RejectedRouteHandler {
6060
};
6161

6262
/**
63-
* A {@link RejectedRouteHandler} that raises an {@link IllegalStateException} for a problematic
63+
* A {@link RejectedRouteHandler} that raises an {@link DuplicateRouteException} for a problematic
6464
* {@link Route}.
6565
*/
6666
RejectedRouteHandler FAIL = (virtualHost, route, existingRoute) -> {
6767
final String a = route.toString();
6868
final String b = existingRoute.toString();
6969
final String hostnamePattern = virtualHost.hostnamePattern();
7070
if (a.equals(b)) {
71-
throw new IllegalStateException(
71+
throw new DuplicateRouteException(
7272
"Virtual host '" + hostnamePattern + "' has a duplicate route: " + a);
7373
} else {
74-
throw new IllegalStateException(
74+
throw new DuplicateRouteException(
7575
"Virtual host '" + hostnamePattern + "' has routes with a conflict: " +
7676
a + " vs. " + b);
7777
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,8 @@ static Router<ServiceConfig> ofVirtualHost(VirtualHost virtualHost, Iterable<Ser
7272
final BiConsumer<Route, Route> rejectionConsumer = (route, existingRoute) -> {
7373
try {
7474
rejectionHandler.handleDuplicateRoute(virtualHost, route, existingRoute);
75+
} catch (DuplicateRouteException duplicateRouteException) {
76+
throw duplicateRouteException;
7577
} catch (Exception e) {
7678
logger.warn("Unexpected exception from a {}:",
7779
RejectedRouteHandler.class.getSimpleName(), e);

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,7 @@ protected void configure(ServerBuilder sb) throws Exception {
142142
.get("/route1")
143143
.build((ctx, req) -> HttpResponse.of("route1"))
144144
.serviceUnder("/serviceUnder1", (ctx, req) -> HttpResponse.of("serviceUnder1"))
145-
.serviceUnder("/serviceUnder2", serviceWithRoutes)
146-
// virtual host decorator
147-
.service("/decorated1", (ctx, req) -> HttpResponse.of(500))
148-
.decorator("/decorated1", (delegate, ctx, req) -> HttpResponse.of("decorated1"))
149-
.routeDecorator()
150-
.path("/decorated2")
151-
.build((delegate, ctx, req) -> HttpResponse.of("decorated2"));
145+
.serviceUnder("/serviceUnder2", serviceWithRoutes);
152146
sb.decorator(LoggingService.newDecorator());
153147
sb.rejectedRouteHandler(RejectedRouteHandler.FAIL);
154148
}
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
/*
2+
* Copyright 2025 LINE Corporation
3+
*
4+
* LINE Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
17+
package com.linecorp.armeria.server;
18+
19+
import static org.assertj.core.api.Assertions.assertThatCode;
20+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
21+
22+
import java.util.List;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import com.google.common.collect.ImmutableList;
27+
28+
import com.linecorp.armeria.common.HttpResponse;
29+
30+
class RejectRouterHandlerTest {
31+
32+
@Test
33+
void when_duplicate_route_exists_then_server_builder_should_ignore_or_warn() {
34+
final List<RejectedRouteHandler> handlers = ImmutableList.of(
35+
RejectedRouteHandler.DISABLED,
36+
RejectedRouteHandler.WARN
37+
);
38+
39+
for (RejectedRouteHandler handler : handlers) {
40+
assertThatCode(
41+
() -> duplicateRouteServerBuilder()
42+
.rejectedRouteHandler(handler)
43+
.build()
44+
).doesNotThrowAnyException();
45+
}
46+
}
47+
48+
@Test
49+
void when_duplicate_route_exists_then_server_builder_should_throw_error() {
50+
assertThatThrownBy(
51+
() -> duplicateRouteServerBuilder()
52+
.rejectedRouteHandler(RejectedRouteHandler.FAIL)
53+
.build()
54+
).isInstanceOf(DuplicateRouteException.class);
55+
}
56+
57+
@Test
58+
void when_duplicate_route_exists_then_virtual_host_builder_should_ignore_or_warn() {
59+
final List<RejectedRouteHandler> handlers = ImmutableList.of(
60+
RejectedRouteHandler.DISABLED,
61+
RejectedRouteHandler.WARN
62+
);
63+
64+
for (RejectedRouteHandler handler : handlers) {
65+
assertThatCode(
66+
() -> duplicatedRouteVirtualHostBuilder()
67+
.rejectedRouteHandler(handler)
68+
.and()
69+
.build()
70+
).doesNotThrowAnyException();
71+
}
72+
}
73+
74+
@Test
75+
void when_duplicate_route_exists_then_virtual_host_builder_should_throw_error() {
76+
assertThatThrownBy(
77+
() -> duplicatedRouteVirtualHostBuilder()
78+
.rejectedRouteHandler(RejectedRouteHandler.FAIL)
79+
.and()
80+
.build()
81+
).isInstanceOf(DuplicateRouteException.class);
82+
}
83+
84+
@Test
85+
void fail_route_handler_do_not_consider_decorator_in_same_path() {
86+
assertThatCode(() -> serverBuilderWithDecorator()
87+
.rejectedRouteHandler(RejectedRouteHandler.FAIL)
88+
.build()).doesNotThrowAnyException();
89+
90+
assertThatCode(() -> virtualServerBuilderWithDecorator()
91+
.rejectedRouteHandler(RejectedRouteHandler.FAIL)
92+
.and()
93+
.build()).doesNotThrowAnyException();
94+
}
95+
96+
private VirtualHostBuilder duplicatedRouteVirtualHostBuilder() {
97+
return Server.builder()
98+
.virtualHost("foo.com")
99+
.service("/foo", (ctx, req) -> HttpResponse.of("ok"))
100+
.service("/foo", (ctx, req) -> HttpResponse.of("duplicate"));
101+
}
102+
103+
private VirtualHostBuilder virtualServerBuilderWithDecorator() {
104+
return Server.builder()
105+
.virtualHost("foo.com")
106+
.service("/foo", (ctx, req) -> HttpResponse.of("ok"))
107+
.decorator("/foo", (delegate, ctx, req) -> HttpResponse.of("duplicate"));
108+
}
109+
110+
private ServerBuilder duplicateRouteServerBuilder() {
111+
return Server.builder()
112+
.service("/foo", (ctx, req) -> HttpResponse.of("ok"))
113+
.service("/foo", (ctx, req) -> HttpResponse.of("duplicate"));
114+
}
115+
116+
private ServerBuilder serverBuilderWithDecorator() {
117+
return Server.builder()
118+
.service("/foo", (ctx, req) -> HttpResponse.of("ok"))
119+
.decorator("/foo", (delegate, ctx, req) -> HttpResponse.of("duplicate"));
120+
}
121+
}

0 commit comments

Comments
 (0)