Code review updates

cBioPortal · Aug 19, 2024 · a74cade · a74cade
1 parent 95fcc74
commit a74cade
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 37 deletions.
diff --git a/src/main/java/org/cbioportal/persistence/StudyViewRepository.java b/src/main/java/org/cbioportal/persistence/StudyViewRepository.java
@@ -40,7 +40,7 @@ public interface StudyViewRepository {
 
     Map<String, ClinicalDataType> getClinicalAttributeDatatypeMap();
 
-    List<CaseListDataCount> getCaseListDataCounts(StudyViewFilter studyViewFilter);
+    List<CaseListDataCount> getCaseListDataCountsPerStudy(StudyViewFilter studyViewFilter);
 
     Map<String, Integer> 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
@@ -39,7 +39,7 @@ List<AlterationCountByGene> getStructuralVariantGenes(StudyViewFilter studyViewF
     List<ClinicalDataCount> getClinicalDataCounts(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter,
                                                   boolean applyPatientIdFilters, List<String> attributeIds, List<String> filteredAttributeValues);
 
-    List<CaseListDataCount> getCaseListDataCounts(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter, boolean applyPatientIdFilters);
+    List<CaseListDataCount> getCaseListDataCountsPerStudy(StudyViewFilter studyViewFilter, CategorizedClinicalDataCountFilter categorizedClinicalDataCountFilter, boolean applyPatientIdFilters);
 
     List<ClinicalAttribute> getClinicalAttributes();
 

diff --git a/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java b/src/main/java/org/cbioportal/persistence/mybatisclickhouse/StudyViewMyBatisRepository.java
@@ -135,34 +135,10 @@ public Map<String, ClinicalDataType> getClinicalAttributeDatatypeMap() {
         return attributeDatatypeMap;
     }
 
-    public static List<CaseListDataCount> mergeCaseListCounts(List<CaseListDataCount> counts) {
-        Map<String, List<CaseListDataCount>> 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<CaseListDataCount> mergedCounts = new ArrayList<>();
-        for (Map.Entry<String,List<CaseListDataCount>> 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<CaseListDataCount> getCaseListDataCounts(StudyViewFilter studyViewFilter) {
+    public List<CaseListDataCount> 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
@@ -105,7 +105,10 @@ public List<ClinicalDataCountItem> getClinicalDataCounts(StudyViewFilter studyVi
 
     @Override
     public List<CaseListDataCount> 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);
     }
 
 
@@ -146,4 +149,31 @@ public List<GenomicDataCountItem> getMutationTypeCountsByGeneSpecific(StudyViewF
         return studyViewRepository.getMutationCountsByType(studyViewFilter, genomicDataFilters);
     }
 
+
+
+    public static List<CaseListDataCount> mergeCaseListCounts(List<CaseListDataCount> counts) {
+        Map<String, List<CaseListDataCount>> 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<CaseListDataCount> mergedCounts = new ArrayList<>();
+        for (Map.Entry<String,List<CaseListDataCount>> 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
@@ -148,7 +148,7 @@
     </select>
 
     <!-- for /sample-lists-counts/fetch (returns CaseListDataCount) -->
-    <select id="getCaseListDataCounts" resultType="org.cbioportal.model.CaseListDataCount">
+    <select id="getCaseListDataCountsPerStudy" resultType="org.cbioportal.model.CaseListDataCount">
         SELECT
             name AS label,
             REPLACE(stable_id, CONCAT(cancer_study_identifier, '_'), '') AS value,

diff --git a/...clickhouse/CaseListSamplesCountsTest.java → ...e/StudyViewCaseListSamplesCountsTest.java b/...clickhouse/CaseListSamplesCountsTest.java → ...e/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
         );