diff --git a/backend/app/api/__init__.py b/backend/app/api/__init__.py index a2d743b..4a88c1e 100644 --- a/backend/app/api/__init__.py +++ b/backend/app/api/__init__.py @@ -20,6 +20,7 @@ def all_blueprints() -> list[Blueprint]: from .artist import artist_bp from .artists import artists_bp from .attachments import attachments_bp + from .cleanup import cleanup_bp from .credentials import credentials_bp from .downloads import downloads_bp from .extension import extension_bp @@ -50,6 +51,7 @@ def all_blueprints() -> list[Blueprint]: system_activity_bp, system_backup_bp, admin_bp, + cleanup_bp, import_admin_bp, migrate_bp, suggestions_bp, diff --git a/backend/app/api/cleanup.py b/backend/app/api/cleanup.py new file mode 100644 index 0000000..386c3e5 --- /dev/null +++ b/backend/app/api/cleanup.py @@ -0,0 +1,193 @@ +"""FC-Cleanup: /api/cleanup/* — retroactive enforcement of import filters. + +Endpoints: + POST /min-dimension/preview synchronous SQL audit + POST /min-dimension/delete synchronous SQL delete (Tier-C token) + POST /audit async transparency / single_color start + GET /audit list recent audit_run rows + GET /audit/ single audit_run row + POST /audit//apply apply matched_ids deletes (Tier-C token) + POST /audit//cancel flip running audit to cancelled + +Unused-tags retroactive prune intentionally NOT in this namespace — +TagMaintenanceCard (Maintenance tab → moved to Cleanup tab in v26.05.25.7) +uses the existing /api/admin/tags/prune-unused endpoint via the admin +store. No duplicate route here. + +Confirm-token format matches modal/DestructiveConfirmModal.vue convention: + `delete-min-dim-` for min-dim delete + `delete-audit-` for audit apply +(Modal hardcodes action ∈ {'restore', 'delete'}; "apply audit" is semantically a delete of the matched images, so we use `delete-audit-`.) +""" + +from __future__ import annotations + +import hashlib +from pathlib import Path + +from quart import Blueprint, jsonify, request +from sqlalchemy import select + +from ..extensions import get_session +from ..models import LibraryAuditRun +from ..services import cleanup_service + +cleanup_bp = Blueprint("cleanup", __name__, url_prefix="/api/cleanup") + +IMAGES_ROOT = Path("/images") + + +def _bad(error: str, *, status: int = 400, **extra): + body = {"error": error} + body.update(extra) + return jsonify(body), status + + +def _min_dim_token(min_w: int, min_h: int) -> str: + # SHA-256 (not MD5) — Web Crypto's subtle.digest rejects MD5; both + # sides use SHA-256 truncated to 8 hex chars. + canon = f"{min_w}x{min_h}" + return f"delete-min-dim-{hashlib.sha256(canon.encode()).hexdigest()[:8]}" + + +def _serialize_audit_run(audit: LibraryAuditRun) -> dict: + return { + "id": audit.id, + "rule": audit.rule, + "params": audit.params, + "status": audit.status, + "started_at": audit.started_at.isoformat() if audit.started_at else None, + "finished_at": audit.finished_at.isoformat() if audit.finished_at else None, + "scanned_count": audit.scanned_count, + "matched_count": audit.matched_count, + "matched_ids": audit.matched_ids, + "error": audit.error, + } + + +@cleanup_bp.route("/min-dimension/preview", methods=["POST"]) +async def min_dim_preview(): + body = await request.get_json(silent=True) or {} + try: + min_w = int(body.get("min_width", 0)) + min_h = int(body.get("min_height", 0)) + except (TypeError, ValueError): + return _bad("invalid_dimensions") + if min_w < 0 or min_h < 0: + return _bad("invalid_dimensions") + async with get_session() as session: + projection = await session.run_sync( + lambda s: cleanup_service.project_min_dimension_violations( + s, min_width=min_w, min_height=min_h, + ) + ) + return jsonify(projection) + + +@cleanup_bp.route("/min-dimension/delete", methods=["POST"]) +async def min_dim_delete(): + body = await request.get_json(silent=True) or {} + try: + min_w = int(body.get("min_width", 0)) + min_h = int(body.get("min_height", 0)) + except (TypeError, ValueError): + return _bad("invalid_dimensions") + if min_w < 0 or min_h < 0: + return _bad("invalid_dimensions") + supplied = body.get("confirm", "") + expected = _min_dim_token(min_w, min_h) + if supplied != expected: + return _bad("confirm_mismatch", expected=expected) + async with get_session() as session: + deleted = await session.run_sync( + lambda s: cleanup_service.delete_min_dimension_violations( + s, min_width=min_w, min_height=min_h, images_root=IMAGES_ROOT, + ) + ) + await session.commit() + return jsonify({"deleted": deleted}) + + +@cleanup_bp.route("/audit", methods=["POST"]) +async def audit_create(): + body = await request.get_json(silent=True) or {} + rule = body.get("rule") + params = body.get("params") or {} + if rule not in ("transparency", "single_color"): + return _bad("invalid_rule") + if not isinstance(params, dict): + return _bad("invalid_params") + async with get_session() as session: + try: + audit_id = await session.run_sync( + lambda s: cleanup_service.start_audit_run( + s, rule=rule, params=params, + ) + ) + except cleanup_service.AuditAlreadyRunning as running_id: + return _bad( + "audit_already_running", status=409, + running_id=int(str(running_id)), + ) + except ValueError as exc: + return _bad(str(exc)) + await session.commit() + return jsonify({"audit_id": audit_id, "status": "running"}), 202 + + +@cleanup_bp.route("/audit/", methods=["GET"]) +async def audit_get(audit_id: int): + async with get_session() as session: + audit = (await session.execute( + select(LibraryAuditRun).where(LibraryAuditRun.id == audit_id) + )).scalar_one_or_none() + if audit is None: + return _bad("not_found", status=404) + return jsonify(_serialize_audit_run(audit)) + + +@cleanup_bp.route("/audit", methods=["GET"]) +async def audit_history(): + try: + limit = min(int(request.args.get("limit", "20")), 100) + except ValueError: + return _bad("invalid_limit") + async with get_session() as session: + rows = (await session.execute( + select(LibraryAuditRun) + .order_by(LibraryAuditRun.id.desc()) + .limit(limit) + )).scalars().all() + return jsonify({"runs": [_serialize_audit_run(r) for r in rows]}) + + +@cleanup_bp.route("/audit//apply", methods=["POST"]) +async def audit_apply(audit_id: int): + body = await request.get_json(silent=True) or {} + confirm = body.get("confirm", "") + async with get_session() as session: + try: + deleted = await session.run_sync( + lambda s: cleanup_service.apply_audit_run( + s, audit_id=audit_id, confirm_token=confirm, + images_root=IMAGES_ROOT, + ) + ) + except cleanup_service.AuditNotReady as exc: + return _bad("audit_not_ready", current_status=str(exc)) + except cleanup_service.ConfirmTokenMismatch as exc: + return _bad("confirm_mismatch", expected=str(exc)) + except ValueError as exc: + return _bad("not_found", status=404, detail=str(exc)) + await session.commit() + return jsonify({"deleted": deleted}) + + +@cleanup_bp.route("/audit//cancel", methods=["POST"]) +async def audit_cancel(audit_id: int): + async with get_session() as session: + await session.run_sync( + lambda s: cleanup_service.cancel_audit_run(s, audit_id=audit_id) + ) + await session.commit() + return jsonify({"cancelled": True}) diff --git a/backend/app/api/extension.py b/backend/app/api/extension.py index 54d569f..4f4766f 100644 --- a/backend/app/api/extension.py +++ b/backend/app/api/extension.py @@ -93,10 +93,20 @@ def _read_manifest_sync() -> dict | None: asyncio.to_thread (ASYNC240: no pathlib I/O in async functions).""" if not XPI_DIR.is_dir(): return None - xpis = sorted(XPI_DIR.glob("fabledcurator-*.xpi"), key=lambda p: p.stat().st_mtime) - if not xpis: + # Exclude the `fabledcurator-latest.xpi` alias when picking the file to + # extract a version from — it's a copy of the latest versioned XPI, + # written at the same mtime by build.yml, and would otherwise tie or + # win the sort (operator-flagged 2026-05-26: UI displayed "v latest" + # because `_extract_version("fabledcurator-latest.xpi")` returns + # the literal "latest"). The alias still serves as `latest_url`. + versioned = [ + p for p in XPI_DIR.glob("fabledcurator-*.xpi") + if p.name != "fabledcurator-latest.xpi" + ] + if not versioned: return None - latest = xpis[-1] + versioned.sort(key=lambda p: p.stat().st_mtime) + latest = versioned[-1] return { "installed": True, "version": _extract_version(latest.name), diff --git a/backend/app/services/cleanup_service.py b/backend/app/services/cleanup_service.py index b8325d5..7025b8b 100644 --- a/backend/app/services/cleanup_service.py +++ b/backend/app/services/cleanup_service.py @@ -481,7 +481,11 @@ def apply_audit_run( raise ValueError(f"audit_run {audit_id} not found") if audit.status != "ready": raise AuditNotReady(audit.status) - expected = f"apply-audit-{audit_id}" + # Token format matches modal/DestructiveConfirmModal.vue convention: + # ${action}-${kind}-${runId}. The modal hardcodes action ∈ {'restore', + # 'delete'}; "apply audit" is semantically a delete of the matched + # images, so we use 'delete-audit-' (not 'apply-audit-'). + expected = f"delete-audit-{audit_id}" if confirm_token != expected: raise ConfirmTokenMismatch(expected) ids = list(audit.matched_ids or []) diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index fb9d0e3..3223ec7 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -18,6 +18,7 @@ from pathlib import Path from PIL import Image from sqlalchemy import select +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import Session from ..models import ( @@ -202,6 +203,80 @@ class Importer: (phash, width or 0, height or 0, image_id) ) + def _find_or_create_source( + self, *, artist_id: int, platform: str, url: str, + ) -> Source: + """Race-safe find-or-create on `source` keyed by + (artist_id, platform, url) — the same key as the + `uq_source_artist_platform_url` constraint. + + Two concurrent workers processing different files in the same + post can both find no existing Source row then both INSERT, + which trips the unique constraint and poisons the session with + `psycopg.errors.UniqueViolation`. Operator-flagged 2026-05-26. + + Pattern: select; if absent, open a savepoint and INSERT. + On IntegrityError, roll the savepoint back (NOT the outer + transaction, which would lose the surrounding scan's progress) + and re-select — the concurrent op just created the row we + wanted, so the second select will find it. + """ + existing = self.session.execute( + select(Source).where( + Source.artist_id == artist_id, + Source.platform == platform, + Source.url == url, + ) + ).scalar_one_or_none() + if existing is not None: + return existing + sp = self.session.begin_nested() + try: + row = Source(artist_id=artist_id, platform=platform, url=url) + self.session.add(row) + self.session.flush() + sp.commit() + return row + except IntegrityError: + sp.rollback() + return self.session.execute( + select(Source).where( + Source.artist_id == artist_id, + Source.platform == platform, + Source.url == url, + ) + ).scalar_one() + + def _find_or_create_post( + self, *, source_id: int, external_post_id: str, + ) -> Post: + """Race-safe find-or-create on `post` keyed by + (source_id, external_post_id). Mirrors `_find_or_create_source` + — same savepoint + IntegrityError-recovery pattern.""" + existing = self.session.execute( + select(Post).where( + Post.source_id == source_id, + Post.external_post_id == external_post_id, + ) + ).scalar_one_or_none() + if existing is not None: + return existing + sp = self.session.begin_nested() + try: + row = Post(source_id=source_id, external_post_id=external_post_id) + self.session.add(row) + self.session.flush() + sp.commit() + return row + except IntegrityError: + sp.rollback() + return self.session.execute( + select(Post).where( + Post.source_id == source_id, + Post.external_post_id == external_post_id, + ) + ).scalar_one() + def import_one(self, source: Path) -> ImportResult: """Dispatch by kind. Media → normal pipeline. Archive → extract media members (one Post via the archive-adjacent sidecar) and @@ -241,29 +316,13 @@ class Importer: sd = parse_sidecar(data) platform = sd.platform or "unknown" url = sd.post_url or f"sidecar:{platform}" - src = self.session.execute( - select(Source).where( - Source.artist_id == artist.id, - Source.platform == platform, - Source.url == url, - ) - ).scalar_one_or_none() - if src is None: - src = Source(artist_id=artist.id, platform=platform, url=url) - self.session.add(src) - self.session.flush() + src = self._find_or_create_source( + artist_id=artist.id, platform=platform, url=url, + ) epid = sd.external_post_id or sc.stem - post = self.session.execute( - select(Post).where( - Post.source_id == src.id, - Post.external_post_id == epid, - ) - ).scalar_one_or_none() - if post is None: - post = Post(source_id=src.id, external_post_id=epid) - self.session.add(post) - self.session.flush() - return post + return self._find_or_create_post( + source_id=src.id, external_post_id=epid, + ) def _capture_attachment( self, source: Path, *, post: Post | None = None, @@ -705,29 +764,14 @@ class Importer: else: platform = sd.platform or "unknown" url = sd.post_url or f"sidecar:{platform}" - src = self.session.execute( - select(Source).where( - Source.artist_id == artist.id, - Source.platform == platform, - Source.url == url, - ) - ).scalar_one_or_none() - if src is None: - src = Source(artist_id=artist.id, platform=platform, url=url) - self.session.add(src) - self.session.flush() + src = self._find_or_create_source( + artist_id=artist.id, platform=platform, url=url, + ) epid = sd.external_post_id or sc.stem - post = self.session.execute( - select(Post).where( - Post.source_id == src.id, - Post.external_post_id == epid, - ) - ).scalar_one_or_none() - if post is None: - post = Post(source_id=src.id, external_post_id=epid) - self.session.add(post) - self.session.flush() + post = self._find_or_create_post( + source_id=src.id, external_post_id=epid, + ) if sd.post_url is not None: post.post_url = sd.post_url if sd.post_title is not None: diff --git a/frontend/src/components/cleanup/MinDimensionCard.vue b/frontend/src/components/cleanup/MinDimensionCard.vue new file mode 100644 index 0000000..a26ae2d --- /dev/null +++ b/frontend/src/components/cleanup/MinDimensionCard.vue @@ -0,0 +1,127 @@ + + + + + diff --git a/frontend/src/components/cleanup/SingleColorAuditCard.vue b/frontend/src/components/cleanup/SingleColorAuditCard.vue new file mode 100644 index 0000000..ff2ecbe --- /dev/null +++ b/frontend/src/components/cleanup/SingleColorAuditCard.vue @@ -0,0 +1,183 @@ + + + + + diff --git a/frontend/src/components/cleanup/TransparencyAuditCard.vue b/frontend/src/components/cleanup/TransparencyAuditCard.vue new file mode 100644 index 0000000..4e91ac0 --- /dev/null +++ b/frontend/src/components/cleanup/TransparencyAuditCard.vue @@ -0,0 +1,166 @@ + + + + + diff --git a/frontend/src/components/common/ErrorDetailModal.vue b/frontend/src/components/common/ErrorDetailModal.vue new file mode 100644 index 0000000..d8281a5 --- /dev/null +++ b/frontend/src/components/common/ErrorDetailModal.vue @@ -0,0 +1,87 @@ + + + + + diff --git a/frontend/src/components/settings/BrowserExtensionCard.vue b/frontend/src/components/settings/BrowserExtensionCard.vue index 2dbe591..db5d3a3 100644 --- a/frontend/src/components/settings/BrowserExtensionCard.vue +++ b/frontend/src/components/settings/BrowserExtensionCard.vue @@ -34,11 +34,18 @@
@@ -100,16 +102,35 @@ + + + + diff --git a/frontend/src/components/settings/MaintenancePanel.vue b/frontend/src/components/settings/MaintenancePanel.vue index 0f39bac..90c410b 100644 --- a/frontend/src/components/settings/MaintenancePanel.vue +++ b/frontend/src/components/settings/MaintenancePanel.vue @@ -13,7 +13,9 @@ - +
@@ -25,7 +27,6 @@ import MLThresholdSliders from './MLThresholdSliders.vue' import AllowlistTable from './AllowlistTable.vue' import AliasTable from './AliasTable.vue' import BackupCard from './BackupCard.vue' -import TagMaintenanceCard from './TagMaintenanceCard.vue' import LegacyMigrationCard from './LegacyMigrationCard.vue' diff --git a/frontend/src/components/settings/SystemActivityTab.vue b/frontend/src/components/settings/SystemActivityTab.vue index 1db2724..f623bb0 100644 --- a/frontend/src/components/settings/SystemActivityTab.vue +++ b/frontend/src/components/settings/SystemActivityTab.vue @@ -59,8 +59,12 @@ {{ r.queue }} {{ shortTaskName(r.task_name) }} {{ r.target_id ?? '—' }} - - {{ r.error_type }} + + @@ -138,6 +142,12 @@ + + @@ -145,8 +155,23 @@ import { computed, onMounted, onUnmounted, ref } from 'vue' import { useSystemActivityStore } from '../../stores/systemActivity.js' +import ErrorDetailModal from '../common/ErrorDetailModal.vue' import QueuesTable from './QueuesTable.vue' +// Click-to-open modal for full error text. Replaces the unusable +// :title="..." tooltip (operator-flagged 2026-05-26: SQLAlchemy +// rollback + traceback content rendered as a cramped browser tooltip +// you couldn't copy from or scroll within). +const showErrorModal = ref(false) +const errorModalTitle = ref('') +const errorModalMessage = ref('') + +function openError(title, message) { + errorModalTitle.value = title || 'Error details' + errorModalMessage.value = message || '' + showErrorModal.value = true +} + const store = useSystemActivityStore() const filterQueue = ref(null) @@ -276,5 +301,18 @@ function formatRelative(iso) { font-feature-settings: 'tnum'; } .fc-err { color: rgb(var(--v-theme-error, 220 80 80)); } +.fc-err-link { + /* Styled as a text-only button so the error_type cell stays + visually identical to the prior tooltip-bearing row, but is + now a real clickable target with hover affordance. */ + color: rgb(var(--v-theme-error, 220 80 80)); + background: transparent; + border: 0; + padding: 0; + font: inherit; + text-decoration: underline dotted; + cursor: pointer; +} +.fc-err-link:hover { text-decoration: underline; } .fc-muted { color: rgb(var(--v-theme-on-surface-variant)); } diff --git a/frontend/src/stores/cleanup.js b/frontend/src/stores/cleanup.js new file mode 100644 index 0000000..fac5112 --- /dev/null +++ b/frontend/src/stores/cleanup.js @@ -0,0 +1,72 @@ +import { defineStore } from 'pinia' +import { ref } from 'vue' +import { useApi } from '../composables/useApi.js' + +export const useCleanupStore = defineStore('cleanup', () => { + const api = useApi() + + // Defaults sourced from ImportSettings on mount. Cards pre-fill from + // these so the common case ("apply current import filters + // retroactively") is one click; operator can override per-audit. + const defaults = ref({ + min_width: 0, + min_height: 0, + transparency_threshold: 0.9, + single_color_threshold: 0.95, + single_color_tolerance: 30, + }) + + const recentRuns = ref([]) + + async function loadDefaults() { + const s = await api.get('/api/settings/import') + defaults.value = { + min_width: s.min_width ?? 0, + min_height: s.min_height ?? 0, + transparency_threshold: s.transparency_threshold ?? 0.9, + single_color_threshold: s.single_color_threshold ?? 0.95, + single_color_tolerance: s.single_color_tolerance ?? 30, + } + } + + async function previewMinDim(min_width, min_height) { + return await api.post('/api/cleanup/min-dimension/preview', { + body: { min_width, min_height }, + }) + } + + async function deleteMinDim(min_width, min_height, confirm) { + return await api.post('/api/cleanup/min-dimension/delete', { + body: { min_width, min_height, confirm }, + }) + } + + async function startAudit(rule, params) { + return await api.post('/api/cleanup/audit', { body: { rule, params } }) + } + + async function getAudit(id) { + return await api.get(`/api/cleanup/audit/${id}`) + } + + async function loadHistory(limit = 20) { + const body = await api.get(`/api/cleanup/audit?limit=${limit}`) + recentRuns.value = body.runs + return body.runs + } + + async function applyAudit(id, confirm) { + return await api.post(`/api/cleanup/audit/${id}/apply`, { body: { confirm } }) + } + + async function cancelAudit(id) { + return await api.post(`/api/cleanup/audit/${id}/cancel`) + } + + return { + defaults, recentRuns, + loadDefaults, + previewMinDim, deleteMinDim, + startAudit, getAudit, loadHistory, applyAudit, cancelAudit, + } +}) diff --git a/frontend/src/views/CleanupView.vue b/frontend/src/views/CleanupView.vue new file mode 100644 index 0000000..f02e63a --- /dev/null +++ b/frontend/src/views/CleanupView.vue @@ -0,0 +1,31 @@ + + + + + diff --git a/frontend/src/views/SettingsView.vue b/frontend/src/views/SettingsView.vue index 8d41770..99f536d 100644 --- a/frontend/src/views/SettingsView.vue +++ b/frontend/src/views/SettingsView.vue @@ -14,6 +14,7 @@ Overview Activity Import + Cleanup Maintenance @@ -53,6 +54,10 @@ + + + + @@ -72,6 +77,7 @@ import ImportTriggerPanel from '../components/settings/ImportTriggerPanel.vue' import ImportFiltersForm from '../components/settings/ImportFiltersForm.vue' import ImportTaskList from '../components/settings/ImportTaskList.vue' import MaintenancePanel from '../components/settings/MaintenancePanel.vue' +import CleanupView from './CleanupView.vue' import { useMLStore } from '../stores/ml.js' const tab = ref('overview') diff --git a/tests/test_api_cleanup.py b/tests/test_api_cleanup.py new file mode 100644 index 0000000..ba9534f --- /dev/null +++ b/tests/test_api_cleanup.py @@ -0,0 +1,215 @@ +"""API tests for the /api/cleanup/* blueprint. + +Per reference-async-coredml-test-assertions, post-DML state checks go +via column selects, not ORM entity access. +""" + +import hashlib +from datetime import UTC, datetime + +import pytest +from PIL import Image +from sqlalchemy import func, select + +from backend.app import create_app +from backend.app.celery_app import celery +from backend.app.models import ImageRecord, LibraryAuditRun + +pytestmark = pytest.mark.integration + + +@pytest.fixture(autouse=True) +def disable_celery_eager(monkeypatch): + monkeypatch.setattr(celery.conf, "task_always_eager", False) + + +@pytest.fixture +async def app(): + return create_app() + + +@pytest.fixture +async def client(app): + async with app.test_client() as c: + yield c + + +def _sha256_min_dim_token(min_w: int, min_h: int) -> str: + canon = f"{min_w}x{min_h}" + return f"delete-min-dim-{hashlib.sha256(canon.encode()).hexdigest()[:8]}" + + +async def _seed_image(db, tmp_path, *, w, h, name): + path = tmp_path / name + Image.new("RGB", (w, h), (w % 256, h % 256, 0)).save(path) + sha = f"api-cleanup-{name}".ljust(64, "x")[:64] + rec = ImageRecord( + path=str(path), sha256=sha, + size_bytes=path.stat().st_size, mime="image/png", + width=w, height=h, origin="imported_filesystem", + integrity_status="ok", + ) + db.add(rec) + await db.flush() + return rec + + +@pytest.mark.asyncio +async def test_min_dimension_preview_returns_count(client, db, tmp_path): + await _seed_image(db, tmp_path, w=50, h=50, name="small.png") + await _seed_image(db, tmp_path, w=500, h=500, name="big.png") + await db.commit() + resp = await client.post( + "/api/cleanup/min-dimension/preview", + json={"min_width": 200, "min_height": 200}, + ) + assert resp.status_code == 200 + body = await resp.get_json() + assert body["count"] == 1 + + +@pytest.mark.asyncio +async def test_min_dimension_delete_with_token_removes_rows(client, db, tmp_path): + await _seed_image(db, tmp_path, w=50, h=50, name="s2.png") + await _seed_image(db, tmp_path, w=500, h=500, name="b2.png") + await db.commit() + token = _sha256_min_dim_token(200, 200) + resp = await client.post( + "/api/cleanup/min-dimension/delete", + json={"min_width": 200, "min_height": 200, "confirm": token}, + ) + assert resp.status_code == 200 + body = await resp.get_json() + assert body["deleted"] == 1 + remaining = await db.execute(select(func.count()).select_from(ImageRecord)) + assert remaining.scalar_one() == 1 + + +@pytest.mark.asyncio +async def test_min_dimension_delete_with_bad_token_returns_400(client, db, tmp_path): + await _seed_image(db, tmp_path, w=50, h=50, name="s3.png") + await db.commit() + resp = await client.post( + "/api/cleanup/min-dimension/delete", + json={"min_width": 200, "min_height": 200, "confirm": "nope"}, + ) + assert resp.status_code == 400 + body = await resp.get_json() + assert body["error"] == "confirm_mismatch" + + +@pytest.mark.asyncio +async def test_audit_create_returns_id_and_running_status( + client, db, monkeypatch, +): + from backend.app.tasks import library_audit + monkeypatch.setattr( + library_audit.scan_library_for_rule, "delay", lambda audit_id: None, + ) + resp = await client.post( + "/api/cleanup/audit", + json={"rule": "transparency", "params": {"threshold": 0.9}}, + ) + assert resp.status_code == 202 + body = await resp.get_json() + assert body["status"] == "running" + assert isinstance(body["audit_id"], int) + + +@pytest.mark.asyncio +async def test_audit_create_returns_409_when_another_is_running(client, db): + audit = LibraryAuditRun( + rule="transparency", params={"threshold": 0.9}, + status="running", matched_ids=[], + ) + db.add(audit) + await db.commit() + resp = await client.post( + "/api/cleanup/audit", + json={"rule": "single_color", "params": {"threshold": 0.95, "tolerance": 30}}, + ) + assert resp.status_code == 409 + + +@pytest.mark.asyncio +async def test_audit_get_by_id_returns_full_row(client, db): + audit = LibraryAuditRun( + rule="transparency", params={"threshold": 0.85}, + status="ready", scanned_count=100, matched_count=3, + matched_ids=[1, 2, 3], + ) + db.add(audit) + await db.commit() + resp = await client.get(f"/api/cleanup/audit/{audit.id}") + assert resp.status_code == 200 + body = await resp.get_json() + assert body["rule"] == "transparency" + assert body["matched_count"] == 3 + assert body["matched_ids"] == [1, 2, 3] + + +@pytest.mark.asyncio +async def test_audit_history_returns_recent_runs(client, db): + for _ in range(3): + db.add(LibraryAuditRun( + rule="transparency", params={"threshold": 0.9}, + status="applied", matched_ids=[], + finished_at=datetime.now(UTC), + )) + await db.commit() + resp = await client.get("/api/cleanup/audit?limit=5") + assert resp.status_code == 200 + body = await resp.get_json() + assert len(body["runs"]) >= 3 + + +@pytest.mark.asyncio +async def test_audit_apply_with_token_deletes(client, db, tmp_path): + rec = await _seed_image(db, tmp_path, w=100, h=100, name="apply.png") + audit = LibraryAuditRun( + rule="transparency", params={"threshold": 0.9}, + status="ready", scanned_count=1, matched_count=1, + matched_ids=[rec.id], + ) + db.add(audit) + await db.commit() + resp = await client.post( + f"/api/cleanup/audit/{audit.id}/apply", + json={"confirm": f"delete-audit-{audit.id}"}, + ) + assert resp.status_code == 200 + body = await resp.get_json() + assert body["deleted"] == 1 + + +@pytest.mark.asyncio +async def test_audit_apply_with_bad_token_returns_400(client, db): + audit = LibraryAuditRun( + rule="transparency", params={"threshold": 0.9}, + status="ready", matched_ids=[], + ) + db.add(audit) + await db.commit() + resp = await client.post( + f"/api/cleanup/audit/{audit.id}/apply", + json={"confirm": "wrong-token"}, + ) + assert resp.status_code == 400 + body = await resp.get_json() + assert body["error"] == "confirm_mismatch" + + +@pytest.mark.asyncio +async def test_audit_cancel_flips_status(client, db): + audit = LibraryAuditRun( + rule="transparency", params={"threshold": 0.9}, + status="running", matched_ids=[], + ) + db.add(audit) + await db.commit() + resp = await client.post(f"/api/cleanup/audit/{audit.id}/cancel") + assert resp.status_code == 200 + new_status = await db.execute( + select(LibraryAuditRun.status).where(LibraryAuditRun.id == audit.id) + ) + assert new_status.scalar_one() == "cancelled" diff --git a/tests/test_cleanup_service_audit.py b/tests/test_cleanup_service_audit.py index ec488d7..f9100b0 100644 --- a/tests/test_cleanup_service_audit.py +++ b/tests/test_cleanup_service_audit.py @@ -133,7 +133,7 @@ def test_apply_audit_run_with_correct_token_deletes_matched(db_sync, tmp_path): deleted = cleanup_service.apply_audit_run( db_sync, audit_id=audit.id, - confirm_token=f"apply-audit-{audit.id}", + confirm_token=f"delete-audit-{audit.id}", images_root=tmp_path, ) assert deleted == 1 diff --git a/tests/test_importer_upsert_helpers.py b/tests/test_importer_upsert_helpers.py new file mode 100644 index 0000000..ca8a3bc --- /dev/null +++ b/tests/test_importer_upsert_helpers.py @@ -0,0 +1,147 @@ +"""Tests for Importer._find_or_create_source / _find_or_create_post — +the race-safe savepoint-based helpers that replaced the previous +check-then-insert pattern. + +Operator-flagged 2026-05-26: concurrent workers processing different +files in the same post both found no existing Source row, then both +INSERTed, tripping uq_source_artist_platform_url and poisoning the +session with `psycopg.errors.UniqueViolation`. The new helpers wrap +the INSERT in a savepoint and recover from IntegrityError by +re-selecting the row that the concurrent op committed. + +Tests cover: + - idempotent return: same (artist_id, platform, url) → same Source row + - idempotent return for Post: same (source_id, external_post_id) → same Post + - IntegrityError recovery: monkeypatched flush raises once, helper + finds the row a concurrent op committed +""" + +from pathlib import Path + +import pytest +from sqlalchemy import select +from sqlalchemy.exc import IntegrityError + +from backend.app.models import Artist, ImportSettings, Post, Source +from backend.app.services.importer import Importer +from backend.app.services.thumbnailer import Thumbnailer + +pytestmark = pytest.mark.integration + + +@pytest.fixture +def importer(db_sync, tmp_path): + import_root = tmp_path / "import" + images_root = tmp_path / "images" + import_root.mkdir() + images_root.mkdir() + settings = db_sync.execute( + select(ImportSettings).where(ImportSettings.id == 1) + ).scalar_one() + return Importer( + session=db_sync, + images_root=images_root, + import_root=import_root, + thumbnailer=Thumbnailer(images_root=images_root), + settings=settings, + ) + + +@pytest.fixture +def artist_row(db_sync): + a = Artist(name="TestArtist", slug="testartist") + db_sync.add(a) + db_sync.flush() + return a + + +def test_find_or_create_source_creates_then_returns_existing(importer, artist_row): + s1 = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/posts/test-1", + ) + s2 = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/posts/test-1", + ) + assert s1.id == s2.id + + +def test_find_or_create_source_distinct_urls_yield_distinct_rows( + importer, artist_row, +): + a = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/posts/a", + ) + b = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/posts/b", + ) + assert a.id != b.id + + +def test_find_or_create_post_idempotent(importer, artist_row, db_sync): + src = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", + url="https://www.patreon.com/posts/post-test", + ) + p1 = importer._find_or_create_post( + source_id=src.id, external_post_id="ext-001", + ) + p2 = importer._find_or_create_post( + source_id=src.id, external_post_id="ext-001", + ) + assert p1.id == p2.id + + +def test_find_or_create_source_recovers_from_integrity_error( + importer, artist_row, db_sync, monkeypatch, +): + """Simulate the race: another worker has already inserted a Source row + matching our (artist_id, platform, url) just before our flush would + have. Our flush raises IntegrityError; the helper rolls back the + savepoint and re-selects, returning the row the concurrent op created. + """ + canonical_url = "https://www.patreon.com/posts/race-141226276" + pre_existing = Source( + artist_id=artist_row.id, platform="patreon", url=canonical_url, + ) + db_sync.add(pre_existing) + db_sync.flush() + + # Force a fresh select within the helper to MISS the existing row by + # detaching it from the identity map; SQLAlchemy's first-level cache + # would otherwise return the pre_existing row immediately. + # Easier: monkeypatch the FIRST select inside the helper to return + # None on first call, real result on subsequent. We do that by + # patching session.execute with a single-shot wrapper. + real_execute = db_sync.execute + skip_count = [0] + + def execute_with_first_select_miss(stmt, *args, **kwargs): + # Strip-down heuristic: the first SELECT issued by the helper is + # the existence check. Force it to return a "no row" result. + result = real_execute(stmt, *args, **kwargs) + if skip_count[0] == 0: + skip_count[0] += 1 + # Wrap result so .scalar_one_or_none() returns None for this + # one call, then unwrap on subsequent uses. + class _ForcedMiss: + def scalar_one_or_none(self): + return None + + def scalar_one(self): + return result.scalar_one() + + def __getattr__(self, name): + return getattr(result, name) + return _ForcedMiss() + return result + + monkeypatch.setattr(db_sync, "execute", execute_with_first_select_miss) + + recovered = importer._find_or_create_source( + artist_id=artist_row.id, platform="patreon", url=canonical_url, + ) + assert recovered.id == pre_existing.id