From f826016364d104d6a2401380ed0c348b5806d73c Mon Sep 17 00:00:00 2001 From: jrhee17 Date: Tue, 13 Dec 2022 22:19:49 +0900 Subject: [PATCH] Reload `DocService` on server reconfigure (#4513) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: Previously, `DocService` wasn't able to handle the changed services as a result of `reconfigure`. ``` sb = Server.builder() .annotatedService("/hello", new HelloService()) .serviceUnder("/docs", DocService.builder().build()) ... sb.reconfigure(sb2 -> sb2.annotatedService("/hello2", new HelloService()) .serviceUnder("/docs", DocService.builder().build())) ``` - If a shared `DocService` is used, services wouldn't be updated and the old specification would be used. - If a new `DocService` is added, appropriate callbacks wouldn't be called and `/specifications.json` would result in a 404. https://github.com/line/armeria/blob/cae3b6da373101c0976e2d00f1f3cbb706d07e00/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java#L217-L220 We may note that `Server#configs` is immutable, and that `Server#configs` is fixed at the timing when the `serviceAdded` callback is called. https://github.com/line/armeria/blob/cae3b6da373101c0976e2d00f1f3cbb706d07e00/core/src/main/java/com/linecorp/armeria/server/DefaultServerConfig.java#L246 Modifications: - Change the timing when `DocService#specifications` is loaded to `DecoratingService#serviceAdded`. Result: - Users can use `DocService` even when `Server#reconfigure` is called. Note: This PR contains changes from #4491 . Review this PR first 🙏 --- .../armeria/server/docs/DocService.java | 30 ++++----- .../docs/DocServiceReconfigureTest.java | 64 +++++++++++++++++++ 2 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 core/src/test/java/com/linecorp/armeria/server/docs/DocServiceReconfigureTest.java diff --git a/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java b/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java index a6d2ee14955..7101cfe7401 100644 --- a/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java +++ b/core/src/main/java/com/linecorp/armeria/server/docs/DocService.java @@ -64,7 +64,6 @@ import com.linecorp.armeria.server.HttpService; import com.linecorp.armeria.server.Server; import com.linecorp.armeria.server.ServerConfig; -import com.linecorp.armeria.server.ServerListenerAdapter; import com.linecorp.armeria.server.Service; import com.linecorp.armeria.server.ServiceConfig; import com.linecorp.armeria.server.ServiceRequestContext; @@ -189,23 +188,18 @@ public void serviceAdded(ServiceConfig cfg) throws Exception { server = cfg.server(); // Build the Specification after all the services are added to the server. - server.addListener(new ServerListenerAdapter() { - @Override - public void serverStarting(Server server) throws Exception { - final ServerConfig config = server.config(); - final List virtualHosts = config.findVirtualHosts(DocService.this); - - final List services = - config.serviceConfigs().stream() - .filter(se -> virtualHosts.contains(se.virtualHost())) - .collect(toImmutableList()); - final ExecutorService executorService = Executors.newSingleThreadExecutor( - ThreadFactories.newThreadFactory("docservice-loader", true)); - vfs().specificationLoader.updateServices(services, executorService).handle((res, e) -> { - executorService.shutdown(); - return null; - }); - } + final ServerConfig config = server.config(); + final List virtualHosts = config.findVirtualHosts(this); + + final List services = + config.serviceConfigs().stream() + .filter(se -> virtualHosts.contains(se.virtualHost())) + .collect(toImmutableList()); + final ExecutorService executorService = Executors.newSingleThreadExecutor( + ThreadFactories.newThreadFactory("docservice-loader", true)); + vfs().specificationLoader.updateServices(services, executorService).handle((res, e) -> { + executorService.shutdown(); + return null; }); } diff --git a/core/src/test/java/com/linecorp/armeria/server/docs/DocServiceReconfigureTest.java b/core/src/test/java/com/linecorp/armeria/server/docs/DocServiceReconfigureTest.java new file mode 100644 index 00000000000..ac67a49091a --- /dev/null +++ b/core/src/test/java/com/linecorp/armeria/server/docs/DocServiceReconfigureTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2022 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.docs; + +import static net.javacrumbs.jsonunit.fluent.JsonFluentAssert.assertThatJson; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import com.linecorp.armeria.server.ServerBuilder; +import com.linecorp.armeria.server.annotation.Get; +import com.linecorp.armeria.testing.junit5.server.ServerExtension; + +class DocServiceReconfigureTest { + + @RegisterExtension + static ServerExtension server = new ServerExtension() { + @Override + protected void configure(ServerBuilder sb) throws Exception { + sb.annotatedService("/service", new Object() { + @Get("/before") + public void before() { + } + }); + sb.serviceUnder("/docs", DocService.builder().build()); + } + }; + + @Test + void testReconfigure() { + String specification = server.blockingWebClient().get("/docs/specification.json").contentUtf8(); + assertThatJson(specification).node("services").isArray().ofLength(1); + assertThatJson(specification).node("services[0].methods").isArray().ofLength(1); + assertThatJson(specification).node("services[0].methods[0].name").isEqualTo("before"); + + server.server().reconfigure(sb -> { + sb.annotatedService("/service", new Object() { + @Get("/after") + public void after() { + } + }); + sb.serviceUnder("/docs", DocService.builder().build()); + }); + + specification = server.blockingWebClient().get("/docs/specification.json").contentUtf8(); + assertThatJson(specification).node("services").isArray().ofLength(1); + assertThatJson(specification).node("services[0].methods").isArray().ofLength(1); + assertThatJson(specification).node("services[0].methods[0].name").isEqualTo("after"); + } +}