Updated jobs handling in compliance with specification: UWS result contains...

Updated jobs handling in compliance with specification: UWS result contains transferDetails URL and transfer negotiation is available only on transferDetails endpoint (jobInfo contains original transfer object)

src/main/java/it/inaf/oats/vospace/JobService.java

@@ -18,8 +18,9 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import it.inaf.oats.vospace.exception.VoSpaceErrorSummarizableException;
 import java.util.concurrent.CompletableFuture;
-import java.util.function.Consumer;
+import java.util.function.Function;
 import javax.servlet.http.HttpServletRequest;
+import net.ivoa.xml.uws.v1.ResultReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -100,15 +101,18 @@ public class JobService {
             }
             job.setPhase(phase);
             
-            jobDAO.updateJob(job);
+            jobDAO.updateJob(job, null);
+
+            Transfer negotiatedTransfer = null;
 
             switch (getJobDirection(transfer)) {
                 case pullToVoSpace:
-                    handlePullToVoSpace(job, transfer);
+                    negotiatedTransfer = handlePullToVoSpace(job, transfer);
                     break;
                 case pullFromVoSpace:
                 case pushToVoSpace:
-                    handleVoSpaceUrlsListResult(job, transfer);
+                    negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
+                    setJobResults(job, transfer);
                     break;
                 case moveNode:
                     handleMoveNode(job, transfer);
@@ -121,16 +125,18 @@ public class JobService {
             // the previous job are asynchronous. Each job has to set its
             // completion independently. Only jobs started from the /synctrans
             // endpoints are completed immediately (see createSyncJobResult() method)
+            
+            return negotiatedTransfer;
         });
     }
 
