OK-424: Korjattu katselmointihavaintoja

Opetushallitus · Jul 12, 2024 · 1910ca5 · 1910ca5
1 parent bc204d8
commit 1910ca5
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 24 deletions.
diff --git 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<String> 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
@@ -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) {
     }
   }