diff --git a/src/main/java/it/inaf/oats/vospace/JobService.java b/src/main/java/it/inaf/oats/vospace/JobService.java index a8baf0eb43acd8a47bac90fc8eeeeb2564bec472..77c7e047ab468cf504af30f97802d97aca87e1a7 100644 --- a/src/main/java/it/inaf/oats/vospace/JobService.java +++ b/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; @@ -99,16 +100,19 @@ public class JobService { phase = ExecutionPhase.EXECUTING; } job.setPhase(phase); + + jobDAO.updateJob(job, null); - jobDAO.updateJob(job); + 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; } } } diff --git a/src/main/java/it/inaf/oats/vospace/TransferController.java b/src/main/java/it/inaf/oats/vospace/TransferController.java index eda5ff0f1ea8fe9af09ac8685242c6baee924bd5..235003cfbe178d04c84e942d5bd6ab3bfa6ac6bb 100644 --- a/src/main/java/it/inaf/oats/vospace/TransferController.java +++ b/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()); } diff --git a/src/main/java/it/inaf/oats/vospace/UriService.java b/src/main/java/it/inaf/oats/vospace/UriService.java index aa8db7b479e895e9fff4c0a87d2b22facf433940..de871b380807b86e18b14732d9a3135876346752 100644 --- a/src/main/java/it/inaf/oats/vospace/UriService.java +++ b/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,25 +66,18 @@ 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) { - - Transfer transfer = getTransfer(job); - + public Transfer getNegotiatedTransfer(JobSummary job, Transfer transfer) { + + // 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 throw new InvalidArgumentException("Transfer contains no protocols"); @@ -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)); - - // Returns modified transfer containing only valid protocols - transfer.getProtocols().clear(); - transfer.getProtocols().addAll(validProtocols); + negotiatedTransfer.getProtocols().addAll(validProtocols); + + return negotiatedTransfer; } private Node getEndpointNode(String relativePath, diff --git a/src/main/java/it/inaf/oats/vospace/exception/InvalidArgumentException.java b/src/main/java/it/inaf/oats/vospace/exception/InvalidArgumentException.java index d3bf46ec98b40acd2f552f89143dc1c811a04302..b94230a94a3d43bc054a8b2eee70742a8d914ce2 100644 --- a/src/main/java/it/inaf/oats/vospace/exception/InvalidArgumentException.java +++ b/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); } } diff --git a/src/main/java/it/inaf/oats/vospace/persistence/JobDAO.java b/src/main/java/it/inaf/oats/vospace/persistence/JobDAO.java index c231c2c764b3e07bc9cb117b9cae51d2340058c1..fef4d399cf8f10c1806be7e783411a22b678b370 100644 --- a/src/main/java/it/inaf/oats/vospace/persistence/JobDAO.java +++ b/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); @@ -293,8 +296,27 @@ public class JobDAO { ps.setString(++i, job.getJobId()); }); } + + 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) { diff --git a/src/test/java/it/inaf/oats/vospace/JobServiceTest.java b/src/test/java/it/inaf/oats/vospace/JobServiceTest.java index 1d19b72cdb38c84383debc716371f6acf2319124..a0e7b73899a031675d7ea372a358ee8a1e68384f 100644 --- a/src/test/java/it/inaf/oats/vospace/JobServiceTest.java +++ b/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; @@ -52,15 +55,21 @@ 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"); diff --git a/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java b/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java index 87754f08f67bd70e59c57f9043ce86a710ee7b2e..eff82558d8830903b603e442bf410ec600c86ea0 100644 --- a/src/test/java/it/inaf/oats/vospace/TransferControllerTest.java +++ b/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")); - - // job completion will be set by file service - verify(jobDao, times(2)).updateJob(argThat(j -> ExecutionPhase.EXECUTING == j.getPhase())); + // 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()); + + 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/.*")); } @@ -219,6 +242,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") @@ -280,53 +305,53 @@ public class TransferControllerTest { verify(jobDao, times(1)).getJob(eq("123")); } - + @Test - public void testErrorEndpoint() throws Exception { + public void testErrorEndpoint() throws Exception { JobSummary job = new JobSummary(); job.setJobId("123"); job.setPhase(ExecutionPhase.EXECUTING); ErrorSummary e = ErrorSummaryFactory.newErrorSummary( new PermissionDeniedException("/pippo1/pippo2") ); - job.setErrorSummary(e); + job.setErrorSummary(e); when(jobDao.getJob(eq("123"))).thenReturn(Optional.of(job)); - + String response = mockMvc.perform(get("/transfers/123/error") .accept(MediaType.TEXT_PLAIN_VALUE)) .andDo(print()) .andExpect(status().isOk()) .andReturn().getResponse().getContentAsString(); - + assertEquals("Job is not in ERROR phase", response); - + job.setPhase(ExecutionPhase.ERROR); - + response = mockMvc.perform(get("/transfers/123/error") .accept(MediaType.TEXT_PLAIN_VALUE)) .andDo(print()) .andExpect(status().isOk()) .andReturn().getResponse().getContentAsString(); - + assertEquals(e.getDetailMessage(), response); - - e.setHasDetail(false); - + + e.setHasDetail(false); + response = mockMvc.perform(get("/transfers/123/error") .accept(MediaType.TEXT_PLAIN_VALUE)) .andDo(print()) .andExpect(status().isOk()) .andReturn().getResponse().getContentAsString(); - + assertEquals("No error details available", response); - + when(jobDao.getJob(eq("124"))).thenReturn(Optional.ofNullable(null)); - + mockMvc.perform(get("/transfers/124/error") .accept(MediaType.TEXT_PLAIN_VALUE)) .andDo(print()) - .andExpect(status().is4xxClientError()); + .andExpect(status().is4xxClientError()); } @Test @@ -409,7 +434,7 @@ public class TransferControllerTest { } protected static String getResourceFileContent(String fileName) throws Exception { - try (InputStream in = TransferControllerTest.class.getClassLoader().getResourceAsStream(fileName)) { + try ( InputStream in = TransferControllerTest.class.getClassLoader().getResourceAsStream(fileName)) { return new String(in.readAllBytes(), StandardCharsets.UTF_8); } } diff --git a/src/test/java/it/inaf/oats/vospace/UriServiceTest.java b/src/test/java/it/inaf/oats/vospace/UriServiceTest.java index d1dbe7d475aaad453e0dd7fc4f281a9275bf0617..8806661300c447d9c9b3a4fcb83a196bb41a27a9 100644 --- a/src/test/java/it/inaf/oats/vospace/UriServiceTest.java +++ b/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) { } @@ -410,12 +413,36 @@ public class UriServiceTest { public void testZipArchiveViewEndpoint() { 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"); diff --git a/src/test/java/it/inaf/oats/vospace/persistence/JobDAOTest.java b/src/test/java/it/inaf/oats/vospace/persistence/JobDAOTest.java index 47e806004305f5d84c847ccdb631dade1353ce4b..1657cee60383bc1130ba83e61136f410436793e2 100644 --- a/src/test/java/it/inaf/oats/vospace/persistence/JobDAOTest.java +++ b/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());