#415 - Make fhir client timeouts configurable

- introduce env variables for connect, socket and connectionRequest timeouts in the fhir client
medizininformatik-initiative · Jan 10, 2025 · 2233f96 · 2233f96
1 parent 904634f
commit 2233f96
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 16 deletions.
diff --git 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
@@ -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/...main/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfig.java b/...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
@@ -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/...st/java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigIT.java b/...st/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/.../java/de/numcodex/feasibility_gui_backend/query/broker/direct/DirectSpringConfigTest.java b/.../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);