From 695bc8586517c8a8487bafce0743493e2bcf9a2e Mon Sep 17 00:00:00 2001 From: Eric Enns Date: Tue, 26 Jul 2022 13:39:41 -0500 Subject: [PATCH 1/3] Remove synced sequence files and assemblies that have been remotely deleted. --- .../ProjectSyncScheduledTaskConfig.java | 2 +- .../remote/ProjectSynchronizationService.java | 223 ++++++++++-------- .../ProjectSynchronizationServiceTest.java | 146 ++++++++---- 3 files changed, 226 insertions(+), 145 deletions(-) diff --git a/src/main/java/ca/corefacility/bioinformatics/irida/config/services/scheduled/ProjectSyncScheduledTaskConfig.java b/src/main/java/ca/corefacility/bioinformatics/irida/config/services/scheduled/ProjectSyncScheduledTaskConfig.java index 149673eaa2f..2aded9ab071 100644 --- a/src/main/java/ca/corefacility/bioinformatics/irida/config/services/scheduled/ProjectSyncScheduledTaskConfig.java +++ b/src/main/java/ca/corefacility/bioinformatics/irida/config/services/scheduled/ProjectSyncScheduledTaskConfig.java @@ -9,7 +9,7 @@ /** * Scheduled task configuration for synchronizing projects from other IRIDA installations */ -@Profile({ "prod", "sync" }) +@Profile({ "prod", "sync", "dev" }) @Configuration public class ProjectSyncScheduledTaskConfig { diff --git a/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java b/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java index 1419ef77e5c..352a4f18b2b 100644 --- a/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java +++ b/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java @@ -37,9 +37,8 @@ import com.google.common.collect.Lists; /** - * Service class to run a project synchornization task. Ths class will be - * responsible for communicating with Remote IRIDA installations and pulling - * metadata and sequencing data into the local installation. + * Service class to run a project synchornization task. Ths class will be responsible for communicating with Remote + * IRIDA installations and pulling metadata and sequencing data into the local installation. */ @Service public class ProjectSynchronizationService { @@ -66,7 +65,8 @@ public ProjectSynchronizationService(ProjectService projectService, SampleServic GenomeAssemblyService assemblyService, ProjectRemoteService projectRemoteService, SampleRemoteService sampleRemoteService, SingleEndSequenceFileRemoteService singleEndRemoteService, SequenceFilePairRemoteService pairRemoteService, GenomeAssemblyRemoteService assemblyRemoteService, - Fast5ObjectRemoteService fast5ObjectRemoteService, RemoteAPITokenService tokenService, EmailController emailController) { + Fast5ObjectRemoteService fast5ObjectRemoteService, RemoteAPITokenService tokenService, + EmailController emailController) { this.projectService = projectService; this.sampleService = sampleService; @@ -84,8 +84,8 @@ public ProjectSynchronizationService(ProjectService projectService, SampleServic } /** - * Method checking for remote projects that have passed their frequency - * time. It will mark them as {@link SyncStatus#MARKED} + * Method checking for remote projects that have passed their frequency time. It will mark them as + * {@link SyncStatus#MARKED} */ public void findProjectsToMark() { List remoteProjects = projectService.getRemoteProjects(); @@ -126,8 +126,7 @@ public void findProjectsToMark() { } /** - * Find projects which should be synchronized and launch a synchornization - * task. + * Find projects which should be synchronized and launch a synchornization task. */ public synchronized void findMarkedProjectsToSync() { // mark any projects which should be synched first @@ -181,9 +180,7 @@ public synchronized void findMarkedProjectsToSync() { /** * Synchronize a given {@link Project} to the local installation. * - * @param project - * the {@link Project} to synchronize. This should have been read - * from a remote api. + * @param project the {@link Project} to synchronize. This should have been read from a remote api. */ private void syncProject(Project project) { SyncStatus syncType = project.getRemoteStatus().getSyncStatus(); @@ -208,7 +205,8 @@ private void syncProject(Project project) { } //check if the project hashes are different. if projectHash is null the other service doesn't support hashing so do a full check - if (syncType.equals(SyncStatus.FORCE) || projectHash == null || !projectHash.equals(project.getRemoteProjectHash())) { + if (syncType.equals(SyncStatus.FORCE) || projectHash == null + || !projectHash.equals(project.getRemoteProjectHash())) { // ensure we use the same IDs readProject = updateIds(project, readProject); @@ -247,8 +245,7 @@ private void syncProject(Project project) { //get a list of all remote URLs in the project Set remoteUrls = readSamplesForProject.stream() - .map(sample -> sample.getRemoteStatus() - .getURL()) + .map(sample -> sample.getRemoteStatus().getURL()) .collect(Collectors.toSet()); // Check for local samples which no longer exist by URL @@ -273,8 +270,7 @@ private void syncProject(Project project) { syncExceptions.addAll(syncExceptionsSample); } - } - else{ + } else { logger.debug("No changes to project. Skipping"); } @@ -298,13 +294,14 @@ private void syncProject(Project project) { /** * Synchronize a given {@link Sample} to the local installation. * - * @param sample the {@link Sample} to synchronize. This should have been read - * from a remote api. + * @param sample the {@link Sample} to synchronize. This should have been read from a remote api. * @param project The {@link Project} the {@link Sample} belongs in. - * @param existingSamples A map of samples that have already been synchronized. These will be checked to see if they've been updated + * @param existingSamples A map of samples that have already been synchronized. These will be checked to see if + * they've been updated * @return A list of {@link ProjectSynchronizationException}s, empty if no errors. */ - public List syncSample(Sample sample, Project project, Map existingSamples) { + public List syncSample(Sample sample, Project project, + Map existingSamples) { Sample localSample; if (existingSamples.containsKey(sample.getRemoteStatus().getURL())) { @@ -335,29 +332,28 @@ public List syncSample(Sample sample, Project p //get a collection of the files already sync'd. we don't want to grab them a 2nd time. Collection localObjects = objectService.getSequencingObjectsForSample(localSample); - Set objectsByUrl = new HashSet<>(); + Map objectsByUrl = new HashMap<>(); localObjects.forEach(sequencingObjectJoin -> { - SequencingObject pair = sequencingObjectJoin.getObject(); - + SequencingObject object = sequencingObjectJoin.getObject(); + // check if the file was actually sync'd. Someone may have // concatenated it - if (pair.getRemoteStatus() != null) { - String url = pair.getRemoteStatus().getURL(); + if (object.getRemoteStatus() != null) { + String url = object.getRemoteStatus().getURL(); - objectsByUrl.add(url); + objectsByUrl.put(url, object); } }); //same with assemblies. get the ones we've already grabbed and store their URL so we don't double-sync Collection assembliesForSample = assemblyService.getAssembliesForSample(localSample); - Set localAssemblyUrls = new HashSet<>(); + Map assembliesByUrl = new HashMap<>(); assembliesForSample.forEach(assemblyJoin -> { GenomeAssembly genomeAssembly = assemblyJoin.getObject(); if (genomeAssembly.getRemoteStatus() != null) { - String url = genomeAssembly.getRemoteStatus() - .getURL(); - localAssemblyUrls.add(url); + String url = genomeAssembly.getRemoteStatus().getURL(); + assembliesByUrl.put(url, genomeAssembly); } }); @@ -366,12 +362,51 @@ public List syncSample(Sample sample, Project p //list the pairs from the remote api List sequenceFilePairsForSample = pairRemoteService.getSequenceFilePairsForSample(sample); - + + //list the single files from the remote api + List unpairedFilesForSample = singleEndRemoteService.getUnpairedFilesForSample(sample); + + //list the fast5 files from the remote api + List fast5FilesForSample; + try { + fast5FilesForSample = fast5ObjectRemoteService.getFast5FilesForSample(sample); + } catch (LinkNotFoundException e) { + logger.warn("The sample on the referenced IRIDA doesn't support fast5 data: " + sample.getSelfHref()); + fast5FilesForSample = Lists.newArrayList(); + } + + //get a list of all remote sequencingObject URLs in the sample + Set remoteObjectUrls = new HashSet(); + remoteObjectUrls.addAll(sequenceFilePairsForSample.stream() + .map(pair -> pair.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + remoteObjectUrls.addAll(unpairedFilesForSample.stream() + .map(file -> file.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + remoteObjectUrls.addAll(fast5FilesForSample.stream() + .map(fast5Object -> fast5Object.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + + //get a list of all remote sequencingObject URLs in the sample + Set localObjectUrls = new HashSet<>(objectsByUrl.keySet()); + //remove any URL from the local list that we've seen remotely + remoteObjectUrls.forEach(objectUrl -> { + localObjectUrls.remove(objectUrl); + }); + + //if any URLs still exists in localObjectUrls, it must have been deleted remotely + for (String localObjectUrl : localObjectUrls) { + logger.trace( + "SequencingObject " + localObjectUrl + " has been removed remotely. Removing from local sample."); + + sampleService.removeSequencingObjectFromSample(sample, objectsByUrl.get(localObjectUrl)); + objectsByUrl.remove(localObjectUrl); + } //for each pair for (SequenceFilePair pair : sequenceFilePairsForSample) { //check if we've already got it - if (!objectsByUrl.contains(pair.getRemoteStatus().getURL())) { + if (!objectsByUrl.containsKey(pair.getRemoteStatus().getURL())) { pair.setId(null); //if not, download it locally try { @@ -382,13 +417,10 @@ public List syncSample(Sample sample, Project p } } - //list the single files from the remote api - List unpairedFilesForSample = singleEndRemoteService.getUnpairedFilesForSample(sample); - //for each single file for (SingleEndSequenceFile file : unpairedFilesForSample) { //check if we already have it - if (!objectsByUrl.contains(file.getRemoteStatus().getURL())) { + if (!objectsByUrl.containsKey(file.getRemoteStatus().getURL())) { file.setId(null); //if not, get it locally and save it try { @@ -399,20 +431,10 @@ public List syncSample(Sample sample, Project p } } - //list the fast5 files from the remote api - List fast5FilesForSample; - try { - fast5FilesForSample = fast5ObjectRemoteService.getFast5FilesForSample(sample); - } catch (LinkNotFoundException e) { - logger.warn("The sample on the referenced IRIDA doesn't support fast5 data: " + sample.getSelfHref()); - fast5FilesForSample = Lists.newArrayList(); - } - //for each fast5 file for (Fast5Object fast5Object : fast5FilesForSample) { //check if we already have it - if (!objectsByUrl.contains(fast5Object.getRemoteStatus() - .getURL())) { + if (!objectsByUrl.containsKey(fast5Object.getRemoteStatus().getURL())) { fast5Object.setId(null); //if not, get it locally and save it try { @@ -423,7 +445,6 @@ public List syncSample(Sample sample, Project p } } - //list the remote assemblies for the sample. List genomeAssembliesForSample; try { @@ -434,11 +455,38 @@ public List syncSample(Sample sample, Project p genomeAssembliesForSample = Lists.newArrayList(); } + //get a list of all remote sequencingObject URLs in the sample + Set remoteAssemblyUrls = new HashSet(); + remoteObjectUrls.addAll(sequenceFilePairsForSample.stream() + .map(pair -> pair.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + remoteObjectUrls.addAll(unpairedFilesForSample.stream() + .map(file -> file.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + remoteObjectUrls.addAll(fast5FilesForSample.stream() + .map(fast5Object -> fast5Object.getRemoteStatus().getURL()) + .collect(Collectors.toSet())); + + //get a list of all remote assembly URLs in the sample + Set localAssemblyUrls = new HashSet<>(assembliesByUrl.keySet()); + //remove any URL from the local list that we've seen remotely + remoteAssemblyUrls.forEach(assemblyUrl -> { + localAssemblyUrls.remove(assemblyUrl); + }); + + //if any URLs still exists in localAssemblyUrls, it must have been deleted remotely + for (String localAssemblyUrl : localAssemblyUrls) { + logger.trace( + "GenomeAssembly " + localAssemblyUrl + " has been removed remotely. Removing from local sample."); + + assemblyService.removeGenomeAssemblyFromSample(sample, assembliesByUrl.get(localAssemblyUrl).getId()); + assembliesByUrl.remove(localAssemblyUrl); + } + //for each assembly for (UploadedAssembly file : genomeAssembliesForSample) { //if we haven't already sync'd this assembly, get it - if (!localAssemblyUrls.contains(file.getRemoteStatus() - .getURL())) { + if (!localAssemblyUrls.contains(file.getRemoteStatus().getURL())) { file.setId(null); try { syncAssembly(file, localSample); @@ -448,9 +496,6 @@ public List syncSample(Sample sample, Project p } } - - - //if we have no errors, report that the sample is sync'd if (syncErrors.isEmpty()) { localSample.getRemoteStatus().setSyncStatus(SyncStatus.SYNCHRONIZED); @@ -476,21 +521,17 @@ public List syncSample(Sample sample, Project p public void syncSampleMetadata(Sample remoteSample, Sample localSample) { Map sampleMetadata = sampleRemoteService.getSampleMetadata(remoteSample); - sampleMetadata.values() - .forEach(e -> e.setId(null)); + sampleMetadata.values().forEach(e -> e.setId(null)); Set metadata = metadataTemplateService.convertMetadataStringsToSet(sampleMetadata); sampleService.updateSampleMetadata(localSample, metadata); } /** - * Synchronize a given {@link SingleEndSequenceFile} to the local - * installation + * Synchronize a given {@link SingleEndSequenceFile} to the local installation * - * @param file - * the {@link SingleEndSequenceFile} to sync - * @param sample - * the {@link Sample} to add the file to + * @param file the {@link SingleEndSequenceFile} to sync + * @param sample the {@link Sample} to add the file to */ public void syncSingleEndSequenceFile(SingleEndSequenceFile file, Sample sample) { RemoteStatus fileStatus = file.getRemoteStatus(); @@ -506,13 +547,10 @@ public void syncSingleEndSequenceFile(SingleEndSequenceFile file, Sample sample) } /** - * Synchronize a given {@link Fast5Object} to the local - * installation + * Synchronize a given {@link Fast5Object} to the local installation * - * @param fast5Object - * the {@link Fast5Object} to sync - * @param sample - * the {@link Sample} to add the file to + * @param fast5Object the {@link Fast5Object} to sync + * @param sample the {@link Sample} to add the file to */ public void syncFast5File(Fast5Object fast5Object, Sample sample) { RemoteStatus fileStatus = fast5Object.getRemoteStatus(); @@ -522,14 +560,13 @@ public void syncFast5File(Fast5Object fast5Object, Sample sample) { syncSequencingObject(fast5Object, sample, fileStatus); } catch (Exception e) { logger.error("Error transferring file: " + fast5Object.getRemoteStatus().getURL(), e); - throw new ProjectSynchronizationException("Could not synchronize file " + fast5Object.getRemoteStatus().getURL(), - e); + throw new ProjectSynchronizationException( + "Could not synchronize file " + fast5Object.getRemoteStatus().getURL(), e); } } /** - * Synchronize a given {@link UploadedAssembly} to the local - * installation + * Synchronize a given {@link UploadedAssembly} to the local installation * * @param assembly the {@link UploadedAssembly} to sync * @param sample the {@link Sample} to add the assembly to @@ -540,26 +577,21 @@ public void syncAssembly(UploadedAssembly assembly, Sample sample) { try { assembly = assemblyRemoteService.mirrorAssembly(assembly); - assembly.getRemoteStatus() - .setSyncStatus(SyncStatus.SYNCHRONIZED); + assembly.getRemoteStatus().setSyncStatus(SyncStatus.SYNCHRONIZED); assemblyService.createAssemblyInSample(sample, assembly); } catch (Exception e) { - logger.error("Error transferring assembly: " + assembly.getRemoteStatus() - .getURL(), e); - throw new ProjectSynchronizationException("Could not synchronize assembly " + assembly.getRemoteStatus() - .getURL(), e); + logger.error("Error transferring assembly: " + assembly.getRemoteStatus().getURL(), e); + throw new ProjectSynchronizationException( + "Could not synchronize assembly " + assembly.getRemoteStatus().getURL(), e); } } /** * Synchronize a given {@link SequenceFilePair} to the local installation. * - * @param pair - * the {@link SequenceFilePair} to sync. This should have been - * read from a remote api. - * @param sample - * The {@link Sample} to add the pair to. + * @param pair the {@link SequenceFilePair} to sync. This should have been read from a remote api. + * @param sample The {@link Sample} to add the pair to. */ public void syncSequenceFilePair(SequenceFilePair pair, Sample sample) { RemoteStatus fileStatus = pair.getRemoteStatus(); @@ -577,10 +609,8 @@ public void syncSequenceFilePair(SequenceFilePair pair, Sample sample) { /** * Check if an object has been updated since it was last read * - * @param originalStatus - * the original object's {@link RemoteStatus} - * @param read - * the newly read {@link RemoteSynchronizable} object + * @param originalStatus the original object's {@link RemoteStatus} + * @param read the newly read {@link RemoteSynchronizable} object * @return true if the object has changed, false if not */ private boolean checkForChanges(RemoteStatus originalStatus, RemoteSynchronizable read) { @@ -588,13 +618,10 @@ private boolean checkForChanges(RemoteStatus originalStatus, RemoteSynchronizabl } /** - * Update the IDs of a newly read object and it's associated RemoteStatus to - * the IDs of a local copy + * Update the IDs of a newly read object and it's associated RemoteStatus to the IDs of a local copy * - * @param original - * the original object - * @param updated - * the newly read updated object + * @param original the original object + * @param updated the newly read updated object * @return the enhanced newly read object */ private Type updateIds(Type original, Type updated) { @@ -607,8 +634,7 @@ private Type updateIds(T /** * Set the given user's authentication in the SecurityContextHolder * - * @param user - * The {@link User} to set in the context holder + * @param user The {@link User} to set in the context holder */ private void setAuthentication(User user) { ProjectSynchronizationAuthenticationToken userAuthentication = new ProjectSynchronizationAuthenticationToken( @@ -620,13 +646,14 @@ private void setAuthentication(User user) { } /** - * Synchronize a given {@link SequencingObject} to the local installation. + * Synchronize a given {@link SequencingObject} to the local installation. * - * @param sequencingObject The sequencing object to sync - * @param sample The sample to sync + * @param sequencingObject The sequencing object to sync + * @param sample The sample to sync * @param sequencingObjectStatus The remote status of the sequencing object */ - private void syncSequencingObject(SequencingObject sequencingObject, Sample sample, RemoteStatus sequencingObjectStatus) { + private void syncSequencingObject(SequencingObject sequencingObject, Sample sample, + RemoteStatus sequencingObjectStatus) { sequencingObject.setProcessingState(SequencingObject.ProcessingState.UNPROCESSED); sequencingObject.setFileProcessor(null); diff --git a/src/test/java/ca/corefacility/bioinformatics/irida/service/ProjectSynchronizationServiceTest.java b/src/test/java/ca/corefacility/bioinformatics/irida/service/ProjectSynchronizationServiceTest.java index f0ab863ddac..2b9db9c6a50 100644 --- a/src/test/java/ca/corefacility/bioinformatics/irida/service/ProjectSynchronizationServiceTest.java +++ b/src/test/java/ca/corefacility/bioinformatics/irida/service/ProjectSynchronizationServiceTest.java @@ -2,6 +2,8 @@ import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collection; +import java.util.Collections; import java.util.Date; import java.util.Set; @@ -16,15 +18,15 @@ import ca.corefacility.bioinformatics.irida.exceptions.ProjectSynchronizationException; import ca.corefacility.bioinformatics.irida.model.RemoteAPI; import ca.corefacility.bioinformatics.irida.model.assembly.UploadedAssembly; +import ca.corefacility.bioinformatics.irida.model.joins.impl.SampleGenomeAssemblyJoin; import ca.corefacility.bioinformatics.irida.model.project.Project; import ca.corefacility.bioinformatics.irida.model.project.ProjectSyncFrequency; import ca.corefacility.bioinformatics.irida.model.remote.RemoteStatus; import ca.corefacility.bioinformatics.irida.model.remote.RemoteStatus.SyncStatus; import ca.corefacility.bioinformatics.irida.model.sample.Sample; +import ca.corefacility.bioinformatics.irida.model.sample.SampleSequencingObjectJoin; import ca.corefacility.bioinformatics.irida.model.sample.metadata.MetadataEntry; -import ca.corefacility.bioinformatics.irida.model.sequenceFile.Fast5Object; -import ca.corefacility.bioinformatics.irida.model.sequenceFile.SequenceFile; -import ca.corefacility.bioinformatics.irida.model.sequenceFile.SequenceFilePair; +import ca.corefacility.bioinformatics.irida.model.sequenceFile.*; import ca.corefacility.bioinformatics.irida.model.user.User; import ca.corefacility.bioinformatics.irida.service.remote.*; import ca.corefacility.bioinformatics.irida.service.sample.MetadataTemplateService; @@ -80,8 +82,9 @@ public void setup() { MockitoAnnotations.openMocks(this); syncService = new ProjectSynchronizationService(projectService, sampleService, objectService, - metadataTemplateService, assemblyService, projectRemoteService, sampleRemoteService, singleEndRemoteService, - pairRemoteService, assemblyRemoteService, fast5ObjectRemoteService, tokenService, emailController); + metadataTemplateService, assemblyService, projectRemoteService, sampleRemoteService, + singleEndRemoteService, pairRemoteService, assemblyRemoteService, fast5ObjectRemoteService, + tokenService, emailController); api = new RemoteAPI(); expired = new Project(); @@ -176,23 +179,20 @@ public void testSyncProjectsSameHash() { @Test public void testSyncProjectsUnauthorized() { - expired.getRemoteStatus() - .setSyncStatus(SyncStatus.MARKED); + expired.getRemoteStatus().setSyncStatus(SyncStatus.MARKED); when(projectService.read(expired.getId())).thenReturn(expired); Project remoteProject = new Project(); remoteProject.setRemoteStatus(expired.getRemoteStatus()); User readBy = new User(); - expired.getRemoteStatus() - .setReadBy(readBy); - when(projectService.getProjectsWithRemoteSyncStatus(RemoteStatus.SyncStatus.MARKED)).thenReturn( - Lists.newArrayList(expired)); - when(projectRemoteService.read(expired.getRemoteStatus() - .getURL())).thenThrow(new IridaOAuthException("unauthorized", api)); + expired.getRemoteStatus().setReadBy(readBy); + when(projectService.getProjectsWithRemoteSyncStatus(RemoteStatus.SyncStatus.MARKED)) + .thenReturn(Lists.newArrayList(expired)); + when(projectRemoteService.read(expired.getRemoteStatus().getURL())) + .thenThrow(new IridaOAuthException("unauthorized", api)); syncService.findMarkedProjectsToSync(); - assertEquals(SyncStatus.UNAUTHORIZED, remoteProject.getRemoteStatus() - .getSyncStatus()); + assertEquals(SyncStatus.UNAUTHORIZED, remoteProject.getRemoteStatus().getSyncStatus()); verify(emailController).sendProjectSyncUnauthorizedEmail(expired); } @@ -200,36 +200,35 @@ public void testSyncProjectsUnauthorized() { @Test public void testSyncNewSample() { Sample sample = new Sample(); - RemoteStatus sampleStatus = new RemoteStatus("http://sample",api); + RemoteStatus sampleStatus = new RemoteStatus("http://sample", api); sample.setRemoteStatus(sampleStatus); - + when(sampleService.create(sample)).thenReturn(sample); when(sampleService.updateSampleMetadata(eq(sample), anySet())).thenReturn(sample); - + syncService.syncSample(sample, expired, Maps.newHashMap()); - + verify(projectService).addSampleToProject(expired, sample, true); - - assertEquals(SyncStatus.SYNCHRONIZED,sample.getRemoteStatus().getSyncStatus()); + + assertEquals(SyncStatus.SYNCHRONIZED, sample.getRemoteStatus().getSyncStatus()); } - + @Test - public void testExistingSample(){ + public void testExistingSample() { Sample sample = new Sample(); - RemoteStatus sampleStatus = new RemoteStatus("http://sample",api); + RemoteStatus sampleStatus = new RemoteStatus("http://sample", api); sample.setRemoteStatus(sampleStatus); - + Sample existingSample = new Sample(); existingSample.setRemoteStatus(sampleStatus); - + when(sampleService.update(any(Sample.class))).thenReturn(sample); - when(sampleService.updateSampleMetadata(eq(sample),anySet() )).thenReturn(sample); + when(sampleService.updateSampleMetadata(eq(sample), anySet())).thenReturn(sample); + syncService.syncSample(sample, expired, ImmutableMap.of("http://sample", existingSample)); - syncService.syncSample(sample, expired, ImmutableMap.of("http://sample",existingSample)); - - verify(projectService,times(0)).addSampleToProject(expired, sample, true); - verify(sampleService,times(2)).update(any(Sample.class)); + verify(projectService, times(0)).addSampleToProject(expired, sample, true); + verify(sampleService, times(2)).update(any(Sample.class)); } @Test @@ -239,8 +238,10 @@ public void testSyncSampleNoAssemblies() { sample.setRemoteStatus(sampleStatus); when(sampleService.create(sample)).thenReturn(sample); - when(sampleService.updateSampleMetadata(eq(sample), ArgumentMatchers.>any())).thenReturn(sample); - when(assemblyRemoteService.getGenomeAssembliesForSample(sample)).thenThrow(new LinkNotFoundException("no link")); + when(sampleService.updateSampleMetadata(eq(sample), ArgumentMatchers.>any())) + .thenReturn(sample); + when(assemblyRemoteService.getGenomeAssembliesForSample(sample)) + .thenThrow(new LinkNotFoundException("no link")); syncService.syncSample(sample, expired, Maps.newHashMap()); @@ -249,20 +250,71 @@ public void testSyncSampleNoAssemblies() { assertEquals(SyncStatus.SYNCHRONIZED, sample.getRemoteStatus().getSyncStatus()); } - + + @Test + public void testSyncExistingSampleWithDeletedFiles() { + Sample sample = new Sample(); + RemoteStatus sampleStatus = new RemoteStatus("http://sample", api); + sample.setRemoteStatus(sampleStatus); + + Sample existingSample = new Sample(); + existingSample.setRemoteStatus(sampleStatus); + + SequenceFilePair localPair = new SequenceFilePair(); + RemoteStatus localPairStatus = new RemoteStatus("http://pair", api); + localPair.setRemoteStatus(localPairStatus); + SampleSequencingObjectJoin sso = new SampleSequencingObjectJoin(sample, localPair); + when(objectService.getSequencingObjectsForSample(existingSample)).thenReturn(Collections.singletonList(sso)); + + when(sampleService.update(any(Sample.class))).thenReturn(sample); + when(sampleService.updateSampleMetadata(eq(sample), anySet())).thenReturn(sample); + + syncService.syncSample(sample, expired, ImmutableMap.of("http://sample", existingSample)); + + verify(projectService, times(0)).addSampleToProject(expired, sample, true); + verify(sampleService, times(2)).update(any(Sample.class)); + verify(sampleService, times(1)).removeSequencingObjectFromSample(sample, localPair); + } + + @Test + public void testSyncExistingSampleWithDeletedAssemblies() { + Sample sample = new Sample(); + RemoteStatus sampleStatus = new RemoteStatus("http://sample", api); + sample.setRemoteStatus(sampleStatus); + + Sample existingSample = new Sample(); + existingSample.setRemoteStatus(sampleStatus); + + UploadedAssembly localAssembly = new UploadedAssembly(null); + localAssembly.setId(1L); + RemoteStatus localAssemblyStatus = new RemoteStatus("http://assembly", api); + localAssembly.setRemoteStatus(localAssemblyStatus); + SampleGenomeAssemblyJoin sga = new SampleGenomeAssemblyJoin(sample, localAssembly); + when(assemblyService.getAssembliesForSample(existingSample)).thenReturn(Collections.singletonList(sga)); + + when(sampleService.update(any(Sample.class))).thenReturn(sample); + when(sampleService.updateSampleMetadata(eq(sample), anySet())).thenReturn(sample); + + syncService.syncSample(sample, expired, ImmutableMap.of("http://sample", existingSample)); + + verify(projectService, times(0)).addSampleToProject(expired, sample, true); + verify(sampleService, times(2)).update(any(Sample.class)); + verify(assemblyService, times(1)).removeGenomeAssemblyFromSample(sample, localAssembly.getId()); + } + @Test public void testSyncFiles() { Sample sample = new Sample(); - + SequenceFilePair pair = new SequenceFilePair(); - RemoteStatus pairStatus = new RemoteStatus("http://pair",api); + RemoteStatus pairStatus = new RemoteStatus("http://pair", api); pair.setRemoteStatus(pairStatus); pair.setId(1L); - + when(pairRemoteService.mirrorSequencingObject(pair)).thenReturn(pair); - + syncService.syncSequenceFilePair(pair, sample); - + verify(pairRemoteService).mirrorSequencingObject(pair); verify(objectService).createSequencingObjectInSample(pair, sample); } @@ -274,7 +326,8 @@ public void testSyncSampleNoFast5() { sample.setRemoteStatus(sampleStatus); when(sampleService.create(sample)).thenReturn(sample); - when(sampleService.updateSampleMetadata(eq(sample), ArgumentMatchers.>any())).thenReturn(sample); + when(sampleService.updateSampleMetadata(eq(sample), ArgumentMatchers.>any())) + .thenReturn(sample); when(fast5ObjectRemoteService.getFast5FilesForSample(sample)).thenThrow(new LinkNotFoundException("no link")); syncService.syncSample(sample, expired, Maps.newHashMap()); @@ -318,19 +371,20 @@ public void testSyncFast5Files() { verify(fast5ObjectRemoteService).mirrorSequencingObject(fast5Object); verify(objectService).createSequencingObjectInSample(fast5Object, sample); } - + @Test public void testSyncFilesError() { Sample sample = new Sample(); - + SequenceFilePair pair = new SequenceFilePair(); - RemoteStatus pairStatus = new RemoteStatus("http://pair",api); + RemoteStatus pairStatus = new RemoteStatus("http://pair", api); pair.setRemoteStatus(pairStatus); pair.setId(1L); - + when(pairRemoteService.mirrorSequencingObject(pair)).thenReturn(pair); - when(objectService.createSequencingObjectInSample(pair, sample)).thenThrow(new NullPointerException("Bad file")); - + when(objectService.createSequencingObjectInSample(pair, sample)) + .thenThrow(new NullPointerException("Bad file")); + assertThrows(ProjectSynchronizationException.class, () -> { syncService.syncSequenceFilePair(pair, sample); }); From 2f383ba9571eee815082207032d79e88125e6d44 Mon Sep 17 00:00:00 2001 From: Eric Enns Date: Tue, 26 Jul 2022 13:43:56 -0500 Subject: [PATCH 2/3] Update changelog for updated sample syncing. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60db4a2f044..6e9321c862c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ * [Developer]: Switched custom exception handling to use built in Spring Boot exception handling. See [PR 1319](https://github.com/phac-nml/irida/pull/1319) * [UI]: Fixed issue with sorting OAuth clients table in admin panel. See [PR 1342](https://github.com/phac-nml/irida/pull/1342) * [UI]: Updated share samples review page to list the actual samples which will not be shared with the target project either due to the same sample identifiers or the same samples names already in the target project. See [PR 1343](https://github.com/phac-nml/irida/pull/1343) +* [REST]: Updated synchronizing of sample data to remove sequencing objects and assemblies that no longer exist on the remote sample. See [PR 1345](https://github.com/phac-nml/irida/pull/1345) ## [22.05.5] - 2022/06/28 * [UI]: Fixed bug preventing export of project samples table due to invalid url. [PR 1331](https://github.com/phac-nml/irida/pull/1331) From ea085f15ea3121daffc8ff01fd711cfc432fb29a Mon Sep 17 00:00:00 2001 From: Eric Enns Date: Wed, 3 Aug 2022 07:51:32 -0500 Subject: [PATCH 3/3] Fix comments in ProjectSynchronizationService --- .../remote/ProjectSynchronizationService.java | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java b/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java index 352a4f18b2b..8325d58c30e 100644 --- a/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java +++ b/src/main/java/ca/corefacility/bioinformatics/irida/service/remote/ProjectSynchronizationService.java @@ -1,5 +1,16 @@ package ca.corefacility.bioinformatics.irida.service.remote; +import java.util.*; +import java.util.stream.Collectors; + +import org.apache.commons.lang3.time.DateUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.stereotype.Service; + import ca.corefacility.bioinformatics.irida.exceptions.IridaOAuthException; import ca.corefacility.bioinformatics.irida.exceptions.LinkNotFoundException; import ca.corefacility.bioinformatics.irida.exceptions.ProjectSynchronizationException; @@ -17,22 +28,15 @@ import ca.corefacility.bioinformatics.irida.model.sample.Sample; import ca.corefacility.bioinformatics.irida.model.sample.SampleSequencingObjectJoin; import ca.corefacility.bioinformatics.irida.model.sample.metadata.MetadataEntry; -import ca.corefacility.bioinformatics.irida.model.sequenceFile.*; +import ca.corefacility.bioinformatics.irida.model.sequenceFile.Fast5Object; +import ca.corefacility.bioinformatics.irida.model.sequenceFile.SequenceFilePair; +import ca.corefacility.bioinformatics.irida.model.sequenceFile.SequencingObject; +import ca.corefacility.bioinformatics.irida.model.sequenceFile.SingleEndSequenceFile; import ca.corefacility.bioinformatics.irida.model.user.User; import ca.corefacility.bioinformatics.irida.security.ProjectSynchronizationAuthenticationToken; import ca.corefacility.bioinformatics.irida.service.*; import ca.corefacility.bioinformatics.irida.service.sample.MetadataTemplateService; import ca.corefacility.bioinformatics.irida.service.sample.SampleService; -import org.apache.commons.lang3.time.DateUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.security.core.context.SecurityContext; -import org.springframework.security.core.context.SecurityContextHolder; -import org.springframework.stereotype.Service; - -import java.util.*; -import java.util.stream.Collectors; import com.google.common.collect.Lists; @@ -387,14 +391,14 @@ public List syncSample(Sample sample, Project p .map(fast5Object -> fast5Object.getRemoteStatus().getURL()) .collect(Collectors.toSet())); - //get a list of all remote sequencingObject URLs in the sample + //get a list of all local sequencingObject URLs in the sample Set localObjectUrls = new HashSet<>(objectsByUrl.keySet()); //remove any URL from the local list that we've seen remotely remoteObjectUrls.forEach(objectUrl -> { localObjectUrls.remove(objectUrl); }); - //if any URLs still exists in localObjectUrls, it must have been deleted remotely + //if any URLs still exist in localObjectUrls, it must have been deleted remotely for (String localObjectUrl : localObjectUrls) { logger.trace( "SequencingObject " + localObjectUrl + " has been removed remotely. Removing from local sample."); @@ -467,14 +471,14 @@ public List syncSample(Sample sample, Project p .map(fast5Object -> fast5Object.getRemoteStatus().getURL()) .collect(Collectors.toSet())); - //get a list of all remote assembly URLs in the sample + //get a list of all local assembly URLs in the sample Set localAssemblyUrls = new HashSet<>(assembliesByUrl.keySet()); //remove any URL from the local list that we've seen remotely remoteAssemblyUrls.forEach(assemblyUrl -> { localAssemblyUrls.remove(assemblyUrl); }); - //if any URLs still exists in localAssemblyUrls, it must have been deleted remotely + //if any URLs still exist in localAssemblyUrls, it must have been deleted remotely for (String localAssemblyUrl : localAssemblyUrls) { logger.trace( "GenomeAssembly " + localAssemblyUrl + " has been removed remotely. Removing from local sample.");