diff --git a/cellxgene_gateway/items/s3/s3item_source.py b/cellxgene_gateway/items/s3/s3item_source.py index 7a93a95..6f1ab26 100644 --- a/cellxgene_gateway/items/s3/s3item_source.py +++ b/cellxgene_gateway/items/s3/s3item_source.py @@ -41,6 +41,9 @@ class S3ItemSource(ItemSource): def url(self, key): return "s3://" + self.bucket + "/" + key + def remove_bucket(self, filepath): + return filepath[len(self.bucket) :].lstrip("/") + @property def name(self): return self._name or f"Items:{self.url('')}" @@ -71,7 +74,7 @@ class S3ItemSource(ItemSource): raise Exception(f"S3 url '{url}' does not exist.") s3key_map = dict( - (filepath[len(self.bucket) :].lstrip("/"), "s3://" + filepath) + (self.remove_bucket(filepath), "s3://" + filepath) for filepath in sorted(self.s3.ls(url)) ) @@ -93,12 +96,7 @@ class S3ItemSource(ItemSource): if self.s3.isdir(item_url) and not is_annotation_dir(filepath) ] - items = [ - self.make_s3item_from_key( - key[key.rindex("/") + 1 :] if "/" in key else key, key - ) - for key in h5ad_keys - ] + items = [self.make_s3item_from_key(basename(key), key) for key in h5ad_keys] branches = None if len(subdir_keys) > 0: branches = [self.scan_directory(key) for key in subdir_keys] @@ -163,11 +161,11 @@ class S3ItemSource(ItemSource): if self.s3.isdir(annotations_fullpath): return [ self.make_s3item_from_key( - annotation, join(annotations_subpath, annotation), True + basename(annotation), self.remove_bucket(annotation), True ) for annotation in sorted(self.s3.ls(annotations_fullpath)) if annotation.endswith(self.annotation_file_suffix) - and self.s3.isfile(join(annotations_fullpath, annotation)) + and self.s3.isfile("s3://" + annotation) ] else: return None diff --git a/tests/items/s3/__init__.py b/tests/items/s3/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/items/s3/test_s3item_source.py b/tests/items/s3/test_s3item_source.py new file mode 100644 index 0000000..7b20adf --- /dev/null +++ b/tests/items/s3/test_s3item_source.py @@ -0,0 +1,158 @@ +import unittest +from unittest.mock import MagicMock, Mock, patch + +from cellxgene_gateway.items.item import ItemType +from cellxgene_gateway.items.s3.s3item import S3Item +from cellxgene_gateway.items.s3.s3item_source import S3ItemSource + + +class TestScanDirectory(unittest.TestCase): + @patch("s3fs.S3FileSystem") + def test_GIVEN_invalid_bucket_THEN_throws_error(self, s3func): + class S3Mock: + def exists(path): + if path in ["s3://my-bucket/"]: + return False + + s3func.return_value = S3Mock + source = S3ItemSource("my-bucket") + with self.assertRaises(Exception) as context: + source.scan_directory() + self.assertEqual( + "S3 url 's3://my-bucket/' does not exist.", + str(context.exception), + ) + + @patch("s3fs.S3FileSystem") + def test__scan_directory__properly_recurses_suburls(self, s3func): + class S3Mock: + def exists(path): + if path in [ + "s3://my-bucket/", + "s3://my-bucket/pbmc3k.h5ad", + "s3://my-bucket/lvl1", + "s3://my-bucket/lvl1/pbmc3k_l1.h5ad", + "s3://my-bucket/lvl1/lvl2", + "s3://my-bucket/lvl1/lvl2/pbmc3k_l2.h5ad", + ]: + return True + raise Exception("exists called with " + path) + + def ls(path): + if path == "s3://my-bucket/": + return [ + "my-bucket/lvl1", + "my-bucket/pbmc3k.h5ad", + "my-bucket/pbmc3k_annotations", + ] + elif path == "s3://my-bucket/pbmc3k_annotations": + return ["my-bucket/pbmc3k_annotations/annot.csv"] + elif path == "s3://my-bucket/lvl1": + return ["my-bucket/lvl1/lvl2", "my-bucket/lvl1/pbmc3k_l1.h5ad"] + elif path == "s3://my-bucket/lvl1/lvl2": + return ["my-bucket/lvl1/lvl2/pbmc3k_l2.h5ad"] + + raise Exception("ls called with " + path) + + def isdir(path): + if path in [ + "s3://my-bucket/lvl1", + "s3://my-bucket/pbmc3k_annotations", + "s3://my-bucket/lvl1/lvl2", + ]: + return True + if path in [ + "s3://my-bucket/pbmc3k.h5ad", + "s3://my-bucket/lvl1/pbmc3k_l1.h5ad", + "s3://my-bucket/lvl1/pbmc3k_l1_annotations", + "s3://my-bucket/lvl1/lvl2/pbmc3k_l2.h5ad", + "s3://my-bucket/lvl1/lvl2/pbmc3k_l2_annotations", + ]: + return False + raise Exception("isdir called with " + path) + + def isfile(path): + if path in ["s3://my-bucket/pbmc3k_annotations/annot.csv"]: + return True + if path in ["s3://my-bucket/pbmc3k_annotations"]: + return False + raise Exception("isfile called with " + path) + + s3func.return_value = S3Mock + source = S3ItemSource("my-bucket") + tree = source.scan_directory() + + def s3item_compare(i1, i2, msg=""): + self.assertEqual(i1.name, i2.name, "name equals") + self.assertEqual(i1.type, i2.type, "type equals") + self.assertEqual(i1.s3key, i2.s3key, "s3key equals") + if i1.annotations is None: + self.assertEqual(i1.annotations, i2.annotations, "annotations equals") + else: + self.assertEqual( + len(i1.annotations), + len(i2.annotations), + "annotations length equals", + ) + for a1, a2 in zip(i1.annotations, i2.annotations): + self.assertEqual(a1, a2) + return True + + self.addTypeEqualityFunc(S3Item, s3item_compare) + + def assertTree(t, descriptor, items): + self.assertEqual(t.descriptor, descriptor) + self.assertEqual(len(t.items), len(items)) + for i1, i2 in zip(t.items, items): + self.assertEqual(i1, i2) + + assertTree( + tree, + "", + [ + S3Item( + "pbmc3k.h5ad", + name="pbmc3k.h5ad", + type=ItemType.h5ad, + annotations=[ + S3Item( + "pbmc3k_annotations/annot.csv", + name="annot.csv", + type=ItemType.annotation, + ) + ], + ) + ], + ) + self.assertEqual(len(tree.branches), 1) + lvl1 = tree.branches[0] + assertTree( + lvl1, + "lvl1", + [ + S3Item( + "lvl1/pbmc3k_l1.h5ad", + name="pbmc3k_l1.h5ad", + type=ItemType.h5ad, + annotations=None, + ) + ], + ) + self.assertEqual(len(lvl1.branches), 1) + lvl2 = lvl1.branches[0] + assertTree( + lvl2, + "lvl1/lvl2", + [ + S3Item( + "lvl1/lvl2/pbmc3k_l2.h5ad", + name="pbmc3k_l2.h5ad", + type=ItemType.h5ad, + annotations=None, + ) + ], + ) + self.assertEqual(lvl2.branches, None) + + def test__list_items__pass_filter_into_scan_directory(self): + pass