PR review change

model/pom.xml

@@ -25,6 +25,10 @@
             <groupId>org.hibernate</groupId>
             <artifactId>hibernate-validator</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-lang3</artifactId>
+        </dependency>
     </dependencies>
 
 </project>

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

@@ -0,0 +1,24 @@
+package org.cbioportal.model;
+
+import org.apache.commons.lang3.tuple.ImmutablePair;
+
+public class ClinicalDataTableResult {
+
+    private final ImmutablePair<SampleClinicalDataCollection, Integer> pair;
+
+    public ClinicalDataTableResult(SampleClinicalDataCollection pagedCollectionFragment, Integer totalCount) {
+        this.pair = new ImmutablePair<>(pagedCollectionFragment, totalCount);
+    }
+
+    public ClinicalDataTableResult() {
+        this.pair =  new ImmutablePair<>(new SampleClinicalDataCollection(), 0);
+    }
+
+    public SampleClinicalDataCollection getSampleClinicalDataCollection() {
+        return pair.getLeft();
+    }
+    public Integer getTotalCount() {
+        return pair.getRight();
+    }
+
+}

persistence/persistence-mybatis/src/test/java/org/cbioportal/persistence/mybatis/util/PaginationCalculatorTest.java

@@ -46,4 +46,5 @@ public void testLastIndex() {
         Assert.assertEquals(6, (int) paginationCalculator.lastIndex(3, 3, 26));
         Assert.assertEquals(26, (int) paginationCalculator.lastIndex(25, 3, 26));
     }
+
 }

service/src/main/java/org/cbioportal/service/ClinicalDataService.java

@@ -1,9 +1,8 @@
 package org.cbioportal.service;
 
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.cbioportal.model.ClinicalData;
 import org.cbioportal.model.ClinicalDataCountItem;
-import org.cbioportal.model.SampleClinicalDataCollection;
+import org.cbioportal.model.ClinicalDataTableResult;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.service.exception.PatientNotFoundException;
 import org.cbioportal.service.exception.SampleNotFoundException;
@@ -54,7 +53,7 @@ BaseMeta fetchMetaClinicalData(List<String> studyIds, List<String> ids, List<Str
     List<ClinicalData> getPatientClinicalDataDetailedToSample(List<String> studyIds, List<String> patientIds,
             List<String> attributeIds);
 
-    ImmutablePair<SampleClinicalDataCollection, Integer> fetchSampleClinicalTable(List<String> studyIds, List<String> sampleIds, Integer pageSize,
-                                                                                  Integer pageNumber, String searchTerm, String sortBy, String direction);
+    ClinicalDataTableResult fetchSampleClinicalTable(List<String> studyIds, List<String> sampleIds, Integer pageSize,
+                                                     Integer pageNumber, String searchTerm, String sortBy, String direction);
 
 }

service/src/main/java/org/cbioportal/service/impl/ClinicalDataServiceImpl.java

@@ -1,6 +1,5 @@
 package org.cbioportal.service.impl;
 
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.cbioportal.model.*;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.ClinicalDataRepository;
@@ -234,11 +233,11 @@ public List<ClinicalData> getPatientClinicalDataDetailedToSample(List<String> st
     }
 
     @Override
