diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c62ea4c4..b312ec193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,13 @@ All notable changes to this project are documented in this file. The format is b previously served, and anonymous, in production. ### Added +- Resumable multipart uploads for `file_storage` (#454): `POST /api/file-storage/uploads` + sessions with presigned S3 part URLs (or relayed parts on the filesystem backend), + 4-way parallel parts with retry and `localStorage` resume in the Files page, + `s3_public_endpoint_url`, CSP `connect-src` for the S3 origin, and an hourly + `file_storage.reap_stale_uploads` janitor (with `background_tasks`). Sessions count + against the quota and the assembled size is re-verified at completion. Migration + `a659221c464b` adds `file_storage_upload_session`. - background_tasks Celery config (#447, #448, #450): a module can ship a light `task_config.py` (`TASK_ROUTES`, `QUEUES`, `BEAT_SCHEDULE`) read without importing `tasks.py`; `task_routes_override` re-routes tasks at runtime and the @@ -140,6 +147,9 @@ All notable changes to this project are documented in this file. The format is b production-mode containers pass `UsersSettings` boot validation. ### Changed +- `StoredFile.checksum_sha256` / `StoredFileOut.checksum_sha256` are now nullable (#454): + files uploaded through the multipart flow have no checksum until + `file_storage.compute_checksum` runs, and `ETag`s fall back to id + size. - **Dashboard stats are admin-only on single-tenant installs too** (#464). `dashboard.view` was mapped to the ordinary `user` role when `multi_tenant` was off, so any self-registered account could read user diff --git a/docs/modules/file_storage.md b/docs/modules/file_storage.md index 7c9c6dbfa..7f4544e42 100644 --- a/docs/modules/file_storage.md +++ b/docs/modules/file_storage.md @@ -108,6 +108,43 @@ Redirected (presigned) downloads are served by the object store, which handles `Range` itself. Always redirecting large downloads to a presigned URL, as an opt-in setting, is not implemented. +### Direct, resumable uploads + +Files above `direct_upload_threshold_bytes` (default 64 MiB) upload in parts through a *session*, so a dropped connection or a reload loses at most one part. The Files page does this automatically; API clients drive it themselves. All routes need `file_storage.upload` and act only on the caller's own sessions (anyone else's id is a 404). + +| Route | Purpose | +|---|---| +| `POST /api/file-storage/uploads` `{filename, content_type, total_size, public}` | Open a session. Checks the content-type allow-list, `max_direct_upload_bytes` (default 10 GiB) and the quota (stored bytes plus the declared size of every still-open session). Returns `part_size` (16 MiB, grown so the file needs at most 10,000 parts) and `total_parts`. | +| `GET /uploads/{id}` | The session plus the parts the backend holds — the source of truth for resuming. | +| `POST /uploads/{id}/part-urls` `{parts: [n]}` | Where to PUT each part: presigned S3 URLs (`direct: true`, bound to the part's length) or the app's own part route. Only part numbers `1..total_parts`. | +| `PUT /uploads/{id}/parts/{n}` | Filesystem backend only: the part body, relayed through the app (capped at that part's exact size). | +| `POST /uploads/{id}/complete` `{parts: [{n, etag}]}` | Assemble, verify, create the `StoredFile`, publish `FileUploaded`. The assembled size must equal `total_size`; the maximum and the quota are re-checked against the stored object, and a violation deletes it. Retrying a completed session returns the same file. | +| `DELETE /uploads/{id}` | Abort and discard the parts. | + +Sessions expire after `upload_session_ttl_seconds` (24 h). With `background_tasks` installed, the hourly beat task `file_storage.reap_stale_uploads` aborts expired sessions and, on S3, orphaned multipart uploads older than twice the TTL that have this module's key shape and no active session (a bucket shared with other applications is left alone); without it, `POST /uploads` aborts the caller's own expired sessions. Completed and aborted session rows are deleted `upload_session_retention_days` (default 7) after their expiry, by the same task or, without `background_tasks`, by the caller's next `POST /uploads` (own rows only). `file_storage.compute_checksum` fills `checksum_sha256` after completion when `background_tasks` is installed. + +In the browser, parts go four at a time with per-part retry and backoff. The session id is kept in `localStorage` keyed by file name, size and last-modified time; after a reload, select the same file again and the page offers **Resume upload** or **Start over**. (Browsers cannot keep a file handle across a reload, so the file must be picked again.) + +Known limitations: + +- Two simultaneous `PUT`s of the same part on the filesystem backend can leave the part's data and ETag from different writers; `complete` then reports a mismatch and the client re-sends the part. +- The resume key is file name, size and last-modified time, so two tabs uploading an identical file share one session. +- A part shorter than expected is stored and refused with 400; it stays until the client re-sends the part (the resend overwrites it), and `complete` flags it as mismatched. + +#### S3 setup + +Parts are PUT by the browser straight to the bucket, so: + +- **CORS** on the bucket must allow `PUT` from the app's origin and expose `ETag`: + ```json + [{"AllowedOrigins": ["https://app.example.com"], "AllowedMethods": ["PUT"], + "AllowedHeaders": ["*"], "ExposeHeaders": ["ETag"], "MaxAgeSeconds": 3000}] + ``` +- `s3_public_endpoint_url` is the endpoint browsers reach, when it differs from the one the server uses (`s3_endpoint_url`, e.g. an in-cluster MinIO hostname). Presigned URLs are signed for it, and the redirect for presigned downloads uses it too. Blank falls back to `s3_endpoint_url`. +- The endpoint's origin is added to the CSP `connect-src` automatically (for AWS defaults, the bucket's `*.s3..amazonaws.com` hosts). + +New settings: `direct_upload_threshold_bytes`, `max_direct_upload_bytes`, `multipart_part_size_bytes`, `upload_session_ttl_seconds`, `upload_session_retention_days`, `s3_public_endpoint_url`. A backend opts in with `supports_direct_upload = True` and the `DirectUploadBackend` methods in `file_storage.contracts.service`; without it, `POST /uploads` answers 501 and the page never uses sessions. + ### View | Method + path | Inertia component | Permission | @@ -153,7 +190,7 @@ from file_storage.contracts import ( | `content_type` | `str(128)` | sniffed / declared MIME type | | `size_bytes` | `int` | | | `backend` | `str(32)` | `"filesystem"` or `"s3"` — recorded at upload time | -| `checksum_sha256` | `str(64)` | computed during stream-upload | +| `checksum_sha256` | `str(64) \| null` | computed during stream-upload; **null** for a direct (multipart) upload until `file_storage.compute_checksum` runs (never, without `background_tasks`) | | `public` | `bool` | default `false`; opt-in anonymous serving | | `extra_metadata` | `dict` | per-backend extras | | audit + soft-delete | from `AuditMixin` + `SoftDeleteMixin` | | diff --git a/host/migrations/versions/a659221c464b_file_storage_upload_sessions.py b/host/migrations/versions/a659221c464b_file_storage_upload_sessions.py new file mode 100644 index 000000000..8998b1a2b --- /dev/null +++ b/host/migrations/versions/a659221c464b_file_storage_upload_sessions.py @@ -0,0 +1,96 @@ +"""file_storage: resumable upload sessions, nullable checksum (#454) + +Multipart uploads bypass the app, so nothing hashes them on the way in: the +checksum becomes nullable and is filled in later by a background task. The +downgrade backfills nulls with an empty string before restoring NOT NULL. + +Revision ID: a659221c464b +Revises: e5a8c1f27b94 +Create Date: 2026-10-15 10:18:19.289035 +""" + +from collections.abc import Sequence + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "a659221c464b" +down_revision: str | None = "e5a8c1f27b94" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + op.create_table( + "file_storage_upload_session", + sa.Column("tenant_id", sa.String(length=50), nullable=False), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("(CURRENT_TIMESTAMP)"), + nullable=False, + ), + sa.Column("updated_at", sa.DateTime(timezone=True), nullable=True), + sa.Column("created_by", sa.String(length=255), nullable=True), + sa.Column("updated_by", sa.String(length=255), nullable=True), + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("key", sa.String(length=512), nullable=False), + sa.Column("filename", sa.String(length=255), nullable=False), + sa.Column("content_type", sa.String(length=128), nullable=False), + sa.Column("total_size", sa.BigInteger(), nullable=False), + sa.Column("part_size", sa.BigInteger(), nullable=False), + sa.Column("backend_upload_id", sa.String(length=255), nullable=False), + sa.Column("status", sa.String(length=16), nullable=False), + sa.Column("public", sa.Boolean(), server_default=sa.false(), nullable=False), + sa.Column("expires_at", sa.DateTime(timezone=True), nullable=False), + sa.Column("file_id", sa.Uuid(), nullable=True), + sa.PrimaryKeyConstraint("id", name=op.f("pk_file_storage_upload_session")), + ) + op.create_index( + "ix_file_storage_upload_session_created_by", + "file_storage_upload_session", + ["created_by"], + unique=False, + ) + op.create_index( + "ix_file_storage_upload_session_status_expires", + "file_storage_upload_session", + ["status", "expires_at"], + unique=False, + ) + op.create_index( + op.f("ix_file_storage_upload_session_tenant_id"), + "file_storage_upload_session", + ["tenant_id"], + unique=False, + ) + op.create_index( + "ix_file_storage_upload_session_tenant_key", + "file_storage_upload_session", + ["tenant_id", "key"], + unique=True, + ) + with op.batch_alter_table("file_storage_stored_file") as batch: + batch.alter_column("checksum_sha256", existing_type=sa.String(length=64), nullable=True) + + +def downgrade() -> None: + op.execute( + "UPDATE file_storage_stored_file SET checksum_sha256 = '' WHERE checksum_sha256 IS NULL" + ) + with op.batch_alter_table("file_storage_stored_file") as batch: + batch.alter_column("checksum_sha256", existing_type=sa.String(length=64), nullable=False) + op.drop_index( + "ix_file_storage_upload_session_tenant_key", table_name="file_storage_upload_session" + ) + op.drop_index( + op.f("ix_file_storage_upload_session_tenant_id"), table_name="file_storage_upload_session" + ) + op.drop_index( + "ix_file_storage_upload_session_status_expires", table_name="file_storage_upload_session" + ) + op.drop_index( + "ix_file_storage_upload_session_created_by", table_name="file_storage_upload_session" + ) + op.drop_table("file_storage_upload_session") diff --git a/modules/file_storage/file_storage/backends/filesystem.py b/modules/file_storage/file_storage/backends/filesystem.py index a5e74c1e5..f2c7505ba 100644 --- a/modules/file_storage/file_storage/backends/filesystem.py +++ b/modules/file_storage/file_storage/backends/filesystem.py @@ -11,6 +11,7 @@ from file_storage import constants from file_storage.backends import register_backend +from file_storage.backends.filesystem_multipart import FilesystemMultipartMixin from file_storage.contracts.service import ( NotSupportedError, StorageBackendError, @@ -19,7 +20,7 @@ from file_storage.settings import FileStorageSettings -class FilesystemBackend: +class FilesystemBackend(FilesystemMultipartMixin): """Stores objects on the local filesystem under ``root``. Keys are sharded by their first two characters to keep any single diff --git a/modules/file_storage/file_storage/backends/filesystem_multipart.py b/modules/file_storage/file_storage/backends/filesystem_multipart.py new file mode 100644 index 000000000..3124faf1d --- /dev/null +++ b/modules/file_storage/file_storage/backends/filesystem_multipart.py @@ -0,0 +1,147 @@ +"""Multipart-upload half of the filesystem backend (#454). + +Parts live under ``/.multipart//`` with the part's MD5 in a +``.etag`` sidecar, so :meth:`list_parts` never re-reads gigabytes to answer. +:meth:`complete_multipart` concatenates them into the final object through a +temp file and an atomic rename, like :meth:`FilesystemBackend.put`. +""" + +from __future__ import annotations + +import contextlib +import hashlib +import re +import shutil +import uuid +from collections.abc import AsyncIterator +from pathlib import Path + +import aiofiles + +from file_storage import constants +from file_storage.contracts.service import ( + NotSupportedError, + ObjectInfo, + Part, + StorageBackendError, + StorageNotFoundError, +) + +_UPLOAD_ID = re.compile(r"^[0-9a-f]{32}$") +_ETAG_SUFFIX = ".etag" + + +class FilesystemMultipartMixin: + """Mixed into :class:`~file_storage.backends.filesystem.FilesystemBackend`.""" + + supports_direct_upload = True + root: Path + + def _resolve(self, key: str) -> Path: # provided by the concrete backend + raise NotImplementedError + + def _parts_dir(self, upload_id: str) -> Path: + # Upload ids are minted here, but they come back in a URL and a DB row: + # refuse anything that is not exactly what we issued. + if not _UPLOAD_ID.match(upload_id): + raise StorageBackendError(f"Invalid upload id: {upload_id!r}") + return self.root / constants.FS_MULTIPART_DIR / upload_id + + async def create_multipart(self, key: str, content_type: str) -> str: + self._resolve(key) # validates the key before we hand out an id + upload_id = uuid.uuid4().hex + self._parts_dir(upload_id).mkdir(parents=True, exist_ok=True) + return upload_id + + async def presign_part( + self, key: str, upload_id: str, part_no: int, ttl: int, size: int | None = None + ) -> str: + raise NotSupportedError("Filesystem parts are uploaded through the app.") + + async def presigned_put_url(self, key: str, ttl: int, content_type: str) -> str: + raise NotSupportedError("Filesystem backend cannot mint presigned URLs.") + + async def write_part( + self, + key: str, + upload_id: str, + part_no: int, + stream: AsyncIterator[bytes], + *, + max_bytes: int, + ) -> Part: + directory = self._parts_dir(upload_id) + if not directory.is_dir(): + raise StorageNotFoundError(upload_id) + target = directory / str(part_no) + tmp = directory / f"{part_no}.{uuid.uuid4().hex}.tmp" + md5 = hashlib.md5(usedforsecurity=False) + size = 0 + cleanup = [tmp] + try: + async with aiofiles.open(tmp, "wb") as fh: + async for chunk in stream: + size += len(chunk) + if size > max_bytes: + raise StorageBackendError(f"Part {part_no} exceeds {max_bytes} bytes.") + md5.update(chunk) + await fh.write(chunk) + etag = md5.hexdigest() + # Etag first: a part is "present" only once its data file is, and the + # data file is renamed last. + # The sidecar goes through its own temp name too, so a concurrent + # writer of the same part never leaves a half-written etag. + etag_tmp = directory / f"{part_no}.{uuid.uuid4().hex}.etag.tmp" + cleanup.append(etag_tmp) + etag_tmp.write_text(etag) + etag_tmp.replace(directory / f"{part_no}{_ETAG_SUFFIX}") + tmp.replace(target) + except BaseException: + for leftover in cleanup: + with contextlib.suppress(OSError): + leftover.unlink(missing_ok=True) + raise + return Part(no=part_no, etag=etag, size=size) + + async def list_parts(self, key: str, upload_id: str) -> list[Part]: + directory = self._parts_dir(upload_id) + if not directory.is_dir(): + raise StorageNotFoundError(upload_id) + parts = [] + for path in directory.iterdir(): + if not path.name.isdigit(): + continue # ``.etag`` sidecars and in-flight ``.tmp`` files + etag_file = path.with_name(path.name + _ETAG_SUFFIX) + etag = etag_file.read_text() if etag_file.exists() else "" + parts.append(Part(no=int(path.name), etag=etag, size=path.stat().st_size)) + return sorted(parts, key=lambda p: p.no) + + async def complete_multipart(self, key: str, upload_id: str, parts: list[Part]) -> None: + directory = self._parts_dir(upload_id) + path = self._resolve(key) + path.parent.mkdir(parents=True, exist_ok=True) + tmp = path.with_name(f"{path.name}.{uuid.uuid4().hex}.tmp") + try: + async with aiofiles.open(tmp, "wb") as out: + for part in sorted(parts, key=lambda p: p.no): + source = directory / str(part.no) + if not source.is_file(): + raise StorageBackendError(f"Part {part.no} is missing.") + async with aiofiles.open(source, "rb") as fh: + while chunk := await fh.read(constants.DEFAULT_CHUNK_SIZE): + await out.write(chunk) + tmp.replace(path) + except BaseException: + with contextlib.suppress(OSError): + tmp.unlink(missing_ok=True) + raise + shutil.rmtree(directory, ignore_errors=True) + + async def abort_multipart(self, key: str, upload_id: str) -> None: + shutil.rmtree(self._parts_dir(upload_id), ignore_errors=True) + + async def head(self, key: str) -> ObjectInfo: + path = self._resolve(key) + if not path.is_file(): + raise StorageNotFoundError(key) + return ObjectInfo(size=path.stat().st_size) diff --git a/modules/file_storage/file_storage/backends/s3.py b/modules/file_storage/file_storage/backends/s3.py index 1ad5cbe2d..040c8c343 100644 --- a/modules/file_storage/file_storage/backends/s3.py +++ b/modules/file_storage/file_storage/backends/s3.py @@ -13,6 +13,7 @@ from file_storage import constants from file_storage.backends import register_backend +from file_storage.backends.s3_multipart import S3MultipartMixin from file_storage.contracts.service import ( ConfigurationError, StorageBackendError, @@ -26,7 +27,7 @@ ) -class S3Backend: +class S3Backend(S3MultipartMixin): """S3-compatible object storage. Each operation opens a fresh ``aioboto3`` client context (cheap; the @@ -38,7 +39,14 @@ class S3Backend: backend_id = constants.BackendId.S3 supports_presigned_url = True - def __init__(self, *, bucket: str, region: str, client_kwargs: dict[str, Any]) -> None: + def __init__( + self, + *, + bucket: str, + region: str, + client_kwargs: dict[str, Any], + public_endpoint_url: str = "", + ) -> None: try: import aioboto3 except ImportError as exc: @@ -48,6 +56,7 @@ def __init__(self, *, bucket: str, region: str, client_kwargs: dict[str, Any]) - self.bucket = bucket self.region = region self.client_kwargs = client_kwargs + self.public_endpoint_url = public_endpoint_url def _client(self): return self._session.client("s3", **self.client_kwargs) @@ -132,7 +141,7 @@ async def exists(self, key: str) -> bool: async def presigned_get_url(self, key: str, ttl_seconds: int) -> str: try: - async with self._client() as client: + async with self._presign_client() as client: return await client.generate_presigned_url( "get_object", Params={"Bucket": self.bucket, "Key": key}, @@ -200,4 +209,5 @@ def _build(settings: FileStorageSettings) -> S3Backend: bucket=settings.s3_bucket, region=settings.s3_region, client_kwargs=client_kwargs, + public_endpoint_url=settings.s3_public_endpoint_url, ) diff --git a/modules/file_storage/file_storage/backends/s3_multipart.py b/modules/file_storage/file_storage/backends/s3_multipart.py new file mode 100644 index 000000000..8679ef550 --- /dev/null +++ b/modules/file_storage/file_storage/backends/s3_multipart.py @@ -0,0 +1,186 @@ +"""Multipart-upload half of the S3 backend (#454). + +Parts go from the browser straight to the bucket through presigned +``upload_part`` URLs, so a multi-GB file never crosses the app server. URLs +are presigned against the *browser-facing* endpoint (``s3_public_endpoint_url`` +when set): a SigV4 signature covers the host, so one signed for an internal +hostname would be rejected when the browser sent it to the public one. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta +from typing import Any + +from file_storage.contracts.service import ( + NotSupportedError, + ObjectInfo, + Part, + StorageBackendError, + StorageNotFoundError, +) + +_MAX_LIST_PAGE = 1000 + + +class S3MultipartMixin: + """Mixed into :class:`~file_storage.backends.s3.S3Backend`.""" + + supports_direct_upload = True + bucket: str + client_kwargs: dict[str, Any] + public_endpoint_url: str + + def _client(self): # provided by the concrete backend + raise NotImplementedError + + def _presign_client(self): + if not self.public_endpoint_url: + return self._client() + return self._session.client( # type: ignore[attr-defined] + "s3", **{**self.client_kwargs, "endpoint_url": self.public_endpoint_url} + ) + + async def create_multipart(self, key: str, content_type: str) -> str: + try: + async with self._client() as client: + resp = await client.create_multipart_upload( + Bucket=self.bucket, Key=key, ContentType=content_type + ) + return resp["UploadId"] + except Exception as exc: + raise StorageBackendError(f"S3 create_multipart_upload failed: {exc}") from exc + + async def presign_part( + self, key: str, upload_id: str, part_no: int, ttl: int, size: int | None = None + ) -> str: + params: dict[str, Any] = { + "Bucket": self.bucket, + "Key": key, + "UploadId": upload_id, + "PartNumber": part_no, + } + if size is not None: + # Signed, so S3 refuses a body of any other length. + params["ContentLength"] = size + return await self._presign("upload_part", params, ttl) + + async def presigned_put_url(self, key: str, ttl: int, content_type: str) -> str: + return await self._presign( + "put_object", {"Bucket": self.bucket, "Key": key, "ContentType": content_type}, ttl + ) + + async def _presign(self, operation: str, params: dict[str, Any], ttl: int) -> str: + try: + async with self._presign_client() as client: + return await client.generate_presigned_url(operation, Params=params, ExpiresIn=ttl) + except Exception as exc: + raise StorageBackendError(f"presign failed: {exc}") from exc + + async def write_part(self, key, upload_id, part_no, stream, *, max_bytes: int) -> Part: + raise NotSupportedError("S3 parts are uploaded directly with presigned URLs.") + + async def list_parts(self, key: str, upload_id: str) -> list[Part]: + parts: list[Part] = [] + marker = 0 + try: + async with self._client() as client: + while True: + resp = await client.list_parts( + Bucket=self.bucket, + Key=key, + UploadId=upload_id, + MaxParts=_MAX_LIST_PAGE, + PartNumberMarker=marker, + ) + parts.extend( + Part(no=p["PartNumber"], etag=p["ETag"], size=p["Size"]) + for p in resp.get("Parts", []) + ) + if not resp.get("IsTruncated"): + return sorted(parts, key=lambda p: p.no) + marker = resp["NextPartNumberMarker"] + except Exception as exc: + if _is_no_such_upload(exc): + raise StorageNotFoundError(upload_id) from exc + raise StorageBackendError(f"S3 list_parts failed: {exc}") from exc + + async def complete_multipart(self, key: str, upload_id: str, parts: list[Part]) -> None: + try: + async with self._client() as client: + await client.complete_multipart_upload( + Bucket=self.bucket, + Key=key, + UploadId=upload_id, + MultipartUpload={ + "Parts": [ + {"PartNumber": p.no, "ETag": p.etag} + for p in sorted(parts, key=lambda p: p.no) + ] + }, + ) + except Exception as exc: + raise StorageBackendError(f"S3 complete_multipart_upload failed: {exc}") from exc + + async def abort_multipart(self, key: str, upload_id: str) -> None: + try: + async with self._client() as client: + await client.abort_multipart_upload(Bucket=self.bucket, Key=key, UploadId=upload_id) + except Exception as exc: + if _is_no_such_upload(exc): + return + raise StorageBackendError(f"S3 abort_multipart_upload failed: {exc}") from exc + + async def head(self, key: str) -> ObjectInfo: + try: + async with self._client() as client: + resp = await client.head_object(Bucket=self.bucket, Key=key) + except Exception as exc: + if _is_no_such_key(exc): + raise StorageNotFoundError(key) from exc + raise StorageBackendError(f"S3 head_object failed: {exc}") from exc + return ObjectInfo(size=resp["ContentLength"], etag=resp.get("ETag")) + + async def list_stale_multipart(self, older_than: timedelta) -> list[tuple[str, str]]: + """``(key, upload_id)`` of in-progress uploads started before ``now - older_than``. + + For the janitor: a browser that vanished mid-upload leaves parts that + S3 bills for until someone aborts the upload. + """ + cutoff = datetime.now(UTC) - older_than + found: list[tuple[str, str]] = [] + key_marker = upload_marker = None + try: + async with self._client() as client: + while True: + kwargs: dict[str, Any] = {"Bucket": self.bucket} + if key_marker: + kwargs["KeyMarker"] = key_marker + kwargs["UploadIdMarker"] = upload_marker + resp = await client.list_multipart_uploads(**kwargs) + found.extend( + (u["Key"], u["UploadId"]) + for u in resp.get("Uploads", []) + if u["Initiated"] < cutoff + ) + if not resp.get("IsTruncated"): + return found + key_marker = resp.get("NextKeyMarker") + upload_marker = resp.get("NextUploadIdMarker") + except Exception as exc: + raise StorageBackendError(f"S3 list_multipart_uploads failed: {exc}") from exc + + +def _code(exc: Exception) -> str | None: + response = getattr(exc, "response", None) + if isinstance(response, dict): + return response.get("Error", {}).get("Code") + return None + + +def _is_no_such_upload(exc: Exception) -> bool: + return _code(exc) == "NoSuchUpload" + + +def _is_no_such_key(exc: Exception) -> bool: + return _code(exc) in {"NoSuchKey", "404", "NotFound"} diff --git a/modules/file_storage/file_storage/checksum.py b/modules/file_storage/file_storage/checksum.py new file mode 100644 index 000000000..9a490b791 --- /dev/null +++ b/modules/file_storage/file_storage/checksum.py @@ -0,0 +1,57 @@ +"""Filling in the checksum of a multipart upload after the fact (#454). + +The bytes of a multipart upload never pass through the app, so there was +nothing to hash on the way in. When ``background_tasks`` is installed, a task +streams the stored object once and writes the digest; without it the column +stays null, which every reader tolerates. +""" + +from __future__ import annotations + +import hashlib +import logging +import uuid +from collections.abc import Callable +from typing import TYPE_CHECKING, Any + +from file_storage import constants + +if TYPE_CHECKING: + from fastapi import FastAPI + from sqlalchemy.ext.asyncio import AsyncSession + + from file_storage.contracts.service import StorageBackend + +_logger = logging.getLogger(__name__) + + +async def hash_object( + backend: StorageBackend, key: str, *, on_chunk: Callable[[], Any] | None = None +) -> str: + """SHA-256 of ``key``'s bytes, streamed in bounded chunks.""" + digest = hashlib.sha256() + async for chunk in await backend.get(key): + digest.update(chunk) + if on_chunk is not None: + on_chunk() + return digest.hexdigest() + + +def schedule_checksum(app: FastAPI, db: AsyncSession, file_id: uuid.UUID, tenant_id: str) -> bool: + """Queue ``file_storage.compute_checksum`` once ``db`` commits, if a worker can run it. + + Soft coupling: ``app.state.background_tasks`` existing is the whole test, and + the task is addressed by name, so this module never imports the task module + (and with it, Celery). Returns whether a task was queued. + """ + if getattr(app.state, "background_tasks", None) is None: + return False + try: + from background_tasks import enqueue_after_commit + + enqueue_after_commit(db, constants.Task.COMPUTE_CHECKSUM, str(file_id), tenant_id) + except Exception: + # The file is stored and usable; a missing checksum is not worth a 500. + _logger.exception("file_storage.checksum_enqueue_failed file=%s", file_id) + return False + return True diff --git a/modules/file_storage/file_storage/constants.py b/modules/file_storage/file_storage/constants.py index 2242d55ef..62ff947da 100644 --- a/modules/file_storage/file_storage/constants.py +++ b/modules/file_storage/file_storage/constants.py @@ -46,12 +46,21 @@ # or silently stripped by enough proxies that it cannot be relied on. PATH_FILES_BULK_DELETE: Final = "/files/bulk-delete" +# Resumable / direct uploads (#454). ``PATH_UPLOAD`` is the one-shot multipart +# form; these are the session flow for files too big for it. +PATH_UPLOAD_SESSIONS: Final = "/uploads" +PATH_UPLOAD_SESSION: Final = "/uploads/{upload_id}" +PATH_UPLOAD_PART_URLS: Final = "/uploads/{upload_id}/part-urls" +PATH_UPLOAD_PART: Final = "/uploads/{upload_id}/parts/{part_no}" +PATH_UPLOAD_COMPLETE: Final = "/uploads/{upload_id}/complete" + # ── Inertia page names ─────────────────────────────────────────────── PAGE_BROWSE: Final = "FileStorage/Browse" # ── Database ───────────────────────────────────────────────────────── SCHEMA_NAME: Final = "file_storage" TABLE_STORED_FILE: Final = "file_storage_stored_file" +TABLE_UPLOAD_SESSION: Final = "file_storage_upload_session" # ── i18n ───────────────────────────────────────────────────────────── LOCALE_NAMESPACE: Final = MODULE_NAME @@ -93,6 +102,27 @@ class ErrorCode: NOT_FOUND: Final = "file_storage.not_found" BACKEND_ERROR: Final = "file_storage.backend_error" BAD_IMAGE: Final = "file_storage.bad_image" + QUOTA_EXCEEDED: Final = "file_storage.quota_exceeded" + DIRECT_UNSUPPORTED: Final = "file_storage.direct_upload_unsupported" + BAD_PART: Final = "file_storage.bad_part" + PARTS_MISMATCH: Final = "file_storage.parts_mismatch" + SIZE_MISMATCH: Final = "file_storage.size_mismatch" + SESSION_CLOSED: Final = "file_storage.upload_session_closed" + + +class UploadStatus: + """Lifecycle of an ``UploadSession`` row.""" + + ACTIVE: Final = "active" + COMPLETED: Final = "completed" + ABORTED: Final = "aborted" + + +class Task: + """Celery task names (``tasks.py``; only loaded when background_tasks is installed).""" + + REAP_STALE_UPLOADS: Final = "file_storage.reap_stale_uploads" + COMPUTE_CHECKSUM: Final = "file_storage.compute_checksum" class I18nKey: @@ -103,6 +133,12 @@ class I18nKey: ERR_BAD_TYPE: Final = "file_storage.errors.bad_type" ERR_BACKEND: Final = "file_storage.errors.backend_error" ERR_BAD_IMAGE: Final = "file_storage.errors.bad_image" + ERR_QUOTA: Final = "file_storage.errors.quota_exceeded" + ERR_DIRECT_UNSUPPORTED: Final = "file_storage.errors.direct_upload_unsupported" + ERR_BAD_PART: Final = "file_storage.errors.bad_part" + ERR_PARTS_MISMATCH: Final = "file_storage.errors.parts_mismatch" + ERR_SIZE_MISMATCH: Final = "file_storage.errors.size_mismatch" + ERR_SESSION_CLOSED: Final = "file_storage.errors.upload_session_closed" # ── Defaults ───────────────────────────────────────────────────────── @@ -116,6 +152,22 @@ class I18nKey: DEFAULT_CHUNK_SIZE: Final = 64 * 1024 # 64 KB SPOOL_MAX_SIZE_BYTES: Final = 10 * 1024 * 1024 # 10 MB before disk-spill +# ── Resumable uploads (#454) ───────────────────────────────────────── +MIB: Final = 1024 * 1024 +DEFAULT_DIRECT_UPLOAD_THRESHOLD_BYTES: Final = 64 * MIB +DEFAULT_MAX_DIRECT_UPLOAD_BYTES: Final = 10 * 1024 * MIB # 10 GiB +DEFAULT_PART_SIZE_BYTES: Final = 16 * MIB +# S3's hard ceiling; ``part_size`` is grown until the upload fits under it. +MAX_UPLOAD_PARTS: Final = 10_000 +# S3 rejects non-final parts below 5 MiB. +MIN_PART_SIZE_BYTES: Final = 5 * MIB +DEFAULT_UPLOAD_SESSION_TTL_SECONDS: Final = 24 * 3600 +DEFAULT_UPLOAD_SESSION_RETENTION_DAYS: Final = 7 +# Headroom over ``part_size`` for the filesystem part PUT's body-limit rule. +PART_BODY_SLACK_BYTES: Final = 64 * 1024 +# The directory under the filesystem root that holds in-flight parts. +FS_MULTIPART_DIR: Final = ".multipart" + # Shown where a file has no uploader at all — rows predating authenticated # uploads. The label is resolved server-side, so this is the only copy. UNKNOWN_UPLOADER: Final = "—" diff --git a/modules/file_storage/file_storage/contracts/__init__.py b/modules/file_storage/file_storage/contracts/__init__.py index c5ba7f4e4..8a4c2c5a7 100644 --- a/modules/file_storage/file_storage/contracts/__init__.py +++ b/modules/file_storage/file_storage/contracts/__init__.py @@ -4,7 +4,10 @@ from file_storage.contracts.schemas import StoredFileListOut, StoredFileOut from file_storage.contracts.service import ( ConfigurationError, + DirectUploadBackend, NotSupportedError, + ObjectInfo, + Part, StorageBackend, StorageBackendError, StorageError, @@ -13,9 +16,12 @@ __all__ = [ "ConfigurationError", + "DirectUploadBackend", "FileDeleted", "FileUploaded", "NotSupportedError", + "ObjectInfo", + "Part", "StorageBackend", "StorageBackendError", "StorageError", diff --git a/modules/file_storage/file_storage/contracts/schemas.py b/modules/file_storage/file_storage/contracts/schemas.py index 24977fa4e..6eca2ed9c 100644 --- a/modules/file_storage/file_storage/contracts/schemas.py +++ b/modules/file_storage/file_storage/contracts/schemas.py @@ -20,7 +20,10 @@ class StoredFileOut(SQLModel): content_type: str size_bytes: int backend: str - checksum_sha256: str + checksum_sha256: str | None = Field( + default=None, + description="SHA-256, or null for a multipart upload not yet hashed.", + ) uploaded_by: str | None = Field( default=None, description="User id from AuditMixin.created_by — populated by the audit listener.", @@ -72,3 +75,64 @@ class StoredFileListOut(SQLModel): total: int page: int per_page: int + + +class UploadSessionCreate(SQLModel): + """Body for POST /api/file-storage/uploads.""" + + filename: str = Field(min_length=1, max_length=255) + content_type: str = Field(default="application/octet-stream", max_length=128) + total_size: int = Field(gt=0) + public: StrictBool = False + + +class PartOut(SQLModel): + """One part the backend holds.""" + + no: int + etag: str + size: int + + +class UploadSessionOut(SQLModel): + """A resumable upload: what it is, and which parts the backend already has.""" + + id: uuid.UUID + filename: str + content_type: str + total_size: int + part_size: int + total_parts: int + status: str + public: bool + expires_at: datetime + parts: list[PartOut] = Field(default_factory=list) + file: StoredFileOut | None = Field( + default=None, description="The stored file, once the session is completed." + ) + + +class PartUrlsRequest(SQLModel): + """Body for POST /uploads/{id}/part-urls.""" + + parts: list[int] = Field(min_length=1, max_length=100) + + +class PartUrlsOut(SQLModel): + """Where to PUT each requested part; ``direct`` is false for app-relayed parts.""" + + urls: dict[int, str] + direct: bool + + +class CompletePart(SQLModel): + no: int = Field(alias="n", ge=1) + etag: str + + model_config = ConfigDict(populate_by_name=True) + + +class UploadCompleteRequest(SQLModel): + """Body for POST /uploads/{id}/complete: the parts the client believes it sent.""" + + parts: list[CompletePart] = Field(default_factory=list, max_length=10_000) diff --git a/modules/file_storage/file_storage/contracts/service.py b/modules/file_storage/file_storage/contracts/service.py index e818efc8a..5fda50960 100644 --- a/modules/file_storage/file_storage/contracts/service.py +++ b/modules/file_storage/file_storage/contracts/service.py @@ -13,6 +13,7 @@ from __future__ import annotations from collections.abc import AsyncIterator +from dataclasses import dataclass from typing import Protocol, runtime_checkable @@ -95,3 +96,74 @@ async def presigned_get_url(self, key: str, ttl_seconds: int) -> str: :class:`NotSupportedError`. The endpoint will not call this on backends with ``supports_presigned_url=False``. """ + + +@dataclass(frozen=True) +class Part: + """One uploaded part of a multipart upload, as the backend reports it.""" + + no: int + etag: str + size: int + + +@dataclass(frozen=True) +class ObjectInfo: + """What :meth:`DirectUploadBackend.head` knows about a stored object.""" + + size: int + etag: str | None = None + + +class DirectUploadBackend(Protocol): + """Optional capability: resumable multipart uploads (#454). + + Backends advertise it with ``supports_direct_upload = True``; the service + reads the flag with ``getattr(..., False)`` so third-party backends written + before this existed keep working. The backend's :meth:`list_parts` is the + source of truth for what has been uploaded — the database never mirrors it. + + A backend that can mint URLs the browser PUTs to directly (S3) implements + :meth:`presign_part`; one that cannot (filesystem) raises + :class:`NotSupportedError` from it, and parts travel through the app + instead, landing in :meth:`write_part`. + """ + + supports_direct_upload: bool + + async def create_multipart(self, key: str, content_type: str) -> str: + """Start an upload; return the backend's upload id.""" + + async def presign_part( + self, key: str, upload_id: str, part_no: int, ttl: int, size: int | None = None + ) -> str: + """URL the browser PUTs one part to, length-bound to ``size`` where the store can. + + :class:`NotSupportedError` if the backend has no such URLs. + """ + + async def write_part( + self, + key: str, + upload_id: str, + part_no: int, + stream: AsyncIterator[bytes], + *, + max_bytes: int, + ) -> Part: + """Store one part streamed through the app. :class:`NotSupportedError` if presigned.""" + + async def list_parts(self, key: str, upload_id: str) -> list[Part]: + """Parts received so far, ascending by number.""" + + async def complete_multipart(self, key: str, upload_id: str, parts: list[Part]) -> None: + """Assemble ``parts`` into the object at ``key``.""" + + async def abort_multipart(self, key: str, upload_id: str) -> None: + """Discard the upload and its parts. No-op if already gone.""" + + async def presigned_put_url(self, key: str, ttl: int, content_type: str) -> str: + """URL for a single-request direct PUT. :class:`NotSupportedError` if none.""" + + async def head(self, key: str) -> ObjectInfo: + """Size of the stored object; :class:`StorageNotFoundError` if absent.""" diff --git a/modules/file_storage/file_storage/csp.py b/modules/file_storage/file_storage/csp.py new file mode 100644 index 000000000..83ac3a23a --- /dev/null +++ b/modules/file_storage/file_storage/csp.py @@ -0,0 +1,40 @@ +"""CSP ``connect-src`` origins for direct-to-bucket uploads (#454).""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from urllib.parse import urlsplit + +from file_storage import constants + +if TYPE_CHECKING: + from fastapi import FastAPI + + from file_storage.settings import FileStorageSettings + + +def s3_connect_origins(settings: FileStorageSettings) -> list[str]: + """Origins a browser PUTs S3 parts to; empty unless the backend is S3. + + A custom endpoint (MinIO, R2) contributes its own origin, taken from + ``s3_public_endpoint_url`` when set. With none, botocore presigns virtual- + hosted AWS URLs, so the bucket's own hostnames are allowed instead. + """ + if settings.backend != constants.BackendId.S3: + return [] + endpoint = settings.presign_endpoint_url() + if endpoint: + parts = urlsplit(endpoint) + return [f"{parts.scheme}://{parts.netloc}"] if parts.scheme and parts.netloc else [] + bucket = settings.s3_bucket + return [ + f"https://{bucket}.s3.{settings.s3_region}.amazonaws.com", + f"https://{bucket}.s3.amazonaws.com", + ] + + +def connect_src_provider(app: FastAPI) -> list[tuple[str, str]]: + services = getattr(app.state, "file_storage", None) + if services is None: + return [] + return [("connect-src", origin) for origin in s3_connect_origins(services.settings)] diff --git a/modules/file_storage/file_storage/endpoints/upload_errors.py b/modules/file_storage/file_storage/endpoints/upload_errors.py new file mode 100644 index 000000000..d30be2e8c --- /dev/null +++ b/modules/file_storage/file_storage/endpoints/upload_errors.py @@ -0,0 +1,92 @@ +"""Error builders shared by the upload-session endpoints (#454).""" + +from __future__ import annotations + +from fastapi import HTTPException, status +from fastapi.responses import JSONResponse + +from file_storage import constants +from file_storage.format import format_bytes +from file_storage.service import FileTooLargeError +from file_storage.upload_support import QuotaExceededError, SizeMismatchError +from file_storage.uploads import UploadSessionService + + +def error(t, status_code: int, code: str, key: str, **fmt) -> HTTPException: + extra = {k: v for k, v in fmt.items() if k.startswith("_")} + params = {k: v for k, v in fmt.items() if not k.startswith("_")} + detail = {"code": code, "message": t.t(key, **params)} + detail.update({k.lstrip("_"): v for k, v in extra.items()}) + return HTTPException(status_code=status_code, detail=detail) + + +def committed(failure: HTTPException) -> JSONResponse: + """``failure`` as a returned response, so the request's writes still commit. + + A raised exception rolls the transaction back; callers use this where the + service has already changed state (aborted a session) that must stick. + """ + return JSONResponse(status_code=failure.status_code, content={"detail": failure.detail}) + + +def not_found(t) -> HTTPException: + return error( + t, + status.HTTP_404_NOT_FOUND, + constants.ErrorCode.NOT_FOUND, + constants.I18nKey.ERR_NOT_FOUND, + ) + + +def bad_part(t) -> HTTPException: + return error( + t, + status.HTTP_400_BAD_REQUEST, + constants.ErrorCode.BAD_PART, + constants.I18nKey.ERR_BAD_PART, + ) + + +def quota(t) -> HTTPException: + return error( + t, + status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + constants.ErrorCode.QUOTA_EXCEEDED, + constants.I18nKey.ERR_QUOTA, + ) + + +def too_large(limit: int, t) -> HTTPException: + return error( + t, + status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + constants.ErrorCode.TOO_LARGE, + constants.I18nKey.ERR_TOO_LARGE, + max_size=format_bytes(limit), + _max_bytes=limit, + ) + + +def closed(t) -> HTTPException: + return error( + t, + status.HTTP_409_CONFLICT, + constants.ErrorCode.SESSION_CLOSED, + constants.I18nKey.ERR_SESSION_CLOSED, + ) + + +def rejection(exc: Exception, sessions: UploadSessionService, t) -> HTTPException: + if isinstance(exc, FileTooLargeError): + return too_large(sessions.settings.max_direct_upload_bytes, t) + if isinstance(exc, QuotaExceededError): + return quota(t) + assert isinstance(exc, SizeMismatchError) + return error( + t, + status.HTTP_422_UNPROCESSABLE_ENTITY, + constants.ErrorCode.SIZE_MISMATCH, + constants.I18nKey.ERR_SIZE_MISMATCH, + expected=format_bytes(exc.expected), + actual=format_bytes(exc.actual), + ) diff --git a/modules/file_storage/file_storage/endpoints/uploads.py b/modules/file_storage/file_storage/endpoints/uploads.py new file mode 100644 index 000000000..14ceed955 --- /dev/null +++ b/modules/file_storage/file_storage/endpoints/uploads.py @@ -0,0 +1,234 @@ +"""Resumable multipart upload endpoints (#454). + +``POST /uploads`` opens a session; the client then sends parts — to the +presigned URLs ``part-urls`` returns (S3), or to ``PUT /uploads/{id}/parts/{n}`` +(filesystem) — and finishes with ``complete``. ``GET /uploads/{id}`` lists what +the backend holds, which is how a reloaded page resumes. +""" + +from __future__ import annotations + +import uuid + +from fastapi import APIRouter, Depends, Request, Response, status +from fastapi.responses import JSONResponse +from simple_module_core.events import EventBus +from simple_module_hosting.i18n_deps import TranslatorDep +from simple_module_hosting.permissions import RequiresPermission + +from file_storage import constants +from file_storage.checksum import schedule_checksum +from file_storage.contracts.events import FileUploaded +from file_storage.contracts.schemas import ( + PartOut, + PartUrlsOut, + PartUrlsRequest, + UploadCompleteRequest, + UploadSessionCreate, + UploadSessionOut, +) +from file_storage.deps import get_event_bus, get_file_storage_service +from file_storage.endpoints.upload_errors import ( + bad_part, + closed, + committed, + error, + not_found, + quota, + rejection, + too_large, +) +from file_storage.models import UploadSession +from file_storage.service import ContentTypeNotAllowedError, FileStorageService, FileTooLargeError +from file_storage.upload_support import ( + DirectUploadUnsupportedError, + InvalidPartError, + PartsMismatchError, + QuotaExceededError, + SizeMismatchError, + UploadClosedError, + UploadNotFoundError, + total_parts, +) +from file_storage.uploads import UploadSessionService + +router = APIRouter(dependencies=[Depends(RequiresPermission(constants.Permission.UPLOAD))]) + + +async def get_upload_service( + files: FileStorageService = Depends(get_file_storage_service), +) -> UploadSessionService: + return UploadSessionService(files) + + +Sessions = Depends(get_upload_service) + + +def _out(session: UploadSession, parts=(), file=None) -> UploadSessionOut: + return UploadSessionOut( + id=session.id, + filename=session.filename, + content_type=session.content_type, + total_size=session.total_size, + part_size=session.part_size, + total_parts=total_parts(session), + status=session.status, + public=session.public, + expires_at=session.expires_at, + parts=[PartOut(no=p.no, etag=p.etag, size=p.size) for p in parts], + file=file, + ) + + +@router.post( + constants.PATH_UPLOAD_SESSIONS, + response_model=UploadSessionOut, + status_code=status.HTTP_201_CREATED, +) +async def create_session( + body: UploadSessionCreate, t: TranslatorDep, sessions: UploadSessionService = Sessions +) -> UploadSessionOut: + try: + session = await sessions.create( + filename=body.filename, + content_type=body.content_type, + total_size=body.total_size, + public=body.public, + ) + except DirectUploadUnsupportedError as exc: + raise error( + t, + status.HTTP_501_NOT_IMPLEMENTED, + constants.ErrorCode.DIRECT_UNSUPPORTED, + constants.I18nKey.ERR_DIRECT_UNSUPPORTED, + ) from exc + except FileTooLargeError as exc: + limit = sessions.settings.max_direct_upload_bytes + raise too_large(limit, t) from exc + except ContentTypeNotAllowedError as exc: + raise error( + t, + status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, + constants.ErrorCode.BAD_TYPE, + constants.I18nKey.ERR_BAD_TYPE, + ) from exc + except QuotaExceededError as exc: + raise quota(t) from exc + return _out(session) + + +async def _open( + sessions: UploadSessionService, upload_id: uuid.UUID, t +) -> UploadSession | JSONResponse: + try: + return await sessions.get_open(upload_id) + except UploadNotFoundError as exc: + raise not_found(t) from exc + except UploadClosedError: + # Opening an expired session aborts it; that abort must commit. + return committed(closed(t)) + + +@router.get(constants.PATH_UPLOAD_SESSION, response_model=UploadSessionOut) +async def get_session( + upload_id: uuid.UUID, + t: TranslatorDep, + sessions: UploadSessionService = Sessions, +) -> UploadSessionOut: + try: + session = await sessions.get(upload_id) + except UploadNotFoundError as exc: + raise not_found(t) from exc + return _out(session, await sessions.parts(session)) + + +@router.post(constants.PATH_UPLOAD_PART_URLS, response_model=PartUrlsOut) +async def part_urls( + upload_id: uuid.UUID, + body: PartUrlsRequest, + t: TranslatorDep, + sessions: UploadSessionService = Sessions, +) -> PartUrlsOut | JSONResponse: + session = await _open(sessions, upload_id, t) + if isinstance(session, JSONResponse): + return session + try: + urls, direct = await sessions.part_urls(session, body.parts) + except InvalidPartError as exc: + raise bad_part(t) from exc + return PartUrlsOut(urls=urls, direct=direct) + + +@router.put(constants.PATH_UPLOAD_PART, response_model=PartOut) +async def put_part( + upload_id: uuid.UUID, + part_no: int, + request: Request, + t: TranslatorDep, + sessions: UploadSessionService = Sessions, +) -> PartOut | JSONResponse: + session = await _open(sessions, upload_id, t) + if isinstance(session, JSONResponse): + return session + try: + part = await sessions.write_part(session, part_no, request.stream()) + except InvalidPartError as exc: + raise bad_part(t) from exc + return PartOut(no=part.no, etag=part.etag, size=part.size) + + +@router.post(constants.PATH_UPLOAD_COMPLETE, response_model=UploadSessionOut) +async def complete_session( + upload_id: uuid.UUID, + body: UploadCompleteRequest, + request: Request, + t: TranslatorDep, + sessions: UploadSessionService = Sessions, + bus: EventBus = Depends(get_event_bus), +) -> UploadSessionOut | JSONResponse: + try: + session = await sessions.get(upload_id) + out, created = await sessions.complete(session, {p.no: p.etag for p in body.parts}) + except UploadNotFoundError as exc: + raise not_found(t) from exc + except UploadClosedError: + return committed(closed(t)) + except PartsMismatchError as exc: + raise error( + t, + status.HTTP_409_CONFLICT, + constants.ErrorCode.PARTS_MISMATCH, + constants.I18nKey.ERR_PARTS_MISMATCH, + _missing=exc.missing, + _mismatched=exc.mismatched, + ) from exc + except (FileTooLargeError, QuotaExceededError, SizeMismatchError) as exc: + # The service has already deleted the object and closed the session. + # Returned, not raised: an exception rolls the request's transaction + # back, which would reopen a session whose object is gone. + return committed(rejection(exc, sessions, t)) + + if created: + schedule_checksum(request.app, sessions.db, out.id, session.tenant_id) + await bus.publish( + FileUploaded( + file_id=out.id, + key=out.key, + backend=out.backend, + size_bytes=out.size_bytes, + uploaded_by=out.uploaded_by, + ) + ) + return _out(session, file=out) + + +@router.delete(constants.PATH_UPLOAD_SESSION, status_code=status.HTTP_204_NO_CONTENT) +async def abort_session( + upload_id: uuid.UUID, t: TranslatorDep, sessions: UploadSessionService = Sessions +) -> Response: + try: + session = await sessions.get(upload_id) + except UploadNotFoundError as exc: + raise not_found(t) from exc + await sessions.abort(session) + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/modules/file_storage/file_storage/endpoints/views.py b/modules/file_storage/file_storage/endpoints/views.py index fb073c366..c9b0d8804 100644 --- a/modules/file_storage/file_storage/endpoints/views.py +++ b/modules/file_storage/file_storage/endpoints/views.py @@ -93,6 +93,14 @@ async def browse( "used_bytes": aggregates.used_bytes, "quota_bytes": settings.quota_bytes, "max_file_size_bytes": settings.max_file_size_bytes, + # Above this the page uploads in resumable parts; null when the + # backend cannot (a third-party one without multipart support). + "direct_upload_threshold_bytes": ( + settings.direct_upload_threshold_bytes + if getattr(service.backend, "supports_direct_upload", False) + else None + ), + "max_direct_upload_bytes": settings.max_direct_upload_bytes, "allowed_content_types": settings.allowed_content_types, }, ) diff --git a/modules/file_storage/file_storage/locales/en.json b/modules/file_storage/file_storage/locales/en.json index 5f8b9e10b..c67f9f74f 100644 --- a/modules/file_storage/file_storage/locales/en.json +++ b/modules/file_storage/file_storage/locales/en.json @@ -66,7 +66,13 @@ "too_large": "File exceeds the {max_size} limit for a single upload", "bad_type": "This file type is not allowed", "backend_error": "Storage backend error", - "bad_image": "This image could not be processed" + "bad_image": "This image could not be processed", + "quota_exceeded": "This file would exceed the storage quota", + "direct_upload_unsupported": "The storage backend does not support resumable uploads", + "bad_part": "That upload part is not valid for this file", + "parts_mismatch": "The uploaded parts do not match what was sent; resume the upload to send the rest", + "size_mismatch": "The assembled file is {actual}, but {expected} was expected; upload it again", + "upload_session_closed": "This upload is no longer active; start it again" }, "filters": { "search_placeholder": "Search filenames…", @@ -87,7 +93,10 @@ "retry": "Retry", "in_progress": "Uploading {name}", "cancel": "Cancel upload", - "dismiss": "Dismiss" + "dismiss": "Dismiss", + "resume_prompt": "Partly uploaded earlier — {percent}% done", + "resume": "Resume upload", + "start_over": "Start over" }, "nav": { "files": "Files" diff --git a/modules/file_storage/file_storage/models.py b/modules/file_storage/file_storage/models.py index 630cb83f6..71a57feb0 100644 --- a/modules/file_storage/file_storage/models.py +++ b/modules/file_storage/file_storage/models.py @@ -3,10 +3,12 @@ from __future__ import annotations import uuid +from datetime import datetime import sqlalchemy as sa from simple_module_db.base import create_module_base from simple_module_db.mixins import AuditMixin, MultiTenantMixin, SoftDeleteMixin +from simple_module_db.types import AwareDateTime from sqlalchemy import Index from sqlmodel import Field @@ -38,7 +40,10 @@ class StoredFile(Base, AuditMixin, SoftDeleteMixin, MultiTenantMixin, table=True content_type: str = Field(max_length=128) size_bytes: int = Field() backend: str = Field(max_length=32) - checksum_sha256: str = Field(max_length=64) + # Null for multipart uploads until ``file_storage.compute_checksum`` has + # streamed the object (or forever, with no background_tasks): the bytes + # never pass through the app, so there is nothing to hash on the way in. + checksum_sha256: str | None = Field(default=None, max_length=64) # Opt-in anonymous serving (#353): only ``public`` rows are reachable from # ``GET {prefix}/public/{id}``. ``sa.false()`` renders as ``0`` on SQLite and # ``false`` on Postgres, which a literal ``text("0")`` default would not. @@ -56,3 +61,36 @@ class StoredFile(Base, AuditMixin, SoftDeleteMixin, MultiTenantMixin, table=True Index(f"ix_{constants.TABLE_STORED_FILE}_created_by", "created_by"), Index(f"ix_{constants.TABLE_STORED_FILE}_is_deleted", "is_deleted"), ) + + +class UploadSession(Base, AuditMixin, MultiTenantMixin, table=True): # ty: ignore[unsupported-base] + """A resumable, multipart upload in progress (#454). + + ``created_by`` is the owner: only they can resume, finish or abort it. The + backend's own part listing — not this row — says which parts have arrived, + so a lost response or a second tab can never disagree with the bucket. + """ + + __tablename__ = constants.TABLE_UPLOAD_SESSION + + id: uuid.UUID = Field(default_factory=uuid.uuid4, primary_key=True) + key: str = Field(max_length=512) + filename: str = Field(max_length=255) + content_type: str = Field(max_length=128) + total_size: int = Field(sa_column=sa.Column(sa.BigInteger(), nullable=False)) + part_size: int = Field(sa_column=sa.Column(sa.BigInteger(), nullable=False)) + backend_upload_id: str = Field(max_length=255) + status: str = Field(default=constants.UploadStatus.ACTIVE, max_length=16) + public: bool = Field( + default=False, + sa_column=sa.Column(sa.Boolean(), nullable=False, server_default=sa.false()), + ) + expires_at: datetime = Field(sa_column=sa.Column(AwareDateTime(), nullable=False)) + # Set on completion, so a retried ``complete`` answers with the same file. + file_id: uuid.UUID | None = Field(default=None, sa_column=sa.Column(sa.Uuid(), nullable=True)) + + __table_args__ = ( + Index(f"ix_{constants.TABLE_UPLOAD_SESSION}_tenant_key", "tenant_id", "key", unique=True), + Index(f"ix_{constants.TABLE_UPLOAD_SESSION}_status_expires", "status", "expires_at"), + Index(f"ix_{constants.TABLE_UPLOAD_SESSION}_created_by", "created_by"), + ) diff --git a/modules/file_storage/file_storage/module.py b/modules/file_storage/file_storage/module.py index d714106e9..58a6bb1b6 100644 --- a/modules/file_storage/file_storage/module.py +++ b/modules/file_storage/file_storage/module.py @@ -10,6 +10,7 @@ from fastapi import APIRouter from simple_module_core.audit_links import AuditLink, AuditLinkRegistry from simple_module_core.body_limits import BodyLimitRegistry +from simple_module_core.csp import CspSourceRegistry from simple_module_core.feature_flags import FeatureFlagDefinition, FeatureFlagRegistry from simple_module_core.menu import MenuItem, MenuRegistry, MenuSection from simple_module_core.module import ModuleBase, ModuleMeta @@ -18,6 +19,7 @@ from simple_module_core.tenancy import TenantRole, tenant_role from file_storage import constants +from file_storage.upload_support import choose_part_size if TYPE_CHECKING: from fastapi import FastAPI @@ -91,9 +93,11 @@ def register_settings(self, app: FastAPI) -> None: def register_routes(self, api_router: APIRouter, view_router: APIRouter) -> None: from file_storage.endpoints.api import router as api from file_storage.endpoints.public import router as public + from file_storage.endpoints.uploads import router as uploads from file_storage.endpoints.views import router as views api_router.include_router(api) + api_router.include_router(uploads) api_router.include_router(public) view_router.include_router(views) @@ -189,6 +193,26 @@ def register_body_limits(self, registry: BodyLimitRegistry) -> None: methods={"POST"}, ) + # Filesystem parts are relayed through the app: one part at most, plus + # slack. (S3 parts never reach us — they go straight to the bucket.) + registry.add_regex( + constants.ROUTE_PREFIX_API + r"/uploads/[^/]+/parts/\d+$", + lambda app: ( + choose_part_size( + app.state.file_storage.settings.max_direct_upload_bytes, + app.state.file_storage.settings.multipart_part_size_bytes, + ) + + constants.PART_BODY_SLACK_BYTES + ), + methods={"PUT"}, + ) + + def register_csp_sources(self, registry: CspSourceRegistry) -> None: + """Let the browser PUT multipart parts straight to the S3 endpoint.""" + from file_storage.csp import connect_src_provider + + registry.add_provider(connect_src_provider) + def register_feature_flags(self, registry: FeatureFlagRegistry) -> None: registry.add( FeatureFlagDefinition( diff --git a/modules/file_storage/file_storage/pages/Browse.tsx b/modules/file_storage/file_storage/pages/Browse.tsx index e10708174..8a8642953 100644 --- a/modules/file_storage/file_storage/pages/Browse.tsx +++ b/modules/file_storage/file_storage/pages/Browse.tsx @@ -29,7 +29,9 @@ function Browse() { const { can } = usePermissions(); const canUpload = can(PERMISSIONS.UPLOAD); const canDelete = can(PERMISSIONS.DELETE); - const { jobs, start, retry, cancel, dismiss, busy } = useUploadQueue(); + const { jobs, start, busy, actions } = useUploadQueue({ + directThresholdBytes: props.direct_upload_threshold_bytes, + }); const filters: FileFilters = { q: props.filters?.q ?? '', @@ -206,7 +208,7 @@ function Browse() { /> )} - + void; + retry: (id: string) => void; + resume: (id: string) => void; + startOver: (id: string) => void; + dismiss: (id: string) => void; +} + interface Props { jobs: UploadJob[]; - onCancel: (id: string) => void; - onRetry: (id: string) => void; - onDismiss: (id: string) => void; + actions: UploadActions; } /** @@ -20,7 +26,9 @@ interface Props { * the two things people do while waiting — swept the progress bars away. Here * they survive both, which is what the card's own subtitle promises. */ -export function UploadsCard({ jobs, onCancel, onRetry, onDismiss }: Props) { +export function UploadsCard({ jobs, actions }: Props) { + const { cancel: onCancel, retry: onRetry, resume: onResume } = actions; + const { startOver: onStartOver, dismiss: onDismiss } = actions; const { t } = useT(); if (jobs.length === 0) return null; @@ -46,7 +54,33 @@ export function UploadsCard({ jobs, onCancel, onRetry, onDismiss }: Props) { {job.name} - {job.status === 'error' ? ( + {job.status === 'resumable' ? ( + <> + + {t(keys.file_storage.upload.resume_prompt, { percent: job.percent })} + + + + onDismiss(job.id)} + /> + + ) : job.status === 'error' ? ( <> {failure} diff --git a/modules/file_storage/file_storage/pages/resumable-upload.ts b/modules/file_storage/file_storage/pages/resumable-upload.ts new file mode 100644 index 000000000..d84ae77f7 --- /dev/null +++ b/modules/file_storage/file_storage/pages/resumable-upload.ts @@ -0,0 +1,225 @@ +import { clearSessionId, loadSessionId, saveSessionId } from './resume-store'; +import { parseFailureReason, UploadAbortedError, UploadFailedError } from './upload-errors'; + +/** Parts in flight at once, per file. */ +export const PART_CONCURRENCY = 4; +const MAX_ATTEMPTS = 4; +const BACKOFF_MS = 500; +const BASE = '/api/file-storage/uploads'; + +export interface RemotePart { + no: number; + etag: string; + size: number; +} + +export interface RemoteSession { + id: string; + total_size: number; + part_size: number; + total_parts: number; + status: 'active' | 'completed' | 'aborted'; + parts: RemotePart[]; +} + +export interface RunOptions { + /** Called with bytes sent so far and the file's size. */ + onProgress: (sent: number, total: number) => void; + signal: AbortSignal; + /** Milliseconds before retry 1; doubles each time. Tests pass 0. */ + backoffMs?: number; +} + +async function api(method: string, url: string, body?: unknown): Promise { + let response: Response; + try { + response = await fetch(url, { + method, + headers: body === undefined ? undefined : { 'Content-Type': 'application/json' }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + } catch { + throw new UploadFailedError(undefined); + } + const text = await response.text(); + if (!response.ok) throw new UploadFailedError(response.status, parseFailureReason(text)); + return (text ? JSON.parse(text) : undefined) as T; +} + +/** The stored session for this file, if the server still has it open. */ +export async function findSession(file: File): Promise { + const id = loadSessionId(file); + if (!id) return null; + try { + const session = await api('GET', `${BASE}/${id}`); + if (session.status === 'active' && session.total_size === file.size) return session; + } catch (error) { + // A network blip says nothing about the session; keep the id for next time. + if (error instanceof UploadFailedError && error.status === undefined) return null; + } + clearSessionId(file); + return null; +} + +/** Abort the server session and forget it; best effort. */ +export async function discardSession(file: File, id: string): Promise { + clearSessionId(file); + try { + await api('DELETE', `${BASE}/${id}`); + } catch { + // The reaper collects it at expiry. + } +} + +function createSession(file: File): Promise { + return api('POST', BASE, { + filename: file.name, + content_type: file.type || 'application/octet-stream', + total_size: file.size, + }); +} + +/** PUT one blob; resolves with the ETag the store (or the app) answered. */ +function putBlob( + url: string, + blob: Blob, + signal: AbortSignal, + onProgress: (loaded: number) => void, +): Promise { + return new Promise((resolve, reject) => { + const xhr = new XMLHttpRequest(); + const onAbort = () => xhr.abort(); + signal.addEventListener('abort', onAbort); + const done = () => signal.removeEventListener('abort', onAbort); + xhr.open('PUT', url); + xhr.upload.addEventListener('progress', (event) => onProgress(event.loaded)); + xhr.addEventListener('load', () => { + done(); + if (xhr.status < 200 || xhr.status >= 300) { + reject(new UploadFailedError(xhr.status, parseFailureReason(xhr.responseText))); + return; + } + // S3 answers with an ETag header (if its CORS rule exposes it); the app's + // relay answers with the part as JSON. A missing ETag is tolerated: the + // server lists parts itself when completing. + let etag = xhr.getResponseHeader('ETag') ?? ''; + if (!etag) { + try { + etag = (JSON.parse(xhr.responseText) as { etag?: string }).etag ?? ''; + } catch { + // not JSON + } + } + resolve(etag); + }); + xhr.addEventListener('error', () => { + done(); + reject(new UploadFailedError(undefined)); + }); + xhr.addEventListener('abort', () => { + done(); + reject(new UploadAbortedError()); + }); + xhr.send(blob); + }); +} + +/** Retrying cannot help a request the server refused on its merits. */ +function isPermanent(error: unknown): boolean { + if (!(error instanceof UploadFailedError) || error.status === undefined) return false; + // 403 is a presigned URL that expired mid-retry; 408/429 are "try later". + return error.status < 500 && ![403, 408, 429].includes(error.status); +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +/** + * Upload `file` in parts through a server session, resuming `existing` if given. + * + * Parts already held by the server are skipped. Each part is retried with + * exponential backoff, asking for a fresh URL every attempt so an expired + * presign never strands it. Resolves once the server has assembled the file; + * rejects with `UploadFailedError` (session left resumable) or + * `UploadAbortedError`. + */ +export async function runResumable( + file: File, + options: RunOptions, + existing?: RemoteSession | null, +): Promise { + const session = existing ?? (await createSession(file)); + if (options.signal.aborted) { + // Cancelled while the create was in flight: the cancel found no session id + // to discard, so this is the only place that can release it. + if (!existing) await discardSession(file, session.id); + throw new UploadAbortedError(); + } + saveSessionId(file, session.id); + + const etags = new Map(session.parts.map((p) => [p.no, p.etag])); + const sent = new Map( + session.parts.map((p) => [p.no, Math.min(p.size, file.size)]), + ); + const report = () => { + let total = 0; + sent.forEach((bytes) => { + total += bytes; + }); + options.onProgress(Math.min(total, file.size), file.size); + }; + report(); + + const inner = new AbortController(); + const forward = () => inner.abort(); + options.signal.addEventListener('abort', forward); + + const pending: number[] = []; + for (let n = 1; n <= session.total_parts; n += 1) if (!etags.has(n)) pending.push(n); + + const uploadPart = async (n: number) => { + const start = (n - 1) * session.part_size; + const blob = file.slice(start, Math.min(start + session.part_size, file.size)); + for (let attempt = 0; ; attempt += 1) { + if (inner.signal.aborted) throw new UploadAbortedError(); + try { + const { urls } = await api<{ urls: Record }>( + 'POST', + `${BASE}/${session.id}/part-urls`, + { parts: [n] }, + ); + const etag = await putBlob(urls[String(n)], blob, inner.signal, (loaded) => { + sent.set(n, loaded); + report(); + }); + etags.set(n, etag); + sent.set(n, blob.size); + report(); + return; + } catch (error) { + sent.set(n, 0); + if (error instanceof UploadAbortedError || isPermanent(error)) throw error; + if (attempt + 1 >= MAX_ATTEMPTS) throw error; + await sleep((options.backoffMs ?? BACKOFF_MS) * 2 ** attempt); + } + } + }; + + const worker = async () => { + for (let n = pending.shift(); n !== undefined; n = pending.shift()) await uploadPart(n); + }; + + try { + await Promise.all(Array.from({ length: PART_CONCURRENCY }, worker)); + await api('POST', `${BASE}/${session.id}/complete`, { + parts: [...etags].filter(([, etag]) => etag).map(([n, etag]) => ({ n, etag })), + }); + } catch (error) { + // A sibling's failure aborts the rest; the user's cancel does too. + inner.abort(); + if (options.signal.aborted) throw new UploadAbortedError(); + throw error; + } finally { + options.signal.removeEventListener('abort', forward); + } + clearSessionId(file); +} diff --git a/modules/file_storage/file_storage/pages/resume-store.ts b/modules/file_storage/file_storage/pages/resume-store.ts new file mode 100644 index 000000000..ef3e30546 --- /dev/null +++ b/modules/file_storage/file_storage/pages/resume-store.ts @@ -0,0 +1,40 @@ +/** + * Remembers which server-side session belongs to which file, across reloads. + * + * A browser cannot keep a `File` handle through a reload, so resuming means + * the user selects the same file again; this is how the page recognises it. + * The key is name + size + lastModified — cheap, and wrong only for two + * different files that agree on all three. Storage may be blocked or full + * (private windows, quota), so every access is guarded and its absence just + * means "no resume", never an error. + */ + +const PREFIX = 'file_storage.upload.'; + +export function resumeKey(file: Pick): string { + return `${PREFIX}${file.name}:${file.size}:${file.lastModified}`; +} + +export function loadSessionId(file: File): string | null { + try { + return window.localStorage.getItem(resumeKey(file)); + } catch { + return null; + } +} + +export function saveSessionId(file: File, sessionId: string): void { + try { + window.localStorage.setItem(resumeKey(file), sessionId); + } catch { + // Without storage the upload still works; it just cannot be resumed. + } +} + +export function clearSessionId(file: File): void { + try { + window.localStorage.removeItem(resumeKey(file)); + } catch { + // Nothing to clear. + } +} diff --git a/modules/file_storage/file_storage/pages/types.ts b/modules/file_storage/file_storage/pages/types.ts index e78d00882..b3ea8c717 100644 --- a/modules/file_storage/file_storage/pages/types.ts +++ b/modules/file_storage/file_storage/pages/types.ts @@ -49,6 +49,9 @@ export interface BrowseProps { /** Null until an operator says what the bucket's ceiling is. */ quota_bytes: number | null; max_file_size_bytes: number; + /** Files larger than this upload in resumable parts; null when the backend cannot. */ + direct_upload_threshold_bytes: number | null; + max_direct_upload_bytes: number; /** Null means any type is accepted. */ allowed_content_types: string[] | null; } diff --git a/modules/file_storage/file_storage/pages/upload-errors.ts b/modules/file_storage/file_storage/pages/upload-errors.ts new file mode 100644 index 000000000..ef7f7fd7e --- /dev/null +++ b/modules/file_storage/file_storage/pages/upload-errors.ts @@ -0,0 +1,34 @@ +/** + * The server's own explanation for a rejected upload. + * + * "Upload failed" is true of a 413, a 415 and a dropped connection alike, and + * it tells the person holding a 40 MB file nothing they can act on. The API + * already answers with `{"detail": {"code", "message"}}`; this reads it back + * out and tolerates every shape that is not that — a proxy's HTML error page, + * an empty body, a plain-string `detail`. + */ +export function parseFailureReason(body: string): string | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(body); + } catch { + return undefined; + } + const detail = (parsed as { detail?: unknown } | null)?.detail; + if (typeof detail === 'string') return detail || undefined; + const message = (detail as { message?: unknown } | null)?.message; + return typeof message === 'string' && message ? message : undefined; +} + +/** A resumable upload that stopped, with the server's reason when it gave one. */ +export class UploadFailedError extends Error { + constructor( + readonly status: number | undefined, + readonly reason?: string, + ) { + super(reason ?? `upload failed (${status ?? 'network'})`); + } +} + +/** The user (or a sibling part's failure) stopped the upload on purpose. */ +export class UploadAbortedError extends Error {} diff --git a/modules/file_storage/file_storage/pages/upload-queue.ts b/modules/file_storage/file_storage/pages/upload-queue.ts index 87e4ceeba..56e7aa351 100644 --- a/modules/file_storage/file_storage/pages/upload-queue.ts +++ b/modules/file_storage/file_storage/pages/upload-queue.ts @@ -2,6 +2,12 @@ import { router } from '@inertiajs/react'; import { useCallback, useRef, useState } from 'react'; import { RELOAD_PROPS, ROUTES } from './constants'; +import type { RemoteSession } from './resumable-upload'; +import { discardSession, findSession, runResumable } from './resumable-upload'; +import { loadSessionId } from './resume-store'; +import { parseFailureReason, UploadAbortedError, UploadFailedError } from './upload-errors'; + +export { parseFailureReason }; export interface UploadJob { id: string; @@ -9,12 +15,18 @@ export interface UploadJob { size: number; /** 0–100. Stays at 100 while the server finishes writing to the backend. */ percent: number; - status: 'uploading' | 'done' | 'error'; + /** `resumable`: a partly-sent copy of this file is on the server, awaiting a choice. */ + status: 'uploading' | 'done' | 'error' | 'resumable'; /** Why it failed, in the server's words. Undefined for a transport failure. */ reason?: string; } -type Outcome = 'done' | 'error' | 'canceled'; +type Outcome = 'done' | 'error' | 'canceled' | 'paused'; + +export interface UploadQueueOptions { + /** Files larger than this go through a resumable session; null/undefined: never. */ + directThresholdBytes?: number | null; +} let counter = 0; function nextId(): string { @@ -29,28 +41,6 @@ function nextId(): string { // swamp the server or the browser's per-origin connection limit. const UPLOAD_CONCURRENCY = 4; -/** - * The server's own explanation for a rejected upload. - * - * "Upload failed" is true of a 413, a 415 and a dropped connection alike, and - * it tells the person holding a 40 MB file nothing they can act on. The API - * already answers with `{"detail": {"code", "message"}}`; this reads it back - * out and tolerates every shape that is not that — a proxy's HTML error page, - * an empty body, a plain-string `detail`. - */ -export function parseFailureReason(body: string): string | undefined { - let parsed: unknown; - try { - parsed = JSON.parse(body); - } catch { - return undefined; - } - const detail = (parsed as { detail?: unknown } | null)?.detail; - if (typeof detail === 'string') return detail || undefined; - const message = (detail as { message?: unknown } | null)?.message; - return typeof message === 'string' && message ? message : undefined; -} - /** * Uploads files with bounded concurrency, reporting byte progress per file. * @@ -62,16 +52,62 @@ export function parseFailureReason(body: string): string | undefined { * can be retried from the row itself — the alternative is asking the person to * find the file on disk again to answer a transient 502. */ -export function useUploadQueue() { +export function useUploadQueue({ directThresholdBytes }: UploadQueueOptions = {}) { const [jobs, setJobs] = useState([]); const files = useRef(new Map()); const requests = useRef(new Map()); + // Resumable jobs: one controller per running upload, and the server session + // a job is waiting on while the user decides between resuming and starting over. + const controllers = useRef(new Map()); + const waiting = useRef(new Map()); const patch = useCallback((id: string, changes: Partial) => { setJobs((current) => current.map((j) => (j.id === id ? { ...j, ...changes } : j))); }, []); - const upload = useCallback( + const uploadInParts = useCallback( + async (jobId: string, file: File, session: RemoteSession | null): Promise => { + const controller = new AbortController(); + controllers.current.set(jobId, controller); + patch(jobId, { status: 'uploading', reason: undefined }); + try { + await runResumable( + file, + { + signal: controller.signal, + onProgress: (sent, total) => + patch(jobId, { percent: Math.round((sent / total) * 100) }), + }, + session, + ); + patch(jobId, { percent: 100, status: 'done' }); + return 'done'; + } catch (error) { + if (error instanceof UploadAbortedError) return 'canceled'; + const reason = error instanceof UploadFailedError ? error.reason : undefined; + patch(jobId, { status: 'error', reason }); + return 'error'; + } finally { + controllers.current.delete(jobId); + } + }, + [patch], + ); + + /** A copy of this file is already on the server: let the user pick before sending more. */ + const maybePause = useCallback( + async (jobId: string, file: File): Promise => { + const session = await findSession(file); + if (!session || session.parts.length === 0) return session; + const sent = session.parts.reduce((sum, part) => sum + part.size, 0); + waiting.current.set(jobId, session); + patch(jobId, { status: 'resumable', percent: Math.round((sent / file.size) * 100) }); + return 'paused'; + }, + [patch], + ); + + const uploadOnce = useCallback( (jobId: string, file: File) => new Promise((resolve) => { const form = new FormData(); @@ -111,6 +147,18 @@ export function useUploadQueue() { [patch], ); + const upload = useCallback( + async (jobId: string, file: File, resumeAutomatically = false): Promise => { + if (directThresholdBytes && file.size > directThresholdBytes) { + const found = resumeAutomatically ? await findSession(file) : await maybePause(jobId, file); + if (found === 'paused') return 'paused'; + return uploadInParts(jobId, file, found); + } + return uploadOnce(jobId, file); + }, + [directThresholdBytes, maybePause, uploadInParts, uploadOnce], + ); + // Completed rows disappear once the reloaded table can *show* the real // record — dropping them before the reply lands leaves a gap where the // table still holds the pre-upload list, and a first upload into an @@ -170,15 +218,52 @@ export function useUploadQueue() { const file = files.current.get(id); if (!file) return false; patch(id, { percent: 0, status: 'uploading', reason: undefined }); - const outcome = await upload(id, file); + // A failed resumable upload picks up where it stopped, without asking. + const outcome = await upload(id, file, true); settleBatch(outcome === 'done' ? 1 : 0); return outcome === 'done'; }, [patch, upload, settleBatch], ); + const finishResumed = useCallback( + async (id: string, run: (file: File) => Promise): Promise => { + const file = files.current.get(id); + if (!file) return false; + const outcome = await run(file); + settleBatch(outcome === 'done' ? 1 : 0); + return outcome === 'done'; + }, + [settleBatch], + ); + + /** Continue the partly-sent copy found on the server. */ + const resume = useCallback( + (id: string) => { + const session = waiting.current.get(id) ?? null; + waiting.current.delete(id); + return finishResumed(id, (file) => uploadInParts(id, file, session)); + }, + [finishResumed, uploadInParts], + ); + + /** Throw the partly-sent copy away and send the file again from the beginning. */ + const startOver = useCallback( + (id: string) => { + const session = waiting.current.get(id); + waiting.current.delete(id); + patch(id, { percent: 0, status: 'uploading' }); + return finishResumed(id, async (file) => { + if (session) await discardSession(file, session.id); + return uploadInParts(id, file, null); + }); + }, + [finishResumed, patch, uploadInParts], + ); + const forget = useCallback((id: string) => { files.current.delete(id); + waiting.current.delete(id); setJobs((current) => current.filter((j) => j.id !== id)); }, []); @@ -186,6 +271,12 @@ export function useUploadQueue() { const cancel = useCallback( (id: string) => { requests.current.get(id)?.abort(); + // A cancelled resumable upload is abandoned, not paused: free the server's parts. + const controller = controllers.current.get(id); + const file = files.current.get(id); + const sessionId = file && controller ? loadSessionId(file) : null; + controller?.abort(); + if (file && sessionId) void discardSession(file, sessionId); forget(id); }, [forget], @@ -195,5 +286,6 @@ export function useUploadQueue() { // a second state field could disagree with the rows on screen. const busy = jobs.some((j) => j.status === 'uploading'); - return { jobs, start, retry, cancel, dismiss: forget, busy }; + const actions = { cancel, retry, resume, startOver, dismiss: forget }; + return { jobs, start, retry, cancel, resume, startOver, dismiss: forget, actions, busy }; } diff --git a/modules/file_storage/file_storage/service.py b/modules/file_storage/file_storage/service.py index 0b5ecbb47..e17a8ff51 100644 --- a/modules/file_storage/file_storage/service.py +++ b/modules/file_storage/file_storage/service.py @@ -122,7 +122,7 @@ async def _hashing_stream() -> AsyncIterator[bytes]: sha.update(chunk) yield chunk - key = _generate_key(tenant_id, upload.filename or "file") + key = generate_key(tenant_id, upload.filename or "file") await self.backend.put( key, _hashing_stream(), @@ -287,7 +287,7 @@ async def drop_object(self, row: StoredFile) -> None: await thumbnails.delete_variants(self.backend, row.key) -def _generate_key(tenant_id: str, filename: str) -> str: +def generate_key(tenant_id: str, filename: str) -> str: """Build a tenant-prefixed, date-sharded, collision-proof key. The ``{tenant_id}/`` prefix keeps each tenant's objects in a disjoint diff --git a/modules/file_storage/file_storage/serving.py b/modules/file_storage/file_storage/serving.py index 3316cb0e7..99e852ea5 100644 --- a/modules/file_storage/file_storage/serving.py +++ b/modules/file_storage/file_storage/serving.py @@ -16,7 +16,10 @@ def etag_for(row: StoredFile, suffix: str = "") -> str: - return f'"{row.checksum_sha256}{suffix}"' + # A multipart upload has no checksum until the background job fills it in; + # id + size still changes whenever the bytes can. + tag = row.checksum_sha256 or f"{row.id.hex}-{row.size_bytes}" + return f'"{tag}{suffix}"' def not_found(t) -> HTTPException: diff --git a/modules/file_storage/file_storage/settings.py b/modules/file_storage/file_storage/settings.py index ad3dedc9d..e85b7535f 100644 --- a/modules/file_storage/file_storage/settings.py +++ b/modules/file_storage/file_storage/settings.py @@ -56,6 +56,15 @@ class FileStorageSettings(DbBackedSettings): json_schema_extra={"group": "S3"}, description="Custom endpoint for MinIO / R2. Blank uses AWS default.", ) + s3_public_endpoint_url: str = Field( + default="", + json_schema_extra={"group": "S3"}, + description=( + "Browser-facing S3 endpoint used to presign direct part uploads, when it " + "differs from s3_endpoint_url (e.g. an internal MinIO hostname). Blank " + "falls back to s3_endpoint_url. Its origin is added to CSP connect-src." + ), + ) s3_signature_version: str = Field( default="s3v4", json_schema_extra={"group": "S3"}, @@ -81,6 +90,33 @@ class FileStorageSettings(DbBackedSettings): description="Total storage the browse screen measures usage against. None = unmeasured.", ) + # Resumable / direct uploads (#454) + direct_upload_threshold_bytes: int = Field( + default=constants.DEFAULT_DIRECT_UPLOAD_THRESHOLD_BYTES, + gt=0, + description="Files larger than this upload in parts through a resumable session.", + ) + max_direct_upload_bytes: int = Field( + default=constants.DEFAULT_MAX_DIRECT_UPLOAD_BYTES, + gt=0, + description="Largest file the resumable session flow accepts.", + ) + multipart_part_size_bytes: int = Field( + default=constants.DEFAULT_PART_SIZE_BYTES, + ge=constants.MIN_PART_SIZE_BYTES, + description="Preferred part size; grown automatically to stay under 10,000 parts.", + ) + upload_session_ttl_seconds: int = Field( + default=constants.DEFAULT_UPLOAD_SESSION_TTL_SECONDS, + gt=0, + description="How long an unfinished upload session stays resumable.", + ) + upload_session_retention_days: int = Field( + default=constants.DEFAULT_UPLOAD_SESSION_RETENTION_DAYS, + ge=0, + description="How long finished (completed or aborted) upload session rows are kept.", + ) + @field_validator("s3_signature_version") @classmethod def _known_signature_version(cls, value: str) -> str: @@ -111,6 +147,10 @@ def _validate_backend_config(self) -> FileStorageSettings: raise ValueError(f"S3 backend selected but {', '.join(missing)} is not set.") return self + def presign_endpoint_url(self) -> str: + """The endpoint browsers reach: ``s3_public_endpoint_url``, else ``s3_endpoint_url``.""" + return self.s3_public_endpoint_url or self.s3_endpoint_url + def resolved_fs_root(self) -> Path: """Return ``fs_root_path`` as an absolute Path.""" return Path(self.fs_root_path).expanduser().resolve() diff --git a/modules/file_storage/file_storage/tasks.py b/modules/file_storage/file_storage/tasks.py new file mode 100644 index 000000000..45da6d54a --- /dev/null +++ b/modules/file_storage/file_storage/tasks.py @@ -0,0 +1,171 @@ +"""Celery tasks for file_storage (#454). + +Imported only when ``background_tasks`` is installed (its Celery app +autodiscovers ``.tasks``), which is why Celery is imported here and +nowhere else in the module. Everything the tasks need from ``background_tasks`` +is imported inside the task bodies. + +- ``file_storage.reap_stale_uploads`` (hourly beat): aborts upload sessions past + their deadline and, on S3, multipart uploads nobody tracks any more. +- ``file_storage.compute_checksum``: streams a multipart-uploaded object once + and records its SHA-256. +""" + +from __future__ import annotations + +import asyncio +import logging +import re +import uuid +from datetime import UTC, datetime, timedelta + +from celery import shared_task + +from file_storage import constants + +logger = logging.getLogger(__name__) + +BEAT_SCHEDULE = { + "file-storage-reap-stale-uploads": { + "task": constants.Task.REAP_STALE_UPLOADS, + "schedule": 3600.0, + }, +} + +_REAP_BATCH = 500 + + +def _backend_and_settings(): + from background_tasks import settings_for + + from file_storage.backends import build_backend + from file_storage.settings import FileStorageSettings + + settings = settings_for(FileStorageSettings, constants.MODULE_NAME) + return build_backend(settings), settings + + +@shared_task(name=constants.Task.REAP_STALE_UPLOADS) +def reap_stale_uploads() -> dict[str, int]: + """Abort expired sessions, purge old finished rows, then orphaned S3 uploads (> 2 x TTL).""" + from background_tasks.sync_db import sync_session + from simple_module_db import all_tenants + from sqlalchemy import delete, select + + from file_storage.models import UploadSession + + backend, settings = _backend_and_settings() + if not getattr(backend, "supports_direct_upload", False): + return {"sessions": 0, "orphans": 0, "purged": 0} + + now = datetime.now(UTC) + with sync_session() as db, all_tenants(): + # Finished rows keep nothing alive (the backend already let go of them); + # they only linger for support questions, so drop them after a window. + purged = db.execute( + delete(UploadSession).where( + UploadSession.status != constants.UploadStatus.ACTIVE, + UploadSession.expires_at + < now - timedelta(days=settings.upload_session_retention_days), + ) + ).rowcount + expired = list( + db.execute( + select(UploadSession) + .where( + UploadSession.status == constants.UploadStatus.ACTIVE, + UploadSession.expires_at <= now, + ) + .limit(_REAP_BATCH) + ).scalars() + ) + targets = [(s.key, s.backend_upload_id) for s in expired] + orphan_age = timedelta(seconds=2 * settings.upload_session_ttl_seconds) + failed = asyncio.run(_abort_all(backend, targets)) + for session in expired: + if (session.key, session.backend_upload_id) not in failed: + session.status = constants.UploadStatus.ABORTED + # Autoflush has marked the sessions aborted above, so what remains active + # is exactly what must not be touched. + tracked = set( + db.execute( + select(UploadSession.backend_upload_id).where( + UploadSession.status == constants.UploadStatus.ACTIVE + ) + ).scalars() + ) + orphans = asyncio.run(_abort_orphans(backend, orphan_age, tracked)) + reaped = len(expired) - len(failed) + logger.info( + "file_storage.reaped sessions=%d orphans=%d purged=%d", reaped, orphans, purged or 0 + ) + return {"sessions": reaped, "orphans": orphans, "purged": purged or 0} + + +async def _abort_all(backend, targets: list[tuple[str, str]]) -> set[tuple[str, str]]: + """Abort each ``(key, upload_id)``; those that failed (retried next hour).""" + failed: set[tuple[str, str]] = set() + for key, upload_id in targets: + try: + await backend.abort_multipart(key, upload_id) + except Exception: + logger.exception("file_storage.reap_failed key=%s upload=%s", key, upload_id) + failed.add((key, upload_id)) + return failed + + +# Keys this module mints: ``///
/<32 hex>``. +_OWN_KEY = re.compile(r"^[^/]+/\d{4}/\d{2}/\d{2}/[0-9a-f]{32}(\.[^/]*)?$") + + +async def _abort_orphans(backend, older_than: timedelta, tracked: set[str]) -> int: + """Abort multipart uploads that no session row accounts for. + + A bucket may be shared with other applications, and the listing is bucket- + wide, so an upload is only touched if its key has the shape this module + mints *and* no still-active session owns its upload id. Everything else is + somebody else's, or in use. + """ + lister = getattr(backend, "list_stale_multipart", None) + if lister is None: + return 0 + stale = [ + (key, upload_id) + for key, upload_id in await lister(older_than) + if _OWN_KEY.match(key) and upload_id not in tracked + ] + return len(stale) - len(await _abort_all(backend, stale)) + + +@shared_task(name=constants.Task.COMPUTE_CHECKSUM, stuck_after=4 * 3600) +def compute_checksum(file_id: str, tenant_id: str) -> str | None: + """Fill in ``StoredFile.checksum_sha256`` for a multipart upload.""" + from background_tasks import heartbeat + from background_tasks.sync_db import sync_session + from simple_module_db import tenant_context + + from file_storage.checksum import hash_object + from file_storage.contracts.service import StorageNotFoundError + from file_storage.models import StoredFile + + backend, _ = _backend_and_settings() + target = uuid.UUID(file_id) + with tenant_context(tenant_id): + # Two short transactions around the hash: a multi-GB read must not hold + # a database connection open. + with sync_session() as db: + row = db.get(StoredFile, target) + if row is None or row.checksum_sha256: + return None + key = row.key + try: + digest = asyncio.run(hash_object(backend, key, on_chunk=heartbeat)) + except StorageNotFoundError: + # Deleted since the upload finished: nothing to hash, nothing to retry. + logger.warning("file_storage.checksum_object_missing file=%s", file_id) + return None + with sync_session() as db: + row = db.get(StoredFile, target) + if row is not None and not row.checksum_sha256: + row.checksum_sha256 = digest + return digest diff --git a/modules/file_storage/file_storage/upload_guards.py b/modules/file_storage/file_storage/upload_guards.py new file mode 100644 index 000000000..909eb4409 --- /dev/null +++ b/modules/file_storage/file_storage/upload_guards.py @@ -0,0 +1,117 @@ +"""Cleanup and quota guards for upload sessions (#454).""" + +from __future__ import annotations + +import logging +import zlib +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING, cast + +from sqlalchemy import delete, func, select, text + +from file_storage import constants +from file_storage.contracts.service import StorageError +from file_storage.models import StoredFile, UploadSession +from file_storage.scope import owning_tenant +from file_storage.upload_support import DirectUploadUnsupportedError, QuotaExceededError + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + from file_storage.contracts.service import DirectUploadBackend + from file_storage.service import FileStorageService + from file_storage.settings import FileStorageSettings + +_logger = logging.getLogger(__name__) + +# Namespace for the quota advisory lock (spells "FSTO"); the tenant's crc32 is added. +_QUOTA_LOCK_KEY = 0x4653544F << 32 + + +class UploadGuards: + """Mixed into :class:`~file_storage.uploads.UploadSessionService`.""" + + files: FileStorageService + db: AsyncSession + settings: FileStorageSettings + + @property + def backend(self) -> DirectUploadBackend: + backend = self.files.backend + if not getattr(backend, "supports_direct_upload", False): + raise DirectUploadUnsupportedError(backend.backend_id) + return cast("DirectUploadBackend", backend) + + async def _abort_backend(self, key: str, upload_id: str) -> None: + try: + await self.backend.abort_multipart(key, upload_id) + except StorageError: + _logger.exception("file_storage.abort_failed key=%s upload=%s", key, upload_id) + + async def _drop_object(self, key: str) -> None: + try: + await self.files.backend.delete(key) + except StorageError: + _logger.exception("file_storage.cleanup_failed key=%s", key) + + async def _reject(self, session: UploadSession) -> None: + """Delete the assembled object and close the session.""" + await self._drop_object(session.key) + session.status = constants.UploadStatus.ABORTED + await self.db.flush() + + async def purge_finished(self, *, owner: str | None) -> int: + """Delete ``owner``'s completed/aborted rows past the retention window.""" + if not owner: + return 0 + cutoff = datetime.now(UTC) - timedelta(days=self.settings.upload_session_retention_days) + result = await self.db.execute( + delete(UploadSession).where( + UploadSession.created_by == owner, + UploadSession.status != constants.UploadStatus.ACTIVE, + UploadSession.expires_at < cutoff, + ) + ) + return result.rowcount or 0 + + async def _serialize_reservations(self) -> None: + """Make concurrent session openers take turns on Postgres. + + Each opener sums stored + reserved bytes and then inserts its own + reservation; under READ COMMITTED, N openers all read the same sum and + all pass. A transaction-scoped advisory lock (held until this request + commits, which is when the new reservation becomes visible) closes the + window. SQLite serialises writers already. + """ + bind = self.db.get_bind() + if bind.dialect.name != "postgresql": + return + await self.db.execute( + text("SELECT pg_advisory_xact_lock(:key)"), + {"key": _QUOTA_LOCK_KEY + zlib.crc32(owning_tenant(self.db).encode())}, + ) + + async def _check_quota(self, size: int, *, reserve: bool) -> None: + """Refuse ``size`` bytes that would push stored (+ reserved) bytes past the quota. + + Read live, not from the browse screen's cached totals: a cache a minute + stale would let a burst of sessions each see an empty bucket. With + ``reserve``, sessions still open count at their declared size, so opening + many small-looking sessions cannot overbook the quota. + """ + quota = self.settings.quota_bytes + if quota is None: + return + if reserve: + await self._serialize_reservations() + stored = await self.db.scalar(select(func.coalesce(func.sum(StoredFile.size_bytes), 0))) + reserved = 0 + if reserve: + reserved = await self.db.scalar( + select(func.coalesce(func.sum(UploadSession.total_size), 0)).where( + UploadSession.status == constants.UploadStatus.ACTIVE, + UploadSession.expires_at > datetime.now(UTC), + ) + ) + if (stored or 0) + (reserved or 0) + size > quota: + raise QuotaExceededError(str(quota)) diff --git a/modules/file_storage/file_storage/upload_support.py b/modules/file_storage/file_storage/upload_support.py new file mode 100644 index 000000000..01350ea51 --- /dev/null +++ b/modules/file_storage/file_storage/upload_support.py @@ -0,0 +1,74 @@ +"""Errors and part arithmetic for resumable uploads (#454).""" + +from __future__ import annotations + +import math +import uuid +from typing import TYPE_CHECKING + +from file_storage import constants + +if TYPE_CHECKING: + from file_storage.models import UploadSession + + +class UploadNotFoundError(Exception): + """No such session for this user in this tenant (also: someone else's).""" + + +class UploadClosedError(Exception): + """The session is completed, aborted or past its deadline.""" + + +class DirectUploadUnsupportedError(Exception): + """The active backend cannot do multipart uploads.""" + + +class QuotaExceededError(Exception): + """The file would not fit in the configured storage quota.""" + + +class InvalidPartError(Exception): + """A part number outside the file, or a part of the wrong size.""" + + +class PartsMismatchError(Exception): + """The backend does not hold the parts the client claims to have sent.""" + + def __init__(self, missing: list[int], mismatched: list[int]) -> None: + super().__init__(f"missing={missing} mismatched={mismatched}") + self.missing = missing + self.mismatched = mismatched + + +class SizeMismatchError(Exception): + """The assembled object is not the size the session promised.""" + + def __init__(self, expected: int, actual: int) -> None: + super().__init__(f"expected {expected} bytes, assembled {actual}") + self.expected = expected + self.actual = actual + + +def choose_part_size(total_size: int, preferred: int) -> int: + """``preferred``, grown (in whole MiB) until the file needs <= 10,000 parts.""" + needed = math.ceil(total_size / constants.MAX_UPLOAD_PARTS) + size = max(preferred, needed) + return math.ceil(size / constants.MIB) * constants.MIB + + +def total_parts(session: UploadSession) -> int: + return math.ceil(session.total_size / session.part_size) + + +def expected_part_size(session: UploadSession, part_no: int) -> int: + if part_no < total_parts(session): + return session.part_size + return session.total_size - (total_parts(session) - 1) * session.part_size + + +def part_path(upload_id: uuid.UUID, part_no: int) -> str: + """The app route that relays a part, for backends that cannot presign.""" + return constants.ROUTE_PREFIX_API + constants.PATH_UPLOAD_PART.format( + upload_id=upload_id, part_no=part_no + ) diff --git a/modules/file_storage/file_storage/uploads.py b/modules/file_storage/file_storage/uploads.py new file mode 100644 index 000000000..c9cf613bf --- /dev/null +++ b/modules/file_storage/file_storage/uploads.py @@ -0,0 +1,281 @@ +"""Resumable multipart upload sessions (#454). + +A browser uploading a multi-GB file cannot do it in one request: a dropped +connection loses everything, and a proxy's body limit refuses it outright. A +session splits the file into parts the client sends (straight to S3, or through +the app for the filesystem) and can resume after a reload. The backend's own +part listing is the truth about what arrived; the ``UploadSession`` row records +only what was promised — key, size, owner, deadline. + +Reuses the one-shot upload's rules (content type, tenant-prefixed key, owning +tenant, compensation on DB failure) so a file that arrives this way is +indistinguishable from one posted to ``/upload`` — except that nothing hashed +it on the way through, so its checksum starts null. +""" + +from __future__ import annotations + +import uuid +from collections.abc import AsyncIterator +from datetime import UTC, datetime, timedelta +from typing import TYPE_CHECKING + +from simple_module_db.listeners import current_user_id +from sqlalchemy import select + +from file_storage import constants, queries +from file_storage.contracts.schemas import StoredFileOut +from file_storage.contracts.service import ( + NotSupportedError, + Part, + StorageNotFoundError, +) +from file_storage.models import StoredFile, UploadSession +from file_storage.scope import owning_tenant +from file_storage.service import FileStorageService, FileTooLargeError, generate_key +from file_storage.upload_guards import UploadGuards +from file_storage.upload_support import ( + InvalidPartError, + PartsMismatchError, + QuotaExceededError, + SizeMismatchError, + UploadClosedError, + UploadNotFoundError, + choose_part_size, + expected_part_size, + part_path, + total_parts, +) + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession + + +def _etag(value: str) -> str: + return value.strip('"') + + +class UploadSessionService(UploadGuards): + """Creates, resumes, completes and aborts a user's upload sessions.""" + + def __init__(self, files: FileStorageService) -> None: + self.files = files + self.db: AsyncSession = files.db + self.settings = files.settings + + # ── Lifecycle ──────────────────────────────────────────────────── + + async def create( + self, *, filename: str, content_type: str, total_size: int, public: bool = False + ) -> UploadSession: + backend = self.backend + self.files._check_content_type(content_type) + if total_size > self.settings.max_direct_upload_bytes: + raise FileTooLargeError( + f"Upload exceeds {self.settings.max_direct_upload_bytes} bytes." + ) + tenant_id = owning_tenant(self.db) + # Housekeeping that needs no worker: without background_tasks nothing + # else ever reaps, so a user's abandoned uploads go when they start one. + # Done first so their reservations stop counting against the quota. + owner = current_user_id.get() + await self.abort_expired(owner=owner) + await self.purge_finished(owner=owner) + await self._check_quota(total_size, reserve=True) + + key = generate_key(tenant_id, filename) + upload_id = await backend.create_multipart(key, content_type) + try: + session = UploadSession( + tenant_id=tenant_id, + key=key, + filename=filename, + content_type=content_type, + total_size=total_size, + part_size=choose_part_size(total_size, self.settings.multipart_part_size_bytes), + backend_upload_id=upload_id, + public=public, + expires_at=datetime.now(UTC) + + timedelta(seconds=self.settings.upload_session_ttl_seconds), + ) + self.db.add(session) + await self.db.flush() + except Exception: + await self._abort_backend(key, upload_id) + raise + return session + + async def get(self, upload_id: uuid.UUID) -> UploadSession: + """The caller's own session. Others' ids miss exactly like unknown ones.""" + owner = current_user_id.get() + stmt = select(UploadSession).where( + UploadSession.id == upload_id, UploadSession.created_by == owner + ) + session = (await self.db.execute(stmt)).scalar_one_or_none() if owner else None + if session is None: + raise UploadNotFoundError(str(upload_id)) + return session + + async def get_open(self, upload_id: uuid.UUID) -> UploadSession: + """:meth:`get`, refusing a finished or expired session.""" + return await self._require_open(await self.get(upload_id)) + + async def _require_open(self, session: UploadSession) -> UploadSession: + if session.status != constants.UploadStatus.ACTIVE: + raise UploadClosedError(session.status) + if session.expires_at <= datetime.now(UTC): + await self.abort(session) + raise UploadClosedError(constants.UploadStatus.ABORTED) + return session + + async def _lock(self, session: UploadSession) -> None: + """Re-read ``session`` under a row lock, held until this request commits. + + Complete and abort are the two transitions out of ``active``; without + the lock two concurrent completes both assemble the object and record + two files (and publish twice), and an abort can delete parts a complete + is reading. The loser waits, then sees the winner's status. + """ + await self.db.refresh(session, with_for_update=True) + + async def parts(self, session: UploadSession) -> list[Part]: + if session.status != constants.UploadStatus.ACTIVE: + return [] + try: + return await self.backend.list_parts(session.key, session.backend_upload_id) + except StorageNotFoundError: + return [] + + async def abort(self, session: UploadSession) -> None: + await self._lock(session) + if session.status == constants.UploadStatus.ACTIVE: + await self._abort_backend(session.key, session.backend_upload_id) + session.status = constants.UploadStatus.ABORTED + await self.db.flush() + + async def abort_expired(self, *, owner: str | None) -> int: + """Abort ``owner``'s sessions past their deadline; how many went.""" + if not owner: + return 0 + stmt = select(UploadSession).where( + UploadSession.created_by == owner, + UploadSession.status == constants.UploadStatus.ACTIVE, + UploadSession.expires_at <= datetime.now(UTC), + ) + expired = list((await self.db.execute(stmt)).scalars()) + for session in expired: + await self.abort(session) + return len(expired) + + # ── Parts ──────────────────────────────────────────────────────── + + def _check_part_no(self, session: UploadSession, part_no: int) -> None: + if not 1 <= part_no <= total_parts(session): + raise InvalidPartError(f"part {part_no} outside 1..{total_parts(session)}") + + async def part_urls( + self, session: UploadSession, numbers: list[int] + ) -> tuple[dict[int, str], bool]: + """``({part: url}, direct)``; ``direct`` is false when parts go through the app.""" + for number in numbers: + self._check_part_no(session, number) + ttl = self.settings.s3_presign_ttl_seconds + try: + return { + n: await self.backend.presign_part( + session.key, session.backend_upload_id, n, ttl, expected_part_size(session, n) + ) + for n in numbers + }, True + except NotSupportedError: + return {n: part_path(session.id, n) for n in numbers}, False + + async def write_part( + self, session: UploadSession, part_no: int, body: AsyncIterator[bytes] + ) -> Part: + """Relay one part through the app (backends that cannot presign).""" + self._check_part_no(session, part_no) + limit = expected_part_size(session, part_no) + seen = 0 + + async def _bounded() -> AsyncIterator[bytes]: + nonlocal seen + async for chunk in body: + seen += len(chunk) + if seen > limit: + raise InvalidPartError(f"part {part_no} exceeds {limit} bytes") + yield chunk + + part = await self.backend.write_part( + session.key, session.backend_upload_id, part_no, _bounded(), max_bytes=limit + ) + if part.size != limit: + raise InvalidPartError(f"part {part_no} is {part.size} bytes, expected {limit}") + return part + + # ── Completion ─────────────────────────────────────────────────── + + async def complete( + self, session: UploadSession, claimed: dict[int, str] + ) -> tuple[StoredFileOut, bool]: + """Assemble the object and record the file; ``(file, created)``. + + ``created`` is false when a retried request finds the session already + completed, so the caller does not publish ``FileUploaded`` twice. + """ + await self._lock(session) + if session.status == constants.UploadStatus.COMPLETED and session.file_id: + existing = await self.files.get(session.file_id) + return StoredFileOut.model_validate(queries.to_out_dict(existing)), False + await self._require_open(session) + + backend = self.backend + held = {p.no: p for p in await self.parts(session)} + wanted = range(1, total_parts(session) + 1) + missing = [n for n in wanted if n not in held] + mismatched = [ + n for n, etag in claimed.items() if n in held and _etag(held[n].etag) != _etag(etag) + ] + [n for n in wanted if n in held and held[n].size != expected_part_size(session, n)] + if missing or mismatched or set(held) - set(wanted): + raise PartsMismatchError(missing, sorted(set(mismatched))) + + await backend.complete_multipart( + session.key, session.backend_upload_id, [held[n] for n in wanted] + ) + info = await backend.head(session.key) + if info.size != session.total_size: + await self._reject(session) + raise SizeMismatchError(session.total_size, info.size) + # The declared size is only a claim; the stored object is the fact. Limits + # are re-checked against it, in the transaction that records the file. + max_bytes = self.settings.max_direct_upload_bytes + if info.size > max_bytes: + await self._reject(session) + raise FileTooLargeError(f"Upload exceeds {max_bytes} bytes.") + try: + await self._check_quota(info.size, reserve=False) + except QuotaExceededError: + await self._reject(session) + raise + + try: + row = StoredFile( + tenant_id=session.tenant_id, + key=session.key, + filename=session.filename, + content_type=session.content_type, + size_bytes=info.size, + backend=self.files.backend.backend_id, + checksum_sha256=None, + public=session.public, + ) + self.db.add(row) + await self.db.flush() + await self.db.refresh(row) + session.status = constants.UploadStatus.COMPLETED + session.file_id = row.id + await self.db.flush() + except Exception: + await self._drop_object(session.key) + raise + return StoredFileOut.model_validate(queries.to_out_dict(row)), True diff --git a/modules/file_storage/tests-js/resumable-queue.test.ts b/modules/file_storage/tests-js/resumable-queue.test.ts new file mode 100644 index 000000000..62b6d39d0 --- /dev/null +++ b/modules/file_storage/tests-js/resumable-queue.test.ts @@ -0,0 +1,177 @@ +import { act, renderHook, waitFor } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +vi.mock('@inertiajs/react', () => ({ + router: { + reload: vi.fn((options?: { onFinish?: () => void }) => options?.onFinish?.()), + }, +})); + +const runResumable = vi.fn(); +const findSession = vi.fn(); +const discardSession = vi.fn(); +vi.mock('../file_storage/pages/resumable-upload', () => ({ + runResumable: (...args: unknown[]) => runResumable(...args), + findSession: (...args: unknown[]) => findSession(...args), + discardSession: (...args: unknown[]) => discardSession(...args), +})); + +import { UploadAbortedError, UploadFailedError } from '../file_storage/pages/upload-errors'; +import { useUploadQueue } from '../file_storage/pages/upload-queue'; + +const THRESHOLD = 100; + +function file(size: number, name = 'movie.bin') { + return new File([new Uint8Array(size)], name, { lastModified: 1 }); +} + +class NeverXhr { + static opened = 0; + upload = { addEventListener() {} }; + open() { + NeverXhr.opened += 1; + } + addEventListener() {} + send() {} + abort() {} +} + +beforeEach(() => { + runResumable.mockReset().mockResolvedValue(undefined); + findSession.mockReset().mockResolvedValue(null); + discardSession.mockReset().mockResolvedValue(undefined); + NeverXhr.opened = 0; + vi.stubGlobal('XMLHttpRequest', NeverXhr); +}); + +afterEach(() => vi.unstubAllGlobals()); + +describe('the resumable path in useUploadQueue', () => { + test('files at or under the threshold still use the one-shot form post', async () => { + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + act(() => { + void result.current.start([file(THRESHOLD)]); + }); + await waitFor(() => expect(NeverXhr.opened).toBe(1)); + expect(runResumable).not.toHaveBeenCalled(); + }); + + test('no threshold (backend cannot do it) never uses sessions', async () => { + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: null })); + act(() => { + void result.current.start([file(10_000)]); + }); + await waitFor(() => expect(NeverXhr.opened).toBe(1)); + expect(runResumable).not.toHaveBeenCalled(); + }); + + test('a file over the threshold uploads in parts and reports progress', async () => { + runResumable.mockImplementation(async (_file, options) => { + options.onProgress(50, 200); + }); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + let batch!: Promise<{ uploaded: number; failed: string[] }>; + act(() => { + batch = result.current.start([file(200)]); + }); + await act(async () => { + expect(await batch).toEqual({ uploaded: 1, failed: [] }); + }); + expect(NeverXhr.opened).toBe(0); + expect(result.current.jobs).toEqual([]); + }); + + test('a failed part upload keeps the server reason and retry resumes without asking', async () => { + runResumable.mockRejectedValueOnce(new UploadFailedError(413, 'over the quota')); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + let batch!: Promise<{ uploaded: number; failed: string[] }>; + act(() => { + batch = result.current.start([file(200)]); + }); + await act(async () => { + expect((await batch).failed).toEqual(['movie.bin']); + }); + expect(result.current.jobs[0]).toMatchObject({ status: 'error', reason: 'over the quota' }); + + // The stored session has parts, yet a retry must not stop to prompt. + findSession.mockResolvedValue({ id: 's', parts: [{ no: 1, etag: 'e', size: 10 }] }); + await act(async () => { + expect(await result.current.retry(result.current.jobs[0].id)).toBe(true); + }); + expect(runResumable).toHaveBeenLastCalledWith(expect.any(File), expect.anything(), { + id: 's', + parts: [{ no: 1, etag: 'e', size: 10 }], + }); + }); + + test('re-selecting a partly sent file offers to resume instead of sending', async () => { + findSession.mockResolvedValue({ + id: 'sess', + parts: [{ no: 1, etag: 'e', size: 50 }], + }); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + await act(async () => { + await result.current.start([file(200)]); + }); + + expect(runResumable).not.toHaveBeenCalled(); + expect(result.current.jobs[0]).toMatchObject({ status: 'resumable', percent: 25 }); + expect(result.current.busy).toBe(false); + + await act(async () => { + expect(await result.current.resume(result.current.jobs[0].id)).toBe(true); + }); + expect(runResumable.mock.calls[0][2]).toMatchObject({ id: 'sess' }); + }); + + test('start over discards the stored session and begins a fresh one', async () => { + findSession.mockResolvedValue({ id: 'sess', parts: [{ no: 1, etag: 'e', size: 50 }] }); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + await act(async () => { + await result.current.start([file(200)]); + }); + + await act(async () => { + await result.current.startOver(result.current.jobs[0].id); + }); + + expect(discardSession).toHaveBeenCalledWith(expect.any(File), 'sess'); + expect(runResumable.mock.calls[0][2]).toBeNull(); + }); + + test('a session with no parts yet just continues, no prompt', async () => { + findSession.mockResolvedValue({ id: 'sess', parts: [] }); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + await act(async () => { + await result.current.start([file(200)]); + }); + expect(runResumable.mock.calls[0][2]).toMatchObject({ id: 'sess' }); + }); + + test('cancelling aborts the parts and frees the server session', async () => { + let signal!: AbortSignal; + runResumable.mockImplementation( + (_f: File, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + signal = options.signal; + signal.addEventListener('abort', () => reject(new UploadAbortedError())); + }), + ); + window.localStorage.setItem(`file_storage.upload.movie.bin:200:1`, 'sess'); + const { result } = renderHook(() => useUploadQueue({ directThresholdBytes: THRESHOLD })); + let batch!: Promise<{ uploaded: number; failed: string[] }>; + act(() => { + batch = result.current.start([file(200)]); + }); + await waitFor(() => expect(runResumable).toHaveBeenCalled()); + + await act(async () => { + result.current.cancel(result.current.jobs[0].id); + expect(await batch).toEqual({ uploaded: 0, failed: [] }); + }); + + expect(signal.aborted).toBe(true); + expect(discardSession).toHaveBeenCalledWith(expect.any(File), 'sess'); + expect(result.current.jobs).toEqual([]); + }); +}); diff --git a/modules/file_storage/tests-js/resumable-upload.test.ts b/modules/file_storage/tests-js/resumable-upload.test.ts new file mode 100644 index 000000000..6d71ebf94 --- /dev/null +++ b/modules/file_storage/tests-js/resumable-upload.test.ts @@ -0,0 +1,280 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; + +import { + discardSession, + findSession, + PART_CONCURRENCY, + runResumable, +} from '../file_storage/pages/resumable-upload'; +import { resumeKey } from '../file_storage/pages/resume-store'; +import { UploadAbortedError, UploadFailedError } from '../file_storage/pages/upload-errors'; + +const PART = 10; + +interface PutCall { + url: string; + size: number; +} + +/** A tiny in-memory server: sessions over fetch, part PUTs over XHR. */ +class FakeServer { + calls: string[] = []; + puts: PutCall[] = []; + held = new Map(); + /** Status to answer each PUT with, per part, consumed in order; default 200. */ + putStatuses = new Map(); + inFlight = 0; + maxInFlight = 0; + completeBody: unknown = null; + completeStatus = 200; + total = 0; + + fetch = async (url: string, init: RequestInit = {}): Promise => { + const method = init.method ?? 'GET'; + this.calls.push(`${method} ${url}`); + const json = (status: number, body: unknown) => new Response(JSON.stringify(body), { status }); + if (method === 'POST' && url === '/api/file-storage/uploads') { + const body = JSON.parse(init.body as string); + this.total = body.total_size; + return json(201, this.session()); + } + if (method === 'GET') return json(200, this.session()); + if (method === 'DELETE') return new Response(null, { status: 204 }); + if (url.endsWith('/part-urls')) { + const { parts } = JSON.parse(init.body as string); + return json(200, { + direct: true, + urls: Object.fromEntries( + parts.map((n: number) => [String(n), `https://s3.test/part/${n}`]), + ), + }); + } + if (url.endsWith('/complete')) { + this.completeBody = JSON.parse(init.body as string); + return json(this.completeStatus, { + detail: { code: 'x', message: 'parts do not match' }, + }); + } + throw new Error(`unexpected ${method} ${url}`); + }; + + session() { + return { + id: 'sess-1', + total_size: this.total, + part_size: PART, + total_parts: Math.ceil(this.total / PART), + status: 'active', + parts: [...this.held].map(([no, etag]) => ({ no, etag, size: PART })), + }; + } +} + +let server: FakeServer; + +class FakeXhr { + status = 0; + responseText = ''; + private handlers: Record void> = {}; + private url = ''; + private aborted = false; + upload = { addEventListener: (_t: string, _fn: unknown) => {} }; + + open(_method: string, url: string) { + this.url = url; + } + addEventListener(type: string, fn: () => void) { + this.handlers[type] = fn; + } + getResponseHeader(name: string) { + return name === 'ETag' ? `"etag-${this.url.split('/').pop()}"` : null; + } + abort() { + this.aborted = true; + this.handlers.abort?.(); + } + send(blob: Blob) { + const n = Number(this.url.split('/').pop()); + server.puts.push({ url: this.url, size: blob.size }); + server.inFlight += 1; + server.maxInFlight = Math.max(server.maxInFlight, server.inFlight); + setTimeout(() => { + server.inFlight -= 1; + if (this.aborted) return; + this.status = server.putStatuses.get(n)?.shift() ?? 200; + this.handlers.load?.(); + }, 1); + } +} + +function bigFile(parts: number, name = 'movie.bin') { + return new File([new Uint8Array(PART * parts - 3)], name, { lastModified: 1234 }); +} + +const OPTIONS = (signal = new AbortController().signal) => ({ + signal, + onProgress: vi.fn(), + backoffMs: 0, +}); + +beforeEach(() => { + server = new FakeServer(); + vi.stubGlobal('fetch', server.fetch); + vi.stubGlobal('XMLHttpRequest', FakeXhr); + window.localStorage.clear(); +}); + +afterEach(() => vi.unstubAllGlobals()); + +describe('runResumable', () => { + test('sends every part, at most four at once, then completes with the etags', async () => { + const file = bigFile(9); + const options = OPTIONS(); + await runResumable(file, options); + + expect(server.puts).toHaveLength(9); + expect(server.maxInFlight).toBeLessThanOrEqual(PART_CONCURRENCY); + expect(server.maxInFlight).toBeGreaterThan(1); + // The last part is the remainder, not a full part. + expect(server.puts.find((p) => p.url.endsWith('/9'))?.size).toBe(PART - 3); + const body = server.completeBody as { parts: { n: number; etag: string }[] }; + expect(body.parts).toHaveLength(9); + expect(body.parts[0]).toEqual({ n: 1, etag: '"etag-1"' }); + // Progress ends at the whole file. + expect(options.onProgress).toHaveBeenLastCalledWith(file.size, file.size); + }); + + test('remembers the session while running and forgets it once complete', async () => { + const file = bigFile(2); + const key = resumeKey(file); + let seen: string | null = null; + server.putStatuses.set(1, [200]); + const original = server.fetch; + vi.stubGlobal('fetch', async (url: string, init?: RequestInit) => { + if (url.endsWith('/complete')) seen = window.localStorage.getItem(key); + return original(url, init); + }); + + await runResumable(file, OPTIONS()); + + expect(seen).toBe('sess-1'); + expect(window.localStorage.getItem(key)).toBeNull(); + }); + + test('skips the parts the server already holds when resuming', async () => { + const file = bigFile(4); + server.total = file.size; + server.held = new Map([ + [1, '"e1"'], + [3, '"e3"'], + ]); + + await runResumable(file, OPTIONS(), server.session() as never); + + expect(server.puts.map((p) => p.url.split('/').pop()).sort()).toEqual(['2', '4']); + expect(server.calls.some((c) => c === 'POST /api/file-storage/uploads')).toBe(false); + const body = server.completeBody as { parts: { n: number; etag: string }[] }; + expect(body.parts.map((p) => p.n).sort()).toEqual([1, 2, 3, 4]); + }); + + test('retries a failing part with a fresh URL each time', async () => { + server.putStatuses.set(2, [500, 403]); + await runResumable(bigFile(3), OPTIONS()); + + const urlRequests = server.calls.filter((c) => c.endsWith('/part-urls')); + expect(urlRequests).toHaveLength(3 + 2); + expect(server.puts.filter((p) => p.url.endsWith('/2'))).toHaveLength(3); + }); + + test('gives up on a part after four attempts and keeps the session to resume', async () => { + server.putStatuses.set(1, [500, 500, 500, 500]); + const file = bigFile(1); + + await expect(runResumable(file, OPTIONS())).rejects.toBeInstanceOf(UploadFailedError); + + expect(server.puts).toHaveLength(4); + expect(window.localStorage.getItem(resumeKey(file))).toBe('sess-1'); + }); + + test('does not retry a part the server refused on its merits', async () => { + server.putStatuses.set(1, [400]); + await expect(runResumable(bigFile(1), OPTIONS())).rejects.toMatchObject({ status: 400 }); + expect(server.puts).toHaveLength(1); + }); + + test("surfaces the server's reason when completing fails", async () => { + server.completeStatus = 409; + await expect(runResumable(bigFile(1), OPTIONS())).rejects.toMatchObject({ + status: 409, + reason: 'parts do not match', + }); + }); + + test('cancelling stops the upload with an abort, not a failure', async () => { + const controller = new AbortController(); + const running = runResumable(bigFile(8), OPTIONS(controller.signal)); + setTimeout(() => controller.abort(), 0); + await expect(running).rejects.toBeInstanceOf(UploadAbortedError); + }); +}); + +describe('cancelling while the session is being created', () => { + test('releases the session the create resolved with', async () => { + const controller = new AbortController(); + const file = bigFile(3); + const running = runResumable(file, OPTIONS(controller.signal)); + controller.abort(); // the POST is in flight; no session id exists yet + await expect(running).rejects.toBeInstanceOf(UploadAbortedError); + expect(server.calls).toContain('DELETE /api/file-storage/uploads/sess-1'); + expect(server.puts).toHaveLength(0); + expect(window.localStorage.getItem(resumeKey(file))).toBeNull(); + }); +}); + +describe('findSession / discardSession', () => { + test('finds the open session stored for the same file', async () => { + const file = bigFile(3); + server.total = file.size; + window.localStorage.setItem(resumeKey(file), 'sess-1'); + expect(await findSession(file)).toMatchObject({ id: 'sess-1', status: 'active' }); + }); + + test('a different file finds nothing', async () => { + window.localStorage.setItem(resumeKey(bigFile(3, 'other.bin')), 'sess-1'); + expect(await findSession(bigFile(3))).toBeNull(); + }); + + test('a session the server no longer knows is forgotten', async () => { + const file = bigFile(3); + window.localStorage.setItem(resumeKey(file), 'gone'); + vi.stubGlobal('fetch', async () => new Response('{}', { status: 404 })); + expect(await findSession(file)).toBeNull(); + expect(window.localStorage.getItem(resumeKey(file))).toBeNull(); + }); + + test('a network failure keeps the stored id for next time', async () => { + const file = bigFile(3); + window.localStorage.setItem(resumeKey(file), 'sess-1'); + vi.stubGlobal('fetch', async () => { + throw new TypeError('offline'); + }); + expect(await findSession(file)).toBeNull(); + expect(window.localStorage.getItem(resumeKey(file))).toBe('sess-1'); + }); + + test('works with storage unavailable', async () => { + const spy = vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { + throw new Error('blocked'); + }); + expect(await findSession(bigFile(2))).toBeNull(); + spy.mockRestore(); + }); + + test('discarding aborts the session and clears the key', async () => { + const file = bigFile(2); + window.localStorage.setItem(resumeKey(file), 'sess-1'); + await discardSession(file, 'sess-1'); + expect(server.calls).toContain('DELETE /api/file-storage/uploads/sess-1'); + expect(window.localStorage.getItem(resumeKey(file))).toBeNull(); + }); +}); diff --git a/modules/file_storage/tests/conftest.py b/modules/file_storage/tests/conftest.py index 4351a9676..a6387828a 100644 --- a/modules/file_storage/tests/conftest.py +++ b/modules/file_storage/tests/conftest.py @@ -13,6 +13,8 @@ from collections.abc import Generator import pytest +from file_storage.backends.filesystem import FilesystemBackend +from file_storage_uploads_helpers import PART from sqlalchemy import event from sqlalchemy.engine import Engine @@ -83,3 +85,31 @@ async def admin_tenant_id(app) -> str: async def authenticated_client(authenticated_client, admin_tenant_id): """The plugin's admin client, acting for the admin's own organisation.""" return authenticated_client + + +@pytest.fixture +def checksum_calls(monkeypatch) -> list[tuple]: + """Capture ``enqueue_after_commit`` instead of running the (eager) task. + + The test Celery app is eager, so an enqueue would run + ``file_storage.compute_checksum`` inline against whichever database the + worker-side sync engine points at. The task is exercised on its own in + ``test_file_storage_upload_tasks``; request tests only need to see that it + was asked for. + """ + import background_tasks + + calls: list[tuple] = [] + monkeypatch.setattr( + background_tasks, "enqueue_after_commit", lambda db, task, *a, **kw: calls.append((task, a)) + ) + return calls + + +@pytest.fixture +def storage(app, tmp_path): + """The app's file_storage state, pointed at an isolated filesystem root.""" + services = app.state.file_storage + services.backend = FilesystemBackend(root=tmp_path) + services.settings.multipart_part_size_bytes = PART + return services diff --git a/modules/file_storage/tests/file_storage_uploads_helpers.py b/modules/file_storage/tests/file_storage_uploads_helpers.py new file mode 100644 index 000000000..7f0f17525 --- /dev/null +++ b/modules/file_storage/tests/file_storage_uploads_helpers.py @@ -0,0 +1,52 @@ +"""Shared helpers for the resumable-upload tests (imported, not collected).""" + +from __future__ import annotations + +import hashlib + +import httpx +from file_storage import constants + +API = constants.ROUTE_PREFIX_API +MIB = constants.MIB +PART = 5 * MIB # the smallest part size the settings accept + + +async def open_session( + client: httpx.AsyncClient, + *, + filename: str = "big.bin", + total: int, + content_type: str = "application/octet-stream", + public: bool = False, +) -> httpx.Response: + return await client.post( + f"{API}/uploads", + json={ + "filename": filename, + "content_type": content_type, + "total_size": total, + "public": public, + }, + ) + + +async def put_part(client: httpx.AsyncClient, upload_id: str, n: int, body: bytes): + return await client.put(f"{API}/uploads/{upload_id}/parts/{n}", content=body) + + +def payload(total: int) -> bytes: + block = hashlib.sha256(b"seed").digest() + return (block * (total // len(block) + 1))[:total] + + +async def upload_all(client, upload_id: str, data: bytes, only=None): + etags = {} + for i in range(0, len(data), PART): + n = i // PART + 1 + if only is not None and n not in only: + continue + resp = await put_part(client, upload_id, n, data[i : i + PART]) + assert resp.status_code == 200, resp.text + etags[n] = resp.json()["etag"] + return etags diff --git a/modules/file_storage/tests/test_file_storage_upload_tasks.py b/modules/file_storage/tests/test_file_storage_upload_tasks.py new file mode 100644 index 000000000..409c6f3e5 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_upload_tasks.py @@ -0,0 +1,212 @@ +"""The janitor and checksum tasks (``file_storage.tasks``) (#454). + +Tasks run in a worker with its own sync engine, so these point +``background_tasks.sync_db`` at a throwaway SQLite file and swap in a +filesystem backend, which is all a worker needs to exercise. +""" + +from __future__ import annotations + +import asyncio +import hashlib +import uuid +from datetime import UTC, datetime, timedelta + +import pytest +from background_tasks import sync_db +from file_storage import constants, tasks +from file_storage.backends.filesystem import FilesystemBackend +from file_storage.models import Base, StoredFile, UploadSession +from file_storage.settings import FileStorageSettings +from simple_module_db import all_tenants, tenant_context +from sqlalchemy import create_engine, select + +TENANT = "tenant-a" + + +@pytest.fixture +def worker(tmp_path, monkeypatch): + url = f"sqlite:///{tmp_path / 'worker.db'}" + engine = create_engine(url) + Base.metadata.create_all(engine) + engine.dispose() + sync_db.set_database_url(url) + backend = FilesystemBackend(root=tmp_path / "objects") + settings = FileStorageSettings() + monkeypatch.setattr(tasks, "_backend_and_settings", lambda: (backend, settings)) + yield backend + sync_db.dispose_sync_engine() + + +def _session_row(upload_id: str, *, expires_in: timedelta, key: str | None = None): + return UploadSession( + tenant_id=TENANT, + key=key or f"{TENANT}/2026/10/15/{uuid.uuid4().hex}.bin", + filename="a.bin", + content_type="application/octet-stream", + total_size=10, + part_size=constants.MIN_PART_SIZE_BYTES, + backend_upload_id=upload_id, + expires_at=datetime.now(UTC) + expires_in, + ) + + +def test_beat_schedule_runs_the_reaper_hourly(): + entry = tasks.BEAT_SCHEDULE["file-storage-reap-stale-uploads"] + assert entry == {"task": constants.Task.REAP_STALE_UPLOADS, "schedule": 3600.0} + + +def test_the_reaper_aborts_only_expired_active_sessions(worker): + expired_id = asyncio.run(worker.create_multipart("k1", "x/y")) + live_id = asyncio.run(worker.create_multipart("k2", "x/y")) + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add_all( + [ + _session_row(expired_id, expires_in=timedelta(hours=-1)), + _session_row(live_id, expires_in=timedelta(hours=1)), + ] + ) + + result = tasks.reap_stale_uploads() + + assert result == {"sessions": 1, "orphans": 0, "purged": 0} + assert not worker._parts_dir(expired_id).exists() + assert worker._parts_dir(live_id).exists() + with sync_db.sync_session() as db, all_tenants(): + statuses = { + s.backend_upload_id: s.status for s in db.execute(select(UploadSession)).scalars() + } + assert statuses == {expired_id: "aborted", live_id: "active"} + + # Idempotent: nothing left to reap. + assert tasks.reap_stale_uploads() == {"sessions": 0, "orphans": 0, "purged": 0} + + +def test_a_failing_abort_is_retried_next_hour(worker, monkeypatch): + upload_id = asyncio.run(worker.create_multipart("k", "x/y")) + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add(_session_row(upload_id, expires_in=timedelta(hours=-1))) + + async def boom(key, upload): + raise OSError("bucket unreachable") + + monkeypatch.setattr(worker, "abort_multipart", boom) + assert tasks.reap_stale_uploads() == {"sessions": 0, "orphans": 0, "purged": 0} + with sync_db.sync_session() as db, all_tenants(): + assert db.execute(select(UploadSession.status)).scalar_one() == "active" + + +def test_compute_checksum_fills_the_digest(worker): + payload = b"streamed once" * 1000 + key = f"{TENANT}/2026/10/15/obj.bin" + + async def _store(): + async def _body(): + yield payload + + await worker.put(key, _body(), content_type="x/y", size=len(payload)) + + asyncio.run(_store()) + file_id = uuid.uuid4() + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add( + StoredFile( + id=file_id, + tenant_id=TENANT, + key=key, + filename="obj.bin", + content_type="x/y", + size_bytes=len(payload), + backend="filesystem", + checksum_sha256=None, + ) + ) + + digest = tasks.compute_checksum(str(file_id), TENANT) + + assert digest == hashlib.sha256(payload).hexdigest() + with sync_db.sync_session() as db, tenant_context(TENANT): + assert db.get(StoredFile, file_id).checksum_sha256 == digest + # Already filled: a duplicate delivery does nothing. + assert tasks.compute_checksum(str(file_id), TENANT) is None + + +def test_compute_checksum_ignores_another_tenants_file(worker): + file_id = uuid.uuid4() + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add( + StoredFile( + id=file_id, + tenant_id=TENANT, + key="k", + filename="a", + content_type="x/y", + size_bytes=1, + backend="filesystem", + ) + ) + assert tasks.compute_checksum(str(file_id), "tenant-b") is None + + +def test_the_orphan_sweep_spares_active_sessions_and_foreign_keys(worker, monkeypatch): + own = f"{TENANT}/2026/10/15/{uuid.uuid4().hex}.bin" + tracked_key = f"{TENANT}/2026/10/15/{uuid.uuid4().hex}.bin" + tracked_id = asyncio.run(worker.create_multipart(tracked_key, "x/y")) + orphan_id = asyncio.run(worker.create_multipart(own, "x/y")) + foreign_id = asyncio.run(worker.create_multipart("another-app/exports/data.csv", "x/y")) + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add(_session_row(tracked_id, expires_in=timedelta(hours=1), key=tracked_key)) + + async def stale(older_than): + return [ + (own, orphan_id), + (tracked_key, tracked_id), + ("another-app/exports/data.csv", foreign_id), + ] + + monkeypatch.setattr(worker, "list_stale_multipart", stale, raising=False) + + assert tasks.reap_stale_uploads() == {"sessions": 0, "orphans": 1, "purged": 0} + assert not worker._parts_dir(orphan_id).exists() + assert worker._parts_dir(tracked_id).exists() + assert worker._parts_dir(foreign_id).exists() + + +def test_compute_checksum_survives_a_deleted_object(worker): + file_id = uuid.uuid4() + with sync_db.sync_session() as db, tenant_context(TENANT): + db.add( + StoredFile( + id=file_id, + tenant_id=TENANT, + key=f"{TENANT}/gone.bin", + filename="a", + content_type="x/y", + size_bytes=1, + backend="filesystem", + ) + ) + assert tasks.compute_checksum(str(file_id), TENANT) is None + + +def test_the_reaper_purges_finished_rows_past_the_retention_window(worker): + retention = FileStorageSettings().upload_session_retention_days + old, recent = timedelta(days=-(retention + 1)), timedelta(days=-(retention - 1)) + with sync_db.sync_session() as db, tenant_context(TENANT): + rows = [ + _session_row("a", expires_in=old), + _session_row("b", expires_in=old), + _session_row("c", expires_in=recent), + _session_row(asyncio.run(worker.create_multipart("kd", "x/y")), expires_in=old), + ] + rows[0].status = constants.UploadStatus.COMPLETED + rows[1].status = constants.UploadStatus.ABORTED + rows[2].status = constants.UploadStatus.ABORTED + db.add_all(rows) # "d" stays active and expired: aborted this run, kept for now + + result = tasks.reap_stale_uploads() + + assert result["purged"] == 2 + with sync_db.sync_session() as db, all_tenants(): + left = {s.backend_upload_id: s.status for s in db.execute(select(UploadSession)).scalars()} + assert left == {"c": "aborted", rows[3].backend_upload_id: "aborted"} diff --git a/modules/file_storage/tests/test_file_storage_uploads_expiry.py b/modules/file_storage/tests/test_file_storage_uploads_expiry.py new file mode 100644 index 000000000..65bcd2e80 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_expiry.py @@ -0,0 +1,63 @@ +"""Expiry and retention of upload session rows (#454).""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime, timedelta + +import pytest +from file_storage import constants +from file_storage.models import UploadSession +from file_storage_uploads_helpers import API, PART, open_session +from simple_module_db import all_tenants +from sqlalchemy import select, update + +pytestmark = pytest.mark.usefixtures("checksum_calls") + + +async def _expire_everything(app) -> None: + async with app.state.sm.db.session_factory() as db: + with all_tenants(): + await db.execute( + update(UploadSession).values(expires_at=datetime.now(UTC) - timedelta(hours=1)) + ) + await db.commit() + + +async def test_opening_an_expired_session_persists_the_abort(app, authenticated_client, storage): + session = (await open_session(authenticated_client, total=PART)).json() + await _expire_everything(app) + urls = await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [1]} + ) + assert urls.status_code == 409 + assert urls.json()["detail"]["code"] == constants.ErrorCode.SESSION_CLOSED + async with app.state.sm.db.session_factory() as db: + with all_tenants(): + row = (await db.execute(select(UploadSession))).scalar_one() + assert row.status == "aborted" + assert not (storage.backend.root / constants.FS_MULTIPART_DIR / session["id"]).exists() + + +async def test_creating_a_session_purges_the_callers_old_finished_rows( + app, authenticated_client, storage +): + old = (await open_session(authenticated_client, total=PART)).json() + await authenticated_client.delete(f"{API}/uploads/{old['id']}") + recent = (await open_session(authenticated_client, total=PART)).json() + await authenticated_client.delete(f"{API}/uploads/{recent['id']}") + stale = datetime.now(UTC) - timedelta(days=storage.settings.upload_session_retention_days + 1) + async with app.state.sm.db.session_factory() as db: + with all_tenants(): + await db.execute( + update(UploadSession) + .where(UploadSession.id == uuid.UUID(old["id"])) + .values(expires_at=stale) + ) + await db.commit() + + live = (await open_session(authenticated_client, total=PART)).json() + async with app.state.sm.db.session_factory() as db: + with all_tenants(): + ids = {str(i) for i in (await db.execute(select(UploadSession.id))).scalars()} + assert ids == {recent["id"], live["id"]} diff --git a/modules/file_storage/tests/test_file_storage_uploads_fs.py b/modules/file_storage/tests/test_file_storage_uploads_fs.py new file mode 100644 index 000000000..3e5394962 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_fs.py @@ -0,0 +1,275 @@ +"""Resumable multipart uploads end-to-end on the filesystem backend (#454).""" + +from __future__ import annotations + +import uuid +from datetime import UTC, datetime, timedelta + +import pytest +from file_storage import constants +from file_storage.contracts.events import FileUploaded +from file_storage.models import StoredFile, UploadSession +from file_storage_uploads_helpers import API, MIB, PART, open_session, payload, put_part, upload_all +from simple_module_db import all_tenants, tenant_context +from sqlalchemy import select, update + +pytestmark = pytest.mark.usefixtures("checksum_calls") + + +async def _expire_everything(app) -> None: + async with app.state.sm.db.session_factory() as db: + with all_tenants(): + await db.execute( + update(UploadSession).values(expires_at=datetime.now(UTC) - timedelta(hours=1)) + ) + await db.commit() + + +async def test_full_flow_creates_a_stored_file_and_publishes_the_event( + app, authenticated_client, storage, checksum_calls +): + seen = [] + + async def _collect(event): + seen.append(event) + + app.state.sm.event_bus.subscribe(FileUploaded, _collect) + data = payload(PART * 2 + 123) + + created = await open_session(authenticated_client, total=len(data)) + assert created.status_code == 201, created.text + session = created.json() + assert session["part_size"] == PART + assert session["total_parts"] == 3 + assert session["parts"] == [] + + # Parts can arrive out of order, and a reload lists what the backend holds. + etags = await upload_all(authenticated_client, session["id"], data, only={3, 1}) + resumed = await authenticated_client.get(f"{API}/uploads/{session['id']}") + assert [p["no"] for p in resumed.json()["parts"]] == [1, 3] + + etags.update(await upload_all(authenticated_client, session["id"], data, only={2})) + done = await authenticated_client.post( + f"{API}/uploads/{session['id']}/complete", + json={"parts": [{"n": n, "etag": e} for n, e in etags.items()]}, + ) + assert done.status_code == 200, done.text + body = done.json() + assert body["status"] == "completed" + file = body["file"] + assert file["size_bytes"] == len(data) + assert file["checksum_sha256"] is None + + download = await authenticated_client.get(f"{API}/files/{file['id']}/download") + assert download.status_code == 200 + assert download.content == data + assert [e.file_id for e in seen] == [uuid.UUID(file["id"])] + assert checksum_calls == [ + (constants.Task.COMPUTE_CHECKSUM, (file["id"], file["key"].split("/")[0])) + ] + + # A retried complete answers with the same file and publishes nothing new. + again = await authenticated_client.post(f"{API}/uploads/{session['id']}/complete", json={}) + assert again.status_code == 200 + assert again.json()["file"]["id"] == file["id"] + assert len(seen) == 1 + assert len(checksum_calls) == 1 + + +async def test_part_urls_point_at_the_app_for_filesystem(authenticated_client, storage): + session = (await open_session(authenticated_client, total=PART + 1)).json() + resp = await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [1, 2]} + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["direct"] is False + assert body["urls"]["1"] == f"{API}/uploads/{session['id']}/parts/1" + + bad = await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [3]} + ) + assert bad.status_code == 400 + assert bad.json()["detail"]["code"] == constants.ErrorCode.BAD_PART + + +async def test_complete_with_missing_parts_is_rejected_and_resumable(authenticated_client, storage): + data = payload(PART * 2) + session = (await open_session(authenticated_client, total=len(data))).json() + await upload_all(authenticated_client, session["id"], data, only={1}) + + resp = await authenticated_client.post(f"{API}/uploads/{session['id']}/complete", json={}) + assert resp.status_code == 409 + detail = resp.json()["detail"] + assert detail["code"] == constants.ErrorCode.PARTS_MISMATCH + assert detail["missing"] == [2] + + await upload_all(authenticated_client, session["id"], data, only={2}) + done = await authenticated_client.post(f"{API}/uploads/{session['id']}/complete", json={}) + assert done.status_code == 200 + + +async def test_wrong_etag_is_a_mismatch(authenticated_client, storage): + data = payload(PART) + session = (await open_session(authenticated_client, total=len(data))).json() + await upload_all(authenticated_client, session["id"], data) + resp = await authenticated_client.post( + f"{API}/uploads/{session['id']}/complete", json={"parts": [{"n": 1, "etag": "nope"}]} + ) + assert resp.status_code == 409 + assert resp.json()["detail"]["mismatched"] == [1] + + +async def test_a_part_of_the_wrong_size_is_refused(authenticated_client, storage): + session = (await open_session(authenticated_client, total=PART * 2)).json() + short = await put_part(authenticated_client, session["id"], 1, b"x" * 10) + assert short.status_code == 400 + assert short.json()["detail"]["code"] == constants.ErrorCode.BAD_PART + long = await put_part(authenticated_client, session["id"], 1, b"x" * (PART + 1)) + assert long.status_code == 400 + assert (await put_part(authenticated_client, session["id"], 9, b"x")).status_code == 400 + + +async def test_abort_discards_the_parts_and_closes_the_session(authenticated_client, storage): + data = payload(PART) + session = (await open_session(authenticated_client, total=len(data))).json() + await upload_all(authenticated_client, session["id"], data) + assert (await authenticated_client.delete(f"{API}/uploads/{session['id']}")).status_code == 204 + + assert not list((storage.backend.root / constants.FS_MULTIPART_DIR).glob("*")) + got = await authenticated_client.get(f"{API}/uploads/{session['id']}") + assert got.json()["status"] == "aborted" + closed = await put_part(authenticated_client, session["id"], 1, data) + assert closed.status_code == 409 + assert closed.json()["detail"]["code"] == constants.ErrorCode.SESSION_CLOSED + + +async def test_creating_a_session_reaps_the_callers_expired_ones( + app, authenticated_client, storage +): + old = (await open_session(authenticated_client, total=PART)).json() + await _expire_everything(app) + + assert (await open_session(authenticated_client, total=PART)).status_code == 201 + got = await authenticated_client.get(f"{API}/uploads/{old['id']}") + assert got.json()["status"] == "aborted" + assert not (storage.backend.root / constants.FS_MULTIPART_DIR / old["id"]).exists() + + +async def test_an_expired_session_cannot_take_parts(app, authenticated_client, storage): + session = (await open_session(authenticated_client, total=PART)).json() + await _expire_everything(app) + resp = await put_part(authenticated_client, session["id"], 1, b"x" * PART) + assert resp.status_code == 409 + # The abort that discovering the expiry performed outlives the 409. + got = await authenticated_client.get(f"{API}/uploads/{session['id']}") + assert got.json()["status"] == "aborted" + urls = await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [1]} + ) + assert urls.status_code == 409 + + +class TestValidation: + async def test_disallowed_type(self, authenticated_client, storage): + storage.settings.allowed_content_types = ["text/plain"] + resp = await open_session(authenticated_client, total=PART, content_type="image/png") + assert resp.status_code == 415 + assert resp.json()["detail"]["code"] == constants.ErrorCode.BAD_TYPE + + async def test_over_the_direct_upload_maximum(self, authenticated_client, storage): + storage.settings.max_direct_upload_bytes = 10 * MIB + resp = await open_session(authenticated_client, total=11 * MIB) + assert resp.status_code == 413 + assert resp.json()["detail"]["max_bytes"] == 10 * MIB + + async def test_over_quota(self, authenticated_client, storage): + storage.settings.quota_bytes = PART - 1 + resp = await open_session(authenticated_client, total=PART) + assert resp.status_code == 413 + assert resp.json()["detail"]["code"] == constants.ErrorCode.QUOTA_EXCEEDED + + async def test_unsupported_backend(self, authenticated_client, storage): + class Plain: + backend_id = "plain" + supports_presigned_url = False + + storage.backend = Plain() + resp = await open_session(authenticated_client, total=PART) + assert resp.status_code == 501 + assert resp.json()["detail"]["code"] == constants.ErrorCode.DIRECT_UNSUPPORTED + + async def test_needs_a_session(self, client): + assert (await open_session(client, total=PART)).status_code in {401, 403} + + async def test_part_size_grows_to_stay_under_ten_thousand_parts( + self, authenticated_client, storage + ): + storage.settings.max_direct_upload_bytes = 1000 * 1024 * MIB + total = 500 * 1024 * MIB + body = (await open_session(authenticated_client, total=total)).json() + assert body["total_parts"] <= constants.MAX_UPLOAD_PARTS + assert body["part_size"] % MIB == 0 + assert body["part_size"] > PART + + +class TestOwnership: + async def test_other_tenants_cannot_touch_a_session(self, tenant_client, storage): + async with tenant_client() as owner, tenant_client() as stranger: + sid = (await open_session(owner.client, total=PART)).json()["id"] + assert (await owner.client.get(f"{API}/uploads/{sid}")).status_code == 200 + + for call in ( + stranger.client.get(f"{API}/uploads/{sid}"), + stranger.client.delete(f"{API}/uploads/{sid}"), + stranger.client.post(f"{API}/uploads/{sid}/complete", json={}), + stranger.client.post(f"{API}/uploads/{sid}/part-urls", json={"parts": [1]}), + put_part(stranger.client, sid, 1, b"x"), + ): + assert (await call).status_code == 404 + + assert (await owner.client.get(f"{API}/uploads/{sid}")).json()["status"] == "active" + + async def test_a_colleague_in_the_same_tenant_cannot_either(self, tenant_client, storage): + async with tenant_client() as owner, tenant_client(tenant_id=owner.tenant_id) as colleague: + sid = (await open_session(owner.client, total=PART)).json()["id"] + assert (await colleague.client.get(f"{API}/uploads/{sid}")).status_code == 404 + assert (await colleague.client.delete(f"{API}/uploads/{sid}")).status_code == 404 + assert (await put_part(colleague.client, sid, 1, b"x")).status_code == 404 + + async def test_the_file_belongs_to_the_owners_tenant(self, app, tenant_client, storage): + data = payload(PART) + async with tenant_client() as owner: + sid = (await open_session(owner.client, total=len(data))).json()["id"] + await upload_all(owner.client, sid, data) + out = (await owner.client.post(f"{API}/uploads/{sid}/complete", json={})).json() + assert out["file"]["key"].startswith(f"{owner.tenant_id}/") + async with app.state.sm.db.session_factory() as db: + with tenant_context(owner.tenant_id): + rows = (await db.execute(select(StoredFile))).scalars().all() + assert [r.tenant_id for r in rows] == [owner.tenant_id] + + +def test_no_checksum_task_without_background_tasks(checksum_calls): + from types import SimpleNamespace + + from file_storage.checksum import schedule_checksum + + file_id = uuid.uuid4() + bare = SimpleNamespace(state=SimpleNamespace()) + assert schedule_checksum(bare, object(), file_id, "t") is False + assert checksum_calls == [] + + wired = SimpleNamespace(state=SimpleNamespace(background_tasks=object())) + assert schedule_checksum(wired, object(), file_id, "t") is True + assert checksum_calls == [(constants.Task.COMPUTE_CHECKSUM, (str(file_id), "t"))] + + +def test_the_part_route_gets_a_body_limit_of_one_part(app, storage): + registry = app.state.body_limits + path = f"{API}/uploads/{uuid.uuid4()}/parts/3" + limit = registry.limit_for("PUT", path, app) + assert limit == PART + constants.PART_BODY_SLACK_BYTES + # Only PUT, only that shape: nothing else under /uploads gets the headroom. + assert registry.limit_for("POST", path, app) is None + assert registry.limit_for("PUT", f"{API}/uploads/x/complete", app) is None diff --git a/modules/file_storage/tests/test_file_storage_uploads_minio.py b/modules/file_storage/tests/test_file_storage_uploads_minio.py new file mode 100644 index 000000000..9d547da9b --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_minio.py @@ -0,0 +1,66 @@ +"""Opt-in: multipart upload against the shared dev S3 (RustFS/MinIO) (#454). + +Skipped unless ``SM_TEST_S3_ENDPOINT`` is set, e.g.:: + + SM_TEST_S3_ENDPOINT=http://localhost:9000 uv run pytest \\ + modules/file_storage/tests/test_file_storage_uploads_minio.py + +Credentials default to the dev stack's ``minioadmin``/``minioadmin`` and the +test creates (and leaves) a bucket named ``sm-multipart-test``. +""" + +from __future__ import annotations + +import contextlib +import os +from datetime import timedelta + +import boto3 +import httpx +import pytest +from file_storage import constants +from file_storage.backends import build_backend +from file_storage.settings import FileStorageSettings +from file_storage_uploads_helpers import MIB + +ENDPOINT = os.environ.get("SM_TEST_S3_ENDPOINT") +BUCKET = "sm-multipart-test" +PART = 5 * MIB + +pytestmark = pytest.mark.skipif(not ENDPOINT, reason="SM_TEST_S3_ENDPOINT not set") + + +async def test_two_part_upload_against_real_s3(): + creds = { + "aws_access_key_id": os.environ.get("SM_TEST_S3_KEY", "minioadmin"), + "aws_secret_access_key": os.environ.get("SM_TEST_S3_SECRET", "minioadmin"), + } + client = boto3.client("s3", region_name="us-east-1", endpoint_url=ENDPOINT, **creds) + with contextlib.suppress(client.exceptions.ClientError): + client.create_bucket(Bucket=BUCKET) + + backend = build_backend( + FileStorageSettings( + backend=constants.BackendId.S3, + s3_bucket=BUCKET, + s3_region="us-east-1", + s3_endpoint_url=ENDPOINT, + s3_access_key_id=creds["aws_access_key_id"], + s3_secret_access_key=creds["aws_secret_access_key"], + ) + ) + key = "multipart/real.bin" + upload_id = await backend.create_multipart(key, "application/octet-stream") + async with httpx.AsyncClient() as http: + for n, body in ((1, b"a" * PART), (2, b"tail")): + resp = await http.put(await backend.presign_part(key, upload_id, n, 300), content=body) + assert resp.status_code == 200, resp.text + + # The janitor's orphan listing honours the age cut-off (moto cannot show it). + assert (key, upload_id) not in await backend.list_stale_multipart(timedelta(days=1)) + assert (key, upload_id) in await backend.list_stale_multipart(timedelta(seconds=-60)) + + parts = await backend.list_parts(key, upload_id) + await backend.complete_multipart(key, upload_id, parts) + assert (await backend.head(key)).size == PART + 4 + await backend.delete(key) diff --git a/modules/file_storage/tests/test_file_storage_uploads_pg.py b/modules/file_storage/tests/test_file_storage_uploads_pg.py new file mode 100644 index 000000000..1739b47a4 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_pg.py @@ -0,0 +1,55 @@ +"""Concurrency guarantees that only a real Postgres can show (#454). + +SQLite serialises writers, so the quota reservation race and the row lock on +``complete`` are invisible there. Run with ``SM_TEST_DATABASE_URL`` pointing at +Postgres and ``-p no:anyio`` (``make test-py-pg``); skipped otherwise. +""" + +from __future__ import annotations + +import asyncio +import os + +import pytest +from file_storage import constants +from file_storage_uploads_helpers import API, PART, open_session, payload, upload_all + +pytestmark = [ + pytest.mark.usefixtures("checksum_calls"), + pytest.mark.skipif( + not os.environ.get("SM_TEST_DATABASE_URL", "").startswith("postgresql"), + reason="needs SM_TEST_DATABASE_URL on Postgres", + ), +] + + +async def test_concurrent_openers_cannot_overbook_the_quota(authenticated_client, storage): + storage.settings.quota_bytes = PART * 2 + + responses = await asyncio.gather( + *(open_session(authenticated_client, total=PART) for _ in range(6)) + ) + + codes = sorted(r.status_code for r in responses) + assert codes == [201, 201, 413, 413, 413, 413], codes + assert all( + r.json()["detail"]["code"] == constants.ErrorCode.QUOTA_EXCEEDED + for r in responses + if r.status_code == 413 + ) + + +async def test_concurrent_completes_record_one_file(authenticated_client, storage, checksum_calls): + data = payload(PART + 7) + sid = (await open_session(authenticated_client, total=len(data))).json()["id"] + etags = await upload_all(authenticated_client, sid, data) + body = {"parts": [{"n": n, "etag": e} for n, e in etags.items()]} + + responses = await asyncio.gather( + *(authenticated_client.post(f"{API}/uploads/{sid}/complete", json=body) for _ in range(4)) + ) + + assert [r.status_code for r in responses] == [200] * 4, [r.text for r in responses] + assert len({r.json()["file"]["id"] for r in responses}) == 1 + assert (await authenticated_client.get(f"{API}/files")).json()["total"] == 1 + assert len(checksum_calls) == 1 diff --git a/modules/file_storage/tests/test_file_storage_uploads_quota.py b/modules/file_storage/tests/test_file_storage_uploads_quota.py new file mode 100644 index 000000000..64b70f251 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_quota.py @@ -0,0 +1,94 @@ +"""Quota and size limits hold for what is really stored, not what was declared (#454).""" + +from __future__ import annotations + +import pytest +from file_storage import constants +from file_storage_uploads_helpers import API, PART, open_session, payload, upload_all + +pytestmark = pytest.mark.usefixtures("checksum_calls") + + +class TestQuotaIsEnforcedOnWhatIsReallyStored: + async def test_open_sessions_count_against_the_quota(self, authenticated_client, storage): + storage.settings.quota_bytes = PART * 2 + assert (await open_session(authenticated_client, total=PART)).status_code == 201 + assert (await open_session(authenticated_client, total=PART)).status_code == 201 + third = await open_session(authenticated_client, total=1) + assert third.status_code == 413 + assert third.json()["detail"]["code"] == constants.ErrorCode.QUOTA_EXCEEDED + + async def test_stored_files_and_open_sessions_add_up(self, authenticated_client, storage): + storage.settings.quota_bytes = PART + 10 + await authenticated_client.post( + f"{API}/upload", files={"file": ("a.bin", b"x" * 20, "application/octet-stream")} + ) + assert (await open_session(authenticated_client, total=PART)).status_code == 413 + + async def test_aborted_sessions_release_their_reservation(self, authenticated_client, storage): + storage.settings.quota_bytes = PART + sid = (await open_session(authenticated_client, total=PART)).json()["id"] + assert (await open_session(authenticated_client, total=PART)).status_code == 413 + await authenticated_client.delete(f"{API}/uploads/{sid}") + assert (await open_session(authenticated_client, total=PART)).status_code == 201 + + async def test_complete_rechecks_quota_against_the_stored_size( + self, authenticated_client, storage + ): + data = payload(PART) + sid = (await open_session(authenticated_client, total=len(data))).json()["id"] + await upload_all(authenticated_client, sid, data) + # Tightened after the session opened (an admin change, or other uploads). + storage.settings.quota_bytes = PART - 1 + resp = await authenticated_client.post(f"{API}/uploads/{sid}/complete", json={}) + assert resp.status_code == 413 + assert resp.json()["detail"]["code"] == constants.ErrorCode.QUOTA_EXCEEDED + listing = (await authenticated_client.get(f"{API}/files")).json() + assert listing["total"] == 0 + assert not list(storage.backend.root.glob("*/*/**/*.bin")) + + async def test_complete_rechecks_the_direct_upload_maximum(self, authenticated_client, storage): + data = payload(PART) + sid = (await open_session(authenticated_client, total=len(data))).json()["id"] + await upload_all(authenticated_client, sid, data) + storage.settings.max_direct_upload_bytes = PART - 1 + resp = await authenticated_client.post(f"{API}/uploads/{sid}/complete", json={}) + assert resp.status_code == 413 + assert resp.json()["detail"]["code"] == constants.ErrorCode.TOO_LARGE + + async def test_assembled_size_must_equal_the_declared_total( + self, authenticated_client, storage, monkeypatch + ): + """An object bigger than declared (a store that let an oversized part through).""" + data = payload(PART) + sid = (await open_session(authenticated_client, total=len(data))).json()["id"] + await upload_all(authenticated_client, sid, data) + real_head = storage.backend.head + + async def lying_head(key): + info = await real_head(key) + return type(info)(size=info.size + 1, etag=info.etag) + + monkeypatch.setattr(storage.backend, "head", lying_head) + resp = await authenticated_client.post(f"{API}/uploads/{sid}/complete", json={}) + assert resp.status_code == 422 + assert resp.json()["detail"]["code"] == constants.ErrorCode.SIZE_MISMATCH + assert (await authenticated_client.get(f"{API}/files")).json()["total"] == 0 + assert not await real_head_exists(storage.backend, sid) + + async def test_a_rejected_complete_closes_the_session_for_good( + self, authenticated_client, storage + ): + """The rejection must be committed, not rolled back with the error response.""" + data = payload(PART) + sid = (await open_session(authenticated_client, total=len(data))).json()["id"] + await upload_all(authenticated_client, sid, data) + storage.settings.quota_bytes = PART - 1 + resp = await authenticated_client.post(f"{API}/uploads/{sid}/complete", json={}) + assert resp.status_code == 413 + got = await authenticated_client.get(f"{API}/uploads/{sid}") + assert got.json()["status"] == "aborted" + + +async def real_head_exists(backend, _sid) -> bool: + return any(p.is_file() for p in backend.root.glob("*/*/*/*/*/*.bin")) diff --git a/modules/file_storage/tests/test_file_storage_uploads_s3.py b/modules/file_storage/tests/test_file_storage_uploads_s3.py new file mode 100644 index 000000000..6d64e7789 --- /dev/null +++ b/modules/file_storage/tests/test_file_storage_uploads_s3.py @@ -0,0 +1,231 @@ +"""Multipart uploads against S3 (moto's HTTP server, real aioboto3) (#454). + +Parts are PUT to the presigned URLs exactly as a browser would, so this covers +signing, the ``public`` endpoint override, ``list_parts`` pagination of etags, +completion and the janitor's orphan listing. +""" + +from __future__ import annotations + +import contextlib +from collections.abc import Generator +from datetime import timedelta + +import boto3 +import httpx +import pytest +from file_storage import constants +from file_storage.backends import build_backend +from file_storage.contracts.service import Part, StorageNotFoundError +from file_storage.csp import s3_connect_origins +from file_storage.settings import FileStorageSettings +from file_storage_uploads_helpers import API, MIB, open_session +from moto.server import ThreadedMotoServer + +_BUCKET = "mp-test-bucket" +_REGION = "us-east-1" +pytestmark = pytest.mark.usefixtures("checksum_calls") +PART = 5 * MIB + + +@pytest.fixture(scope="module") +def moto_endpoint() -> Generator[str]: + server = ThreadedMotoServer(port=0) + server.start() + host, port = server.get_host_and_port() + yield f"http://{host}:{port}" + server.stop() + + +@pytest.fixture +def s3_settings(moto_endpoint: str) -> FileStorageSettings: + return FileStorageSettings( + backend=constants.BackendId.S3, + s3_bucket=_BUCKET, + s3_region=_REGION, + s3_access_key_id="test", + s3_secret_access_key="test", + s3_endpoint_url=moto_endpoint, + ) + + +@pytest.fixture(autouse=True) +def fresh_bucket(moto_endpoint: str): + client = boto3.client( + "s3", + region_name=_REGION, + endpoint_url=moto_endpoint, + aws_access_key_id="test", + aws_secret_access_key="test", + ) + with contextlib.suppress(client.exceptions.ClientError): + for obj in client.list_objects_v2(Bucket=_BUCKET).get("Contents", []): + client.delete_object(Bucket=_BUCKET, Key=obj["Key"]) + client.delete_bucket(Bucket=_BUCKET) + client.create_bucket(Bucket=_BUCKET) + yield client + + +async def _put(url: str, body: bytes) -> str: + async with httpx.AsyncClient() as http: + resp = await http.put(url, content=body) + assert resp.status_code == 200, resp.text + return resp.headers["ETag"] + + +async def test_backend_multipart_roundtrip(s3_settings): + backend = build_backend(s3_settings) + assert backend.supports_direct_upload is True + key = "t/2026/10/15/big.bin" + upload_id = await backend.create_multipart(key, "application/octet-stream") + + body1, body2 = b"a" * PART, b"tail" + await _put(await backend.presign_part(key, upload_id, 1, 300), body1) + await _put(await backend.presign_part(key, upload_id, 2, 300), body2) + + parts = await backend.list_parts(key, upload_id) + assert [(p.no, p.size) for p in parts] == [(1, PART), (2, 4)] + assert all(isinstance(p, Part) and p.etag for p in parts) + + await backend.complete_multipart(key, upload_id, parts) + info = await backend.head(key) + assert info.size == PART + 4 + chunks = [c async for c in await backend.get(key)] + assert b"".join(chunks) == body1 + body2 + + +async def test_head_of_a_missing_object(s3_settings): + backend = build_backend(s3_settings) + with pytest.raises(StorageNotFoundError): + await backend.head("nope") + + +async def test_abort_discards_parts_and_is_idempotent(s3_settings): + backend = build_backend(s3_settings) + key = "t/abort.bin" + upload_id = await backend.create_multipart(key, "application/octet-stream") + await _put(await backend.presign_part(key, upload_id, 1, 300), b"x") + + await backend.abort_multipart(key, upload_id) + await backend.abort_multipart(key, upload_id) # NoSuchUpload is not an error + with pytest.raises(StorageNotFoundError): + await backend.list_parts(key, upload_id) + + +async def test_presigned_put_url_stores_a_whole_object(s3_settings): + backend = build_backend(s3_settings) + await _put(await backend.presigned_put_url("t/one.txt", 300, "text/plain"), b"hello") + assert (await backend.head("t/one.txt")).size == 5 + + +async def test_a_part_url_is_bound_to_the_part_length(s3_settings): + backend = build_backend(s3_settings) + bound = await backend.presign_part("k", "uid", 1, 300, 1234) + assert "content-length" in bound.lower().split("x-amz-signedheaders=")[1].split("&")[0] + unbound = await backend.presign_part("k", "uid", 1, 300) + assert "content-length" not in unbound.lower().split("x-amz-signedheaders=")[1].split("&")[0] + + +async def test_public_endpoint_is_what_gets_signed(s3_settings): + settings = s3_settings.model_copy( + update={"s3_public_endpoint_url": "https://files.example.test"} + ) + backend = build_backend(settings) + url = await backend.presign_part("k", "uid", 1, 300) + assert url.startswith("https://files.example.test/") + assert "X-Amz-Signature" in url + assert (await backend.presigned_get_url("k", 300)).startswith("https://files.example.test/") + + +async def test_list_stale_multipart_lists_in_progress_uploads(s3_settings): + # moto hard-codes `Initiated` to 2010, so the age filter itself is covered + # by the opt-in real-S3 test; here only the listing and pairing are checked. + backend = build_backend(s3_settings) + upload_id = await backend.create_multipart("t/stale.bin", "application/octet-stream") + + assert await backend.list_stale_multipart(timedelta(days=1)) == [("t/stale.bin", upload_id)] + await backend.abort_multipart("t/stale.bin", upload_id) + assert await backend.list_stale_multipart(timedelta(days=1)) == [] + + +class TestCsp: + def test_custom_endpoint_contributes_its_origin(self, s3_settings): + assert s3_connect_origins(s3_settings) == [s3_settings.s3_endpoint_url] + + def test_public_endpoint_wins_and_loses_its_path(self, s3_settings): + settings = s3_settings.model_copy( + update={"s3_public_endpoint_url": "https://files.example.test/prefix?x=1"} + ) + assert s3_connect_origins(settings) == ["https://files.example.test"] + + def test_aws_default_allows_the_bucket_hosts(self): + settings = FileStorageSettings( + backend=constants.BackendId.S3, s3_bucket="b", s3_region="eu-west-1" + ) + assert s3_connect_origins(settings) == [ + "https://b.s3.eu-west-1.amazonaws.com", + "https://b.s3.amazonaws.com", + ] + + def test_filesystem_adds_nothing(self): + assert s3_connect_origins(FileStorageSettings()) == [] + + +async def test_api_flow_uploads_straight_to_the_bucket( + app, authenticated_client, s3_settings, fresh_bucket +): + services = app.state.file_storage + services.backend = build_backend(s3_settings) + services.settings.multipart_part_size_bytes = PART + data = b"z" * PART + b"end" + + session = (await open_session(authenticated_client, total=len(data))).json() + urls = await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [1, 2]} + ) + body = urls.json() + assert body["direct"] is True + assert body["urls"]["1"].startswith(s3_settings.s3_endpoint_url) + + etags = { + 1: await _put(body["urls"]["1"], data[:PART]), + 2: await _put(body["urls"]["2"], data[PART:]), + } + resumed = (await authenticated_client.get(f"{API}/uploads/{session['id']}")).json() + assert [p["no"] for p in resumed["parts"]] == [1, 2] + + done = await authenticated_client.post( + f"{API}/uploads/{session['id']}/complete", + json={"parts": [{"n": n, "etag": e} for n, e in etags.items()]}, + ) + assert done.status_code == 200, done.text + file = done.json()["file"] + assert file["backend"] == constants.BackendId.S3 + assert file["size_bytes"] == len(data) + assert file["checksum_sha256"] is None + assert fresh_bucket.head_object(Bucket=_BUCKET, Key=file["key"])["ContentLength"] == len(data) + + +async def test_api_rejects_an_object_of_the_wrong_size( + app, authenticated_client, s3_settings, fresh_bucket +): + """A client that lied about ``total_size`` gets no StoredFile and no orphan object.""" + services = app.state.file_storage + services.backend = build_backend(s3_settings) + services.settings.multipart_part_size_bytes = PART + + session = (await open_session(authenticated_client, total=PART + 10)).json() + urls = ( + await authenticated_client.post( + f"{API}/uploads/{session['id']}/part-urls", json={"parts": [1, 2]} + ) + ).json()["urls"] + etags = {1: await _put(urls["1"], b"a" * PART), 2: await _put(urls["2"], b"b" * 3)} + + resp = await authenticated_client.post( + f"{API}/uploads/{session['id']}/complete", + json={"parts": [{"n": n, "etag": e} for n, e in etags.items()]}, + ) + # The part-size check catches the short last part before assembly. + assert resp.status_code == 409 + assert fresh_bucket.list_objects_v2(Bucket=_BUCKET).get("Contents", []) == [] diff --git a/packages/i18n/src/generated-resources.ts b/packages/i18n/src/generated-resources.ts index 9e37dea83..a4de6401d 100644 --- a/packages/i18n/src/generated-resources.ts +++ b/packages/i18n/src/generated-resources.ts @@ -394,9 +394,15 @@ export default { 'file_storage.delete_dialog.title_other': '', 'file_storage.errors.backend_error': '', 'file_storage.errors.bad_image': '', + 'file_storage.errors.bad_part': '', 'file_storage.errors.bad_type': '', + 'file_storage.errors.direct_upload_unsupported': '', 'file_storage.errors.not_found': '', + 'file_storage.errors.parts_mismatch': '', + 'file_storage.errors.quota_exceeded': '', + 'file_storage.errors.size_mismatch': '', 'file_storage.errors.too_large': '', + 'file_storage.errors.upload_session_closed': '', 'file_storage.filters.search_placeholder': '', 'file_storage.filters.type_all': '', 'file_storage.filters.type_label': '', @@ -436,7 +442,10 @@ export default { 'file_storage.upload.drop_here': '', 'file_storage.upload.failed_reason': '', 'file_storage.upload.in_progress': '', + 'file_storage.upload.resume': '', + 'file_storage.upload.resume_prompt': '', 'file_storage.upload.retry': '', + 'file_storage.upload.start_over': '', 'host.admin.description': '', 'host.admin.empty': '', 'host.admin.overview': '', diff --git a/packages/i18n/src/keys.generated.ts b/packages/i18n/src/keys.generated.ts index e7b4eb8d4..d58ec3dee 100644 --- a/packages/i18n/src/keys.generated.ts +++ b/packages/i18n/src/keys.generated.ts @@ -502,9 +502,15 @@ export const keys = { errors: { backend_error: 'file_storage.errors.backend_error', bad_image: 'file_storage.errors.bad_image', + bad_part: 'file_storage.errors.bad_part', bad_type: 'file_storage.errors.bad_type', + direct_upload_unsupported: 'file_storage.errors.direct_upload_unsupported', not_found: 'file_storage.errors.not_found', + parts_mismatch: 'file_storage.errors.parts_mismatch', + quota_exceeded: 'file_storage.errors.quota_exceeded', + size_mismatch: 'file_storage.errors.size_mismatch', too_large: 'file_storage.errors.too_large', + upload_session_closed: 'file_storage.errors.upload_session_closed', }, filters: { search_placeholder: 'file_storage.filters.search_placeholder', @@ -557,7 +563,10 @@ export const keys = { drop_here: 'file_storage.upload.drop_here', failed_reason: 'file_storage.upload.failed_reason', in_progress: 'file_storage.upload.in_progress', + resume: 'file_storage.upload.resume', + resume_prompt: 'file_storage.upload.resume_prompt', retry: 'file_storage.upload.retry', + start_over: 'file_storage.upload.start_over', }, }, host: {