Improve hosted cellxgene (#1234)

* Improve hosted cellxgene

 - option to turn off the test index page, or supply a page for redirect.
   For EB, The default is to return 404.  For cli launch, the default is the test page.

 - option to select which matrix types are allowed for multi dataset servers.
   For EB, The default is CXG only.  For cli launch, the default is any matrix type.

 - Return early with an error response if diffexp is requested when not configured

 - Verified that reembedings and user annotations also return with an error response
   if used when not enabled.

TODO:  The new options cannot currently be set by the user.
I plan to add a configuration file where these and all other settings can be set.

 Fixes #1210 
 Fixes #1228  
 Fixes #1229

server/app/app.py

@@ -1,7 +1,7 @@
 import os
 import datetime
 
-from flask import Flask, redirect, current_app, make_response, render_template
+from flask import Flask, redirect, current_app, make_response, render_template, abort
 from flask import Blueprint, request, send_from_directory
 from flask_caching import Cache
 from flask_compress import Compress
@@ -14,7 +14,7 @@ import server.common.rest as common_rest
 from server.common.errors import DatasetAccessError
 from server.common.utils import path_join, Float32JSONEncoder
 from server.common.data_locator import DataLocator
-from server.data_common.matrix_loader import MatrixDataLoader, MatrixDataType
+from server.data_common.matrix_loader import MatrixDataLoader
 
 from functools import wraps
 
@@ -85,12 +85,8 @@ def static_redirect(dataset, therest):
     return redirect(f"/static/{therest}", code=301)
 
 
-def dataroot_index():
+def dataroot_test_index():
-    # FIXME with a splash screen that includes a listing of all the datasets.
+    # the following index page is meant for testing/debugging purposes
-    # or perhaps a login screen if this is a hosted environment,
-    # or have a configuration option to redirect to a user specified page.
-
-    # the following is just for demo purposes...
     data = '<!doctype html><html lang="en">'
     data += '<head><title>Hosted Cellxgene</title></head>'
     data += '<body><H1>Welcome to cellxgene</H1>'
@@ -101,9 +97,12 @@ def dataroot_index():
         datasets = []
         for fname in locator.ls():
             location = path_join(config.dataroot, fname)
-            matrix_data_loader = MatrixDataLoader(location)
+            try:
-            if matrix_data_loader.etype != MatrixDataType.UNKNOWN:
+                MatrixDataLoader(location, app_config=config)
                 datasets.append(fname)
+            except DatasetAccessError:
+                # skip over invalid datasets
+                pass
 
         data += '<br/>Select one of these datasets...<br/>'
         data += '<ul>'
@@ -118,6 +117,17 @@ def dataroot_index():
     return make_response(data)
 
 
+def dataroot_index():
+    # Handle the base url for the cellxgene server when running in multi dataset mode
+    config = current_app.app_config
+    if not config.multi_dataset_index:
+        abort(404)
+    elif config.multi_dataset_index is True:
+        return dataroot_test_index()
+    else:
+        return redirect(config.dataroot_index)
+
+
 class SchemaAPI(Resource):
     @rest_get_data_adaptor
     def get(self, data_adaptor):

server/common/app_config.py

@@ -35,6 +35,15 @@ class AppConfig(object):
         self.enable_reembedding = False
         self.anndata_backed = False
 
+        # The index page when in multi-dataset mode:
+        #   False or None:  this returns a 404 code
+        #   True:  loads a test index page, which links to the datasets that are available in the dataroot
+        #   string/URL:  redirect to this URL:  flask.redirect(config.multi_dataset_index)
+        self.multi_dataset_index = False
+
+        # A list of allowed matrix types.  If an empty list, then all matrix types are allowed
+        self.multi_dataset_allowed_matrix_type = []
+
         # TODO these options may not apply to all datasets in the multi dataset.
         # may need to invent a way to associate these config parameters with
         # specific datasets.
@@ -58,12 +67,13 @@ class AppConfig(object):
             "anndata_backed",
             "disable_diffexp",
             "enable_reembedding",
+            "multi_dataset_index",
+            "multi_dataset_allowed_matrix_type",
         ]
 
         self.update(inputs, kw)
 
     def update(self, inputs, kw):
-
         for k, v in kw.items():
             if k in inputs:
                 setattr(self, k, v)

server/common/rest.py

@@ -68,11 +68,12 @@ def annotations_put_fbs_helper(data_adaptor, annotations, fbs):
 
 
 def annotations_obs_put(request, data_adaptor, annotations):
-    anno_collection = request.args.get("annotation-collection-name", default=None)
-    fbs = request.get_data()
     if annotations is None:
         return make_response("Error, annotations are not configured", HTTPStatus.BAD_REQUEST)
 
