Skip to content

Commit

Permalink
Issue #5684 Fix and re-enable ServletRequestLogTest. (#5877)
Browse files Browse the repository at this point in the history
Signed-off-by: Jan Bartel <janb@webtide.com>
  • Loading branch information
janbartel committed Jan 13, 2021
1 parent a5a8327 commit b99eb3f
Showing 1 changed file with 57 additions and 112 deletions.
Expand Up @@ -19,10 +19,7 @@
package org.eclipse.jetty.servlet;

import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.HttpURLConnection;
import java.net.URI;
import java.util.ArrayList;
Expand All @@ -39,6 +36,7 @@

import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.RequestLog;
import org.eclipse.jetty.server.Response;
Expand All @@ -48,24 +46,24 @@
import org.eclipse.jetty.server.handler.DefaultHandler;
import org.eclipse.jetty.server.handler.ErrorHandler;
import org.eclipse.jetty.server.handler.HandlerCollection;
import org.eclipse.jetty.toolchain.test.IO;
import org.eclipse.jetty.util.component.AbstractLifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.util.log.StacklessLogging;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import static java.time.Duration.ofSeconds;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;

/**
* Servlet equivalent of the jetty-server's RequestLogHandlerTest, but with more ErrorHandler twists.
*/
@Disabled
public class ServletRequestLogTest
{
private static final Logger LOG = Log.getLogger(ServletRequestLogTest.class);
Expand Down Expand Up @@ -109,7 +107,7 @@ private static class ResponseSendErrorServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
response.sendError(500, "Whoops");
response.sendError(500, "FromResponseSendErrorServlet");
}
}

Expand All @@ -119,7 +117,7 @@ private static class ServletExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new ServletException("Whoops");
throw new ServletException("FromServletExceptionServlet");
}
}

Expand All @@ -129,7 +127,7 @@ private static class IOExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new IOException("Whoops");
throw new IOException("FromIOExceptionServlet");
}
}

Expand All @@ -139,7 +137,7 @@ private static class RuntimeExceptionServlet extends AbstractTestServlet
@Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
{
throw new RuntimeException("Whoops");
throw new RuntimeException("FromRuntimeExceptionServlet");
}
}

Expand Down Expand Up @@ -273,7 +271,6 @@ public static Stream<Arguments> data()
data.add(new Object[]{new HelloServlet(), "/test/", "GET /test/ HTTP/1.1 200"});
data.add(new Object[]{new AsyncOnTimeoutCompleteServlet(), "/test/", "GET /test/ HTTP/1.1 200"});
data.add(new Object[]{new AsyncOnTimeoutDispatchServlet(), "/test/", "GET /test/ HTTP/1.1 200"});

data.add(new Object[]{new AsyncOnStartIOExceptionServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
data.add(new Object[]{new ResponseSendErrorServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
data.add(new Object[]{new ServletExceptionServlet(), "/test/", "GET /test/ HTTP/1.1 500"});
Expand Down Expand Up @@ -323,46 +320,40 @@ public void testLogHandlerCollection(Servlet testServlet, String requestPath, St

// Add the test servlet
ServletHolder testHolder = new ServletHolder(testServlet);
app.addServlet(testHolder, "/test");
app.addServlet(testHolder, "/test/*");

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{
server.start();

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}

int port = connector.getLocalPort();

URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -413,47 +404,40 @@ public void testLogHandlerCollectionErrorHandlerServerBean(Servlet testServlet,

// Add the test servlet
ServletHolder testHolder = new ServletHolder(testServlet);
app.addServlet(testHolder, "/test");
app.addServlet(testHolder, "/test/*");

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -509,45 +493,38 @@ public void testLogHandlerCollectionSimpleErrorPageMapping(Servlet testServlet,
errorMapper.addErrorPage(500, "/errorpage");
app.setErrorHandler(errorMapper);

try
try (StacklessLogging scope = new StacklessLogging(HttpChannel.class))
{
server.start();

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{

connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, requestPath, null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.debug("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.debug("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand Down Expand Up @@ -608,38 +585,32 @@ public void testLogHandlerWrapped(Servlet testServlet, String requestPath, Strin

Assertions.assertTimeoutPreemptively(ofSeconds(4), () ->
{
connector.addBean(new HttpChannel.Listener()
{
@Override
public void onComplete(Request request)
{
assertRequestLog(expectedLogEntry, captureLog);
}
});

String host = connector.getHost();
if (host == null)
{
host = "localhost";
}
int port = connector.getLocalPort();

int port = connector.getLocalPort();
URI serverUri = new URI("http", null, host, port, "/test", null, null);

// Make call to test handler
HttpURLConnection connection = (HttpURLConnection)serverUri.toURL().openConnection();
try
{
connection.setAllowUserInteraction(false);

// log response status code
int statusCode = connection.getResponseCode();
LOG.info("Response Status Code: {}", statusCode);

if (statusCode == 200)
{
// collect response message and log it
String content = getResponseContent(connection);
LOG.info("Response Content: {}", content);
}
}
finally
{
connection.disconnect();
}

assertRequestLog(expectedLogEntry, captureLog);
});
}
finally
Expand All @@ -650,33 +621,7 @@ public void testLogHandlerWrapped(Servlet testServlet, String requestPath, Strin

private void assertRequestLog(final String expectedLogEntry, CaptureLog captureLog)
{
int captureCount = captureLog.captured.size();

if (captureCount != 1)
{
LOG.warn("Capture Log size is {}, expected to be 1", captureCount);
if (captureCount > 1)
{
for (int i = 0; i < captureCount; i++)
{
LOG.warn("[{}] {}", i, captureLog.captured.get(i));
}
}
assertThat("Capture Log Entry Count", captureLog.captured.size(), is(1));
}

String actual = captureLog.captured.get(0);
assertThat("Capture Log", actual, is(expectedLogEntry));
}

private String getResponseContent(HttpURLConnection connection) throws IOException
{
try (InputStream in = connection.getInputStream();
InputStreamReader reader = new InputStreamReader(in))
{
StringWriter writer = new StringWriter();
IO.copy(reader, writer);
return writer.toString();
}
assertThat("Request log size", captureLog.captured, not(empty()));
assertThat("Request log entry",captureLog.captured.get(0), is(expectedLogEntry));
}
}

0 comments on commit b99eb3f

Please sign in to comment.