diff --git a/CHANGELOG.md b/CHANGELOG.md index 17e42ed89a8..229e3841cc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,8 @@ Changes 21.01 to 20.05 -------------- -*[UI]: Fixed bug which was preventing a user from viewing more than 5 rows of a tabular result file. (21.01.1) +* [UI]: Fixed bug which was preventing a user from viewing more than 5 rows of a tabular result file. (21.01.1) +* [REST]: Added `ownership` option when copying sample to project in REST API (21.01.2) 20.09 to 21.01 -------------- diff --git a/pom.xml b/pom.xml index ff7930c9a38..3b0d22dc468 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ ca.corefacility.bioinformatics irida war - 21.01.1 + 21.01.2 irida http://www.irida.ca diff --git a/src/main/java/ca/corefacility/bioinformatics/irida/web/controller/api/projects/RESTProjectSamplesController.java b/src/main/java/ca/corefacility/bioinformatics/irida/web/controller/api/projects/RESTProjectSamplesController.java index c60fe2677eb..61cd3f60c25 100644 --- a/src/main/java/ca/corefacility/bioinformatics/irida/web/controller/api/projects/RESTProjectSamplesController.java +++ b/src/main/java/ca/corefacility/bioinformatics/irida/web/controller/api/projects/RESTProjectSamplesController.java @@ -14,10 +14,7 @@ import org.springframework.http.MediaType; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RequestBody; -import org.springframework.web.bind.annotation.RequestMapping; -import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.*; import org.springframework.web.servlet.ModelAndView; import org.springframework.web.servlet.view.RedirectView; @@ -91,14 +88,16 @@ public RESTProjectSamplesController(ProjectService projectService, SampleService * * @param projectId the project to copy the sample to. * @param sampleIds the collection of sample IDs to copy. + * @param ownership Whether the receiving project should have ownership of the sample * @param response a reference to the servlet response. - * @param locale The user's in case a warning message is needed + * @param locale The user's in case a warning message is needed * @return the response indicating that the sample was joined to the * project. */ @RequestMapping(value = "/api/projects/{projectId}/samples", method = RequestMethod.POST, consumes = "application/idcollection+json") public ModelMap copySampleToProject(final @PathVariable Long projectId, final @RequestBody List sampleIds, - HttpServletResponse response, Locale locale) { + @RequestParam(name = "ownership", defaultValue = "false") boolean ownership, HttpServletResponse response, + Locale locale) { ModelMap modelMap = new ModelMap(); @@ -112,7 +111,7 @@ public ModelMap copySampleToProject(final @PathVariable Long projectId, final @R Sample sample = sampleService.read(sampleId); Join join = null; try { - join = projectService.addSampleToProject(p, sample, false); + join = projectService.addSampleToProject(p, sample, ownership); } catch (ExistingSampleNameException e) { logger.error( "Could not add sample to project because another sample exists with this name :" + e.getSample() diff --git a/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/integration/project/ProjectSamplesIT.java b/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/integration/project/ProjectSamplesIT.java index 170a42e8acb..f1740bfa1fd 100644 --- a/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/integration/project/ProjectSamplesIT.java +++ b/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/integration/project/ProjectSamplesIT.java @@ -15,7 +15,10 @@ import org.junit.runner.RunWith; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; +import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.security.test.context.support.WithSecurityContextTestExecutionListener; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.TestExecutionListeners; @@ -25,6 +28,11 @@ import ca.corefacility.bioinformatics.irida.config.data.IridaApiJdbcDataSourceConfig; import ca.corefacility.bioinformatics.irida.config.services.IridaApiPropertyPlaceholderConfig; +import ca.corefacility.bioinformatics.irida.config.services.IridaApiServicesConfig; +import ca.corefacility.bioinformatics.irida.model.joins.impl.ProjectSampleJoin; +import ca.corefacility.bioinformatics.irida.model.project.Project; +import ca.corefacility.bioinformatics.irida.service.ProjectService; +import ca.corefacility.bioinformatics.irida.service.sample.SampleService; import ca.corefacility.bioinformatics.irida.web.controller.test.integration.util.ITestAuthUtils; import ca.corefacility.bioinformatics.irida.web.controller.test.integration.util.ITestSystemProperties; @@ -43,35 +51,91 @@ /** * Integration tests for project samples. - * */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(loader = AnnotationConfigContextLoader.class, classes = { IridaApiJdbcDataSourceConfig.class, - IridaApiPropertyPlaceholderConfig.class }) -@TestExecutionListeners({ DependencyInjectionTestExecutionListener.class, DbUnitTestExecutionListener.class }) + IridaApiPropertyPlaceholderConfig.class, IridaApiServicesConfig.class }) +@TestExecutionListeners({ DependencyInjectionTestExecutionListener.class, DbUnitTestExecutionListener.class, + WithSecurityContextTestExecutionListener.class }) @ActiveProfiles("it") @DatabaseSetup("/ca/corefacility/bioinformatics/irida/web/controller/test/integration/project/ProjectSamplesIntegrationTest.xml") @DatabaseTearDown("classpath:/ca/corefacility/bioinformatics/irida/test/integration/TableReset.xml") public class ProjectSamplesIT { private static final Logger logger = LoggerFactory.getLogger(ProjectSamplesIT.class); - @Test - public void testShareSampleToProject() { - final List samples = Lists.newArrayList("1"); + @Autowired + SampleService sampleService; + @Autowired + ProjectService projectService; - final String projectUri = "/api/projects/4"; - final String projectJson = asUser().get(projectUri).asString(); + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testShareSampleToProjectWithoutOwnershipDefault() { + Long projectId = 4L; + Long sampleId = 1L; + final List samples = Lists.newArrayList(sampleId); + + final String projectUri = "/api/projects/" + projectId; + final String projectJson = asUser().get(projectUri) + .asString(); final String samplesUri = from(projectJson).get("resource.links.find{it.rel == 'project/samples'}.href"); assertTrue("The samples URI should end with /api/projects/4/samples", - samplesUri.endsWith("/api/projects/4/samples")); - final Response r = asUser().contentType(ContentType.JSON).body(samples) - .header("Content-Type", "application/idcollection+json").expect().response() - .statusCode(HttpStatus.CREATED.value()).when().post(samplesUri); + samplesUri.endsWith("/api/projects/" + projectId + "/samples")); + final Response r = asUser().contentType(ContentType.JSON) + .body(samples) + .header("Content-Type", "application/idcollection+json") + .expect() + .response() + .statusCode(HttpStatus.CREATED.value()) + .when() + .post(samplesUri); + final String location = r.getHeader(HttpHeaders.LOCATION); + assertNotNull("Location should not be null.", location); + assertEquals("The project/sample location uses the wrong sample ID.", + ITestSystemProperties.BASE_URL + "/api/projects/4/samples/1", location); + + Project project = projectService.read(projectId); + + ProjectSampleJoin sampleForProject = sampleService.getSampleForProject(project, sampleId); + assertFalse("Project should not be owner of this sample", sampleForProject.isOwner()); + } + + @Test + @WithMockUser(username = "admin", roles = "ADMIN") + public void testShareSampleToProjectWithOwnership() { + Long projectId = 4L; + Long sampleId = 1L; + final List samples = Lists.newArrayList(sampleId); + + final String projectUri = "/api/projects/" + projectId; + final String projectJson = asUser().get(projectUri) + .asString(); + String samplesUri = from(projectJson).get("resource.links.find{it.rel == 'project/samples'}.href"); + + assertTrue("The samples URI should end with /api/projects/4/samples", + samplesUri.endsWith("/api/projects/" + projectId + "/samples")); + + //adding ownership flag + samplesUri = samplesUri + "?ownership=true"; + + final Response r = asUser().contentType(ContentType.JSON) + .body(samples) + .header("Content-Type", "application/idcollection+json") + .expect() + .response() + .statusCode(HttpStatus.CREATED.value()) + .when() + .post(samplesUri); final String location = r.getHeader(HttpHeaders.LOCATION); assertNotNull("Location should not be null.", location); - assertEquals("The project/sample location uses the wrong sample ID.", ITestSystemProperties.BASE_URL - + "/api/projects/4/samples/1", location); + assertEquals("The project/sample location uses the wrong sample ID.", + ITestSystemProperties.BASE_URL + "/api/projects/4/samples/1", location); + + Project project = projectService.read(projectId); + + ProjectSampleJoin sampleForProject = sampleService.getSampleForProject(project, sampleId); + assertTrue("Project should be owner of this sample", sampleForProject.isOwner()); } @Test @@ -79,7 +143,8 @@ public void testShareSampleToProjectWithSameId() { final List samples = Lists.newArrayList("1"); final String projectUri = "/api/projects/1"; - final String projectJson = asUser().get(projectUri).asString(); + final String projectJson = asUser().get(projectUri) + .asString(); final String samplesUri = from(projectJson).get("resource.links.find{it.rel == 'project/samples'}.href"); //this used to return CONFLICT, but ended up in errors where samples were partially added. Now it accepts the POST but doesn't change anything since the sample is already there @@ -101,38 +166,45 @@ public void testAddMultithreadedSamplesToProject() throws InterruptedException { // load a project final String projectUri = "/api/projects/1"; // get the uri for creating samples associated with the project. - final String projectJson = asUser().get(projectUri).asString(); + final String projectJson = asUser().get(projectUri) + .asString(); final String samplesUri = from(projectJson).get("resource.links.find{it.rel == 'project/samples'}.href"); final Callable> task = () -> { final List responses = new CopyOnWriteArrayList<>(); - final Random rand = new Random(Thread.currentThread().getId()); + final Random rand = new Random(Thread.currentThread() + .getId()); for (int i = 0; i < numberOfSamples; i++) { - final String sampleName = Thread.currentThread().getName() + "-" + rand.nextInt(); - final HttpClient client = HttpClientBuilder.create().build(); + final String sampleName = Thread.currentThread() + .getName() + "-" + rand.nextInt(); + final HttpClient client = HttpClientBuilder.create() + .build(); final HttpPost request = new HttpPost(samplesUri); request.addHeader("Authorization", "Bearer " + ITestAuthUtils.getTokenForRole("user")); request.addHeader("Content-Type", "application/json"); - final StringEntity content = new StringEntity("{\"sampleName\": \"" + sampleName + "\", \"sequencerSampleId\": \"" + sampleName + "\"}"); + final StringEntity content = new StringEntity( + "{\"sampleName\": \"" + sampleName + "\", \"sequencerSampleId\": \"" + sampleName + "\"}"); request.setEntity(content); final HttpResponse response = client.execute(request); // post that uri - responses.add(response.getStatusLine().getStatusCode()); + responses.add(response.getStatusLine() + .getStatusCode()); } return responses; }; final List>> futures = new CopyOnWriteArrayList<>(); - for (int i = 0 ; i < numberOfThreads; i++) { + for (int i = 0; i < numberOfThreads; i++) { futures.add(executorService.submit(task)); } for (final Future> f : futures) { try { final List responses = f.get(); - assertTrue("All responses should be created.", responses.stream().allMatch(r -> r == HttpStatus.CREATED.value())); + assertTrue("All responses should be created.", responses.stream() + .allMatch(r -> r == HttpStatus.CREATED.value())); } catch (InterruptedException | ExecutionException e) { logger.error("Failed to submit multiple samples simultaneously:", e); fail("Failed to submit multiple samples simultaneously."); @@ -152,11 +224,16 @@ public void testAddSampleToProject() { // load a project String projectUri = "/api/projects/1"; // get the uri for creating samples associated with the project. - String projectJson = asUser().get(projectUri).asString(); + String projectJson = asUser().get(projectUri) + .asString(); String samplesUri = from(projectJson).get("resource.links.find{it.rel == 'project/samples'}.href"); // post that uri - Response r = asUser().body(sample).expect().response().statusCode(HttpStatus.CREATED.value()).when() + Response r = asUser().body(sample) + .expect() + .response() + .statusCode(HttpStatus.CREATED.value()) + .when() .post(samplesUri); // check that the locations are set appropriately. @@ -171,18 +248,25 @@ public void testDeleteSampleFromProject() { String projectUri = ITestSystemProperties.BASE_URL + "/api/projects/4"; // load the project - String projectJson = asUser().get(projectUri).asString(); + String projectJson = asUser().get(projectUri) + .asString(); String projectSamplesUri = from(projectJson).get("resource.links.find{it.rel=='project/samples'}.href"); - String projectSamplesJson = asUser().get(projectSamplesUri).asString(); + String projectSamplesJson = asUser().get(projectSamplesUri) + .asString(); String sampleLabel = from(projectSamplesJson).get("resource.resources[0].sampleName"); // get the uri for a specific sample associated with a project - String sampleUri = from(projectSamplesJson).get("resource.resources[0].links.find{it.rel == 'project/sample'}.href"); + String sampleUri = from(projectSamplesJson).get( + "resource.resources[0].links.find{it.rel == 'project/sample'}.href"); // issue a delete against the service - Response r = asAdmin().expect().statusCode(HttpStatus.OK.value()).when().delete(sampleUri); + Response r = asAdmin().expect() + .statusCode(HttpStatus.OK.value()) + .when() + .delete(sampleUri); // check that the response body contains links to the project and // samples collection - String responseBody = r.getBody().asString(); + String responseBody = r.getBody() + .asString(); String projectUriRel = from(responseBody).get("resource.links.find{it.rel == 'project'}.href"); assertNotNull(projectUriRel); assertEquals(projectUri, projectUriRel); @@ -192,7 +276,9 @@ public void testDeleteSampleFromProject() { assertEquals(projectUri + "/samples", samplesUri); // now confirm that the sample is not there anymore - asUser().expect().body("resource.resources.sampleName", not(hasItem(sampleLabel))).when() + asUser().expect() + .body("resource.resources.sampleName", not(hasItem(sampleLabel))) + .when() .get(projectSamplesUri); } @@ -203,12 +289,18 @@ public void testUpdateProjectSample() { String updatedName = "Totally-different-sample-name"; updatedFields.put("sampleName", updatedName); - asUser().and().body(updatedFields).expect() - .body("resource.links.rel", hasItems("self", "sample/sequenceFiles")).when() + asUser().and() + .body(updatedFields) + .expect() + .body("resource.links.rel", hasItems("self", "sample/sequenceFiles")) + .when() .patch(projectSampleUri); // now confirm that the sample name was updated - asUser().expect().body("resource.sampleName", is(updatedName)).when().get(projectSampleUri); + asUser().expect() + .body("resource.sampleName", is(updatedName)) + .when() + .get(projectSampleUri); } @Test @@ -216,8 +308,10 @@ public void testReadSampleAsAdmin() { String projectUri = ITestSystemProperties.BASE_URL + "/api/projects/5"; String projectSampleUri = projectUri + "/samples/1"; - asAdmin().expect().body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) - .when().get(projectSampleUri); + asAdmin().expect() + .body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) + .when() + .get(projectSampleUri); } @Test @@ -225,8 +319,10 @@ public void testReadSampleCollectionDate() { String projectUri = ITestSystemProperties.BASE_URL + "/api/projects/5"; String projectSampleUri = projectUri + "/samples/1"; - asAdmin().expect().body("resource.collectionDate", is("2019-01-24")) - .when().get(projectSampleUri); + asAdmin().expect() + .body("resource.collectionDate", is("2019-01-24")) + .when() + .get(projectSampleUri); } @Test @@ -234,8 +330,10 @@ public void testReadSampleCollectionDate2() { String projectUri = ITestSystemProperties.BASE_URL + "/api/projects/5"; String projectSampleUri = projectUri + "/samples/3"; - asAdmin().expect().body("resource.collectionDate", is("1999-12-05")) - .when().get(projectSampleUri); + asAdmin().expect() + .body("resource.collectionDate", is("1999-12-05")) + .when() + .get(projectSampleUri); } @Test @@ -243,12 +341,20 @@ public void testReadSampleAsAdminWithDoubledUpSlashes() { String projectUri = ITestSystemProperties.BASE_URL + "/api//projects/5"; String projectSampleUri = projectUri + "/samples/1"; - final Response r = asAdmin().expect().body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) - .when().given().urlEncodingEnabled(false).get(projectSampleUri); - final String responseBody = r.getBody().asString(); + final Response r = asAdmin().expect() + .body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) + .when() + .given() + .urlEncodingEnabled(false) + .get(projectSampleUri); + final String responseBody = r.getBody() + .asString(); final String samplesUri = from(responseBody).get("resource.links.find{it.rel == 'sample/project'}.href"); // now verify that we can actually get this (so doubled slash should not have affected the link) - asAdmin().expect().statusCode(HttpStatus.OK.value()).when().get(samplesUri); + asAdmin().expect() + .statusCode(HttpStatus.OK.value()) + .when() + .get(samplesUri); } @Test @@ -256,7 +362,9 @@ public void testReadSampleAsSequencer() { String projectUri = ITestSystemProperties.BASE_URL + "/api/projects/5"; String projectSampleUri = projectUri + "/samples/1"; - asSequencer().expect().body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) - .when().get(projectSampleUri); + asSequencer().expect() + .body("resource.links.rel", hasItems("self", "sample/project", "sample/sequenceFiles")) + .when() + .get(projectSampleUri); } } diff --git a/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/unit/project/RESTProjectSamplesControllerTest.java b/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/unit/project/RESTProjectSamplesControllerTest.java index e884437ddbc..fd1e4c9b6f2 100644 --- a/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/unit/project/RESTProjectSamplesControllerTest.java +++ b/src/test/java/ca/corefacility/bioinformatics/irida/web/controller/test/unit/project/RESTProjectSamplesControllerTest.java @@ -228,15 +228,17 @@ public void testUpdateSample() { public void testCopySampleToProject() { final Project p = TestDataFactory.constructProject(); final Sample s = TestDataFactory.constructSample(); - final ProjectSampleJoin r = new ProjectSampleJoin(p,s, true); + boolean copyOwner = false; + + final ProjectSampleJoin r = new ProjectSampleJoin(p,s, copyOwner); MockHttpServletResponse response = new MockHttpServletResponse(); when(projectService.read(p.getId())).thenReturn(p); when(sampleService.read(s.getId())).thenReturn(s); when(projectService.addSampleToProject(p, s, false)).thenReturn(r); - ModelMap modelMap = controller - .copySampleToProject(p.getId(), Lists.newArrayList(s.getId()), response, Locale.ENGLISH); - - verify(projectService).addSampleToProject(p, s, false); + ModelMap modelMap = controller.copySampleToProject(p.getId(), Lists.newArrayList(s.getId()), copyOwner, response, + Locale.ENGLISH); + + verify(projectService).addSampleToProject(p, s, copyOwner); assertEquals("response should have CREATED status", HttpStatus.CREATED.value(), response.getStatus()); final String location = response.getHeader(HttpHeaders.LOCATION); assertEquals("location should include sample and project IDs", "http://localhost/api/projects/" + p.getId() @@ -283,7 +285,7 @@ public void testSampleAlreadyCopiedToProject() { when(sampleService.getSampleForProject(p, s.getId())).thenReturn(r); ModelMap modelMap = controller - .copySampleToProject(p.getId(), Lists.newArrayList(s.getId()), response, Locale.ENGLISH); + .copySampleToProject(p.getId(), Lists.newArrayList(s.getId()), false, response, Locale.ENGLISH); verify(projectService).addSampleToProject(p, s, false); verify(sampleService).getSampleForProject(p,s.getId());