Skip to content

Commit

Permalink
Add support for raw byte array as a request input type
Browse files Browse the repository at this point in the history 
Also add some extra exception message and cause details to riposte-backstopper framework listener for various exceptions, and try to fix intermittent test on Travis CI
  • Loading branch information
@nicmunroe
nicmunroe committed Jan 31, 2018
1 parent bfcbcd0 commit 052006c
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 34 deletions.
10 changes: 8 additions & 2 deletions ...java/com/nike/riposte/server/componenttest/ServerAsynchronousProcessingComponentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import static java.util.Collections.singleton;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.hamcrest.number.OrderingComparison.greaterThan;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -128,9 +129,14 @@ protected void doServerAsyncProcessingVerificationTestForUrl(String endpointUrl,
// Now that the same number of executor threads were used as calls were made and that the total time for all calls was less than twice a single call.
// This combination proves that the server was processing calls asynchronously.
assertThat("This test only works if you do more than 1 simultaneous call", numSimultaneousCalls > 1, is(true));
assertThat("The number of executor threads used should have been the same as the number of simultaneous calls", executorThreadsUsed.size(), is(numSimultaneousCalls));
// TODO: Travis CI can sometimes be so slow that executor threads get reused, so we can't do an exact (executorThreadsUsed == numSimultaneousCalls) check. Rethink this test.
assertThat("The number of executor threads used should have been more than one", executorThreadsUsed.size(), is(greaterThan(1)));
long totalTimeForAllCalls = timeAfterAllCallsCompleted - timeBeforeAnyCallsStarted;
assertThat("Total time for the server to process all calls should have been less than twice a single call", totalTimeForAllCalls < (2 * SLEEP_TIME_MILLIS), is(true));
assertThat(
"Total time for the server to process all calls should have been less than calling them serially",
totalTimeForAllCalls < (numSimultaneousCalls * SLEEP_TIME_MILLIS),
is(true)
);

// Additionally, if the netty worker thread is supposed to be different than the executor thread then make sure only one worker thread was ever used.
if (expectNettyWorkerThreadToBeDifferentThanExecutor)
Expand Down
54 changes: 52 additions & 2 deletions ...c/test/java/com/nike/riposte/server/componenttest/VerifyPayloadHandlingComponentTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
import static org.assertj.core.api.Assertions.assertThat;

/**
* Verifies that request payloads are automatically deserialized correctly, and that response payloads are serialized (if appropriate) and sent down the wire
* correctly.
* Verifies that request payloads are automatically deserialized correctly, and that response payloads are serialized
* (if appropriate) and sent down the wire correctly.
*
* @author Nic Munroe
*/
Expand Down Expand Up @@ -199,6 +199,31 @@ public void verify_request_payload_received_for_string_input_type() {
assertThat(responsePayload).isEqualTo("success_string");
}

@Test
public void verify_request_payload_received_for_byte_array_input_type() {
String requestPayload = UUID.randomUUID().toString();
byte[] payloadBytes = requestPayload.getBytes(CharsetUtil.UTF_8);
String payloadHash = getHashForPayload(payloadBytes);

ExtractableResponse response =
given()
.baseUri("http://127.0.0.1")
.port(serverConfig.endpointsPort())
.basePath(ByteArrayTypeDeserializer.MATCHING_PATH)
.header(REQUEST_PAYLOAD_HASH_HEADER_KEY, payloadHash)
.body(requestPayload)
.log().all()
.when()
.post()
.then()
.log().all()
.statusCode(200)
.extract();

String responsePayload = response.asString();
assertThat(responsePayload).isEqualTo("success_string");
}

@Test
public void verify_request_payload_received_for_widget_input_type() throws JsonProcessingException {
SerializableObject widget = new SerializableObject(UUID.randomUUID().toString(), generateRandomBytes(32));
Expand Down Expand Up @@ -291,6 +316,7 @@ public PayloadTypeHandlingTestConfig(int downstreamPortNonSsl, int downstreamPor
new SerializableObjectPayloadReturner(),
new VoidTypeDeserializer(),
new StringTypeDeserializer(),
new ByteArrayTypeDeserializer(),
new WidgetTypeDeserializer(),
new DownstreamProxyNonSsl(downstreamPortNonSsl),
new DownstreamProxySsl(downstreamPortSsl)
Expand Down Expand Up @@ -513,6 +539,30 @@ public Matcher requestMatcher() {
}
}

public static class ByteArrayTypeDeserializer extends StandardEndpoint<byte[], String> {

public static final String MATCHING_PATH = "/byteArrayDeserializer";

@Override
public CompletableFuture<ResponseInfo<String>> execute(RequestInfo<byte[]> request, Executor longRunningTaskExecutor, ChannelHandlerContext ctx) {
if (!request.getContent().equals(request.getRawContentBytes())) {
throw new IllegalStateException(
"Since the deserialized type is byte[], getContent() should return the same thing as "
+ "getRawContentBytes(). getContent(): " + request.getContent() + " - getRawContentBytes(): "
+ request.getRawContentBytes());
}

verifyIncomingPayloadByteHash(request, true);

return CompletableFuture.completedFuture(ResponseInfo.newBuilder("success_string").build());
}

@Override
public Matcher requestMatcher() {
return Matcher.match(MATCHING_PATH);
}
}

public static class WidgetTypeDeserializer extends StandardEndpoint<SerializableObject, String> {

public static final String MATCHING_PATH = "/widgetDeserializer";
Expand Down
74 changes: 51 additions & 23 deletions ...topper/handler/riposte/listener/impl/BackstopperRiposteFrameworkErrorHandlerListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.nike.backstopper.apierror.ApiErrorWithMetadata;
import com.nike.backstopper.apierror.SortedApiErrorSet;
import com.nike.backstopper.apierror.projectspecificinfo.ProjectApiErrors;
import com.nike.backstopper.handler.ApiExceptionHandlerUtils;
import com.nike.backstopper.handler.listener.ApiExceptionHandlerListener;
import com.nike.backstopper.handler.listener.ApiExceptionHandlerListenerResult;
import com.nike.fastbreak.exception.CircuitBreakerException;
Expand All @@ -20,13 +21,13 @@
import com.nike.riposte.server.error.exception.InvalidCharsetInContentTypeHeaderException;
import com.nike.riposte.server.error.exception.InvalidHttpRequestException;
import com.nike.riposte.server.error.exception.MethodNotAllowed405Exception;
import com.nike.riposte.server.error.exception.MissingRequiredContentException;
import com.nike.riposte.server.error.exception.MultipleMatchingEndpointsException;
import com.nike.riposte.server.error.exception.NativeIoExceptionWrapper;
import com.nike.riposte.server.error.exception.NonblockingEndpointCompletableFutureTimedOut;
import com.nike.riposte.server.error.exception.PathNotFound404Exception;
import com.nike.riposte.server.error.exception.PathParameterMatchingException;
import com.nike.riposte.server.error.exception.RequestContentDeserializationException;
import com.nike.riposte.server.error.exception.MissingRequiredContentException;
import com.nike.riposte.server.error.exception.RequestTooBigException;
import com.nike.riposte.server.error.exception.TooManyOpenChannelsException;
import com.nike.riposte.server.error.exception.Unauthorized401Exception;
Expand Down Expand Up @@ -125,9 +126,9 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {
: projectApiErrors.getMalformedRequestApiError();
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(errorToUse),
Arrays.asList(
Pair.of("decoder_exception", "true"),
Pair.of("decoder_exception_message", ex.getMessage())
withBaseExceptionMessage(
ex,
Pair.of("decoder_exception", "true")
)
);
}
Expand All @@ -141,22 +142,24 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {
);
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(errorToUse),
Arrays.asList(
Pair.of("decoder_exception", "true"),
Pair.of("decoder_exception_message", ex.getMessage())
withBaseExceptionMessage(
ex,
Pair.of("decoder_exception", "true")
)
);
}

if (ex instanceof HostnameResolutionException) {
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(projectApiErrors.getTemporaryServiceProblemApiError())
singletonError(projectApiErrors.getTemporaryServiceProblemApiError()),
withBaseExceptionMessage(ex)
);
}

if (ex instanceof NativeIoExceptionWrapper) {
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(projectApiErrors.getTemporaryServiceProblemApiError())
singletonError(projectApiErrors.getTemporaryServiceProblemApiError()),
singletonList(causeDetailsForLogs(ex))
);
}

