From 2f07b4273905bbefc31cecf36157c3093c5b9ea2 Mon Sep 17 00:00:00 2001 From: sajo-ebi <77010716+sajo-ebi@users.noreply.github.com> Date: Thu, 10 Nov 2022 11:10:37 +0000 Subject: [PATCH 1/2] goci#809 Depocuration History fix --- .../impl/ConversionJaversServiceImpl.java | 29 +++++++++++++++++-- .../impl/FileUploadJaversServiceImpl.java | 9 ++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java index 8444a3e..13d9e01 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java @@ -42,6 +42,7 @@ public Optional>> filterJaversResponse(Lis List commitIdChanges = changeList.stream() .filter((javersChangeWrapper) -> ( javersChangeWrapper.getProperty().equals("metadataStatus")) && + javersChangeWrapper.getLeft().toString().equals("VALIDATING") && javersChangeWrapper.getRight().toString().equals("VALID")) .map((change) -> change.getCommitMetadata().getId()) .collect(Collectors.toList()); @@ -90,12 +91,30 @@ public List removeInvalidSumstatsEntries(List javer List invalidSumstatsIndexes = IntStream.range(0, javersChangeWrapperList.size()) .filter(i -> ( javersChangeWrapperList.get(i).getProperty().equals("summaryStatsStatus") && javersChangeWrapperList.get(i).getRight().equals("INVALID"))) + //&& javersChangeWrapperList.get(i).getLeft().toString().equals("VALIDATING"))) .mapToObj(i -> i) .collect(Collectors.toList()); - List versionList = invalidSumstatsIndexes.stream() + List versionList = new ArrayList<>(); + + if(invalidSumstatsIndexes != null && !invalidSumstatsIndexes.isEmpty()) { + for (int i = 0; i < invalidSumstatsIndexes.size(); i++) { + int idx = invalidSumstatsIndexes.get(i); + for (int j = idx + 1; j < javersChangeWrapperList.size(); j++) { + + if (javersChangeWrapperList.get(j).getProperty().equals("metadataStatus") && + javersChangeWrapperList.get(j).getLeft().toString().equals("VALIDATING") && + javersChangeWrapperList.get(j).getRight().toString().equals("VALID")) { + versionList.add(javersChangeWrapperList.get(j).getCommitMetadata().getId()); + break; + } + } + } + } + +/* List versionList = invalidSumstatsIndexes.stream() .map(idx -> javersChangeWrapperList.get(idx+1).getCommitMetadata().getId()) - .collect(Collectors.toList()); + .collect(Collectors.toList());*/ versionList.forEach(version -> log.info(" Version->"+version)); @@ -153,6 +172,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("studies")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(studyJaversService::processStudyTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); List prevStudies = oldChange.stream() @@ -160,6 +180,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("studies")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(studyJaversService::processStudyTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); List newAssociations = newChange.stream() @@ -167,6 +188,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("associations")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(associationJaversService::processAssociationTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -175,6 +197,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("associations")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(associationJaversService::processAssociationTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); List newSamples = newChange.stream() @@ -182,6 +205,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("samples")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(sampleJaversService::processSampleTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); @@ -190,6 +214,7 @@ private VersionSummary compareVersions(List newChange, List javersChangeWrapper.getProperty().equals("samples")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) .map(sampleJaversService::processSampleTag) + .filter(Objects::nonNull) .collect(Collectors.toList()); log.info("newStudies****"+newStudies); diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/FileUploadJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/FileUploadJaversServiceImpl.java index d8d0554..92a5d76 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/FileUploadJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/FileUploadJaversServiceImpl.java @@ -10,7 +10,7 @@ @Service public class FileUploadJaversServiceImpl implements FileUploadJaversService { - @Autowired + @Autowired FileUploadsService fileUploadsService; public FileUpload getFileUploadDetails(String fileId){ @@ -23,8 +23,13 @@ public String processFileUploadTag(ElementChange elementChange) { } else if(elementChange.getElementChangeType().equals("ValueRemoved")){ return elementChange.getValue().toString(); + }else if(elementChange.getElementChangeType().equals("ElementValueChange")) { + return elementChange.getRightValue().toString(); } - return null; + else { + return elementChange.getLeftValue().toString(); + } + //return null; } } From 70a85fd80555a75c475d7f8a687f917eb0ad5ffe Mon Sep 17 00:00:00 2001 From: sajo-ebi <77010716+sajo-ebi@users.noreply.github.com> Date: Fri, 25 Nov 2022 16:59:15 +0000 Subject: [PATCH 2/2] goci#809-Submission-history-issue Handling Edge scenarios which were breaking the version history code --- .../repository/AssociationRepository.java | 1 + .../curation/repository/SampleRepository.java | 3 +- .../rest/SubmissionDiffController.java | 8 +- .../service/AssociationJaversService.java | 2 +- .../curation/service/AssociationsService.java | 3 + .../service/ConversionJaversService.java | 8 +- .../curation/service/SampleJaversService.java | 2 +- .../gwas/curation/service/SamplesService.java | 3 + .../gwas/curation/service/StudiesService.java | 3 + .../curation/service/StudyJaversService.java | 2 +- .../impl/AssociationJaversServiceImpl.java | 21 +- .../service/impl/AssociationsServiceImpl.java | 10 +- .../impl/ConversionJaversServiceImpl.java | 219 +++++++++++++++--- .../service/impl/SampleJaversServiceImpl.java | 21 +- .../service/impl/SamplesServiceImpl.java | 10 +- .../service/impl/StudiesServiceImpl.java | 9 +- .../service/impl/StudyJaversServiceImpl.java | 52 +++-- .../service/ConversionJaversServiceTest.java | 2 +- 18 files changed, 294 insertions(+), 85 deletions(-) diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/AssociationRepository.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/AssociationRepository.java index 845c020..9213b17 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/AssociationRepository.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/AssociationRepository.java @@ -13,6 +13,7 @@ public interface AssociationRepository extends MongoRepository { Stream readBySubmissionId(String submissionId); + Stream readByIdIn(List ids); Integer countByIsValidAndSubmissionId(Boolean isValid, String submissionId); Integer countByIsApprovedAndSubmissionId(Boolean isApproved, String submissionId); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/SampleRepository.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/SampleRepository.java index 6a3bb8a..97b1519 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/SampleRepository.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/repository/SampleRepository.java @@ -16,5 +16,6 @@ public interface SampleRepository extends MongoRepository { Page findBySubmissionId(String submissionId, Pageable page); - List findByIdIn(List ids); + + Stream findByIdIn(List ids); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/rest/SubmissionDiffController.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/rest/SubmissionDiffController.java index 4af9ea7..af36b30 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/rest/SubmissionDiffController.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/rest/SubmissionDiffController.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; @RestController @RequestMapping(value = GeneralCommon.API_V1 + DepositionCurationConstants.API_SUBMISSIONS) @@ -49,9 +50,14 @@ public List diffVersionSubmissions(@PathVariable String submissi if(changeList != null && !changeList.isEmpty()) { Optional>> convertedEntityOptional = conversionService.filterJaversResponse(changeList); List versionMapTobeDeleted = conversionService.removeInvalidSumstatsEntries(changeList); + Optional> versionMapMetaDataDuplicated = conversionService.removeDuplicateMetaDataVersions(convertedEntityOptional); if(versionMapTobeDeleted != null && !versionMapTobeDeleted.isEmpty()) conversionService.removeVersionMap(convertedEntityOptional, versionMapTobeDeleted ); - summaries = conversionService.filterStudiesFromJavers(convertedEntityOptional); + if(versionMapMetaDataDuplicated.isPresent()) { + conversionService.removeVersionMap(convertedEntityOptional,versionMapMetaDataDuplicated.get() ); + } + + summaries = conversionService.buiildVersionSummary(convertedEntityOptional); Optional> fileUploadsOptional = conversionService.filterJaversResponseForFiles(responseEntity.getBody()); summaries = fileUploadsOptional.isPresent()?conversionService.mapFilesToVersionSummary(summaries, fileUploadsOptional.get()):null; } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationJaversService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationJaversService.java index 6416ddf..226ab97 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationJaversService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationJaversService.java @@ -17,5 +17,5 @@ public interface AssociationJaversService { public AddedRemoved getAssociationVersionStats(List prevAscns, List newAscns); - public Association processAssociationTag(ElementChange elementChange); + public String processAssociationTag(ElementChange elementChange); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationsService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationsService.java index 4de28af..c64d3e1 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationsService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/AssociationsService.java @@ -4,12 +4,15 @@ import uk.ac.ebi.spot.gwas.deposition.dto.curation.SnpStatusReportDto; import java.util.List; +import java.util.stream.Stream; public interface AssociationsService { Association getAssociation(String associationId); + Stream readBySeqIds(List ids); + List getAssociations(String submissionId); byte[] getSnpValidationReportTsv(String submissionId); diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/ConversionJaversService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/ConversionJaversService.java index 6c783ed..e51c63a 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/ConversionJaversService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/ConversionJaversService.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; public interface ConversionJaversService { @@ -17,9 +18,14 @@ public interface ConversionJaversService { public void removeVersionMap(Optional>> javersChangeWrapperMap, List versionsTobeRemoved ); + public void removeVersionMap(Optional>> javersChangeWrapperMap, + Set versionsTobeRemoved ); + + public Optional> removeDuplicateMetaDataVersions(Optional>> javersChangeWrapperMap); + public Optional> filterJaversResponseForFiles(List javersChangeWrapperList); - public List filterStudiesFromJavers(Optional>> javersChangeWrapperList); + public List buiildVersionSummary(Optional>> javersChangeWrapperList); public List mapFilesToVersionSummary(List summaries, List fileUploads); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SampleJaversService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SampleJaversService.java index 200ffde..4c5258b 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SampleJaversService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SampleJaversService.java @@ -17,7 +17,7 @@ public interface SampleJaversService { public AddedRemoved getSampleVersionStats(List prevSamples, List newSamples); - public Sample processSampleTag(ElementChange elementChange); + public String processSampleTag(ElementChange elementChange); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SamplesService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SamplesService.java index 62bbbac..1f20a88 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SamplesService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/SamplesService.java @@ -6,10 +6,13 @@ import uk.ac.ebi.spot.gwas.deposition.domain.Submission; import java.util.List; +import java.util.stream.Stream; public interface SamplesService { Sample getSample(String sampleId); + Stream findByIdIn(List ids); + } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudiesService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudiesService.java index c1c6d88..b31d2fa 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudiesService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudiesService.java @@ -9,12 +9,15 @@ import uk.ac.ebi.spot.gwas.deposition.dto.curation.*; import java.util.List; +import java.util.stream.Stream; public interface StudiesService { Study getStudy(String studyId); + Stream getStudies(List ids); + Study getStudyByAccession(String accessionId, String submissionId); Study updateStudies(Study study); diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudyJaversService.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudyJaversService.java index b4c68b0..2b628c7 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudyJaversService.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/StudyJaversService.java @@ -21,6 +21,6 @@ public interface StudyJaversService { public AddedRemoved getReportedEfoVersionStats(List prevStudies, List newStudies); - public Study processStudyTag(ElementChange elementChange); + public String processStudyTag(ElementChange elementChange); } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationJaversServiceImpl.java index 2042d00..bcc3db0 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationJaversServiceImpl.java @@ -42,7 +42,7 @@ public VersionDiffStats findAssociationChanges(String tag, List pre diffStats.setAssociations(new ArrayList<>()); prevAscns.forEach((asscn) -> { - log.info("VariantId*****"+asscn.getVariantId()); + //log.info("VariantId*****"+asscn.getVariantId()); List newAsscnsDto = newAscns.stream() .filter((ascn) -> ascn.getVariantId().equals(asscn.getVariantId())) .map(AssociationDtoAssembler::assemble) @@ -70,8 +70,8 @@ public VersionDiffStats findAssociationChanges(String tag, List pre public List diffAssociations(AssociationDto dto1, AssociationDto dto2) { Javers javers = JaversBuilder.javers().build(); Diff diff = javers.compare(dto1, dto2); - log.info("************"); - log.info("Diff Asscn"+ diff); + //log.info("************"); + //log.info("Diff Asscn"+ diff); List valChanges = diff.getChangesByType(ValueChange.class); try { ValueChangeWrapper[] changes = new ObjectMapper().readValue( @@ -84,7 +84,7 @@ public List diffAssociations(AssociationDto dto1, Associatio } public AddedRemoved getAssociationVersionStats(List prevAscns, List newAscns) { - log.info("Inside getAssociationVersionStats() "); + //log.info("Inside getAssociationVersionStats() "); List newAscnsTags = newAscns.stream() .map(asscn -> asscn.getStudyTag() + asscn.getVariantId()) @@ -102,10 +102,10 @@ public AddedRemoved getAssociationVersionStats(List prevAscns, List .filter(asscn -> !prevAscnsTags.contains(asscn.getStudyTag() + asscn.getVariantId())) .collect(Collectors.toList()); - log.info("newAscnsTags****"+newAscnsTags); - log.info("prevAscnsTags****"+prevAscnsTags); - log.info("asscnsRemoved****"+asscnsRemoved); - log.info("asscnsAdded****"+asscnsAdded); + //log.info("newAscnsTags****"+newAscnsTags); + // log.info("prevAscnsTags****"+prevAscnsTags); + // log.info("asscnsRemoved****"+asscnsRemoved); + //log.info("asscnsAdded****"+asscnsAdded); AddedRemoved addedRemoved = new AddedRemoved(); addedRemoved.setAdded(asscnsAdded.size()); @@ -114,9 +114,10 @@ public AddedRemoved getAssociationVersionStats(List prevAscns, List return addedRemoved; } - public Association processAssociationTag(ElementChange elementChange){ + public String processAssociationTag(ElementChange elementChange){ if (elementChange.getElementChangeType().equals("ValueAdded")){ - return associationsService.getAssociation(elementChange.getValue().toString() ); + //return associationsService.getAssociation(elementChange.getValue().toString() ); + return elementChange.getValue().toString(); } return null; } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationsServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationsServiceImpl.java index d3aeeba..68b4c2f 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationsServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/AssociationsServiceImpl.java @@ -20,6 +20,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; +import java.util.stream.Stream; @Service public class AssociationsServiceImpl implements AssociationsService { @@ -39,10 +40,10 @@ public class AssociationsServiceImpl implements AssociationsService { @Override public Association getAssociation(String associationId) { - log.info("Retrieving association: {}", associationId); + //log.info("Retrieving association: {}", associationId); Optional associationOptional = associationRepository.findById(associationId); if (associationOptional.isPresent()) { - log.info("Found association: {}", associationOptional.get().getStudyTag()); + //log.info("Found association: {}", associationOptional.get().getStudyTag()); return associationOptional.get(); } log.error("Unable to find association: {}", associationId); @@ -100,4 +101,9 @@ public SnpStatusReportDto getSnpStatus(String submissionId) { snpStatusReportDto.setNoValidSnps(associationRepository.countByIsValidAndSubmissionId(true, submissionId)); return snpStatusReportDto; } + + @Override + public Stream readBySeqIds(List ids) { + return associationRepository.readByIdIn(ids); + } } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java index 13d9e01..d80919a 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/ConversionJaversServiceImpl.java @@ -13,6 +13,7 @@ import uk.ac.ebi.spot.gwas.deposition.javers.*; import java.util.*; +import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -33,8 +34,26 @@ public class ConversionJaversServiceImpl implements ConversionJaversService { @Autowired private FileUploadJaversService fileUploadJaversService; + @Autowired + private StudiesService studiesService; + + @Autowired + private AssociationsService associationsService; + + @Autowired + private SamplesService samplesService; + /** + * Look for file upload events from Javers response + * & capture all the versionIds for the events + * use these versionIds to retrieve the studies, + * & other entities , the version id will be key + * for map & events as the value when the file upload + * happens + * @param javersChangeWrapperList + * @return + */ @Override public Optional>> filterJaversResponse(List javersChangeWrapperList) { return Optional.of(javersChangeWrapperList) @@ -57,6 +76,16 @@ public Optional>> filterJaversResponse(Lis }); } + + /** The file details events are not saved in the same event + * as file upload + * so have to parse Javers response for file details + * & programmetically associate them with the specific + * version + * + * @param javersChangeWrapperList + * @return + */ @Override public Optional> filterJaversResponseForFiles(List javersChangeWrapperList) { @@ -80,7 +109,7 @@ public Optional> filterJaversResponseForFiles(List removeInvalidSumstatsEntries(List javer List invalidSumstatsIndexes = IntStream.range(0, javersChangeWrapperList.size()) .filter(i -> ( javersChangeWrapperList.get(i).getProperty().equals("summaryStatsStatus") - && javersChangeWrapperList.get(i).getRight().equals("INVALID"))) + && ( javersChangeWrapperList.get(i).getRight().equals("INVALID") + //|| javersChangeWrapperList.get(i).getLeft().equals("INVALID") + ))) //&& javersChangeWrapperList.get(i).getLeft().toString().equals("VALIDATING"))) .mapToObj(i -> i) .collect(Collectors.toList()); @@ -100,8 +131,9 @@ public List removeInvalidSumstatsEntries(List javer if(invalidSumstatsIndexes != null && !invalidSumstatsIndexes.isEmpty()) { for (int i = 0; i < invalidSumstatsIndexes.size(); i++) { int idx = invalidSumstatsIndexes.get(i); - for (int j = idx + 1; j < javersChangeWrapperList.size(); j++) { - + versionList.add(javersChangeWrapperList.get(idx).getCommitMetadata().getId()); + for (int j = idx; j < javersChangeWrapperList.size(); j++) { + //for (int j = idx; j >= 0; j--) { if (javersChangeWrapperList.get(j).getProperty().equals("metadataStatus") && javersChangeWrapperList.get(j).getLeft().toString().equals("VALIDATING") && javersChangeWrapperList.get(j).getRight().toString().equals("VALID")) { @@ -122,20 +154,83 @@ public List removeInvalidSumstatsEntries(List javer } + /** + * Handle Scenarios where the entities are updated + * possibly due to frequent submission resets + * this create duplicate copies of same fileupload + * events , fetch the version for those & remove + * them from final versionid events which will be + * used for comparison + * @param javersChangeWrapperMap + * @return + */ + @Override + public Optional> removeDuplicateMetaDataVersions(Optional>> javersChangeWrapperMap) { + + Set commitIdstoDelete = new HashSet<>(); + + return javersChangeWrapperMap.map(javersChangeMap -> { + javersChangeMap.forEach((key, changes) -> + changes.forEach(change -> { + if (change.getProperty().equals("overallStatus") && change.getLeft().toString().equals("VALIDATING") && + ( change.getRight().toString().equals("SUBMITTED") || change.getRight().toString().equals("CURATION_COMPLETE") || change.getRight().toString().equals("DEPOSITION_COMPLETE"))) { + commitIdstoDelete.add(change.getCommitMetadata().getId()); + log.info("Version which is duplicated for Metadata -:"+change.getCommitMetadata().getId()); + } + + + })); + + return commitIdstoDelete; + }); + + + } + + /** + * Remove Invalid sumstats events from versions map + * @param javersChangeWrapperMap + * @param versionsTobeRemoved + */ + @Override public void removeVersionMap(Optional>> javersChangeWrapperMap, List versionsTobeRemoved ) { versionsTobeRemoved.forEach((version) -> javersChangeWrapperMap.get().remove(version)); } + /** + * Remove Duplicate metadata evets from versions map + * @param javersChangeWrapperMap + * @param versionsTobeRemoved + */ + @Override + public void removeVersionMap(Optional>> javersChangeWrapperMap, + Set versionsTobeRemoved ) { + versionsTobeRemoved.forEach((version) -> javersChangeWrapperMap.get().remove(version)); + } + + /** + * Calls the compare version method for + * file upload events for each successive + * file uploads + * @param javersChangeWrapperList + * @return + */ @Override - public List filterStudiesFromJavers(Optional>> javersChangeWrapperList) { + public List buiildVersionSummary(Optional>> javersChangeWrapperList) { Map> versionMap = javersChangeWrapperList.orElse(null); log.info("versionMap ****"+versionMap); + + + if(versionMap != null && !versionMap.isEmpty()){ if(versionMap.size() < 2 ){ throw new NoVersionSummaryException("No Version Stats are available currently for this submission," + " Please upload the edited Template file"); } + } else { + throw new NoVersionSummaryException("No Version Stats are available currently for this submission," + + " Since the submission has been reset"); } List summaries = new ArrayList<>(); Set keys = versionMap.keySet(); @@ -151,23 +246,67 @@ public List filterStudiesFromJavers(Optional mapFilesToVersionSummary(List summaries, List fileUploads) { VersionSummary[] summaryArr = summaries.toArray(new VersionSummary[summaries.size()]); FileUpload[] fileUploadArr = fileUploads.toArray(new FileUpload[fileUploads.size()]); - for(int i = 0 ; i < summaryArr.length; i++) { - summaryArr[i].setOldFileDetails(new FileSummaryStats( fileUploadArr[i+1].getFileName(), - fileUploadArr[i+1].getId())); - summaryArr[i].setNewFileDetails(new FileSummaryStats( fileUploadArr[i].getFileName(), - fileUploadArr[i].getId())); + log.info("VersionSummary length ->"+summaryArr.length); + log.info("fileUploadArr length ->"+fileUploadArr.length); + + /* if(summaryArr.length != (fileUploadArr.length - 1) ) + throw new NoVersionSummaryException("No Version Stats are available currently for this submission," + + " Since the submission has been reset");*/ + + if(summaryArr.length > (fileUploadArr.length - 1) ){ + List updatedsummaryArr = new ArrayList<>(); + for(int i = 0 ; i < fileUploadArr.length - 1; i++) { + summaryArr[i].setOldFileDetails(new FileSummaryStats( fileUploadArr[i+1].getFileName(), + fileUploadArr[i+1].getId())); + summaryArr[i].setNewFileDetails(new FileSummaryStats( fileUploadArr[i].getFileName(), + fileUploadArr[i].getId())); + updatedsummaryArr.add(summaryArr[i]); + } + + return updatedsummaryArr; + } else { + + for (int i = 0; i < summaryArr.length; i++) { + summaryArr[i].setOldFileDetails(new FileSummaryStats(fileUploadArr[i + 1].getFileName(), + fileUploadArr[i + 1].getId())); + summaryArr[i].setNewFileDetails(new FileSummaryStats(fileUploadArr[i].getFileName(), + fileUploadArr[i].getId())); + + } + return Arrays.asList(summaryArr); } - return Arrays.asList(summaryArr); - } + } + /** + * Compare version events list events + * Fetch Study & other entity based on SeqId + * in the events + * Group studies, association & samples + * based on study tags for each version + * then Build version summary object comparing + * the entities associated with each study tags + * also the reported Traits & efo traits stats + * are computed + * @param newChange + * @param oldChange + * @return + */ private VersionSummary compareVersions(List newChange, List oldChange) { - List newStudies = newChange.stream() + List newStudiesIds = newChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("studies")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -175,7 +314,12 @@ private VersionSummary compareVersions(List newChange, List .filter(Objects::nonNull) .collect(Collectors.toList()); - List prevStudies = oldChange.stream() + + List newStudies = studiesService.getStudies(newStudiesIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + + + List prevStudiesIds = oldChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("studies")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -183,7 +327,12 @@ private VersionSummary compareVersions(List newChange, List .filter(Objects::nonNull) .collect(Collectors.toList()); - List newAssociations = newChange.stream() + + List prevStudies = studiesService.getStudies(prevStudiesIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + + + List newAssociationsIds = newChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("associations")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -192,7 +341,12 @@ private VersionSummary compareVersions(List newChange, List .collect(Collectors.toList()); - List prevAssociations = oldChange.stream() + List newAssociations = associationsService.readBySeqIds(newAssociationsIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + + + + List prevAssociationsIds = oldChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("associations")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -200,7 +354,10 @@ private VersionSummary compareVersions(List newChange, List .filter(Objects::nonNull) .collect(Collectors.toList()); - List newSamples = newChange.stream() + List prevAssociations = associationsService.readBySeqIds(prevAssociationsIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + + List newSamplesIds = newChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("samples")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -208,8 +365,11 @@ private VersionSummary compareVersions(List newChange, List .filter(Objects::nonNull) .collect(Collectors.toList()); + List newSamples = samplesService.findByIdIn(newSamplesIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + - List prevSamples = oldChange.stream() + List prevSamplesIds = oldChange.stream() .filter( (javersChangeWrapper) -> javersChangeWrapper.getProperty().equals("samples")) .flatMap((javersChange) -> javersChange.getElementChanges().stream()) @@ -217,10 +377,13 @@ private VersionSummary compareVersions(List newChange, List .filter(Objects::nonNull) .collect(Collectors.toList()); - log.info("newStudies****"+newStudies); - log.info("prevStudies****"+prevStudies); - log.info("newAssociations****"+newAssociations); - log.info("prevAssociations****"+prevAssociations); + List prevSamples = samplesService.findByIdIn(prevSamplesIds).filter(Objects::nonNull) + .collect(Collectors.toList()); + + //log.info("newStudies****"+newStudies); + //log.info("prevStudies****"+prevStudies); + //log.info("newAssociations****"+newAssociations); + //log.info("prevAssociations****"+prevAssociations); VersionSummary versionSummary = new VersionSummary(); versionSummary.setCurrentVersionSummary(populateCurrentVersionSummary( @@ -266,10 +429,10 @@ private VersionSummary compareVersions(List newChange, List prevStudyMap.forEach((tag, studyList) -> { - log.info("Study Tag ****"+tag); + //log.info("Study Tag ****"+tag); VersionDiffStats versionStudyDiffStats = studyJaversService.findStudyChanges(tag, studyList, newStudies); if(prevstudyAscnsMap.get(tag) != null ) { - log.info("Inside Association loop "); + //log.info("Inside Association loop "); AddedRemoved addedRemovedAsscns = associationJaversService.getAssociationVersionStats(prevstudyAscnsMap.get(tag), newstudyAscnsMap.get(tag) !=null ? newstudyAscnsMap.get(tag) : Collections.emptyList()); @@ -280,7 +443,7 @@ private VersionSummary compareVersions(List newChange, List } else { if (newstudyAscnsMap.get(tag) != null) { - log.info("Inside Association loop where old study has no asscn "); + //log.info("Inside Association loop where old study has no asscn "); AddedRemoved addedRemovedAsscns = associationJaversService.getAssociationVersionStats(Collections.emptyList(), newstudyAscnsMap.get(tag) ); versionStudyDiffStats.setAscnsAdded(addedRemovedAsscns.getAdded()); @@ -294,7 +457,7 @@ private VersionSummary compareVersions(List newChange, List } if(prevStudySamplesMap.get(tag) != null ) { - log.info("Inside Sample loop "); + //log.info("Inside Sample loop "); AddedRemoved addedRemovedSamples = sampleJaversService.getSampleVersionStats(prevStudySamplesMap.get(tag), newStudySamplesMap.get(tag) !=null ? newStudySamplesMap.get(tag) : Collections.emptyList()); versionStudyDiffStats.setSamplesAdded(addedRemovedSamples.getAdded()); @@ -303,7 +466,7 @@ private VersionSummary compareVersions(List newChange, List newStudySamplesMap.get(tag) : Collections.emptyList(), versionStudyDiffStats); } else { if(newStudySamplesMap.get(tag) != null) { - log.info("Inside Study loop where old study has no Sample "); + //log.info("Inside Study loop where old study has no Sample "); AddedRemoved addedRemovedSamples = sampleJaversService.getSampleVersionStats(Collections.emptyList(), newStudySamplesMap.get(tag) !=null ? newStudySamplesMap.get(tag) : Collections.emptyList()); versionStudyDiffStats.setSamplesAdded(addedRemovedSamples.getAdded()); @@ -324,7 +487,7 @@ private VersionSummary compareVersions(List newChange, List newStudyMap.forEach((tag, studyList) -> { if(studyTagsList != null && studyTagsList.contains(tag)) { log.info("Studies added newly"); - log.info("Studies added ->"+tag); + //log.info("Studies added ->"+tag); VersionDiffStats newversionDiffStats = new VersionDiffStats(); newversionDiffStats.setEntity(tag); AddedRemoved addedRemovedAsscns = associationJaversService.getAssociationVersionStats(Collections.emptyList(), diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SampleJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SampleJaversServiceImpl.java index c16d912..5839242 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SampleJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SampleJaversServiceImpl.java @@ -42,7 +42,7 @@ public VersionDiffStats findSampleChanges(List prevSamples, List diffStats.setSampleGroups(new ArrayList<>()); prevSamples.forEach((sample) -> { - log.info("Sample*****"+sample.getStage()+"|"+sample.getAncestryCategory()); + //log.info("Sample*****"+sample.getStage()+"|"+sample.getAncestryCategory()); List newSamplesDto = newSamples.stream() .filter((sampleGroup) -> (sampleGroup.getStage() + sampleGroup.getAncestryCategory()) .equals(sample.getStage() + sample.getAncestryCategory())) @@ -71,8 +71,8 @@ public VersionDiffStats findSampleChanges(List prevSamples, List public List diffSamples(SampleDto dto1, SampleDto dto2) { Javers javers = JaversBuilder.javers().build(); Diff diff = javers.compare(dto1, dto2); - log.info("************"); - log.info("Diff Sample"+ diff); + //log.info("************"); + //log.info("Diff Sample"+ diff); List valChanges = diff.getChangesByType(ValueChange.class); try { ValueChangeWrapper[] changes = new ObjectMapper().readValue( @@ -85,7 +85,7 @@ public List diffSamples(SampleDto dto1, SampleDto dto2) { } public AddedRemoved getSampleVersionStats(List prevSamples, List newSamples) { - log.info("Inside getAssociationVersionStats() "); + //log.info("Inside getAssociationVersionStats() "); List newSamplesTags = newSamples.stream() .map(sample -> sample.getStudyTag() + sample.getStage() +sample.getAncestryCategory()) @@ -105,10 +105,10 @@ public AddedRemoved getSampleVersionStats(List prevSamples, List sample.getAncestryCategory())) .collect(Collectors.toList()); - log.info("newSamplesTags****"+newSamplesTags); - log.info("prevSamplesTags****"+prevSamplesTags); - log.info("samplesRemoved****"+samplesRemoved); - log.info("samplesAdded****"+samplesAdded); + //log.info("newSamplesTags****"+newSamplesTags); + //log.info("prevSamplesTags****"+prevSamplesTags); + //log.info("samplesRemoved****"+samplesRemoved); + //log.info("samplesAdded****"+samplesAdded); AddedRemoved addedRemoved = new AddedRemoved(); addedRemoved.setAdded(samplesAdded.size()); @@ -117,9 +117,10 @@ public AddedRemoved getSampleVersionStats(List prevSamples, List return addedRemoved; } - public Sample processSampleTag(ElementChange elementChange) { + public String processSampleTag(ElementChange elementChange) { if (elementChange.getElementChangeType().equals("ValueAdded")){ - return samplesService.getSample(elementChange.getValue().toString() ); + //return samplesService.getSample(elementChange.getValue().toString() ); + return elementChange.getValue().toString(); } return null; } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SamplesServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SamplesServiceImpl.java index 5dedfd0..4f8f4c0 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SamplesServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/SamplesServiceImpl.java @@ -8,7 +8,9 @@ import uk.ac.ebi.spot.gwas.curation.service.SamplesService; import uk.ac.ebi.spot.gwas.deposition.domain.Sample; +import java.util.List; import java.util.Optional; +import java.util.stream.Stream; @Service public class SamplesServiceImpl implements SamplesService { @@ -22,16 +24,20 @@ public class SamplesServiceImpl implements SamplesService { @Override public Sample getSample(String sampleId) { - log.info("Retrieving sample: {}", sampleId); + //log.info("Retrieving sample: {}", sampleId); Optional sampleOptional = sampleRepository.findById(sampleId); if (sampleOptional.isPresent()) { - log.info("Found sample: {}", sampleOptional.get().getStudyTag()); + //log.info("Found sample: {}", sampleOptional.get().getStudyTag()); return sampleOptional.get(); } log.error("Unable to find sample: {}", sampleId); return null; } + @Override + public Stream findByIdIn(List ids) { + return sampleRepository.findByIdIn(ids); + } } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudiesServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudiesServiceImpl.java index be0befc..27e30c7 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudiesServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudiesServiceImpl.java @@ -35,6 +35,7 @@ import java.util.*; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import java.util.stream.Stream; @Service public class StudiesServiceImpl implements StudiesService { @@ -81,10 +82,10 @@ public Study updateStudies(Study study) { @Override public Study getStudy(String studyId) { - log.info("Retrieving study: {}", studyId); + //log.info("Retrieving study: {}", studyId); Optional studyOptional = studyRepository.findById(studyId); if (studyOptional.isPresent()) { - log.info("Found study: {}", studyOptional.get().getStudyTag()); + //log.info("Found study: {}", studyOptional.get().getStudyTag()); return studyOptional.get(); } log.error("Unable to find study: {}", studyId); @@ -524,4 +525,8 @@ public byte[] uploadSampleDescriptions(List studySa public void sendStudyChangeMessage(Study study){ studyIngestPublisher.send(StudyDtoAssembler.assemble(study)); } + + public Stream getStudies(List ids) { + return studyRepository.readByIdIn(ids); + } } diff --git a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudyJaversServiceImpl.java b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudyJaversServiceImpl.java index 5242182..3e0bd94 100644 --- a/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudyJaversServiceImpl.java +++ b/src/main/java/uk/ac/ebi/spot/gwas/curation/service/impl/StudyJaversServiceImpl.java @@ -39,7 +39,7 @@ public VersionDiffStats findStudyChanges(String tag, List studyList, List .map(StudyDtoAssembler::assemble) .collect(Collectors.toList()); - log.info("Inside findStudyChanges newStudie**** "); + //log.info("Inside findStudyChanges newStudie**** "); List prevStudiesDTO = studyList.stream() .map(StudyDtoAssembler::assemble) @@ -66,13 +66,16 @@ public VersionDiffStats findStudyChanges(String tag, List studyList, List public List diffStudies(StudyDto dto1, StudyDto dto2) { Javers javers = JaversBuilder.javers().build(); Diff diff = javers.compare(dto1, dto2); - log.info("************"); - log.info("Diff" + diff); + //log.info("************"); + //log.info("Diff" + diff); List valChanges = diff.getChangesByType(ValueChange.class); try { ValueChangeWrapper[] changes = new ObjectMapper().readValue( javers.getJsonConverter().toJson(valChanges), ValueChangeWrapper[].class); - return Arrays.asList(changes); + + List values = Arrays.asList(changes); + List filteredProps = values.stream().filter(val -> (!val.getProperty().equals("studyId") && !val.getProperty().equals("submissionId"))).collect(Collectors.toList()); + return filteredProps; } catch (Exception ex) { log.error("Error in mapping Javers Changes" + ex.getMessage(), ex); return null; @@ -105,10 +108,10 @@ public AddedRemoved getStudyVersionStats(List prevStudies, List ne .collect(Collectors.joining(",")); - log.info("newStudyTags****" + newStudyTags); - log.info("prevStudyTags****" + prevStudyTags); - log.info("studiesRemoved****" + studiesRemoved); - log.info("studiesAdded****" + studiesAdded); + //log.info("newStudyTags****" + newStudyTags); + //log.info("prevStudyTags****" + prevStudyTags); + //log.info("studiesRemoved****" + studiesRemoved); + // log.info("studiesAdded****" + studiesAdded); versionDiffStats.setStudyTagsAdded(studyTagsAdded); versionDiffStats.setStudyTagsRemoved(studyTagsRemoved); @@ -122,7 +125,7 @@ public AddedRemoved getStudyVersionStats(List prevStudies, List ne public AddedRemoved getReportedTraitVersionStats(List prevStudies, List newStudies) { - log.info("Inside getReportedTraitVersionStats()"); + //log.info("Inside getReportedTraitVersionStats()"); List newReportedTraits = newStudies.stream() .map(Study::getTrait) @@ -140,10 +143,10 @@ public AddedRemoved getReportedTraitVersionStats(List prevStudies, List !prevReportedTraits.contains(study.getTrait())) .collect(Collectors.toList()); - log.info("newReportedTraits****"+newReportedTraits); - log.info("prevReportedTraits****"+prevReportedTraits); - log.info("traitsRemoved****"+traitsRemoved); - log.info("traitsAdded****"+traitsAdded); + //log.info("newReportedTraits****"+newReportedTraits); + //log.info("prevReportedTraits****"+prevReportedTraits); + //log.info("traitsRemoved****"+traitsRemoved); + //log.info("traitsAdded****"+traitsAdded); AddedRemoved addedRemoved = new AddedRemoved(); addedRemoved.setAdded(traitsAdded.size()); @@ -155,7 +158,7 @@ public AddedRemoved getReportedTraitVersionStats(List prevStudies, List prevStudies, List newStudies) { - log.info("Inside getReportedEfoVersionStats()"); + //log.info("Inside getReportedEfoVersionStats()"); List newEfoTraits = newStudies.stream() .map(Study::getEfoTrait) .filter((efo) -> efo!=null && !efo.isEmpty()) @@ -164,7 +167,7 @@ public AddedRemoved getReportedEfoVersionStats(List prevStudies, List prevEfoTraits = prevStudies.stream() .map(Study::getEfoTrait) @@ -174,7 +177,7 @@ public AddedRemoved getReportedEfoVersionStats(List prevStudies, List efoRemoved = prevStudies.stream() .filter((study) -> !newEfoTraits.contains(study.getTrait())) .collect(Collectors.toList());*/ @@ -188,7 +191,7 @@ public AddedRemoved getReportedEfoVersionStats(List prevStudies, List efoAdded = newStudies.stream() .filter((study) -> !prevEfoTraits.contains(study.getTrait())) .collect(Collectors.toList());*/ @@ -201,12 +204,12 @@ public AddedRemoved getReportedEfoVersionStats(List prevStudies, List prevStudies, List versions = conversionJaversService.filterStudiesFromJavers(changeMap); + List versions = conversionJaversService.buiildVersionSummary(changeMap); assertNotNull(versions); }