From 40bf787f60e9c7342def7bdcf6fb999753b46de3 Mon Sep 17 00:00:00 2001 From: Lyn Long Date: Tue, 29 Sep 2026 14:35:45 +1000 Subject: [PATCH 1/2] block large download at api level --- .../server/core/configuration/Config.java | 2 + .../DownloadSizeExceededException.java | 22 +++++++ .../exception/GlobalExceptionHandler.java | 14 +++++ .../processes/DownloadAdmissionService.java | 39 ++++++++++-- .../DownloadSizeLimitProperties.java | 16 +++++ .../aodn/ogcapi/server/processes/RestApi.java | 21 +++++-- .../ogcapi/server/processes/RestServices.java | 30 ++++++++- server/src/main/resources/application.yaml | 6 ++ .../core/service/das/DasServiceCacheTest.java | 27 ++++++++ .../DownloadAdmissionServiceTest.java | 62 ++++++++++++++++++- .../server/processes/RestApiJobsTest.java | 19 ++++++ .../server/processes/RestServicesTest.java | 35 +++++++++++ .../src/test/resources/application-test.yaml | 3 + 13 files changed, 286 insertions(+), 10 deletions(-) create mode 100644 server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java create mode 100644 server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadSizeLimitProperties.java diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java index 97363250..6cbbc629 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/configuration/Config.java @@ -10,6 +10,7 @@ import au.org.aodn.ogcapi.server.core.util.RestTemplateUtils; import au.org.aodn.ogcapi.server.processes.BatchJobProperties; import au.org.aodn.ogcapi.server.processes.DownloadLimitProperties; +import au.org.aodn.ogcapi.server.processes.DownloadSizeLimitProperties; import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.ObjectMapper; import jakarta.annotation.PostConstruct; @@ -37,6 +38,7 @@ DasProperties.class, BatchJobProperties.class, DownloadLimitProperties.class, + DownloadSizeLimitProperties.class, OgcApiProperties.class }) public class Config { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java new file mode 100644 index 00000000..75c8f2da --- /dev/null +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java @@ -0,0 +1,22 @@ +package au.org.aodn.ogcapi.server.core.exception; + +import java.math.BigDecimal; +import java.math.RoundingMode; + +public class DownloadSizeExceededException extends RuntimeException { + + private static final BigDecimal BYTES_PER_GB = BigDecimal.valueOf(1024L * 1024 * 1024); + + public DownloadSizeExceededException(long estimatedBytes, long maxBytes) { + super("The selected data is too large to download (estimated " + toGb(estimatedBytes) + + " GB, limit " + toGb(maxBytes) + " GB). " + + "Please reduce the date range or area and try again."); + } + + private static String toGb(long bytes) { + return BigDecimal.valueOf(bytes) + .divide(BYTES_PER_GB, 1, RoundingMode.HALF_UP) + .stripTrailingZeros() + .toPlainString(); + } +} diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/GlobalExceptionHandler.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/GlobalExceptionHandler.java index 474a183d..8bb92b94 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/GlobalExceptionHandler.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/GlobalExceptionHandler.java @@ -130,6 +130,20 @@ public ResponseEntity handleDownloadLimitExceededException( return new ResponseEntity<>(errorResponse, HttpStatus.TOO_MANY_REQUESTS); } + @ExceptionHandler(DownloadSizeExceededException.class) + public ResponseEntity handleDownloadSizeExceededException( + DownloadSizeExceededException ex, + WebRequest request) { + ErrorResponse errorResponse = ErrorResponse + .builder() + .timestamp(LocalDateTime.now()) + .message(ex.getMessage()) + .details(request.getDescription(false)) + .build(); + + return new ResponseEntity<>(errorResponse, HttpStatus.UNPROCESSABLE_ENTITY); + } + @ExceptionHandler(DasUpstreamException.class) public ResponseEntity handleDasUpstreamException(DasUpstreamException ex, WebRequest request) { ErrorResponse errorResponse = ErrorResponse diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionService.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionService.java index a8768d75..f65eeafa 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionService.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionService.java @@ -1,6 +1,7 @@ package au.org.aodn.ogcapi.server.processes; import au.org.aodn.ogcapi.server.core.exception.DownloadLimitExceededException; +import au.org.aodn.ogcapi.server.core.exception.DownloadSizeExceededException; import com.fasterxml.jackson.core.JsonProcessingException; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -11,9 +12,11 @@ import java.util.concurrent.locks.ReentrantLock; /** - * Caps how many downloads one user can have running at once. A user already at the limit is - * rejected outright: the caller has to wait for one of their own downloads to finish and - * try again. + * Decides whether a download can be submitted. Two checks run before anything goes to AWS Batch: + * 1. Size: a download estimated at the size limit or more is rejected, the user has to reduce + * the subset. + * 2. Concurrency: a user already at the limit is rejected outright, the caller has to wait for + * one of their own downloads to finish and try again. */ @Slf4j @Service @@ -22,6 +25,7 @@ public class DownloadAdmissionService { private final RestServices restServices; private final InFlightDownloadCounter counter; private final DownloadLimitProperties limits; + private final DownloadSizeLimitProperties sizeLimit; private final ReentrantLock lock = new ReentrantLock(); @@ -38,21 +42,29 @@ public class DownloadAdmissionService { public DownloadAdmissionService( RestServices restServices, InFlightDownloadCounter counter, - DownloadLimitProperties limits) { + DownloadLimitProperties limits, + DownloadSizeLimitProperties sizeLimit) { this.restServices = restServices; this.counter = counter; this.limits = limits; + this.sizeLimit = sizeLimit; } /** * Submit a download to AWS Batch and return its job id. * + * @throws DownloadSizeExceededException the download is estimated at the size limit or more * @throws DownloadLimitExceededException the recipient already has {@code maxConcurrent} * downloads running */ public String submit(DownloadRequest request) throws JsonProcessingException { String key = InFlightDownloadCounter.recipientKey(request.recipient()); + if (sizeLimit.enabled()) { + // Before the slot is reserved, so a slow estimate does not hold one. + rejectIfTooLarge(request); + } + if (limits.enabled()) { // Outside the lock: the sweep is the only expensive step. counter.refreshIfStale(); @@ -73,6 +85,25 @@ public String submit(DownloadRequest request) throws JsonProcessingException { } } + private void rejectIfTooLarge(DownloadRequest request) { + long estimatedBytes; + try { + estimatedBytes = restServices.estimateDownloadBytes(request); + } catch (Exception e) { + // Let it through, as the portal does when its own estimate fails. Blocking every + // download because DAS cannot estimate would be worse than one job running out of disk. + log.warn("Size estimate failed for uuid {}, submitting without the size check", request.uuid(), e); + return; + } + + long maxBytes = sizeLimit.maxSize().toBytes(); + if (estimatedBytes >= maxBytes) { + log.info("Rejected download for uuid {}: estimated {} bytes, limit {} bytes", + request.uuid(), estimatedBytes, maxBytes); + throw new DownloadSizeExceededException(estimatedBytes, maxBytes); + } + } + private void reserveOrReject(DownloadRequest request, String key) { lock.lock(); try { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadSizeLimitProperties.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadSizeLimitProperties.java new file mode 100644 index 00000000..88e2b2bc --- /dev/null +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/DownloadSizeLimitProperties.java @@ -0,0 +1,16 @@ +package au.org.aodn.ogcapi.server.processes; + +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.bind.DefaultValue; +import org.springframework.util.unit.DataSize; + +/** + * Largest download we accept. The batch job writes its output to the Fargate disk, which is + * 200 GB, so a download estimated at maxSize or more is rejected before it is submitted. + */ +@ConfigurationProperties(prefix = "aws.batch.job.size-limit") +public record DownloadSizeLimitProperties( + @DefaultValue("true") boolean enabled, + @DefaultValue("180GB") DataSize maxSize +) { +} diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java index fcb5e754..bc4ea702 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java @@ -9,6 +9,7 @@ import au.org.aodn.ogcapi.processes.model.JobList; import au.org.aodn.ogcapi.processes.model.StatusInfo; import au.org.aodn.ogcapi.server.core.exception.DownloadLimitExceededException; +import au.org.aodn.ogcapi.server.core.exception.DownloadSizeExceededException; import au.org.aodn.ogcapi.server.core.model.DownloadExecutionResponse; import au.org.aodn.ogcapi.server.core.model.DownloadJobStatusInfo; import au.org.aodn.ogcapi.server.core.model.ErrorResponse; @@ -81,6 +82,17 @@ public class RestApi implements ProcessesApi, JobsApi { "message": "You already have 10 downloads in progress. Wait for one of them to complete before starting another." } """))) + @ApiResponse( + responseCode = "422", + description = "The download is estimated at the size limit or more.", + content = @Content( + mediaType = MediaType.APPLICATION_JSON_VALUE, + schema = @Schema(implementation = ErrorResponse.class), + examples = @ExampleObject(value = """ + { + "message": "The selected data is too large to download (estimated 250.3 GB, limit 180 GB). Please reduce the date range or area and try again." + } + """))) public ResponseEntity execute( @Parameter(in = ParameterIn.PATH, required = true, schema = @Schema()) @PathVariable("processID") @@ -108,8 +120,9 @@ public ResponseEntity execute( DownloadRequest request = new DownloadRequest(uuid, key, startDate, endDate, multiPolygon, recipient, collectionTitle, fullMetadataLink, suggestedCitation, outputFormat); - // The per-user limit is applied here: a recipient already at their limit is - // rejected outright, before anything is submitted to AWS Batch. + // The size limit and the per-user limit are applied here: a download that is too + // large, or a recipient already at their limit, is rejected outright, before + // anything is submitted to AWS Batch. // // The notify user email lives on this side rather than in data-access-service to // make the first email faster. It is sent only once AWS Batch has accepted the @@ -123,8 +136,8 @@ public ResponseEntity execute( return ResponseEntity.ok(results); - } catch (DownloadLimitExceededException e) { - // Let GlobalExceptionHandler turn this into a real 429 with a message the + } catch (DownloadLimitExceededException | DownloadSizeExceededException e) { + // Let GlobalExceptionHandler turn these into a real 429 or 422 with a message the // caller can act on, instead of the generic 200-wrapped error below. throw e; } catch (Exception e) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java index 0e984245..b8cababf 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestServices.java @@ -7,9 +7,11 @@ import au.org.aodn.ogcapi.server.core.service.das.DasService; import au.org.aodn.ogcapi.server.core.service.geoserver.wfs.DownloadWfsDataService; import au.org.aodn.ogcapi.server.core.service.sse.SseStreamHandler; +import au.org.aodn.ogcapi.server.core.util.DasSseFrames; import au.org.aodn.ogcapi.server.core.util.EmailUtils; import au.org.aodn.ogcapi.server.core.util.SubsetParametersUtils; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; @@ -32,6 +34,9 @@ @Slf4j public class RestServices { + // Same field the portal reads to block a download that is too large. + private static final String ESTIMATED_OUTPUT_BYTES = "estimated_output_bytes"; + private final BatchClient batchClient; private final ObjectMapper objectMapper; private final String batchJobDefinition; @@ -115,6 +120,27 @@ public Map buildDownloadParameters(DownloadRequest request) thro return parameters; } + /** + * Estimate how many bytes a download will produce. Uses the same uuid and subset parameters + * as the cloud-optimised estimate, so a subset the portal has just estimated comes from the + * cache instead of calling DAS again. + */ + public long estimateDownloadBytes(DownloadRequest request) throws JsonProcessingException { + Map parameters = SubsetParametersUtils.buildSubsetParameters( + objectMapper, request.uuid(), request.key(), request.startDate(), request.endDate(), + request.multiPolygon(), request.outputFormat()); + + // No client of our own to keep alive, so heartbeats are ignored. + String estimateJson = dasService.estimateCloudOptimisedDownloadSize( + request.uuid(), parameters, DasSseFrames.FrameCallback.IGNORE); + + JsonNode bytes = objectMapper.readTree(estimateJson).get(ESTIMATED_OUTPUT_BYTES); + if (bytes == null || !bytes.isNumber()) { + throw new IllegalStateException("Size estimate has no " + ESTIMATED_OUTPUT_BYTES + ": " + estimateJson); + } + return bytes.longValue(); + } + /** * The AWS Batch job name for a download. Note this sanitises the address, so it is not a * safe key for the owning user - read the recipient job parameter instead. @@ -123,7 +149,9 @@ public static String downloadJobName(String recipient) { return "generating-data-file-for-" + recipient.replaceAll("[^a-zA-Z0-9-_]", "-"); } - /** Submit a prepared download to the configured queue and job definition. */ + /** + * Submit a prepared download to the configured queue and job definition. + */ public String submitDownloadJob(String jobName, Map parameters) { String jobId = submitJob(jobName, this.batchJobQueue, this.batchJobDefinition, parameters); log.info("Job submitted with ID: {}", jobId); diff --git a/server/src/main/resources/application.yaml b/server/src/main/resources/application.yaml index b80c5845..9238a48a 100644 --- a/server/src/main/resources/application.yaml +++ b/server/src/main/resources/application.yaml @@ -66,6 +66,12 @@ aws: max-concurrent: 10 # How long the shared in-flight snapshot is reused before the queues are swept again. refresh-interval: 15s + # Largest download accepted. The job writes to the 200 GB Fargate disk, so a download + # estimated at max-size or more is rejected before it is submitted. Keep in line with + # EXTRA_LARGE_DOWNLOAD_BYTES in the portal. + size-limit: + enabled: true + max-size: 180GB wfs-default-param: fields: diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceCacheTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceCacheTest.java index 5d5a9363..e83d5e98 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceCacheTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/core/service/das/DasServiceCacheTest.java @@ -3,6 +3,9 @@ import au.org.aodn.ogcapi.server.core.configuration.CacheConfig; import au.org.aodn.ogcapi.server.core.http.RecordingSseConnector; import au.org.aodn.ogcapi.server.core.util.DasSseFrames; +import au.org.aodn.ogcapi.server.core.util.SubsetParametersUtils; +import au.org.aodn.ogcapi.server.processes.DownloadRequest; +import au.org.aodn.ogcapi.server.processes.RestServices; import com.fasterxml.jackson.databind.ObjectMapper; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -13,6 +16,7 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.test.util.ReflectionTestUtils; import org.springframework.web.client.RestTemplate; import org.springframework.web.reactive.function.client.WebClient; @@ -188,4 +192,27 @@ public void testCacheHitSkipsTheStreamAndItsHeartbeats() { estimate(parameters("netcdf"), heartbeats::incrementAndGet); assertEquals(1, heartbeats.get(), "A cache hit never opens a stream, so nothing heartbeats"); } + + /** + * The size check before a download is submitted must reuse the estimate the portal has just + * made for the same subset. The portal estimate builds its parameters with + * SubsetParametersUtils, as the SSE estimate in RestServices does. + */ + @Test + public void testDownloadSizeCheckReusesThePortalEstimate() throws Exception { + ObjectMapper objectMapper = new ObjectMapper(); + RestServices restServices = new RestServices(null, objectMapper, "test-job-definition", "test-job-queue"); + ReflectionTestUtils.setField(restServices, "dasService", dasService); + + connector.respondWith(List.of(RESULT_FRAME)); + estimate(SubsetParametersUtils.buildSubsetParameters(objectMapper, "test-uuid", "a.zarr", + "2020-01-01", "2020-12-31", "non-specified", "netcdf"), freshCallback()); + + long bytes = restServices.estimateDownloadBytes(new DownloadRequest("test-uuid", "a.zarr", + "2020-01-01", "2020-12-31", "non-specified", "person@example.com", "Test Collection", + "https://portal.example.test/details/test-uuid", "Cite as", "netcdf")); + + assertEquals(123, bytes); + assertEquals(1, connector.requests(), "The size check is answered from the cache, not by data-access-service"); + } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java index aecad7b2..0f5c8625 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java @@ -1,12 +1,14 @@ package au.org.aodn.ogcapi.server.processes; import au.org.aodn.ogcapi.server.core.exception.DownloadLimitExceededException; +import au.org.aodn.ogcapi.server.core.exception.DownloadSizeExceededException; import com.fasterxml.jackson.core.JsonProcessingException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.util.unit.DataSize; import java.time.Duration; import java.util.HashMap; @@ -23,6 +25,7 @@ import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) class DownloadAdmissionServiceTest { @@ -30,6 +33,7 @@ class DownloadAdmissionServiceTest { private static final String RECIPIENT = "person@example.com"; private static final String RECIPIENT_JOB_NAME = "generating-data-file-for-person-example-com"; private static final String OTHER_RECIPIENT = "someone.else@example.com"; + private static final long MAX_BYTES = 180L * 1024 * 1024 * 1024; @Mock private RestServices restServices; @@ -55,13 +59,21 @@ void setUp() throws JsonProcessingException { } private DownloadAdmissionService build(DownloadLimitProperties limits) { - return new DownloadAdmissionService(restServices, counter, limits); + return build(limits, sizeLimit(true)); + } + + private DownloadAdmissionService build(DownloadLimitProperties limits, DownloadSizeLimitProperties sizeLimit) { + return new DownloadAdmissionService(restServices, counter, limits, sizeLimit); } private static DownloadLimitProperties limits(boolean enabled, int maxConcurrent) { return new DownloadLimitProperties(enabled, maxConcurrent, Duration.ofSeconds(15)); } + private static DownloadSizeLimitProperties sizeLimit(boolean enabled) { + return new DownloadSizeLimitProperties(enabled, DataSize.ofGigabytes(180)); + } + private AtomicInteger current(String recipient) { // Key the way the real counter does, so these tests cannot accidentally rely on // case-sensitive bookkeeping the production class does not have. @@ -171,4 +183,52 @@ void aFailedSubmitDoesNotLeakAReservedSlot() throws Exception { assertNotNull(service.submit(request(RECIPIENT))); } + + @Test + void aDownloadAtTheSizeLimitIsRejectedBeforeSubmitAndEmail() throws Exception { + // The portal blocks at 180 GB and above, so exactly 180 GB is rejected here too. + when(restServices.estimateDownloadBytes(any())).thenReturn(MAX_BYTES); + + DownloadSizeExceededException exception = assertThrows( + DownloadSizeExceededException.class, () -> service.submit(request(RECIPIENT))); + + assertEquals("The selected data is too large to download (estimated 180 GB, limit 180 GB). " + + "Please reduce the date range or area and try again.", exception.getMessage()); + verify(restServices, never()).submitDownloadJob(anyString(), any()); + verify(restServices, never()).notifyUser(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); + } + + @Test + void aDownloadJustUnderTheSizeLimitIsSubmitted() throws Exception { + when(restServices.estimateDownloadBytes(any())).thenReturn(MAX_BYTES - 1); + + assertNotNull(service.submit(request(RECIPIENT))); + verify(restServices).submitDownloadJob(eq(RECIPIENT_JOB_NAME), any()); + } + + @Test + void aTooLargeDownloadDoesNotTakeASlot() throws Exception { + when(restServices.estimateDownloadBytes(any())).thenReturn(MAX_BYTES); + + assertThrows(DownloadSizeExceededException.class, () -> service.submit(request(RECIPIENT))); + + verify(counter, never()).refreshIfStale(); + } + + @Test + void aFailedEstimateStillSubmits() throws Exception { + // Same as the portal: no estimate means no block. + when(restServices.estimateDownloadBytes(any())).thenThrow(new RuntimeException("DAS is down")); + + assertNotNull(service.submit(request(RECIPIENT))); + verify(restServices).submitDownloadJob(eq(RECIPIENT_JOB_NAME), any()); + } + + @Test + void theSizeLimitCanBeTurnedOff() throws Exception { + DownloadAdmissionService disabled = build(limits(true, 10), sizeLimit(false)); + + assertNotNull(disabled.submit(request(RECIPIENT))); + verify(restServices, never()).estimateDownloadBytes(any()); + } } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java index 5a34d9e5..4b653e97 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java @@ -5,6 +5,7 @@ import au.org.aodn.ogcapi.server.core.exception.DownloadJobNotFoundException; import au.org.aodn.ogcapi.server.core.exception.DownloadJobStatusException; import au.org.aodn.ogcapi.server.core.exception.DownloadLimitExceededException; +import au.org.aodn.ogcapi.server.core.exception.DownloadSizeExceededException; import au.org.aodn.ogcapi.server.core.exception.GlobalExceptionHandler; import au.org.aodn.ogcapi.server.core.model.DownloadJobStatusInfo; import com.fasterxml.jackson.databind.ObjectMapper; @@ -90,6 +91,24 @@ void postAtTheDownloadLimitReturnsTooManyRequestsWithAClearMessage() throws Exce + "Wait for one of them to complete before starting another.")); } + @Test + void postOverTheSizeLimitReturnsUnprocessableEntityWithAClearMessage() throws Exception { + long gb = 1024L * 1024 * 1024; + when(downloadAdmissionService.submit(any())) + .thenThrow(new DownloadSizeExceededException(250 * gb + gb / 4, 180 * gb)); + String body = objectMapper.writeValueAsString(Map.of("inputs", Map.of( + "uuid", "collection-id", + "recipient", "person@example.com"))); + + mockMvc.perform(post("/api/v1/ogc/processes/download/execution") + .contentType(MediaType.APPLICATION_JSON) + .accept(MediaType.APPLICATION_JSON) + .content(body)) + .andExpect(status().is(422)) + .andExpect(jsonPath("$.message").value("The selected data is too large to download " + + "(estimated 250.3 GB, limit 180 GB). Please reduce the date range or area and try again.")); + } + @Test void getStatusSerializesExtendedStatusInfo() throws Exception { DownloadJobStatusInfo statusInfo = new DownloadJobStatusInfo(); diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestServicesTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestServicesTest.java index 345132a3..7512bf81 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestServicesTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestServicesTest.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import au.org.aodn.ogcapi.server.core.model.enumeration.DatasetDownloadEnums; +import au.org.aodn.ogcapi.server.core.service.das.DasService; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -10,6 +11,7 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.springframework.test.util.ReflectionTestUtils; import software.amazon.awssdk.services.batch.BatchClient; import software.amazon.awssdk.services.batch.model.SubmitJobRequest; import software.amazon.awssdk.services.batch.model.SubmitJobResponse; @@ -17,6 +19,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.Mockito.*; public class RestServicesTest { @@ -146,4 +149,36 @@ public void submitJobWithoutJobIdThrows() { "test-uuid", "test-dname", "2023-01-01", "2023-01-31", "non-specified", "test@example.com", "Test Ocean Data Collection", "https://metadata.imas.utas.edu.au/.../test-uuid-123", "", "geotiff")); } + + /** + * RestServices with a real ObjectMapper, so the estimate JSON is really parsed. + */ + private RestServices withEstimate(String estimateJson) { + DasService dasService = mock(DasService.class); + when(dasService.estimateCloudOptimisedDownloadSize(any(), anyMap(), any())).thenReturn(estimateJson); + RestServices services = new RestServices(batchClient, new ObjectMapper(), "test-job-definition", "test-job-queue"); + ReflectionTestUtils.setField(services, "dasService", dasService); + return services; + } + + private static DownloadRequest estimateRequest() { + return new DownloadRequest("test-uuid", "test.zarr", "2023-01-01", "2023-01-31", "non-specified", + "test@example.com", "Test Ocean Data Collection", "https://portal.example.test/details/test-uuid", + "Cite as", "netcdf"); + } + + @Test + public void estimateDownloadBytesReadsTheOutputSize() throws JsonProcessingException { + RestServices services = withEstimate( + "{\"estimated_output_bytes\":193273528320,\"estimated_uncompressed_bytes\":400000000000}"); + + assertEquals(193273528320L, services.estimateDownloadBytes(estimateRequest())); + } + + @Test + public void estimateDownloadBytesWithoutTheOutputSizeThrows() { + RestServices services = withEstimate("{\"estimated_uncompressed_bytes\":400000000000}"); + + assertThrows(IllegalStateException.class, () -> services.estimateDownloadBytes(estimateRequest())); + } } diff --git a/server/src/test/resources/application-test.yaml b/server/src/test/resources/application-test.yaml index 6b2db68d..49f3a6df 100644 --- a/server/src/test/resources/application-test.yaml +++ b/server/src/test/resources/application-test.yaml @@ -29,3 +29,6 @@ aws: job: user-limit: enabled: false + # Would call data-access-service for an estimate before each submit. + size-limit: + enabled: false From 8c6af491b114ce641c8a007c17c7de1855c97684 Mon Sep 17 00:00:00 2001 From: Lyn Long Date: Thu, 1 Oct 2026 10:43:56 +1000 Subject: [PATCH 2/2] update the error message more alike frontend wording --- .../core/exception/DownloadSizeExceededException.java | 6 +++--- .../java/au/org/aodn/ogcapi/server/processes/RestApi.java | 2 +- .../server/processes/DownloadAdmissionServiceTest.java | 5 +++-- .../org/aodn/ogcapi/server/processes/RestApiJobsTest.java | 4 ++-- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java index 75c8f2da..df34e7b1 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/core/exception/DownloadSizeExceededException.java @@ -8,9 +8,9 @@ public class DownloadSizeExceededException extends RuntimeException { private static final BigDecimal BYTES_PER_GB = BigDecimal.valueOf(1024L * 1024 * 1024); public DownloadSizeExceededException(long estimatedBytes, long maxBytes) { - super("The selected data is too large to download (estimated " + toGb(estimatedBytes) - + " GB, limit " + toGb(maxBytes) + " GB). " - + "Please reduce the date range or area and try again."); + super("Download is unavailable because the selected dataset is too large (estimated " + + toGb(estimatedBytes) + " GB, limit " + toGb(maxBytes) + " GB). " + + "Please refine your selection to reduce the dataset size."); } private static String toGb(long bytes) { diff --git a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java index bc4ea702..b3ad9754 100644 --- a/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java +++ b/server/src/main/java/au/org/aodn/ogcapi/server/processes/RestApi.java @@ -90,7 +90,7 @@ public class RestApi implements ProcessesApi, JobsApi { schema = @Schema(implementation = ErrorResponse.class), examples = @ExampleObject(value = """ { - "message": "The selected data is too large to download (estimated 250.3 GB, limit 180 GB). Please reduce the date range or area and try again." + "message": "Download is unavailable because the selected dataset is too large (estimated 250.3 GB, limit 180 GB). Please refine your selection to reduce the dataset size." } """))) public ResponseEntity execute( diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java index 0f5c8625..cbdb1135 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/processes/DownloadAdmissionServiceTest.java @@ -192,8 +192,9 @@ void aDownloadAtTheSizeLimitIsRejectedBeforeSubmitAndEmail() throws Exception { DownloadSizeExceededException exception = assertThrows( DownloadSizeExceededException.class, () -> service.submit(request(RECIPIENT))); - assertEquals("The selected data is too large to download (estimated 180 GB, limit 180 GB). " - + "Please reduce the date range or area and try again.", exception.getMessage()); + assertEquals("Download is unavailable because the selected dataset is too large " + + "(estimated 180 GB, limit 180 GB). Please refine your selection to reduce the dataset size.", + exception.getMessage()); verify(restServices, never()).submitDownloadJob(anyString(), any()); verify(restServices, never()).notifyUser(any(), any(), any(), any(), any(), any(), any(), any(), any(), any()); } diff --git a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java index 4b653e97..2ab191b0 100644 --- a/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java +++ b/server/src/test/java/au/org/aodn/ogcapi/server/processes/RestApiJobsTest.java @@ -105,8 +105,8 @@ void postOverTheSizeLimitReturnsUnprocessableEntityWithAClearMessage() throws Ex .accept(MediaType.APPLICATION_JSON) .content(body)) .andExpect(status().is(422)) - .andExpect(jsonPath("$.message").value("The selected data is too large to download " - + "(estimated 250.3 GB, limit 180 GB). Please reduce the date range or area and try again.")); + .andExpect(jsonPath("$.message").value("Download is unavailable because the selected dataset is too large " + + "(estimated 250.3 GB, limit 180 GB). Please refine your selection to reduce the dataset size.")); } @Test