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().getHttpReadTimeout();
int connectionRequestTimeout = ActionExecutorConfig.getInstance().getHttpConnectionRequestTimeout();
int connectionTimeout = ActionExecutorConfig.getInstance().getHttpConnectionTimeout();

RequestConfig config = RequestConfig.custom()
.setConnectTimeout(connectionTimeout)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@
"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 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;
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved

private ActionExecutorConfig() {

Expand Down Expand Up @@ -73,6 +80,56 @@
}
}

/**
* 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() {

return parseTimeoutConfig(HTTP_READ_TIMEOUT_PROPERTY, DEFAULT_HTTP_READ_TIMEOUT);
}

/**
* 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 getHttpConnectionRequestTimeout() {

return parseTimeoutConfig(HTTP_CONNECTION_REQUEST_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_REQUEST_TIMEOUT);
}

/**
* 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 getHttpConnectionTimeout() {

return parseTimeoutConfig(HTTP_CONNECTION_TIMEOUT_PROPERTY, DEFAULT_HTTP_CONNECTION_TIMEOUT);
}
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved

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) {

Check warning on line 123 in components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfig.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L123 was not covered by tests
if (LOG.isDebugEnabled()) {
LOG.debug(" " + timeoutTypeName + " property value must be an integer in identity.xml. " +

Check warning on line 125 in components/action-mgt/org.wso2.carbon.identity.action.execution/src/main/java/org/wso2/carbon/identity/action/execution/util/ActionExecutorConfig.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L125 was not covered by tests
osandamaleesha marked this conversation as resolved.
Show resolved Hide resolved
"Default property value is parsed ", 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,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,31 @@ public void testGetExcludedParamsInActionRequestForEmptyConfigForAllAndDefinedTy
actionExecutorConfig.getExcludedParamsInActionRequestForActionType(ActionType.PRE_ISSUE_ACCESS_TOKEN);
assertEquals(excludedHeaders, Collections.emptySet());
}

@Test
public void testGetHttpReadTimeout() {

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

@Test
public void testGetHttpConnectionRequestTimeout() {

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

@Test
public void testGetHttpConnectionTimeout() {

Map<String, Object> configMap = new HashMap<>();
configMap.put("Actions.HTTPConnections.HTTPConnectionTimeout", "2000");
when(mockIdentityConfigParser.getConfiguration()).thenReturn(configMap);
Assert.assertEquals(2000, actionExecutorConfig.getHttpConnectionTimeout());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2020,6 +2020,11 @@
</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>
<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,9 @@

"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.maximum_actions_per_action_type": 1,
"actions.action_request.excluded_headers": [
"authorization",
Expand Down
Loading