From 65884a9168561c7c6c3d01937178a0c54a2dda40 Mon Sep 17 00:00:00 2001
From: Eduard Heimbuch <eduard.heimbuch@cloudogu.com>
Date: Tue, 17 Nov 2020 12:51:11 +0100
Subject: [PATCH] Enhance trace api to set codes which are accepted as
 successful. This way requests can be traced as successful even if the
 response code is 4xx or 5xx.

---
 .../sonia/scm/net/ahc/BaseHttpRequest.java    | 31 +++++++++++++++++++
 .../net/ahc/DefaultAdvancedHttpClient.java    | 10 +++++-
 .../ahc/DefaultAdvancedHttpClientTest.java    | 13 +++++++-
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java b/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java
index 302f2ac6fe..5b0ebc453c 100644
--- a/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java
+++ b/scm-core/src/main/java/sonia/scm/net/ahc/BaseHttpRequest.java
@@ -257,6 +257,22 @@ public abstract class BaseHttpRequest<T extends BaseHttpRequest>
     return self();
   }
 
+  /**
+   * Sets the response codes which should be traced as successful.
+   *
+   * Example: If 400 is set as {@link #acceptedStatusCodes} then all requests
+   * which get a response with status code 400 will be traced as successful (not failed) request
+   *
+   * @param codes status codes which should be traced as successful
+   * @return request instance
+   *
+   * @since 2.10.0
+   */
+  public T acceptStatusCodes(int... codes) {
+    this.acceptedStatusCodes = codes;
+    return self();
+  }
+
   /**
    * Disables tracing for the request.
    * This should only be done for internal requests.
@@ -314,6 +330,18 @@ public abstract class BaseHttpRequest<T extends BaseHttpRequest>
     return spanKind;
   }
 
+
+  /**
+   * Returns the response codes which are accepted as successful by tracer.
+   *
+   * @return codes
+   *
+   * @since 2.10.0
+   */
+  public int[] getAcceptedStatus() {
+    return acceptedStatusCodes;
+  }
+
   /**
    * Returns true if the request decodes gzip compression.
    *
@@ -434,4 +462,7 @@ public abstract class BaseHttpRequest<T extends BaseHttpRequest>
 
   /** kind of span for trace api */
   private String spanKind = "HTTP Request";
+
+  /** codes which will be marked as successful by tracer */
+  private int[] acceptedStatusCodes = new int[]{};
 }
diff --git a/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java b/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java
index ee3bdbb1e5..75acfc97d2 100644
--- a/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java
+++ b/scm-webapp/src/main/java/sonia/scm/net/ahc/DefaultAdvancedHttpClient.java
@@ -52,6 +52,7 @@ import java.io.OutputStream;
 import java.net.*;
 import java.security.KeyManagementException;
 import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import java.util.Set;
 
 //~--- JDK imports ------------------------------------------------------------
@@ -206,7 +207,7 @@ public class DefaultAdvancedHttpClient extends AdvancedHttpClient
       try {
         DefaultAdvancedHttpResponse response = doRequest(request);
         span.label("status", response.getStatus());
-        if (!response.isSuccessful()) {
+        if (isFailedRequest(request, response)) {
           span.failed();
         }
         return response;
@@ -219,6 +220,13 @@ public class DefaultAdvancedHttpClient extends AdvancedHttpClient
     }
   }
 
+  private boolean isFailedRequest(BaseHttpRequest<?> request, AdvancedHttpResponse responseStatus) {
+    if (Arrays.stream(request.getAcceptedStatus()).anyMatch(code -> code == responseStatus.getStatus())) {
+      return false;
+    }
+     return !responseStatus.isSuccessful();
+  }
+
   @Nonnull
   private DefaultAdvancedHttpResponse doRequest(BaseHttpRequest<?> request) throws IOException {
     HttpURLConnection connection = openConnection(request, new URL(request.getUrl()));
diff --git a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java
index 89004ece27..f1f09521b7 100644
--- a/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java
+++ b/scm-webapp/src/test/java/sonia/scm/net/ahc/DefaultAdvancedHttpClientTest.java
@@ -317,6 +317,17 @@ public class DefaultAdvancedHttpClientTest
     verify(tracer, never()).span(anyString());
   }
 
+  @Test
+  public void shouldNotTraceRequestIfUntracedResponseCode() throws IOException {
+    when(connection.getResponseCode()).thenReturn(400);
+
+    new AdvancedHttpRequest(client, HttpMethod.GET, "https://www.scm-manager.org").acceptStatusCodes(400).request();
+    verify(tracer).span("HTTP Request");
+    verify(span).label("status", 400);
+    verify(span, never()).failed();
+    verify(span).close();
+  }
+
 
   //~--- set methods ----------------------------------------------------------
 
@@ -328,7 +339,7 @@ public class DefaultAdvancedHttpClientTest
   public void setUp()
   {
     configuration = new ScmConfiguration();
-    transformers = new HashSet<ContentTransformer>();
+    transformers = new HashSet<>();
     client = new TestingAdvacedHttpClient(configuration, transformers);
     when(tracer.span(anyString())).thenReturn(span);
   }