Skip to content

Commit

Permalink
Initial sketch of a solution for zalando#172. We do not have a good w…
Browse files Browse the repository at this point in the history
…ay of testing against 2.5 in the build yet, suggestions welcome :)
  • Loading branch information
Team Mongoose committed Aug 22, 2017
1 parent 2497ce3 commit fbf1686
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 24 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package org.zalando.logbook.servlet;

import javax.servlet.http.HttpServletRequest;
import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;
import java.util.Objects;
import java.util.Optional;

/**
* Reflection here is used to support both Servlet API 2.5 and 3.0 in the same code base.
*/
public class AsyncHelper {

public static boolean isFirstRequest(HttpServletRequest request) {
return dispatcherTypeClass()
.map(theClass -> isAsyncDispatcherType(request, theClass))
.orElse(true);
}

public static boolean isLastRequest(final HttpServletRequest request) {
return dispatcherTypeClass().isPresent() && !isAsyncStarted(request);
}

public static void setDispatcherTypeAsync(final HttpServletRequest request) {
dispatcherTypeClass().ifPresent(dispatcherTypeClass -> {
Object asyncDispatcherType = enumConstant(dispatcherTypeClass, "ASYNC");
invoke(request, "setDispatcherType", void.class, new Object[]{asyncDispatcherType}, dispatcherTypeClass);
});
}

private static boolean isAsyncDispatcherType(HttpServletRequest request, Class<?> dispatcherTypeClass) {
Object dispatcherType = invoke(request, "getDispatcherType", Object.class, new Object[]{});
Object asyncDispatcherType = enumConstant(dispatcherTypeClass, "ASYNC");
return !Objects.equals(dispatcherType, asyncDispatcherType);
}

private static boolean isAsyncStarted(HttpServletRequest request) {
return invoke(request, "isAsyncStarted", Boolean.class, new Object[]{});
}

private static <T> T invoke(HttpServletRequest request, String methodName, Class<T> returnType, Object[] arguments, Class<?>... parameterTypes) {
try {
return returnType.cast(request.getClass().getMethod(methodName, parameterTypes).invoke(request, arguments));
} catch (IllegalAccessException | InvocationTargetException | NoSuchMethodException e) {
throw new IllegalStateException(e);
}
}

private static Object enumConstant(Class<?> dispatcherType, String name) {
return Arrays.stream(dispatcherType.getEnumConstants())
.filter(constant -> constant.toString().equals(name))
.findFirst()
.orElseThrow(() -> new IllegalStateException("Could not find DispatcherType." + name));
}

private static Optional<Class<?>> dispatcherTypeClass() {
try {
return Optional.of(Class.forName("javax.servlet.DispatcherType"));
} catch (ClassNotFoundException e) {
return Optional.empty();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,20 @@
package org.zalando.logbook.servlet;

import org.zalando.logbook.HttpResponse;
import org.zalando.logbook.Origin;
import org.zalando.logbook.RawHttpResponse;

import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.nio.charset.Charset;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.*;
import java.util.function.Supplier;
import javax.servlet.ServletOutputStream;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;

import org.zalando.logbook.HttpResponse;
import org.zalando.logbook.Origin;
import org.zalando.logbook.RawHttpResponse;

import static java.nio.charset.StandardCharsets.UTF_8;

Expand All @@ -27,6 +25,16 @@ final class LocalResponse extends HttpServletResponseWrapper implements RawHttpR
private boolean withBody;
private Tee tee;

/**
* All the code connected to this field is to allow for compatibility with Servlet API 2.5
*/
private int status = 200;

/**
* All the code connected to this field is to allow for compatibility with Servlet API 2.5
*/
private Map<String, List<String>> headers = new HashMap<>();

LocalResponse(final HttpServletResponse response, final String protocolVersion) throws IOException {
super(response);
this.protocolVersion = protocolVersion;
Expand All @@ -46,11 +54,9 @@ public String getProtocolVersion() {
@Override
public Map<String, List<String>> getHeaders() {
final HeadersBuilder builder = new HeadersBuilder();

for (final String header : getHeaderNames()) {
builder.put(header, getHeaders(header));
for (final String header : headers.keySet()) {
builder.put(header, headers.get(header));
}

return builder.build();
}

Expand Down Expand Up @@ -108,6 +114,77 @@ private Tee tee() throws IOException {
return tee;
}

@Override
public int getStatus() {
return status;
}

@Override
public void setStatus(int status) {
this.status = status;
super.setStatus(status);
}

@Override
public void sendError(int status) throws IOException {
this.status = status;
super.sendError(status);
}

@Override
public void sendError(int status, String msg) throws IOException {
this.status = status;
super.sendError(status, msg);
}

@Override
public void addDateHeader(String name, long date) {
super.addDateHeader(name, date);
addMultiMapHeader(name, new Date(date).toString()); // TODO: this is dodgy, but in 2.5 we do not have super.getHeader
}

@Override
public void addHeader(String name, String value) {
super.addHeader(name, value);
addMultiMapHeader(name, value);
}

@Override
public void addIntHeader(String name, int value) {
super.addIntHeader(name, value);
addMultiMapHeader(name, String.valueOf(value));
}

@Override
public void setHeader(String name, String value) {
super.setHeader(name, value);
setMultiMapHeader(name, value);
}

@Override
public void setDateHeader(String name, long date) {
super.setDateHeader(name, date);
setMultiMapHeader(name, new Date(date).toString()); // TODO: this is dodgy, but in 2.5 we do not have super.getHeader
}

@Override
public void setIntHeader(String name, int value) {
super.setIntHeader(name, value);
setMultiMapHeader(name, String.valueOf(value));
}

private void addMultiMapHeader(String name, String value) {
List<String> values = headers.computeIfAbsent(name, key -> new ArrayList<>());
values.add(value);
}

private void setMultiMapHeader(String name, String value) {
headers.remove(name);
ArrayList<String> values = new ArrayList<>();
values.add(value);
headers.put(name, values);
}

private static class Tee {

private final ByteArrayOutputStream branch;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@
import java.util.Optional;
import java.util.function.Consumer;

import static javax.servlet.RequestDispatcher.ERROR_EXCEPTION_TYPE;
import static org.zalando.logbook.servlet.Attributes.CORRELATOR;

final class NormalStrategy implements Strategy {

/**
* Do not use RequestDispatcher.ERROR_EXCEPTION_TYPE, since Servlet API 2.5 does not have this constant.
*/
private static final String JAVAX_SERVLET_ERROR_EXCEPTION_TYPE = "javax.servlet.error.exception_type";

private final RawRequestFilter filter;

NormalStrategy(final RawRequestFilter filter) {
Expand Down Expand Up @@ -55,7 +59,7 @@ private Optional<Correlator> logRequestIfNecessary(final Logbook logbook,
}

private RawHttpRequest skipBodyIfErrorDispatch(final RemoteRequest request) {
if (request.getAttribute(ERROR_EXCEPTION_TYPE) == null) {
if (request.getAttribute(JAVAX_SERVLET_ERROR_EXCEPTION_TYPE) == null) {
return request;
}
return filter.filter(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public String getQuery() {
return Optional.ofNullable(getQueryString()).orElse("");
}

@SuppressWarnings("unchecked") // warnings appear when using Servlet API 2.5
@Override
public Map<String, List<String>> getHeaders() {
final HeadersBuilder builder = new HeadersBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void doFilter(final Logbook logbook, final HttpServletRequest httpRequest
}
}

private boolean isUnauthorized(final HttpServletResponse response) {
private boolean isUnauthorized(final LocalResponse response) {
return response.getStatus() == 401;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import org.zalando.logbook.Logbook;

import javax.servlet.DispatcherType;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -20,13 +19,11 @@ void doFilter(final Logbook logbook, final HttpServletRequest httpRequest, final
final FilterChain chain) throws ServletException, IOException;

default boolean isFirstRequest(final HttpServletRequest request) {
return request.getDispatcherType() != DispatcherType.ASYNC;
return AsyncHelper.isFirstRequest(request);
}

default boolean isLastRequest(final HttpServletRequest request) {
return !request.isAsyncStarted();
return AsyncHelper.isLastRequest(request);
}


}

Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import java.io.IOException;
import java.io.PrintWriter;

import static java.util.Collections.singletonList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertNotSame;
Expand Down Expand Up @@ -120,4 +122,86 @@ void shouldReturnNullContentTypeWhenNoContentTypeHasBeenSpecified() {

assertThat(unit.getContentType(), is(nullValue()));
}

@Test
void defaultStatusIs200() {
assertThat(unit.getStatus(), is(200));
}

@Test
void shouldSetStatus() {
unit.setStatus(300);

verify(mock).setStatus(300);
assertThat(unit.getStatus(), is(300));
}

@Test
void shouldSendErrorWithNoMessage() throws IOException {
unit.sendError(403);

verify(mock).sendError(403);
assertThat(unit.getStatus(), is(403));
}

@Test
void shouldSendErrorWithAMessage() throws IOException {
unit.sendError(403, "There is an error");

verify(mock).sendError(403, "There is an error");
assertThat(unit.getStatus(), is(403));
}

@Test
void shouldAddDateHeader() {
unit.addDateHeader("date", 111111);

verify(mock).addDateHeader("date", 111111);
assertThat(unit.getHeaders(), hasEntry("date", singletonList("Thu Jan 01 01:01:51 GMT 1970")));
}

@Test
void shouldAddHeader() {
unit.addHeader("example", "value");

verify(mock).addHeader("example", "value");
assertThat(unit.getHeaders(), hasEntry("example", singletonList("value")));
}

@Test
void shouldAddIntegerHeader() {
unit.addIntHeader("example", 123);

verify(mock).addIntHeader("example", 123);
assertThat(unit.getHeaders(), hasEntry("example", singletonList("123")));
}

@Test
void shouldSetHeader() {
unit.addHeader("example", "first value");
unit.setHeader("example", "Overwritten value");


verify(mock).setHeader("example", "Overwritten value");
assertThat(unit.getHeaders(), hasEntry("example", singletonList("Overwritten value")));
}

@Test
void shouldSetDateHeader() {
unit.addDateHeader("example", 111111);
unit.setDateHeader("example", 333333);

verify(mock).setDateHeader("example", 333333);
assertThat(unit.getHeaders(), hasEntry("example", singletonList("Thu Jan 01 01:05:33 GMT 1970")));
}

@Test
void shouldSetIntegerHeader() {
unit.addIntHeader("example", 111);
unit.setIntHeader("example", 4546);


verify(mock).setIntHeader("example", 4546);
assertThat(unit.getHeaders(), hasEntry("example", singletonList("4546")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.springframework.test.web.servlet.MvcResult;
import org.springframework.test.web.servlet.RequestBuilder;

import javax.servlet.DispatcherType;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.asyncDispatch;

Expand All @@ -16,7 +15,7 @@ static RequestBuilder async(final MvcResult result) {
return context -> {
final MockHttpServletRequest request = builder.buildRequest(context);
// this is missing in MockMvcRequestBuilders#asyncDispatch
request.setDispatcherType(DispatcherType.ASYNC);
AsyncHelper.setDispatcherTypeAsync(request);
return request;
};
}
Expand Down
4 changes: 4 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,10 @@
<goal>check</goal>
</goals>
<configuration>
<excludes>
<!-- This can never have full coverage in one run since some branches are dependent on the Servlet API on the classpath -->
<exclude>org/zalando/logbook/servlet/AsyncHelper*</exclude>
</excludes>
<rules>
<rule>
<element>CLASS</element>
Expand Down

0 comments on commit fbf1686

Please sign in to comment.