diff --git a/README.md b/README.md index dbe8940..9765bf0 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ The publisher accepts a banner image (JPEG, PNG, WebP, or GIF, up to 8 MB) and u The `article_media` table stores the upload-ID-to-Dropbox-ID mapping, including uploads not yet attached to a published article. The public `/api/uploads/{upload_id}` route retrieves bytes from Dropbox by ID, so article rows never contain expiring temporary links or access tokens. Original local uploads continue to work until migrated. Saving an attachment removal or banner replacement deletes the unused file from Dropbox and removes its upload record and any local migration backup. Deleting an article also cleans up its uploaded files. Files referenced by another article are retained. -Removal intent is stored in the private `article_media_deletions` table before the article write. Cleanup runs after a successful save/delete and retries pending work at backend startup and every 60 seconds; Dropbox outages do not lose deletion requests. Cleanup checks published references before deleting anything. In local mode, the queue lives in `backend/data/media-deletions.json`. The form tracks uploads removed before publishing as well; removals take effect when the form is successfully submitted. Apply the current schema to existing Supabase projects before running this version. Article writes and cleanup are serialized within the backend process; run a single backend worker for this workflow. +Removal intent is stored in the private `article_media_deletions` table before the article write. Upload metadata and removal-queue upserts retry temporary Supabase failures up to three times, using the same upload IDs; after a timeout or incomplete response, the server reads back the exact records to confirm whether the write already succeeded. Cleanup runs after a successful save/delete and retries pending work at backend startup and every 60 seconds; Dropbox outages do not lose deletion requests. Cleanup checks published references before deleting anything. In local mode, the queue lives in `backend/data/media-deletions.json`. The form tracks uploads removed before publishing as well; removals take effect when the form is successfully submitted. Apply the current schema to existing Supabase projects before running this version. Article writes and cleanup are serialized within the backend process; run a single backend worker for this workflow. For an existing Supabase project, run `bash scripts/setup_supabase.sh --schema-only` to add the nullable `banner` and default-empty `attachments` columns before running the updated backend. Existing articles remain compatible. diff --git a/backend/main.py b/backend/main.py index 450d264..e61b27b 100644 --- a/backend/main.py +++ b/backend/main.py @@ -27,7 +27,7 @@ from pydantic import BaseModel, EmailStr, Field from starlette.concurrency import run_in_threadpool from httpx import TransportError from supabase import Client, ClientOptions, PostgrestAPIError, create_client -from backend import dropbox_storage, media_cleanup, portfolio_content +from backend import dropbox_storage, media_cleanup, media_registry, portfolio_content BASE_DIR = Path(__file__).resolve().parent @@ -333,9 +333,7 @@ def save_media_metadata(upload_id: str, metadata: dict[str, Any]) -> None: client = get_supabase() if client is not None: try: - result = client.table("article_media").upsert({"upload_id": upload_id, "metadata": metadata}).execute() - if not result.data: - raise RuntimeError("Media record was not returned.") + media_registry.upsert(client, "article_media", [{"upload_id": upload_id, "metadata": metadata}]) except Exception as exc: raise HTTPException(502, "The file uploaded, but its record could not be saved. Please retry.") from exc else: diff --git a/backend/media_cleanup.py b/backend/media_cleanup.py index fe28aba..311bdf9 100644 --- a/backend/media_cleanup.py +++ b/backend/media_cleanup.py @@ -9,6 +9,7 @@ from threading import RLock from fastapi import HTTPException from starlette.concurrency import run_in_threadpool +from backend import media_registry LOCK = RLock() TABLE = "article_media_deletions" @@ -62,9 +63,7 @@ def enqueue(media): client = main.get_supabase() try: if client is not None: - result = client.table(TABLE).upsert(list(records.values())).execute() - if not result.data: - raise RuntimeError("Removal queue was not saved") + media_registry.upsert(client, TABLE, list(records.values())) else: save_local_queue(list({**{r["upload_id"]: r for r in pending()}, **records}.values())) except Exception as exc: diff --git a/backend/media_registry.py b/backend/media_registry.py new file mode 100644 index 0000000..20f0a63 --- /dev/null +++ b/backend/media_registry.py @@ -0,0 +1,54 @@ +"""Confirmed, safely retryable writes for immutable upload-ID metadata.""" +import logging +import re +import time + +from httpx import TransportError +from supabase import PostgrestAPIError + +logger = logging.getLogger("uvicorn.error") +TRANSIENT_CODES = {"500", "502", "503", "504", "520", "522", "524"} + + +class UnconfirmedMediaWrite(RuntimeError): + pass + + +def matches(rows, records): + saved = {row["upload_id"]: row["metadata"] for row in (rows or [])} + return all(saved.get(record["upload_id"]) == record["metadata"] for record in records) + + +def upsert(client, table, records): + """Retry only identical upserts by primary key, never article inserts/deletes. + + A timeout can arrive after commit. Read back the exact batch to confirm it, + including servers/proxies that return no representation of a successful write. + """ + for attempt in range(3): + try: + result = client.table(table).upsert(records, on_conflict="upload_id").retry(False).execute() + if matches(result.data, records): + return + raise UnconfirmedMediaWrite("Media write returned incomplete confirmation") + except Exception as exc: + code = str(getattr(exc, "code", "")) + transient = isinstance(exc, (TransportError, UnconfirmedMediaWrite)) or ( + isinstance(exc, PostgrestAPIError) and code in TRANSIENT_CODES + ) + safe_code = code if re.fullmatch(r"[A-Za-z0-9_]{1,20}", code) else "unknown" + logger.warning("Media registry write: table=%s type=%s code=%s attempt=%s", table, type(exc).__name__, safe_code, attempt + 1) + if not transient: + raise + try: + confirmed = client.table(table).select("upload_id,metadata").in_( + "upload_id", [record["upload_id"] for record in records], + ).retry(False).execute() + if matches(confirmed.data, records): + return + except Exception: + # Preserve the original write failure if neither operation works. + pass + if attempt == 2: + raise + time.sleep(0.25 * (attempt + 1)) diff --git a/backend/test_articles.py b/backend/test_articles.py index 70b5e76..e90e31e 100644 --- a/backend/test_articles.py +++ b/backend/test_articles.py @@ -216,13 +216,14 @@ class ArticleMediaTests(unittest.TestCase): client.table.side_effect = lambda name: media_table if name == "article_media" else article_table metadata = {"url": "/api/uploads/" + "a" * 32, "name": "image.png", "size": 10, "media_type": "image/png", "storage": "dropbox", "dropbox_file_id": "id:remote_photo"} + media_table.upsert.return_value.retry.return_value.execute.return_value.data = [{"upload_id": "a" * 32, "metadata": metadata}] media_query = media_table.select.return_value.eq.return_value.limit.return_value media_query.retry.return_value.execute.return_value.data = [{"metadata": metadata}] article_table.select.return_value.eq.return_value.limit.return_value.execute.return_value.data = [] article_table.insert.side_effect = lambda record: MagicMock(execute=lambda: MagicMock(data=[record])) with patch.object(main, "get_supabase", return_value=client): main.save_media_metadata("a" * 32, metadata) - media_table.upsert.assert_called_once_with({"upload_id": "a" * 32, "metadata": metadata}) + media_table.upsert.assert_called_once_with([{"upload_id": "a" * 32, "metadata": metadata}], on_conflict="upload_id") self.assertEqual(main.uploaded_media("a" * 32), metadata) self.assertFalse(main.UPLOAD_DIR.exists()) result = self.client.post("/api/posts", headers=self.headers, json={**self.article_payload(), "banner": metadata}) diff --git a/backend/test_media_registry.py b/backend/test_media_registry.py new file mode 100644 index 0000000..b81dacd --- /dev/null +++ b/backend/test_media_registry.py @@ -0,0 +1,71 @@ +import unittest +from unittest.mock import MagicMock, patch +from httpx import ReadTimeout +from supabase import PostgrestAPIError +from fastapi import HTTPException + +from backend import main, media_cleanup, media_registry + + +class MediaRegistryTests(unittest.TestCase): + def setUp(self): + self.client = MagicMock() + self.table = self.client.table.return_value + self.write = self.table.upsert.return_value.retry.return_value.execute + self.read = self.table.select.return_value.in_.return_value.retry.return_value.execute + self.records = [{"upload_id": char * 32, "metadata": { + "url": "/api/uploads/" + char * 32, "storage": "dropbox", "dropbox_file_id": "id:file_" + char, + "name": "photo.png", "media_type": "image/png", "size": 100, + }} for char in ("a", "b")] + self.read.return_value.data = [] + sleep = patch.object(media_registry.time, "sleep") + sleep.start(); self.addCleanup(sleep.stop) + + def test_queue_retries_gateway_timeout_with_identical_records(self): + self.write.side_effect = [PostgrestAPIError({"code": "504", "message": "Gateway Timeout"}), MagicMock(data=self.records)] + by_id = {row["upload_id"]: row["metadata"] for row in self.records} + with patch.object(main, "get_supabase", return_value=self.client), patch.object(main, "uploaded_media", side_effect=lambda uid: by_id[uid]): + media_cleanup.enqueue(list(by_id.values())) + self.assertEqual(self.write.call_count, 2) + for call in self.table.upsert.call_args_list: + self.assertEqual(call.args[0], self.records) + self.assertEqual(call.kwargs, {"on_conflict": "upload_id"}) + + def test_committed_timeout_is_confirmed_without_another_write(self): + self.write.side_effect = ReadTimeout("response lost after commit") + self.read.return_value.data = list(reversed(self.records)) + media_registry.upsert(self.client, "article_media_deletions", self.records) + self.assertEqual(self.write.call_count, 1) + + def test_empty_response_checks_every_record_before_success(self): + self.write.side_effect = [MagicMock(data=[]), MagicMock(data=self.records)] + self.read.return_value.data = self.records[:1] + media_registry.upsert(self.client, "article_media", self.records) + self.assertEqual(self.write.call_count, 2) + + def test_empty_response_with_all_records_saved_is_success(self): + self.write.return_value.data = [] + self.read.return_value.data = self.records + media_registry.upsert(self.client, "article_media", self.records) + self.assertEqual(self.write.call_count, 1) + + def test_exhausted_retries_still_block_article_mutation(self): + self.write.side_effect = ReadTimeout("offline") + by_id = {row["upload_id"]: row["metadata"] for row in self.records} + with patch.object(main, "get_supabase", return_value=self.client), patch.object(main, "uploaded_media", side_effect=lambda uid: by_id[uid]), patch.object(main, "post_by_slug", return_value={"banner": self.records[0]["metadata"], "attachments": [self.records[1]["metadata"]]}): + with self.assertRaises(HTTPException) as error: + main.delete_post("existing-article") + self.assertEqual(error.exception.status_code, 502) + self.assertEqual(self.write.call_count, 3) + self.table.delete.assert_not_called() + + def test_configuration_errors_are_not_retried(self): + self.write.side_effect = PostgrestAPIError({"code": "42501", "message": "permission denied"}) + with self.assertRaises(PostgrestAPIError): + media_registry.upsert(self.client, "article_media", self.records) + self.assertEqual(self.write.call_count, 1) + self.read.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/frontend/src/components/Layout.jsx b/frontend/src/components/Layout.jsx index 3d0599b..e03b07c 100644 --- a/frontend/src/components/Layout.jsx +++ b/frontend/src/components/Layout.jsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useId, useState } from "react"; import { NavLink, useLocation } from "react-router-dom"; import Icon from "./Icon"; import { useAdmin } from "./AdminSession"; @@ -14,6 +14,7 @@ const navItems = [ function Header({ name }) { const { isAdmin, checking, openSignIn } = useAdmin(); const [menuOpen, setMenuOpen] = useState(false); + const navigationId = useId(); const location = useLocation(); useEffect(() => setMenuOpen(false), [location.pathname]); @@ -27,7 +28,9 @@ function Header({ name }) {
-
); diff --git a/frontend/src/styles.css b/frontend/src/styles.css index 3455307..3b74649 100644 --- a/frontend/src/styles.css +++ b/frontend/src/styles.css @@ -128,10 +128,11 @@ button { } .site-header { + --header-top: 22px; position: relative; z-index: 50; flex: 0 0 auto; - padding-block: 22px 10px; + padding-block: var(--header-top) 10px; } .header-inner { @@ -178,6 +179,12 @@ button { backdrop-filter: blur(18px); } +/* Only navigation is pinned. Identity and account controls stay in normal flow. */ +.nav-positioner { position: fixed; top: var(--header-top); inset-inline: 0; pointer-events: none; } +.nav-positioner__inner { position: relative; display: flex; align-items: center; justify-content: flex-end; min-height: 58px; padding-right: 56px; } +.nav-positioner .nav-shell, .nav-positioner .menu-button { pointer-events: auto; } +@media (min-width: 721px) and (max-width: 900px) { .brand-name { display: none; } } + .nav-link { padding: 10px 15px; border-radius: 13px; @@ -2383,9 +2390,13 @@ button { } .site-header { + --header-top: 14px; padding-top: 14px; } + .nav-positioner__inner { padding-right: 0; } + .site-header .header-actions { padding-right: 56px; } + .header-inner { position: relative; } @@ -2707,6 +2718,7 @@ button { @media (max-height: 770px) and (min-width: 901px) { .site-header { + --header-top: 14px; padding-top: 14px; }