Expand All @@ -167,7 +170,8 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {
Arrays.asList(
Pair.of("method", theEx.httpMethod),
Pair.of("request_path", theEx.requestPath),
Pair.of("desired_object_type", theEx.desiredObjectType.getType().toString())
Pair.of("desired_object_type", theEx.desiredObjectType.getType().toString()),
causeDetailsForLogs(ex)
)
);
}
Expand All @@ -189,10 +193,11 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {

if (ex instanceof Unauthorized401Exception) {
Unauthorized401Exception theEx = (Unauthorized401Exception) ex;
List<Pair<String, String>> extraDetails = new ArrayList<>();
extraDetails.add(Pair.of("message", ex.getMessage()));
extraDetails.add(Pair.of("incoming_request_path", theEx.requestPath));
extraDetails.add(Pair.of("authorization_header", theEx.authorizationHeader));
List<Pair<String, String>> extraDetails = withBaseExceptionMessage(
ex,
Pair.of("incoming_request_path", theEx.requestPath),
Pair.of("authorization_header", theEx.authorizationHeader)
);
extraDetails.addAll((theEx).extraDetailsForLogging);
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(projectApiErrors.getUnauthorizedApiError()),
Expand All @@ -202,10 +207,11 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {

if (ex instanceof Forbidden403Exception) {
Forbidden403Exception theEx = (Forbidden403Exception) ex;
List<Pair<String, String>> extraDetails = new ArrayList<>();
extraDetails.add(Pair.of("message", ex.getMessage()));
extraDetails.add(Pair.of("incoming_request_path", theEx.requestPath));
extraDetails.add(Pair.of("authorization_header", theEx.authorizationHeader));
List<Pair<String, String>> extraDetails = withBaseExceptionMessage(
ex,
Pair.of("incoming_request_path", theEx.requestPath),
Pair.of("authorization_header", theEx.authorizationHeader)
);
extraDetails.addAll((theEx).extraDetailsForLogging);
return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(projectApiErrors.getForbiddenApiError()),
Expand Down Expand Up @@ -271,15 +277,16 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {
new ApiErrorWithMetadata(projectApiErrors.getMalformedRequestApiError(),
Pair.of("cause", "Unfinished/invalid HTTP request"))
),
Arrays.asList(Pair.of("incomplete_http_call_timeout_millis", String.valueOf(theEx.timeoutMillis)),
Pair.of("exception_message", theEx.getMessage()))
withBaseExceptionMessage(
ex,
Pair.of("incomplete_http_call_timeout_millis", String.valueOf(theEx.timeoutMillis))
)
);
}

if (ex instanceof InvalidHttpRequestException) {
InvalidHttpRequestException theEx = (InvalidHttpRequestException)ex;
Throwable cause = theEx.getCause();
String causeAsString = cause == null ? "null" : cause.toString();

ApiError apiErrorToUse = (cause instanceof TooLongFrameException)
? generateTooLongFrameApiError()
Expand All @@ -288,14 +295,35 @@ public ApiExceptionHandlerListenerResult shouldHandleException(Throwable ex) {

return ApiExceptionHandlerListenerResult.handleResponse(
singletonError(apiErrorToUse),
Arrays.asList(Pair.of("exception_message", theEx.getMessage()),
Pair.of("cause_details", causeAsString))
withBaseExceptionMessage(
ex,
causeDetailsForLogs(theEx)
)
);
}

return ApiExceptionHandlerListenerResult.ignoreResponse();
}

@SafeVarargs
protected final List<Pair<String, String>> withBaseExceptionMessage(
Throwable ex, Pair<String, String>... extraLogMessages
) {
List<Pair<String, String>> logPairs = new ArrayList<>();
ApiExceptionHandlerUtils.DEFAULT_IMPL.addBaseExceptionMessageToExtraDetailsForLogging(ex, logPairs);
if (extraLogMessages != null) {
logPairs.addAll(Arrays.asList(extraLogMessages));
}
return logPairs;
}

protected final Pair<String, String> causeDetailsForLogs(Throwable orig) {
Throwable cause = orig.getCause();
String causeDetails = (cause == null) ? "NO_CAUSE" : cause.toString();
return Pair.of("exception_cause_details",
ApiExceptionHandlerUtils.DEFAULT_IMPL.quotesToApostrophes(causeDetails));
}

protected ApiError generateTooLongFrameApiError() {
return new ApiErrorWithMetadata(projectApiErrors.getMalformedRequestApiError(),
Pair.of("cause", TOO_LONG_FRAME_METADATA_MESSAGE));
Expand Down
5 changes: 5 additions & 0 deletions riposte-spi/src/main/java/com/nike/riposte/server/http/impl/RequestInfoImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,11 @@ protected T deserializeContent() {
Type inputType = contentDeserializerTypeReference.getType();
if (inputType instanceof Class) {
Class inputTypeClass = (Class) inputType;
// If they want a raw byte[] then return getRawContentBytes().
if (byte[].class.equals(inputTypeClass)) {
return (T) getRawContentBytes();
}

// If they want a String or CharSequence then return the getRawContent() string.
if (String.class.equals(inputTypeClass) || CharSequence.class.equals(inputTypeClass)) {
//noinspection unchecked
Expand Down
14 changes: 7 additions & 7 deletions ...er/handler/riposte/listener/impl/BackstopperRiposteFrameworkErrorHandlerListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,8 @@ public void shouldHandleInvalidHttpRequestExceptionWithNullCause() {
assertThat(result.extraDetailsForLogging.get(0).getLeft()).isEqualTo("exception_message");
assertThat(result.extraDetailsForLogging.get(0).getRight()).isEqualTo("message");

assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("cause_details");
assertThat(result.extraDetailsForLogging.get(1).getRight()).isEqualTo("null");
assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("exception_cause_details");
assertThat(result.extraDetailsForLogging.get(1).getRight()).isEqualTo("NO_CAUSE");
}

@DataProvider(value = {
Expand Down Expand Up @@ -258,7 +258,7 @@ public void shouldHandleInvalidHttpRequestExceptionWithNonNullCause(boolean useT
assertThat(result.extraDetailsForLogging.get(0).getLeft()).isEqualTo("exception_message");
assertThat(result.extraDetailsForLogging.get(0).getRight()).isEqualTo(outerExceptionMessage);

assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("cause_details");
assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("exception_cause_details");
assertThat(result.extraDetailsForLogging.get(1).getRight()).isEqualTo(cause.toString());
}

Expand All @@ -276,11 +276,11 @@ public void should_handle_RequestTooBigException() {
assertThat(result.errors).isEqualTo(singletonError(testProjectApiErrors.getMalformedRequestApiError()));
assertThat(result.errors.first().getMetadata().get("cause")).isEqualTo("The request exceeded the maximum payload size allowed");

assertThat(result.extraDetailsForLogging.get(0).getLeft()).isEqualTo("decoder_exception");
assertThat(result.extraDetailsForLogging.get(0).getRight()).isEqualTo("true");
assertThat(result.extraDetailsForLogging.get(0).getLeft()).isEqualTo("exception_message");
assertThat(result.extraDetailsForLogging.get(0).getRight()).isEqualTo(exMsg);

assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("decoder_exception_message");
assertThat(result.extraDetailsForLogging.get(1).getRight()).isEqualTo(exMsg);
assertThat(result.extraDetailsForLogging.get(1).getLeft()).isEqualTo("decoder_exception");
assertThat(result.extraDetailsForLogging.get(1).getRight()).isEqualTo("true");
}

@Test
Expand Down

0 comments on commit 052006c

Please sign in to comment.