Skip to content

Commit

Permalink
resolved comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sachin-Mamoru
Sachin-Mamoru committed Apr 15, 2024
1 parent dd0c083 commit 01b5e2f
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 101 deletions.
42 changes: 24 additions & 18 deletions ...n/java/org/wso2/carbon/identity/conditional/auth/functions/http/AbstractHTTPFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ public AbstractHTTPFunction() {
allowedDomains = ConfigProvider.getInstance().getAllowedDomainsForHttpFunctions();
}

public enum RetryDecision {
RETRY,
NO_RETRY;

public boolean shouldRetry() {
return this == RETRY;
}
}

protected void executeHttpMethod(HttpUriRequest clientRequest, Map<String, Object> eventHandlers,
AuthConfigModel authConfigModel) {

Expand All @@ -107,9 +116,9 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map<String, Objec
endpointURL);
asyncReturn.accept(context, Collections.emptyMap(), Constants.OUTCOME_FAIL);
} else {
Pair<Boolean, Pair<String, JSONObject>> result = executeRequest(request, endpointURL);
if (Boolean.TRUE.equals(result.getLeft())) {
LOG.error("Error while calling endpoint. Url: " + endpointURL);
Pair<RetryDecision, Pair<String, JSONObject>> result = executeRequest(request, endpointURL);
if (result.getLeft().shouldRetry()) {
LOG.info("Failed to invoke the endpoint. Url: " + endpointURL + ". Retrying the request.");
result = executeRequestWithRetries(request, endpointURL, requestRetryCount);
}
outcome = result.getRight().getLeft();
Expand All @@ -132,13 +141,13 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map<String, Objec
* @param maxRetries Maximum number of retries.
* @return Pair of outcome and json.
*/
private Pair<Boolean, Pair<String, JSONObject>> executeRequestWithRetries
private Pair<RetryDecision, Pair<String, JSONObject>> executeRequestWithRetries
(HttpUriRequest request, String endpointURL, int maxRetries) {

Pair<Boolean, Pair<String, JSONObject>> result;
Pair<RetryDecision, Pair<String, JSONObject>> result;
String outcome = Constants.OUTCOME_FAIL;
int attempts = 0;
boolean isRetry = false;
RetryDecision isRetry = RetryDecision.NO_RETRY;

while (attempts < maxRetries) {
attempts++;
Expand All @@ -156,7 +165,7 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map<String, Objec
}
result = executeRequest(request, endpointURL);
isRetry = result.getLeft();
if (!isRetry) {
if (!isRetry.shouldRetry()) {
return result;
}
}
Expand All @@ -170,11 +179,11 @@ protected void executeHttpMethod(HttpUriRequest clientRequest, Map<String, Objec
* @param endpointURL Endpoint URL.
* @return Pair of outcome and json.
*/
private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest request, String endpointURL) {
private Pair<RetryDecision, Pair<String, JSONObject>> executeRequest(HttpUriRequest request, String endpointURL) {

JSONObject json = null;
String outcome;
boolean isRetry = false;
RetryDecision isRetry = RetryDecision.NO_RETRY;

try (CloseableHttpResponse response = client.execute(request)) {
int responseCode = response.getStatusLine().getStatusCode();
Expand Down Expand Up @@ -203,7 +212,7 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
LOG.info("Successfully called the external api. Status code: " + responseCode + ". Url: " +
endpointURL);
outcome = Constants.OUTCOME_SUCCESS;
return Pair.of(false, Pair.of(outcome, json)); // Success, return immediately
return Pair.of(RetryDecision.NO_RETRY, Pair.of(outcome, json)); // Success, return immediately
} else if (responseCode >= 300 && responseCode < 400) {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new
Expand All @@ -219,7 +228,7 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
LOG.warn("External api invocation returned a redirection. Status code: " +
responseCode + ". Url: " + endpointURL);
outcome = Constants.OUTCOME_FAIL;
return Pair.of(false, Pair.of(outcome, null)); // Unauthorized, no retry
return Pair.of(RetryDecision.NO_RETRY, Pair.of(outcome, null)); // Unauthorized, no retry
} else if (responseCode >= 400 && responseCode < 500) {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new
Expand All @@ -235,7 +244,7 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
LOG.warn("External api invocation returned a client error. Status code: " +
responseCode + ". Url: " + endpointURL);
outcome = Constants.OUTCOME_FAIL;
return Pair.of(false, Pair.of(outcome, null)); // Unauthorized, no retry
return Pair.of(RetryDecision.NO_RETRY, Pair.of(outcome, null)); // Unauthorized, no retry
} else {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
DiagnosticLog.DiagnosticLogBuilder diagnosticLogBuilder = new
Expand All @@ -251,7 +260,7 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
LOG.error("Received unknown response from external API call. Status code: " +
responseCode + ". Url: " + endpointURL);
outcome = Constants.OUTCOME_FAIL;
return Pair.of(true, Pair.of(outcome, null)); // Server error, retry if attempts left
return Pair.of(RetryDecision.RETRY, Pair.of(outcome, null)); // Server error, retry if attempts left
}
} catch (Exception e) {
// Log the error based on its type
Expand Down Expand Up @@ -279,12 +288,11 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
.resultStatus(DiagnosticLog.ResultStatus.FAILED);
LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
}
isRetry = true; // Timeout, retry if attempts left
isRetry = RetryDecision.RETRY; // Timeout, retry if attempts left
outcome = Constants.OUTCOME_TIMEOUT;
LOG.error("Error while waiting to connect to " + endpointURL, e);
} else if (e instanceof IOException) {
isRetry = true; // Timeout, retry if attempts left
outcome = Constants.OUTCOME_TIMEOUT;
outcome = Constants.OUTCOME_FAIL;
LOG.error("Error while calling endpoint. ", e);
} else if (e instanceof ParseException) {
if (LoggerUtils.isDiagnosticLogsEnabled()) {
Expand All @@ -297,7 +305,6 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
.resultStatus(DiagnosticLog.ResultStatus.FAILED);
LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
}
isRetry = true; // Timeout, retry if attempts left
outcome = Constants.OUTCOME_FAIL;
LOG.error("Error while parsing response. ", e);
} else {
Expand All @@ -311,7 +318,6 @@ private Pair<Boolean, Pair<String, JSONObject>> executeRequest(HttpUriRequest re
.resultStatus(DiagnosticLog.ResultStatus.FAILED);
LoggerUtils.triggerDiagnosticLogEvent(diagnosticLogBuilder);
}
isRetry = true; // Timeout, retry if attempts left
outcome = Constants.OUTCOME_FAIL;
LOG.error("Error while calling endpoint. ", e);
}
Expand Down
8 changes: 4 additions & 4 deletions ...in/java/org/wso2/carbon/identity/conditional/auth/functions/http/HTTPGetFunctionImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ public void httpGet(String endpointURL, Object... params) {
authConfig = getAuthConfigModel((Map<String, Object>) params[1]);
eventHandlers = (Map<String, Object>) params[2];
} else {
throw new IllegalArgumentException("Invalid argument type. Expected payloadData " +
"(Map<String, Object>), headers (Map<String, String>), and eventHandlers " +
"(Map<String, Object>) respectively.");
throw new IllegalArgumentException("Invalid argument type. Expected " +
"headers (Map<String, String>), authConfig (Map<String, String>)," +
" and eventHandlers (Map<String, Object>) respectively.");
}
break;
default:
throw new IllegalArgumentException("Invalid number of arguments. Expected 1 or 2, but got: " +
throw new IllegalArgumentException("Invalid number of arguments. Expected 1, 2 or 3, but got: " +
params.length + ".");
}

Expand Down
6 changes: 3 additions & 3 deletions ...n/java/org/wso2/carbon/identity/conditional/auth/functions/http/HTTPPostFunctionImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ public void httpPost(String endpointURL, Object... params) {
eventHandlers = (Map<String, Object>) params[3];
} else {
throw new IllegalArgumentException("Invalid argument type. Expected payloadData " +
"(Map<String, Object>), headers (Map<String, String>), and eventHandlers " +
"(Map<String, Object>) respectively.");
"(Map<String, Object>), headers (Map<String, String>), authConfig (Map<String, String>)," +
" and eventHandlers (Map<String, Object>) respectively.");
}
break;
default:
throw new IllegalArgumentException("Invalid number of arguments. Expected 1, 2, or 3. Found: "
throw new IllegalArgumentException("Invalid number of arguments. Expected 1, 2, 3, or 4. Found: "
+ params.length + ".");
}

Expand Down
Loading

0 comments on commit 01b5e2f

Please sign in to comment.