Skip to content

Commit

Permalink
Add framework to allow test-specific assertions on HTTP response.
Browse files Browse the repository at this point in the history
Motivation:

Individual tests may have certain expectations on how IDS responds to
HTTP requests.

Some interactions supported by `TestingClient` exercise IDS requests
that return content; for example, `getData`.  For these interactions,
`TestingClient` returns the content from IDS and not the HTTP response.
This makes it impossible for a test to assert that, for these
interactions, the HTTP response from IDS matches the test's
expectations.

Modification:

Update `TestingClient` to allow the test to include one or more
assertions on the HTTP response.  The support following the builder
pattern.

The assertions are provided by the test as Consumer objects that accept
an HttpResponse object, with lambdas providing an easy to build such
objects.  Some static methods are included that provide common
assertions, to keep code readable and DRY.

The Apache client is customised to include a response interceptor,
through which the assertions are made.

The existing code that checks the combined `Content-Length` (if present)
and `Transfer-Encoding` headers (if present) is valid is refactored to
take advantage of this framework.  This check now applies uniformly to
all HTTP responses from IDS.

Some other, existing tests are updated to take advantage of the new
framework.

Further refactoring is possible; for example, to make the check on the
response's status code more explicit.  Such a refactoring would reduce
the complexity of the `TestingClient` interaction methods by reducing
the parameter count.

Result:

It is now possible to write tests that make test-specific assertions on
the HTTP response from IDS for interactions where IDS returns content.

Signed-off-by: Paul Millar <paul.millar@desy.de>
  • Loading branch information
paulmillar committed Feb 6, 2024
1 parent 1e32fb4 commit cfce753
Show file tree
Hide file tree
Showing 6 changed files with 266 additions and 60 deletions.
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,12 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest</artifactId>
<version>2.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/org/icatproject/ids/integration/BaseTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileVisitResult;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -20,6 +21,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.UUID;
import java.util.zip.CRC32;
Expand Down Expand Up @@ -369,6 +371,15 @@ protected void checkStream(InputStream stream, long id) throws IOException {
assertTrue(found);
}

protected long fileLength(long id) {
return ids.entrySet().stream()
.filter(e -> id == e.getValue())
.map(Map.Entry::getKey)
.map(fsizes::get)
.findFirst()
.orElseThrow(() -> new NoSuchElementException("No file with id " + id));
}

protected void writeToFile(Datafile df, String content, String key)
throws IOException, IcatException_Exception, NoSuchAlgorithmException {
Path path = setup.getStorageDir().resolve(df.getLocation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.io.InputStream;
import java.util.Arrays;
import org.apache.http.HttpResponse;

import static org.junit.Assert.assertEquals;
import org.junit.BeforeClass;
Expand All @@ -15,6 +16,8 @@
import org.icatproject.ids.integration.util.client.InsufficientPrivilegesException;
import org.icatproject.ids.integration.util.client.NotFoundException;
import org.icatproject.ids.integration.util.client.TestingClient.Flag;
import static org.icatproject.ids.integration.util.client.TestingClient.isNotChunkedEncoded;
import static org.icatproject.ids.integration.util.client.TestingClient.hasHeader;

public class GetDataExplicitTest extends BaseTest {

Expand Down Expand Up @@ -98,52 +101,71 @@ public void forbiddenTest() throws Exception {

@Test
public void correctBehaviourTestNone() throws Exception {
try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.NONE, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.NONE, 0, 200)) {
checkZipStream(stream, datafileIds, 57L, 0);
}

try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.NONE, 0, 200)) {
var datafileId = datafileIds.get(0);
var fileLength = fileLength(datafileId);
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded(), // assumes caching
hasHeader("Content-Length", fileLength))
.getData(sessionId, new DataSelection().addDatafile(datafileId),
Flag.NONE, 0, 200)) {
checkStream(stream, datafileIds.get(0));
}
}

@Test
public void correctBehaviourTestCompress() throws Exception {
try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.COMPRESS, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.COMPRESS, 0, 200)) {
checkZipStream(stream, datafileIds, 36L, 0);
}

try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.COMPRESS, 0, 200)) {
checkStream(stream, datafileIds.get(0));
}
}

@Test
public void correctBehaviourTestZip() throws Exception {
try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.ZIP, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.ZIP, 0, 200)) {
checkZipStream(stream, datafileIds, 57L, 0);
}

