diff --git a/src/main/java/it/inaf/oats/vospace/JobService.java b/src/main/java/it/inaf/oats/vospace/JobService.java index 77c7e047ab468cf504af30f97802d97aca87e1a7..122ad4c8ba365ade407444dffa9f7ccd734a480c 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 a0e7b73899a031675d7ea372a358ee8a1e68384f..eff3c3ef23713d9f4e87c4443213aea78590e9eb 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");