From bcb731124dbd4451551b9541d34cad2ad9a360d3 Mon Sep 17 00:00:00 2001
From: Sonia Zorba <sonia.zorba@inaf.it>
Date: Wed, 14 Jul 2021 12:17:10 +0200
Subject: [PATCH] Stripped protocols also from negotiated transfer result in
case of error on synchronous transfer endpoint
---
.../java/it/inaf/oats/vospace/JobService.java | 11 ++++--
.../it/inaf/oats/vospace/JobServiceTest.java | 36 ++++++++++++++++---
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/src/main/java/it/inaf/oats/vospace/JobService.java b/src/main/java/it/inaf/oats/vospace/JobService.java
index 77c7e04..122ad4c 100644
--- a/src/main/java/it/inaf/oats/vospace/JobService.java
+++ b/src/main/java/it/inaf/oats/vospace/JobService.java
@@ -209,17 +209,24 @@ public class JobService {
// Need to catch other exceptions too to avoid inconsistent job status
} catch (VoSpaceErrorSummarizableException e) {
job.setPhase(ExecutionPhase.ERROR);
- uriService.getTransfer(job).getProtocols().clear();
+ stripProtocols(job, negotiatedTransfer);
job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(e));
} catch (Exception e) {
job.setPhase(ExecutionPhase.ERROR);
- uriService.getTransfer(job).getProtocols().clear();
+ stripProtocols(job, negotiatedTransfer);
job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(
new InternalFaultException(e)));
} finally {
jobDAO.createJob(job, negotiatedTransfer);
}
}
+
+ private void stripProtocols(JobSummary job, Transfer negotiatedTransfer) {
+ uriService.getTransfer(job).getProtocols().clear();
+ if (negotiatedTransfer != null) {
+ negotiatedTransfer.getProtocols().clear();
+ }
+ }
private void setJobResults(JobSummary jobSummary, Transfer transfer) {
String baseUrl = servletRequest.getRequestURL().substring(0,
diff --git a/src/test/java/it/inaf/oats/vospace/JobServiceTest.java b/src/test/java/it/inaf/oats/vospace/JobServiceTest.java
index a0e7b73..eff3c3e 100644
--- a/src/test/java/it/inaf/oats/vospace/JobServiceTest.java
+++ b/src/test/java/it/inaf/oats/vospace/JobServiceTest.java
@@ -16,6 +16,8 @@ import net.ivoa.xml.uws.v1.JobSummary;
import net.ivoa.xml.vospace.v2.Protocol;
import net.ivoa.xml.vospace.v2.Transfer;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -49,13 +51,13 @@ public class JobServiceTest {
@Mock
private HttpServletRequest servletRequest;
-
+
@Mock
private MoveService moveService;
@InjectMocks
private JobService jobService;
-
+
@BeforeEach
public void setUp() {
when(servletRequest.getRequestURL()).thenReturn(new StringBuffer("http://localhost/vospace/transfer"));
@@ -108,18 +110,42 @@ public class JobServiceTest {
@Test
public void testSyncJobResultVoSpaceError() {
- when(uriService.getTransfer(any())).thenReturn(getPullFromVoSpaceHttpTransfer());
+ Transfer transfer = getPullFromVoSpaceHttpTransfer();
+ assertFalse(transfer.getProtocols().isEmpty());
+ when(uriService.getTransfer(any())).thenReturn(transfer);
doThrow(new NodeBusyException("/foo")).when(uriService).getNegotiatedTransfer(any(), any());
jobService.createSyncJobResult(new JobSummary());
verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
+ assertTrue(transfer.getProtocols().isEmpty());
}
@Test
public void testSyncJobResultUnexpectedError() {
- when(uriService.getTransfer(any())).thenReturn(getPullFromVoSpaceHttpTransfer());
+ Transfer transfer = getPullFromVoSpaceHttpTransfer();
+ assertFalse(transfer.getProtocols().isEmpty());
+ when(uriService.getTransfer(any())).thenReturn(transfer);
doThrow(new NullPointerException()).when(uriService).getNegotiatedTransfer(any(), any());
jobService.createSyncJobResult(new JobSummary());
verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
+ assertTrue(transfer.getProtocols().isEmpty());
+ }
+
+ @Test
+ public void testSyncJobResultErrorAfterNegotiatedTransfer() {
+ Transfer transfer = getPullFromVoSpaceHttpTransfer();
+ assertFalse(transfer.getProtocols().isEmpty());
+ when(uriService.getTransfer(any())).thenReturn(transfer);
+
+ Transfer negotiatedTransfer = getPullFromVoSpaceHttpTransfer();
+ assertFalse(negotiatedTransfer.getProtocols().isEmpty());
+ when(uriService.getNegotiatedTransfer(any(), any())).thenReturn(negotiatedTransfer);
+
+ doThrow(new NullPointerException()).when(servletRequest).getContextPath();
+ jobService.createSyncJobResult(new JobSummary());
+
+ verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
+ assertTrue(transfer.getProtocols().isEmpty());
+ assertTrue(negotiatedTransfer.getProtocols().isEmpty());
}
@Test
@@ -158,7 +184,7 @@ public class JobServiceTest {
@Test
public void testStartJobMoveNode() {
-
+
Transfer moveNode = new Transfer();
moveNode.setDirection("vos://example.com!vospace/myfile");
--
GitLab