Merge pull request #183 from oxctl/AB#81207

AB#81207 Fix for calendar import failing with 504 errors.

Author: buckett

src/main/java/uk/ac/ox/ctl/RestTemplateConfiguration.java

@@ -21,8 +21,8 @@ public class RestTemplateConfiguration {
     public RestTemplate restTemplate(RestTemplateBuilder restTemplateBuilder)
     {
         RestTemplate restTemplate = restTemplateBuilder
-                .setConnectTimeout(Duration.ofSeconds(5))
-               .setReadTimeout(Duration.ofSeconds(5))
+                .setConnectTimeout(Duration.ofSeconds(10))
+                .setReadTimeout(Duration.ofSeconds(60))
                 .build();
 
         restTemplate.setErrorHandler(new ResponseErrorHandler() {

src/main/java/uk/ac/ox/ctl/canvasproxy/FilterConfig.java

@@ -0,0 +1,20 @@
+package uk.ac.ox.ctl.canvasproxy;
+
+import org.springframework.boot.web.servlet.FilterRegistrationBean;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
+public class FilterConfig {
+
+    @Bean
+    public FilterRegistrationBean<ViaFilter> viaFilter() {
+        FilterRegistrationBean<ViaFilter> registrationBean = new FilterRegistrationBean<>();
+
+        registrationBean.setFilter(new ViaFilter());
+        registrationBean.addUrlPatterns("/api/*"); // All the proxy URLs
+        registrationBean.setOrder(1); 
+
+        return registrationBean;
+    }
+}

src/main/java/uk/ac/ox/ctl/canvasproxy/ProxyController.java

@@ -90,7 +90,7 @@ public ResponseEntity<?> proxy(AbstractOAuth2TokenAuthenticationToken principal,
         } catch (ResourceAccessException e) {
             // When we get a timeout we should translate it to the correct HTTP message.
             if (e.getCause() instanceof SocketTimeoutException) {
-                throw new ResponseStatusException(HttpStatus.GATEWAY_TIMEOUT);
+                throw new ResponseStatusException(HttpStatus.GATEWAY_TIMEOUT, "Socket timeout accessing " + thirdPartyApi, e);
             }
             log.warn("Failed to load {} exception is: {}", thirdPartyApi, e.getMessage());
             // TODO - Need to fix this so we don't log exception as this will be expected in production

src/main/java/uk/ac/ox/ctl/canvasproxy/ViaFilter.java

@@ -0,0 +1,31 @@
+package uk.ac.ox.ctl.canvasproxy;
+
+import jakarta.servlet.Filter;
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+
+import java.io.IOException;
+
+/**
+ * Adds on a Via header to indicate that the request passed through this proxy.
+ * This allows for easier debugging of requests. This is done in a filter as it's dealing with low-level concerns and
+ * doesn't fit well in the controller.
+ */
+public class ViaFilter implements Filter {
+    @Override
+    public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
+        if (servletRequest instanceof HttpServletRequest request && servletResponse instanceof HttpServletResponse response) {
+            String existingVia = response.getHeader("Via");
+            String newVia = request.getProtocol() + " " + request.getServerName() + " tool-support";
+            if (existingVia != null && !existingVia.isEmpty()) {
+                newVia = existingVia + ", " + newVia;
+            }
+            response.setHeader("Via", newVia);
+        }
+        filterChain.doFilter(servletRequest, servletResponse);
+    }
+}

src/test/java/uk/ac/ox/ctl/canvasproxy/ViaFilterTest.java

@@ -0,0 +1,65 @@
+package uk.ac.ox.ctl.canvasproxy;
+
+import jakarta.servlet.FilterChain;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+import static org.mockito.Mockito.*;
+
+class ViaFilterTest {
+
+    private ViaFilter filter;
+    private HttpServletRequest request;
+    private HttpServletResponse response;
+    private FilterChain chain;
+
+    @BeforeEach
+    void setUp() {
+        filter = new ViaFilter();
+        request = mock(HttpServletRequest.class);
+        response = mock(HttpServletResponse.class);
+        chain = mock(FilterChain.class);
+    }
+
+    @Test
+    void shouldAddViaHeaderWhenNoneExists() throws Exception {
+        when(request.getProtocol()).thenReturn("HTTP/1.1");
+        when(request.getServerName()).thenReturn("localhost");
+        when(response.getHeader("Via")).thenReturn(null);
+
+        filter.doFilter(request, response, chain);
+
+        verify(response).setHeader("Via", "HTTP/1.1 localhost tool-support");
+        verify(chain).doFilter(request, response);
+    }
+
+    @Test
+    void shouldAppendToExistingViaHeader() throws Exception {
+        when(request.getProtocol()).thenReturn("HTTP/2");
+        when(request.getServerName()).thenReturn("myserver");
+        when(response.getHeader("Via")).thenReturn("HTTP/1.1 proxy1");
+
+        filter.doFilter(request, response, chain);
+
+        verify(response).setHeader("Via", "HTTP/1.1 proxy1, HTTP/2 myserver tool-support");
+        verify(chain).doFilter(request, response);
+    }
+
+    @Test
+    void shouldHandleNonHttpRequestsGracefully() throws IOException, jakarta.servlet.ServletException {
+        ServletRequest genericRequest = mock(ServletRequest.class);
+        ServletResponse genericResponse = mock(ServletResponse.class);
+
+        filter.doFilter(genericRequest, genericResponse, chain);
+
+        verify(chain).doFilter(genericRequest, genericResponse);
+        // no headers should be set
+        verifyNoInteractions(response);
+    }
+}