prepare for 0.3.8 release

This is required by earlier flask-api versions

  File "/home/alokito/code/cellxgene-gateway/cellxgene_gateway/gateway.py", line 25, in <module>
    from flask_api import status
  File "/home/alokito/miniconda3/envs/cellxgene-gateway/lib/python3.7/site-packages/flask_api/__init__.py", line 1, in <module>
    from flask_api.app import FlaskAPI
  File "/home/alokito/miniconda3/envs/cellxgene-gateway/lib/python3.7/site-packages/flask_api/app.py", line 6, in <module>
    from flask_api.request import APIRequest
  File "/home/alokito/miniconda3/envs/cellxgene-gateway/lib/python3.7/site-packages/flask_api/request.py", line 9, in <module>
    from werkzeug._compat import to_unicode
ModuleNotFoundError: No module named 'werkzeug._compat'

.pre-commit-config.yaml

@@ -8,7 +8,6 @@ repos:
       types: [python]
       stages: [commit]
     - id: black
-      language_version: python3.6+
       name: black
       language: system
       entry: black

Changelog.md

@@ -1,3 +1,27 @@
+# 0.3.8
+
+* Fixed bug #57 affecting deeply nested subdirectory listing
+
+# 0.3.7
+
+* added back /metadata/ip_address endpoint
+
+# 0.3.6
+
+* pinned version of werkzeug
+
+# 0.3.5
+
+* Pinned flask version to match cellxgene 0.17.0
+
+# 0.3.4
+
+* Fixed bug #50 affecting subdirectory listing
+
+# 0.3.3
+
+* Fixed bug #48 affecting cache pruning
+
 # 0.3.2
 
  * Fixed bug #45 affecting multi-level S3 folders

README.md

