Fix that the gloabl defaultServiceNaming is not applied to annotate…

…d services (#3589) Motivation: The global `defaultServiceNaming` should be applied to the annotated service unless the service has its own policy. Modifications: - Remove setting the name of the annotated service as a default name in the `DefaultServiceConfigSetters`. - The name of the annotated service will be used anyway if there's no `ServiceNaming` set. - Remove the usage of deprecated `ServiceConfig.defaultServiceName()`. Result: - You can now set the service name of an annotated service using the global `defaultServiceNaming`.
line · Jun 5, 2021 · d9a50e8 · d9a50e8
1 parent 22bcfb0
commit d9a50e8
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 9 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedService.java b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedService.java
@@ -132,7 +132,8 @@ public final class AnnotatedService implements HttpService {
 
     private final ResponseType responseType;
     private final boolean useBlockingTaskExecutor;
-    private final String defaultServiceName;
+    private final String serviceName;
+    private final boolean serviceNameSetByAnnotation;
 
     AnnotatedService(Object object, Method method,
                      List<AnnotatedValueResolver> resolvers,
@@ -178,9 +179,11 @@ public final class AnnotatedService implements HttpService {
             serviceName = AnnotationUtil.findFirst(object.getClass(), ServiceName.class);
         }
         if (serviceName != null) {
-            defaultServiceName = serviceName.value();
+            this.serviceName = serviceName.value();
+            serviceNameSetByAnnotation = true;
         } else {
-            defaultServiceName = getUserClass(object.getClass()).getName();
+            this.serviceName = getUserClass(object.getClass()).getName();
+            serviceNameSetByAnnotation = false;
         }
 
         this.method.setAccessible(true);
@@ -244,7 +247,11 @@ private static void warnIfHttpResponseArgumentExists(Type returnType, Parameteri
     }
 
     public String serviceName() {
-        return defaultServiceName;
+        return serviceName;
+    }
+
+    public boolean serviceNameSetByAnnotation() {
+        return serviceNameSetByAnnotation;
     }
 
     public String methodName() {

diff --git a/core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java b/core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java
@@ -173,7 +173,9 @@ ServiceConfigBuilder toServiceConfigBuilder(Route route, HttpService service) {
         } else if (defaultServiceNaming != null) {
             serviceConfigBuilder.defaultServiceNaming(defaultServiceNaming);
         } else {
-            if (annotatedService != null) {
+            // Set the default service name only when the service name is set using @ServiceName.
+            // If it's not, the global defaultServiceNaming is used.
+            if (annotatedService != null && annotatedService.serviceNameSetByAnnotation()) {
                 serviceConfigBuilder.defaultServiceName(annotatedService.serviceName());
             }
         }

diff --git a/core/src/test/java/com/linecorp/armeria/server/logging/AnnotatedServiceLogNameTest.java b/core/src/test/java/com/linecorp/armeria/server/logging/AnnotatedServiceLogNameTest.java
@@ -25,6 +25,8 @@
 import com.linecorp.armeria.client.WebClient;
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.HttpStatus;
+import com.linecorp.armeria.common.SessionProtocol;
+import com.linecorp.armeria.server.Server;
 import com.linecorp.armeria.server.ServerBuilder;
 import com.linecorp.armeria.server.ServiceNaming;
 import com.linecorp.armeria.server.ServiceRequestContext;
@@ -103,7 +105,7 @@ void serviceName_withDefault() {
         client.get("/default/service-name").aggregate().join();
         assertThat(sctx.config().defaultServiceNaming().serviceName(sctx))
                 .isEqualTo(MyAnnotatedService.class.getName());
-        assertThat(sctx.config().defaultServiceName()).isEqualTo(MyAnnotatedService.class.getName());
+        assertThat(sctx.config().defaultServiceName()).isNull();
         assertThat(sctx.log().whenComplete().join().serviceName())
                 .isEqualTo(MyAnnotatedService.class.getName());
     }
@@ -176,6 +178,19 @@ void serviceName_withAnonymous() {
                 .matches("^AnnotatedServiceLogNameTest(\\$[0-9]+)+$");
     }
 
+    @Test
+    void globalDefaultServiceNamingIsApplied() {
+        final Server server = Server.builder()
+                                    .defaultServiceNaming(ctx -> "foo")
+                                    .annotatedService(new MyAnnotatedService()).build();
+        server.start().join();
+        final WebClient client =
+                WebClient.of("http://127.0.0.1:" + server.activeLocalPort(SessionProtocol.HTTP));
+        client.get("/service-name").aggregate().join();
+        assertThat(sctx.log().whenComplete().join().serviceName()).isEqualTo("foo");
+        server.stop().join();
+    }
+
     private static class MyAnnotatedService {
         @Get("/service-name")
         public HttpResponse serviceName(ServiceRequestContext ctx) {

diff --git a/...tated-http-service-kotlin/src/main/kotlin/example/armeria/server/annotated/kotlin/Main.kt b/...tated-http-service-kotlin/src/main/kotlin/example/armeria/server/annotated/kotlin/Main.kt
@@ -33,7 +33,7 @@ fun newServer(port: Int): Server {
         .pathPrefix("/contextAware")
         .decorator(
             CoroutineContextService.newDecorator { ctx ->
-                CoroutineName(ctx.config().defaultServiceName() ?: "name")
+                CoroutineName(ctx.config().defaultServiceNaming().serviceName(ctx) ?: "name")
             }
         )
         .applyCommonDecorator()

diff --git a/kotlin/src/main/java/com/linecorp/armeria/server/kotlin/CoroutineContextService.java b/kotlin/src/main/java/com/linecorp/armeria/server/kotlin/CoroutineContextService.java
@@ -42,7 +42,7 @@
  * >         }
  * >     })
  * >     .decorator(CoroutineContextService.newDecorator { ctx ->
- * >         CoroutineName(ctx.config().defaultServiceName() ?: "none")
+ * >         CoroutineName(ctx.config().defaultServiceNaming.serviceName(ctx) ?: "name")
  * >     })
  * }
  * </pre>

diff --git a/site/src/pages/docs/server-annotated-service.mdx b/site/src/pages/docs/server-annotated-service.mdx
@@ -1603,7 +1603,7 @@ import com.linecorp.armeria.server.kotlin.CoroutineContextService
 serverBuilder
     .annotatedService()
     .decorator(CoroutineContextService.newDecorator { ctx ->
-        CoroutineName(ctx.config().defaultServiceName() ?: "none")
+        CoroutineName(ctx.config().defaultServiceNaming().serviceName(ctx) ?: "name")
     })
     .build(MyAnnotatedService())
 ```