Add Invocation.body() and apply review cleanups (chain reduce, no-store regex, annotation cache)

Signed-off-by: Marvin Froeder <velo.br@gmail.com>

Author: velo

core/src/main/java/feign/AsynchronousMethodHandler.java

@@ -24,7 +24,6 @@
 import feign.interceptor.Invocation;
 import feign.interceptor.MethodInterceptor;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
@@ -79,11 +78,11 @@ public Object invoke(Object[] argv) throws Throwable {
             throw e.getCause();
           }
         };
-    MethodInterceptor.Chain chain = endOfChain;
-    List<MethodInterceptor> methodInterceptors = methodHandlerConfiguration.getMethodInterceptors();
-    for (int i = methodInterceptors.size() - 1; i >= 0; i--) {
-      chain = methodInterceptors.get(i).apply(chain);
-    }
+    MethodInterceptor.Chain chain =
+        methodHandlerConfiguration.getMethodInterceptors().stream()
+            .reduce(MethodInterceptor::andThen)
+            .map(interceptor -> interceptor.apply(endOfChain))
+            .orElse(endOfChain);
     return chain.next(invocation);
   }
 
@@ -269,31 +268,6 @@ static class Factory<C> implements MethodHandler.Factory<C> {
     private final RequestTemplateFactoryResolver requestTemplateFactoryResolver;
     private final Options options;
 
-    Factory(
-        AsyncClient<C> client,
-        Retryer retryer,
-        List<RequestInterceptor> requestInterceptors,
-        AsyncResponseHandler responseHandler,
-        Logger logger,
-        Logger.Level logLevel,
-        ExceptionPropagationPolicy propagationPolicy,
-        MethodInfoResolver methodInfoResolver,
-        RequestTemplateFactoryResolver requestTemplateFactoryResolver,
-        Options options) {
-      this(
-          client,
-          retryer,
-          requestInterceptors,
-          Collections.emptyList(),
-          responseHandler,
-          logger,
-          logLevel,
-          propagationPolicy,
-          methodInfoResolver,
-          requestTemplateFactoryResolver,
-          options);
-    }
-
     Factory(
         AsyncClient<C> client,
         Retryer retryer,

core/src/main/java/feign/SynchronousMethodHandler.java

@@ -24,7 +24,6 @@
 import feign.interceptor.Invocation;
 import feign.interceptor.MethodInterceptor;
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Stream;
@@ -58,11 +57,11 @@ public Object invoke(Object[] argv) throws Throwable {
             argv);
 
     MethodInterceptor.Chain endOfChain = inv -> runWithRetry(inv, options);
-    MethodInterceptor.Chain chain = endOfChain;
-    List<MethodInterceptor> methodInterceptors = methodHandlerConfiguration.getMethodInterceptors();
-    for (int i = methodInterceptors.size() - 1; i >= 0; i--) {
-      chain = methodInterceptors.get(i).apply(chain);
-    }
+    MethodInterceptor.Chain chain =
+        methodHandlerConfiguration.getMethodInterceptors().stream()
+            .reduce(MethodInterceptor::andThen)
+            .map(interceptor -> interceptor.apply(endOfChain))
+            .orElse(endOfChain);
     return chain.next(invocation);
   }
 
@@ -174,29 +173,6 @@ static class Factory implements MethodHandler.Factory<Object> {
     private final RequestTemplateFactoryResolver requestTemplateFactoryResolver;
     private final Options options;
 
-    Factory(
-        Client client,
-        Retryer retryer,
-        List<RequestInterceptor> requestInterceptors,
-        ResponseHandler responseHandler,
-        Logger logger,
-        Logger.Level logLevel,
-        ExceptionPropagationPolicy propagationPolicy,
-        RequestTemplateFactoryResolver requestTemplateFactoryResolver,
-        Options options) {
-      this(
-          client,
-          retryer,
-          requestInterceptors,
-          Collections.emptyList(),
-          responseHandler,
-          logger,
-          logLevel,
-          propagationPolicy,
-          requestTemplateFactoryResolver,
-          options);
-    }
-
     Factory(
         Client client,
         Retryer retryer,

core/src/main/java/feign/interceptor/Invocation.java

@@ -73,6 +73,18 @@ public Object[] arguments() {
     return arguments;
   }
 
+  /**
+   * Returns the typed argument that will be serialized as the request body, or {@code null} if the
+   * method has no body parameter or it was passed as {@code null}.
+   */
+  public Object body() {
+    Integer bodyIndex = methodMetadata.bodyIndex();
+    if (bodyIndex == null || arguments == null || bodyIndex >= arguments.length) {
+      return null;
+    }
+    return arguments[bodyIndex];
+  }
+
   /**
    * The HTTP {@link Response} captured after the framework's terminal chain step. Returns {@code
    * null} when the chain has not yet executed the HTTP call, or when the call failed before a

http-cache/src/main/java/feign/cache/HttpCacheInterceptor.java

@@ -19,12 +19,14 @@
 import feign.FeignException;
 import feign.RequestTemplate;
 import feign.Response;
+import feign.Util;
 import feign.interceptor.Invocation;
 import feign.interceptor.MethodInterceptor;
 import java.time.Instant;
 import java.util.Collection;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.regex.Pattern;
 
 /**
  * A {@link MethodInterceptor} that adds conditional revalidation headers ({@code If-None-Match} /
@@ -43,6 +45,8 @@
 @Experimental
 public final class HttpCacheInterceptor implements MethodInterceptor {
 
+  private static final Pattern NO_STORE = Pattern.compile("(?i)\\bno-store\\b");
+
   private final HttpCacheStore store;
   private final Function<Invocation, String> keyFn;
   private final Function<RequestTemplate, Boolean> cacheable;
@@ -78,27 +82,10 @@ public Object intercept(Invocation invocation, Chain chain) throws Throwable {
     }
     String key = keyFn.apply(invocation);
     CachedEntry hit = store.get(key);
-    if (hit != null) {
-      if (hit.etag() != null) {
-        template.header("If-None-Match", hit.etag());
-      }
-      if (hit.lastModified() != null) {
-        template.header("If-Modified-Since", hit.lastModified());
-      }
-    }
+    addConditionalHeaders(template, hit);
     try {
       Object result = chain.next(invocation);
-      Response response = invocation.response();
-      if (response != null
-          && response.status() >= 200
-          && response.status() < 300
-          && !hasNoStore(response)) {
-        String etag = firstHeader(response.headers(), "ETag");
-        String lastMod = firstHeader(response.headers(), "Last-Modified");
-        if (etag != null || lastMod != null) {
-          store.put(key, new CachedEntry(result, etag, lastMod, Instant.now()));
-        }
-      }
+      maybeStore(key, result, invocation.response());
       return result;
     } catch (FeignException e) {
       if (e.status() == 304 && hit != null) {
@@ -108,6 +95,38 @@ public Object intercept(Invocation invocation, Chain chain) throws Throwable {
     }
   }
 
+  private static void addConditionalHeaders(RequestTemplate template, CachedEntry hit) {
+    if (hit == null) {
+      return;
+    }
+    if (hit.etag() != null) {
+      template.header("If-None-Match", hit.etag());
+    }
+    if (hit.lastModified() != null) {
+      template.header("If-Modified-Since", hit.lastModified());
+    }
+  }
+
+  private void maybeStore(String key, Object result, Response response) {
+    if (response == null) {
+      return;
+    }
+    int status = response.status();
+    if (status < 200 || status >= 300) {
+      return;
+    }
+    Map<String, Collection<String>> headers = response.headers();
+    if (containsNoStore(headers)) {
+      return;
+    }
+    String etag = firstHeader(headers, "ETag");
+    String lastMod = firstHeader(headers, "Last-Modified");
+    if (etag == null && lastMod == null) {
+      return;
+    }
+    store.put(key, new CachedEntry(result, etag, lastMod, Instant.now()));
+  }
+
   private static String defaultKey(Invocation invocation) {
     RequestTemplate template = invocation.requestTemplate();
     return invocation.methodMetadata().configKey() + "|" + template.method() + " " + template.url();
@@ -118,13 +137,9 @@ private static Boolean defaultCacheable(RequestTemplate template) {
     return "GET".equalsIgnoreCase(method) || "HEAD".equalsIgnoreCase(method);
   }
 
-  private static boolean hasNoStore(Response response) {
-    Collection<String> values = response.headers().get("Cache-Control");
-    if (values == null) {
-      return false;
-    }
-    for (String value : values) {
-      if (value != null && value.toLowerCase().contains("no-store")) {
+  private static boolean containsNoStore(Map<String, Collection<String>> headers) {
+    for (String value : Util.valuesOrEmpty(headers, "Cache-Control")) {
+      if (value != null && NO_STORE.matcher(value).find()) {
         return true;
       }
     }

validation-jakarta/src/main/java/feign/validation/BeanValidationMethodInterceptor.java

@@ -16,7 +16,6 @@
 package feign.validation;
 
 import feign.Experimental;
-import feign.MethodMetadata;
 import feign.interceptor.Invocation;
 import feign.interceptor.MethodInterceptor;
 import jakarta.validation.ConstraintViolation;
@@ -25,8 +24,12 @@
 import jakarta.validation.Validation;
 import jakarta.validation.Validator;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Method;
+import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * A {@link MethodInterceptor} that validates the method's body argument and any {@link Valid}
@@ -46,6 +49,8 @@ public final class BeanValidationMethodInterceptor implements MethodInterceptor
 
   private final Validator validator;
   private final Class<?>[] groups;
+  private final ConcurrentMap<Method, Annotation[][]> parameterAnnotationsCache =
+      new ConcurrentHashMap<>();
 
   public BeanValidationMethodInterceptor(Validator validator, Class<?>... groups) {
     this.validator = validator;
@@ -62,27 +67,39 @@ public static BeanValidationMethodInterceptor usingDefaultFactory() {
 
   @Override
   public Object intercept(Invocation invocation, Chain chain) throws Throwable {
+    Set<ConstraintViolation<Object>> violations = collectViolations(invocation);
+    if (!violations.isEmpty()) {
+      throw new ConstraintViolationException(violations);
+    }
+    return chain.next(invocation);
+  }
+
+  private Set<ConstraintViolation<Object>> collectViolations(Invocation invocation) {
     Object[] arguments = invocation.arguments();
-    if (arguments != null && arguments.length > 0) {
-      Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>();
-      MethodMetadata metadata = invocation.methodMetadata();
-      Annotation[][] parameterAnnotations = invocation.method().getParameterAnnotations();
-      Integer bodyIndex = metadata.bodyIndex();
-      for (int i = 0; i < arguments.length; i++) {
-        Object argument = arguments[i];
-        if (argument == null) {
-          continue;
-        }
-        boolean isBody = bodyIndex != null && bodyIndex == i;
-        if (isBody || hasValidAnnotation(parameterAnnotations[i])) {
-          violations.addAll(validator.validate(argument, groups));
-        }
+    if (arguments == null || arguments.length == 0) {
+      return Collections.emptySet();
+    }
+    Set<ConstraintViolation<Object>> violations = new LinkedHashSet<>();
+    Object body = invocation.body();
+    if (body != null) {
+      violations.addAll(validator.validate(body, groups));
+    }
+    Integer bodyIndex = invocation.methodMetadata().bodyIndex();
+    Annotation[][] parameterAnnotations =
+        parameterAnnotationsCache.computeIfAbsent(
+            invocation.method(), Method::getParameterAnnotations);
+    for (int i = 0; i < arguments.length; i++) {
+      if (arguments[i] == null) {
+        continue;
       }
-      if (!violations.isEmpty()) {
-        throw new ConstraintViolationException(violations);
+      if (bodyIndex != null && bodyIndex == i) {
+        continue;
+      }
+      if (hasValidAnnotation(parameterAnnotations[i])) {
+        violations.addAll(validator.validate(arguments[i], groups));
       }
     }
-    return chain.next(invocation);
+    return violations;
   }
 
   private static boolean hasValidAnnotation(Annotation[] annotations) {