From 7e1819a97a05c6c5a055033741e6c065f5cf9838 Mon Sep 17 00:00:00 2001 From: lismana Date: Mon, 19 Aug 2024 11:13:08 -0400 Subject: [PATCH] Code review updates --- .../persistence/StudyViewRepository.java | 2 +- .../mybatisclickhouse/StudyViewMapper.java | 2 +- .../StudyViewMyBatisRepository.java | 28 ++-------------- .../impl/StudyViewColumnarServiceImpl.java | 33 ++++++++++++++++++- .../mybatisclickhouse/StudyViewMapper.xml | 2 +- ...> StudyViewCaseListSamplesCountsTest.java} | 14 ++++---- 6 files changed, 44 insertions(+), 37 deletions(-) rename src/test/java/org/cbioportal/persistence/mybatisclickhouse/{CaseListSamplesCountsTest.java => StudyViewCaseListSamplesCountsTest.java} (92%) diff --git a/src/main/java/org/cbioportal/persistence/StudyViewRepository.java b/src/main/java/org/cbioportal/persistence/StudyViewRepository.java index 95dc1b6936d..cf80eefa8d1 100644 --- a/src/main/java/org/cbioportal/persistence/StudyViewRepository.java +++ b/src/main/java/org/cbioportal/persistence/StudyViewRepository.java @@ -38,7 +38,7 @@ public interface StudyViewRepository { Map getClinicalAttributeDatatypeMap(); - List getCaseListDataCounts(StudyViewFilter studyViewFilter); + List getCaseListDataCountsPerStudy(StudyViewFilter studyViewFilter); Map getTotalProfiledCounts(StudyViewFilter studyViewFilter, String alterationType); diff --git a/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.java b/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.java index 4721b93809e..e77553aff81 100644 --- a/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.java +++ b/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.java @@ -36,7 +36,7 @@ List getStructuralVariantGenes(StudyViewFilter studyViewF List getClinicalDataCounts(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter, boolean applyPatientIdFilters, List attributeIds, List filteredAttributeValues); - List getCaseListDataCounts(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter, boolean applyPatientIdFilters); + List getCaseListDataCountsPerStudy(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter, boolean applyPatientIdFilters); List getClinicalAttributes(); diff --git a/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java b/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java index eeb629443e2..61db1213dab 100644 --- a/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java +++ b/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java @@ -110,34 +110,10 @@ public Map getClinicalAttributeDatatypeMap() { return attributeDatatypeMap; } - public static List mergeCaseListCounts(List counts) { - Map> countsPerListType = counts.stream() - .collect((Collectors.groupingBy(CaseListDataCount::getValue))); - - // different cancer studies combined into one cohort will have separate case lists - // of a given type (e.g. rppa). We need to merge the counts for these - // different lists based on the type and choose a label - // this code just picks the first label, which assumes that the labels will match for a give type - List mergedCounts = new ArrayList<>(); - for (Map.Entry> entry : countsPerListType.entrySet()) { - var dc = new CaseListDataCount(); - dc.setValue(entry.getKey()); - // here just snatch the label of the first profile - dc.setLabel(entry.getValue().get(0).getLabel()); - Integer sum = entry.getValue().stream() - .map(x -> x.getCount()) - .collect(Collectors.summingInt(Integer::intValue)); - dc.setCount(sum); - mergedCounts.add(dc); - } - return mergedCounts; - } - @Override - public List getCaseListDataCounts(StudyViewFilter studyViewFilter) { + public List getCaseListDataCountsPerStudy(StudyViewFilter studyViewFilter) { CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter = extractClinicalDataCountFilters(studyViewFilter); - var caseListDataCounts = mapper.getCaseListDataCounts(studyViewFilter, categorizedClinicalDataCountFilter, shouldApplyPatientIdFilters(studyViewFilter,categorizedClinicalDataCountFilter)); - return mergeCaseListCounts(caseListDataCounts); + return mapper.getCaseListDataCountsPerStudy(studyViewFilter, categorizedClinicalDataCountFilter, shouldApplyPatientIdFilters(studyViewFilter,categorizedClinicalDataCountFilter)); } diff --git a/src/main/java/org/cbioportal/service/impl/StudyViewColumnarServiceImpl.java b/src/main/java/org/cbioportal/service/impl/StudyViewColumnarServiceImpl.java index 94142c39b4f..5968248a0c8 100644 --- a/src/main/java/org/cbioportal/service/impl/StudyViewColumnarServiceImpl.java +++ b/src/main/java/org/cbioportal/service/impl/StudyViewColumnarServiceImpl.java @@ -21,6 +21,7 @@ import org.springframework.cache.annotation.Cacheable; import org.springframework.stereotype.Service; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -102,7 +103,10 @@ public List getClinicalDataCounts(StudyViewFilter studyVi @Override public List getCaseListDataCounts(StudyViewFilter studyViewFilter) { - return studyViewRepository.getCaseListDataCounts(studyViewFilter); + // the study view merges case lists by type across studies + // type is determined by the suffix of case list name (after study name) + var caseListDataCountsPerStudy = studyViewRepository.getCaseListDataCountsPerStudy(studyViewFilter); + return mergeCaseListCounts(caseListDataCountsPerStudy); } @@ -115,4 +119,31 @@ public List getPatientClinicalData(StudyViewFilter studyViewFilter public List getSampleClinicalData(StudyViewFilter studyViewFilter, List attributeIds) { return studyViewRepository.getSampleClinicalData(studyViewFilter, attributeIds); } + + + public static List mergeCaseListCounts(List counts) { + Map> countsPerListType = counts.stream() + .collect((Collectors.groupingBy(CaseListDataCount::getValue))); + + // different cancer studies combined into one cohort will have separate case lists + // of a given type (e.g. rppa). We need to merge the counts for these + // different lists based on the type and choose a label + // this code just picks the first label, which assumes that the labels will match for a give type + List mergedCounts = new ArrayList<>(); + for (Map.Entry> entry : countsPerListType.entrySet()) { + var dc = new CaseListDataCount(); + dc.setValue(entry.getKey()); + // here just snatch the label of the first profile + dc.setLabel(entry.getValue().get(0).getLabel()); + Integer sum = entry.getValue().stream() + .map(x -> x.getCount()) + .collect(Collectors.summingInt(Integer::intValue)); + dc.setCount(sum); + mergedCounts.add(dc); + } + return mergedCounts; + } + + + } diff --git a/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.xml b/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.xml index 87cc4da1ab1..1e1acabcee0 100644 --- a/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.xml +++ b/src/main/resources/org/cbioportal/persistence/mybatisclickhouse/StudyViewMapper.xml @@ -148,7 +148,7 @@ - SELECT name AS label, REPLACE(stable_id, CONCAT(cancer_study_identifier, '_'), '') AS value, diff --git a/src/test/java/org/cbioportal/persistence/mybatisclickhouse/CaseListSamplesCountsTest.java b/src/test/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewCaseListSamplesCountsTest.java similarity index 92% rename from src/test/java/org/cbioportal/persistence/mybatisclickhouse/CaseListSamplesCountsTest.java rename to src/test/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewCaseListSamplesCountsTest.java index d9b054cddbb..cb7af6bef7a 100644 --- a/src/test/java/org/cbioportal/persistence/mybatisclickhouse/CaseListSamplesCountsTest.java +++ b/src/test/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewCaseListSamplesCountsTest.java @@ -1,8 +1,8 @@ package org.cbioportal.persistence.mybatisclickhouse; -import org.cbioportal.persistence.StudyViewRepository; import org.cbioportal.persistence.mybatisclickhouse.config.MyBatisConfig; +import org.cbioportal.service.impl.StudyViewColumnarServiceImpl; import org.cbioportal.web.parameter.CategorizedClinicalDataCountFilter; import org.cbioportal.web.parameter.StudyViewFilter; import org.junit.Test; @@ -27,7 +27,7 @@ @DirtiesContext @AutoConfigureTestDatabase(replace= AutoConfigureTestDatabase.Replace.NONE) @ContextConfiguration(initializers = AbstractTestcontainers.Initializer.class) -public class CaseListSamplesCountsTest extends AbstractTestcontainers { +public class StudyViewCaseListSamplesCountsTest extends AbstractTestcontainers { private static final String STUDY_TCGA_PUB = "study_tcga_pub"; private static final String STUDY_ACC_TCGA = "acc_tcga"; @@ -45,7 +45,7 @@ public void getMolecularProfileCounts() { studyViewFilter.setCaseLists(caseListGroups); - var sampleListCounts = studyViewMapper.getCaseListDataCounts(studyViewFilter, + var sampleListCounts = studyViewMapper.getCaseListDataCountsPerStudy(studyViewFilter, CategorizedClinicalDataCountFilter.getBuilder().build(), false ); var size = sampleListCounts.stream().filter(gc->gc.getValue().equals("mrna")) @@ -64,7 +64,7 @@ public void getMolecularProfileCountsMultipleListsOr() { studyViewFilter.setCaseLists(caseListGroups); - var sampleListCounts = studyViewMapper.getCaseListDataCounts(studyViewFilter, + var sampleListCounts = studyViewMapper.getCaseListDataCountsPerStudy(studyViewFilter, CategorizedClinicalDataCountFilter.getBuilder().build(), false ); var size = sampleListCounts.stream().filter(gc->gc.getValue().equals("mrna")) @@ -84,7 +84,7 @@ public void getMolecularProfileCountsMultipleListsAnd() { studyViewFilter.setCaseLists(caseListGroups); - var sampleListCounts = studyViewMapper.getCaseListDataCounts(studyViewFilter, + var sampleListCounts = studyViewMapper.getCaseListDataCountsPerStudy(studyViewFilter, CategorizedClinicalDataCountFilter.getBuilder().build(), false ); var size = sampleListCounts.stream().filter(gc->gc.getValue().equals("mrna")) @@ -103,10 +103,10 @@ public void getMolecularProfileCountsAcrossStudies() { studyViewFilter.setCaseLists(caseListGroups); - var unMergedCounts = studyViewMapper.getCaseListDataCounts(studyViewFilter, + var unMergedCounts = studyViewMapper.getCaseListDataCountsPerStudy(studyViewFilter, CategorizedClinicalDataCountFilter.getBuilder().build(), false ); - var caseListCountsMerged = StudyViewMyBatisRepository.mergeCaseListCounts( + var caseListCountsMerged = StudyViewColumnarServiceImpl.mergeCaseListCounts( unMergedCounts );