Merge pull request #171 from octoenergy/fix-bug-with-s3-subdir-list-f…

…iles

Fix bug with `list_files`  method in `S3SubdirectoryFileStore`

tests/storage/test_storage.py

@@ -175,6 +175,28 @@ def test_list_files(self, get_boto_bucket):
             Prefix="folder/a/b"
         )
 
+    @mock.patch.object(storage.S3FileStore, "_get_boto_bucket")
+    def test_list_files_no_path(self, get_boto_bucket):
+        get_boto_bucket.return_value = mock.Mock(
+            **{
+                "objects.filter.return_value": [
+                    mock.Mock(key="folder/a/b/"),
+                    mock.Mock(key="folder/a/b/foo.txt"),
+                    mock.Mock(key="folder/a/b/bar.txt"),
+                ]
+            }
+        )
+        store = storage.S3SubdirectoryFileStore("s3://some-bucket")
+
+        assert list(store.list_files(namespace="folder/a/b")) == [
+            "folder/a/b/foo.txt",
+            "folder/a/b/bar.txt",
+        ]
+        # Should be called including the subdirectory path.
+        get_boto_bucket.return_value.objects.filter.assert_called_once_with(
+            Prefix="folder/a/b"
+        )
+
     @mock.patch.object(storage.S3FileStore, "_get_boto_object")
     def test_fetch_file_fetches_given_path(self, get_boto_object):
         """Fetch file should act on the given path.

xocto/storage/storage.py

@@ -1013,10 +1013,12 @@ def list_s3_keys_page(
         return objects, next_token
 
     def list_files(self, namespace: str = "") -> Iterable[str]:
+        path_trim = 0
         if self.path:
+            path_trim = len(self.path) + 1
             namespace = os.path.join(self.path, namespace)
         full_paths = super().list_files(namespace)
-        yield from (path[len(self.path) + 1 :] for path in full_paths)
+        yield from (path[path_trim:] for path in full_paths)
 
     def copy(self, *, s3_object: S3Object, destination: str) -> S3Object:
         if self.path: