Merge branch 'hr/request-id' into 'master'

chore: unify the generation of the request id + return it in the response

See merge request !305

autograde-service/pom.xml

@@ -6,7 +6,7 @@
 	<parent>
 		<groupId>org.springframework.boot</groupId>
 		<artifactId>spring-boot-starter-parent</artifactId>
-		<version>3.1.1</version>
+		<version>3.3.4</version>
 		<relativePath/> <!-- lookup parent from repository -->
 	</parent>
 

autograde-service/src/main/java/ch/epfl/autograde/config/FiltersConfig.java

+package ch.epfl.autograde.config;
+
+import ch.epfl.autograde.filters.AssignRequestIdFilter;
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.core.Ordered;
+
+/**
+ *
+ * @author Hamza REMMAL (hamza.remmal@epfl.ch)
+ * @since 1.3.0
+ */
+@Configuration
+public class FiltersConfig {
+
+  @Bean
+  public FilterRegistrationBean<?> requestIdFilterRegistration(AssignRequestIdFilter filter) {
+    final var registration = new FilterRegistrationBean<>();
+    registration.setFilter(filter);
+    // We should generate a request id only for api endpoints
+    registration.addUrlPatterns("/api/v1/*");
+    // Ensure that all the requests will have a request id by running the filter first
+    registration.setOrder(Ordered.HIGHEST_PRECEDENCE);
+    return registration;
+  }
+
+}

autograde-service/src/main/java/ch/epfl/autograde/filters/AssignRequestIdFilter.java

+package ch.epfl.autograde.filters;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import lombok.RequiredArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.slf4j.MDC;
+import org.springframework.stereotype.Component;
+import org.springframework.web.filter.OncePerRequestFilter;
+
+import java.io.IOException;
+import java.util.UUID;
+
+/**
+ * A filter that assigns a unique Request ID to every incoming HTTP request.
+ *
+ * <p>
+ * This filter generates a unique {@code Request ID} for each HTTP request and ensures it is:
+ * <ul>
+ *     <li>Added to the request attributes, making it accessible to downstream components.</li>
+ *     <li>Included as a header in the HTTP response ({@code X-Request-Id}).</li>
+ *     <li>Logged in the Mapped Diagnostic Context (MDC) for consistent tracking in logs.</li>
+ * </ul>
+ * This filter is designed to run once per request and should be one of the earliest filters
+ * in the filter chain.
+ * </p>
+ *
+ * @author Hamza REMMAL (hamza.remmal@epfl.ch)
+ * @since 1.3.0
+ * @see ch.epfl.autograde.config.FiltersConfig#requestIdFilterRegistration(AssignRequestIdFilter)
+ * @see org.springframework.web.filter.OncePerRequestFilter
+ */
+@Slf4j
+@Component
+@RequiredArgsConstructor
+public final class AssignRequestIdFilter extends OncePerRequestFilter {
+
+  public static final String REQUEST_ID_HEADER = "X-Request-Id";
+
+  @Override
+  protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws ServletException, IOException {
+    // Generate a unique Request ID
+    // TODO: Extract the UUID to be a distributed service to have a real UUID and not an UUID per node
+    final var requestId = UUID.randomUUID().toString();
+    request.setAttribute(REQUEST_ID_HEADER, requestId);
+    response.setHeader(REQUEST_ID_HEADER, requestId);
+    MDC.put(REQUEST_ID_HEADER, requestId);
+    log.trace("Generated Request-Id '{}' for request from '{}'", requestId, request.getRemoteAddr());
+    chain.doFilter(request, response);
+  }
+
+}

autograde-service/src/main/java/ch/epfl/autograde/logging/MdcInterceptor.java

-package ch.epfl.autograde.logging;
-
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-import org.slf4j.MDC;
-import org.springframework.stereotype.Component;
-import org.springframework.web.servlet.HandlerInterceptor;
-
-import java.util.UUID;
-
-/**
- * Interceptor that adds a unique request id to the MDC,
- * so that it can be used in the logs to track a request through the system.
- *
- * @implNote To work with async requests, you need to create a TaskDecorator that copies the MDC to the new thread.
- */
-@Component
-public class MdcInterceptor implements HandlerInterceptor {
-
-    @Override
-    public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) {
-        MDC.put("requestId", UUID.randomUUID().toString());
-        return true;
-    }
-
-    @Override
-    public void afterCompletion(HttpServletRequest request, HttpServletResponse response, Object handler, Exception ex) {
-        MDC.remove("requestId");
-    }
-}

autograde-service/src/main/java/ch/epfl/autograde/logging/WebMvcConfiguration.java

-package ch.epfl.autograde.logging;
-
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.stereotype.Component;
-import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
-import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
-
-@Component
-public class WebMvcConfiguration implements WebMvcConfigurer {
-
-    private final MdcInterceptor mdcInterceptor;
-
-    @Autowired
-    public WebMvcConfiguration(MdcInterceptor mdcInterceptor) {
-        this.mdcInterceptor = mdcInterceptor;
-    }
-
-    @Override
-    public void addInterceptors(InterceptorRegistry registry) {
-        registry.addInterceptor(mdcInterceptor);
-    }
-}

autograde-service/src/main/resources/logback-spring.xml

 <configuration>
     <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
         <encoder>
-            <pattern>%d{ISO8601} %X{requestId} [%thread] %-5level %logger{36} - %msg%n</pattern>
+            <pattern>%d{ISO8601} %X{X-Request-Id} [%thread] %-5level %logger{36} - %msg%n</pattern>
         </encoder>
     </appender>
 

autograde-service/src/test/java/ch/epfl/autograde/filters/AssignRequestIdFilterIntegrationTests.java

+package ch.epfl.autograde.filters;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
+import org.springframework.boot.test.context.SpringBootTest;
+import org.springframework.test.web.servlet.MockMvc;
+
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*;
+
+/**
+ *
+ * @author Hamza REMMAL (hamza.remmal@epfl.ch)
+ * @since 1.3.0
+ * @see ch.epfl.autograde.filters.AssignRequestIdFilter
+ */
+@AutoConfigureMockMvc
+@SpringBootTest
+final class AssignRequestIdFilterIntegrationTests {
+
+  @Autowired
+  private MockMvc mockMvc;
+
+  /**
+   * @implNote We call a fake-endpoint to test:
+   * <ul>
+   *   <li>The filter is called when calling an API Endpoint</li>
+   *   <li>The filter is executed before the authentication filters</li>
+   *   <li>The filter is called before dispatching to the controller</li>
+   * </ul>
+   */
+  @Test @DisplayName("Request to API endpoints should have a 'X-Request-Id' header")
+  void apiRequestShouldHaveARequestID() throws Exception {
+    mockMvc.perform(get("/api/v1/fake-endpoint"))
+            .andExpect(header().exists(AssignRequestIdFilter.REQUEST_ID_HEADER));
+  }
+
+    /**
+   * @implNote We call a fake-endpoint to test:
+   * <ul>
+   *   <li>The filter is not called when calling a non-API Endpoint</li>
+   *   <li>The filter is executed before the authentication filters</li>
+   *   <li>The filter is called before dispatching to the controller</li>
+   * </ul>
+   */
+  @Test @DisplayName("Request to non-API endpoints should not have a 'X-Request-Id' header")
+  void nonApiRequestShouldNotHaveARequestID() throws Exception {
+    mockMvc.perform(get("/not-api/v1/fake-endpoint"))
+            .andExpect(header().doesNotExist(AssignRequestIdFilter.REQUEST_ID_HEADER));
+  }
+
+}

autograde-service/src/test/java/ch/epfl/autograde/filters/AssignRequestIdFilterUnitTests.java

+package ch.epfl.autograde.filters;
+
+import org.junit.jupiter.api.*;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.*;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.slf4j.MDC;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+/**
+ *
+ * @author Hamza REMMAL (hamza.remmal@epfl.ch)
+ * @since 1.3.0
+ * @see ch.epfl.autograde.filters.AssignRequestIdFilter
+ */
+@DisplayName("AssignRequestIdFilter Unit Tests")
+@ExtendWith(MockitoExtension.class)
+final class AssignRequestIdFilterUnitTests {
+
+    private final AssignRequestIdFilter filter = new AssignRequestIdFilter();
+
+    @Test @DisplayName("Filter adds the 'X-Request-Id' attribute to the HttpServletRequest")
+    void filterSetsTheAttributeInTheRequest(
+            @Mock HttpServletRequest request,
+            @Mock HttpServletResponse response,
+            @Mock FilterChain chain
+    ) throws Exception {
+        filter.doFilter(request, response, chain);
+        verify(request).setAttribute(eq(AssignRequestIdFilter.REQUEST_ID_HEADER), notNull());
+    }
+
+    @Test @DisplayName("Filter adds the 'X-Request-Id' header to the HttpServletResponse")
+    void filterSetsTheHeaderInTheResponse(
+            @Mock HttpServletRequest request,
+            @Mock HttpServletResponse response,
+            @Mock FilterChain chain
+    ) throws Exception {
+        filter.doFilter(request, response, chain);
+        verify(response).setHeader(eq(AssignRequestIdFilter.REQUEST_ID_HEADER), notNull());
+    }
+
+    @Test @DisplayName("Filter adds the 'X-Request-Id' to the MDC")
+    void filterSetsMDCContext(
+            @Mock HttpServletRequest request,
+            @Mock HttpServletResponse response,
+            @Mock FilterChain chain
+    ) throws Exception {
+        filter.doFilter(request, response, chain);
+        assertNotNull(MDC.get(AssignRequestIdFilter.REQUEST_ID_HEADER));
+    }
+
+    @Test @DisplayName("Filter calls the rest of the FilterChain")
+    void filterCallsTheRestOfTheChain(
+            @Mock HttpServletRequest request,
+            @Mock HttpServletResponse response,
+            @Mock FilterChain chain
+    ) throws Exception {
+        filter.doFilter(request, response, chain);
+        verify(chain).doFilter(request, response);
+    }
+
+    @Test
+    @DisplayName("Filter generates a unique 'X-Request-Id'")
+    void filterUsesSameRequestIdAcrossAll(
+            @Mock HttpServletRequest request,
+            @Mock HttpServletResponse response,
+            @Mock FilterChain chain,
+            @Captor ArgumentCaptor<String> requestCaptor,
+            @Captor ArgumentCaptor<String> responseCaptor
+    ) throws Exception {
+        filter.doFilter(request, response, chain);
+
+        verify(request).setAttribute(eq(AssignRequestIdFilter.REQUEST_ID_HEADER), requestCaptor.capture());
+        verify(response).setHeader(eq(AssignRequestIdFilter.REQUEST_ID_HEADER), responseCaptor.capture());
+
+        final var requestAttrId = requestCaptor.getValue();
+        final var responseHeaderId = responseCaptor.getValue();
+        final var mdcRequestId = MDC.get(AssignRequestIdFilter.REQUEST_ID_HEADER);
+
+        assertAll(
+                () -> assertNotNull(requestAttrId),
+                () -> assertNotNull(responseHeaderId),
+                () -> assertNotNull(mdcRequestId),
+                () -> assertEquals(requestAttrId, responseHeaderId),
+                () -> assertEquals(requestAttrId, mdcRequestId)
+        );
+    }
+
+}