Pull request tracing out for sharing

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/RestfulServer.java

@@ -1277,7 +1277,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 	}
 
 	/**
-	 * Reads a requet ID from the request headers via the {@link Constants#HEADER_REQUEST_ID}
+	 * Reads a request ID from the request headers via the {@link Constants#HEADER_REQUEST_ID}
 	 * header, or generates one if none is supplied.
 	 * <p>
 	 * Note that the generated request ID is a random 64-bit long integer encoded as
@@ -1286,18 +1286,11 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
 	 * </p>
 	 */
 	protected String getOrCreateRequestId(HttpServletRequest theRequest) {
-		String requestId = theRequest.getHeader(Constants.HEADER_REQUEST_ID);
-		if (isNotBlank(requestId)) {
-			for (char nextChar : requestId.toCharArray()) {
-				if (!Character.isLetterOrDigit(nextChar)) {
-					if (nextChar != '.' && nextChar != '-' && nextChar != '_' && nextChar != ' ') {
-						requestId = null;
-						break;
-					}
-				}
-			}
-		}
+		String requestId = ServletRequestTracing.maybeGetRequestId(theRequest);
 
+		// TODO can we delete this and newRequestId()
+		//  and use ServletRequestTracing.getOrGenerateRequestId() instead?
+		//  newRequestId() is protected.  Do you think anyone actually overrode it?
 		if (isBlank(requestId)) {
 			int requestIdLength = Constants.REQUEST_ID_LENGTH;
 			requestId = newRequestId(requestIdLength);

hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/ServletRequestTracing.java

@@ -0,0 +1,65 @@
+package ca.uhn.fhir.rest.server;
+
+import ca.uhn.fhir.rest.api.Constants;
+import org.apache.commons.lang3.RandomStringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.annotation.Nullable;
+import javax.servlet.ServletRequest;
+import javax.servlet.http.HttpServletRequest;
+
+import static org.apache.commons.lang3.StringUtils.isBlank;
+import static org.apache.commons.lang3.StringUtils.isNotBlank;
+
+public class ServletRequestTracing {
+	private static final Logger ourLog = LoggerFactory.getLogger(ServletRequestTracing.class);
+	public static final String ATTRIBUTE_REQUEST_ID = ServletRequestTracing.class.getName() + '.' + Constants.HEADER_REQUEST_ID;
+
+	/**
+	 * Assign a tracing id to this request, using
+	 * the X-Request-ID if present and compatible.
+	 *
+	 * If none present, generate a 64 random alpha-numeric string that is not
+	 * cryptographically secure.
+	 *
+	 * @param theServletRequest the request to trace
+	 * @return the tracing id
+	 */
+	public static String getOrGenerateRequestId(ServletRequest theServletRequest) {
+		String requestId = maybeGetRequestId(theServletRequest);
+		if (isBlank(requestId)) {
+			requestId = RandomStringUtils.randomAlphanumeric(Constants.REQUEST_ID_LENGTH);
+		}
+
+		ourLog.debug("Assigned tracing id {}", requestId);
+
+		theServletRequest.setAttribute(ATTRIBUTE_REQUEST_ID, requestId);
+
+		return requestId;
+	}
+
+	@Nullable
+	public static String maybeGetRequestId(ServletRequest theServletRequest) {
+		// have we already seen this request?
+		String requestId = (String) theServletRequest.getAttribute(ATTRIBUTE_REQUEST_ID);
+
+		if (requestId == null && theServletRequest instanceof HttpServletRequest) {
+			// Also applies to non-FHIR (e.g. admin-json) requests).
+			HttpServletRequest request = (HttpServletRequest) theServletRequest;
+			requestId = request.getHeader(Constants.HEADER_REQUEST_ID);
+			if (isNotBlank(requestId)) {
+				for (char nextChar : requestId.toCharArray()) {
+					if (!Character.isLetterOrDigit(nextChar)) {
+						if (nextChar != '.' && nextChar != '-' && nextChar != '_' && nextChar != ' ') {
+							requestId = null;
+							break;
+						}
+					}
+				}
+			}
+		}
+		return requestId;
+	}
+
+}

hapi-fhir-server/src/test/java/ca/uhn/fhir/rest/server/ServletRequestTracingTest.java

@@ -0,0 +1,57 @@
+package ca.uhn.fhir.rest.server;
+
+import ca.uhn.fhir.rest.api.Constants;
+import org.junit.jupiter.api.Test;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.blankString;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class ServletRequestTracingTest {
+
+	MockHttpServletRequest myRequest = new MockHttpServletRequest();
+	String myRequestIdResult;
+
+	void run() {
+		myRequestIdResult = ServletRequestTracing.getOrGenerateRequestId(myRequest);
+	}
+
+	@Test
+	public void emptyRequestGetsGeneratedId() {
+		// no setup
+
+		run();
+
+		// verify
+		assertThat("id generated", myRequestIdResult, not(blankString()));
+		assertEquals(myRequest.getAttribute(ServletRequestTracing.ATTRIBUTE_REQUEST_ID),myRequestIdResult);
+	}
+
+	@Test
+	public void requestWithCallerHapiIdUsesThat() {
+		// setup
+		myRequest.addHeader(Constants.HEADER_REQUEST_ID, "a_request_id");
+
+		run();
+
+		// verify
+		assertEquals("a_request_id", myRequestIdResult);
+	}
+
+	@Test
+	public void duplicateCallsKeepsSameId() {
+		// no headers
+
+		myRequestIdResult = ServletRequestTracing.getOrGenerateRequestId(myRequest);
+
+		String secondResult = ServletRequestTracing.getOrGenerateRequestId(myRequest);
+
+		// verify
+		assertThat("id generated", secondResult, not(blankString()));
+		assertEquals(myRequestIdResult, secondResult);
+	}
+
+}