-    public ImmutablePair<SampleClinicalDataCollection, Integer> fetchSampleClinicalTable(List<String> studyIds, List<String> sampleIds, Integer pageSize, Integer pageNumber, String searchTerm, String sortBy, String direction) {
+    public ClinicalDataTableResult fetchSampleClinicalTable(List<String> studyIds, List<String> sampleIds, Integer pageSize, Integer pageNumber, String searchTerm, String sortBy, String direction) {
 
         SampleClinicalDataCollection sampleClinicalDataCollection = new SampleClinicalDataCollection();
         if (studyIds == null || studyIds.isEmpty() || sampleIds == null || sampleIds.isEmpty()) {
-            return new ImmutablePair<>(sampleClinicalDataCollection, 0);
+            return new ClinicalDataTableResult();
         }
 
         // Request un-paginated data.
@@ -250,7 +249,7 @@ public ImmutablePair<SampleClinicalDataCollection, Integer> fetchSampleClinicalT
         Integer offset = paginationCalculator.offset(pageSize, pageNumber);
 
         if (allSampleInternalIds.isEmpty() || offset >= allSampleInternalIds.size()) {
-            return new ImmutablePair<>(sampleClinicalDataCollection, 0);
+            return new ClinicalDataTableResult();
         }
 
         // Apply pagination to the sampleId list.
@@ -271,7 +270,7 @@ public ImmutablePair<SampleClinicalDataCollection, Integer> fetchSampleClinicalT
                     calculateBase64(clinicalDatum.getSampleId(), clinicalDatum.getStudyId())
         )));
 
-        return new ImmutablePair<>(sampleClinicalDataCollection, allSampleInternalIds.size());
+        return new ClinicalDataTableResult(sampleClinicalDataCollection, allSampleInternalIds.size());
     }
 
 }

service/src/test/java/org/cbioportal/service/impl/ClinicalDataServiceImplTest.java

@@ -1,6 +1,5 @@
 package org.cbioportal.service.impl;
 
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.cbioportal.model.*;
 import org.cbioportal.model.meta.BaseMeta;
 import org.cbioportal.persistence.ClinicalDataRepository;
@@ -15,14 +14,11 @@
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
-import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.junit.MockitoJUnitRunner;
-import org.springframework.test.context.ContextConfiguration;
 
 import java.util.*;
 
-import static org.mockito.ArgumentMatchers.anyList;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -392,12 +388,12 @@ public void fetchSampleClinicalTableHappyCase() {
         when(clinicalDataRepository.getPatientClinicalDataBySampleInternalIds(sampleInternalIds)).thenReturn(
             List.of(datum1, datum2)
         );
-
-        ImmutablePair<SampleClinicalDataCollection, Integer> result = clinicalDataService.fetchSampleClinicalTable(
+        
+        ClinicalDataTableResult result = clinicalDataService.fetchSampleClinicalTable(
             sampleStudyIds, sampleIds, pageSize, pageNumber, searchTerm, sortBy, direction
         );
-        SampleClinicalDataCollection clinicalDataCollection = result.getLeft();
-        Integer itemCount = result.getRight();
+        SampleClinicalDataCollection clinicalDataCollection = result.getSampleClinicalDataCollection();
+        Integer itemCount = result.getTotalCount();
 
         Assert.assertEquals(4, (int) itemCount);
         Assert.assertEquals(2, clinicalDataCollection.getByUniqueSampleKey().size());
@@ -415,16 +411,16 @@ public void fetchSampleClinicalTableHappyCase() {
     public void fetchSampleClinicalTableEmptyIdLists() {
         Assert.assertEquals(0, clinicalDataService.fetchSampleClinicalTable(
             null, sampleIds, pageSize, pageNumber, searchTerm, sortBy, direction
-        ).getLeft().getByUniqueSampleKey().size());
+        ).getSampleClinicalDataCollection().getByUniqueSampleKey().size());
         Assert.assertEquals(0, clinicalDataService.fetchSampleClinicalTable(
             sampleStudyIds, null, pageSize, pageNumber, searchTerm, sortBy, direction
-        ).getLeft().getByUniqueSampleKey().size());
+        ).getSampleClinicalDataCollection().getByUniqueSampleKey().size());
         Assert.assertEquals(0, clinicalDataService.fetchSampleClinicalTable(
             new ArrayList<>(), sampleIds, pageSize, pageNumber, searchTerm, sortBy, direction
-        ).getLeft().getByUniqueSampleKey().size());
+        ).getSampleClinicalDataCollection().getByUniqueSampleKey().size());
         Assert.assertEquals(0, clinicalDataService.fetchSampleClinicalTable(
             sampleStudyIds, new ArrayList<>(), pageSize, pageNumber, searchTerm, sortBy, direction
-        ).getLeft().getByUniqueSampleKey().size());
+        ).getSampleClinicalDataCollection().getByUniqueSampleKey().size());
     }
 
 }

