Support ResponseEntity in annotated service instead of HttpResult (…

…#5586) Motivation: We have `HttpResult<T>` and `ResponseEntity<T>`, both of them support automatic JSON deserialization. However, we can't use `HttpResult` for clients because it is located in `com.linecorp.armeria.server.annotation`. So, we should support `ResponseEntity` in that package so that users are not confused as to which one to use, and so that it can be used commonly between server and client. Modifications: - Add logic to check if result type is `ResponseEntity`. - Create `ResponseEntityUtil` to build response headers as well as `HttpResult`. - Deprecate `HttpResult`. - Tests - Add `ResponseEntity` cases for the cases that verify `HttpResult` behavior itself. - Replace for the cases that use `HttpResult` to verify other behaviors. Result: - Closes #4910
line · Apr 22, 2024 · a8dea21 · a8dea21
1 parent e1d1377
commit a8dea21
Show file tree

Hide file tree

Showing 12 changed files with 470 additions and 51 deletions.
diff --git a/core/src/main/java/com/linecorp/armeria/common/ResponseEntity.java b/core/src/main/java/com/linecorp/armeria/common/ResponseEntity.java
@@ -43,6 +43,15 @@ static <T> ResponseEntity<T> of(ResponseHeaders headers, T content) {
         return of(headers, content, HttpHeaders.of());
     }
 
+    /**
+     * Returns a newly created {@link ResponseEntity} with the specified {@code content} and
+     * {@link HttpStatus#OK} status.
+     */
+    static <T> ResponseEntity<T> of(T content) {
+        requireNonNull(content, "content");
+        return of(ResponseHeaders.of(HttpStatus.OK), content, HttpHeaders.of());
+    }
+
     /**
      * Returns a newly created {@link ResponseEntity} with the specified {@link ResponseHeaders},
      * {@code content} and {@linkplain HttpHeaders trailers}.

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
@@ -46,6 +46,7 @@
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.common.MediaType;
+import com.linecorp.armeria.common.ResponseEntity;
 import com.linecorp.armeria.common.ResponseHeaders;
 import com.linecorp.armeria.common.ResponseHeadersBuilder;
 import com.linecorp.armeria.common.annotation.Nullable;
@@ -192,25 +193,28 @@ private static Type getActualReturnType(Method method) {
             genericReturnType = method.getGenericReturnType();
         }
 
-        if (HttpResult.class.isAssignableFrom(returnType)) {
+        if (HttpResult.class.isAssignableFrom(returnType) ||
+            ResponseEntity.class.isAssignableFrom(returnType)) {
             final ParameterizedType type = (ParameterizedType) genericReturnType;
-            warnIfHttpResponseArgumentExists(type, type);
+            warnIfHttpResponseArgumentExists(type, type, returnType);
             return type.getActualTypeArguments()[0];
         } else {
             return genericReturnType;
         }
     }
 
-    private static void warnIfHttpResponseArgumentExists(Type returnType, ParameterizedType type) {
+    private static void warnIfHttpResponseArgumentExists(Type returnType,
+                                                         ParameterizedType type,
+                                                         Class<?> originalReturnType) {
         for (final Type arg : type.getActualTypeArguments()) {
             if (arg instanceof ParameterizedType) {
-                warnIfHttpResponseArgumentExists(returnType, (ParameterizedType) arg);
+                warnIfHttpResponseArgumentExists(returnType, (ParameterizedType) arg, originalReturnType);
             } else if (arg instanceof Class) {
                 final Class<?> clazz = (Class<?>) arg;
                 if (HttpResponse.class.isAssignableFrom(clazz) ||
                     AggregatedHttpResponse.class.isAssignableFrom(clazz)) {
                     logger.warn("{} in the return type '{}' may take precedence over {}.",
-                                clazz.getSimpleName(), returnType, HttpResult.class.getSimpleName());
+                                clazz.getSimpleName(), returnType, originalReturnType.getSimpleName());
                 }
             }
         }
@@ -400,7 +404,12 @@ private HttpResponse convertResponse(ServiceRequestContext ctx, @Nullable Object
         final ResponseHeaders headers;
         final HttpHeaders trailers;
 
-        if (result instanceof HttpResult) {
+        if (result instanceof ResponseEntity) {
+            final ResponseEntity<?> responseEntity = (ResponseEntity<?>) result;
+            headers = ResponseEntityUtil.buildResponseHeaders(ctx, responseEntity);
+            result = responseEntity.hasContent() ? responseEntity.content() : null;
+            trailers = responseEntity.trailers();
+        } else if (result instanceof HttpResult) {
             final HttpResult<?> httpResult = (HttpResult<?>) result;
             headers = HttpResultUtil.buildResponseHeaders(ctx, httpResult);
             result = httpResult.content();

diff --git a/...a/com/linecorp/armeria/internal/server/annotation/CompositeResponseConverterFunction.java b/...a/com/linecorp/armeria/internal/server/annotation/CompositeResponseConverterFunction.java
@@ -24,6 +24,7 @@
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.MediaType;
+import com.linecorp.armeria.common.ResponseEntity;
 import com.linecorp.armeria.common.ResponseHeaders;
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.util.SafeCloseable;
@@ -63,7 +64,13 @@ public HttpResponse convertResponse(ServiceRequestContext ctx,
         if (result instanceof HttpResponse) {
             return (HttpResponse) result;
         }
-        if (result instanceof HttpResult) {
+
+        if (result instanceof ResponseEntity) {
+            final ResponseEntity<?> responseEntity = (ResponseEntity<?>) result;
+            headers = ResponseEntityUtil.buildResponseHeaders(ctx, responseEntity);
+            result = responseEntity.hasContent() ? responseEntity.content() : null;
+            trailers = responseEntity.trailers();
+        } else if (result instanceof HttpResult) {
             final HttpResult<?> httpResult = (HttpResult<?>) result;
             headers = HttpResultUtil.buildResponseHeaders(ctx, httpResult);
             result = httpResult.content();

diff --git a/core/src/main/java/com/linecorp/armeria/internal/server/annotation/ResponseEntityUtil.java b/core/src/main/java/com/linecorp/armeria/internal/server/annotation/ResponseEntityUtil.java
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2024 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.internal.server.annotation;
+
+import com.linecorp.armeria.common.MediaType;
+import com.linecorp.armeria.common.ResponseEntity;
+import com.linecorp.armeria.common.ResponseHeaders;
+import com.linecorp.armeria.common.ResponseHeadersBuilder;
+import com.linecorp.armeria.server.ServiceRequestContext;
+
+/**
+ * Utilities for {@link ResponseEntity}.
+ */
+final class ResponseEntityUtil {
+    /**
+     * Build {@link ResponseHeaders} from the given {@link ServiceRequestContext} and {@link ResponseEntity}.
+     */
+    static ResponseHeaders buildResponseHeaders(ServiceRequestContext ctx, ResponseEntity<?> result) {
+        final ResponseHeaders headers = result.headers();
+
+        if (headers.status().isContentAlwaysEmpty()) {
+            return headers;
+        }
+        if (headers.contentType() != null) {
+            return headers;
+        }
+
+        final ResponseHeadersBuilder builder = headers.toBuilder();
+
+        final MediaType negotiatedResponseMediaType = ctx.negotiatedResponseMediaType();
+        if (negotiatedResponseMediaType != null) {
+            builder.contentType(negotiatedResponseMediaType);
+        }
+
+        return builder.build();
+    }
+
+    private ResponseEntityUtil() {}
+}
diff --git a/core/src/main/java/com/linecorp/armeria/server/annotation/HttpResult.java b/core/src/main/java/com/linecorp/armeria/server/annotation/HttpResult.java
@@ -19,6 +19,7 @@
 
 import com.linecorp.armeria.common.HttpHeaders;
 import com.linecorp.armeria.common.HttpStatus;
+import com.linecorp.armeria.common.ResponseEntity;
 import com.linecorp.armeria.common.ResponseHeaders;
 import com.linecorp.armeria.common.annotation.Nullable;
 
@@ -28,8 +29,11 @@
  * and it would be converted into response body by a {@link ResponseConverterFunction}.
  *
  * @param <T> the type of a content which is to be converted into response body
+ *
+ * @deprecated Use {@link ResponseEntity} instead.
  */
 @FunctionalInterface
+@Deprecated
 public interface HttpResult<T> {
 
     /**

diff --git a/core/src/main/java/com/linecorp/armeria/server/annotation/ResponseConverterFunction.java b/core/src/main/java/com/linecorp/armeria/server/annotation/ResponseConverterFunction.java
@@ -25,6 +25,7 @@
 import com.linecorp.armeria.common.HttpResponse;
 import com.linecorp.armeria.common.HttpStatus;
 import com.linecorp.armeria.common.MediaType;
+import com.linecorp.armeria.common.ResponseEntity;
 import com.linecorp.armeria.common.ResponseHeaders;
 import com.linecorp.armeria.common.annotation.Nullable;
 import com.linecorp.armeria.common.annotation.UnstableApi;
@@ -77,15 +78,15 @@ default Boolean isResponseStreaming(Type returnType, @Nullable MediaType produce
      *                unless you specify it with {@link StatusCode} on the method.
      *                The headers also will include a {@link MediaType} if
      *                {@link ServiceRequestContext#negotiatedResponseMediaType()} returns it.
-     *                If the method returns {@link HttpResult}, this headers is the same headers from
-     *                {@link HttpResult#headers()}
+     *                If the annotated method returns an {@link HttpResult} or a {@link ResponseEntity},
+     *                the headers provided by them will be given as they are.
      *                Please note that the additional headers set by
      *                {@link ServiceRequestContext#mutateAdditionalResponseHeaders(Consumer)}
      *                and {@link AdditionalHeader} are not included in this headers.
      * @param result The result of the service method.
      * @param trailers The HTTP trailers that you might want to use to create the {@link HttpResponse}.
-     *                 If the annotated method returns {@link HttpResult}, this trailers is the same
-     *                 trailers from {@link HttpResult#trailers()}.
+     *                 If the annotated method returns an {@link HttpResult} or a {@link ResponseEntity},
+     *                 the trailers provided by them will be given as they are.
      *                 Please note that the additional trailers set by
      *                 {@link ServiceRequestContext#mutateAdditionalResponseTrailers(Consumer)}
      *                 and {@link AdditionalTrailer} are not included in this trailers.