fix: --put-root-index combined with --prefix

CHANGELOG.md

@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [PEP 440](https://www.python.org/dev/peps/pep-0440/).
 
 
+## 1.2.1 - 2023-12-31
+
+### Fixed
+
+- `--put-root-index` combined with `--prefix` puts the index page in that prefix, not in
+  the bucket root.
+
+
 ## 1.2.0 - 2023-12-30
 
 ### Added

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "s3pypi"
-version = "1.2.0"
+version = "1.2.1"
 description = "CLI for creating a Python Package Repository in an S3 bucket"
 authors = [
     "Matteo De Wint <[email protected]>",

s3pypi/__init__.py

@@ -1,2 +1,2 @@
 __prog__ = "s3pypi"
-__version__ = "1.2.0"
+__version__ = "1.2.1"

s3pypi/storage.py

@@ -1,3 +1,4 @@
+from collections import deque
 from dataclasses import dataclass, field
 from pathlib import Path
 from typing import Dict, Optional
@@ -36,8 +37,8 @@ def __init__(self, session: boto3.session.Session, cfg: S3Config):
     def _object(self, directory: str, filename: str) -> Object:
         parts = [directory, filename]
         if parts == [self.root, self.index_name]:
-            parts = [self._index]
-        if self.cfg.prefix:
+            parts = [p, self.index_name] if (p := self.cfg.prefix) else [self._index]
+        elif self.cfg.prefix:
             parts.insert(0, self.cfg.prefix)
         return self.s3.Object(self.cfg.bucket, key="/".join(parts))
 
@@ -49,15 +50,25 @@ def get_index(self, directory: str) -> Index:
         return Index.parse(html.decode())
 
     def build_root_index(self) -> Index:
-        paginator = self.s3.meta.client.get_paginator("list_objects_v2")
-        result = paginator.paginate(
-            Bucket=self.cfg.bucket,
-            Prefix=self.cfg.prefix or "",
-            Delimiter="/",
-        )
-        n = len(self.cfg.prefix) + 1 if self.cfg.prefix else 0
-        dirs = (p.get("Prefix")[n:] for p in result.search("CommonPrefixes"))
-        return Index(dict.fromkeys(dirs))
+        return Index(dict.fromkeys(self._list_dirs()))
+
+    def _list_dirs(self) -> list[str]:
+        results = set()
+        root = f"{p}/" if (p := self.cfg.prefix) else ""
+        todo = deque([root])
+        while todo:
+            current = todo.popleft()
+            if found := [
+                prefix
+                for item in self.s3.meta.client.get_paginator("list_objects_v2")
+                .paginate(Bucket=self.cfg.bucket, Delimiter="/", Prefix=current)
+                .search("CommonPrefixes")
+                if item and (prefix := item.get("Prefix"))
+            ]:
+                todo.extend(found)
+            else:
+                results.add(current[len(root) :])
+        return sorted(results)
 
     def put_index(self, directory: str, index: Index) -> None:
         self._object(directory, self.index_name).put(

setup.cfg

@@ -1,11 +1,12 @@
 [bumpversion]
-current_version = 1.2.0
+current_version = 1.2.1
 commit = True
 message = chore: bump version to {new_version}
 
 [tool:pytest]
 addopts = 
 	--tb=short
+testpaths = tests/unit/ tests/integration/
 
 [flake8]
 max-line-length = 80

tests/integration/test_main.py

@@ -20,24 +20,29 @@ def test_string_dict(text, expected):
     assert string_dict(text) == expected
 
 
-def test_main_upload_package(chdir, data_dir, s3_bucket, dynamodb_table):
+@pytest.mark.parametrize("prefix", ["", "packages", "packages/abc"])
+def test_main_upload_package(chdir, data_dir, s3_bucket, dynamodb_table, prefix):
+    args = ["dists/*", "--bucket", s3_bucket.name, "--lock-indexes", "--put-root-index"]
+    if prefix:
+        args.extend(["--prefix", prefix])
+
     with chdir(data_dir):
-        dist = "dists/*"
-        s3pypi(dist, "--bucket", s3_bucket.name, "--lock-indexes", "--put-root-index")
+        s3pypi(*args)
 
     def read(key: str) -> bytes:
         return s3_bucket.Object(key).get()["Body"].read()
 
-    root_index = read("index.html").decode()
+    root_index = read(f"{prefix}/" if prefix else "index.html").decode()
 
-    def assert_pkg_exists(prefix: str, filename: str):
-        assert read(prefix + filename)
-        assert f">{filename}</a>" in read(prefix).decode()
-        assert f">{prefix.rstrip('/')}</a>" in root_index
+    def assert_pkg_exists(pkg: str, filename: str):
+        path = (f"{prefix}/" if prefix else "") + f"{pkg}/"
+        assert read(path + filename)
+        assert f">{filename}</a>" in read(path).decode()
+        assert f">{pkg}</a>" in root_index
 
-    assert_pkg_exists("foo/", "foo-0.1.0.tar.gz")
-    assert_pkg_exists("hello-world/", "hello_world-0.1.0-py3-none-any.whl")
-    assert_pkg_exists("xyz/", "xyz-0.1.0.zip")
+    assert_pkg_exists("foo", "foo-0.1.0.tar.gz")
+    assert_pkg_exists("hello-world", "hello_world-0.1.0-py3-none-any.whl")
+    assert_pkg_exists("xyz", "xyz-0.1.0.zip")
 
 
 def test_main_upload_package_exists(chdir, data_dir, s3_bucket, caplog):

tests/integration/test_storage.py

@@ -1,3 +1,5 @@
+import pytest
+
 from s3pypi.index import Index
 from s3pypi.storage import S3Config, S3Storage
 
@@ -7,18 +9,61 @@ def test_index_storage_roundtrip(boto3_session, s3_bucket):
     index = Index({"bar": None})
 
     cfg = S3Config(bucket=s3_bucket.name)
-    storage = S3Storage(boto3_session, cfg)
+    s = S3Storage(boto3_session, cfg)
 
-    storage.put_index(directory, index)
-    got = storage.get_index(directory)
+    s.put_index(directory, index)
+    got = s.get_index(directory)
 
     assert got == index
 
 
-def test_prefix_in_s3_key(boto3_session):
-    cfg = S3Config(bucket="example", prefix="1234567890")
-    storage = S3Storage(boto3_session, cfg)
+index = object()
+
+
+@pytest.mark.parametrize(
+    "cfg, directory, filename, expected_key",
+    [
+        (S3Config(""), "/", index, "index.html"),
+        (S3Config(""), "foo", "bar", "foo/bar"),
+        (S3Config("", prefix="P"), "/", index, "P/"),
+        (S3Config("", prefix="P"), "foo", "bar", "P/foo/bar"),
+        (S3Config("", prefix="P", unsafe_s3_website=True), "/", index, "P/index.html"),
+        (S3Config("", unsafe_s3_website=True), "/", index, "index.html"),
+    ],
+)
+def test_s3_key(boto3_session, cfg, directory, filename, expected_key):
+    s = S3Storage(boto3_session, cfg)
+    if filename is index:
+        filename = s.index_name
+
+    obj = s._object(directory, filename)
+
+    assert obj.key == expected_key
+
 
-    obj = storage._object(directory="foo", filename="bar")
+def test_list_dirs(boto3_session, s3_bucket):
+    cfg = S3Config(bucket=s3_bucket.name, prefix="AA")
+    s = S3Storage(boto3_session, cfg)
+    s.put_index("one", Index())
+    s.put_index("two", Index())
+    s.put_index("three", Index())
+
+    assert s._list_dirs() == ["one/", "three/", "two/"]
+
+    cfg = S3Config(bucket=s3_bucket.name, prefix="BBBB")
+    s = S3Storage(boto3_session, cfg)
+    s.put_index("xxx", Index())
+    s.put_index("yyy", Index())
+
+    assert s._list_dirs() == ["xxx/", "yyy/"]
+
+    cfg = S3Config(bucket=s3_bucket.name)
+    s = S3Storage(boto3_session, cfg)
 
-    assert obj.key.startswith(cfg.prefix + "/")
+    assert s._list_dirs() == [
+        "AA/one/",
+        "AA/three/",
+        "AA/two/",
+        "BBBB/xxx/",
+        "BBBB/yyy/",
+    ]

tox.ini

@@ -20,10 +20,11 @@ deps =
     pytest-cov
 commands =
     mypy s3pypi/ tests/
-    pytest tests/unit/ tests/integration/ {posargs} \
+    pytest {posargs} \
         --cov=s3pypi \
         --cov-report term \
-        --cov-report html:coverage
+        --cov-report html:coverage \
+        --no-cov-on-fail
 
 [testenv:py38-lambda]
 deps =