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

Fix flare client not using timeout setting given by environment variable #104

Merged
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
12 changes: 9 additions & 3 deletions feasibility-dsf-process/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -85,19 +85,25 @@
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>testcontainers</artifactId>
<version>1.18.3</version>
<version>${testcontainers.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>nginx</artifactId>
<version>1.18.3</version>
<version>${testcontainers.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>toxiproxy</artifactId>
<version>${testcontainers.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.testcontainers</groupId>
<artifactId>junit-jupiter</artifactId>
<version>1.18.3</version>
<version>${testcontainers.version}</version>
<scope>test</scope>
</dependency>
<dependency>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ private FlareWebserviceClient createFlareClient(HttpClient httpClient) {
public HttpClient flareHttpClient(@Qualifier("base-client") SSLContext sslContext,
EvaluationSettingsProvider evaluationSettingsProvider) {
if (EvaluationStrategy.STRUCTURED_QUERY == evaluationSettingsProvider.evaluationStrategy()) {
HttpClientBuilder builder = new TlsClientFactory(null, sslContext).getNativeHttpClientBuilder();
TlsClientFactory clientFactory = new TlsClientFactory(null, sslContext);
clientFactory.setConnectTimeout(connectTimeout);
clientFactory.setConnectionRequestTimeout(connectTimeout);
clientFactory.setSocketTimeout(connectTimeout);
HttpClientBuilder builder = clientFactory.getNativeHttpClientBuilder();

BasicCredentialsProvider credentialsProvider = new BasicCredentialsProvider();

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package de.medizininformatik_initiative.feasibility_dsf_process.client.flare;

import com.google.common.base.Stopwatch;
import eu.rekawek.toxiproxy.Proxy;
import eu.rekawek.toxiproxy.ToxiproxyClient;
import eu.rekawek.toxiproxy.model.ToxicDirection;
import eu.rekawek.toxiproxy.model.toxic.Latency;
import org.assertj.core.api.Condition;
import org.assertj.core.description.Description;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.testcontainers.containers.ToxiproxyContainer;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;

import java.io.IOException;
import java.net.SocketTimeoutException;
import java.time.Duration;
import java.util.Random;
import java.util.concurrent.TimeUnit;

import static java.lang.String.format;
import static org.assertj.core.api.Assertions.assertThatNoException;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

@Tag("client")
@Tag("flare")
@SpringBootTest(classes = FlareWebserviceClientSpringConfig.class)
@Testcontainers
public class FlareWebserviceClientImplTimeoutsIT extends FlareWebserviceClientImplBaseIT {

private static final int PROXY_PORT = 8666;
private static final int RANDOM_CLIENT_TIMEOUT = new Random().nextInt(5000, 20000);

private Stopwatch executionTimer = Stopwatch.createUnstarted();

@Autowired protected FlareWebserviceClient flareClient;

@Container
public static ToxiproxyContainer toxiproxy = new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.7.0")
.withNetwork(DEFAULT_CONTAINER_NETWORK)
.dependsOn(flare);
private static ToxiproxyClient toxiproxyClient;
private static Proxy proxy;
private static Latency latency;

@DynamicPropertySource
static void dynamicProperties(DynamicPropertyRegistry registry) {
var flareHost = toxiproxy.getHost();
var flarePort = toxiproxy.getMappedPort(PROXY_PORT);

registry.add("de.medizininformatik_initiative.feasibility_dsf_process.client.flare.timeout.connect",
() -> RANDOM_CLIENT_TIMEOUT);
registry.add("de.medizininformatik_initiative.feasibility_dsf_process.evaluation.strategy",
() -> "structured-query");
registry.add("de.medizininformatik_initiative.feasibility_dsf_process.client.flare.base_url",
() -> String.format("http://%s:%s/", flareHost, flarePort));
}

@BeforeAll
static void setup() throws IOException {
toxiproxyClient = new ToxiproxyClient(toxiproxy.getHost(), toxiproxy.getControlPort());
proxy = toxiproxyClient.createProxy("flare", "0.0.0.0:" + PROXY_PORT,
format("%s:%s", flare.getNetworkAliases().get(0), flare.getExposedPorts().get(0)));
latency = proxy.toxics().latency("latency", ToxicDirection.UPSTREAM, 0);
}

@BeforeEach
void startClock() {
executionTimer.reset().start();
}

@Test
@DisplayName("flare client fails getting no response after given socket timeout")
public void requestFeasibilityWithLongerProxyTimeoutFails() throws IOException {
var rawStructuredQuery = this.getClass().getResource("valid-structured-query.json")
.openStream().readAllBytes();
var proxyTimeout = RANDOM_CLIENT_TIMEOUT + 10000;
latency.setLatency(proxyTimeout);

assertThatThrownBy(() -> flareClient.requestFeasibility(rawStructuredQuery))
.describedAs(new Description() {

@Override
public String value() {
executionTimer.stop();
return format("execution time is %s ms", executionTimer.elapsed(TimeUnit.MILLISECONDS));
}
})
.isInstanceOf(IOException.class)
.hasMessageStartingWith("Error sending POST request to flare webservice")
.hasCauseInstanceOf(SocketTimeoutException.class)
.is(new Condition<>(
_e -> Duration.ofMillis(RANDOM_CLIENT_TIMEOUT).minus(executionTimer.elapsed()).isNegative(),
"executed longer than client timeout of %dms", RANDOM_CLIENT_TIMEOUT))
.is(new Condition<>(_e -> executionTimer.elapsed().minusMillis(proxyTimeout).isNegative(),
"executed shorter than proxy timeout of %dms", proxyTimeout));
}

@Test
@DisplayName("flare client succeeds getting a response before given socket timeout")
public void requestFeasibilityWithShorterProxyTimeoutSucceeds() throws IOException {
var rawStructuredQuery = this.getClass().getResource("valid-structured-query.json")
.openStream().readAllBytes();
latency.setLatency(RANDOM_CLIENT_TIMEOUT - 2000);

assertThatNoException().isThrownBy(() -> flareClient.requestFeasibility(rawStructuredQuery));
}
}
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<compileTarget>17</compileTarget>
<dsf.version>1.3.1</dsf.version>
<hapi.fhir.version>5.1.0</hapi.fhir.version>
<testcontainers.version>1.19.3</testcontainers.version>
</properties>

<name>MII Feasibility Processes</name>
Expand Down
Loading