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().getHttpClientConnectionPoolSize());
httpClient = HttpClientBuilder.create().setDefaultRequestConfig(config).setConnectionManager(connectionManager)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ 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.HTTPConnections.HTTPReadTimeout";
private static final String HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY =
"Actions.HTTPConnections.HTTPConnectionRequestTimeout";
private static final String HTTP_CONNECTION_TIMEOUT_PROPERTY = "Actions.HTTPConnections.HTTPConnectionTimeout";
private static final String HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY = "Actions.HTTPClientConnectionPoolSize";
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved
private static final int DEFAULT_HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY = 20;
private static final int DEFAULT_HTTP_READ_TIMEOUT_IN_MILLIS = 5000;
private static final int DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT_MILLIS = 2000;
private static final int DEFAULT_HTTP_CONNECTION_TIMEOUT_MILLIS = 2000;

private ActionExecutorConfig() {

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

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

int poolSizePropertyValue = DEFAULT_HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY;
String poolSizeValue = (String) IdentityConfigParser.getInstance().getConfiguration().
get(HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY);
if (StringUtils.isNotBlank(poolSizeValue)) {
try {
poolSizePropertyValue = Integer.parseInt(poolSizeValue);
} catch (NumberFormatException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Failed to read Http client connection pool size property in identity.xml." +
" Expects a number. Using the default value: " +
DEFAULT_HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY, e);
}
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved
}
}
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_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_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) {
if (LOG.isDebugEnabled()) {
LOG.debug("Failed to read " + timeoutTypeName + " property in identity.xml." +
" Expects a number. Using the default value: " + defaultTimeout, e);
}
}
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved
}
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,27 @@ 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);
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,58 @@ 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.HTTPConnections.HTTPReadTimeout", "5000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(5000, actionExecutorConfig.getHttpReadTimeoutInMillis());
}

@Test
public void testGetHttpReadTimeoutInMillisWithoutNumberFormat() {
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPConnections.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.HTTPConnections.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.HTTPConnections.HTTPConnectionTimeout", "2000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionTimeoutInMillis());
}

@Test
public void testGetHttpClientConnectionPoolSize() {

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

@Test
public void testGetHttpClientConnectionPoolSizeWithoutNumberFormat() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPClientConnectionPoolSize", "value");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(20, actionExecutorConfig.getHttpClientConnectionPoolSize());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,12 @@
</OutboundProvisioning>

<Actions>
<HTTPConnections>
<HTTPConnectionTimeout>{{actions.http_connections.connection_timeout}}</HTTPConnectionTimeout>
<HTTPReadTimeout>{{actions.http_connections.read_timeout}}</HTTPReadTimeout>
<HTTPConnectionRequestTimeout>{{actions.http_connections.request_timeout}}</HTTPConnectionRequestTimeout>
</HTTPConnections>
<HTTPClientConnectionPoolSize>{{actions.http_client_connection_pool_size}}</HTTPClientConnectionPoolSize>
<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,10 @@

"on_demand_config.on_initial_use.enable_sms_otp_password_recovery_if_connector_enabled": false,

"actions.http_connections.read_timeout": 5000,
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved
"actions.http_connections.request_timeout": 2000,
"actions.http_connections.connection_timeout": 2000,
"actions.http_client_connection_pool_size": 20,
"actions.maximum_actions_per_action_type": 1,
"actions.action_request.excluded_headers": [
"authorization",
Expand Down
Loading