-    private void handlePullToVoSpace(JobSummary job, Transfer transfer) {
+    private Transfer handlePullToVoSpace(JobSummary job, Transfer transfer) {
 
         for (Protocol protocol : transfer.getProtocols()) {
             switch (protocol.getUri()) {
                 case "ia2:async-recall":
                     asyncTransfService.startJob(job);
-                    return;
+                    return transfer;
                 case "ivo://ivoa.net/vospace/core#httpget":
                     if (transfer.getTarget().size() != 1) {
                         throw new InvalidArgumentException("Invalid target size for pullToVoSpace: " + transfer.getTarget().size());
@@ -138,19 +144,18 @@ public class JobService {
                     String nodeUri = transfer.getTarget().get(0);
                     String contentUri = protocol.getEndpoint();
                     uriService.setNodeRemoteLocation(nodeUri, contentUri);
-                    uriService.setTransferJobResult(job, transfer);
+                    Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
+                    setJobResults(job, transfer);
                     // Special case: import of a node from a portal file server
                     // doesn't imply file transfer, so it can be set to completed
                     job.setPhase(ExecutionPhase.COMPLETED);
-                    return;
+                    return negotiatedTransfer;
                 default:
-                    throw new InternalFaultException("Unsupported pullToVoSpace protocol: " + protocol.getUri());
-            }
+                    throw new InvalidArgumentException("Unsupported pullToVoSpace protocol: " + protocol.getUri());
             }
         }
 
-    private void handleVoSpaceUrlsListResult(JobSummary job, Transfer transfer) {
-        uriService.setTransferJobResult(job, transfer);
+        throw new InvalidArgumentException("Transfer contains no protocols");
     }
 
     private void handleMoveNode(JobSummary jobSummary, Transfer transfer) {
@@ -161,13 +166,15 @@ public class JobService {
             handleJobErrors(jobSummary, job -> {
                 moveService.processMoveJob(transfer, user);
                 job.setPhase(ExecutionPhase.COMPLETED);
+                return null;
             });
         });
     }
 
-    private void handleJobErrors(JobSummary job, Consumer<JobSummary> jobConsumer) {
+    private void handleJobErrors(JobSummary job, Function<JobSummary, Transfer> jobConsumer) {
+        Transfer negotiatedTransfer = null;
         try {
-            jobConsumer.accept(job);
+            negotiatedTransfer = jobConsumer.apply(job);
         } catch (VoSpaceErrorSummarizableException e) {
             job.setPhase(ExecutionPhase.ERROR);
             job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(e));
@@ -176,7 +183,7 @@ public class JobService {
             job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(
                     new InternalFaultException(e)));
         } finally {
-            jobDAO.updateJob(job);
+            jobDAO.updateJob(job, negotiatedTransfer);
         }
     }
 
@@ -193,8 +200,11 @@ public class JobService {
      *
      */
     public void createSyncJobResult(JobSummary job) {
+        Transfer negotiatedTransfer = null;
         try {
-            uriService.setSyncTransferEndpoints(job);
+            Transfer transfer = uriService.getTransfer(job);
+            negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
+            setJobResults(job, transfer);
             job.setPhase(ExecutionPhase.COMPLETED);
             // Need to catch other exceptions too to avoid inconsistent job status
         } catch (VoSpaceErrorSummarizableException e) {
@@ -207,7 +217,27 @@ public class JobService {
             job.setErrorSummary(ErrorSummaryFactory.newErrorSummary(
                     new InternalFaultException(e)));
         } finally {
-            jobDAO.createJob(job);
+            jobDAO.createJob(job, negotiatedTransfer);
+        }
+    }
+
+    private void setJobResults(JobSummary jobSummary, Transfer transfer) {
+        String baseUrl = servletRequest.getRequestURL().substring(0,
+                servletRequest.getRequestURL().indexOf(servletRequest.getContextPath()));
+        String href = baseUrl + servletRequest.getContextPath()
+                + "/transfers/" + jobSummary.getJobId() + "/results/transferDetails";
+        ResultReference transferDetailsRef = new ResultReference();
+        transferDetailsRef.setId("transferDetails");
+        transferDetailsRef.setHref(href);
+        jobSummary.getResults().add(transferDetailsRef);
+        switch (getJobDirection(transfer)) {
+            case pullFromVoSpace:
+            case pushToVoSpace:
+                ResultReference dataNodeRef = new ResultReference();
+                dataNodeRef.setId("dataNode");
+                dataNodeRef.setHref(transfer.getTarget().get(0));
+                jobSummary.getResults().add(dataNodeRef);
+                break;
         }
     }
 }

src/main/java/it/inaf/oats/vospace/TransferController.java

@@ -53,7 +53,7 @@ public class TransferController {
 
         JobSummary jobSummary = newJobSummary(transfer, principal);
 
-        jobDAO.createJob(jobSummary);
+        jobDAO.createJob(jobSummary, null);
 
         if (phase.isPresent()) {
             jobService.setJobPhase(jobSummary, phase.get());
@@ -157,8 +157,7 @@ public class TransferController {
                 return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
             }
             
-            // TODO: check type
-            return ResponseEntity.ok((Transfer) (job.getJobInfo().getAny().get(0)));
+            return ResponseEntity.ok(jobDAO.getTransferDetails(jobId));
 
         }).orElse(ResponseEntity.notFound().build());
     }

src/main/java/it/inaf/oats/vospace/UriService.java

@@ -31,7 +31,6 @@ import java.util.Optional;
 import java.util.stream.Collectors;
 import javax.servlet.http.HttpServletRequest;
 import net.ivoa.xml.uws.v1.JobSummary;
-import net.ivoa.xml.uws.v1.ResultReference;
 import net.ivoa.xml.vospace.v2.DataNode;
 import net.ivoa.xml.vospace.v2.Node;
 import net.ivoa.xml.vospace.v2.Protocol;
@@ -67,24 +66,17 @@ public class UriService {
     @Autowired
     private FileServiceClient fileServiceClient;
 
-    public void setTransferJobResult(JobSummary job, Transfer transfer) {
-
-        List<ResultReference> results = new ArrayList<>();
-
-        ResultReference result = new ResultReference();
-        result.setHref(getEndpoint(job, transfer));
-        results.add(result);
-
-        job.setResults(results);
-        // Moved phase setting to caller method for ERROR management
-    }
-
     /**
-     * Sets the endpoint value for all valid protocols (protocol negotiation).
+     * For a given job, returns a new transfer object containing only valid
+     * protocols (protocol negotiation) and sets proper endpoints on them.
      */
-    public void setSyncTransferEndpoints(JobSummary job) {
+    public Transfer getNegotiatedTransfer(JobSummary job, Transfer transfer) {
 
-        Transfer transfer = getTransfer(job);
+        // Original transfer object shouldn't be modified, so a new transfer object is created
+        Transfer negotiatedTransfer = new Transfer();
+        negotiatedTransfer.setTarget(transfer.getTarget());
+        negotiatedTransfer.setDirection(transfer.getDirection());
+        // according to examples found in specification view is not copied
         
         if (transfer.getProtocols().isEmpty()) {
             // At least one protocol is expected from client
@@ -97,6 +89,7 @@ public class UriService {
         List<String> validProtocolUris = new ArrayList<>();
         switch (jobDirection) {
             case pullFromVoSpace:
+            case pullToVoSpace:
                 validProtocolUris.add("ivo://ivoa.net/vospace/core#httpget");
                 break;
             case pushToVoSpace:
@@ -109,20 +102,24 @@ public class UriService {
 
         List<Protocol> validProtocols
                 = transfer.getProtocols().stream()
+                        // discard invalid protocols
                         .filter(protocol -> validProtocolUris.contains(protocol.getUri()))
-                        .collect(Collectors.toList());
+                        .map(p -> {
+                            // set endpoints
+                            Protocol protocol = new Protocol();
+                            protocol.setUri(p.getUri());
+                            protocol.setEndpoint(getEndpoint(job, transfer));
+                            return protocol;
+                        }).collect(Collectors.toList());
 
         if (validProtocols.isEmpty()) {
             Protocol protocol = transfer.getProtocols().get(0);
             throw new ProtocolNotSupportedException(protocol.getUri());
         }
 
-        String endpoint = getEndpoint(job, transfer);
-        validProtocols.stream().forEach(p -> p.setEndpoint(endpoint));
+        negotiatedTransfer.getProtocols().addAll(validProtocols);
         
-        // Returns modified transfer containing only valid protocols
-        transfer.getProtocols().clear();
-        transfer.getProtocols().addAll(validProtocols);
+        return negotiatedTransfer;
     }
 
     private Node getEndpointNode(String relativePath,

src/main/java/it/inaf/oats/vospace/exception/InvalidArgumentException.java

@@ -12,6 +12,6 @@ import org.springframework.web.bind.annotation.ResponseStatus;
 public class InvalidArgumentException extends VoSpaceErrorSummarizableException {
 
     public InvalidArgumentException(String message) {
-        super("Description: " + message, VOSpaceFaultEnum.NODE_NOT_FOUND);
+        super("Description: " + message, VOSpaceFaultEnum.INVALID_ARGUMENT);
     }
 }

src/main/java/it/inaf/oats/vospace/persistence/JobDAO.java

@@ -49,12 +49,12 @@ public class JobDAO {
         jdbcTemplate = new JdbcTemplate(dataSource);
     }
 
-    public void createJob(JobSummary jobSummary) {
+    public void createJob(JobSummary jobSummary, Transfer transferDetails) {
 
         String sql
-                = "INSERT INTO job(job_id, owner_id, job_type, phase, job_info,"
-                + " error_message, error_type, error_has_detail, error_detail) "
-                + "VALUES (?, ?, ?, ?, ?, ? ,? ,? ,?)";
+                = "INSERT INTO job(job_id, owner_id, job_type, phase, job_info, transfer_details, "
+                + " results, error_message, error_type, error_has_detail, error_detail) "
+                + "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
 
         jdbcTemplate.update(sql, ps -> {
             int i = 0;
@@ -63,6 +63,8 @@ public class JobDAO {
             ps.setObject(++i, getJobDirection(jobSummary), Types.VARCHAR);
             ps.setObject(++i, jobSummary.getPhase().value(), Types.OTHER);
             ps.setObject(++i, toJson(jobSummary.getJobInfo()), Types.OTHER);
+            ps.setObject(++i, toJson(transferDetails), Types.OTHER);
+            ps.setObject(++i, toJson(jobSummary.getResults()), Types.OTHER);
 
             ErrorSummary errorSummary = jobSummary.getErrorSummary();
             if (errorSummary != null) {
@@ -263,16 +265,16 @@ public class JobDAO {
         }
     }
 
-    public void updateJob(JobSummary job) {
+    public void updateJob(JobSummary job, Transfer transferDetails) {
 
-        String sql = "UPDATE job SET (phase, results";
+        String sql = "UPDATE job SET (phase, results, transfer_details ";
 
         ErrorSummary errorSummary = job.getErrorSummary();
         if (errorSummary != null) {
             sql += ", error_message, error_type, error_has_detail, error_detail";
         }
 
-        sql += ") = (?, ?";
+        sql += ") = (?, ?, ?";
 
         if (errorSummary != null) {
             sql += ", ?, ?, ?, ?";
@@ -284,6 +286,7 @@ public class JobDAO {
             int i = 0;
             ps.setObject(++i, job.getPhase().name(), Types.OTHER);
             ps.setObject(++i, toJson(job.getResults()), Types.OTHER);
+            ps.setObject(++i, toJson(transferDetails), Types.OTHER);
             if (errorSummary != null) {
                 ps.setString(++i, errorSummary.getMessage());
                 ps.setObject(++i, errorSummary.getType().value(), Types.OTHER);
@@ -294,7 +297,26 @@ public class JobDAO {
         });
     }
     
+    public Transfer getTransferDetails(String jobId) {
+
+        String sql = "SELECT transfer_details FROM job WHERE job_id = ?";
+
+        String json = jdbcTemplate.queryForObject(sql, String.class, new Object[]{jobId});
+        if (json == null) {
+            return null;
+        }
+
+        try {
+            return MAPPER.readValue(json, Transfer.class);
+        } catch (JsonProcessingException ex) {
+            throw new RuntimeException(ex);
+        }
+    }
+
     private String toJson(Object data) {
+        if (data == null) {
+            return null;
+        }
         try {
             return MAPPER.writeValueAsString(data);
         } catch (JsonProcessingException ex) {

src/test/java/it/inaf/oats/vospace/JobServiceTest.java

@@ -8,6 +8,7 @@ package it.inaf.oats.vospace;
 import it.inaf.oats.vospace.exception.NodeBusyException;
 import it.inaf.oats.vospace.persistence.JobDAO;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import javax.servlet.http.HttpServletRequest;
 import net.ivoa.xml.uws.v1.ExecutionPhase;
@@ -15,10 +16,12 @@ 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 org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.eq;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import static org.mockito.Mockito.doAnswer;
@@ -53,14 +56,20 @@ public class JobServiceTest {
     @InjectMocks
     private JobService jobService;
     
+    @BeforeEach
+    public void setUp() {
+        when(servletRequest.getRequestURL()).thenReturn(new StringBuffer("http://localhost/vospace/transfer"));
+        when(servletRequest.getContextPath()).thenReturn("/vospace");
+    }
+
     @Test
     public void testStartJobDefault() {
-        when(uriService.getTransfer(any())).thenReturn(getHttpTransfer());
+        when(uriService.getTransfer(any())).thenReturn(getPullFromVoSpaceHttpTransfer());
 
         JobSummary job = new JobSummary();
         jobService.setJobPhase(job, "RUN");
 
-        verify(jobDAO, times(2)).updateJob(job);
+        verify(jobDAO, times(2)).updateJob(eq(job), any());
     }
 
     @Test
@@ -70,7 +79,7 @@ public class JobServiceTest {
         JobSummary job = new JobSummary();
         jobService.setJobPhase(job, "RUN");
 
-        verify(jobDAO, times(2)).updateJob(job);
+        verify(jobDAO, times(2)).updateJob(eq(job), any());
     }
 
     @Test
@@ -82,7 +91,7 @@ public class JobServiceTest {
         JobSummary job = new JobSummary();
         jobService.setJobPhase(job, "RUN");
 
-        verify(jobDAO, times(2)).updateJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())));
+        verify(jobDAO, times(2)).updateJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
     }
 
     @Test
@@ -94,23 +103,23 @@ public class JobServiceTest {
         JobSummary job = new JobSummary();
         jobService.setJobPhase(job, "RUN");
 
-        verify(jobDAO, times(2)).updateJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())));
+        verify(jobDAO, times(2)).updateJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
     }
 
     @Test
     public void testSyncJobResultVoSpaceError() {
-        when(uriService.getTransfer(any())).thenReturn(getHttpTransfer());
-        doThrow(new NodeBusyException("/foo")).when(uriService).setSyncTransferEndpoints(any());
+        when(uriService.getTransfer(any())).thenReturn(getPullFromVoSpaceHttpTransfer());
+        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())));
+        verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
     }
 
     @Test
     public void testSyncJobResultUnexpectedError() {
-        when(uriService.getTransfer(any())).thenReturn(getHttpTransfer());
-        doThrow(new NullPointerException()).when(uriService).setSyncTransferEndpoints(any());
+        when(uriService.getTransfer(any())).thenReturn(getPullFromVoSpaceHttpTransfer());
+        doThrow(new NullPointerException()).when(uriService).getNegotiatedTransfer(any(), any());
         jobService.createSyncJobResult(new JobSummary());
-        verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())));
+        verify(jobDAO, times(1)).createJob(argThat(j -> ExecutionPhase.ERROR.equals(j.getPhase())), any());
     }
 
     @Test
@@ -134,7 +143,7 @@ public class JobServiceTest {
     @Test
     public void testStartJobSetExecutingPhaseForAsyncPullFromVoSpace() {
 
-        Transfer httpTransfer = getHttpTransfer();
+        Transfer httpTransfer = getPullFromVoSpaceHttpTransfer();
 
         JobSummary job = new JobSummary();
         setJobInfo(job, httpTransfer);
@@ -163,20 +172,21 @@ public class JobServiceTest {
             JobSummary j = invocation.getArgument(0);
             phases.add(j.getPhase());
             return null;
-        }).when(jobDAO).updateJob(any());
+        }).when(jobDAO).updateJob(any(), any());
 
         jobService.setJobPhase(job, "RUN");
 
         verify(moveService, timeout(1000).times(1)).processMoveJob(any(), any());
 
-        verify(jobDAO, times(3)).updateJob(any());
+        verify(jobDAO, times(3)).updateJob(any(), any());
         assertEquals(ExecutionPhase.EXECUTING, phases.get(0));
         assertEquals(ExecutionPhase.EXECUTING, phases.get(1));
         assertEquals(ExecutionPhase.COMPLETED, phases.get(2));
     }
 
-    private Transfer getHttpTransfer() {
+    private Transfer getPullFromVoSpaceHttpTransfer() {
         Transfer transfer = new Transfer();
+        transfer.setTarget(Arrays.asList("vos://example.com!vospace/myfile"));
         transfer.setDirection("pullFromVoSpace");
         Protocol protocol = new Protocol();
         protocol.setUri("ivo://ivoa.net/vospace/core#httpget");

src/test/java/it/inaf/oats/vospace/TransferControllerTest.java

@@ -20,6 +20,7 @@ import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.sql.Timestamp;
 import java.time.LocalDateTime;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Optional;
 import net.ivoa.xml.uws.v1.ExecutionPhase;
@@ -55,14 +56,17 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
 import org.w3c.dom.Document;
 import java.util.List;
 import net.ivoa.xml.uws.v1.ErrorSummary;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import org.junit.jupiter.api.BeforeEach;
+import org.mockito.ArgumentCaptor;
 import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.Mockito.doAnswer;
 
 @SpringBootTest
 @AutoConfigureMockMvc
 @ContextConfiguration(classes = {TokenFilterConfig.class})
-@TestPropertySource(properties = "spring.main.allow-bean-definition-overriding=true")
+@TestPropertySource(properties = {"spring.main.allow-bean-definition-overriding=true", "file-service-url=http://file-service"})
 public class TransferControllerTest {
 
     @MockBean
@@ -100,21 +104,10 @@ public class TransferControllerTest {
 
     @Test
     public void testPullFromVoSpaceAsync() throws Exception {
-
-        Node node = mockPublicDataNode();
-        when(nodeDao.listNode(eq("/mynode"))).thenReturn(Optional.of(node));
-
-        String requestBody = getResourceFileContent("pullFromVoSpace.xml");
-
-        String redirect = mockMvc.perform(post("/transfers?PHASE=RUN")
-                .content(requestBody)
-                .contentType(MediaType.APPLICATION_XML)
-                .accept(MediaType.APPLICATION_XML))
-                .andDo(print())
-                .andExpect(status().is3xxRedirection())
-                .andReturn().getResponse().getHeader("Location");
-
-        assertThat(redirect, matchesPattern("^/transfers/.*"));
+        // job completion will be set by file service
+        String endpoint = testAsyncTransferNegotiation("/mynode",
+                getResourceFileContent("pullFromVoSpace.xml"), ExecutionPhase.EXECUTING);
+        assertTrue(endpoint.startsWith("http://file-service/mynode?jobId="));
     }
 
     @Test
@@ -134,15 +127,22 @@ public class TransferControllerTest {
                 .andReturn().getResponse().getHeader("Location");
 
         assertThat(redirect, matchesPattern("^/transfers/.*/results/transferDetails"));
+
+        verify(jobDao, times(1)).createJob(argThat(j -> {
+            return ExecutionPhase.COMPLETED == j.getPhase()
+                    && j.getResults().get(0).getHref().contains("/transferDetails");
+        }), argThat(t -> {
+            return t.getProtocols().get(0).getEndpoint().startsWith("http://file-service/mynode?jobId=");
+        }));
     }
 
     @Test
     public void testPullToVoSpaceTape() throws Exception {
-        testPullToVoSpace("/mynode", getResourceFileContent("pullToVoSpace-tape.xml"));
+        testVoSpaceAsyncTransfer("/mynode", getResourceFileContent("pullToVoSpace-tape.xml"));
 
         verify(asyncTransfService, times(1)).startJob(any());
 
-        verify(jobDao, times(2)).updateJob(argThat(j -> ExecutionPhase.QUEUED == j.getPhase()));
+        verify(jobDao, times(2)).updateJob(argThat(j -> ExecutionPhase.QUEUED == j.getPhase()), any());
     }
 
     @Test
@@ -150,29 +150,52 @@ public class TransferControllerTest {
 
         when(nodeDao.getNodeOsName(eq("/portalnode"))).thenReturn("file.fits");
 
-        testPullToVoSpace("/portalnode", getResourceFileContent("pullToVoSpace-portal.xml"));
+        String endpoint = testAsyncTransferNegotiation("/portalnode",
+                getResourceFileContent("pullToVoSpace-portal.xml"), ExecutionPhase.COMPLETED);
 
-        verify(nodeDao, times(1)).setNodeLocation(eq("/portalnode"), eq(2), eq("lbcr.20130512.060722.fits.gz"));
+        assertTrue(endpoint.startsWith("http://archive.lbto.org"));
 
-        verify(jobDao, times(2)).updateJob(argThat(j -> {
-            assertTrue(j.getResults().get(0).getHref().startsWith("http://archive.lbto.org"));
-            assertEquals(ExecutionPhase.COMPLETED, j.getPhase());
-            return true;
-        }));
+        verify(nodeDao, times(1)).setNodeLocation(eq("/portalnode"), eq(2), eq("lbcr.20130512.060722.fits.gz"));
     }
 
     @Test
     public void testPushToVoSpace() throws Exception {
+        // job completion will be set by file service
+        String endpoint = testAsyncTransferNegotiation("/uploadedfile",
+                getResourceFileContent("pushToVoSpace.xml"), ExecutionPhase.EXECUTING);
+        assertTrue(endpoint.startsWith("http://file-service/uploadedfile?jobId="));
+    }
 
-        when(nodeDao.getNodeOsName(eq("/uploadedfile"))).thenReturn("file.fits");
+    private String testAsyncTransferNegotiation(String path, String requestBody, ExecutionPhase endPhase) throws Exception {
 
-        testPullToVoSpace("/uploadedfile", getResourceFileContent("pushToVoSpace.xml"));
+        // detect phase updates
+        List<ExecutionPhase> phases = new ArrayList<>();
+        List<Transfer> negotiatedTransfers = new ArrayList<>();
+        doAnswer(invocation -> {
+            phases.add(((JobSummary) invocation.getArgument(0)).getPhase());
+            negotiatedTransfers.add(invocation.getArgument(1));
+            return null;
+        }).when(jobDao).updateJob(any(), any());
 
-        // job completion will be set by file service
-        verify(jobDao, times(2)).updateJob(argThat(j -> ExecutionPhase.EXECUTING == j.getPhase()));
+        testVoSpaceAsyncTransfer(path, requestBody);
+
+        ArgumentCaptor<JobSummary> jobCaptor = ArgumentCaptor.forClass(JobSummary.class);
+        verify(jobDao, times(2)).updateJob(jobCaptor.capture(), any());
+
+        assertEquals(2, phases.size());
+        assertEquals(ExecutionPhase.EXECUTING, phases.get(0));
+        assertEquals(endPhase, phases.get(1));
+
+        JobSummary job = jobCaptor.getAllValues().get(1);
+        assertEquals(endPhase, job.getPhase());
+        assertTrue(job.getResults().get(0).getHref().contains("/transferDetails"));
+
+        assertNull(negotiatedTransfers.get(0));
+        Transfer negotiatedTransfer = negotiatedTransfers.get(1);
+        return negotiatedTransfer.getProtocols().get(0).getEndpoint();
     }
 
-    private void testPullToVoSpace(String path, String requestBody) throws Exception {
+    private void testVoSpaceAsyncTransfer(String path, String requestBody) throws Exception {
 
         Node node = mockPublicDataNode();
         when(nodeDao.listNode(eq(path))).thenReturn(Optional.of(node));
@@ -209,7 +232,7 @@ public class TransferControllerTest {
                 .andExpect(status().is3xxRedirection())
                 .andReturn().getResponse().getHeader("Location");
 
-        verify(jobDao, times(2)).updateJob(any());
+        verify(jobDao, times(2)).updateJob(any(), any());
 
         assertThat(redirect, matchesPattern("^/transfers/.*"));
     }
@@ -220,6 +243,8 @@ public class TransferControllerTest {
         JobSummary job = getFakePendingJob();
         when(jobDao.getJob(eq("123"))).thenReturn(Optional.of(job));
         
+        when(jobDao.getTransferDetails(eq("123"))).thenReturn(new Transfer());
+
         mockMvc.perform(get("/transfers/123/results/transferDetails")
                 .header("Authorization", "Bearer user1_token")
                 .accept(MediaType.APPLICATION_XML))

src/test/java/it/inaf/oats/vospace/UriServiceTest.java

@@ -30,6 +30,7 @@ import net.ivoa.xml.vospace.v2.Transfer;
 import net.ivoa.xml.vospace.v2.View;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -110,9 +111,9 @@ public class UriServiceTest {
         when(nodeDAO.listNode(eq("/mydata1"))).thenReturn(Optional.of(node));
 
         JobSummary job = getJob();
-        uriService.setTransferJobResult(job, uriService.getTransfer(job));
+        Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, uriService.getTransfer(job));
 
-        assertEquals("http://file-service/mydata1?jobId=job-id", job.getResults().get(0).getHref());
+        assertEquals("http://file-service/mydata1?jobId=job-id", negotiatedTransfer.getProtocols().get(0).getEndpoint());
     }
 
     @Test
@@ -145,9 +146,9 @@ public class UriServiceTest {
 
         JobSummary job = getJob();
         Transfer tr = uriService.getTransfer(job);
-        uriService.setTransferJobResult(job, tr);
+        Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, tr);
 
-        assertEquals("http://file-service/mydata1?jobId=job-id&token=<new-token>", job.getResults().get(0).getHref());
+        assertEquals("http://file-service/mydata1?jobId=job-id&token=<new-token>", negotiatedTransfer.getProtocols().get(0).getEndpoint());
     }
     
     @Test
@@ -180,8 +181,9 @@ public class UriServiceTest {
 
         JobSummary job = getJob();
         Transfer tr = uriService.getTransfer(job);
-        assertThrows(PermissionDeniedException.class, 
-                ()->{ uriService.setTransferJobResult(job, tr);});        
+        assertThrows(PermissionDeniedException.class, () -> {
+            uriService.getNegotiatedTransfer(job, tr);
+        });
     }
     
     @Test
@@ -216,8 +218,9 @@ public class UriServiceTest {
 
         JobSummary job = getJob();
         Transfer tr = uriService.getTransfer(job);
-        assertThrows(NodeBusyException.class, 
-                ()->{ uriService.setTransferJobResult(job, tr);});        
+        assertThrows(NodeBusyException.class, () -> {
+            uriService.getNegotiatedTransfer(job, tr);
+        });
     }
 
     @Test
@@ -265,11 +268,11 @@ public class UriServiceTest {
 
         when(createNodeService.createNode(any(), any(), eq(user))).thenReturn(dnode);
 
-        uriService.setTransferJobResult(job, tr);
+        Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, tr);
 
         verify(createNodeService, times(1)).createNode(any(), any(), eq(user));
 
-        assertEquals("http://file-service/mydata1/mydata2?jobId=job-id2&token=<new-token>", job.getResults().get(0).getHref());
+        assertEquals("http://file-service/mydata1/mydata2?jobId=job-id2&token=<new-token>", negotiatedTransfer.getProtocols().get(0).getEndpoint());
     }
 
     @Test
@@ -312,11 +315,11 @@ public class UriServiceTest {
 
         assertEquals(2, transfer.getProtocols().size());
 
-        uriService.setSyncTransferEndpoints(job);
+        Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
 
         // invalid protocol is removed
-        assertEquals(1, transfer.getProtocols().size());
-        assertEquals("ivo://ivoa.net/vospace/core#httpget", transfer.getProtocols().get(0).getUri());
+        assertEquals(1, negotiatedTransfer.getProtocols().size());
+        assertEquals("ivo://ivoa.net/vospace/core#httpget", negotiatedTransfer.getProtocols().get(0).getUri());
     }
 
     @Test
@@ -352,11 +355,11 @@ public class UriServiceTest {
 
         assertEquals(2, transfer.getProtocols().size());
 
-        uriService.setSyncTransferEndpoints(job);
+        Transfer negotiatedTransfer = uriService.getNegotiatedTransfer(job, transfer);
 
         // invalid protocol is removed
-        assertEquals(1, transfer.getProtocols().size());
-        assertEquals("ivo://ivoa.net/vospace/core#httpput", transfer.getProtocols().get(0).getUri());
+        assertEquals(1, negotiatedTransfer.getProtocols().size());
+        assertEquals("ivo://ivoa.net/vospace/core#httpput", negotiatedTransfer.getProtocols().get(0).getUri());
     }
 
     @Test
@@ -376,7 +379,7 @@ public class UriServiceTest {
         job.setJobInfo(jobInfo);
 
         try {
-            uriService.setSyncTransferEndpoints(job);
+            uriService.getNegotiatedTransfer(job, transfer);
             fail("Expected ProtocolNotSupportedException");
         } catch (ProtocolNotSupportedException ex) {
         }
@@ -395,7 +398,7 @@ public class UriServiceTest {
         job.setJobInfo(jobInfo);
 
         try {
-            uriService.setSyncTransferEndpoints(job);
+            uriService.getNegotiatedTransfer(job, transfer);
             fail("Expected InvalidArgumentException");
         } catch (InvalidArgumentException ex) {
         }
@@ -411,11 +414,35 @@ public class UriServiceTest {
         testArchiveViewEndpoint(Views.ZIP_VIEW_URI);
     }
     
+    @Test
+    public void testInvalidTransferNoProtocols() {
+
+        Transfer transfer = new Transfer();
+        transfer.setDirection("pullFromVoSpace");
+        transfer.setTarget(Arrays.asList("vos://example.com!vospace/file1"));
+
+        JobSummary job = new JobSummary();
+        JobSummary.JobInfo jobInfo = new JobSummary.JobInfo();
+        jobInfo.getAny().add(transfer);
+        job.setJobInfo(jobInfo);
+
+        mockPublicNode("file1");
+        mockPublicNode("file2");
+
+        InvalidArgumentException ex = assertThrows(InvalidArgumentException.class, () -> {
+            uriService.getNegotiatedTransfer(job, transfer);
+        });
+        assertTrue(ex.getMessage().contains("no protocol"));
+    }
+
     private void testArchiveViewEndpoint(String viewUri) {
 
         Transfer transfer = new Transfer();
         transfer.setDirection("pullFromVoSpace");
         transfer.setTarget(Arrays.asList("vos://example.com!vospace/file1", "vos://example.com!vospace/file2"));
+        Protocol protocol = new Protocol();
+        protocol.setUri("ivo://ivoa.net/vospace/core#httpget");
+        transfer.getProtocols().add(protocol);
         View view = new View();
         view.setUri(viewUri);
         transfer.setView(view);
@@ -429,7 +456,7 @@ public class UriServiceTest {
         mockPublicNode("file1");
         mockPublicNode("file2");
 
-        uriService.setTransferJobResult(job, transfer);
+        uriService.getNegotiatedTransfer(job, transfer);
 
         verify(fileServiceClient, times(1)).startArchiveJob(transfer, "archive-job-id");
     }
@@ -454,6 +481,9 @@ public class UriServiceTest {
         Transfer transfer = new Transfer();
         transfer.setTarget(Arrays.asList("vos://example.com!vospace/mydata1"));
         transfer.setDirection(JobService.JobDirection.pullFromVoSpace.toString());
+        Protocol protocol = new Protocol();
+        protocol.setUri("ivo://ivoa.net/vospace/core#httpget");
+        transfer.getProtocols().add(protocol);
 
         JobSummary job = new JobSummary();
         job.setJobId("job-id");
@@ -470,6 +500,9 @@ public class UriServiceTest {
         Transfer transfer = new Transfer();
         transfer.setTarget(Arrays.asList("vos://example.com!vospace/mydata1/mydata2"));
         transfer.setDirection(JobService.JobDirection.pushToVoSpace.toString());
+        Protocol protocol = new Protocol();
+        protocol.setUri("ivo://ivoa.net/vospace/core#httpput");
+        transfer.getProtocols().add(protocol);
 
         JobSummary job = new JobSummary();
         job.setJobId("job-id2");

src/test/java/it/inaf/oats/vospace/persistence/JobDAOTest.java

@@ -29,6 +29,7 @@ import net.ivoa.xml.uws.v1.Jobs;
 import it.inaf.oats.vospace.exception.ErrorSummaryFactory;
 import it.inaf.oats.vospace.exception.PermissionDeniedException;
 import java.util.Arrays;
+import net.ivoa.xml.uws.v1.ResultReference;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 
@@ -75,7 +76,7 @@ public class JobDAOTest {
 
         JobSummary job = getJob();
 
-        dao.createJob(job);
+        dao.createJob(job, null);
 
         assertTrue(dao.getJob("123").isPresent());
         assertEquals(ExecutionPhase.PENDING, dao.getJob("123").get().getPhase());
@@ -86,12 +87,53 @@ public class JobDAOTest {
         assertNull(job.getStartTime());
 
         job.setPhase(ExecutionPhase.EXECUTING);
-        dao.updateJob(job);
+        dao.updateJob(job, null);
 
         job = dao.getJob("123").get();
         assertEquals(ExecutionPhase.EXECUTING, job.getPhase());
         assertNotNull(job.getStartTime());
         assertNull(job.getEndTime());
+
+        assertNull(dao.getTransferDetails(job.getJobId()));
+
+        Transfer negotiatedTransfer = new Transfer();
+        job.setPhase(ExecutionPhase.COMPLETED);
+        dao.updateJob(job, negotiatedTransfer);
+
+        job = dao.getJob("123").get();
+        assertEquals(ExecutionPhase.COMPLETED, job.getPhase());
+        assertNotNull(job.getStartTime());
+        assertNotNull(job.getEndTime());
+        assertNotNull(dao.getTransferDetails(job.getJobId()));
+    }
+    
+    /**
+     * Jobs created by the /synctrans endpoint contains results list at creation time.
+     */
+    @Test
+    public void testCreateJobWithResults() {
+        JobSummary job = getJob();
+        job.setPhase(ExecutionPhase.COMPLETED);
+
+        ResultReference result = new ResultReference();
+        result.setId("transferDetails");
+        result.setHref("http://ia2.inaf.it");
+        job.getResults().add(result);
+
+        Transfer negotiatedTransfer = new Transfer();
+
+        dao.createJob(job, negotiatedTransfer);
+
+        // Retrieve it back        
+        Optional<JobSummary> retrievedJobOpt = dao.getJob(job.getJobId());
+        assertTrue(retrievedJobOpt.isPresent());
+
+        JobSummary retrievedJob = retrievedJobOpt.get();
+        assertEquals(1, retrievedJob.getResults().size());
+        assertNotNull(retrievedJob.getStartTime());
+        assertNotNull(retrievedJob.getEndTime());
+
+        assertNotNull(dao.getTransferDetails(retrievedJob.getJobId()));
     }
 
     @Test
@@ -111,7 +153,7 @@ public class JobDAOTest {
 
         job.setErrorSummary(errorSummary);
 
-        dao.createJob(job);
+        dao.createJob(job, null);
 
         // Retrieve it back        
         Optional<JobSummary> retrievedJobOpt = dao.getJob(job.getJobId());
@@ -120,14 +162,15 @@ public class JobDAOTest {
         JobSummary retrievedJob = retrievedJobOpt.get();
         assertEquals(ExecutionPhase.ERROR, retrievedJob.getPhase());
         assertTrue(areEqual(job.getErrorSummary(), retrievedJob.getErrorSummary()));
-
+        assertNotNull(retrievedJob.getStartTime());
+        assertNotNull(retrievedJob.getEndTime());
     }
 
     @Test
     public void testUpdateJobWithError() {
         JobSummary job = getJob();
 
-        dao.createJob(job);
+        dao.createJob(job, null);
 
         job.setPhase(ExecutionPhase.ERROR);
         // Generate it from exception        
@@ -141,7 +184,7 @@ public class JobDAOTest {
                 
         job.setErrorSummary(errorSummary);
 
-        dao.updateJob(job);
+        dao.updateJob(job, null);
 
         // Retrieve it back        
         Optional<JobSummary> retrievedJobOpt = dao.getJob(job.getJobId());