[tests] Stabilize the HTTP server's request log

The TestRequestLog added log entries only after a request had been
handled. But the response is already sent at that point, so the
following sequence was possible:

  client sends request A
  server thread T1 handles request A and sends back response
  client receives response
  client sends request B
  server thread T2 handles request B (and sends back response)
  server thread T2 logs B
  server thread T1 logs A

Fix this by logging events before handling a request, and then filling
in the response data after the request has been handled. This should
avoid such inversions, at least for tests using a single single-threaded
client. With multiple concurrent or multi-threaded clients, all bets
about the log order would be off anyway.

Bug: 528187
Change-Id: I99a46df17231fa7c4f75a8c37e2c14dc098c4e22
Signed-off-by: Thomas Wolf <twolf@apache.org>

2 files changed, 18 insertions, 12 deletions

diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AccessEvent.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AccessEvent.java
index 22e19979e6..873d430675 100644
--- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AccessEvent.java
+++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/AccessEvent.java
@@ -30,16 +30,18 @@ public class AccessEvent {
 
 	private final Map<String, String[]> parameters;
 
-	private final int status;
+	private int status;
 
-	private final Map<String, String> responseHeaders;
+	private Map<String, String> responseHeaders;
 
-	AccessEvent(Request req, Response rsp) {
+	AccessEvent(Request req) {
 		method = req.getMethod();
 		uri = req.getRequestURI();
 		requestHeaders = cloneHeaders(req);
 		parameters = clone(req.getParameterMap());
+	}
 
+	void setResponse(Response rsp) {
 		status = rsp.getStatus();
 		responseHeaders = cloneHeaders(rsp);
 	}
@@ -141,7 +143,7 @@ public class AccessEvent {
 	 * @return first value of the response header; null if not sent.
 	 */
 	public String getResponseHeader(String name) {
-		return responseHeaders.get(name);
+		return responseHeaders != null ? responseHeaders.get(name) : null;
 	}
 
 	/** {@inheritDoc} */
diff --git a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java
index a86edd2f39..04cb2428a2 100644
--- a/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java
+++ b/org.eclipse.jgit.junit.http/src/org/eclipse/jgit/junit/http/TestRequestLog.java
@@ -87,19 +87,23 @@ class TestRequestLog extends HandlerWrapper {
 				}
 			}
 
+			AccessEvent event = null;
+			if (DispatcherType.REQUEST
+					.equals(baseRequest.getDispatcherType())) {
+				event = new AccessEvent((Request) request);
+				synchronized (events) {
+					events.add(event);
+				}
+			}
+
 			super.handle(target, baseRequest, request, response);
 
-			if (DispatcherType.REQUEST.equals(baseRequest.getDispatcherType()))
-				log((Request) request, (Response) response);
+			if (event != null) {
+				event.setResponse((Response) response);
+			}
 
 		} finally {
 			active.release();
 		}
 	}
-
-	private void log(Request request, Response response) {
-		synchronized (events) {
-			events.add(new AccessEvent(request, response));
-		}
-	}
 }