Skip to content

Commit

Permalink
Reload DocService on server reconfigure (#4513)
Browse files Browse the repository at this point in the history
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 🙏
  • Loading branch information
jrhee17 authored Dec 13, 2022
1 parent 4cc5b8b commit f826016
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 18 deletions.
30 changes: 12 additions & 18 deletions core/src/main/java/com/linecorp/armeria/server/docs/DocService.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<VirtualHost> virtualHosts = config.findVirtualHosts(DocService.this);

final List<ServiceConfig> 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<VirtualHost> virtualHosts = config.findVirtualHosts(this);

final List<ServiceConfig> 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;
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
}

0 comments on commit f826016

Please sign in to comment.