diff --git a/db-scripts/src/main/resources/migration.sql b/db-scripts/src/main/resources/migration.sql index eb649e95a17..78c0d25ea14 100644 --- a/db-scripts/src/main/resources/migration.sql +++ b/db-scripts/src/main/resources/migration.sql @@ -1026,4 +1026,5 @@ UPDATE `info` SET `DB_SCHEMA_VERSION`="2.13.0"; ALTER TABLE `clinical_event_data` MODIFY COLUMN `VALUE` varchar(3000) NOT NULL; CREATE INDEX idx_clinical_event_key ON clinical_event_data (`KEY`); CREATE INDEX idx_clinical_event_value ON clinical_event_data (`VALUE`); +CREATE INDEX idx_sample_stable_id ON sample (`STABLE_ID`); UPDATE `info` SET `DB_SCHEMA_VERSION`="2.13.1"; \ No newline at end of file diff --git a/persistence/persistence-api/src/main/java/org/cbioportal/persistence/TreatmentRepository.java b/persistence/persistence-api/src/main/java/org/cbioportal/persistence/TreatmentRepository.java index 99011ef74c8..bc2e0705b16 100644 --- a/persistence/persistence-api/src/main/java/org/cbioportal/persistence/TreatmentRepository.java +++ b/persistence/persistence-api/src/main/java/org/cbioportal/persistence/TreatmentRepository.java @@ -14,20 +14,18 @@ public interface TreatmentRepository { public Map> getTreatmentsByPatientId(List sampleIds, List studyIds, ClinicalEventKeyCode key); @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") - public Map> getSamplesByPatientId(List sampleIds, List studyIds); + public List getTreatments(List sampleIds, List studyIds, ClinicalEventKeyCode key); @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") - public Map> getShallowSamplesByPatientId(List sampleIds, List studyIds); + public Map> getSamplesByPatientId(List sampleIds, List studyIds); @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") - public Set getAllUniqueTreatments(List sampleIds, List studyIds, ClinicalEventKeyCode key); + public Map> getShallowSamplesByPatientId(List sampleIds, List studyIds); @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") - public Boolean hasTreatmentData(List studies, String key); + public Boolean hasTreatmentData(List studies, ClinicalEventKeyCode key); @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") public Boolean hasSampleTimelineData(List studies); - @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()") - public Boolean studyIdHasTreatments(String studyId, ClinicalEventKeyCode key); } diff --git a/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMapper.java b/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMapper.java index b92b086e21e..44e4f856079 100644 --- a/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMapper.java +++ b/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMapper.java @@ -13,11 +13,7 @@ public interface TreatmentMapper { List getAllShallowSamples(List sampleIds, List studyIds); - Set getAllUniqueTreatments(List sampleIds, List studyIds, String key); - Boolean hasTreatmentData(List sampleIds, List studyIds, String key); Boolean hasSampleTimelineData(List sampleIds, List studyIds); - - Boolean studyIdHasTreatments(String studyId, String key); } diff --git a/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepository.java b/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepository.java index 768d3a0b7d1..d44bdfc808a 100644 --- a/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepository.java +++ b/persistence/persistence-mybatis/src/main/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepository.java @@ -18,12 +18,17 @@ public class TreatmentMyBatisRepository implements TreatmentRepository { @Override public Map> getTreatmentsByPatientId(List sampleIds, List studyIds, ClinicalEventKeyCode key) { + return getTreatments(sampleIds, studyIds, key) + .stream() + .collect(groupingBy(Treatment::getPatientId)); + } + + @Override + public List getTreatments(List sampleIds, List studyIds, ClinicalEventKeyCode key) { return treatmentMapper.getAllTreatments(sampleIds, studyIds, key.getKey()) .stream() .flatMap(treatment -> splitIfDelimited(treatment, key)) - .collect(groupingBy(Treatment::getPatientId)); - - + .collect(Collectors.toList()); } private Stream splitIfDelimited(Treatment unsplitTreatment, ClinicalEventKeyCode key) { @@ -61,31 +66,12 @@ public Map> getShallowSamplesByPatientId(List< } @Override - public Set getAllUniqueTreatments(List sampleIds, List studyIds, ClinicalEventKeyCode key) { - return treatmentMapper.getAllUniqueTreatments(sampleIds, studyIds, key.getKey()) - .stream() - .flatMap(treatment -> { - if (key.isDelimited()) { - return Arrays.stream(treatment.split(key.getDelimiter())); - } else { - return Stream.of(treatment); - } - }) - .collect(Collectors.toSet()); - } - - @Override - public Boolean hasTreatmentData(List studies, String key) { - return treatmentMapper.hasTreatmentData(null, studies, key); + public Boolean hasTreatmentData(List studies, ClinicalEventKeyCode key) { + return treatmentMapper.hasTreatmentData(null, studies, key.getKey()); } @Override public Boolean hasSampleTimelineData(List studies) { return treatmentMapper.hasSampleTimelineData(null, studies); } - - @Override - public Boolean studyIdHasTreatments(String studyId, ClinicalEventKeyCode key) { - return treatmentMapper.studyIdHasTreatments(studyId, key.getKey()); - } } diff --git a/persistence/persistence-mybatis/src/main/resources/org/cbioportal/persistence/mybatis/TreatmentMapper.xml b/persistence/persistence-mybatis/src/main/resources/org/cbioportal/persistence/mybatis/TreatmentMapper.xml index 3a4aa1a6ad9..de633ab7f55 100644 --- a/persistence/persistence-mybatis/src/main/resources/org/cbioportal/persistence/mybatis/TreatmentMapper.xml +++ b/persistence/persistence-mybatis/src/main/resources/org/cbioportal/persistence/mybatis/TreatmentMapper.xml @@ -16,6 +16,7 @@ INNER JOIN sample ON patient.INTERNAL_ID = sample.PATIENT_ID INNER JOIN cancer_study ON patient.CANCER_STUDY_ID = cancer_study.CANCER_STUDY_ID + AND clinical_event.EVENT_TYPE = 'TREATMENT' AND clinical_event_data.KEY = #{key} @@ -48,19 +49,6 @@ - - @@ -88,20 +77,6 @@ AND clinical_event_data.KEY = 'SAMPLE_ID' AND (clinical_event.EVENT_TYPE LIKE 'Sample Acquisition' OR clinical_event.EVENT_TYPE LIKE 'SPECIMEN') LIMIT 1) - - - diff --git a/persistence/persistence-mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java b/persistence/persistence-mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java index 5f4e0bcd1b0..ec5b95fa12a 100644 --- a/persistence/persistence-mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java +++ b/persistence/persistence-mybatis/src/test/java/org/cbioportal/persistence/mybatis/TreatmentMyBatisRepositoryTest.java @@ -133,22 +133,9 @@ public void getShallowSamplesByPatientId() { Assert.assertEquals(actual, expected); } - @Test - public void getAllUniqueTreatments() { - HashSet expected = new HashSet<>(Collections.singletonList("Madeupanib")); - - Set actual = treatmentRepository.getAllUniqueTreatments( - Collections.singletonList("TCGA-A1-A0SD-01"), - Collections.singletonList("study_tcga_pub"), - ClinicalEventKeyCode.Agent - ); - - Assert.assertEquals(actual, expected); - } - @Test public void hasTreatmentData() { - Boolean actual = treatmentRepository.hasTreatmentData(Collections.singletonList("study_tcga_pub"), ClinicalEventKeyCode.Agent.getKey()); + Boolean actual = treatmentRepository.hasTreatmentData(Collections.singletonList("study_tcga_pub"), ClinicalEventKeyCode.Agent); Assert.assertEquals(actual, true); } diff --git a/service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java b/service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java index 0409fac2e52..ab7e2ed778f 100644 --- a/service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java +++ b/service/src/main/java/org/cbioportal/service/impl/TreatmentServiceImpl.java @@ -25,7 +25,7 @@ private Pair, List> filterIds(List sampleIds, List< } Set studiesWithTreatments = studyIds.stream() .distinct() - .filter(studyId -> treatmentRepository.studyIdHasTreatments(studyId, key)) + .filter(studyId -> treatmentRepository.hasTreatmentData(Collections.singletonList(studyId), key)) .collect(Collectors.toSet()); ArrayList filteredSampleIds = new ArrayList<>(); @@ -158,62 +158,37 @@ public List getAllPatientTreatmentRows( sampleIds = filteredIds.getLeft(); studyIds = filteredIds.getRight(); - Map> treatmentsByPatient = - treatmentRepository.getTreatmentsByPatientId(sampleIds, studyIds, key); Map> samplesByPatient = treatmentRepository - .getShallowSamplesByPatientId(sampleIds, studyIds) - .entrySet() - .stream() - .filter(e -> treatmentsByPatient.containsKey(e.getKey())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - Set treatments = treatmentRepository.getAllUniqueTreatments(sampleIds, studyIds, key); - - return treatments.stream() - .map(t -> createPatientTreatmentRowForTreatment(t, treatmentsByPatient, samplesByPatient)) - .collect(Collectors.toList()); - } - - private PatientTreatmentRow createPatientTreatmentRowForTreatment( - String treatment, - Map> treatmentsByPatient, - Map> samplesByPatient - ) { - // find all the patients that have received this treatment - List>> matchingPatients = matchingPatients(treatment, treatmentsByPatient); + .getShallowSamplesByPatientId(sampleIds, studyIds); - // from those patients, extract the unique samples - Set samples = matchingPatients + Map> treatmentSet = treatmentRepository.getTreatments(sampleIds, studyIds, key) .stream() - .map(Map.Entry::getKey) - .flatMap(patient -> samplesByPatient.getOrDefault(patient, new ArrayList<>()).stream()) - .collect(Collectors.toSet()); - - - return new PatientTreatmentRow(treatment, matchingPatients.size(), samples); - } + .collect(groupingBy(Treatment::getTreatment)); - private List>> matchingPatients( - String treatment, - Map> treatmentsByPatient - ) { - return treatmentsByPatient.entrySet().stream() - .filter(p -> p.getValue().stream().anyMatch(t -> t.getTreatment().equals(treatment))) + return treatmentSet.entrySet() + .stream() + .map(entry -> { + String treatment = entry.getKey(); + Set patientIds = entry.getValue().stream().map(Treatment::getPatientId).collect(toSet()); + Set clinicalEventSamples = patientIds + .stream() + .flatMap(patientId -> samplesByPatient.getOrDefault(patientId, new ArrayList<>()).stream()) + .collect(toSet()); + return new PatientTreatmentRow(treatment, patientIds.size(), clinicalEventSamples); + }) .collect(toList()); } @Override public Boolean containsTreatmentData(List studies, ClinicalEventKeyCode key) { - return treatmentRepository.hasTreatmentData(studies, key.getKey()); + return treatmentRepository.hasTreatmentData(studies, key); } @Override public Boolean containsSampleTreatmentData(List studyIds, ClinicalEventKeyCode key) { studyIds = studyIds.stream() - .filter(studyId -> treatmentRepository.studyIdHasTreatments(studyId, key)) + .filter(studyId -> treatmentRepository.hasTreatmentData(Collections.singletonList(studyId), key)) .collect(Collectors.toList()); - if(studyIds.size() > 0 && treatmentRepository.hasSampleTimelineData(studyIds)) { - return treatmentRepository.hasTreatmentData(studyIds, key.getKey()); - } - return false; + return studyIds.size() > 0 && treatmentRepository.hasSampleTimelineData(studyIds); } } diff --git a/service/src/test/java/org/cbioportal/service/impl/TreatmentServiceImplTest.java b/service/src/test/java/org/cbioportal/service/impl/TreatmentServiceImplTest.java index 9b489f16690..8a86e465d3f 100644 --- a/service/src/test/java/org/cbioportal/service/impl/TreatmentServiceImplTest.java +++ b/service/src/test/java/org/cbioportal/service/impl/TreatmentServiceImplTest.java @@ -26,14 +26,11 @@ public class TreatmentServiceImplTest { private TreatmentRepository treatmentRepository; @Test - public void getAllSampleTreatmentsSingleRow() { - mockTreatmentsByPatient( - makeTreatment("madeupanib", "P0", 0, 10) - ); + public void getAllPatientTreatmentRows() { mockSamplesByPatient( makeSample("S0", "P0", 5) ); - mockAllTreatments("madeupanib"); + mockTreatments(makeTreatment("madeupanib", "P0", 0, 10)); PatientTreatmentRow rowA = makePatientRow("madeupanib", 1, Collections.singletonList("S0"), Collections.singletonList("P0")); List expected = Collections.singletonList(rowA); @@ -43,15 +40,14 @@ public void getAllSampleTreatmentsSingleRow() { } @Test - public void getAllSampleTreatmentsOneSampleTwoTreatmentsOnePatient() { - mockTreatmentsByPatient( + public void getAllPatientTreatmentRowsOneSampleTwoTreatmentsOnePatient() { + mockTreatments( makeTreatment("madeupanib", "P0", 0, 10), makeTreatment("fakedrugazol", "P0", 0, 10) ); mockSamplesByPatient( makeSample("S0", "P0", 5) ); - mockAllTreatments("madeupanib", "fakedrugazol"); PatientTreatmentRow rowA = makePatientRow("fakedrugazol", 1, Collections.singletonList("S0"), Collections.singletonList("P0")); @@ -63,15 +59,14 @@ public void getAllSampleTreatmentsOneSampleTwoTreatmentsOnePatient() { } @Test - public void getAllSampleTreatmentsTwoSamplesOnePatientOneTreatment() { - mockTreatmentsByPatient( + public void getAllPatientTreatmentRowsTwoSamplesOnePatientOneTreatment() { + mockTreatments( makeTreatment("fakedrugazol", "P0", 0, 10) ); mockSamplesByPatient( makeSample("S0", "P0", 5), makeSample("S1", "P0", 10) ); - mockAllTreatments("fakedrugazol"); // even though there are two samples, you expect a count of 1, // because this is from the patient level, and both samples are for the same patient @@ -83,8 +78,8 @@ public void getAllSampleTreatmentsTwoSamplesOnePatientOneTreatment() { } @Test - public void getAllSampleTreatmentsTwoSamplesTwoPatientsTwoTreatments() { - mockTreatmentsByPatient( + public void getAllPatientTreatmentRowsTwoSamplesTwoPatientsTwoTreatments() { + mockTreatments( makeTreatment("fakedrugazol", "P0", 0, 10), makeTreatment("fakedrugazol", "P1", 0, 10) ); @@ -92,7 +87,6 @@ public void getAllSampleTreatmentsTwoSamplesTwoPatientsTwoTreatments() { makeSample("S0", "P0", 5), makeSample("S1", "P1", 10) ); - mockAllTreatments("fakedrugazol"); // now there are two patients, so you expect a count of two PatientTreatmentRow rowA = makePatientRow("fakedrugazol", 2, Arrays.asList("S0", "S1"), Arrays.asList("P0", "P1")); @@ -103,8 +97,8 @@ public void getAllSampleTreatmentsTwoSamplesTwoPatientsTwoTreatments() { } @Test - public void getAllSampleTreatmentsTwoSamplesTwoPatientsTwoDifferentTreatments() { - mockTreatmentsByPatient( + public void getAllPatientTreatmentRowsTwoSamplesTwoPatientsTwoDifferentTreatments() { + mockTreatments( makeTreatment("fakedrugazol", "P0", 0, 10), makeTreatment("madeupanib", "P1", 0, 10) ); @@ -112,7 +106,6 @@ public void getAllSampleTreatmentsTwoSamplesTwoPatientsTwoDifferentTreatments() makeSample("S0", "P0", 5), makeSample("S1", "P1", 10) ); - mockAllTreatments("fakedrugazol", "madeupanib"); PatientTreatmentRow rowA = makePatientRow("fakedrugazol", 1, Collections.singletonList("S0"), Collections.singletonList("P0")); PatientTreatmentRow rowB = makePatientRow("madeupanib", 1, Collections.singletonList("S1"), Collections.singletonList("P1")); @@ -252,6 +245,11 @@ private void mockTreatmentsByPatient(Treatment... treatments) { .thenReturn(treatmentsByPatient); } + private void mockTreatments(Treatment... treatments) { + Mockito.when(treatmentRepository.getTreatments(Mockito.any(), Mockito.any(), Mockito.any())) + .thenReturn(Arrays.stream(treatments).collect(Collectors.toList())); + } + private void mockSamplesByPatient(ClinicalEventSample... samples) { Map> samplesByPatient = Arrays.stream(samples) .collect(Collectors.groupingBy(ClinicalEventSample::getPatientId)); @@ -261,12 +259,6 @@ private void mockSamplesByPatient(ClinicalEventSample... samples) { .thenReturn(samplesByPatient); } - private void mockAllTreatments(String... treatments) { - Set allTreatments = new HashSet<>(Arrays.asList(treatments)); - Mockito.when(treatmentRepository.getAllUniqueTreatments(Mockito.any(), Mockito.any(), Mockito.any())) - .thenReturn(allTreatments); - } - private Treatment makeTreatment(String treatment, String patientId, Integer start, Integer stop) { Treatment t = new Treatment(); t.setTreatment(treatment);