Skip to content

Commit

Permalink
Merge pull request #5935 from osandamaleesha/add/ServerConfigurations
Browse files Browse the repository at this point in the history
Add timeout server configurations to action execution
  • Loading branch information
malithie committed Sep 19, 2024
2 parents 22f2975 + a8fffd8 commit c2afbe4
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 5 deletions.
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

0 comments on commit c2afbe4

Please sign in to comment.