try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.ZIP, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.ZIP, 0, 200)) {
checkZipStream(stream, datafileIds.subList(0, 1), 57L, 0);
}
}

@Test
public void correctBehaviourTestZipAndCompress() throws Exception {
try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.ZIP_AND_COMPRESS, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafiles(datafileIds),
Flag.ZIP_AND_COMPRESS, 0, 200)) {
checkZipStream(stream, datafileIds, 36L, 0);
}

try (InputStream stream = testingClient.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.ZIP_AND_COMPRESS, 0, 200)) {
try (InputStream stream = testingClient
.assertingHttpResponse(isNotChunkedEncoded()) // assumes caching
.getData(sessionId, new DataSelection().addDatafile(datafileIds.get(0)),
Flag.ZIP_AND_COMPRESS, 0, 200)) {
checkZipStream(stream, datafileIds.subList(0, 1), 36L, 0);
}
}
Expand Down Expand Up @@ -173,5 +195,4 @@ public void correctBehaviourInvestigations() throws Exception {
checkZipStream(stream, datafileIds.subList(0, 1), 57L, 0);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package org.icatproject.ids.integration.util;

import static java.util.Objects.requireNonNull;
import org.apache.http.Header;
import org.apache.http.HttpResponse;
import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import static org.hamcrest.Matchers.equalToIgnoringCase;

/**
* A custom header that matches HTTP headers in some HttpResponse object. It
* supports two kind of matches: that the header is missing or that the header
* is present and the value matches some other Matcher.
*/
public class HeaderMatcher extends BaseMatcher<HttpResponse> {

private final String headerName;
private final Matcher<String> matcher;

public HeaderMatcher(String headerName, Matcher<String> valueMatcher) {
this.headerName = requireNonNull(headerName);
this.matcher = valueMatcher;
}

private boolean isHeaderExpected() {
return matcher != null;
}

@Override
public boolean matches(Object item) {
HttpResponse response = (HttpResponse) item; // It is a bug if item is not an HttpResponse.

Header header = response.getFirstHeader(headerName);

if (!isHeaderExpected()) {
return header == null;
}

if (header == null) {
return false;
}

String headerValue = header.getValue();

return matcher.matches(headerValue);
}

@Override
public void describeTo(Description description) {
if (isHeaderExpected()) {
description.appendText("a '")
.appendText(headerName)
.appendText("' header that ")
.appendDescriptionOf(matcher);
} else {
description.appendText("no '").appendText(headerName).appendText("' header");
}
}

/**
* Match an HttpResponse that does not contain the named header.
* @param header The name of the HTTP response header.
* @return a Matcher that verifies expected response.
*/
public static Matcher<HttpResponse> hasNoHeader(String header) {
return new HeaderMatcher(header, null);
}

/**
* Match an HttpResponse that contains the named header with a value that
* matches the supplied matcher.
* @param header The name of the HTTP response header.
* @param matcher The matcher for the header value.
* @return a Matcher that verifies expected response.
*/
public static Matcher hasHeaderMatching(String header, Matcher<String> matcher) {
return new HeaderMatcher(header, matcher);
}

/**
* Match an HttpResponse that contains the named header with the specific
* value (ignoring case).
* @param header The name of the HTTP response header.
* @param value The expected value.
* @return a Matcher that verifies expected response.
*/
public static Matcher hasHeader(String header, Object value) {
Matcher<String> m = equalToIgnoringCase(String.valueOf(value));
return hasHeaderMatching(header, m);
}
}
33 changes: 33 additions & 0 deletions src/test/java/org/icatproject/ids/integration/util/LongValue.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package org.icatproject.ids.integration.util;

import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.hamcrest.TypeSafeMatcher;

/**
* A Matcher that checks whether a String argument is a Long value. The code is
* heavily based on an answer from Ezequiel to
* <a href="https://stackoverflow.com/questions/58099695/is-there-a-way-in-hamcrest-to-test-for-a-value-to-be-a-number">
* a stack overview question</a>.
*/
public class LongValue extends TypeSafeMatcher<String> {

@Override
protected boolean matchesSafely(String s) {
try {
Long.valueOf(s);
} catch (NumberFormatException nfe) {
return false;
}
return true;
}

@Override
public void describeTo(Description description) {
description.appendText("is long");
}

public static Matcher<String> longValue() {
return new LongValue();
}
}
Loading

0 comments on commit cfce753

Please sign in to comment.