Add sample to project refactor

CHANGELOG.md

@@ -38,6 +38,7 @@ Changes
 * [Workflow]: Updated the SNVPhyl pipeline to version `1.2.2`.
 * [Developer]: Updated the Galaxy Docker instance to Galaxy `20.05`.
 * [UI]: Created admin panel for simplifying administrator tasks (Users, Clients, Remote IRIDA Connections, Sequencing Runs, NCBI Exports, and Announcements).
+* [REST]: Updated REST API to handle errors when copying existing samples more gracefully.
 
 20.01 to 20.05
 --------------

src/main/java/ca/corefacility/bioinformatics/irida/exceptions/ExistingSampleNameException.java

@@ -0,0 +1,27 @@
+package ca.corefacility.bioinformatics.irida.exceptions;
+
+import ca.corefacility.bioinformatics.irida.model.sample.Sample;
+
+/**
+ * Exception thrown when trying to add a {@link ca.corefacility.bioinformatics.irida.model.sample.Sample} to a {@link ca.corefacility.bioinformatics.irida.model.project.Project} with a duplicate sample name
+ */
+public class ExistingSampleNameException extends EntityExistsException {
+	Sample sample;
+
+	public ExistingSampleNameException(String message) {
+		super(message);
+	}
+
+	public ExistingSampleNameException(String message, Throwable cause) {
+		super(message, cause);
+	}
+
+	public ExistingSampleNameException(String message, Sample sample) {
+		super(message);
+		this.sample = sample;
+	}
+
+	public Sample getSample() {
+		return sample;
+	}
+}

src/main/java/ca/corefacility/bioinformatics/irida/ria/web/services/UIProjectSampleService.java