web/src/main/java/org/cbioportal/web/StudyViewController.java

@@ -6,7 +6,6 @@
 
 import org.apache.commons.collections4.CollectionUtils;
 import org.apache.commons.lang3.math.NumberUtils;
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.math3.stat.correlation.PearsonsCorrelation;
 import org.apache.commons.math3.stat.correlation.SpearmansCorrelation;
 import org.apache.commons.math3.util.Pair;
@@ -17,7 +16,6 @@
 import org.cbioportal.web.config.annotation.InternalApi;
 import org.cbioportal.model.AlterationFilter;
 import org.cbioportal.web.parameter.*;
-import org.cbioportal.web.parameter.sort.ClinicalDataSortBy;
 import org.cbioportal.web.util.*;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.cache.annotation.Cacheable;
@@ -991,13 +989,13 @@ public ResponseEntity<SampleClinicalDataCollection> fetchClinicalDataClinicalTab
     ) {
 
         boolean singleStudyUnfiltered = studyViewFilterUtil.isSingleStudyUnfiltered(interceptedStudyViewFilter);
-        ImmutablePair<SampleClinicalDataCollection, Integer> sampleClinicalData = cachedClinicalDataTableData(
+        ClinicalDataTableResult sampleClinicalData = cachedClinicalDataTableData(
             interceptedStudyViewFilter, singleStudyUnfiltered, pageNumber, pageSize, sortBy, searchTerm, direction.name()
         );
 
         // Because of pagination, the total number of sample matches can be larger than the items in the requested page.
-        SampleClinicalDataCollection aggregatedClinicalDataByUniqueSampleKey = sampleClinicalData.getLeft();
-        Integer totalNumberOfResults = sampleClinicalData.getRight();
+        SampleClinicalDataCollection aggregatedClinicalDataByUniqueSampleKey = sampleClinicalData.getSampleClinicalDataCollection();
+        Integer totalNumberOfResults = sampleClinicalData.getTotalCount();
 
         HttpHeaders responseHeaders = new HttpHeaders();
         responseHeaders.add(HeaderKeyConstants.TOTAL_COUNT, String.valueOf(totalNumberOfResults));
@@ -1012,7 +1010,7 @@ public ResponseEntity<SampleClinicalDataCollection> fetchClinicalDataClinicalTab
         cacheResolver = "staticRepositoryCacheOneResolver",
         condition = "@cacheEnabledConfig.getEnabled() && #singleStudyUnfiltered && (#sortBy == null || #sortBy.isEmpty()) && (#searchTerm == null || #searchTerm.isEmpty()) && #pageNumber == 0"
     )
-    public ImmutablePair<SampleClinicalDataCollection, Integer> cachedClinicalDataTableData(
+    public ClinicalDataTableResult cachedClinicalDataTableData(
         StudyViewFilter interceptedStudyViewFilter, boolean singleStudyUnfiltered, Integer pageNumber, 
         Integer pageSize, String sortBy, String searchTerm, String sortDirection
     ) {

web/src/test/java/org/cbioportal/web/StudyViewControllerTest.java

@@ -1,7 +1,6 @@
 package org.cbioportal.web;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.cbioportal.model.*;
 import org.cbioportal.model.util.Select;
 import org.cbioportal.persistence.AlterationRepository;
@@ -887,7 +886,7 @@ public void fetchClinicalDataClinicalTable() throws Exception {
         // For this sake of this test the sample clinical data and patient clinical data are identical.
         when(clinicalDataService.fetchSampleClinicalTable(anyList(), anyList(),
             anyInt(), anyInt(), anyString(), any(), anyString())).thenReturn(
-                new ImmutablePair<>(tableClinicalData, 100)
+                new ClinicalDataTableResult(tableClinicalData, 100)
             );
 
         StudyViewFilter studyViewFilter = new StudyViewFilter();