+    anno_collection = request.args.get("annotation-collection-name", default=None)
+    fbs = request.get_data()
+
     if anno_collection is not None:
         if not annotations.is_safe_collection_name(anno_collection):
             return make_response(f"Error, bad annotation collection name", HTTPStatus.BAD_REQUEST)
@@ -130,11 +131,14 @@ def data_var_put(request, data_adaptor):
 
 
 def diffexp_obs_post(request, data_adaptor):
+    if data_adaptor.config.disable_diffexp:
+        return make_response(f"diffexp not supported.", HTTPStatus.BAD_REQUEST)
+
     args = request.get_json()
     # confirm mode is present and legal
     try:
         mode = DiffExpMode(args["mode"])
-    except KeyError:
+    except (KeyError, TypeError):
         return make_response("Error: mode is required", HTTPStatus.BAD_REQUEST)
     except ValueError:
         return make_response(f"Error: invalid mode option {args['mode']}", HTTPStatus.BAD_REQUEST)

server/data_common/matrix_loader.py

@@ -119,7 +119,7 @@ class MatrixDataCacheManager(object):
                     del self.datasets[oldest_key]
 
                 last_accessed = time.time()
-                loader = MatrixDataLoader(location)
+                loader = MatrixDataLoader(location, app_config=app_config)
                 cache_item = MatrixDataCacheItem(loader)
                 self.datasets[location] = (cache_item, last_accessed)
         try:
@@ -136,24 +136,31 @@ class MatrixDataType(Enum):
 
 
 class MatrixDataLoader(object):
-    def __init__(self, location, etype=None):
+    def __init__(self, location, matrix_data_type=None, app_config=None):
         """ location can be a string or DataLocator """
         self.location = DataLocator(location)
-        if etype is None:
+        # matrix_data_type is an enum value of type MatrixDataType
-            self.etype = self.matrix_data_type()
+        self.matrix_data_type = matrix_data_type
-        else:
+        # matrix_type is a DataAdaptor type, which corresonds to the matrix_data_type
-            self.etype = etype
         self.matrix_type = None
-        if self.etype == MatrixDataType.H5AD:
+
+        if matrix_data_type is None:
+            self.matrix_data_type = self.__matrix_data_type()
+
+        if not self.__matrix_data_type_allowed(app_config):
+            raise DatasetAccessError(
+                f"{self.location} does not have an allowed type: {str(self.matrix_data_type)}")
+
+        if self.matrix_data_type == MatrixDataType.H5AD:
             from server.data_anndata.anndata_adaptor import AnndataAdaptor
 
             self.matrix_type = AnndataAdaptor
-        elif self.etype == MatrixDataType.CXG:
+        elif self.matrix_data_type == MatrixDataType.CXG:
             from server.data_cxg.cxg_adaptor import CxgAdaptor
 
             self.matrix_type = CxgAdaptor
 
-    def matrix_data_type(self):
+    def __matrix_data_type(self):
         if self.location.path.endswith(".h5ad"):
             return MatrixDataType.H5AD
         elif ".cxg" in self.location.path:
@@ -161,8 +168,31 @@ class MatrixDataLoader(object):
         else:
             return MatrixDataType.UNKNOWN
 
+    def __matrix_data_type_allowed(self, app_config):
+        if self.matrix_data_type == MatrixDataType.UNKNOWN:
+            return False
+
+        if not app_config:
+            return True
+        if not app_config.dataroot:
+            return True
+        if len(app_config.multi_dataset_allowed_matrix_type) == 0:
+            return True
+
+        for val in app_config.multi_dataset_allowed_matrix_type:
+            try:
+                if self.matrix_data_type == MatrixDataType(val):
+                    return True
+            except ValueError:
+                # Check case where multi_dataset_allowed_matrix_type does not have a
+                # valid MatrixDataType value.  TODO:  Add a feature to check
+                # the AppConfig for errors on startup
+                return False
+
+        return False
+
     def pre_load_validation(self):
-        if self.etype == MatrixDataType.UNKNOWN:
+        if self.matrix_data_type == MatrixDataType.UNKNOWN:
             raise DatasetAccessError(f"{self.location} does not have a recognized type: .h5ad or .cxg")
         self.matrix_type.pre_load_validation(self.location)
 

server/eb/app.py

@@ -45,7 +45,9 @@ try:
         obs_names=None,
         var_names=None,
         anndata_backed=False,
-        disable_diffexp=False,
+        disable_diffexp=True,
+        multi_dataset_index=None,
+        multi_dataset_allowed_matrix_type=["cxg"],
     )
 
     matrix_data_cache_manager = MatrixDataCacheManager()