Fixing log management in RetryRequestExecutor

Tirasa · Jun 14, 2021 · 36113d4 · 36113d4
1 parent 3393a03
commit 36113d4
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 15 deletions.
diff --git a/connector/src/main/java/com/okta/commons/http/RetryRequestExecutor.java b/connector/src/main/java/com/okta/commons/http/RetryRequestExecutor.java
@@ -61,7 +61,6 @@ public RetryRequestExecutor(HttpClientConfiguration clientConfiguration, Request
 
     @Override
     public Response executeRequest(Request request) throws HttpException {
-
         Assert.notNull(request, "Request argument cannot be null.");
 
         int retryCount = 0;
@@ -78,9 +77,7 @@ public Response executeRequest(Request request) throws HttpException {
         originalHeaders.putAll(request.getHeaders());
 
         while (true) {
-
             try {
-
                 if (retryCount > 0) {
                     request.setQueryString(originalQuery);
                     request.setHeaders(originalHeaders);
@@ -99,11 +96,7 @@ public Response executeRequest(Request request) throws HttpException {
                         // if we cannot pause, then return the original response
                         pauseBeforeRetry(retryCount, response, timer.split());
                     } catch (HttpException e) {
-                        if (LOG.isOk()) {
-                            LOG.ok("Unable to pause for retry: {}", e.getMessage(), e);
-                        } else {
-                            LOG.warn("Unable to pause for retry: {}", e.getMessage());
-                        }
+                        LOG.warn(e, "Unable to pause for retry: {0}", e.getMessage());
 
                         // First attempt failed, and we were not able to retry
                         if (response == null) {
@@ -130,7 +123,7 @@ public Response executeRequest(Request request) throws HttpException {
                 if (!shouldRetry(retryCount, timer.split())) {
                     throw new HttpException("Unable to execute HTTP request: " + e.getMessage(), e);
                 }
-                LOG.ok("Retrying on {}: {}", e.getClass().getName(), e.getMessage());
+                LOG.ok("Retrying on {0}: {1}", e.getClass().getName(), e.getMessage());
             } catch (HttpException e) {
                 // exceptions from delegate marked as retrHttpClientRequestExecutoryable
                 if (!e.isRetryable() || !shouldRetry(retryCount, timer.split())) {
@@ -168,7 +161,7 @@ private void pauseBeforeRetry(int retries, Response response, long timeElapsed)
             if (!shouldRetry(retries, timeElapsed + delay)) {
                 throw failedToRetry();
             }
-            LOG.ok("429 detected, will retry in {}ms, attempt number: {}", delay, retries);
+            LOG.ok("429 detected, will retry in {0}ms, attempt number: {1}", delay, retries);
         }
 
         // default / fallback strategy (backwards compatible implementation)
@@ -181,7 +174,7 @@ private void pauseBeforeRetry(int retries, Response response, long timeElapsed)
             throw failedToRetry();
         }
 
-        LOG.ok("Retryable condition detected, will retry in {}ms, attempt number: {}", delay, retries);
+        LOG.ok("Retryable condition detected, will retry in {0}ms, attempt number: {1}", delay, retries);
 
         try {
             Thread.sleep(delay);
@@ -208,7 +201,7 @@ private long get429DelayMillis(Response response) {
         long waitUntil = resetLimit * 1000L;
         long requestTime = requestDate.getTime();
         long delay = Math.max(waitUntil - requestTime + 1000, 1000);
-        LOG.ok("429 wait: Math.max({} - {} + 1s), 1s = {})", waitUntil, requestTime, delay);
+        LOG.ok("429 wait: Math.max({0} - {1} + 1s), 1s = {2})", waitUntil, requestTime, delay);
 
         return delay;
     }

diff --git a/connector/src/main/java/net/tirasa/connid/bundles/okta/OktaConnector.java b/connector/src/main/java/net/tirasa/connid/bundles/okta/OktaConnector.java
@@ -19,6 +19,7 @@
 import com.okta.sdk.client.Client;
 import com.okta.sdk.client.Clients;
 import com.okta.sdk.impl.resource.AbstractCollectionResource;
+import com.okta.sdk.impl.resource.DefaultUserBuilder;
 import com.okta.sdk.impl.resource.application.DefaultApplicationList;
 import com.okta.sdk.impl.resource.group.DefaultGroupList;
 import com.okta.sdk.impl.resource.user.DefaultUserList;
@@ -206,7 +207,7 @@ public Uid create(
             Attribute status = accessor.find(OperationalAttributes.ENABLE_NAME);
             Attribute email = accessor.find(OktaAttribute.EMAIL);
             try {
-                UserBuilder userBuilder = UserBuilder.instance();
+                UserBuilder userBuilder = new DefaultUserBuilder();
 
                 if (status == null || CollectionUtil.isEmpty(status.getValue())) {
                     LOG.warn("{0} attribute value not correct or not found, won't handle User status",
@@ -971,14 +972,14 @@ private void selfPasswordUpdate(final User user, final GuardedString oldPassword
                             setValue(SecurityUtil.decrypt(newPassword).toCharArray())));
             LOG.ok("Self change passsowrd user {0}" + user.getId());
         } catch (ResourceException e) {
-            LOG.error(e.getMessage(), e);
+            LOG.error(e, e.getMessage());
             if (!CollectionUtil.isEmpty(e.getCauses())) {
                 OktaUtils.handleGeneralError(e.getError().getCauses().get(0).getSummary());
             } else {
                 OktaUtils.handleGeneralError(e.getMessage(), e);
             }
         } catch (Exception e) {
-            LOG.error(e.getMessage(), e);
+            LOG.error(e, e.getMessage());
             OktaUtils.handleGeneralError(e.getMessage(), e);
         }
     }