Refactored data clumps with the help of LLMs (research project) (#2497)

* refactor: data clumps with LLM * Align with other openfeign projects * Refactored to favor immutability and easy of read * Update codeql-analysis.yml * Quickbuild * Update license header --------- Co-authored-by: Marvin <velo@users.noreply.github.com> Co-authored-by: Marvin Froeder <velo.br@gmail.com>
OpenFeign · Aug 5, 2024 · 6d7b35f · 6d7b35f
1 parent 695e781
commit 6d7b35f
Show file tree

Hide file tree

Showing 8 changed files with 325 additions and 129 deletions.
diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml
@@ -0,0 +1,2 @@
+github: [velo]
+patreon: velo132
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
@@ -9,3 +9,4 @@ updates:
     directory: "/"
     schedule:
       interval: "weekly"
+    open-pull-requests-limit: 100
diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml
@@ -0,0 +1,99 @@
+# ----------------------------------------------------------------------------
+# Copyright 2012-2014 The Feign Authors
+#
+# The Netty Project 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.
+# ----------------------------------------------------------------------------
+# For most projects, this workflow file will not need changing; you simply need
+# to commit it to your repository.
+#
+# You may wish to alter this file to override the set of languages analyzed,
+# or to provide custom queries or build logic.
+name: "CodeQL"
+
+on:
+  push:
+    branches: ["master"]
+  pull_request:
+    # The branches below must be a subset of the branches above
+    branches: ["master"]
+  schedule:
+    - cron: '0 13 * * 3'
+
+permissions:
+  contents: read
+
+jobs:
+  analyze:
+    permissions:
+      actions: read  # for github/codeql-action/init to get workflow details
+      contents: read  # for actions/checkout to fetch code
+      security-events: write  # for github/codeql-action/analyze to upload SARIF results
+    name: Analyze
+    runs-on: ubuntu-latest
+
+    strategy:
+      fail-fast: false
+      matrix:
+        # Override automatic language detection by changing the below list
+        # Supported options are ['csharp', 'cpp', 'go', 'java', 'javascript', 'python']
+        language: ['java']
+        # Learn more...
+        # https://docs.github.com/en/github/finding-security-vulnerabilities-and-errors-in-your-code/configuring-code-scanning#overriding-automatic-language-detection
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    # Cache .m2/repository
+    - name: Cache local Maven repository
+      uses: actions/cache@v3
+      continue-on-error: true
+      with:
+        path: ~/.m2/repository
+        key: ${{ runner.os }}-maven-${{ matrix.language }} ${{ hashFiles('**/pom.xml') }}
+        restore-keys: |
+          ${{ runner.os }}-maven-${{ matrix.language }}
+          ${{ runner.os }}-maven-
+
+    # Initializes the CodeQL tools for scanning.
+    - name: Initialize CodeQL
+      uses: github/codeql-action/init@v2
+      with:
+        languages: ${{ matrix.language }}
+        # If you wish to specify custom queries, you can do so here or in a config file.
+        # By default, queries listed here will override any specified in a config file.
+        # Prefix the list here with "+" to use these queries and those in the config file.
+        # queries: ./path/to/local/query, your-org/your-repo/queries@main
+
+    # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
+    # If this step fails, then you should remove it and run the build manually (see below)
+    # - name: Autobuild
+    #  uses: github/codeql-action/autobuild@v2
+
+    # ℹ️ Command-line programs to run using the OS shell.
+    # 📚 https://git.io/JvXDl
+
+    # ✏️ If the Autobuild fails above, remove it and uncomment the following three lines
+    #    and modify them (or add more) to build your code if your project
+    #    uses a compiled language
+    - name: Setup Java JDK
+      uses: actions/setup-java@v4.0.0
+      with:
+        distribution: 'temurin'
+        java-version: '21'
+
+    - name: Compile project
+      run: ./mvnw -B -ntp clean package -Pquickbuild -Dtoolchain.skip=true
+
+    - name: Perform CodeQL Analysis
+      uses: github/codeql-action/analyze@v2
diff --git a/core/src/main/java/feign/AsyncFeign.java b/core/src/main/java/feign/AsyncFeign.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 The Feign Authors
+ * Copyright 2012-2024 The Feign Authors
  *
  * Licensed 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
@@ -219,9 +219,7 @@ public AsyncFeign<C> internalBuild() {
               propagationPolicy,
               methodInfoResolver,
               new RequestTemplateFactoryResolver(encoder, queryMapEncoder),
-              options,
-              decoder,
-              errorDecoder);
+              options);
       final ReflectiveFeign<C> feign =
           new ReflectiveFeign<>(
               contract, methodHandlerFactory, invocationHandlerFactory, defaultContextSupplier);

diff --git a/core/src/main/java/feign/AsynchronousMethodHandler.java b/core/src/main/java/feign/AsynchronousMethodHandler.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2023 The Feign Authors
+ * Copyright 2012-2024 The Feign Authors
  *
  * Licensed 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
