From 7eb741fa1e42ff0ccaa0743b7b5b31550e327d55 Mon Sep 17 00:00:00 2001 From: Michael Folz Date: Thu, 12 Dec 2024 10:10:41 +0100 Subject: [PATCH] #415 - Make fhir client timeouts configurable - introduce env variables for connect, socket and connectionRequest timeouts in the fhir client --- README.md | 8 +++++ docker-compose.yml | 3 ++ .../broker/direct/DirectSpringConfig.java | 24 ++++++++++--- src/main/resources/application.yml | 7 +++- .../broker/direct/DirectSpringConfigIT.java | 7 ++-- .../broker/direct/DirectSpringConfigTest.java | 35 +++++++++++++++---- 6 files changed, 68 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index b8aee440..89781a44 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/docker-compose.yml b/docker-compose.yml index 8b7af92e..baee96c2 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -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/} diff --git a/src/main/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfig.java b/src/main/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfig.java index cf563d9d..b9fa88b6 100644 --- a/src/main/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfig.java +++ b/src/main/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfig.java @@ -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; @@ -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") @@ -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: {})", @@ -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; } diff --git a/src/main/resources/application.yml b/src/main/resources/application.yml index 5a5f62f4..94736ceb 100644 --- a/src/main/resources/application.yml +++ b/src/main/resources/application.yml @@ -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: diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigIT.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigIT.java index 89f45635..f2e6d1ce 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigIT.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigIT.java @@ -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)); @@ -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(); @@ -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(); diff --git a/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigTest.java b/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigTest.java index a7e08b32..ae7a655a 100644 --- a/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigTest.java +++ b/src/test/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigTest.java @@ -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; @@ -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(); @@ -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(); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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);