diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java index 7e3bed50..a9042629 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/service/das/DasService.java @@ -40,7 +40,7 @@ public DasService( * expansion (which would throw). Any path variables in {@code path} are supplied via * {@code pathVariables}. */ - private byte[] getFeatureCollection(String path, String start, String end, Map pathVariables) { + private ResponseEntity getFeatureCollection(String path, String start, String end, Map pathVariables) { UriComponentsBuilder builder = UriComponentsBuilder.fromUriString(dasProperties.host() + path); Map params = new HashMap<>(pathVariables); @@ -54,38 +54,38 @@ private byte[] getFeatureCollection(String path, String start, String end, Map getWaveBuoysBetweenDates(String start, String end) { return getFeatureCollection("/api/v1/das/data/feature-collection/wave-buoy", start, end, Map.of()); } - public byte[] getWaveBuoysLatestAvailableDate() { + public ResponseEntity getWaveBuoysLatestAvailableDate() { String waveBuoysUrlTemplate = UriComponentsBuilder.fromUriString(dasProperties.host() + "/api/v1/das/data/feature-collection/wave-buoy/latest") .encode() .toUriString(); - return httpClient.getForObject(waveBuoysUrlTemplate, byte[].class); + return httpClient.getForEntity(waveBuoysUrlTemplate, byte[].class); } - public byte[] getWaveBuoyDetailsBetweenDates(String startDateTime, String endDateTime, String buoy) { + public ResponseEntity getWaveBuoyDetailsBetweenDates(String startDateTime, String endDateTime, String buoy) { return getFeatureCollection("/api/v1/das/data/feature-collection/wave-buoy/{buoy}", startDateTime, endDateTime, Map.of("buoy", buoy)); } - public byte[] getMooringsBetweenDates(String start, String end) { + public ResponseEntity getMooringsBetweenDates(String start, String end) { return getFeatureCollection("/api/v1/das/data/feature-collection/mooring", start, end, Map.of()); } - public byte[] getMooringsLatestAvailableDate() { + public ResponseEntity getMooringsLatestAvailableDate() { String mooringsUrlTemplate = UriComponentsBuilder.fromUriString(dasProperties.host() + "/api/v1/das/data/feature-collection/mooring/latest") .encode() .toUriString(); - return httpClient.getForObject(mooringsUrlTemplate, byte[].class); + return httpClient.getForEntity(mooringsUrlTemplate, byte[].class); } - public byte[] getMooringDetailsBetweenDates(String startDateTime, String endDateTime, String mooring) { + public ResponseEntity getMooringDetailsBetweenDates(String startDateTime, String endDateTime, String mooring) { return getFeatureCollection("/api/v1/das/data/feature-collection/mooring/{mooring}", startDateTime, endDateTime, Map.of("mooring", mooring)); } diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java b/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java index fd26964f..c5df22da 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/features/RestServices.java @@ -33,6 +33,7 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; +import org.springframework.web.client.HttpStatusCodeException; import java.io.IOException; import java.io.StringReader; @@ -284,6 +285,29 @@ private Optional> validateUtcDateRange(String startDateTime, S return Optional.empty(); } + /** + * Wraps a DAS response for return to the client, preserving the upstream status, + * headers (e.g. cache/rate-limit headers), and body rather than just the body. + */ + private ResponseEntity forwardDasResponse(ResponseEntity dasResponse) { + return ResponseEntity + .status(dasResponse.getStatusCode()) + .headers(dasResponse.getHeaders()) + .body(dasResponse.getBody()); + } + + /** + * RestTemplate throws on any non-2xx DAS response rather than returning it, so the + * error status/headers/body have to be pulled off the exception to forward them to + * the client instead of collapsing every DAS error into a 500. + */ + private ResponseEntity forwardDasError(HttpStatusCodeException e) { + return ResponseEntity + .status(e.getStatusCode()) + .headers(e.getResponseHeaders()) + .body(e.getResponseBodyAsByteArray()); + } + /** * Returns wave buoy sites recorded within the given UTC date range. * @@ -296,11 +320,11 @@ public ResponseEntity getWaveBuoysBetweenDates(String startDateTime, String e } try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getWaveBuoysBetweenDates(startDateTime, endDateTime)); + return forwardDasResponse(dasService.getWaveBuoysBetweenDates(startDateTime, endDateTime)); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching wave buoys data: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching wave buoys data: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); @@ -325,11 +349,11 @@ public ResponseEntity getWaveBuoyDetailsBetweenDates(String startDateTime, St } try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getWaveBuoyDetailsBetweenDates(startDateTime, endDateTime, buoy)); + return forwardDasResponse(dasService.getWaveBuoyDetailsBetweenDates(startDateTime, endDateTime, buoy)); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching wave buoy historical data: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching wave buoy historical data: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); @@ -342,11 +366,11 @@ public ResponseEntity getWaveBuoyDetailsBetweenDates(String startDateTime, St */ public ResponseEntity getWaveBuoysLatestAvailableDate() { try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getWaveBuoysLatestAvailableDate()); + return forwardDasResponse(dasService.getWaveBuoysLatestAvailableDate()); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching wave buoys latest date: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching wave buoys latest date: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); @@ -365,11 +389,11 @@ public ResponseEntity getMooringsBetweenDates(String startDateTime, String en } try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getMooringsBetweenDates(startDateTime, endDateTime)); + return forwardDasResponse(dasService.getMooringsBetweenDates(startDateTime, endDateTime)); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching moorings data: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching moorings data: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); @@ -392,11 +416,11 @@ public ResponseEntity getMooringDetailsBetweenDates(String startDateTime, Str } try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getMooringDetailsBetweenDates(startDateTime, endDateTime, mooring)); + return forwardDasResponse(dasService.getMooringDetailsBetweenDates(startDateTime, endDateTime, mooring)); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching mooring historical data: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching mooring historical data: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); @@ -408,11 +432,11 @@ public ResponseEntity getMooringDetailsBetweenDates(String startDateTime, Str */ public ResponseEntity getMooringsLatestAvailableDate() { try { - return ResponseEntity - .ok() - .header("Content-Type", MediaType.APPLICATION_JSON_VALUE) - .body(dasService.getMooringsLatestAvailableDate()); + return forwardDasResponse(dasService.getMooringsLatestAvailableDate()); + } catch (HttpStatusCodeException e) { + log.error("DAS returned an error fetching moorings latest date: {}", e.getStatusCode()); + return forwardDasError(e); } catch (Exception e) { log.error("Error fetching moorings latest date: {}", e.getMessage()); return ResponseEntity.internalServerError().build(); diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceTest.java index 466fc73f..f0f21077 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceTest.java @@ -8,6 +8,7 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; @@ -48,17 +49,17 @@ public void setUp() { dasService = new DasService(config, httpClient, new ObjectMapper()); - when(httpClient.getForObject(anyString(), eq(byte[].class), anyMap())) - .thenReturn("ok".getBytes()); - when(httpClient.getForObject(anyString(), eq(byte[].class))) - .thenReturn("ok".getBytes()); + when(httpClient.getForEntity(anyString(), eq(byte[].class), anyMap())) + .thenReturn(ResponseEntity.ok("ok".getBytes())); + when(httpClient.getForEntity(anyString(), eq(byte[].class))) + .thenReturn(ResponseEntity.ok("ok".getBytes())); } @SuppressWarnings("unchecked") private CapturedRequest captureMapRequest() { ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(String.class); ArgumentCaptor> mapCaptor = ArgumentCaptor.forClass(Map.class); - verify(httpClient).getForObject(urlCaptor.capture(), eq(byte[].class), mapCaptor.capture()); + verify(httpClient).getForEntity(urlCaptor.capture(), eq(byte[].class), mapCaptor.capture()); return new CapturedRequest(urlCaptor.getValue(), mapCaptor.getValue()); } @@ -119,7 +120,7 @@ public void testLatestAvailableDateUsesNoUriVariables() { dasService.getWaveBuoysLatestAvailableDate(); ArgumentCaptor urlCaptor = ArgumentCaptor.forClass(String.class); - verify(httpClient).getForObject(urlCaptor.capture(), eq(byte[].class)); + verify(httpClient).getForEntity(urlCaptor.capture(), eq(byte[].class)); assertEquals(HOST + "/api/v1/das/data/feature-collection/wave-buoy/latest", urlCaptor.getValue()); } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestServicesTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestServicesTest.java index 6d995abe..2196249e 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/features/RestServicesTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/features/RestServicesTest.java @@ -16,8 +16,10 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.springframework.core.ParameterizedTypeReference; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.web.client.HttpClientErrorException; import java.util.List; import java.util.Map; @@ -65,20 +67,32 @@ void cleanUp() throws Exception { @Test public void testGetWaveBuoysBetweenDatesSuccess() { - byte[] mockResponse = "{\"type\":\"FeatureCollection\"}".getBytes(); - when(dasService.getWaveBuoysBetweenDates(VALID_START, VALID_END)).thenReturn(mockResponse); + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + when(dasService.getWaveBuoysBetweenDates(VALID_START, VALID_END)).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getWaveBuoysBetweenDates(VALID_START, VALID_END); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); + } + + @Test + public void testGetWaveBuoysBetweenDatesForwardsDasHeaders() { + // Headers set by DAS (e.g. caching/rate-limit headers) must reach the client, not just the body. + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + ResponseEntity dasResponse = ResponseEntity.ok().header("X-Custom-Header", "das-value").body(body); + when(dasService.getWaveBuoysBetweenDates(VALID_START, VALID_END)).thenReturn(dasResponse); + + ResponseEntity response = restServices.getWaveBuoysBetweenDates(VALID_START, VALID_END); + + assertEquals("das-value", response.getHeaders().getFirst("X-Custom-Header")); } @Test public void testGetWaveBuoysBetweenDatesNullDatesPassThrough() { // Both dates null is allowed; the service is still called (with nulls) and the result returned - byte[] mockResponse = "{\"type\":\"FeatureCollection\"}".getBytes(); - when(dasService.getWaveBuoysBetweenDates(null, null)).thenReturn(mockResponse); + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + when(dasService.getWaveBuoysBetweenDates(null, null)).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getWaveBuoysBetweenDates(null, null); @@ -120,17 +134,34 @@ public void testGetWaveBuoysBetweenDatesServiceError() { assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); } + @Test + public void testGetWaveBuoysBetweenDatesDasErrorIsForwardedNotMaskedAs500() { + // DAS itself returning e.g. a 404/502 must reach the client as that status with its + // body, not get collapsed into a generic 500 by the catch-all error handler. + byte[] errorBody = "{\"message\":\"upstream unavailable\"}".getBytes(); + HttpHeaders dasErrorHeaders = new HttpHeaders(); + dasErrorHeaders.set("Content-Type", "application/json"); + HttpClientErrorException dasError = HttpClientErrorException.create( + HttpStatus.BAD_GATEWAY, "Bad Gateway", dasErrorHeaders, errorBody, null); + when(dasService.getWaveBuoysBetweenDates(VALID_START, VALID_END)).thenThrow(dasError); + + ResponseEntity response = restServices.getWaveBuoysBetweenDates(VALID_START, VALID_END); + + assertEquals(HttpStatus.BAD_GATEWAY, response.getStatusCode()); + assertArrayEquals(errorBody, (byte[]) response.getBody()); + } + // ----- wave_buoys_latest_available_date ----- @Test public void testGetWaveBuoysLatestAvailableDateSuccess() { - byte[] mockResponse = "{\"latest_date\":\"2024-01-01\"}".getBytes(); - when(dasService.getWaveBuoysLatestAvailableDate()).thenReturn(mockResponse); + byte[] body = "{\"latest_date\":\"2024-01-01\"}".getBytes(); + when(dasService.getWaveBuoysLatestAvailableDate()).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getWaveBuoysLatestAvailableDate(); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); } @Test @@ -146,13 +177,13 @@ public void testGetWaveBuoysLatestAvailableDateServiceError() { @Test public void testGetWaveBuoyDetailsBetweenDatesSuccess() { - byte[] mockResponse = "{\"type\":\"FeatureCollection\"}".getBytes(); - when(dasService.getWaveBuoyDetailsBetweenDates(VALID_START, VALID_END, "BUOY-1")).thenReturn(mockResponse); + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + when(dasService.getWaveBuoyDetailsBetweenDates(VALID_START, VALID_END, "BUOY-1")).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getWaveBuoyDetailsBetweenDates(VALID_START, VALID_END, "BUOY-1"); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); } @Test @@ -185,13 +216,13 @@ public void testGetWaveBuoyDetailsBetweenDatesServiceError() { @Test public void testGetMooringsBetweenDatesSuccess() { - byte[] mockResponse = "{\"type\":\"FeatureCollection\"}".getBytes(); - when(dasService.getMooringsBetweenDates(VALID_START, VALID_END)).thenReturn(mockResponse); + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + when(dasService.getMooringsBetweenDates(VALID_START, VALID_END)).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getMooringsBetweenDates(VALID_START, VALID_END); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); } @Test @@ -216,13 +247,13 @@ public void testGetMooringsBetweenDatesServiceError() { @Test public void testGetMooringsLatestAvailableDateSuccess() { - byte[] mockResponse = "{\"latest_date\":\"2024-01-01\"}".getBytes(); - when(dasService.getMooringsLatestAvailableDate()).thenReturn(mockResponse); + byte[] body = "{\"latest_date\":\"2024-01-01\"}".getBytes(); + when(dasService.getMooringsLatestAvailableDate()).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getMooringsLatestAvailableDate(); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); } @Test @@ -238,13 +269,13 @@ public void testGetMooringsLatestAvailableDateServiceError() { @Test public void testGetMooringDetailsBetweenDatesSuccess() { - byte[] mockResponse = "{\"type\":\"FeatureCollection\"}".getBytes(); - when(dasService.getMooringDetailsBetweenDates(VALID_START, VALID_END, "MOORING-1")).thenReturn(mockResponse); + byte[] body = "{\"type\":\"FeatureCollection\"}".getBytes(); + when(dasService.getMooringDetailsBetweenDates(VALID_START, VALID_END, "MOORING-1")).thenReturn(ResponseEntity.ok(body)); ResponseEntity response = restServices.getMooringDetailsBetweenDates(VALID_START, VALID_END, "MOORING-1"); assertEquals(HttpStatus.OK, response.getStatusCode()); - assertEquals(mockResponse, response.getBody()); + assertEquals(body, response.getBody()); } @Test