Fix get_files(directory) on S3 and local storage drivers

fastapi_startkit/pyproject.toml

@@ -108,6 +108,7 @@ dev = [
     "langchain>=1.0.0",
     "langchain-core>=1.0.0",
     "pyright>=1.1.411",
+    "boto3>=1.43.93",
 ]
 
 

fastapi_startkit/src/fastapi_startkit/storage/drivers/local.py

@@ -115,13 +115,22 @@ def make_file_path_if_not_exists(self, file_path):
         return False
 
     def get_files(self, directory=""):
+        """List the files directly under ``directory`` (non-recursive).
+
+        Directory entries are excluded and an empty or nonexistent directory
+        yields ``[]``. Each ``File`` is named by its bare filename and carries
+        the file's content.
+        """
         file_path = self.get_path(directory)
+        if not os.path.isdir(file_path):
+            return []
+
         files = []
         for f in os.listdir(file_path):
             if not isfile(join(file_path, f)):
                 continue
 
-            files.append(File(self.get(f), f))
+            files.append(File(self.get(join(directory, f)), f))
 
         return files
 

fastapi_startkit/src/fastapi_startkit/storage/drivers/s3.py

@@ -148,17 +148,25 @@ def make_file_path_if_not_exists(self, file_path):
         return False
 
     def get_files(self, directory=None):
+        """List the files directly under ``directory`` (non-recursive).
+
+        Directory entries are excluded and an empty or nonexistent directory
+        yields ``[]``. Each ``File`` is named by its bare filename, matching
+        the local driver's contract.
+        """
         bucket = self.get_resource().Bucket(self.get_bucket())
 
-        if directory:
-            objects = bucket.objects.all().filter(Prefix=directory)
+        prefix = f"{directory.rstrip('/')}/" if directory else ""
+        if prefix:
+            objects = bucket.objects.filter(Prefix=prefix)
         else:
             objects = bucket.objects.all()
 
         files = []
-        for my_bucket_object in objects.all():
-            if "/" not in my_bucket_object.key:
-                files.append(File(my_bucket_object, my_bucket_object.key))
+        for my_bucket_object in objects:
+            relative = my_bucket_object.key[len(prefix):]
+            if relative and "/" not in relative:
+                files.append(File(my_bucket_object, relative))
 
         return files
 

fastapi_startkit/tests/storage/test_local_driver.py

@@ -112,6 +112,50 @@ def test_get_files_skips_subdirectories(self, driver, tmp_path):
         files = driver.get_files("")
         assert [f.name() for f in files] == ["file.txt"]
 
+    def test_get_files_root_returns_real_content(self, driver, tmp_path):
+        storage = tmp_path / "storage"
+        storage.mkdir(parents=True, exist_ok=True)
+        (storage / "root.txt").write_text("root content")
+        files = driver.get_files("")
+        assert [f.stream() for f in files] == ["root content"]
+
+    def test_get_files_in_subdirectory_returns_real_content(self, driver, tmp_path):
+        audio = tmp_path / "storage" / "audio"
+        audio.mkdir(parents=True)
+        (audio / "tts-1.txt").write_text("speech one")
+        (audio / "tts-2.txt").write_text("speech two")
+
+        files = driver.get_files("audio")
+
+        by_name = {f.name(): f.stream() for f in files}
+        assert by_name == {"tts-1.txt": "speech one", "tts-2.txt": "speech two"}
+
+    def test_get_files_directory_with_trailing_slash(self, driver, tmp_path):
+        audio = tmp_path / "storage" / "audio"
+        audio.mkdir(parents=True)
+        (audio / "a.txt").write_text("a")
+
+        files = driver.get_files("audio/")
+
+        assert [(f.name(), f.stream()) for f in files] == [("a.txt", "a")]
+
+    def test_get_files_nonexistent_directory_returns_empty_list(self, driver):
+        assert driver.get_files("does-not-exist") == []
+
+    def test_get_files_empty_directory_returns_empty_list(self, driver, tmp_path):
+        (tmp_path / "storage" / "empty").mkdir(parents=True)
+        assert driver.get_files("empty") == []
+
+    def test_get_files_in_subdirectory_skips_nested_directories(self, driver, tmp_path):
+        audio = tmp_path / "storage" / "audio"
+        (audio / "nested").mkdir(parents=True)
+        (audio / "keep.txt").write_text("kept")
+        (audio / "nested" / "skip.txt").write_text("skipped")
+
+        files = driver.get_files("audio")
+
+        assert [f.name() for f in files] == ["keep.txt"]
+
 
 class TestLocalDriverPathResolution:
     def test_get_path_joins_relative_root_to_base_path(self, tmp_path):
