Refactor sort/search/pagination for ClinicalData table endpoint

src/main/java/org/cbioportal/model/SampleClinicalDataCollection.java

@@ -0,0 +1,36 @@
+package org.cbioportal.model;
+
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+public final class SampleClinicalDataCollection {
+
+    private final Map<String, List<ClinicalData>> byUniqueSampleKey;
+
+    private SampleClinicalDataCollection(Builder builder) {
+        this.byUniqueSampleKey = Collections.unmodifiableMap(new HashMap<>(builder.byUniqueSampleKey));
+    }
+
+    public Map<String, List<ClinicalData>> getByUniqueSampleKey() {
+        return byUniqueSampleKey;
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public static class Builder {
+        private final Map<String, List<ClinicalData>> byUniqueSampleKey = new HashMap<>();
+
+        public Builder withByUniqueSampleKey(Map<String, List<ClinicalData>> byUniqueSampleKey) {
+            this.byUniqueSampleKey.putAll(byUniqueSampleKey);
+            return this;
+        }
+
+        public SampleClinicalDataCollection build() {
+            return new SampleClinicalDataCollection(this);
+        }
+    }
+}

src/main/java/org/cbioportal/persistence/ClinicalDataRepository.java

@@ -47,14 +47,6 @@ BaseMeta fetchMetaClinicalDataInStudy(String studyId, List<String> ids, List<Str
     List<ClinicalData> fetchClinicalData(List<String> studyIds, List<String> ids, List<String> attributeIds,
                                          String clinicalDataType, String projection);
 
-    List<ClinicalData> fetchSampleClinicalTable(List<String> studyIds, List<String> ids,
-                                                Integer pageSize, Integer pageNumber, String searchTerm,
-                                                String sortBy, String direction);
-
-    @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()")
-    Integer fetchSampleClinicalTableCount(List<String> studyIds, List<String> ids, String searchTerm,
-                                          String sortBy, String direction);
-
     @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()")
     BaseMeta fetchMetaClinicalData(List<String> studyIds, List<String> ids, List<String> attributeIds,
                                    String clinicalDataType);
@@ -65,5 +57,13 @@ List<ClinicalDataCount> fetchClinicalDataCounts(List<String> studyIds, List<Stri
 
     @Cacheable(cacheResolver = "generalRepositoryCacheResolver", condition = "@cacheEnabledConfig.getEnabled()")
     List<ClinicalData> getPatientClinicalDataDetailedToSample(List<String> studyIds, List<String> patientIds,
-            List<String> attributeIds);
+                                                              List<String> attributeIds);
+
+    List<Integer> getVisibleSampleInternalIdsForClinicalTable(List<String> studyIds, List<String> sampleIds,
+                                                              Integer pageSize, Integer pageNumber, String searchTerm,
+                                                              String sortBy, String direction);
+
+    List<ClinicalData> getSampleClinicalDataBySampleInternalIds(List<Integer> visibleSampleInternalIds);
+
+    List<ClinicalData> getPatientClinicalDataBySampleInternalIds(List<Integer> visibleSampleInternalIds);
 }

src/main/java/org/cbioportal/persistence/mybatis/CancerTypeMyBatisRepository.java

@@ -4,7 +4,7 @@
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.CancerTypeRepository;
 import org.cbioportal.persistence.PersistenceConstants;
-import org.cbioportal.persistence.mybatis.util.OffsetCalculator;
+import org.cbioportal.persistence.mybatis.util.PaginationCalculator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
 
@@ -15,15 +15,13 @@ public class CancerTypeMyBatisRepository implements CancerTypeRepository {
 
     @Autowired
     private CancerTypeMapper cancerTypeMapper;
-    @Autowired
-    private OffsetCalculator offsetCalculator;
 
     @Override
     public List<TypeOfCancer> getAllCancerTypes(String projection, Integer pageSize, Integer pageNumber, String sortBy,
                                                 String direction) {
 
         return cancerTypeMapper.getAllCancerTypes(projection, pageSize,
-                offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+                PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override

src/main/java/org/cbioportal/persistence/mybatis/ClinicalAttributeMyBatisRepository.java

@@ -5,7 +5,7 @@
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.ClinicalAttributeRepository;
 import org.cbioportal.persistence.PersistenceConstants;
-import org.cbioportal.persistence.mybatis.util.OffsetCalculator;
+import org.cbioportal.persistence.mybatis.util.PaginationCalculator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
 
@@ -17,15 +17,13 @@ public class ClinicalAttributeMyBatisRepository implements ClinicalAttributeRepo
 
     @Autowired
     private ClinicalAttributeMapper clinicalAttributeMapper;
-    @Autowired
-    private OffsetCalculator offsetCalculator;
 
     @Override
     public List<ClinicalAttribute> getAllClinicalAttributes(String projection, Integer pageSize, Integer pageNumber,
                                                             String sortBy, String direction) {
 
         return clinicalAttributeMapper.getClinicalAttributes(null, projection, pageSize,
-                offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+                PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override
@@ -47,7 +45,7 @@ public List<ClinicalAttribute> getAllClinicalAttributesInStudy(String studyId, S
                                                                    String direction) {
 
         return clinicalAttributeMapper.getClinicalAttributes(Arrays.asList(studyId), projection, pageSize,
-                offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+                PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override

src/main/java/org/cbioportal/persistence/mybatis/ClinicalDataMapper.java

@@ -20,7 +20,8 @@ List<ClinicalData> getPatientClinicalData(List<String> studyIds, List<String> pa
 
     List<ClinicalData> getSampleClinicalTable(List<String> studyIds, List<String> sampleIds, String projection,
                                               Integer limit, Integer offset, String searchTerm,
-                                              String sortBy, String direction);
+                                              String sortByAttrId, Boolean sortAttrIsNumber, Boolean sortIsPatientAttr,
+                                              String direction);
 
     Integer getSampleClinicalTableCount(List<String> studyIds, List<String> sampleIds, String projection,
                                         String searchTerm, String sortBy, String direction);
@@ -36,4 +37,13 @@ List<ClinicalDataCount> fetchPatientClinicalDataCounts(List<String> studyIds, Li
     List<ClinicalData> getPatientClinicalDataDetailedToSample(List<String> studyIds, List<String> patientIds,
                                                               List<String> attributeIds, String projection, Integer limit,
                                                               Integer offset, String sortBy, String direction);
+
+    List<Integer> getVisibleSampleInternalIdsForClinicalTable(List<String> studyIds, List<String> sampleIds,
+                                                              String projection, Integer limit, Integer offset,
+                                                              String searchTerm, String sortAttrId, Boolean sortAttrIsNumber,
+                                                              Boolean sortIsPatientAttr, String direction);
+
+    List<ClinicalData> getSampleClinicalDataBySampleInternalIds(List<Integer> sampleInternalIds);
+
+    List<ClinicalData> getPatientClinicalDataBySampleInternalIds(List<Integer> sampleInternalIds);
 }

src/main/java/org/cbioportal/persistence/mybatis/ClinicalDataMyBatisRepository.java

@@ -1,20 +1,22 @@
 package org.cbioportal.persistence.mybatis;
 
+import org.cbioportal.model.ClinicalAttribute;
 import org.cbioportal.model.ClinicalData;
 import org.cbioportal.model.ClinicalDataCount;
 import org.cbioportal.model.Patient;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.ClinicalDataRepository;
+import org.cbioportal.persistence.ClinicalAttributeRepository;
 import org.cbioportal.persistence.PatientRepository;
 import org.cbioportal.persistence.PersistenceConstants;
-import org.cbioportal.persistence.mybatis.util.OffsetCalculator;
+import org.cbioportal.persistence.mybatis.util.PaginationCalculator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
+import org.springframework.util.Assert;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.util.*;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 @Repository
 public class ClinicalDataMyBatisRepository implements ClinicalDataRepository {
@@ -24,7 +26,7 @@ public class ClinicalDataMyBatisRepository implements ClinicalDataRepository {
     @Autowired
     private PatientRepository patientRepository;
     @Autowired
-    private OffsetCalculator offsetCalculator;
+    private ClinicalAttributeRepository clinicalAttributeRepository;
 
     @Override
     public List<ClinicalData> getAllClinicalDataOfSampleInStudy(String studyId, String sampleId,
@@ -34,7 +36,7 @@ public List<ClinicalData> getAllClinicalDataOfSampleInStudy(String studyId, Stri
 
         return clinicalDataMapper.getSampleClinicalData(Arrays.asList(studyId), Arrays.asList(sampleId),
             attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize, 
-            offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+            PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override
@@ -50,8 +52,8 @@ public List<ClinicalData> getAllClinicalDataOfPatientInStudy(String studyId, Str
                                                                  String sortBy, String direction) {
 
         return clinicalDataMapper.getPatientClinicalData(Arrays.asList(studyId), Arrays.asList(patientId),
-            attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize, 
-            offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+            attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize,
+            PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override
@@ -68,12 +70,12 @@ public List<ClinicalData> getAllClinicalDataInStudy(String studyId, String attri
 
         if (clinicalDataType.equals(PersistenceConstants.SAMPLE_CLINICAL_DATA_TYPE)) {
             return clinicalDataMapper.getSampleClinicalData(Arrays.asList(studyId), null, 
-                attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize, 
-                offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+                attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize,
+                PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
         } else {
             return clinicalDataMapper.getPatientClinicalData(Arrays.asList(studyId), null, 
-                attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize, 
-                offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+                attributeId != null ? Arrays.asList(attributeId) : null, projection, pageSize,
+                PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
         }
     }
 
@@ -137,23 +139,35 @@ public List<ClinicalData> fetchClinicalData(List<String> studyIds, List<String>
         }
     }
 
-    @Override
-    public List<ClinicalData> fetchSampleClinicalTable(List<String> studyIds, List<String> ids,
-                                                       Integer pageSize, Integer pageNumber, String searchTerm,
-                                                       String sortBy, String direction) {
-        if (ids.isEmpty()) {
+    public List<Integer> getVisibleSampleInternalIdsForClinicalTable(List<String> studyIds, List<String> sampleIds,
+                                                                       Integer pageSize, Integer pageNumber, String searchTerm,
+                                                                       String sortAttrId, String direction) {
+        if (sampleIds.isEmpty()) {
             return new ArrayList<>();
         }
-        int offset = offsetCalculator.calculate(pageSize, pageNumber);
-        return clinicalDataMapper.getSampleClinicalTable(studyIds, ids,"SUMMARY", pageSize,
-            offset, searchTerm, sortBy, direction);
+        Integer offset = PaginationCalculator.offset(pageSize, pageNumber);
+        Boolean sortAttrIsNumber = null;
+        Boolean sortIsPatientAttr = null;
+        if (sortAttrId != null && ! sortAttrId.isEmpty()) {
+            ClinicalAttribute clinicalAttributeMeta = getClinicalAttributeMeta(studyIds, sortAttrId);
+            sortAttrIsNumber = clinicalAttributeMeta.getDatatype().equals("NUMBER");
+            sortIsPatientAttr = clinicalAttributeMeta.getPatientAttribute();
+        }
+        return clinicalDataMapper.getVisibleSampleInternalIdsForClinicalTable(studyIds, sampleIds,"SUMMARY", pageSize, 
+            offset, searchTerm, sortAttrId, sortAttrIsNumber, sortIsPatientAttr, direction);
     }
-
-    @Override
-    public Integer fetchSampleClinicalTableCount(List<String> studyIds, List<String> sampleIds,
-                                                 String searchTerm, String sortBy, String direction) {
-        return clinicalDataMapper.getSampleClinicalTableCount(studyIds, sampleIds,"SUMMARY", 
-            searchTerm, sortBy, direction);
+
+    private ClinicalAttribute getClinicalAttributeMeta(List<String> studyIds, String attrId) {
+        Assert.notNull(studyIds, "Arguments may not be null");
+        Assert.notNull(attrId, "Arguments may not be null");
+        Stream<String> uniqueStudyIds = studyIds.stream().distinct();
+        Optional<ClinicalAttribute> clinicalAttributeMeta = uniqueStudyIds
+            .map(studyId -> clinicalAttributeRepository.getClinicalAttribute(studyId, attrId))
+            .filter(Objects::nonNull)
+            .findFirst();
+        Assert.isTrue(clinicalAttributeMeta.isPresent(), "Clinical Attribute " + attrId + 
+            " was not found in studies " + studyIds.stream().collect(Collectors.joining(", ")) );
+        return clinicalAttributeMeta.get();
     }
 
     @Override
@@ -195,4 +209,16 @@ public List<ClinicalData> getPatientClinicalDataDetailedToSample(List<String> st
         return clinicalDataMapper.getPatientClinicalDataDetailedToSample(studyIds, patientIds, attributeIds, "SUMMARY",
                 0, 0, null, null);
     }
+
+    @Override
+    public List<ClinicalData> getSampleClinicalDataBySampleInternalIds(List<Integer> sampleInternalIds) {
+        return sampleInternalIds == null || sampleInternalIds.isEmpty() ?
+            new ArrayList<>() : clinicalDataMapper.getSampleClinicalDataBySampleInternalIds(sampleInternalIds);
+    }
+
+    @Override
+    public List<ClinicalData> getPatientClinicalDataBySampleInternalIds(List<Integer> sampleInternalIds) {
+        return sampleInternalIds == null || sampleInternalIds.isEmpty() ?
+            new ArrayList<>() : clinicalDataMapper.getPatientClinicalDataBySampleInternalIds(sampleInternalIds);
+    }
 }

src/main/java/org/cbioportal/persistence/mybatis/ClinicalEventMyBatisRepository.java

@@ -4,7 +4,7 @@
 import org.cbioportal.model.ClinicalEventData;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.ClinicalEventRepository;
-import org.cbioportal.persistence.mybatis.util.OffsetCalculator;
+import org.cbioportal.persistence.mybatis.util.PaginationCalculator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
 
@@ -20,15 +20,13 @@ public class ClinicalEventMyBatisRepository implements ClinicalEventRepository {
 
     @Autowired
     private ClinicalEventMapper clinicalEventMapper;
-    @Autowired
-    private OffsetCalculator offsetCalculator;
 
     @Override
     public List<ClinicalEvent> getAllClinicalEventsOfPatientInStudy(String studyId, String patientId, String projection, 
                                                                     Integer pageSize, Integer pageNumber, String sortBy, 
                                                                     String direction) {
         return clinicalEventMapper.getPatientClinicalEvent(studyId, patientId, projection, pageSize, 
-            offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+            PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override
@@ -46,8 +44,8 @@ public List<ClinicalEventData> getDataOfClinicalEvents(List<Integer> clinicalEve
     @Override
     public List<ClinicalEvent> getAllClinicalEventsInStudy(String studyId, String projection, Integer pageSize,
                                                            Integer pageNumber, String sortBy, String direction) {
-        return clinicalEventMapper.getStudyClinicalEvent(studyId, projection, pageSize, 
-            offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+        return clinicalEventMapper.getStudyClinicalEvent(studyId, projection, pageSize,
+            PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override

src/main/java/org/cbioportal/persistence/mybatis/CopyNumberSegmentMyBatisRepository.java

@@ -3,7 +3,7 @@
 import org.cbioportal.model.CopyNumberSeg;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.CopyNumberSegmentRepository;
-import org.cbioportal.persistence.mybatis.util.OffsetCalculator;
+import org.cbioportal.persistence.mybatis.util.PaginationCalculator;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Repository;
 
@@ -15,8 +15,6 @@ public class CopyNumberSegmentMyBatisRepository implements CopyNumberSegmentRepo
 
     @Autowired
     private CopyNumberSegmentMapper copyNumberSegmentMapper;
-    @Autowired
-    private OffsetCalculator offsetCalculator;
 
 
     @Override
@@ -26,7 +24,7 @@ public List<CopyNumberSeg> getCopyNumberSegmentsInSampleInStudy(String studyId,
                                                                     String direction) {
 
         return copyNumberSegmentMapper.getCopyNumberSegments(Arrays.asList(studyId), Arrays.asList(sampleId), chromosome,
-            projection, pageSize, offsetCalculator.calculate(pageSize, pageNumber), sortBy, direction);
+            projection, pageSize, PaginationCalculator.offset(pageSize, pageNumber), sortBy, direction);
     }
 
     @Override