Address reviewed changes

components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/APIClient.java

@@ -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)
@@ -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();
  }

components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfig.java

@@ -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() {
 
@@ -80,15 +82,34 @@ public boolean isExecutionForActionTypeEnabled(ActionType actionType) {
  }
  }
 
+ 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);
  }
 
  /**
@@ -97,9 +118,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);
  }
 
  /**
@@ -108,9 +130,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) {
@@ -122,8 +144,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);
  }
  }
  }

components/action-mgt/org.wso2.carbon.identity.action.execution/src/test/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfigTest.java

@@ -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 testGetHttpReadTimeoutInMillisWithoutNumber(){
+
+ 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());
  }
 }

features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2

@@ -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>

features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json

@@ -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",