From 7a9d688e636f2e2c923d9861f4f7bf984d07fd16 Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Thu, 29 Jul 2021 12:28:22 +0200
Subject: [PATCH] Handled error status codes in synctrans URL params
 convenience mode - #4129

---
 .../java/it/inaf/oats/vospace/JobService.java     | 10 +++++++---
 .../it/inaf/oats/vospace/TransferController.java  | 10 +++-------
 .../inaf/oats/vospace/TransferControllerTest.java | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/src/main/java/it/inaf/oats/vospace/JobService.java b/src/main/java/it/inaf/oats/vospace/JobService.java
index 381a304..5a8c511 100644
--- a/src/main/java/it/inaf/oats/vospace/JobService.java
+++ b/src/main/java/it/inaf/oats/vospace/JobService.java
@@ -18,6 +18,7 @@ import it.inaf.oats.vospace.exception.InvalidArgumentException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import it.inaf.oats.vospace.exception.VoSpaceErrorSummarizableException;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;
 import javax.servlet.http.HttpServletRequest;
@@ -204,8 +205,9 @@ public class JobService {
      * compliance with specifications
      *
      */
-    public void createSyncJobResult(JobSummary job) {
+    public Optional<VoSpaceErrorSummarizableException> createSyncJobResult(JobSummary job) {
         Transfer negotiatedTransfer = null;
+        VoSpaceErrorSummarizableException exception = null;
         try {
             Transfer transfer = uriService.getTransfer(job);
             negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
@@ -216,14 +218,16 @@ public class JobService {
             job.setPhase(ExecutionPhase.ERROR);
             stripProtocols(job, negotiatedTransfer);
             job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(e));
+            exception = e;
         } catch (Exception e) {
             job.setPhase(ExecutionPhase.ERROR);
             stripProtocols(job, negotiatedTransfer);
-            job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(
-                    new InternalFaultException(e)));
+            exception = new InternalFaultException(e);
+            job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(exception));
         } finally {
             jobDAO.createJob(job, negotiatedTransfer);
         }
+        return Optional.ofNullable(exception);
     }
     
     private void stripProtocols(JobSummary job, Transfer negotiatedTransfer) {
diff --git a/src/main/java/it/inaf/oats/vospace/TransferController.java b/src/main/java/it/inaf/oats/vospace/TransferController.java
index 52d0bed..96805ba 100644
--- a/src/main/java/it/inaf/oats/vospace/TransferController.java
+++ b/src/main/java/it/inaf/oats/vospace/TransferController.java
@@ -93,16 +93,12 @@ public class TransferController {
         }
 
         JobSummary jobSummary = newJobSummary(transfer, principal);
-        jobService.createSyncJobResult(jobSummary);
-
-        if (jobSummary.getErrorSummary() != null) {
-            // TODO: decide how to hanlde HTTP error codes
+        jobService.createSyncJobResult(jobSummary).ifPresent(ex -> {
             // If an error occurs with the synchronous convenience mode where the preferred endpoint
             // is immediately returned as a redirect, the error information is returned directly in 
             // the response body with the associated HTTP status code. 
-            return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR)
-                    .body(jobSummary.getErrorSummary().getMessage());
-        }
+            throw ex;
+        });
 
         // Behaves as if REQUEST=redirect was set, for compatibility with CADC client
         String endpoint = transfer.getProtocols().get(0).getEndpoint();
diff --git a/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java b/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java
index 49e7544..2a52387 100644
--- a/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java
+++ b/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java
@@ -11,6 +11,7 @@ import it.inaf.oats.vospace.datamodel.NodeProperties;
 import it.inaf.oats.vospace.datamodel.Views;
 import it.inaf.oats.vospace.exception.ErrorSummaryFactory;
 import it.inaf.oats.vospace.exception.PermissionDeniedException;
+import it.inaf.oats.vospace.exception.ProtocolNotSupportedException;
 import it.inaf.oats.vospace.persistence.JobDAO;
 import it.inaf.oats.vospace.persistence.LocationDAO;
 import it.inaf.oats.vospace.persistence.NodeDAO;
@@ -427,6 +428,20 @@ public class TransferControllerTest {
                 .andExpect(status().is3xxRedirection());
     }
 
+    @Test
+    public void testSyncTransferUrlParamsModeUnsupportedProtocol() throws Exception {
+
+        Exception ex = mockMvc.perform(get("/synctrans")
+                .header("Authorization", "Bearer user1_token")
+                .param("TARGET", "vos://example.com!vospace/mynode")
+                .param("DIRECTION", "pullFromVoSpace")
+                .param("PROTOCOL", "ivo://ivoa.net/vospace/core#httpput"))
+                .andExpect(status().isBadRequest())
+                .andReturn().getResolvedException();
+
+        assertTrue(ex instanceof ProtocolNotSupportedException);
+    }
+    
     private Jobs getFakeJobs() {
         Jobs jobs = new Jobs();
         jobs.setVersion("1.1");
-- 
GitLab