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

#415 - Make fhir client timeouts configurable #416

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,14 @@ instance the backend is allowed to connect to.
In order to run the backend using the DIRECT broker path with CQL,
the _CQL_SERVER_BASE_URL_ environment variable needs to be set to a running instance of a CQL compatible
FHIR server.
Timeouts can be configured via the following variables:

| EnvVar | Description | Example | Default |
|-------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------|---------|----------|
| BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECT_MS | This is the amount of time that the initial connection attempt network operation may block without failing. | | 10000 |
| BROKER_CLIENT_DIRECT_CQL_TIMEOUT_SOCKET_MS | the amount of time that a read/write network operation may block without failing. | | 10000 |
| BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECTIONREQUEST_MS | This is the amount of time that the HTTPClient connection pool may wait for an available connection before failing. | | 10000 |



### Running the AKTIN Broker Path
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ services:
BROKER_CLIENT_DIRECT_AUTH_OAUTH_CLIENT_SECRET: ${DATAPORTAL_BACKEND_BROKER_CLIENT_DIRECT_AUTH_OAUTH_CLIENT_SECRET:-insecure}
FLARE_WEBSERVICE_BASE_URL: ${DATAPORTAL_BACKEND_FLARE_WEBSERVICE_BASE_URL:-http://flare:8080}
CQL_SERVER_BASE_URL: ${DATAPORTAL_BACKEND_CQL_SERVER_BASE_URL:-http://fhir-server:8080/fhir}
BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECT_MS: ${DATAPORTAL_BACKEND_BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECT_MS}
BROKER_CLIENT_DIRECT_CQL_TIMEOUT_SOCKET_MS: ${DATAPORTAL_BACKEND_BROKER_CLIENT_DIRECT_CQL_TIMEOUT_SOCKET_MS}
BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECTIONREQUEST_MS: ${DATAPORTAL_BACKEND_BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECTIONREQUEST_MS}
# ---- Aktin broker
BROKER_CLIENT_AKTIN_ENABLED: ${DATAPORTAL_BACKEND_AKTIN_ENABLED:-false}
AKTIN_BROKER_BASE_URL: ${DATAPORTAL_BACKEND_AKTIN_BROKER_BASE_URL:-http://aktin-broker:8080/broker/}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,23 @@ public class DirectSpringConfig {
private final String cqlBaseUrl;
private final String username;
private final String password;
private String issuer;
private String clientId;
private String clientSecret;
private final String issuer;
private final String clientId;
private final String clientSecret;
private final int cqlConnectTimeout;
private final int cqlSocketTimeout;
private final int cqlConnectionRequestTimeout;

public DirectSpringConfig(@Value("${app.broker.direct.useCql:false}") boolean useCql,
public DirectSpringConfig(@Value("${app.broker.direct.cql.enabled:false}") boolean useCql,
@Value("${app.flare.baseUrl}") String flareBaseUrl, @Value("${app.cql.baseUrl}") String cqlBaseUrl,
@Value("${app.broker.direct.auth.basic.username}") String username,
@Value("${app.broker.direct.auth.basic.password}") String password,
@Value("${app.broker.direct.auth.oauth.issuer.url}") String issuer,
@Value("${app.broker.direct.auth.oauth.client.id}") String clientId,
@Value("${app.broker.direct.auth.oauth.client.secret}") String clientSecret) {
@Value("${app.broker.direct.auth.oauth.client.secret}") String clientSecret,
@Value("${app.broker.direct.cql.timeout.connect}") int cqlConnectTimeout,
@Value("${app.broker.direct.cql.timeout.socket}") int cqlSocketTimeout,
@Value("${app.broker.direct.cql.timeout.connectionRequest}") int cqlConnectionRequestTimeout) {
this.useCql = useCql;
this.flareBaseUrl = flareBaseUrl;
this.cqlBaseUrl = cqlBaseUrl;
Expand All @@ -49,6 +55,9 @@ public DirectSpringConfig(@Value("${app.broker.direct.useCql:false}") boolean us
this.issuer = issuer;
this.clientId = clientId;
this.clientSecret = clientSecret;
this.cqlConnectTimeout = cqlConnectTimeout;
this.cqlSocketTimeout = cqlSocketTimeout;
this.cqlConnectionRequestTimeout = cqlConnectionRequestTimeout;
}

@Qualifier("direct")
Expand All @@ -67,6 +76,9 @@ public BrokerClient directBrokerClient(WebClient directWebClientFlare,

@Bean
public IGenericClient getFhirClient(FhirContext fhirContext) {
fhirContext.getRestfulClientFactory().setConnectTimeout(cqlConnectTimeout);
fhirContext.getRestfulClientFactory().setSocketTimeout(cqlSocketTimeout);
fhirContext.getRestfulClientFactory().setConnectionRequestTimeout(cqlConnectionRequestTimeout);
IGenericClient iGenericClient = fhirContext.newRestfulGenericClient(cqlBaseUrl);
if (!isNullOrEmpty(password) && !isNullOrEmpty(username)) {
log.info("Configure direct broker instance with basic authentication (type: cql, url: {}, username: {})",
Expand All @@ -81,6 +93,8 @@ public IGenericClient getFhirClient(FhirContext fhirContext) {
} else {
log.info("Configure direct broker instance (type: cql, url: {})", cqlBaseUrl);
}
log.info("Direct broker instance timeouts are set to: Connect - {}ms, Socket - {}ms, ConnectionRequest - {}ms",
cqlConnectTimeout, cqlSocketTimeout, cqlConnectionRequestTimeout);
return iGenericClient;
}

Expand Down
7 changes: 6 additions & 1 deletion src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,13 @@ app:
id: ${BROKER_CLIENT_DIRECT_AUTH_OAUTH_CLIENT_ID:}
secret: ${BROKER_CLIENT_DIRECT_AUTH_OAUTH_CLIENT_SECRET:}
enabled: ${BROKER_CLIENT_DIRECT_ENABLED:false}
useCql: ${BROKER_CLIENT_DIRECT_USE_CQL:false}
obfuscateResultCount: ${BROKER_CLIENT_OBFUSCATE_RESULT_COUNT:false}
cql:
enabled: ${BROKER_CLIENT_DIRECT_USE_CQL:false}
timeout:
connect: ${BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECT_MS:10000}
socket: ${BROKER_CLIENT_DIRECT_CQL_TIMEOUT_SOCKET_MS:10000}
connectionRequest: ${BROKER_CLIENT_DIRECT_CQL_TIMEOUT_CONNECTIONREQUEST_MS:10000}
aktin:
enabled: ${BROKER_CLIENT_AKTIN_ENABLED:false}
broker:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ void tearDown() throws IOException {
void testDirectWebClientFlare_withCredentials() throws InterruptedException {
mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody("Foo"));
directSpringConfig = new DirectSpringConfig(true, String.format("http://localhost:%s", mockWebServer.getPort()),
null, USERNAME, PASSWORD, null, null, null);
null, USERNAME, PASSWORD, null, null, null, 10000, 10000, 10000);
var authHeaderValue = "Basic "
+ Base64.getEncoder().encodeToString((USERNAME + ":" + PASSWORD).getBytes(StandardCharsets.UTF_8));

Expand All @@ -80,7 +80,7 @@ void testDirectWebClientFlare_withCredentials() throws InterruptedException {
void testDirectWebClientFlare_withoutCredentials() throws InterruptedException {
mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody("Foo"));
directSpringConfig = new DirectSpringConfig(true, String.format("http://localhost:%s", mockWebServer.getPort()),
null, null, null, null, null, null);
null, null, null, null, null, null, 10000, 10000, 10000);

WebClient webClient = directSpringConfig.directWebClientFlare();

Expand Down Expand Up @@ -114,7 +114,8 @@ public MockResponse dispatch(RecordedRequest arg0) throws InterruptedException {
});
directSpringConfig = new DirectSpringConfig(true, null,
String.format("http://localhost:%s", mockWebServer.getPort()), null, null,
String.format("http://localhost:%s/realms/test", keycloak.getFirstMappedPort()), "account", "test");
String.format("http://localhost:%s/realms/test", keycloak.getFirstMappedPort()), "account", "test",
10000, 10000, 10000);
IGenericClient client = directSpringConfig.getFhirClient(FhirContext.forR4());

client.capabilities().ofType(CapabilityStatement.class).execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.web.reactive.function.client.WebClient;

import java.util.Random;
import java.util.SplittableRandom;

import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;

Expand Down Expand Up @@ -42,7 +45,7 @@ void setUp() {
@Test
void directWebClientFlare_withCredentials() {
directSpringConfig = new DirectSpringConfig(true, "http://my.flare.url", null, "username", "password", null, null,
null);
null, 10000, 10000, 10000);

WebClient webClient = directSpringConfig.directWebClientFlare();

Expand All @@ -52,7 +55,7 @@ void directWebClientFlare_withCredentials() {

@Test
void directWebClientFlare_withoutCredentials() {
directSpringConfig = new DirectSpringConfig(true, "http://my.flare.url", null, null, null, null, null, null);
directSpringConfig = new DirectSpringConfig(true, "http://my.flare.url", null, null, null, null, null, null, 10000, 10000, 10000);

WebClient webClient = directSpringConfig.directWebClientFlare();

Expand All @@ -63,7 +66,7 @@ void directWebClientFlare_withoutCredentials() {
@Test
void getFhirClient_withCredentials() {
directSpringConfig = new DirectSpringConfig(true, null, "http://my.fhir.url", "username", "password", null, null,
null);
null, 10000, 10000, 10000);

IGenericClient fhirClient = directSpringConfig.getFhirClient(fhirContext);

Expand All @@ -74,7 +77,7 @@ void getFhirClient_withCredentials() {

@Test
void getFhirClient_withoutCredentials() {
directSpringConfig = new DirectSpringConfig(true, null, "http://my.fhir.url", null, null, null, null, null);
directSpringConfig = new DirectSpringConfig(true, null, "http://my.fhir.url", null, null, null, null, null, 10000, 10000, 10000);

IGenericClient fhirClient = directSpringConfig.getFhirClient(fhirContext);

Expand All @@ -86,7 +89,7 @@ void getFhirClient_withoutCredentials() {
@Test
void directBrokerClient_withOAuthCredentials() {
directSpringConfig = new DirectSpringConfig(true, null, "http://my.fhir.url", null, null, "http://my.oauth.url",
"foo", "bar");
"foo", "bar", 10000, 10000, 10000);

IGenericClient fhirClient = directSpringConfig.getFhirClient(fhirContext);

Expand All @@ -95,9 +98,27 @@ void directBrokerClient_withOAuthCredentials() {
.anySatisfy(interceptor -> Assertions.assertThat(interceptor).isInstanceOf(OAuthInterceptor.class));
}

@Test
void directBrokerClient_customTimeoutsAreSet() {
SplittableRandom splittableRandom = new SplittableRandom();
int connectTimeout = splittableRandom.nextInt();
int socketTimeout = splittableRandom.nextInt();
int connectionRequestTimeout = splittableRandom.nextInt();

directSpringConfig = new DirectSpringConfig(true, null, "http://my.fhir.url", null, null, "http://my.oauth.url",
"foo", "bar", connectTimeout, socketTimeout, connectionRequestTimeout);

IGenericClient fhirClient = directSpringConfig.getFhirClient(fhirContext);

assertNotNull(fhirClient);
Assertions.assertThat(fhirClient.getFhirContext().getRestfulClientFactory().getConnectTimeout()).isEqualTo(connectTimeout);
Assertions.assertThat(fhirClient.getFhirContext().getRestfulClientFactory().getSocketTimeout()).isEqualTo(socketTimeout);
Assertions.assertThat(fhirClient.getFhirContext().getRestfulClientFactory().getConnectionRequestTimeout()).isEqualTo(connectionRequestTimeout);
}

@Test
void directBrokerClient_useCql() {
directSpringConfig = new DirectSpringConfig(true, null, null, null, null, null, null, null);
directSpringConfig = new DirectSpringConfig(true, null, null, null, null, null, null, null, 10000, 10000, 10000);

BrokerClient brokerClient = directSpringConfig.directBrokerClient(webClient, false, fhirConnector, fhirHelper);

Expand All @@ -106,7 +127,7 @@ void directBrokerClient_useCql() {

@Test
void directBrokerClient_useFlare() {
directSpringConfig = new DirectSpringConfig(false, null, null, null, null, null, null, null);
directSpringConfig = new DirectSpringConfig(false, null, null, null, null, null, null, null, 10000, 10000, 10000);

BrokerClient brokerClient = directSpringConfig.directBrokerClient(webClient, false, fhirConnector, fhirHelper);

Expand Down
Loading