@@ -254,6 +298,31 @@ def test_fake_driver_context_manager(self):
             root = fake._root
         assert not os.path.exists(root)
 
+    def test_fake_driver_get_files_in_subdirectory(self, fake):
+        fake.put("audio/one.txt", "first")
+        fake.put("audio/two.txt", "second")
+
+        files = fake.get_files("audio")
+
+        by_name = {f.name(): f.stream() for f in files}
+        assert by_name == {"one.txt": "first", "two.txt": "second"}
+
+    def test_fake_driver_get_files_nonexistent_directory(self, fake):
+        assert fake.get_files("missing") == []
+
+    def test_fake_driver_get_files_skips_nested_directories(self, fake):
+        fake.put("docs/keep.txt", "kept")
+        fake.put("docs/nested/skip.txt", "skipped")
+
+        files = fake.get_files("docs")
+
+        assert [f.name() for f in files] == ["keep.txt"]
+
+    def test_fake_driver_assert_count_in_subdirectory(self, fake):
+        fake.put("uploads/a.txt", "a")
+        fake.put("uploads/b.txt", "b")
+        fake.assert_count(2, "uploads")
+
     def test_fake_driver_isolated_between_instances(self):
         app = MagicMock()
         app.base_path = "/fake"

fastapi_startkit/tests/storage/test_s3_driver.py

@@ -261,6 +261,69 @@ def test_move_does_not_delete_source_when_copy_fails(self, driver):
             mock_delete.assert_not_called()
 
 
