Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add timeout server configurations to action execution #5935

Merged
merged 10 commits into from
Sep 19, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ public APIClient() {

// todo: read connection configurations related to the http client of actions from the server configuration.
// Initialize the http client. Set connection time out to 2s and read time out to 5s.
int readTimeout = 5000;
int connectionRequestTimeout = 2000;
int connectionTimeout = 2000;
int readTimeout = ActionExecutorConfig.getInstance().getHttpReadTimeoutInMillis();
int connectionRequestTimeout = ActionExecutorConfig.getInstance().getHttpConnectionRequestTimeoutInMillis();
int connectionTimeout = ActionExecutorConfig.getInstance().getHttpConnectionTimeoutInMillis();

RequestConfig config = RequestConfig.custom()
.setConnectTimeout(connectionTimeout)
Expand All @@ -66,7 +66,7 @@ public APIClient() {
.setRelativeRedirectsAllowed(false)
.build();
PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
connectionManager.setMaxTotal(20);
connectionManager.setMaxTotal(ActionExecutorConfig.getInstance().getHttpConnectionPoolSize());
httpClient = HttpClientBuilder.create().setDefaultRequestConfig(config).setConnectionManager(connectionManager)
.build();
}
Expand Down Expand Up @@ -94,7 +94,7 @@ private void setRequestEntity(HttpPost httpPost, String jsonRequest, AuthMethods
private ActionInvocationResponse executeRequest(HttpPost request) {

int attempts = 0;
int retryCount = 2; // todo: read from server configurations
int retryCount = ActionExecutorConfig.getInstance().getHttpRequestRetryCount();
ActionInvocationResponse actionInvocationResponse = null;

while (attempts < retryCount) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ public class ActionExecutorConfig {
"Actions.ActionRequest.ExcludedHeaders.Header";
private static final String EXCLUDED_PARAMS_IN_ACTION_REQUEST_PROPERTY =
"Actions.ActionRequest.ExcludedParameters.Parameter";
private static final String HTTP_READ_TIMEOUT_PROPERTY = "Actions.HTTPClient.HTTPReadTimeout";
private static final String HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY =
"Actions.HTTPClient.HTTPConnectionRequestTimeout";
private static final String HTTP_CONNECTION_TIMEOUT_PROPERTY = "Actions.HTTPClient.HTTPConnectionTimeout";
private static final String HTTP_CONNECTION_POOL_SIZE_PROPERTY = "Actions.HTTPClient.HTTPConnectionPoolSize";
private static final String HTTP_REQUEST_RETRY_COUNT_PROPERTY = "Actions.HTTPClient.HTTPRequestRetryCount";
private static final int DEFAULT_HTTP_REQUEST_RETRY_COUNT = 2;
private static final int DEFAULT_HTTP_CONNECTION_POOL_SIZE = 20;
private static final int DEFAULT_HTTP_READ_TIMEOUT_IN_MILLIS = 5000;
private static final int DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT_IN_MILLIS = 2000;
private static final int DEFAULT_HTTP_CONNECTION_TIMEOUT_IN_MILLIS = 2000;

private ActionExecutorConfig() {

Expand Down Expand Up @@ -73,6 +84,99 @@ public boolean isExecutionForActionTypeEnabled(ActionType actionType) {
}
}

/**
* Returns the HTTP request retry count based on the system configuration.
*
* @return The HTTP request retry count, or the default if the property is missing or invalid.
*/
public int getHttpRequestRetryCount() {

int retryCountPropertyValue = DEFAULT_HTTP_REQUEST_RETRY_COUNT;
String retryCountValue = (String) IdentityConfigParser.getInstance().getConfiguration().
get(HTTP_REQUEST_RETRY_COUNT_PROPERTY);
if (StringUtils.isNotBlank(retryCountValue)) {
try {
retryCountPropertyValue = Integer.parseInt(retryCountValue);
} catch (NumberFormatException e) {
LOG.debug("Failed to read Http request retry count property in identity.xml." +
" Expects a number. Using the default value: " +
DEFAULT_HTTP_REQUEST_RETRY_COUNT, e);
}
}
return retryCountPropertyValue;
}

/**
* Returns the HTTP connection pool size based on the system configuration.
*
* @return The HTTP connection pool size, or the default if the property is missing or invalid.
*/
public int getHttpConnectionPoolSize() {

int poolSizePropertyValue = DEFAULT_HTTP_CONNECTION_POOL_SIZE;
String poolSizeValue = (String) IdentityConfigParser.getInstance().getConfiguration().
get(HTTP_CONNECTION_POOL_SIZE_PROPERTY);
if (StringUtils.isNotBlank(poolSizeValue)) {
try {
poolSizePropertyValue = Integer.parseInt(poolSizeValue);
} catch (NumberFormatException e) {
LOG.debug("Failed to read Http client connection pool size property in identity.xml." +
" Expects a number. Using the default value: " +
DEFAULT_HTTP_CONNECTION_POOL_SIZE, e);
}
}
return poolSizePropertyValue;
}

/**
* Retrieves the HTTP read timeout configuration.
* If the configuration value is invalid or missing, the default timeout value is parsed.
*
* @return The HTTP read timeout int value in milliseconds.
*/
public int getHttpReadTimeoutInMillis() {

return parseTimeoutConfig(HTTP_READ_TIMEOUT_PROPERTY, DEFAULT_HTTP_READ_TIMEOUT_IN_MILLIS);
}

/**
* Retrieves the HTTP connection request timeout configuration.
* If the configuration value is invalid or missing, the default timeout value is parsed.
*
* @return The HTTP connection request timeout int value in milliseconds.
*/
public int getHttpConnectionRequestTimeoutInMillis() {

return parseTimeoutConfig(HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY,
DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT_IN_MILLIS);
}

/**
* Retrieves the HTTP connection timeout configuration.
* If the configuration value is invalid or missing, the default timeout value is parsed.
*
* @return The HTTP connection timeout int value in milliseconds.
*/
public int getHttpConnectionTimeoutInMillis() {

return parseTimeoutConfig(HTTP_CONNECTION_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_TIMEOUT_IN_MILLIS);
}

private int parseTimeoutConfig(String timeoutTypeName, int defaultTimeout) {

int timeoutPropertyValue = defaultTimeout;
String timeoutValue = (String) IdentityConfigParser.getInstance().getConfiguration().get(timeoutTypeName);
if (StringUtils.isNotBlank(timeoutValue)) {
try {
timeoutPropertyValue = Integer.parseInt(timeoutValue);
} catch (NumberFormatException e) {
LOG.debug("Failed to read " + timeoutTypeName + " property in identity.xml." +
" Expects a number. Using the default value: " + defaultTimeout, e);
}
}
return timeoutPropertyValue;
}

private boolean isActionTypeEnabled(String actionTypePropertyName) {

boolean isActionTypeEnabled = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.wso2.carbon.identity.action.execution.model.User;
import org.wso2.carbon.identity.action.execution.model.UserStore;
import org.wso2.carbon.identity.action.execution.util.APIClient;
import org.wso2.carbon.identity.action.execution.util.ActionExecutorConfig;
import org.wso2.carbon.identity.action.execution.util.RequestFilter;
import org.wso2.carbon.identity.action.management.ActionManagementService;
import org.wso2.carbon.identity.action.management.exception.ActionMgtException;
Expand Down Expand Up @@ -84,6 +85,7 @@ public class ActionExecutorServiceImplTest {
private APIClient apiClient;
@InjectMocks
private ActionExecutorServiceImpl actionExecutorService;
private MockedStatic<ActionExecutorConfig> actionExecutorConfigStatic;

private MockedStatic<RequestFilter> requestFilter;
private MockedStatic<ActionExecutionRequestBuilderFactory> actionExecutionRequestBuilderFactory;
Expand All @@ -92,6 +94,9 @@ public class ActionExecutorServiceImplTest {
@BeforeMethod
public void setUp() throws Exception {

actionExecutorConfigStatic = mockStatic(ActionExecutorConfig.class);
ActionExecutorConfig actionExecutorConfig = mock(ActionExecutorConfig.class);
actionExecutorConfigStatic.when(ActionExecutorConfig::getInstance).thenReturn(actionExecutorConfig);
MockitoAnnotations.openMocks(this);
ActionExecutionServiceComponentHolder actionExecutionServiceComponentHolder =
ActionExecutionServiceComponentHolder.getInstance();
Expand All @@ -110,6 +115,7 @@ public void tearDown() {
requestFilter.close();
actionExecutionRequestBuilderFactory.close();
actionExecutionResponseProcessorFactory.close();
actionExecutorConfigStatic.close();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
import org.apache.http.impl.client.CloseableHttpClient;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockitoAnnotations;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
Expand All @@ -46,6 +48,8 @@
import java.util.HashMap;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -66,16 +70,28 @@ public class APIClientTest {
@Mock
private StatusLine statusLine;

private MockedStatic<ActionExecutorConfig> actionExecutorConfigStatic;

@InjectMocks
private APIClient apiClient;

@BeforeMethod
public void setUp() throws Exception {

actionExecutorConfigStatic = mockStatic(ActionExecutorConfig.class);
ActionExecutorConfig actionExecutorConfig = mock(ActionExecutorConfig.class);
actionExecutorConfigStatic.when(ActionExecutorConfig::getInstance).thenReturn(actionExecutorConfig);
MockitoAnnotations.openMocks(this);
when(actionExecutorConfig.getHttpRequestRetryCount()).thenReturn(2);
setField(apiClient, "httpClient", httpClient);
}

@AfterMethod
public void tearDown() {

actionExecutorConfigStatic.close();
}

@Test
public void testCallAPIUnacceptableContentTypeForSuccessResponse()
throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockitoAnnotations;
import org.testng.Assert;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
Expand Down Expand Up @@ -302,4 +303,79 @@ public void testGetExcludedParamsInActionRequestForEmptyConfigForAllAndDefinedTy
actionExecutorConfig.getExcludedParamsInActionRequestForActionType(ActionType.PRE_ISSUE_ACCESS_TOKEN);
assertEquals(excludedHeaders, Collections.emptySet());
}

@Test
public void testGetHttpReadTimeoutInMillis() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPReadTimeout", "5000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(5000, actionExecutorConfig.getHttpReadTimeoutInMillis());
}

@Test
public void testGetHttpReadTimeoutInMillisForInvalidConfig() {

//If the server configuration value is not a number, the default http read timeout value of 5000 is parsed
Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPReadTimeout", "value");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(5000, actionExecutorConfig.getHttpReadTimeoutInMillis());
}

@Test
public void testGetHttpConnectionRequestTimeoutInMillis() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPConnectionRequestTimeout", "2000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionRequestTimeoutInMillis());
}

@Test
public void testGetHttpConnectionTimeoutInMillis() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPConnectionTimeout", "2000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionTimeoutInMillis());
}

@Test
public void testGetHttpConnectionPoolSize() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPConnectionPoolSize", "20");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(20, actionExecutorConfig.getHttpConnectionPoolSize());
}

@Test
public void testGetHttpConnectionPoolSizeForInvalidConfig() {

//If the server configuration value is not a number, the default http connection pool size value of 20 is parsed
Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPConnectionPoolSize", "value");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(20, actionExecutorConfig.getHttpConnectionPoolSize());
}

@Test
public void testGetHttpRequestRetryCount() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPRequestRetryCount", "2");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2, actionExecutorConfig.getHttpRequestRetryCount());
}

@Test
public void testGetHttpRequestRetryCountForInvalidConfig() {

//If the server configuration value is not a number, the default http request retry count value of 2 is parsed
Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClient.HTTPRequestRetryCount", "value");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2, actionExecutorConfig.getHttpRequestRetryCount());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,13 @@
</OutboundProvisioning>

<Actions>
<HTTPClient>
<HTTPConnectionTimeout>{{actions.http_client.connection_timeout}}</HTTPConnectionTimeout>
<HTTPReadTimeout>{{actions.http_client.read_timeout}}</HTTPReadTimeout>
<HTTPConnectionRequestTimeout>{{actions.http_client.request_timeout}}</HTTPConnectionRequestTimeout>
<HTTPConnectionPoolSize>{{actions.http_client.connection_pool_size}}</HTTPConnectionPoolSize>
<HTTPRequestRetryCount>{{actions.http_client.retry_count}}</HTTPRequestRetryCount>
</HTTPClient>
<MaximumActionsPerActionType>{{actions.maximum_actions_per_action_type}}</MaximumActionsPerActionType>
<ActionRequest>
<ExcludedHeaders>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1571,6 +1571,11 @@

"on_demand_config.on_initial_use.enable_sms_otp_password_recovery_if_connector_enabled": false,

"actions.http_client.connection_timeout": 2000,
"actions.http_client.read_timeout": 5000,
"actions.http_client.request_timeout": 2000,
"actions.http_client.connection_pool_size": 20,
"actions.http_client.retry_count": 2,
"actions.maximum_actions_per_action_type": 1,
"actions.action_request.excluded_headers": [
"authorization",
Expand Down
Loading