From 9c7e3e2a3b98d2b760517d1e24168fbe481ed791 Mon Sep 17 00:00:00 2001 From: Ameya Shendre Date: Fri, 25 Apr 2025 12:23:14 +0100 Subject: [PATCH 1/4] feat: Ingore large payload support for generic importers --- .../README.md | 21 +++- .../datatransfer/generic/GenericImporter.java | 17 ++- .../generic/GenericImporterTest.java | 107 +++++++++++------- 3 files changed, 100 insertions(+), 45 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-generic/README.md b/extensions/data-transfer/portability-data-transfer-generic/README.md index fd411cf31..48b814282 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/README.md +++ b/extensions/data-transfer/portability-data-transfer-generic/README.md @@ -753,4 +753,23 @@ 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" +} +``` +The worker will ignore such imports in this case & 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..0102cf018 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 @@ -139,20 +139,29 @@ 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")) { + monitor.debug(() -> "Payload rejected: too large %s", error.toString()); + return true; + } + + 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..e768da6ec 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,29 @@ 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")); + + assertTrue(executor.getErrors().isEmpty()); + } } From c40b71ad78dea102ff74129a71b820db61a5c421 Mon Sep 17 00:00:00 2001 From: Ameya Shendre Date: Fri, 25 Apr 2025 15:21:52 +0100 Subject: [PATCH 2/4] feat: Ingore large payload support for generic importers --- .../datatransfer/generic/GenericImporter.java | 6 +++--- .../datatransfer/generic/GenericImporterTest.java | 6 +++++- 2 files changed, 8 insertions(+), 4 deletions(-) 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 0102cf018..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; @@ -151,8 +152,7 @@ boolean parseResponse(Response response) throws IOException, InvalidTokenExcepti } if (response.code() == 413 && error.getError().equals("too_large_payload")) { - monitor.debug(() -> "Payload rejected: too large %s", error.toString()); - return true; + throw new IOException(format("%s (%d) %s", PAYLOAD_TOO_LARGE, response.code(), error)); } throw new IOException(format("Error (%d) %s", response.code(), error.toString())); 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 e768da6ec..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 @@ -376,6 +376,10 @@ public void testGenericImporterIgnoreLargePayload() throws Exception { "accessToken", "refreshToken", webServer.url("/refresh").toString()), new IdOnlyContainerResource("itemId")); - assertTrue(executor.getErrors().isEmpty()); + Collection errors = executor.getErrors(); + assertEquals(1, errors.size()); + ErrorDetail error = errors.iterator().next(); + assertEquals("itemId", error.title()); + assertContains(GenericImporter.PAYLOAD_TOO_LARGE, error.exception()); } } From d199df6364043a83135353e8b28a6e3a681b45be Mon Sep 17 00:00:00 2001 From: Ameya Shendre Date: Fri, 25 Apr 2025 15:29:07 +0100 Subject: [PATCH 3/4] feat: Ingore large payload support for generic importers --- .../portability-data-transfer-generic/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/extensions/data-transfer/portability-data-transfer-generic/README.md b/extensions/data-transfer/portability-data-transfer-generic/README.md index 48b814282..2fcd98e9f 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/README.md +++ b/extensions/data-transfer/portability-data-transfer-generic/README.md @@ -772,4 +772,8 @@ Content-Type: application/json "error_description": "Destination not accepting payloads above XMB" } ``` -The worker will ignore such imports in this case & continue with other imports. \ No newline at end of file + +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 handle 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 From ad82eed046eea85c6f71a8bc18001c7a4f825366 Mon Sep 17 00:00:00 2001 From: Ameya Shendre Date: Fri, 25 Apr 2025 15:30:09 +0100 Subject: [PATCH 4/4] feat: Ingore large payload support for generic importers --- .../data-transfer/portability-data-transfer-generic/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/data-transfer/portability-data-transfer-generic/README.md b/extensions/data-transfer/portability-data-transfer-generic/README.md index 2fcd98e9f..49f9792a1 100644 --- a/extensions/data-transfer/portability-data-transfer-generic/README.md +++ b/extensions/data-transfer/portability-data-transfer-generic/README.md @@ -775,5 +775,5 @@ Content-Type: application/json 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 handle large payload errors gracefully. +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