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

added read timeout to http wait strategy #2058

Merged
merged 6 commits into from
Nov 11, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import com.google.common.base.Strings;
import com.google.common.io.BaseEncoding;
import java.util.Optional;
import lombok.extern.slf4j.Slf4j;
import org.rnorth.ducttape.TimeoutException;
import org.testcontainers.containers.ContainerLaunchException;
Expand All @@ -13,7 +12,10 @@
import java.net.HttpURLConnection;
import java.net.URI;
import java.net.URL;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
Expand Down Expand Up @@ -41,6 +43,7 @@ public class HttpWaitStrategy extends AbstractWaitStrategy {
private Predicate<String> responsePredicate;
private Predicate<Integer> statusCodePredicate = null;
private Optional<Integer> livenessPort = Optional.empty();
private Duration readTimeout = Duration.ofSeconds(1);

/**
* Waits for the given status code.
Expand Down Expand Up @@ -108,6 +111,17 @@ public HttpWaitStrategy withBasicCredentials(String username, String password) {
return this;
}

/**
* Set the HTTP connections read timeout.
*
* @param timeout the timeout in millis
* @return this
*/
public HttpWaitStrategy withReadTimeout(Duration timeout) {
this.readTimeout = timeout;
return this;
}

/**
* Waits for the response to pass the given predicate
* @param responsePredicate The predicate to test the response against
Expand Down Expand Up @@ -143,6 +157,7 @@ protected void waitUntilReady() {
getRateLimiter().doWhenReady(() -> {
try {
final HttpURLConnection connection = (HttpURLConnection) new URL(uri).openConnection();
connection.setReadTimeout(Math.toIntExact(readTimeout.get(ChronoUnit.MILLIS)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC it should be readTimeout.toMillis(), otherwise you're only getting the milliseconds part of it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but I have to adjust this tomorrow ;)
I'll leave a comment when I am ready.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We plan to release tonight, I can do the review leftovers for you if you wish :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I updated it just now. Please check it and if you need further adjustments feel free to do it yourself if you want ;)


// authenticate
if (!Strings.isNullOrEmpty(username)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.rnorth.ducttape.RetryCountExceededException;
import org.testcontainers.containers.wait.strategy.HttpWaitStrategy;

import java.time.Duration;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Predicate;

Expand Down Expand Up @@ -131,6 +132,15 @@ public void testWaitUntilReadyWithSpecificPort() {
));
}

@Test
public void testWaitUntilReadyWithTimoutCausedByReadTimeout() {
waitUntilReadyAndTimeout(
startContainerWithCommand(createShellCommand("0 Connection Refused", GOOD_RESPONSE_BODY, 9090),
createHttpWaitStrategy(ready).forPort(9090).withReadTimeout(Duration.ofMillis(1)),
9090
));
}

/**
* @param ready the AtomicBoolean on which to indicate success
* @return the WaitStrategy under test
Expand All @@ -143,6 +153,7 @@ protected HttpWaitStrategy buildWaitStrategy(final AtomicBoolean ready) {

/**
* Create a HttpWaitStrategy instance with a waitUntilReady implementation
*
* @param ready Indicates that the WaitStrategy has completed waiting successfully.
* @return the HttpWaitStrategy instance
*/
Expand All @@ -163,9 +174,9 @@ private String createShellCommand(String header, String responseBody) {

private String createShellCommand(String header, String responseBody, int port) {
int length = responseBody.getBytes().length;
return "while true; do { echo -e \"HTTP/1.1 "+header+NEWLINE+
"Content-Type: text/html"+NEWLINE+
"Content-Length: "+length +NEWLINE+ "\";"
+" echo \""+responseBody+"\";} | nc -lp " + port + "; done";
return "while true; do { echo -e \"HTTP/1.1 " + header + NEWLINE +
"Content-Type: text/html" + NEWLINE +
"Content-Length: " + length + NEWLINE + "\";"
+ " echo \"" + responseBody + "\";} | nc -lp " + port + "; done";
}
}