+# ---------------------------------------------------------------------------
+# get_files
+# ---------------------------------------------------------------------------
+
+
+class _Obj:
+    def __init__(self, key):
+        self.key = key
+
+
+BUCKET_KEYS = ["backups/a.dump", "backups/b.dump", "backups/nested/deep.dump", "root.txt", "audio/x/y.mp3"]
+
+
+def _stub_listing(mock_resource):
+    """Make the mocked bucket answer .objects.all() / .filter(Prefix=...) like S3 would."""
+    objects = [_Obj(key) for key in BUCKET_KEYS]
+    bucket = mock_resource.Bucket.return_value
+    bucket.objects.all.return_value = objects
+    bucket.objects.filter.side_effect = lambda Prefix: [o for o in objects if o.key.startswith(Prefix)]
+    return bucket
+
+
+class TestS3DriverGetFiles:
+    def test_root_listing_keeps_only_root_level_keys(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        files = driver.get_files()
+        assert [f.name() for f in files] == ["root.txt"]
+
+    def test_directory_listing_returns_files_directly_under_prefix(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        files = driver.get_files("backups")
+        assert sorted(f.name() for f in files) == ["a.dump", "b.dump"]
+
+    def test_directory_listing_filters_with_slash_terminated_prefix(self, driver, mock_resource):
+        bucket = _stub_listing(mock_resource)
+        driver.get_files("backups")
+        bucket.objects.filter.assert_called_once_with(Prefix="backups/")
+
+    def test_trailing_slash_directory_is_equivalent(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        files = driver.get_files("backups/")
+        assert sorted(f.name() for f in files) == ["a.dump", "b.dump"]
+
+    def test_partial_name_prefix_is_not_a_directory_match(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        assert driver.get_files("back") == []
+
+    def test_directory_with_only_nested_subdirectories_is_empty(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        assert driver.get_files("audio") == []
+
+    def test_nonexistent_directory_returns_empty_list(self, driver, mock_resource):
+        _stub_listing(mock_resource)
+        assert driver.get_files("nope") == []
+
+    def test_directory_marker_key_is_skipped(self, driver, mock_resource):
+        objects = [_Obj("backups/"), _Obj("backups/a.dump")]
+        bucket = mock_resource.Bucket.return_value
+        bucket.objects.filter.side_effect = lambda Prefix: [o for o in objects if o.key.startswith(Prefix)]
+        files = driver.get_files("backups")
+        assert [f.name() for f in files] == ["a.dump"]
+
+
 # ---------------------------------------------------------------------------
 # Connection caching
 # ---------------------------------------------------------------------------

fastapi_startkit/tests/storage/test_s3_minio_integration.py

@@ -0,0 +1,123 @@
+"""Integration tests for S3Driver.get_files against a real S3 API (MinIO).
+
+These run against a live MinIO server (issue #218). They are skipped when
+boto3 is not installed or no server is reachable, so the suite stays green
+in environments without MinIO.
+
+Override the target with MINIO_ENDPOINT / MINIO_ACCESS_KEY / MINIO_SECRET_KEY.
+"""
+
+import os
+import uuid
+from unittest.mock import MagicMock
+
+import pytest
+
+boto3 = pytest.importorskip("boto3")
+import botocore.config  # noqa: E402
+
+from fastapi_startkit.storage.drivers.s3 import S3Driver  # noqa: E402
+
+ENDPOINT = os.environ.get("MINIO_ENDPOINT", "http://localhost:9002")
+ACCESS_KEY = os.environ.get("MINIO_ACCESS_KEY", "minio")
+SECRET_KEY = os.environ.get("MINIO_SECRET_KEY", "minio123")
+
+KEYS = {
+    "root.txt": b"root content",
+    "backups/a.dump": b"dump a",
+    "backups/b.dump": b"dump b",
+    "backups/nested/deep.dump": b"deep",
+    "audio/x/y.mp3": b"audio bytes",
+}
+
+
+def _client():
+    session = boto3.Session(
+        aws_access_key_id=ACCESS_KEY,
+        aws_secret_access_key=SECRET_KEY,
+        region_name="us-east-1",
+    )
+    return session.client(
+        "s3",
+        endpoint_url=ENDPOINT,
+        config=botocore.config.Config(
+            s3={"addressing_style": "path"},
+            connect_timeout=2,
+            retries={"max_attempts": 1},
+        ),
+    )
+
+
+def _minio_reachable():
+    try:
+        _client().list_buckets()
+        return True
+    except Exception:
+        return False
+
+
+pytestmark = pytest.mark.skipif(not _minio_reachable(), reason=f"MinIO not reachable at {ENDPOINT}")
+
+
+@pytest.fixture(scope="module")
+def bucket_name():
+    client = _client()
+    name = f"fsk-get-files-{uuid.uuid4().hex[:12]}"
+    client.create_bucket(Bucket=name)
+    for key, body in KEYS.items():
+        client.put_object(Bucket=name, Key=key, Body=body)
+
+    yield name
+
+    listing = client.list_objects_v2(Bucket=name).get("Contents", [])
+    for obj in listing:
+        client.delete_object(Bucket=name, Key=obj["Key"])
+    client.delete_bucket(Bucket=name)
+
+
+@pytest.fixture
+def driver(bucket_name):
+    d = S3Driver(MagicMock())
+    d.set_options(
+        {
+            "bucket": bucket_name,
+            "key": ACCESS_KEY,
+            "secret": SECRET_KEY,
+            "region": "us-east-1",
+            "endpoint": ENDPOINT,
+            "use_path_style_endpoint": True,
+        }
+    )
+    return d
+
+
+class TestGetFilesAgainstMinio:
+    def test_root_listing_returns_only_root_level_files(self, driver):
+        files = driver.get_files()
+        assert [f.name() for f in files] == ["root.txt"]
+
+    def test_one_level_deep_directory(self, driver):
+        files = driver.get_files("backups")
+        assert sorted(f.name() for f in files) == ["a.dump", "b.dump"]
+
+    def test_trailing_slash_is_equivalent(self, driver):
+        files = driver.get_files("backups/")
+        assert sorted(f.name() for f in files) == ["a.dump", "b.dump"]
+
+    def test_two_levels_deep_directory(self, driver):
+        files = driver.get_files("backups/nested")
+        assert [f.name() for f in files] == ["deep.dump"]
+
+    def test_directory_containing_only_subdirectories_is_empty(self, driver):
+        assert driver.get_files("audio") == []
+
+    def test_nonexistent_directory_returns_empty_list(self, driver):
+        assert driver.get_files("does-not-exist") == []
+
+    def test_partial_name_prefix_is_not_a_directory_match(self, driver):
+        assert driver.get_files("back") == []
+
+    def test_listed_objects_point_at_the_real_keys(self, driver):
+        files = driver.get_files("backups")
+        keys = sorted(f.stream().key for f in files)
+        assert keys == ["backups/a.dump", "backups/b.dump"]