#50 fixed bug in listing subdirs

Fix cache pruning

Changelog.md

@@ -1,3 +1,7 @@
+# 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.3"

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/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

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:

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})"
                 )

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_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__":