@@ -2,7 +2,7 @@
 
 Cellxgene Gateway allows you to use the Cellxgene Server provided by the Chan Zuckerberg Institute (https://github.com/chanzuckerberg/cellxgene) with multiple datasets. It displays an index of available h5ad (anndata) files. When a user clicks on a file name, it launches a Cellxgene Server instance that loads that particular data file and once it is available  proxies requests to that server.
 
-[![codecov](https://codecov.io/gh/Novartis/cellxgene-gateway/branch/master/graph/badge.svg?token=ndEFSzRKJn)](https://codecov.io/gh/Novartis/cellxgene-gateway)
+[![codecov](https://codecov.io/gh/Novartis/cellxgene-gateway/branch/master/graph/badge.svg?token=ndEFSzRKJn)](https://codecov.io/gh/Novartis/cellxgene-gateway) [![PyPI](https://img.shields.io/pypi/v/cellxgene-gateway)](https://pypi.org/project/cellxgene-gateway/) [![PyPI - Downloads](https://img.shields.io/pypi/dm/cellxgene-gateway)](https://pypistats.org/packages/cellxgene-gateway)
 
 # Running locally
 

SECURITY.md

@@ -0,0 +1,15 @@
+# Security Policy
+
+## Supported Versions
+
+Use this section to tell people about which versions of your project are
+currently being supported with security updates.
+
+| Version | Supported          |
+| ------- | ------------------ |
+| 0.3.2   | :white_check_mark: |
+| <= 0.3.1   | :x:                |
+
+## Reporting a Vulnerability
+
+Please file a bug report issue.

cellxgene_gateway/__init__.py

@@ -7,4 +7,4 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-__version__ = "0.3.2"
+__version__ = "0.3.8"

cellxgene_gateway/dir_util.py

@@ -14,44 +14,6 @@ from flask_api import status
 from cellxgene_gateway import env
 from cellxgene_gateway.cellxgene_exception import CellxgeneException
 
-
-def is_subdir(full_path, parent_path):
-    subdir = os.path.realpath(full_path)
-    parent = os.path.realpath(parent_path)
-    return subdir.startswith(parent)
-
-
-def create_dir(parent_path, dir_name):
-    full_path = os.path.join(parent_path, dir_name)
-
-    if "/" in dir_name:
-        raise CellxgeneException(
-            "Please have no slashes in the intended directory.",
-            status.HTTP_400_BAD_REQUEST,
-        )
-    elif not os.path.exists(parent_path):
-        raise CellxgeneException(
-            "The selected User directory does not exist.",
-            status.HTTP_400_BAD_REQUEST,
-        )
-    elif os.path.exists(full_path):
-        raise CellxgeneException(
-            "The provided subdirectory already exists within Directory.",
-            status.HTTP_400_BAD_REQUEST,
-        )
-    elif not is_subdir(full_path, parent_path):
-        raise CellxgeneException(
-            "The directory must be a subdirectory of the parent path.",
-            status.HTTP_400_BAD_REQUEST,
-        )
-    elif not os.path.isdir(parent_path):
-        raise CellxgeneException(
-            "The parent is not a directory.", status.HTTP_400_BAD_REQUEST
-        )
-    else:
-        os.mkdir(full_path)
-
-
 annotations_suffix = "_annotations"
 h5ad_suffix = ".h5ad"
 

cellxgene_gateway/filecrawl.py

@@ -54,7 +54,7 @@ def render_item_tree(item_tree, item_source):
     if item_tree.descriptor:
         descriptor = item_tree.descriptor.lstrip("/")
         url = f"/filecrawl/{descriptor}?source={item_source.name}"
-        name = descriptor.rsplit("/")[1] if descriptor.find("/") >= 0 else descriptor
+        name = descriptor.rsplit("/", 1)[-1]
         return f"<li><a href='{url}'>{name}</a>{html}</li>"
     else:
         return html

cellxgene_gateway/gateway.py

@@ -31,7 +31,6 @@ from cellxgene_gateway.backend_cache import BackendCache
 from cellxgene_gateway.cache_entry import CacheEntryStatus
 from cellxgene_gateway.cache_key import CacheKey
 from cellxgene_gateway.cellxgene_exception import CellxgeneException
-from cellxgene_gateway.dir_util import create_dir, is_subdir
 from cellxgene_gateway.extra_scripts import get_extra_scripts
 from cellxgene_gateway.filecrawl import render_item_source
 from cellxgene_gateway.process_exception import ProcessException
@@ -53,6 +52,14 @@ def _force_https(app):
     return wrapper
 
 
+def set_no_cache(resp):
+    resp.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
+    resp.headers["Pragma"] = "no-cache"
+    resp.headers["Expires"] = "0"
+    resp.headers["Cache-Control"] = "public, max-age=0"
+    return resp
+
+
 app.wsgi_app = _force_https(app.wsgi_app)
 if (
     env.proxy_fix_for > 0
@@ -158,10 +165,7 @@ def filecrawl(path=None):
             path=path,
         )
     )
-    resp.headers["Cache-Control"] = "no-cache, no-store, must-revalidate"
-    resp.headers["Pragma"] = "no-cache"
-    resp.headers["Expires"] = "0"
-    resp.headers["Cache-Control"] = "public, max-age=0"
+    set_no_cache(resp)
     return resp
 
 
@@ -210,7 +214,10 @@ def do_view(path, source_name=None):
         match.status == CacheEntryStatus.loaded
         or match.status == CacheEntryStatus.loading
     ):
-        return match.serve_content(path)
+        if source.is_authorized(match.key.descriptor):
+            return match.serve_content(path)
+        else:
+            raise CellxgeneException("User not authorized to access this data", 403)
     elif match.status == CacheEntryStatus.error:
         raise ProcessException.from_cache_entry(match)
 
@@ -268,6 +275,12 @@ def do_terminate(path):
     return redirect(url_for("do_GET_status"), code=302)
 
 
+@app.route("/metadata/ip_address", methods=["GET"])
+def ip_address():
+    resp = make_response(env.ip)
+    return set_no_cache(resp)
+
+
 def launch():
     env.validate()
     if not item_sources or not len(item_sources):

cellxgene_gateway/items/file/fileitem_source.py

@@ -51,7 +51,7 @@ class FileItemSource(ItemSource):
         return self.convert_h5ad_path_to_annotation(item.descriptor)
 
     def list_items(self, filter: str = None) -> ItemTree:
-        item_tree = self.scan_directory()
+        item_tree = self.scan_directory("" if filter is None else filter)
 
         """def get_items(dir):
             if dir.branches:
@@ -121,6 +121,9 @@ class FileItemSource(ItemSource):
         if self.is_h5ad_file(full_path):
             return self.shallowitem_from_descriptor(descriptor)
 
+    def is_authorized(self, descriptor):
+        return True
+
     def lookup(self, indescriptor: str) -> LookupResult:
         descriptor = indescriptor.strip("/")
         if descriptor.endswith(self.annotation_file_suffix):

cellxgene_gateway/items/item_source.py

@@ -40,6 +40,10 @@ class ItemSource(ABC):
     def update(self, item: Item) -> None:
         raise Exception('"update" unimplemented')
 
+    @abstractmethod
+    def is_authorized(self, descriptor: str) -> bool:
+        raise Exception('"is_authorized" unimplemented')
+
     @abstractmethod
     def lookup(self, descriptor: str) -> LookupResult:
         raise Exception('"lookup" unimplemented')

cellxgene_gateway/items/s3/s3item_source.py

@@ -113,6 +113,9 @@ class S3ItemSource(ItemSource):
     def update(self, item: S3Item) -> None:
         pass
 
+    def is_authorized(self, descriptor):
+        return True
+
     def lookup_item(self, descriptor):
         full_path = self.url(descriptor)
         if self.is_h5ad_url(full_path):

cellxgene_gateway/path_util.py

@@ -1,41 +0,0 @@
-# Copyright 2019 Novartis Institutes for BioMedical Research Inc. Licensed
-# under the Apache License, Version 2.0 (the "License"); you may not use
-# this file except in compliance with the License. You may obtain a copy
-# of the License at http://www.apache.org/licenses/LICENSE-2.0. Unless
-# required by applicable law or agreed to in writing, software distributed
-# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
-# OR CONDITIONS OF ANY KIND, either express or implied. See the License for
-# the specific language governing permissions and limitations under the License.
-
-import os
-
-from flask_api import status
-
-from cellxgene_gateway.cache_key import CacheKey
-from cellxgene_gateway.cellxgene_exception import CellxgeneException
-from cellxgene_gateway.dir_util import make_h5ad
-
-
-def validate_exists(file_path):
-    if not os.path.exists(file_path):
-        raise CellxgeneException(
-            "File does not exist: " + file_path, status.HTTP_400_BAD_REQUEST
-        )
-
-
-def validate_is_file(file_path):
-    validate_exists(file_path)
-    if not os.path.isfile(file_path):
-        raise CellxgeneException(
-            "Path is not file: " + file_path, status.HTTP_400_BAD_REQUEST
-        )
-    return
-
-
-def validate_is_dir(file_path):
-    validate_exists(file_path)
-    if not os.path.isdir(file_path):
-        raise CellxgeneException(
-            "Path is not dir: " + file_path, status.HTTP_400_BAD_REQUEST
-        )
-    return

cellxgene_gateway/prune_process_cache.py

@@ -39,9 +39,9 @@ class PruneProcessCache:
 
         for process in processes_to_delete:
             try:
-                logger.info(f"pruning process {process.pid} ({process.key.dataset})")
+                logger.info(f"pruning process {process.pid} ({process.key.descriptor})")
                 self.cache.prune(process)
             except Exception:
                 logger.exception(
-                    "failed to prune process {process.pid} ({process.dataset})"
+                    "failed to prune process {process.pid} ({process.key.descriptor})"
                 )

cellxgene_gateway/templates/cache_status.html

@@ -75,7 +75,9 @@
                     const el = $(this);
                     const ts = el.text();
                     const dt = new Date(parseInt(ts * 1000));
-                    el.html(`${dt.toISOString()}<br>(${ts})`);
+                    el.prepend(`${dt.toISOString()}<br>(`);
+                    el.append(')');
+
                });
           })
      </script>

environment.yml

@@ -4,11 +4,15 @@ channels:
 dependencies:
 - python=3.7
 - requests
-- flask
+- flask<2.0.0,>=1.0.2
 - psutil
 - black
+- twine
+- isort
 - coverage
 - pip
 - pip:
-  - flask-api
+  - pre_commit
+  - flask-api==2.0
+  - werkzeug==1.0.1
   - cellxgene>=0.15

requirements.txt

@@ -1,5 +1,6 @@
 cellxgene>=0.15
-flask
-flask_api
+flask<2.0.0,>=1.0.2
+flask-api==2.0
+werkzeug==1.0.1
 psutil
 requests

tests/items/file/test_fileitem_source.py

@@ -1,10 +1,31 @@
 import tempfile
 import unittest
+from unittest.mock import patch
 
 from cellxgene_gateway.items.file.fileitem_source import FileItemSource
 
 
+def stub_join(path):
+    path.join = lambda x, y: x + "/" + y
+
+
 class TestFileItemSource(unittest.TestCase):
+    @patch("os.path")
+    @patch("os.listdir")
+    def test_list_items_GIVEN_no_subpath_THEN_checks_dir(self, listdir, path):
+        stub_join(path)
+        source = FileItemSource("/tmp/unittest", "local")
+        source.list_items()
+        path.exists.assert_called_once_with("/tmp/unittest/")
+
+    @patch("os.path")
+    @patch("os.listdir")
+    def test_list_items_GIVEN_subpath_THEN_checks_subpath(self, listdir, path):
+        stub_join(path)
+        source = FileItemSource("/tmp/unittest", "local")
+        source.list_items("foo")
+        path.exists.assert_called_once_with("/tmp/unittest/foo")
+
     def test_make_fileitem_from_path_GIVEN_annotation_file_THEN_name_lacks_csv(
         self,
     ):

tests/test_backend_cache.py

@@ -0,0 +1,28 @@
+import unittest
+from unittest.mock import MagicMock, patch
+
+from cellxgene_gateway.backend_cache import is_port_in_use
+
+
+class TestIsPortInUse(unittest.TestCase):
+    @patch("socket.socket")
+    def test_GIVEN_free_port_THEN_returns_true(self, socketMock):
+        connectMock = socketMock()
+        connectMock.connect_ex.return_value = 0
+        connectMock.__enter__.return_value = connectMock
+        self.assertEqual(is_port_in_use(123), True)
+        self.assertTrue(connectMock.__enter__.calledOnce)
+        self.assertTrue(connectMock.__exit__.calledOnce)
+        self.assertTrue(connectMock.connect_ex.calledOnceWith("a"))
+        self.assertTrue(socketMock.calledOnceWith("a"))
+
+    @patch("socket.socket")
+    def test_GIVEN_used_port_THEN_returns_false(self, socketMock):
+        connectMock = socketMock()
+        connectMock.__enter__.return_value = connectMock
+        connectMock.connect_ex.return_value = 1
+        self.assertTrue(connectMock.__enter__.calledOnce)
+        self.assertTrue(connectMock.__exit__.calledOnce)
+        self.assertTrue(connectMock.connect_ex.calledOnceWith("a"))
+        self.assertTrue(socketMock.calledOnceWith("a"))
+        self.assertEqual(is_port_in_use(123), False)

tests/test_dir_util.py

@@ -0,0 +1,35 @@
+import unittest
+from unittest.mock import MagicMock, patch
+
+from cellxgene_gateway.dir_util import ensure_dir_exists, make_annotations, make_h5ad
+
+
+class TestMakeH5ad(unittest.TestCase):
+    def test_GIVEN_annotation_dir_THEN_returns_h5ad(self):
+        self.assertEqual(make_h5ad("pbmc_annotations"), "pbmc.h5ad")
+
+
+class TestMakeAnnotations(unittest.TestCase):
+    def test_GIVEN_h5ad_THEN_returns_annotations(self):
+        self.assertEqual(make_annotations("pbmc.h5ad"), "pbmc_annotations")
+
+
+class TestMakeAnnotations(unittest.TestCase):
+    def test_GIVEN_h5ad_THEN_returns_annotations(self):
+        self.assertEqual(make_annotations("pbmc.h5ad"), "pbmc_annotations")
+
+
+class TestEnsureDirExists(unittest.TestCase):
+    @patch("os.path.exists")
+    @patch("os.makedirs")
+    def test_GIVEN_existing_THEN_does_not_call_makedir(self, makedirsMock, existsMock):
+        existsMock.return_value = True
+        ensure_dir_exists("/foo")
+        makedirsMock.assert_not_called()
+
+    @patch("os.path.exists")
+    @patch("os.makedirs")
+    def test_GIVEN_not_existing_THEN_calls_makedir(self, makedirsMock, existsMock):
+        existsMock.return_value = False
+        ensure_dir_exists("/foo")
+        makedirsMock.assert_called_once_with("/foo")

tests/test_filecrawl.py

@@ -1,7 +1,11 @@
 import unittest
 from unittest.mock import MagicMock, patch
 
-from cellxgene_gateway.filecrawl import render_item, render_item_source
+from cellxgene_gateway.filecrawl import (
+    render_item,
+    render_item_source,
+    render_item_tree,
+)
 from cellxgene_gateway.items.file.fileitem import FileItem
 from cellxgene_gateway.items.file.fileitem_source import FileItemSource
 from cellxgene_gateway.items.item import ItemTree, ItemType
@@ -41,3 +45,15 @@ class TestRenderItemSource(unittest.TestCase):
             rendered,
             "<h6><a href='/filecrawl.html?source=FakeSource'>FakeSource</a>:some_filter</h6><li><a href='/filecrawl/rootdir?source=FakeSource'>rootdir</a><ul></ul></li>",
         )
+
+
+class TestRenderItemTree(unittest.TestCase):
+    @patch("cellxgene_gateway.items.file.fileitem_source.FileItemSource")
+    def test_GIVEN_deep_nested_dirs_THEN_includes_dirs_in_output(self, item_source):
+        item_source.name = "FakeSource"
+        item_tree = ItemTree("foo/bar/baz", [], [])
+        rendered = render_item_tree(item_tree, item_source)
+        self.assertEqual(
+            rendered,
+            "<li><a href='/filecrawl/foo/bar/baz?source=FakeSource'>baz</a><ul></ul></li>",
+        )

tests/test_prune_process_cache.py

@@ -1,8 +1,16 @@
 import unittest
-from unittest.mock import MagicMock, patch
+from unittest.mock import patch, seal
 
 from cellxgene_gateway.backend_cache import BackendCache
-from cellxgene_gateway.cache_entry import CacheEntry
+from cellxgene_gateway.cache_key import CacheKey
+from cellxgene_gateway.items.file.fileitem import FileItem
+from cellxgene_gateway.items.file.fileitem_source import FileItemSource
+from cellxgene_gateway.items.item import ItemType
+
+key = CacheKey(
+    FileItem("/czi/", name="pbmc3k.h5ad", type=ItemType.h5ad),
+    FileItemSource("/tmp", "local"),
+)
 
 
 class TestPruneProcessCache(unittest.TestCase):
@@ -15,14 +23,23 @@ class TestPruneProcessCache(unittest.TestCase):
 
         cache = BackendCache()
         old.timestamp = -100
+        old.foo = 12
+        old.pid = 1
+        old.key = key
+        old.terminate.return_value = None
+        seal(old)
+        new.key = key
         cache.entry_list.append(old)
         new.timestamp = -5
+        seal(new)
         cache.entry_list.append(new)
         self.assertEqual(len(cache.entry_list), 2)
         ppc = PruneProcessCache(cache)
         ppc.prune()
         self.assertEqual(len(cache.entry_list), 1)
         self.assertEqual(cache.entry_list[0], new)
+        self.assertEqual(cache.entry_list[0], new)
+        self.assertTrue(old.terminate.called)
 
 
 if __name__ == "__main__":

tests/test_subprocess_backend.py

@@ -33,7 +33,7 @@ class TestSubprocessBackend(unittest.TestCase):
             backend.launch(cellxgene_loc, scripts, entry)
         popen.assert_called_once_with(
             [
-                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --scripts http://example.com/script.js --scripts http://example.com/script2.js"
+                "yes | /some/cellxgene launch /tmp/czi/pbmc3k.h5ad --port 8000 --host 127.0.0.1 --disable-annotations --disable-gene-sets-save --scripts http://example.com/script.js --scripts http://example.com/script2.js"
             ],
             shell=True,
             stderr=-1,