@@ -96,7 +96,7 @@ public ResponseEntity<AjaxResponse> createSample(CreateSampleRequest request, lo
 			if (!Strings.isNullOrEmpty(request.getOrganism())) {
 				sample.setOrganism(request.getOrganism());
 			}
-			Join<Project, Sample> join = projectService.addSampleToProject(project, sample, true);
+			Join<Project, Sample> join = projectService.createNewSampleInProject(project, sample);
 			return ResponseEntity.ok(new AjaxCreateItemSuccessResponse(join.getObject()
 					.getId()));
 		} catch (EntityNotFoundException e) {

src/main/java/ca/corefacility/bioinformatics/irida/service/ProjectService.java

@@ -5,6 +5,8 @@
 import java.util.Map;
 import java.util.Set;
 
+import javax.validation.Valid;
+
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.domain.Sort.Direction;
@@ -126,15 +128,24 @@ public Join<Project, UserGroup> updateUserGroupProjectRole(Project project, User
 			ProjectRole projectRole) throws ProjectWithoutOwnerException;
 
 	/**
-	 * Add the specified {@link Sample} to the {@link Project}.
+	 * Create a new {@link Sample} to the given {@link Project}.
 	 *
 	 * @param project the {@link Project} to add the {@link Sample} to.
 	 * @param sample  the {@link Sample} to add to the {@link Project}. If the {@link Sample} has not previously been persisted, the
 	 *                service will persist the {@link Sample}.
-	 * @param owner   Whether the project will have modification access for this sample
 	 * @return a reference to the relationship resource created between the two entities.
 	 */
-	public Join<Project, Sample> addSampleToProject(Project project, Sample sample, boolean owner);
+	public Join<Project, Sample> createNewSampleInProject(Project project, Sample sample);
+
+	/**
+	 * Share an existing sample to a given project
+	 *
+	 * @param project the project to share the sample to
+	 * @param sample  the sample to share
+	 * @param owner   whether to grant modification access to the sample in the new project
+	 * @return the newly created join
+	 */
+	public ProjectSampleJoin addExistingSampleToProject(Project project, @Valid Sample sample, boolean owner);
 
 	/**
 	 * Move a {@link Sample} from one {@link Project} to another

src/main/java/ca/corefacility/bioinformatics/irida/service/impl/ProjectServiceImpl.java

@@ -6,6 +6,7 @@
 import javax.persistence.criteria.*;
 import javax.validation.ConstraintViolation;
 import javax.validation.ConstraintViolationException;
+import javax.validation.Valid;
 import javax.validation.Validator;
 
 import org.slf4j.Logger;
@@ -340,29 +341,59 @@ private boolean allowRoleChange(final Project project, final ProjectRole project
 	@Transactional
 	@LaunchesProjectEvent(SampleAddedProjectEvent.class)
 	@PreAuthorize("hasAnyRole('ROLE_ADMIN', 'ROLE_SEQUENCER') or (hasPermission(#project, 'isProjectOwner'))")
-	public ProjectSampleJoin addSampleToProject(Project project, Sample sample, boolean owner) {
+	public ProjectSampleJoin createNewSampleInProject(Project project, Sample sample) {
 		logger.trace("Adding sample to project.");
 
+		//if the sample exists, ensure it isn't already on the project
+		if (sample.getId() != null) {
+			throw new IllegalArgumentException("Samples provided to this method must not have been persisted to the database");
+		}
+
 		// Check to ensure a sample with this sequencer id doesn't exist in this
 		// project already
 		if (sampleRepository.getSampleBySampleName(project, sample.getSampleName()) != null) {
-			throw new EntityExistsException(
-					"Sample with sequencer id '" + sample.getSampleName() + "' already exists in project " + project.getId());
+			throw new ExistingSampleNameException(
+					"Sample with sequencer id '" + sample.getSampleName() + "' already exists in project "
+							+ project.getId(), sample);
 		}
 
-		// the sample hasn't been persisted before, persist it before calling
-		// the relationshipRepository.
-		if (sample.getId() == null) {
-			logger.trace("Going to validate and persist sample prior to creating relationship.");
-			// validate the sample, then persist it:
-			Set<ConstraintViolation<Sample>> constraintViolations = validator.validate(sample);
-			if (constraintViolations.isEmpty()) {
-				sample = sampleRepository.save(sample);
-			} else {
-				throw new ConstraintViolationException(constraintViolations);
-			}
+		logger.trace("Going to validate and persist sample prior to creating relationship.");
+		// validate the sample, then persist it:
+		Set<ConstraintViolation<Sample>> constraintViolations = validator.validate(sample);
+		if (constraintViolations.isEmpty()) {
+			sample = sampleRepository.save(sample);
+		} else {
+			throw new ConstraintViolationException(constraintViolations);
+		}
+
+		ProjectSampleJoin join = new ProjectSampleJoin(project, sample, true);
+
+		try {
+			return psjRepository.save(join);
+		} catch (DataIntegrityViolationException e) {
+			throw new EntityExistsException(
+					"Sample [" + sample.getId() + "] has already been added to project [" + project.getId() + "]");
 		}
+	}
 
+	/**
+	 * {@inheritDoc}
+	 */
+	@Override
+	@Transactional
+	@LaunchesProjectEvent(SampleAddedProjectEvent.class)
+	@PreAuthorize("hasPermission(#project, 'isProjectOwner') and hasPermission(#sample, 'canReadSample')"
+			+ " and ((not #giveOwner) or hasPermission(#sample, 'canUpdateSample'))")
+	public ProjectSampleJoin addExistingSampleToProject(Project project, @Valid Sample sample, boolean owner) {
+		logger.trace("Adding sample to project.");
+
+		// Check to ensure a sample with this sequencer id doesn't exist in this
+		// project already
+		if (sampleRepository.getSampleBySampleName(project, sample.getSampleName()) != null) {
+			throw new ExistingSampleNameException(
+					"Sample with sequencer id '" + sample.getSampleName() + "' already exists in project "
+							+ project.getId(), sample);
+		}
 		ProjectSampleJoin join = new ProjectSampleJoin(project, sample, owner);
 
 		try {
@@ -385,7 +416,7 @@ public ProjectSampleJoin moveSampleBetweenProjects(Project source, Project desti
 		ProjectSampleJoin projectSampleJoin = psjRepository.readSampleForProject(source, sample);
 
 		//set the ownership on the sample given the existing permissions
-		ProjectSampleJoin join = addSampleToProject(destination, sample, projectSampleJoin.isOwner());
+		ProjectSampleJoin join = addExistingSampleToProject(destination, sample, projectSampleJoin.isOwner());
 
 		//remove the old join
 		removeSampleFromProject(source, sample);
@@ -408,7 +439,7 @@ public List<ProjectSampleJoin> shareSamples(Project source, Project destination,
 		List<ProjectSampleJoin> newJoins = new ArrayList<>();
 
 		for (Sample sample : samples) {
-			ProjectSampleJoin newJoin = addSampleToProject(destination, sample, giveOwner);
+			ProjectSampleJoin newJoin = addExistingSampleToProject(destination, sample, giveOwner);
 
 			logger.trace("Shared sample " + sample.getId() + " to project " + destination.getId());
 
@@ -733,7 +764,7 @@ public Project createProjectWithSamples(Project project, List<Long> sampleIds, b
 
 		sampleIds.forEach(sid -> {
 			Sample s = sampleRepository.findById(sid).orElse(null);
-			addSampleToProject(project, s, owner);
+			addExistingSampleToProject(project, s, owner);
 		});
 
 		return created;

src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java

@@ -302,7 +302,7 @@ public List<ProjectSynchronizationException> syncSample(Sample sample, Project p
 			// if the sample doesn't already exist create it
 			sample.getRemoteStatus().setSyncStatus(SyncStatus.UPDATING);
 			localSample = sampleService.create(sample);
-			projectService.addSampleToProject(project, sample, true);
+			projectService.createNewSampleInProject(project, sample);
 		}
 
 		//get a collection of the files already sync'd.  we don't want to grab them a 2nd time.