@@ -19,8 +19,6 @@
 
 import feign.InvocationHandlerFactory.MethodHandler;
 import feign.Request.Options;
-import feign.codec.Decoder;
-import feign.codec.ErrorDecoder;
 import java.io.IOException;
 import java.util.List;
 import java.util.Optional;
@@ -32,56 +30,31 @@
 
 final class AsynchronousMethodHandler<C> implements MethodHandler {
 
-  private final MethodMetadata metadata;
-  private final Target<?> target;
   private final AsyncClient<C> client;
-  private final Retryer retryer;
-  private final List<RequestInterceptor> requestInterceptors;
-  private final Logger logger;
-  private final Logger.Level logLevel;
-  private final RequestTemplate.Factory buildTemplateFromArgs;
-  private final Options options;
-  private final ExceptionPropagationPolicy propagationPolicy;
   private final C requestContext;
   private final AsyncResponseHandler asyncResponseHandler;
   private final MethodInfo methodInfo;
+  private final MethodHandlerConfiguration methodHandlerConfiguration;
 
   private AsynchronousMethodHandler(
-      Target<?> target,
+      MethodHandlerConfiguration methodHandlerConfiguration,
       AsyncClient<C> client,
-      Retryer retryer,
-      List<RequestInterceptor> requestInterceptors,
-      Logger logger,
-      Logger.Level logLevel,
-      MethodMetadata metadata,
-      RequestTemplate.Factory buildTemplateFromArgs,
-      Options options,
       AsyncResponseHandler asyncResponseHandler,
-      ExceptionPropagationPolicy propagationPolicy,
       C requestContext,
       MethodInfo methodInfo) {
-
-    this.target = checkNotNull(target, "target");
-    this.client = checkNotNull(client, "client for %s", target);
-    this.retryer = checkNotNull(retryer, "retryer for %s", target);
-    this.requestInterceptors =
-        checkNotNull(requestInterceptors, "requestInterceptors for %s", target);
-    this.logger = checkNotNull(logger, "logger for %s", target);
-    this.logLevel = checkNotNull(logLevel, "logLevel for %s", target);
-    this.metadata = checkNotNull(metadata, "metadata for %s", target);
-    this.buildTemplateFromArgs = checkNotNull(buildTemplateFromArgs, "metadata for %s", target);
-    this.options = checkNotNull(options, "options for %s", target);
-    this.propagationPolicy = propagationPolicy;
+    this.methodHandlerConfiguration =
+        checkNotNull(methodHandlerConfiguration, "methodHandlerConfiguration");
+    this.client = checkNotNull(client, "client for %s", methodHandlerConfiguration.getTarget());
     this.requestContext = requestContext;
     this.asyncResponseHandler = asyncResponseHandler;
     this.methodInfo = methodInfo;
   }
 
   @Override
   public Object invoke(Object[] argv) throws Throwable {
-    RequestTemplate template = buildTemplateFromArgs.create(argv);
+    RequestTemplate template = methodHandlerConfiguration.getBuildTemplateFromArgs().create(argv);
     Options options = findOptions(argv);
-    Retryer retryer = this.retryer.clone();
+    Retryer retryer = this.methodHandlerConfiguration.getRetryer().clone();
     try {
       if (methodInfo.isAsyncReturnType()) {
         return executeAndDecode(template, options, retryer);
@@ -102,8 +75,12 @@ private CompletableFuture<Object> executeAndDecode(
             (response, throwable) -> {
               if (throwable != null) {
                 if (!resultFuture.isDone() && shouldRetry(retryer, throwable, resultFuture)) {
-                  if (logLevel != Logger.Level.NONE) {
-                    logger.logRetry(metadata.configKey(), logLevel);
+                  if (methodHandlerConfiguration.getLogLevel() != Logger.Level.NONE) {
+                    methodHandlerConfiguration
+                        .getLogger()
+                        .logRetry(
+                            methodHandlerConfiguration.getMetadata().configKey(),
+                            methodHandlerConfiguration.getLogLevel());
                   }
 
                   resultFuture.setInner(executeAndDecode(template, options, retryer));
@@ -166,7 +143,7 @@ private boolean shouldRetry(
       return true;
     } catch (RetryableException th) {
       Throwable cause = th.getCause();
-      if (propagationPolicy == UNWRAP && cause != null) {
+      if (methodHandlerConfiguration.getPropagationPolicy() == UNWRAP && cause != null) {
         resultFuture.completeExceptionally(cause);
       } else {
         resultFuture.completeExceptionally(th);
@@ -178,8 +155,13 @@ private boolean shouldRetry(
   private CompletableFuture<Object> executeAndDecode(RequestTemplate template, Options options) {
     Request request = targetRequest(template);
 
-    if (logLevel != Logger.Level.NONE) {
-      logger.logRequest(metadata.configKey(), logLevel, request);
+    if (methodHandlerConfiguration.getLogLevel() != Logger.Level.NONE) {
+      methodHandlerConfiguration
+          .getLogger()
+          .logRequest(
+              methodHandlerConfiguration.getMetadata().configKey(),
+              methodHandlerConfiguration.getLogLevel(),
+              request);
     }
 
     long start = System.nanoTime();
@@ -197,9 +179,14 @@ private CompletableFuture<Object> executeAndDecode(RequestTemplate template, Opt
                       : new CompletionException(throwable);
               if (completionException.getCause() instanceof IOException) {
                 IOException ioException = (IOException) completionException.getCause();
-                if (logLevel != Logger.Level.NONE) {
-                  logger.logIOException(
-                      metadata.configKey(), logLevel, ioException, elapsedTime(start));
+                if (methodHandlerConfiguration.getLogLevel() != Logger.Level.NONE) {
+                  methodHandlerConfiguration
+                      .getLogger()
+                      .logIOException(
+                          methodHandlerConfiguration.getMetadata().configKey(),
+                          methodHandlerConfiguration.getLogLevel(),
+                          ioException,
+                          elapsedTime(start));
                 }
 
                 throw errorExecuting(request, ioException);
@@ -217,29 +204,30 @@ private static Response ensureRequestIsSet(
 
   private CompletableFuture<Object> handleResponse(Response response, long elapsedTime) {
     return asyncResponseHandler.handleResponse(
-        metadata.configKey(), response, methodInfo.underlyingReturnType(), elapsedTime);
+        methodHandlerConfiguration.getMetadata().configKey(), response,
+        methodInfo.underlyingReturnType(), elapsedTime);
   }
 
   private long elapsedTime(long start) {
     return TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
   }
 
   private Request targetRequest(RequestTemplate template) {
-    for (RequestInterceptor interceptor : requestInterceptors) {
+    for (RequestInterceptor interceptor : methodHandlerConfiguration.getRequestInterceptors()) {
       interceptor.apply(template);
     }
-    return target.apply(template);
+    return methodHandlerConfiguration.getTarget().apply(template);
   }
 
   private Options findOptions(Object[] argv) {
     if (argv == null || argv.length == 0) {
-      return this.options;
+      return this.methodHandlerConfiguration.getOptions();
     }
     return Stream.of(argv)
         .filter(Options.class::isInstance)
         .map(Options.class::cast)
         .findFirst()
-        .orElse(this.options);
+        .orElse(this.methodHandlerConfiguration.getOptions());
   }
 
   static class Factory<C> implements MethodHandler.Factory<C> {
@@ -254,8 +242,6 @@ static class Factory<C> implements MethodHandler.Factory<C> {
     private final MethodInfoResolver methodInfoResolver;
     private final RequestTemplateFactoryResolver requestTemplateFactoryResolver;
     private final Options options;
-    private final Decoder decoder;
-    private final ErrorDecoder errorDecoder;
 
     Factory(
         AsyncClient<C> client,
@@ -267,9 +253,7 @@ static class Factory<C> implements MethodHandler.Factory<C> {
         ExceptionPropagationPolicy propagationPolicy,
         MethodInfoResolver methodInfoResolver,
         RequestTemplateFactoryResolver requestTemplateFactoryResolver,
-        Options options,
-        Decoder decoder,
-        ErrorDecoder errorDecoder) {
+        Options options) {
       this.client = checkNotNull(client, "client");
       this.retryer = checkNotNull(retryer, "retryer");
       this.requestInterceptors = checkNotNull(requestInterceptors, "requestInterceptors");
@@ -281,27 +265,30 @@ static class Factory<C> implements MethodHandler.Factory<C> {
       this.requestTemplateFactoryResolver =
           checkNotNull(requestTemplateFactoryResolver, "requestTemplateFactoryResolver");
       this.options = checkNotNull(options, "options");
-      this.errorDecoder = checkNotNull(errorDecoder, "errorDecoder");
-      this.decoder = checkNotNull(decoder, "decoder");
     }
 
-    public MethodHandler create(Target<?> target, MethodMetadata md, C requestContext) {
+    @Override
+    public MethodHandler create(Target<?> target, MethodMetadata metadata, C requestContext) {
       final RequestTemplate.Factory buildTemplateFromArgs =
-          requestTemplateFactoryResolver.resolve(target, md);
+          requestTemplateFactoryResolver.resolve(target, metadata);
+
+      MethodHandlerConfiguration methodHandlerConfiguration =
+          new MethodHandlerConfiguration(
+              metadata,
+              target,
+              retryer,
+              requestInterceptors,
+              logger,
+              logLevel,
+              buildTemplateFromArgs,
+              options,
+              propagationPolicy);
       return new AsynchronousMethodHandler<C>(
-          target,
+          methodHandlerConfiguration,
           client,
-          retryer,
-          requestInterceptors,
-          logger,
-          logLevel,
-          md,
-          buildTemplateFromArgs,
-          options,
           responseHandler,
-          propagationPolicy,
           requestContext,
-          methodInfoResolver.resolve(target.type(), md.method()));
+          methodInfoResolver.resolve(target.type(), metadata.method()));
     }
   }
 }