don't duplicate apikey=value in re-tries; mask api key in outboundApiLog urls

qqq-backend-module-api/src/main/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtil.java

@@ -734,20 +734,7 @@ public class BaseAPIActionUtil
          case API_KEY_HEADER -> request.setHeader("API-Key", backendMetaData.getApiKey());
          case API_TOKEN -> request.setHeader("Authorization", "Token " + backendMetaData.getApiKey());
          case OAUTH2 -> request.setHeader("Authorization", "Bearer " + getOAuth2Token());
-         case API_KEY_QUERY_PARAM ->
+         case API_KEY_QUERY_PARAM -> addApiKeyQueryParamToRequest(request);
-         {
-            try
-            {
-               String uri = request.getURI().toString();
-               uri += (uri.contains("?") ? "&" : "?");
-               uri += backendMetaData.getApiKeyQueryParamName() + "=" + backendMetaData.getApiKey();
-               request.setURI(new URI(uri));
-            }
-            catch(URISyntaxException e)
-            {
-               throw (new QException("Error setting authorization query parameter", e));
-            }
-         }
          case CUSTOM ->
          {
             handleCustomAuthorization(request);
@@ -758,6 +745,35 @@ public class BaseAPIActionUtil
 
 
 
+   /***************************************************************************
+    **
+    ***************************************************************************/
+   protected void addApiKeyQueryParamToRequest(HttpRequestBase request) throws QException
+   {
+      try
+      {
+         String uri = request.getURI().toString();
+         String pair = backendMetaData.getApiKeyQueryParamName() + "=" + backendMetaData.getApiKey();
+
+         ///////////////////////////////////////////////////////////////////////////////////
+         // avoid re-adding the name=value pair if it's already there (e.g., for a retry) //
+         ///////////////////////////////////////////////////////////////////////////////////
+         if(!uri.contains(pair))
+         {
+            uri += (uri.contains("?") ? "&" : "?");
+            uri += pair;
+         }
+
+         request.setURI(new URI(uri));
+      }
+      catch(URISyntaxException e)
+      {
+         throw (new QException("Error setting authorization query parameter", e));
+      }
+   }
+
+
+
    /*******************************************************************************
     **
     *******************************************************************************/
@@ -1207,12 +1223,32 @@ public class BaseAPIActionUtil
             return;
          }
 
