From dd0c0830ab854ed1533c10099ec4bb705ddeb218 Mon Sep 17 00:00:00 2001 From: Sachin-Mamoru Date: Sat, 13 Apr 2024 21:01:10 +0530 Subject: [PATCH] resolved comments --- .../functions/http/AbstractHTTPFunction.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/components/org.wso2.carbon.identity.conditional.auth.functions.http/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/http/AbstractHTTPFunction.java b/components/org.wso2.carbon.identity.conditional.auth.functions.http/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/http/AbstractHTTPFunction.java index 38f86260..b8830058 100644 --- a/components/org.wso2.carbon.identity.conditional.auth.functions.http/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/http/AbstractHTTPFunction.java +++ b/components/org.wso2.carbon.identity.conditional.auth.functions.http/src/main/java/org/wso2/carbon/identity/conditional/auth/functions/http/AbstractHTTPFunction.java @@ -62,9 +62,6 @@ public abstract class AbstractHTTPFunction { protected static final String TYPE_TEXT_PLAIN = "text/plain"; private static final char DOMAIN_SEPARATOR = '.'; private static final String RESPONSE = "response"; - private static final int HTTP_STATUS_REQUEST_TIMEOUT = 408; - private static final int HTTP_STATUS_INTERNAL_SERVER_ERROR = 500; - private static final int HTTP_STATUS_BAD_REQUEST = 400; private final int requestRetryCount; private final List allowedDomains; @@ -110,8 +107,8 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map> result = executeRequest(request, endpointURL); - if (result.getLeft() >= 400 && result.getLeft() <= 500) { + Pair> result = executeRequest(request, endpointURL); + if (Boolean.TRUE.equals(result.getLeft())) { LOG.error("Error while calling endpoint. Url: " + endpointURL); result = executeRequestWithRetries(request, endpointURL, requestRetryCount); } @@ -135,13 +132,13 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map> executeRequestWithRetries + private Pair> executeRequestWithRetries (HttpUriRequest request, String endpointURL, int maxRetries) { - Pair> result = Pair.of(0, Pair.of(Constants.OUTCOME_FAIL, null)); - JSONObject json = null; + Pair> result; String outcome = Constants.OUTCOME_FAIL; int attempts = 0; + boolean isRetry = false; while (attempts < maxRetries) { attempts++; @@ -158,11 +155,12 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map= 200 && result.getLeft() < 400) { + isRetry = result.getLeft(); + if (!isRetry) { return result; } } - return Pair.of(result.getLeft(), Pair.of(outcome, json)); + return Pair.of(isRetry, Pair.of(outcome, null)); } /** @@ -172,11 +170,11 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map> executeRequest(HttpUriRequest request, String endpointURL) { + private Pair> executeRequest(HttpUriRequest request, String endpointURL) { JSONObject json = null; String outcome; - int statuscode = 500; + boolean isRetry = false; try (CloseableHttpResponse response = client.execute(request)) { int responseCode = response.getStatusLine().getStatusCode(); @@ -205,7 +203,7 @@ private Pair> executeRequest(HttpUriRequest re LOG.info("Successfully called the external api. Status code: " + responseCode + ". Url: " + endpointURL); outcome = Constants.OUTCOME_SUCCESS; - return Pair.of(responseCode, Pair.of(outcome, json)); // Success, return immediately + return Pair.of(false, Pair.of(outcome, json)); // Success, return immediately } else if (responseCode >= 300 && responseCode < 400) { if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new @@ -221,7 +219,7 @@ private Pair> executeRequest(HttpUriRequest re LOG.warn("External api invocation returned a redirection. Status code: " + responseCode + ". Url: " + endpointURL); outcome = Constants.OUTCOME_FAIL; - return Pair.of(responseCode, Pair.of(outcome, null)); // Unauthorized, no retry + return Pair.of(false, Pair.of(outcome, null)); // Unauthorized, no retry } else if (responseCode >= 400 && responseCode < 500) { if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new @@ -237,7 +235,7 @@ private Pair> executeRequest(HttpUriRequest re LOG.warn("External api invocation returned a client error. Status code: " + responseCode + ". Url: " + endpointURL); outcome = Constants.OUTCOME_FAIL; - return Pair.of(responseCode, Pair.of(outcome, null)); // Unauthorized, no retry + return Pair.of(false, Pair.of(outcome, null)); // Unauthorized, no retry } else { if (LoggerUtils.isDiagnosticLogsEnabled()) { DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new @@ -253,7 +251,7 @@ private Pair> executeRequest(HttpUriRequest re LOG.error("Received unknown response from external API call. Status code: " + responseCode + ". Url: " + endpointURL); outcome = Constants.OUTCOME_FAIL; - return Pair.of(responseCode, Pair.of(outcome, null)); // Server error, retry if attempts left + return Pair.of(true, Pair.of(outcome, null)); // Server error, retry if attempts left } } catch (Exception e) { // Log the error based on its type @@ -268,7 +266,6 @@ private Pair> executeRequest(HttpUriRequest re .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - statuscode = HTTP_STATUS_BAD_REQUEST; // Client error, retry if attempts left outcome = Constants.OUTCOME_FAIL; LOG.error("Invalid Url: " + endpointURL, e); } else if (e instanceof ConnectTimeoutException || e instanceof SocketTimeoutException) { @@ -277,15 +274,17 @@ private Pair> executeRequest(HttpUriRequest re DiagnosticLog.DiagnosticLogBuilder(Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, Constants.LogConstants.ActionIDs.RECEIVE_API_RESPONSE); diagnosticLogBuilder.inputParam(Constants.LogConstants.InputKeys.API, endpointURL) - .resultMessage("Creating a connection for the external API call timed out.") + .resultMessage("Request for the external API timed out.") .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - outcome = Constants.OUTCOME_TIMEOUT; // Timeout, retry if attempts left + isRetry = true; // Timeout, retry if attempts left + outcome = Constants.OUTCOME_TIMEOUT; LOG.error("Error while waiting to connect to " + endpointURL, e); } else if (e instanceof IOException) { - outcome = Constants.OUTCOME_TIMEOUT; // Timeout, retry if attempts left + isRetry = true; // Timeout, retry if attempts left + outcome = Constants.OUTCOME_TIMEOUT; LOG.error("Error while calling endpoint. ", e); } else if (e instanceof ParseException) { if (LoggerUtils.isDiagnosticLogsEnabled()) { @@ -298,7 +297,8 @@ private Pair> executeRequest(HttpUriRequest re .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - outcome = Constants.OUTCOME_FAIL; // Server error, retry if attempts left + isRetry = true; // Timeout, retry if attempts left + outcome = Constants.OUTCOME_FAIL; LOG.error("Error while parsing response. ", e); } else { if (LoggerUtils.isDiagnosticLogsEnabled()) { @@ -306,17 +306,18 @@ private Pair> executeRequest(HttpUriRequest re DiagnosticLog.DiagnosticLogBuilder(Constants.LogConstants.ADAPTIVE_AUTH_SERVICE, Constants.LogConstants.ActionIDs.RECEIVE_API_RESPONSE); diagnosticLogBuilder.inputParam(Constants.LogConstants.InputKeys.API, endpointURL) - .resultMessage("Received unknown exception from external API call.") + .resultMessage("Received an error while invoking the external API.") .logDetailLevel(DiagnosticLog.LogDetailLevel.APPLICATION) .resultStatus(DiagnosticLog.ResultStatus.FAILED); LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder); } - outcome = Constants.OUTCOME_FAIL; // Server error, retry if attempts left + isRetry = true; // Timeout, retry if attempts left + outcome = Constants.OUTCOME_FAIL; LOG.error("Error while calling endpoint. ", e); } } // Return the outcome and json (which might be null if never successful) - return Pair.of(statuscode, Pair.of(outcome, json)); + return Pair.of(isRetry, Pair.of(outcome, json)); } private boolean isValidRequestDomain(URI url) {