Skip to content

Commit

Permalink
Address reviewed changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@osandamaleesha
osandamaleesha committed Sep 13, 2024
1 parent b3c446a commit 5f6e873
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 21 deletions.
8 changes: 4 additions & 4 deletions ...ion.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java
View file
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 = ActionExecutorConfig.getInstance().getHttpReadTimeout();
int connectionRequestTimeout = ActionExecutorConfig.getInstance().getHttpConnectionRequestTimeout();
int connectionTimeout = ActionExecutorConfig.getInstance().getHttpConnectionTimeout();
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
49 changes: 38 additions & 11 deletions ...on/src/main/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ public class ActionExecutorConfig {
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 int DEFAULT_HTTP_READ_TIMEOUT = 5000;
private static final int DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT = 2000;
private static final int DEFAULT_HTTP_CONNECTION_TIMEOUT = 2000;
private static final String HTTP_CLIENT_CONNECTION_POOL_SIZE_PROPERTY = "Actions.HTTPClientConnectionPoolSize";
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 @@ -80,15 +82,39 @@ 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);
}
}
}
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 getHttpReadTimeout() {
public int getHttpReadTimeoutInMillis() {

return parseTimeoutConfig(HTTP_READ_TIMEOUT_PROPERTY, DEFAULT_HTTP_READ_TIMEOUT);
return parseTimeoutConfig(HTTP_READ_TIMEOUT_PROPERTY, DEFAULT_HTTP_READ_TIMEOUT_IN_MILLIS);
}

/**
Expand All @@ -97,9 +123,10 @@ public int getHttpReadTimeout() {
*
* @return The HTTP connection request timeout int value in milliseconds.
*/
public int getHttpConnectionRequestTimeout() {
public int getHttpConnectionRequestTimeoutInMillis() {

return parseTimeoutConfig(HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT);
return parseTimeoutConfig(HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY,
DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT_MILLIS);
}

/**
Expand All @@ -108,9 +135,9 @@ public int getHttpConnectionRequestTimeout() {
*
* @return The HTTP connection timeout int value in milliseconds.
*/
public int getHttpConnectionTimeout() {
public int getHttpConnectionTimeoutInMillis() {

return parseTimeoutConfig(HTTP_CONNECTION_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_TIMEOUT);
return parseTimeoutConfig(HTTP_CONNECTION_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_TIMEOUT_MILLIS);
}

private int parseTimeoutConfig(String timeoutTypeName, int defaultTimeout) {
Expand All @@ -122,8 +149,8 @@ private int parseTimeoutConfig(String timeoutTypeName, int defaultTimeout) {
timeoutPropertyValue = Integer.parseInt(timeoutValue);
} catch (NumberFormatException e) {
if (LOG.isDebugEnabled()) {
LOG.debug(" " + timeoutTypeName + " property value must be an integer in identity.xml. " +
"Default property value is parsed ", e);
LOG.debug("Failed to read " + timeoutTypeName + " property in identity.xml." +
" Expects a number. Using the default value: " + defaultTimeout, e);
}
}
}
Expand Down
39 changes: 33 additions & 6 deletions ...rc/test/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfigTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,29 +305,56 @@ public void testGetExcludedParamsInActionRequestForEmptyConfigForAllAndDefinedTy
}

@Test
public void testGetHttpReadTimeout() {
public void testGetHttpReadTimeoutInMillis() {

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

@Test
public void testGetHttpConnectionRequestTimeout() {
public void testGetHttpReadTimeoutInMillisWithoutNumberFormat() {

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.getHttpConnectionRequestTimeout());
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionRequestTimeoutInMillis());
}

@Test
public void testGetHttpConnectionTimeout() {
public void testGetHttpConnectionTimeoutInMillis() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPConnections.HTTPConnectionTimeout", "2000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionTimeout());
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());
}
}
1 change: 1 addition & 0 deletions ...ures/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2025,6 +2025,7 @@
<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
1 change: 1 addition & 0 deletions ...y.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1574,6 +1574,7 @@
"actions.http_connections.read_timeout": 5000,
"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

0 comments on commit 5f6e873

Please sign in to comment.