+         OutboundAPILog outboundAPILog = generateOutboundApiLogRecord(request, response);
+
+         InsertInput insertInput = new InsertInput();
+         insertInput.setTableName(table.getName());
+         insertInput.setRecords(List.of(outboundAPILog.toQRecord()));
+         new InsertAction().executeAsync(insertInput);
+      }
+      catch(Exception e)
+      {
+         LOG.warn("Error logging outbound api call", e);
+      }
+   }
+
+
+
+   /***************************************************************************
+    **
+    ***************************************************************************/
+   public OutboundAPILog generateOutboundApiLogRecord(HttpRequestBase request, QHttpResponse response)
+   {
       String requestBody = null;
       if(request instanceof HttpEntityEnclosingRequest entityRequest)
       {
          try
          {
-               requestBody = StringUtils.join("\n", IOUtils.readLines(entityRequest.getEntity().getContent()));
+            requestBody = StringUtils.join("\n", IOUtils.readLines(entityRequest.getEntity().getContent(), StandardCharsets.UTF_8));
          }
          catch(Exception e)
          {
@@ -1226,26 +1262,16 @@ public class BaseAPIActionUtil
       String url = request.getURI().toString();
       if(backendMetaData.getAuthorizationType().equals(AuthorizationType.API_KEY_QUERY_PARAM))
       {
-            url = url.replaceFirst(backendMetaData.getApiKey(), "******");
+         url = url.replaceAll(backendMetaData.getApiKey(), "******");
       }
 
-         InsertInput insertInput = new InsertInput();
+      return new OutboundAPILog()
-         insertInput.setTableName(table.getName());
-         insertInput.setRecords(List.of(new OutboundAPILog()
          .withMethod(request.getMethod())
          .withUrl(url)
          .withTimestamp(Instant.now())
          .withRequestBody(requestBody)
          .withStatusCode(response.getStatusCode())
-            .withResponseBody(response.getContent())
+         .withResponseBody(response.getContent());
-            .toQRecord()
-         ));
-         new InsertAction().executeAsync(insertInput);
-      }
-      catch(Exception e)
-      {
-         LOG.warn("Error logging outbound api call", e);
-      }
    }
 
 

qqq-backend-module-api/src/test/java/com/kingsrook/qqq/backend/module/api/actions/BaseAPIActionUtilTest.java

@@ -61,6 +61,7 @@ import com.kingsrook.qqq.backend.core.model.metadata.QInstance;
 import com.kingsrook.qqq.backend.core.model.metadata.tables.UniqueKey;
 import com.kingsrook.qqq.backend.core.utils.CollectionUtils;
 import com.kingsrook.qqq.backend.core.utils.SleepUtils;
+import com.kingsrook.qqq.backend.core.utils.lambdas.UnsafeConsumer;
 import com.kingsrook.qqq.backend.module.api.BaseTest;
 import com.kingsrook.qqq.backend.module.api.TestUtils;
 import com.kingsrook.qqq.backend.module.api.exceptions.RateLimitException;
@@ -74,6 +75,7 @@ import org.apache.http.Header;
 import org.apache.http.HttpEntity;
 import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
 import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpRequestBase;
 import org.json.JSONArray;
 import org.json.JSONObject;
 import org.junit.jupiter.api.BeforeEach;
@@ -868,6 +870,72 @@ class BaseAPIActionUtilTest extends BaseTest
 
 
 
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   @Test
+   void testAuthorizationApiKeyQueryParam() throws QException
+   {
+      APIBackendMetaData backend = (APIBackendMetaData) QContext.getQInstance().getBackend(TestUtils.MOCK_BACKEND_NAME);
+      backend.setAuthorizationType(AuthorizationType.API_KEY_QUERY_PARAM);
+      backend.setApiKeyQueryParamName("apikey");
+      backend.setApiKey("9876-WXYZ");
+
+      ////////////////////////////////////////////////////////////////////////////////////////////
+      // this will make it not use the mock makeRequest method,                                 //
+      // but instead the mock executeHttpRequest, so we can test code from the base makeRequest //
+      ////////////////////////////////////////////////////////////////////////////////////////////
+      mockApiUtilsHelper.setUseMock(false);
+
+      //////////////////////////////////////////////////////////////////////////////
+      // we'll want to assert that the URL has the api query string - and just    //
+      // one copy of it (as we once had a bug where it got duplicated upon retry) //
+      //////////////////////////////////////////////////////////////////////////////
+      UnsafeConsumer<HttpRequestBase, Exception> asserter = request -> assertThat(request.getURI().toString())
+         .contains("?apikey=9876-WXYZ")
+         .doesNotContain("?apikey=9876-WXYZ&apikey=9876-WXYZ");
+
+      ////////////////////////////////////////
+      // queue up a 429, so we'll try-again //
+      ////////////////////////////////////////
+      mockApiUtilsHelper.setMockRequestAsserter(asserter);
+      mockApiUtilsHelper.enqueueMockResponse(new QHttpResponse().withStatusCode(429).withContent(""));
+
+      //////////////////////
+      // queue a response //
+      //////////////////////
+      mockApiUtilsHelper.setMockRequestAsserter(asserter);
+      mockApiUtilsHelper.enqueueMockResponse("""
+         {"id": 3, "name": "Bart"},
+         """);
+
+      GetOutput getOutput = runSimpleGetAction();
+   }
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   @Test
+   void testGenerateOutboundApiLogRecord() throws QException
+   {
+      APIBackendMetaData backend = (APIBackendMetaData) QContext.getQInstance().getBackend(TestUtils.MOCK_BACKEND_NAME);
+      backend.setAuthorizationType(AuthorizationType.API_KEY_QUERY_PARAM);
+      backend.setApiKeyQueryParamName("apikey");
+      backend.setApiKey("9876-WXYZ");
+
+      MockApiActionUtils mockApiActionUtils = new MockApiActionUtils();
+      mockApiActionUtils.setBackendMetaData(backend);
+      OutboundAPILog outboundAPILog = mockApiActionUtils.generateOutboundApiLogRecord(new HttpGet("...?apikey=9876-WXYZ"), new QHttpResponse());
+
+      assertThat(outboundAPILog.getUrl())
+         .doesNotContain("9876-WXYZ")
+         .contains("?apikey=*****");
+   }
+
+
+
    /*******************************************************************************
     **
     *******************************************************************************/