diff --git a/src/main/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalvelu.java b/src/main/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalvelu.java index a719ae2..973f6e5 100644 --- a/src/main/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalvelu.java +++ b/src/main/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalvelu.java @@ -150,24 +150,82 @@ public ObjectMetadata saveSiirtotiedosto( private ObjectMetadata saveWithRetry( String documentId, Collection tags, InputStream data, int retryCount) { int retryNumber = 0; - PutObjectResponse saveResponse = null; - ObjectMetadata metadata = null; String key = composeKey(tags, documentId); + SaveOperationData operationData = doPutObject(key, documentId, data); + while (++retryNumber < retryCount && operationData.failed()) { + operationData = doPutObject(key, documentId, data); + } - while (retryNumber++ <= retryCount) { - try { - if (saveResponse == null) { - saveResponse = putObject(key, documentId, "json", data).join(); - retryNumber = 1; - } - metadata = getObjectAttributesASync(documentId, key).join(); - break; - } catch (RuntimeException exp) { - if (!retryable(exp) || retryNumber > retryCount) { - throw exp; - } + if (!operationData.failed()) { + retryNumber = 0; + operationData = doGetObjectAttributes(key, documentId); + while (++retryNumber < retryCount && operationData.failed()) { + operationData = doGetObjectAttributes(key, documentId); } } - return metadata; + + if (operationData.failed()) { + throw new RuntimeException(operationData.getException()); + } + return operationData.getObjectMetadata(); + } + + private SaveOperationData doPutObject(String key, String documentId, InputStream data) { + try { + return new SaveOperationData(putObject(key, documentId, "json", data).join()); + } catch (RuntimeException exp) { + return throwOrReturnRetryableError(exp); + } + } + + private SaveOperationData doGetObjectAttributes(String key, String documentId) { + try { + return new SaveOperationData(getObjectAttributesASync(documentId, key).join()); + } catch (RuntimeException exp) { + return throwOrReturnRetryableError(exp); + } + } + + private SaveOperationData throwOrReturnRetryableError(RuntimeException exp) { + if (!retryable(exp)) { + throw new RuntimeException(exp); + } + return new SaveOperationData(exp); + } + + private static class SaveOperationData { + private PutObjectResponse putResponse; + private ObjectMetadata objectMetadata; + private RuntimeException exception; + + public SaveOperationData(PutObjectResponse putResponse) { + this.putResponse = putResponse; + exception = null; + } + + public SaveOperationData(ObjectMetadata objectMetadata) { + this.objectMetadata = objectMetadata; + exception = null; + } + + public SaveOperationData(RuntimeException exception) { + this.exception = exception; + } + + public PutObjectResponse getPutResponse() { + return putResponse; + } + + public ObjectMetadata getObjectMetadata() { + return objectMetadata; + } + + public RuntimeException getException() { + return exception; + } + + public boolean failed() { + return this.exception != null; + } } } diff --git a/src/test/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalveluTest.java b/src/test/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalveluTest.java index 430e656..0389c65 100644 --- a/src/test/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalveluTest.java +++ b/src/test/java/fi/vm/sade/valinta/dokumenttipalvelu/SiirtotiedostoPalveluTest.java @@ -11,7 +11,6 @@ import java.nio.file.Paths; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import org.junit.jupiter.api.Test; import org.mockito.stubbing.OngoingStubbing; import software.amazon.awssdk.core.async.AsyncRequestBody; @@ -52,7 +51,7 @@ public void testDocumentIdWithAdditionalInfo() throws IOException { UUID.randomUUID().toString(), 0, Files.newInputStream(Paths.get("src/test/resources/testfile.txt")), - 2); + 0); assertNotNull(metadata); assertTrue(metadata.documentId.startsWith("source_sub__")); assertTrue(metadata.documentId.contains("_someInfo_")); @@ -69,14 +68,14 @@ public void testSaveSucceedsAfterRetry() throws IOException { UUID.randomUUID().toString(), 0, Files.newInputStream(Paths.get("src/test/resources/testfile.txt")), - 2); + 3); assertNotNull(metadata); assertTrue(metadata.documentId.startsWith("source_sub__")); } @Test public void testSaveFailsAfterPutObjectRetries() throws IOException { - mockSequenceForSave(4, 2); + mockSequenceForSave(2, 0); try { siirtotiedostoPalvelu.saveSiirtotiedosto( "source", @@ -85,15 +84,15 @@ public void testSaveFailsAfterPutObjectRetries() throws IOException { UUID.randomUUID().toString(), 0, Files.newInputStream(Paths.get("src/test/resources/testfile.txt")), - 3); + 2); fail("Expected exception not thrown"); - } catch (CompletionException | Testbase.RetryableException ignored) { + } catch (RuntimeException ignored) { } } @Test public void testSaveFailsAfterGetAttributesRetries() throws IOException { - mockSequenceForSave(0, 4); + mockSequenceForSave(0, 3); try { siirtotiedostoPalvelu.saveSiirtotiedosto( "source", @@ -102,9 +101,9 @@ public void testSaveFailsAfterGetAttributesRetries() throws IOException { UUID.randomUUID().toString(), 0, Files.newInputStream(Paths.get("src/test/resources/testfile.txt")), - 2); + 3); fail("Expected exception not thrown"); - } catch (CompletionException | Testbase.RetryableException ignored) { + } catch (RuntimeException ignored) { } }