diff --git a/extensions/data-transfer/portability-data-transfer-generic/README.md b/extensions/data-transfer/portability-data-transfer-generic/README.md index fd411cf31..49f9792a1 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/README.md +++ b/extensions/data-transfer/portability-data-transfer-generic/README.md @@ -753,4 +753,27 @@ Content-Type: application/json } ``` The worker will pause the job and won't retry in such scenario. -Some exporters also support the ability to retry transfers after user frees up space in the destination. \ No newline at end of file +Some exporters also support the ability to retry transfers after user frees up space in the destination. + +### Destination Not Accepting Large Payloads + +If the destination is unable to accept large payloads due to API gateway limitations or other reasons, +the POST APIs should also throw a `413 Payload Too Large` error. + +`error` field should strictly be set to `too_large_payload` to indicate that the destination cannot handle large payloads. + +Example error response: + +``` +HTTP/1.1 413 Payload Too Large +Content-Type: application/json +{ + "error": "too_large_payload", + "error_description": "Destination not accepting payloads above XMB" +} +``` + +Generic Importers will throw an IOException with a message starting with: + `GenericImporter payload too large: ` +Idempotent executors can ignore such IOExceptions if they want. This allows us to ignore large payload errors gracefully. +If Idempotent executors are configured correctly, the worker will ignore such imports and continue with other imports. \ No newline at end of file diff --git a/extensions/data-transfer/portability-data-transfer-generic/src/main/java/org/datatransferproject/datatransfer/generic/GenericImporter.java b/extensions/data-transfer/portability-data-transfer-generic/src/main/java/org/datatransferproject/datatransfer/generic/GenericImporter.java index cc5680d0c..86f84d5f9 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/src/main/java/org/datatransferproject/datatransfer/generic/GenericImporter.java +++ b/extensions/data-transfer/portability-data-transfer-generic/src/main/java/org/datatransferproject/datatransfer/generic/GenericImporter.java @@ -39,7 +39,8 @@ public class GenericImporter implements Importer { - + public static final String PAYLOAD_TOO_LARGE = "GenericImporter payload too large: "; + @JsonIgnoreProperties(ignoreUnknown = true) static class ErrorResponse { private final String error; @@ -139,20 +140,28 @@ boolean parseResponse(Response response) throws IOException, InvalidTokenExcepti response.code(), new String(body, StandardCharsets.UTF_8)), e); } - + if (response.code() == 401 && error.getError().equals("invalid_token")) { throw new InvalidTokenException(error.toString(), null); - } if (response.code() == 413 && error.getError().equals("destination_full")) { + } + + if (response.code() == 413 && error.getError().equals("destination_full")) { throw new DestinationMemoryFullException( String.format("Generic importer failed with code (%s)", response.code()), new RuntimeException("destination_full")); - } else { - throw new IOException(format("Error (%d) %s", response.code(), error.toString())); } + + if (response.code() == 413 && error.getError().equals("too_large_payload")) { + throw new IOException(format("%s (%d) %s", PAYLOAD_TOO_LARGE, response.code(), error)); + } + + throw new IOException(format("Error (%d) %s", response.code(), error.toString())); + } if (response.code() < 200 || response.code() >= 300) { throw new IOException(format("Unexpected response code (%d)", response.code())); } + return true; } diff --git a/extensions/data-transfer/portability-data-transfer-generic/src/test/java/org/datatransferproject/datatransfer/generic/GenericImporterTest.java b/extensions/data-transfer/portability-data-transfer-generic/src/test/java/org/datatransferproject/datatransfer/generic/GenericImporterTest.java index f51526226..203e7c5f3 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/src/test/java/org/datatransferproject/datatransfer/generic/GenericImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-generic/src/test/java/org/datatransferproject/datatransfer/generic/GenericImporterTest.java @@ -107,11 +107,11 @@ public void testGenericImporter() throws Exception { getImporter( importerClass, container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); webServer.setDispatcher(getDispatcher()); importer.importItem( @@ -199,11 +199,11 @@ public void testGenericImporterTokenRefresh() throws Exception { getImporter( importerClass, container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); webServer.setDispatcher(getDispatcher()); importer.importItem( @@ -243,11 +243,11 @@ public void testGenericImporterBadRequest() throws Exception { getImporter( importerClass, container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); webServer.enqueue( new MockResponse().setResponseCode(400).setBody("{\"error\":\"bad_request\"}")); @@ -272,11 +272,11 @@ public void testGenericImporterUnexpectedResponse() throws Exception { getImporter( importerClass, container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); webServer.enqueue(new MockResponse().setResponseCode(400).setBody("notjson")); importer.importItem( @@ -300,11 +300,11 @@ public void testGenericImporterUnexpectedResponseCode() throws Exception { getImporter( importerClass, container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); webServer.enqueue(new MockResponse().setResponseCode(111)); importer.importItem( @@ -325,25 +325,27 @@ public void testGenericImporterUnexpectedResponseCode() throws Exception { public void testGenericImporterDestinationFull() throws Exception { InMemoryIdempotentImportExecutor executor = new InMemoryIdempotentImportExecutor(monitor); GenericImporter importer = - getImporter( - importerClass, - container -> - List.of( - new ImportableData<>( - new GenericPayload<>(container.getId(), "schemasource"), - container.getId(), - container.getId()))); - webServer.enqueue(new MockResponse().setResponseCode(413).setBody("{\"error\":\"destination_full\"}")); - - assertThrows(DestinationMemoryFullException.class, () -> { - importer.importItem( + getImporter( + importerClass, + container -> + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); + webServer.enqueue( + new MockResponse().setResponseCode(413).setBody("{\"error\":\"destination_full\"}")); + + assertThrows( + DestinationMemoryFullException.class, + () -> { + importer.importItem( UUID.randomUUID(), executor, new TokensAndUrlAuthData( - "accessToken", "refreshToken", webServer.url("/refresh").toString()), + "accessToken", "refreshToken", webServer.url("/refresh").toString()), new IdOnlyContainerResource("itemId")); - }); - + }); Collection errors = executor.getErrors(); assertEquals(1, errors.size()); @@ -351,4 +353,33 @@ public void testGenericImporterDestinationFull() throws Exception { assertEquals("itemId", error.title()); assertContains("Generic importer failed with code (413)", error.exception()); } + + @Test + public void testGenericImporterIgnoreLargePayload() throws Exception { + InMemoryIdempotentImportExecutor executor = new InMemoryIdempotentImportExecutor(monitor); + GenericImporter importer = + getImporter( + importerClass, + container -> + List.of( + new ImportableData<>( + new GenericPayload<>(container.getId(), "schemasource"), + container.getId(), + container.getId()))); + webServer.enqueue( + new MockResponse().setResponseCode(413).setBody("{\"error\":\"too_large_payload\"}")); + + importer.importItem( + UUID.randomUUID(), + executor, + new TokensAndUrlAuthData( + "accessToken", "refreshToken", webServer.url("/refresh").toString()), + new IdOnlyContainerResource("itemId")); + + Collection errors = executor.getErrors(); + assertEquals(1, errors.size()); + ErrorDetail error = errors.iterator().next(); + assertEquals("itemId", error.title()); + assertContains(GenericImporter.PAYLOAD_TOO_LARGE, error.exception()); + } }