Refactor cell types endpoint (#353)

* Pull and update latest submodule changes

* Add organism part and cell type search to analytics search DAO

* Clean variable naming

* Add cell types as an input parameter to OrganismPartSearchDao

* Add cell types as an input parameter to OrganismPartSearchService

* clean code in JsonGeneSearchController

* remove unneeded JdbcUtils from CellTypeSearchDaoIT

* Add cell type query parameter to the JsonGeneSearchController - WIP

* Rename test method name and remove unneeded test case

* Add organism parts parameter to CellTypeSearchDao

* Implement CellTypeSearchService

* Use new CellTypeSearchService in JsonGeneSearchController

* Remove null value from cell type search results

app/src/main/java/uk/ac/ebi/atlas/search/JsonGeneSearchController.java

@@ -17,7 +17,7 @@
 import uk.ac.ebi.atlas.controllers.JsonExceptionHandlingController;
 import uk.ac.ebi.atlas.experimentpage.ExperimentAttributesService;
 import uk.ac.ebi.atlas.model.experiment.singlecell.SingleCellBaselineExperiment;
-import uk.ac.ebi.atlas.search.analytics.AnalyticsSearchService;
+import uk.ac.ebi.atlas.search.celltype.CellTypeSearchService;
 import uk.ac.ebi.atlas.search.geneids.GeneIdSearchService;
 import uk.ac.ebi.atlas.search.geneids.QueryParsingException;
 import uk.ac.ebi.atlas.search.organismpart.OrganismPartSearchService;
@@ -47,8 +47,8 @@ public class JsonGeneSearchController extends JsonExceptionHandlingController {
     private final ExperimentTrader experimentTrader;
     private final ExperimentAttributesService experimentAttributesService;
 
-    private final AnalyticsSearchService analyticsSearchService;
     private final OrganismPartSearchService organismPartSearchService;
+    private final CellTypeSearchService cellTypeSearchService;
     private final SpeciesSearchService speciesSearchService;
 
     @GetMapping(value = "/json/search", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)
@@ -267,7 +267,10 @@ public Set<String> getCellTypeBySearchTerm(@RequestParam MultiValueMap<String, S
             return ImmutableSet.of();
         }
 
-        return analyticsSearchService.searchCellType(geneIds.get());
+        var organismParts = requestParams.get("organismParts");
+
+        return cellTypeSearchService.search(geneIds.get(),
+                organismParts != null ? ImmutableSet.copyOf(organismParts) : ImmutableSet.of());
     }
 
     @GetMapping(value = "/json/gene-search/species", produces = MediaType.APPLICATION_JSON_UTF8_VALUE)

app/src/main/java/uk/ac/ebi/atlas/search/celltype/CellTypeSearchDao.java

@@ -19,6 +19,8 @@
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CELL_ID;
+import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CTW_CELL_TYPE;
+import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CTW_ORGANISM_PART;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.EXPERIMENT_ACCESSION;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.FACET_CHARACTERISTIC_NAME;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.FACET_CHARACTERISTIC_VALUE;
@@ -37,9 +39,9 @@ public class CellTypeSearchDao {
 
     private final SingleCellAnalyticsCollectionProxy singleCellAnalyticsCollectionProxy;
 
-    public CellTypeSearchDao(SolrCloudCollectionProxyFactory solrCloudCollectionProxyFactory) {
+    public CellTypeSearchDao(SolrCloudCollectionProxyFactory collectionProxyFactory) {
         this.singleCellAnalyticsCollectionProxy =
-                solrCloudCollectionProxyFactory.create(SingleCellAnalyticsCollectionProxy.class);
+                collectionProxyFactory.create(SingleCellAnalyticsCollectionProxy.class);
     }
 
     @Cacheable(cacheNames = "inferredCellTypesOntology", key = "{#experimentAccession, #organOrOrganismPart}")
@@ -100,7 +102,7 @@ public ImmutableSet<String> getInferredCellTypeAuthorsLabels(String experimentAc
      */
     private ImmutableSet<String> getCellTypeMetadata(String experimentAccession,
                                                      ImmutableSet<String> organOrOrganismPart,
-                                                     String cellTypeValue) {
+                                                     String cellTypeFacetName) {
         var cellIdsInOrganOrOrganismPartQueryBuilder =
                 new SolrQueryBuilder<SingleCellAnalyticsCollectionProxy>()
                         .addQueryFieldByTerm(EXPERIMENT_ACCESSION, experimentAccession)
@@ -124,8 +126,8 @@ private ImmutableSet<String> getCellTypeMetadata(String experimentAccession,
                         .addQueryFieldByTerm(EXPERIMENT_ACCESSION, experimentAccession)
                         .addQueryFieldByTerm(
                                 ImmutableMap.of(
-                                        FACET_FACTOR_NAME, ImmutableSet.of(cellTypeValue),
-                                        FACET_CHARACTERISTIC_NAME, ImmutableSet.of(cellTypeValue)))
+                                        FACET_FACTOR_NAME, ImmutableSet.of(cellTypeFacetName),
+                                        FACET_CHARACTERISTIC_NAME, ImmutableSet.of(cellTypeFacetName)))
                         .setFieldList(ImmutableSet.of(CELL_ID, FACET_FACTOR_VALUE, FACET_CHARACTERISTIC_VALUE))
                         .sortBy(CELL_ID, SolrQuery.ORDER.asc);
         var uniqueCellIdsAnnotatedWithCellTypeValue =
@@ -162,4 +164,45 @@ private ImmutableSet<String> getCellTypeMetadata(String experimentAccession,
                     .collect(toImmutableSet());
         }
     }
+
+    public ImmutableSet<String> searchCellTypes(ImmutableSet<String> cellIds, ImmutableSet<String> organismParts) {
+//        Streaming query for getting the cell types provided by set of cell IDs and organism parts
+//        unique(
+//            search(scxa-analytics-v6, q=cell_id:<SET_OF_CELL_IDS> AND ctw_organism_part:<SET_OF_ORGANISM_PART>,
+//            fl="ctw_cell_type",
+//            sort="ctw_cell_type asc"
+//            ),
+//            over="ctw_cell_type"
+//        )
+        return getCellTypeFromStreamQuery(
+                new UniqueStreamBuilder(getStreamBuilderForCellTypeByCellIdsAndOrganismParts(
+                        cellIds, organismParts), CTW_CELL_TYPE.name()));
+    }
+
+    private SearchStreamBuilder<SingleCellAnalyticsCollectionProxy> getStreamBuilderForCellTypeByCellIdsAndOrganismParts(
+            ImmutableSet<String> cellIDs, ImmutableSet<String> organismParts) {
+        var cellTypeQueryBuilder = new SolrQueryBuilder<SingleCellAnalyticsCollectionProxy>()
+                .addQueryFieldByTerm(CELL_ID, cellIDs)
+                .setFieldList(CTW_CELL_TYPE)
+                .sortBy(CTW_CELL_TYPE, SolrQuery.ORDER.asc);
+
+        if (organismParts != null && !organismParts.isEmpty()) {
+            cellTypeQueryBuilder.addQueryFieldByTerm(CTW_ORGANISM_PART, organismParts);
+        }
+
+        return new SearchStreamBuilder<>(
+                singleCellAnalyticsCollectionProxy,
+                cellTypeQueryBuilder
+        ).returnAllDocs();
+    }
+
+    private ImmutableSet<String> getCellTypeFromStreamQuery(UniqueStreamBuilder uniqueCellTypeStreamBuilder) {
+        try (TupleStreamer tupleStreamer = TupleStreamer.of(uniqueCellTypeStreamBuilder.build())) {
+            return tupleStreamer.get()
+                    .filter(tuple -> !tuple.getFields().isEmpty())
+                    .map(tuple -> tuple.getString(CTW_CELL_TYPE.name()))
+                    .collect(toImmutableSet()
+                    );
+        }
+    }
 }

app/src/main/java/uk/ac/ebi/atlas/search/celltype/CellTypeSearchService.java

@@ -0,0 +1,29 @@
+package uk.ac.ebi.atlas.search.celltype;
+
+import com.google.common.collect.ImmutableSet;
+import lombok.RequiredArgsConstructor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.stereotype.Component;
+import uk.ac.ebi.atlas.search.GeneSearchService;
+
+@Component
+@RequiredArgsConstructor
+public class CellTypeSearchService {
+
+    private final CellTypeSearchDao cellTypeSearchDao;
+    private final GeneSearchService geneSearchService;
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(CellTypeSearchService.class);
+
+    public ImmutableSet<String> search(ImmutableSet<String> geneIds, ImmutableSet<String> organismParts) {
+        if (geneIds.isEmpty()) {
+            LOGGER.warn("Can't query for organism part as no gene IDs has given.");
+            return ImmutableSet.of();
+        }
+
+        LOGGER.info("Searching organism parts for this gene ids: {}", geneIds.asList());
+
+        return cellTypeSearchDao.searchCellTypes(geneSearchService.getCellIdsFromGeneIds(geneIds), organismParts);
+    }
+}

app/src/main/java/uk/ac/ebi/atlas/search/organismpart/OrganismPartSearchDao.java

@@ -25,10 +25,10 @@ public OrganismPartSearchDao(SolrCloudCollectionProxyFactory collectionProxyFact
                 collectionProxyFactory.create(SingleCellAnalyticsCollectionProxy.class);
     }
 
-    public ImmutableSet<String> searchOrganismPart(ImmutableSet<String> cellIDs, ImmutableSet<String> cellTypes) {
-//        Streaming query for getting the organism_part provided by set of cell IDs
+        public ImmutableSet<String> searchOrganismPart(ImmutableSet<String> cellIDs, ImmutableSet<String> cellTypes) {
+//        Streaming query for getting the organism_part provided by set of cell IDs and cell types
 //        unique(
-//            search(scxa-analytics-v6, q=cell_id:<SET_OF_CELL_IDS> AND cell_type:<SET_OF_CELL_TYPES>,
+//            search(scxa-analytics-v6, q=cell_id:<SET_OF_CELL_IDS> AND ctw_cell_type:<SET_OF_CELL_TYPES>,
 //            fl="ctw_organism_part",
 //            sort="ctw_organism_part asc"
 //            ),

app/src/main/java/uk/ac/ebi/atlas/solr/SingleCellSolrUtils.java

@@ -13,6 +13,7 @@
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CELL_ID;
 import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CTW_CELL_TYPE;
+import static uk.ac.ebi.atlas.solr.cloud.collections.SingleCellAnalyticsCollectionProxy.CTW_ORGANISM_PART;
 
 @Component
 public class SingleCellSolrUtils {
@@ -33,12 +34,32 @@ public ImmutableSet<String> fetchedRandomCellTypesByCellIDs(ImmutableSet<String>
                 .setFieldList(CTW_CELL_TYPE)
                 .setRows(MAX_ROWS);
 
-        return getRandomCellTypesFromQueryResult(singleCellAnalyticsCollectionProxy.query(queryBuilder).getResults(), numberOfCellTypes);
+        return getRandomCellTypesFromQueryResult(
+                singleCellAnalyticsCollectionProxy.query(queryBuilder).getResults(),
+                CTW_CELL_TYPE.name(),
+                numberOfCellTypes);
     }
 
-    private ImmutableSet<String> getRandomCellTypesFromQueryResult(SolrDocumentList solrDocumentList, int numberOfCellTypes) {
+    public ImmutableSet<String> fetchedRandomOrganismPartsByCellIDs(ImmutableSet<String> cellIDs, int numberOfOrganismParts) {
+        SolrQueryBuilder<SingleCellAnalyticsCollectionProxy> queryBuilder = new SolrQueryBuilder<>();
+        queryBuilder
+                .addQueryFieldByTerm(CELL_ID, cellIDs)
+                .setFieldList(CTW_ORGANISM_PART)
+                .setRows(MAX_ROWS);
+
+        return getRandomCellTypesFromQueryResult(
+                singleCellAnalyticsCollectionProxy.query(queryBuilder).getResults(),
+                CTW_ORGANISM_PART.name(),
+                numberOfOrganismParts);
+    }
+
+
+    private ImmutableSet<String> getRandomCellTypesFromQueryResult(
+            SolrDocumentList solrDocumentList,
+            String schemaFieldName,
+            int numberOfCellTypes) {
         return Arrays.stream(new Random().ints(numberOfCellTypes, 0, solrDocumentList.size()).toArray())
-                .mapToObj(index -> solrDocumentList.get(index).getFieldValue(CTW_CELL_TYPE.name()).toString())
+                .mapToObj(index -> solrDocumentList.get(index).getFieldValue(schemaFieldName).toString())
                 .collect(toImmutableSet());
     }
 }

app/src/test/java/uk/ac/ebi/atlas/search/JsonGeneSearchControllerIT.java

@@ -14,7 +14,7 @@
 import org.springframework.util.LinkedMultiValueMap;
 import uk.ac.ebi.atlas.configuration.TestConfig;
 import uk.ac.ebi.atlas.experimentpage.ExperimentAttributesService;
-import uk.ac.ebi.atlas.search.analytics.AnalyticsSearchService;
+import uk.ac.ebi.atlas.search.celltype.CellTypeSearchService;
 import uk.ac.ebi.atlas.search.geneids.GeneIdSearchService;
 import uk.ac.ebi.atlas.search.geneids.GeneQuery;
 import uk.ac.ebi.atlas.search.geneids.QueryParsingException;
@@ -45,12 +45,11 @@ class JsonGeneSearchControllerIT {
     @Mock
     private GeneSearchService geneSearchServiceMock;
 
-    @Mock
-    private AnalyticsSearchService analyticsSearchServiceMock;
-
     @Mock
     private OrganismPartSearchService organismPartSearchServiceMock;
 
+    @Mock
+    private CellTypeSearchService cellTypeSearchService;
 
     @Inject
     private ExperimentTrader experimentTrader;
@@ -71,8 +70,8 @@ void setUp() {
                         geneSearchServiceMock,
                         experimentTrader,
                         experimentAttributesService,
-                        analyticsSearchServiceMock,
                         organismPartSearchServiceMock,
+                        cellTypeSearchService,
                         speciesSearchService);
     }
 
@@ -254,7 +253,7 @@ void whenSearchTermIsFoundAndThereAreRelatedCellIdsThenReturnsOrganismParts() {
     }
 
     @Test
-    void whenRequestParamIsEmptyCellTypeSearchReturnsEmptySet() {
+    void whenRequestParamIsEmptyCellTypeSearchReturnsException() {
         var requestParams = new LinkedMultiValueMap<String, String>();
 
         when(geneIdSearchServiceMock.getGeneQueryByRequestParams(requestParams))
@@ -277,7 +276,7 @@ void whenSearchTermIsNotFoundAnyGeneIdsCellTypeSearchReturnsEmptySet() {
                 .thenReturn(geneQuery);
         when(geneIdSearchServiceMock.search(geneQuery))
                 .thenReturn(Optional.of(ImmutableSet.of()));
-        when(analyticsSearchServiceMock.searchCellType(ImmutableSet.of()))
+        when(cellTypeSearchService.search(ImmutableSet.of(), ImmutableSet.of()))
                 .thenReturn(ImmutableSet.of());
 
         var emptyCellTypeSet = subject.getCellTypeBySearchTerm(requestParams);
@@ -299,7 +298,7 @@ void whenSearchTermIsFoundButNoRelatedCellIdsThenCellTypeSearchReturnsEmptySet()
                 .thenReturn(geneQuery);
         when(geneIdSearchServiceMock.search(geneQuery))
                 .thenReturn(Optional.of(geneIdsFromService));
-        when(analyticsSearchServiceMock.searchCellType(geneIdsFromService))
+        when(cellTypeSearchService.search(geneIdsFromService, ImmutableSet.of()))
                 .thenReturn(ImmutableSet.of());
 
         var emptyCellTypeSet = subject.getCellTypeBySearchTerm(requestParams);
@@ -322,7 +321,7 @@ void whenSearchTermIsFoundAndThereAreRelatedCellIdsThenReturnsCellTypes() {
                 .thenReturn(geneQuery);
         when(geneIdSearchServiceMock.search(geneQuery))
                 .thenReturn(Optional.of(geneIdsFromService));
-        when(analyticsSearchServiceMock.searchCellType(geneIdsFromService))
+        when(cellTypeSearchService.search(geneIdsFromService, ImmutableSet.of()))
                 .thenReturn(ImmutableSet.of(expectedCellType));
 
         var actualCellTypes = subject.getCellTypeBySearchTerm(requestParams);

app/src/test/java/uk/ac/ebi/atlas/search/JsonGeneSearchControllerWIT.java

@@ -1,6 +1,7 @@
 package uk.ac.ebi.atlas.search;
 
 import com.google.common.collect.ImmutableSet;
+import org.hamcrest.core.IsNull;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -29,6 +30,7 @@
 import javax.inject.Inject;
 import javax.sql.DataSource;
 
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
@@ -346,4 +348,14 @@ void whenSearchTermExistsInDBThenReturnsSetOfCellType() throws Exception {
                 .andExpect(jsonPath("$", hasSize(equalTo(expectedCellTypes.size()))))
                 .andExpect(jsonPath("$", containsInAnyOrder(expectedCellTypes.toArray())));
     }
+
+    @Test
+    void whenGivenExistingSymbolReturnedCellTypesNotContainsNullValue() throws Exception {
+        var symbolValue = "CFTR";
+
+        this.mockMvc.perform(get("/json/gene-search/cell-types").param("symbol", symbolValue))
+                .andExpect(status().isOk())
+                .andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8))
+                .andExpect(jsonPath("$", not(contains(IsNull.nullValue()))));
+    }
 }