diff --git a/.env.sample b/.env.sample index 9f28413..5d7b6ce 100644 --- a/.env.sample +++ b/.env.sample @@ -40,6 +40,17 @@ FEATURE_ADMIN_OCR=false FEATURE_OCR_QUEUE_V2=false FEATURE_OCR_SHADOW_LAUNCH=false OCR_SHADOW_PROVIDER= +# When true, uploaded queue images are OCR'd automatically in the background; when false they +# land as `pending` for manual review on the admin OCR queue page. Requires FEATURE_ADMIN_OCR. +FEATURE_OCR_AUTO_EXTRACT=false +# Admin UI toggle to tag mail as promotional. When true, the admin record-entry form and +# the OCR queue review screen expose a "Promotional" checkbox and the flag is persisted on +# the resulting MAIL document (`isPromotional: true`, omitted otherwise). +FEATURE_PROMO_CLASSIFICATION=false + +# Local photo staging: uploaded queue images are written here (bind-mounted into backend), +# and anything older than the retention window is pruned nightly by the scheduler. +IMAGE_RETENTION_HOURS=24 OCR_PROVIDER=paddleocr # OCR_PROVIDER: paddleocr (self-hosted, default), easyocr, tesseract, or ocrspace (external API) @@ -48,6 +59,7 @@ OCR_SPACE_API_KEY= BACKEND_API_URL=http://backend:8000 SCHEDULER_CRON=0 8 * * 1 +IMAGE_PRUNE_CRON=0 3 * * * SCHEDULER_TIMEZONE=America/New_York SCHEDULER_TICK_SECONDS=20 @@ -59,3 +71,6 @@ GRAFANA_ROOT_URL=https://hub.avenuworkspaces.com/mail/grafana PROMETHEUS_DATA_DIR=/mnt/Main/other/PrometheusMail/ PROMETHEUS_CONFIG_DIR=/mnt/Main/other/PrometheusMailEtc/ GRAFANA_DATA_DIR=/mnt/Main/other/GrafanaMail/ +# Host directory bind-mounted into backend at /var/lib/avenu/images. Must exist and be +# writable by the backend container uid. Pruned nightly per IMAGE_PRUNE_CRON + IMAGE_RETENTION_HOURS. +MAIL_IMAGES_DIR=/mnt/Main/other/AvenuMailImages/ diff --git a/backend/config.py b/backend/config.py index 5e3d295..fce81ce 100644 --- a/backend/config.py +++ b/backend/config.py @@ -79,12 +79,25 @@ def _env_positive_int(name: str, default: int) -> int: FEATURE_OCR_SHADOW_LAUNCH = _env_bool("FEATURE_OCR_SHADOW_LAUNCH", False) OCR_SHADOW_PROVIDER = os.getenv("OCR_SHADOW_PROVIDER", "").strip().lower() or None +# Admin toggle to mark incoming mail as promotional. Independent of OCR; default off. +FEATURE_PROMO_CLASSIFICATION = _env_bool("FEATURE_PROMO_CLASSIFICATION", False) +# When true, uploaded images are OCR'd in a background thread; when false, items land as +# `pending` for manual review. OCR auto-sort is descoped; default off. +FEATURE_OCR_AUTO_EXTRACT = _env_bool("FEATURE_OCR_AUTO_EXTRACT", False) + +# Host-mounted directory where uploaded mail photos live (see docker-compose volumes). +IMAGE_STORE_DIR = os.getenv("IMAGE_STORE_DIR", "/var/lib/avenu/images").strip() or "/var/lib/avenu/images" +# Nightly prune deletes anything older than this many hours (both files and queue rows). +IMAGE_RETENTION_HOURS = _env_positive_int("IMAGE_RETENTION_HOURS", 24) + def get_feature_flags() -> dict[str, bool]: return { "adminOcr": FEATURE_ADMIN_OCR, "ocrQueueV2": FEATURE_OCR_QUEUE_V2 and FEATURE_ADMIN_OCR, "ocrShadowLaunch": FEATURE_OCR_SHADOW_LAUNCH and FEATURE_ADMIN_OCR, + "ocrAutoExtract": FEATURE_OCR_AUTO_EXTRACT and FEATURE_ADMIN_OCR, + "promoClassification": FEATURE_PROMO_CLASSIFICATION, } diff --git a/backend/controllers/internal_jobs_controller.py b/backend/controllers/internal_jobs_controller.py index 3270da6..6689592 100644 --- a/backend/controllers/internal_jobs_controller.py +++ b/backend/controllers/internal_jobs_controller.py @@ -8,6 +8,7 @@ from controllers.common import parse_iso_date from errors import APIError from services.idempotency_service import begin_request, commit_response, rollback_reservation +from services.image_pruner import prune_expired_images from services.notifications.channels.factory import build_notification_channels from services.notifications.weekly_summary_cron_job import run_weekly_summary_cron_job from services.notifications.weekly_summary_notifier import WeeklySummaryNotifier @@ -84,3 +85,36 @@ def internal_weekly_summary_job_route(): except Exception: rollback_reservation(key=idempotency_key, route=route, method="POST") raise + + +@internal_jobs_bp.route("/api/internal/jobs/image-prune", methods=["POST"]) +def internal_image_prune_job_route(): + _require_scheduler_token() + + body_raw = request.get_json(silent=True) + payload = {} if body_raw is None else require_dict(body_raw) + retention_override = payload.get("retentionHours") + retention_hours: int | None = None + if retention_override is not None: + if not isinstance(retention_override, int) or isinstance(retention_override, bool) or retention_override <= 0: + raise APIError(422, "retentionHours must be a positive integer") + retention_hours = retention_override + + route = "/api/internal/jobs/image-prune" + idempotency_key, replay = begin_request( + headers=request.headers, + payload=payload, + route=route, + method="POST", + ) + if replay is not None: + return jsonify(replay["body"]), replay["status"] + + try: + result = prune_expired_images(retention_hours=retention_hours) + body = result.to_dict() + commit_response(key=idempotency_key, route=route, method="POST", status=200, body=body) + return jsonify(body), 200 + except Exception: + rollback_reservation(key=idempotency_key, route=route, method="POST") + raise diff --git a/backend/controllers/ocr_queue_controller.py b/backend/controllers/ocr_queue_controller.py index 3371770..b94a04f 100644 --- a/backend/controllers/ocr_queue_controller.py +++ b/backend/controllers/ocr_queue_controller.py @@ -10,7 +10,15 @@ from bson import ObjectId from flask import Blueprint, jsonify, request, send_file -from config import FEATURE_ADMIN_OCR, FEATURE_OCR_QUEUE_V2, OCR_MAX_FILE_BYTES, ocr_queue_items_collection, fs +from config import ( + FEATURE_ADMIN_OCR, + FEATURE_OCR_AUTO_EXTRACT, + FEATURE_OCR_QUEUE_V2, + FEATURE_PROMO_CLASSIFICATION, + OCR_MAX_FILE_BYTES, + fs, + ocr_queue_items_collection, +) from controllers.auth_guard import require_admin_session from errors import APIError from idempotency import payload_hash @@ -27,6 +35,7 @@ update_ocr_queue_item, ) from repositories import to_api_doc +from services import image_store from services.mail_service import create_mail from services.ocr.ocr_parser import parse_ocr_text @@ -127,6 +136,8 @@ def _serialize_item(doc: dict[str, Any]) -> dict: "error": doc.get("error"), "mailboxId": str(doc["mailboxId"]) if doc.get("mailboxId") else None, "fileId": str(doc["fileId"]) if doc.get("fileId") else None, + "imagePath": doc.get("imagePath"), + "isPromotional": bool(doc.get("isPromotional")), "confirmedAt": doc["confirmedAt"].isoformat() if doc.get("confirmedAt") else None, } @@ -163,27 +174,34 @@ def create_job(): if not image_payloads: raise APIError(422, "no valid images to process") - # Store images in GridFS - file_ids: list[ObjectId] = [] - for img_data, content_type in image_payloads: - fid = fs.put(img_data, content_type=content_type, filename="ocr_upload") - file_ids.append(fid) + image_paths: list[str] = [ + image_store.save_bytes(img_data, content_type) + for img_data, content_type in image_payloads + ] date = request.form.get("date") or datetime.utcnow().strftime("%Y-%m-%d") job_id = create_ocr_job(created_by=admin_id, date=date, item_count=len(image_payloads)) - create_ocr_queue_items(job_id, len(image_payloads), file_ids=file_ids) - - from flask import current_app - app = current_app._get_current_object() - thread = threading.Thread(target=_process_ocr_job, args=(app, job_id, image_payloads), daemon=True) - thread.start() + create_ocr_queue_items(job_id, len(image_payloads), image_paths=image_paths) + + if FEATURE_OCR_AUTO_EXTRACT: + from flask import current_app + app = current_app._get_current_object() + thread = threading.Thread(target=_process_ocr_job, args=(app, job_id, image_payloads), daemon=True) + thread.start() + status = "processing" + else: + # Auto-extract is off: items stay `pending`; admin fills in receiver/sender + # manually during review. Mark the job itself as processed so the UI stops + # showing a spinner — there's no background work to wait for. + update_ocr_job_status(job_id, "processed", completed_count=len(image_payloads)) + status = "processed" return jsonify({ "id": str(job_id), - "status": "processing", + "status": status, "totalCount": len(image_payloads), - "completedCount": 0, + "completedCount": len(image_payloads) if not FEATURE_OCR_AUTO_EXTRACT else 0, "date": date, }), 201 @@ -245,6 +263,8 @@ def update_queue_item(item_id: str): if "mailboxId" in data: mb = data["mailboxId"] repo_kwargs["mailbox_id"] = ObjectId(mb) if mb and ObjectId.is_valid(mb) else None + if "isPromotional" in data and FEATURE_PROMO_CLASSIFICATION: + repo_kwargs["is_promotional"] = bool(data["isPromotional"]) if repo_kwargs: update_ocr_queue_item(oid, **repo_kwargs) @@ -303,6 +323,8 @@ def confirm_queue_item(item_id: str): "receiverName": item.get("receiverName"), "senderInfo": item.get("senderInfo"), } + if FEATURE_PROMO_CLASSIFICATION and item.get("isPromotional"): + payload["isPromotional"] = True created_mail = create_mail(payload) now = datetime.utcnow() @@ -354,19 +376,35 @@ def update_job_stage(job_id: str): @ocr_queue_bp.route("/api/ocr/queue//image", methods=["GET"]) @require_admin_session def get_item_image(item_id: str): - """Get the image for a queue item.""" + """Stream the image for a queue item. + + Prefers the filesystem path (new uploads). Falls back to GridFS for any + legacy items written before the volume-backed store existed. + """ if not ObjectId.is_valid(item_id): raise APIError(404, "item not found") - + item = get_ocr_queue_item(ObjectId(item_id)) - if not item or not item.get("fileId"): - raise APIError(404, "image not found") + if not item: + raise APIError(404, "item not found") - try: - grid_out = fs.get(item["fileId"]) - return send_file(grid_out, mimetype=grid_out.content_type) - except Exception: - raise APIError(404, "image file missing") + rel_path = item.get("imagePath") + if rel_path: + try: + stream, content_type = image_store.open_path(rel_path) + except FileNotFoundError: + raise APIError(404, "image file missing") + return send_file(stream, mimetype=content_type) + + file_id = item.get("fileId") + if file_id: + try: + grid_out = fs.get(file_id) + return send_file(grid_out, mimetype=grid_out.content_type) + except Exception: + raise APIError(404, "image file missing") + + raise APIError(404, "image not found") @ocr_queue_bp.route("/api/ocr/jobs", methods=["OPTIONS"], endpoint="ocr_jobs_options") diff --git a/backend/models/builders.py b/backend/models/builders.py index 24e15d0..5d27ca4 100644 --- a/backend/models/builders.py +++ b/backend/models/builders.py @@ -162,6 +162,9 @@ def build_mail_create(payload: dict[str, Any]) -> dict[str, Any]: sender = _optional_text(payload, "senderInfo", 500) if sender is not None: doc["senderInfo"] = sender + if "isPromotional" in payload: + if optional_bool(payload, "isPromotional"): + doc["isPromotional"] = True return doc @@ -187,6 +190,8 @@ def build_mail_patch(payload: dict[str, Any]) -> dict[str, Any]: patch["receiverName"] = _optional_text(payload, "receiverName", 2000) if "senderInfo" in payload: patch["senderInfo"] = _optional_text(payload, "senderInfo", 500) + if "isPromotional" in payload: + patch["isPromotional"] = optional_bool(payload, "isPromotional") if "count" in payload: n = _optional_mail_piece_count(payload) diff --git a/backend/repositories/ocr_queue_repository.py b/backend/repositories/ocr_queue_repository.py index e598bf8..7e31b14 100644 --- a/backend/repositories/ocr_queue_repository.py +++ b/backend/repositories/ocr_queue_repository.py @@ -24,7 +24,13 @@ def create_ocr_job(*, created_by: ObjectId, date: str, item_count: int) -> Objec return r.inserted_id -def create_ocr_queue_items(job_id: ObjectId, count: int, file_ids: list[ObjectId] | None = None) -> list[ObjectId]: +def create_ocr_queue_items( + job_id: ObjectId, + count: int, + *, + file_ids: list[ObjectId] | None = None, + image_paths: list[str] | None = None, +) -> list[ObjectId]: now = datetime.utcnow() docs = [ { @@ -38,6 +44,8 @@ def create_ocr_queue_items(job_id: ObjectId, count: int, file_ids: list[ObjectId "error": None, "mailboxId": None, "fileId": file_ids[i] if file_ids and i < len(file_ids) else None, + "imagePath": image_paths[i] if image_paths and i < len(image_paths) else None, + "isPromotional": False, "confirmedAt": None, "createdAt": now, "updatedAt": now, @@ -92,6 +100,7 @@ def update_ocr_queue_item( raw_text: str | None = None, error: str | None = None, mailbox_id: ObjectId | None | object = _SENTINEL, + is_promotional: bool | object = _SENTINEL, confirmed_at: datetime | None = None, ) -> bool: updates: dict[str, Any] = {"updatedAt": datetime.utcnow()} @@ -109,6 +118,8 @@ def update_ocr_queue_item( updates["error"] = error if mailbox_id is not _SENTINEL: updates["mailboxId"] = mailbox_id + if is_promotional is not _SENTINEL: + updates["isPromotional"] = bool(is_promotional) if confirmed_at is not None: updates["confirmedAt"] = confirmed_at r = ocr_queue_items_collection.update_one({"_id": item_id}, {"$set": updates}) diff --git a/backend/services/image_pruner.py b/backend/services/image_pruner.py new file mode 100644 index 0000000..2a448f2 --- /dev/null +++ b/backend/services/image_pruner.py @@ -0,0 +1,99 @@ +"""Nightly prune: drop image files + their queue rows once past retention. + +Composition is the whole point here: we take the list of stale rows from +Mongo, unlink their files via :mod:`image_store`, then soft-delete the rows. +The store walk is also run so any dangling files (uploads whose row was +purged separately, or files whose path was lost) don't linger. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime, timedelta +from typing import Callable, Iterable + +from config import IMAGE_RETENTION_HOURS, ocr_queue_items_collection +from services import image_store + + +@dataclass(frozen=True) +class PruneResult: + rows_scanned: int + files_deleted: int + rows_marked_deleted: int + orphan_files_deleted: int + + def to_dict(self) -> dict[str, int]: + return { + "rowsScanned": self.rows_scanned, + "filesDeleted": self.files_deleted, + "rowsMarkedDeleted": self.rows_marked_deleted, + "orphanFilesDeleted": self.orphan_files_deleted, + } + + +def prune_expired_images( + *, + retention_hours: int | None = None, + now: datetime | None = None, + find_stale: Callable[[datetime], Iterable[dict]] | None = None, + mark_deleted: Callable[[list], int] | None = None, + delete_file: Callable[[str], bool] | None = None, + prune_orphans: Callable[[int], int] | None = None, +) -> PruneResult: + """Delete images (and their queue rows) older than retention. + + Dependency-injected for testability; production defaults hit Mongo + the + filesystem store. + """ + hours = retention_hours if retention_hours is not None else IMAGE_RETENTION_HOURS + current = now or datetime.utcnow() + cutoff = current - timedelta(hours=hours) + retention_seconds = hours * 3600 + + _find_stale = find_stale or _default_find_stale + _mark = mark_deleted or _default_mark_deleted + _delete_file = delete_file or image_store.delete_path + _prune_orphans = prune_orphans or image_store.prune_older_than + + stale_rows = list(_find_stale(cutoff)) + files_deleted = 0 + deletable_ids: list = [] + for row in stale_rows: + deletable_ids.append(row["_id"]) + path = row.get("imagePath") + if path and _delete_file(path): + files_deleted += 1 + + rows_marked = _mark(deletable_ids) if deletable_ids else 0 + orphan_count = _prune_orphans(retention_seconds) + + return PruneResult( + rows_scanned=len(stale_rows), + files_deleted=files_deleted, + rows_marked_deleted=rows_marked, + orphan_files_deleted=orphan_count, + ) + + +def _default_find_stale(cutoff: datetime) -> Iterable[dict]: + # Rows with an imagePath created before the cutoff, not already soft-deleted. + cursor = ocr_queue_items_collection.find( + { + "imagePath": {"$ne": None}, + "status": {"$ne": "deleted"}, + "createdAt": {"$lt": cutoff}, + }, + {"_id": 1, "imagePath": 1}, + ) + return cursor + + +def _default_mark_deleted(ids: list) -> int: + if not ids: + return 0 + result = ocr_queue_items_collection.update_many( + {"_id": {"$in": ids}}, + {"$set": {"status": "deleted", "imagePath": None, "updatedAt": datetime.utcnow()}}, + ) + return result.modified_count diff --git a/backend/services/image_store.py b/backend/services/image_store.py new file mode 100644 index 0000000..b85f6a6 --- /dev/null +++ b/backend/services/image_store.py @@ -0,0 +1,107 @@ +"""Filesystem-backed image store for mail photos. + +Purely-stdlib module; no Mongo, no Flask. Images live under +``IMAGE_STORE_DIR`` (typically a host-bind-mounted Docker volume). The module +exposes a small, value-oriented surface: bytes + content-type go in, a +relative path comes out; paths later resolve back to streams. The store does +not track metadata; callers (the queue layer) persist ``imagePath`` on their +own documents. +""" + +from __future__ import annotations + +import mimetypes +import os +import secrets +import time +from pathlib import Path +from typing import BinaryIO, Iterator + +_DEFAULT_EXT = ".bin" +_CONTENT_TYPE_EXTENSIONS: dict[str, str] = { + "image/png": ".png", + "image/jpeg": ".jpg", + "image/jpg": ".jpg", + "image/gif": ".gif", + "image/webp": ".webp", + "image/tiff": ".tif", + "image/bmp": ".bmp", +} + + +def _ext_for(content_type: str) -> str: + ct = (content_type or "").split(";", 1)[0].strip().lower() + return _CONTENT_TYPE_EXTENSIONS.get(ct) or mimetypes.guess_extension(ct) or _DEFAULT_EXT + + +def _ensure_root(root: str) -> Path: + path = Path(root) + path.mkdir(parents=True, exist_ok=True) + return path.resolve() + + +def save_bytes(data: bytes, content_type: str, *, root: str | None = None) -> str: + """Write ``data`` under the store root; return the relative path stored on the item.""" + base = _ensure_root(root or _default_root()) + ext = _ext_for(content_type) + name = f"{secrets.token_hex(12)}{ext}" + target = base / name + with target.open("wb") as f: + f.write(data) + return name + + +def open_path(rel_path: str, *, root: str | None = None) -> tuple[BinaryIO, str]: + """Open ``rel_path`` for reading; refuses anything resolving outside the store root.""" + base = _ensure_root(root or _default_root()) + resolved = (base / rel_path).resolve() + if base != resolved and base not in resolved.parents: + raise FileNotFoundError(rel_path) + if not resolved.is_file(): + raise FileNotFoundError(rel_path) + content_type, _ = mimetypes.guess_type(str(resolved)) + return resolved.open("rb"), content_type or "application/octet-stream" + + +def delete_path(rel_path: str, *, root: str | None = None) -> bool: + """Best-effort unlink; returns True when a file was actually removed.""" + base = _ensure_root(root or _default_root()) + resolved = (base / rel_path).resolve() + if base != resolved and base not in resolved.parents: + return False + try: + resolved.unlink() + return True + except FileNotFoundError: + return False + except OSError: + return False + + +def prune_older_than(seconds: int, *, root: str | None = None) -> int: + """Remove files under the store root whose mtime is older than ``seconds`` ago.""" + base = _ensure_root(root or _default_root()) + cutoff = time.time() - max(0, seconds) + removed = 0 + for entry in _iter_files(base): + try: + if entry.stat().st_mtime < cutoff: + entry.unlink() + removed += 1 + except FileNotFoundError: + continue + except OSError: + continue + return removed + + +def _iter_files(base: Path) -> Iterator[Path]: + for root, _dirs, files in os.walk(base): + for name in files: + yield Path(root) / name + + +def _default_root() -> str: + """Late-bound to avoid import-time coupling with ``config``; tests can override via ``root``.""" + from config import IMAGE_STORE_DIR + return IMAGE_STORE_DIR diff --git a/backend/tests/unit/test_image_pruner.py b/backend/tests/unit/test_image_pruner.py new file mode 100644 index 0000000..6306512 --- /dev/null +++ b/backend/tests/unit/test_image_pruner.py @@ -0,0 +1,136 @@ +"""Unit tests for the nightly image pruner. + +``prune_expired_images`` is pure-function-shaped: all external dependencies +(Mongo + filesystem) are injected, so tests exercise the composition without +touching either. +""" + +from __future__ import annotations + +import os +import unittest +from datetime import datetime, timedelta + +os.environ.setdefault("MONGO_URI", "mongodb://localhost:27017") + +from services.image_pruner import prune_expired_images + + +class ImagePrunerTests(unittest.TestCase): + def test_prune_deletes_stale_rows_and_files(self): + now = datetime(2026, 4, 21, 3, 0) + stale = [ + {"_id": "r1", "imagePath": "a.jpg"}, + {"_id": "r2", "imagePath": "b.png"}, + ] + deleted_files: list[str] = [] + marked: list[list] = [] + + def fake_find(cutoff): + self.assertEqual(cutoff, now - timedelta(hours=24)) + return stale + + def fake_mark(ids): + marked.append(ids) + return len(ids) + + def fake_delete_file(path): + deleted_files.append(path) + return True + + def fake_prune_orphans(seconds): + self.assertEqual(seconds, 24 * 3600) + return 3 + + result = prune_expired_images( + retention_hours=24, + now=now, + find_stale=fake_find, + mark_deleted=fake_mark, + delete_file=fake_delete_file, + prune_orphans=fake_prune_orphans, + ) + + self.assertEqual(deleted_files, ["a.jpg", "b.png"]) + self.assertEqual(marked, [["r1", "r2"]]) + self.assertEqual(result.rows_scanned, 2) + self.assertEqual(result.files_deleted, 2) + self.assertEqual(result.rows_marked_deleted, 2) + self.assertEqual(result.orphan_files_deleted, 3) + + def test_prune_skips_rows_without_image_path(self): + rows = [{"_id": "r1", "imagePath": None}, {"_id": "r2", "imagePath": "x.jpg"}] + file_calls: list[str] = [] + + result = prune_expired_images( + retention_hours=24, + now=datetime(2026, 4, 21), + find_stale=lambda _cutoff: rows, + mark_deleted=lambda ids: len(ids), + delete_file=lambda p: (file_calls.append(p) or True), + prune_orphans=lambda _s: 0, + ) + + self.assertEqual(file_calls, ["x.jpg"]) + self.assertEqual(result.rows_scanned, 2) + self.assertEqual(result.files_deleted, 1) + self.assertEqual(result.rows_marked_deleted, 2) + + def test_prune_counts_only_actually_removed_files(self): + rows = [{"_id": "r1", "imagePath": "gone.jpg"}, {"_id": "r2", "imagePath": "there.jpg"}] + + def fake_delete(path): + return path == "there.jpg" + + result = prune_expired_images( + retention_hours=24, + now=datetime(2026, 4, 21), + find_stale=lambda _c: rows, + mark_deleted=lambda ids: len(ids), + delete_file=fake_delete, + prune_orphans=lambda _s: 0, + ) + + self.assertEqual(result.files_deleted, 1) + self.assertEqual(result.rows_marked_deleted, 2) + + def test_prune_with_no_stale_rows_still_runs_orphan_sweep(self): + orphan_calls: list[int] = [] + + result = prune_expired_images( + retention_hours=12, + now=datetime(2026, 4, 21), + find_stale=lambda _c: [], + mark_deleted=lambda ids: (_ for _ in ()).throw(AssertionError("should not be called")), + delete_file=lambda p: True, + prune_orphans=lambda s: (orphan_calls.append(s) or 5), + ) + + self.assertEqual(result.rows_scanned, 0) + self.assertEqual(result.files_deleted, 0) + self.assertEqual(result.rows_marked_deleted, 0) + self.assertEqual(result.orphan_files_deleted, 5) + self.assertEqual(orphan_calls, [12 * 3600]) + + def test_prune_result_to_dict_uses_camel_case(self): + result = prune_expired_images( + retention_hours=24, + now=datetime(2026, 4, 21), + find_stale=lambda _c: [{"_id": "r1", "imagePath": "a.jpg"}], + mark_deleted=lambda ids: len(ids), + delete_file=lambda _p: True, + prune_orphans=lambda _s: 7, + ) + self.assertEqual( + result.to_dict(), + { + "rowsScanned": 1, + "filesDeleted": 1, + "rowsMarkedDeleted": 1, + "orphanFilesDeleted": 7, + }, + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/backend/tests/unit/test_image_store.py b/backend/tests/unit/test_image_store.py new file mode 100644 index 0000000..ad52e02 --- /dev/null +++ b/backend/tests/unit/test_image_store.py @@ -0,0 +1,89 @@ +"""Unit tests for the filesystem image store service. + +The store is a pure-stdlib module; these tests use a ``tmp_path``-style +directory to avoid any dependency on MongoDB or the configured +``IMAGE_STORE_DIR``. +""" + +from __future__ import annotations + +import os +import time +import unittest +from pathlib import Path +from tempfile import TemporaryDirectory + +os.environ.setdefault("MONGO_URI", "mongodb://localhost:27017") + +from services import image_store + + +class ImageStoreTests(unittest.TestCase): + def setUp(self): + self._tmp = TemporaryDirectory() + self.root = self._tmp.name + + def tearDown(self): + self._tmp.cleanup() + + def test_save_then_open_round_trips_bytes_and_content_type(self): + rel = image_store.save_bytes(b"hello png", "image/png", root=self.root) + self.assertTrue(rel.endswith(".png")) + stream, content_type = image_store.open_path(rel, root=self.root) + try: + self.assertEqual(stream.read(), b"hello png") + finally: + stream.close() + self.assertEqual(content_type, "image/png") + + def test_save_uses_content_type_extension(self): + jpg = image_store.save_bytes(b"x", "image/jpeg", root=self.root) + gif = image_store.save_bytes(b"x", "image/gif", root=self.root) + unknown = image_store.save_bytes(b"x", "application/unknown", root=self.root) + self.assertTrue(jpg.endswith(".jpg")) + self.assertTrue(gif.endswith(".gif")) + self.assertTrue(unknown.endswith(".bin")) + + def test_open_path_rejects_traversal(self): + image_store.save_bytes(b"x", "image/png", root=self.root) + with self.assertRaises(FileNotFoundError): + image_store.open_path("../escape.png", root=self.root) + with self.assertRaises(FileNotFoundError): + image_store.open_path("/etc/passwd", root=self.root) + + def test_open_path_missing_file_raises(self): + with self.assertRaises(FileNotFoundError): + image_store.open_path("nonexistent.png", root=self.root) + + def test_delete_path_removes_file(self): + rel = image_store.save_bytes(b"x", "image/png", root=self.root) + self.assertTrue(image_store.delete_path(rel, root=self.root)) + self.assertFalse((Path(self.root) / rel).exists()) + + def test_delete_path_returns_false_when_missing(self): + self.assertFalse(image_store.delete_path("gone.png", root=self.root)) + + def test_delete_path_rejects_traversal(self): + self.assertFalse(image_store.delete_path("../../etc/passwd", root=self.root)) + + def test_prune_older_than_only_deletes_stale_files(self): + fresh = image_store.save_bytes(b"fresh", "image/png", root=self.root) + stale = image_store.save_bytes(b"stale", "image/png", root=self.root) + stale_path = Path(self.root) / stale + old_time = time.time() - (3600 * 48) + os.utime(stale_path, (old_time, old_time)) + + removed = image_store.prune_older_than(3600 * 24, root=self.root) + + self.assertEqual(removed, 1) + self.assertFalse((Path(self.root) / stale).exists()) + self.assertTrue((Path(self.root) / fresh).exists()) + + def test_prune_older_than_with_zero_keeps_everything(self): + image_store.save_bytes(b"x", "image/png", root=self.root) + removed = image_store.prune_older_than(60 * 60 * 24 * 365, root=self.root) + self.assertEqual(removed, 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/backend/tests/unit/test_mail_service_create.py b/backend/tests/unit/test_mail_service_create.py index b57d3e5..0a04b59 100644 --- a/backend/tests/unit/test_mail_service_create.py +++ b/backend/tests/unit/test_mail_service_create.py @@ -68,6 +68,26 @@ def test_count_n_expands_to_n_inserts_each_without_count(self): self.assertEqual(d["type"], "package") self.assertEqual(result["_id"], ids[0]) + def test_is_promotional_true_persists_on_each_insert(self): + n = 3 + ids = [ObjectId() for _ in range(n)] + payload = {**self.base_payload, "count": n, "isPromotional": True} + _result, inserted = self._run(payload, insert_ids=ids) + self.assertEqual(len(inserted), n) + for d in inserted: + self.assertIs(d["isPromotional"], True) + + def test_is_promotional_omitted_when_absent(self): + ids = [ObjectId()] + _result, inserted = self._run(self.base_payload, insert_ids=ids) + self.assertNotIn("isPromotional", inserted[0]) + + def test_is_promotional_false_is_omitted(self): + ids = [ObjectId()] + payload = {**self.base_payload, "isPromotional": False} + _result, inserted = self._run(payload, insert_ids=ids) + self.assertNotIn("isPromotional", inserted[0]) + def test_invalid_count_raises(self): from errors import APIError with patch("services.mail_service.mailbox_exists", return_value=True), \ diff --git a/backend/tests/unit/test_models.py b/backend/tests/unit/test_models.py index d35dc72..057d423 100644 --- a/backend/tests/unit/test_models.py +++ b/backend/tests/unit/test_models.py @@ -76,6 +76,61 @@ def test_build_mail_patch_omits_count_when_one(self): ) self.assertNotIn("count", patch) + def test_build_mail_create_omits_is_promotional_when_absent(self): + doc = build_mail_create( + { + "mailboxId": str(ObjectId()), + "date": "2025-01-01T10:00:00Z", + "type": "letter", + } + ) + self.assertNotIn("isPromotional", doc) + + def test_build_mail_create_sets_is_promotional_true(self): + doc = build_mail_create( + { + "mailboxId": str(ObjectId()), + "date": "2025-01-01T10:00:00Z", + "type": "letter", + "isPromotional": True, + } + ) + self.assertIs(doc["isPromotional"], True) + + def test_build_mail_create_omits_is_promotional_when_false(self): + doc = build_mail_create( + { + "mailboxId": str(ObjectId()), + "date": "2025-01-01T10:00:00Z", + "type": "letter", + "isPromotional": False, + } + ) + self.assertNotIn("isPromotional", doc) + + def test_build_mail_create_rejects_non_bool_is_promotional(self): + with self.assertRaises(APIError): + build_mail_create( + { + "mailboxId": str(ObjectId()), + "date": "2025-01-01T10:00:00Z", + "type": "letter", + "isPromotional": "yes", + } + ) + + def test_build_mail_patch_sets_is_promotional_true(self): + patch = build_mail_patch({"isPromotional": True}) + self.assertIs(patch["isPromotional"], True) + + def test_build_mail_patch_sets_is_promotional_false(self): + patch = build_mail_patch({"isPromotional": False}) + self.assertIs(patch["isPromotional"], False) + + def test_build_mail_patch_rejects_non_bool_is_promotional(self): + with self.assertRaises(APIError): + build_mail_patch({"isPromotional": 1}) + def test_build_mail_request_create_requires_expected_sender_or_description(self): with self.assertRaises(APIError) as ctx: build_mail_request_create( diff --git a/backend/tests/unit/test_ocr_queue_controller.py b/backend/tests/unit/test_ocr_queue_controller.py index c6b2276..ca62d46 100644 --- a/backend/tests/unit/test_ocr_queue_controller.py +++ b/backend/tests/unit/test_ocr_queue_controller.py @@ -72,9 +72,8 @@ def test_post_with_oversized_file_returns_422(self): resp = self.client.post("/api/ocr/jobs", data=data, content_type="multipart/form-data") self.assertEqual(resp.status_code, 422) - def test_post_happy_path_creates_job_and_returns_201(self): + def test_post_happy_path_with_auto_extract_off_writes_paths_and_skips_thread(self): job_id = ObjectId() - file_ids = [ObjectId(), ObjectId()] data = { "files": [ @@ -84,23 +83,49 @@ def test_post_happy_path_creates_job_and_returns_201(self): "date": "2025-01-01", } - with patch("controllers.ocr_queue_controller.fs") as fs_mock, \ + saved_paths = ["abc.jpg", "def.png"] + with patch("controllers.ocr_queue_controller.FEATURE_OCR_AUTO_EXTRACT", False), \ + patch("controllers.ocr_queue_controller.image_store.save_bytes", side_effect=saved_paths) as save_mock, \ patch("controllers.ocr_queue_controller.create_ocr_job", return_value=job_id) as create_job, \ patch("controllers.ocr_queue_controller.create_ocr_queue_items", return_value=[ObjectId(), ObjectId()]) as create_items, \ + patch("controllers.ocr_queue_controller.update_ocr_job_status") as status_mock, \ patch("controllers.ocr_queue_controller.threading.Thread") as thread_mock: - fs_mock.put.side_effect = file_ids resp = self.client.post("/api/ocr/jobs", data=data, content_type="multipart/form-data") self.assertEqual(resp.status_code, 201) body = resp.get_json() self.assertEqual(body["id"], str(job_id)) - self.assertEqual(body["status"], "processing") + self.assertEqual(body["status"], "processed") self.assertEqual(body["totalCount"], 2) - self.assertEqual(body["completedCount"], 0) + self.assertEqual(body["completedCount"], 2) self.assertEqual(body["date"], "2025-01-01") create_job.assert_called_once() create_items.assert_called_once() + self.assertEqual(create_items.call_args.kwargs["image_paths"], saved_paths) + self.assertEqual(save_mock.call_count, 2) + status_mock.assert_called_once_with(job_id, "processed", completed_count=2) + thread_mock.assert_not_called() + + def test_post_happy_path_with_auto_extract_on_starts_thread(self): + job_id = ObjectId() + data = { + "files": [(io.BytesIO(b"img"), "a.jpg", "image/jpeg")], + "date": "2025-01-01", + } + with patch("controllers.ocr_queue_controller.FEATURE_OCR_AUTO_EXTRACT", True), \ + patch("controllers.ocr_queue_controller.image_store.save_bytes", return_value="xyz.jpg"), \ + patch("controllers.ocr_queue_controller.create_ocr_job", return_value=job_id), \ + patch("controllers.ocr_queue_controller.create_ocr_queue_items", return_value=[ObjectId()]), \ + patch("controllers.ocr_queue_controller.update_ocr_job_status") as status_mock, \ + patch("controllers.ocr_queue_controller.threading.Thread") as thread_mock: + resp = self.client.post("/api/ocr/jobs", data=data, content_type="multipart/form-data") + + self.assertEqual(resp.status_code, 201) + body = resp.get_json() + self.assertEqual(body["status"], "processing") + self.assertEqual(body["completedCount"], 0) thread_mock.return_value.start.assert_called_once() + status_mock.assert_not_called() class QueueItemPatchTests(_OcrQueueControllerTestBase): @@ -265,6 +290,62 @@ def test_confirm_releases_reservation_on_error(self): release.assert_called_once() +class GetItemImageTests(_OcrQueueControllerTestBase): + def test_get_image_prefers_filesystem_when_path_present(self): + item_id = ObjectId() + item = {"_id": item_id, "imagePath": "abc.jpg", "fileId": None} + stream = io.BytesIO(b"pixels") + with patch("controllers.ocr_queue_controller.get_ocr_queue_item", return_value=item), \ + patch("controllers.ocr_queue_controller.image_store.open_path", + return_value=(stream, "image/jpeg")) as open_mock, \ + patch("controllers.ocr_queue_controller.fs") as fs_mock: + resp = self.client.get(f"/api/ocr/queue/{item_id}/image") + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.data, b"pixels") + self.assertEqual(resp.mimetype, "image/jpeg") + open_mock.assert_called_once_with("abc.jpg") + fs_mock.get.assert_not_called() + + def test_get_image_falls_back_to_gridfs_when_no_path(self): + item_id = ObjectId() + file_id = ObjectId() + item = {"_id": item_id, "imagePath": None, "fileId": file_id} + + class _FakeGrid: + content_type = "image/png" + def read(self, n=-1): + return b"legacy" + def close(self): + pass + + with patch("controllers.ocr_queue_controller.get_ocr_queue_item", return_value=item), \ + patch("controllers.ocr_queue_controller.image_store.open_path") as open_mock, \ + patch("controllers.ocr_queue_controller.fs") as fs_mock: + fs_mock.get.return_value = _FakeGrid() + resp = self.client.get(f"/api/ocr/queue/{item_id}/image") + self.assertEqual(resp.status_code, 200) + self.assertEqual(resp.data, b"legacy") + self.assertEqual(resp.mimetype, "image/png") + fs_mock.get.assert_called_once_with(file_id) + open_mock.assert_not_called() + + def test_get_image_returns_404_when_filesystem_path_missing(self): + item_id = ObjectId() + item = {"_id": item_id, "imagePath": "gone.jpg", "fileId": None} + with patch("controllers.ocr_queue_controller.get_ocr_queue_item", return_value=item), \ + patch("controllers.ocr_queue_controller.image_store.open_path", + side_effect=FileNotFoundError("gone.jpg")): + resp = self.client.get(f"/api/ocr/queue/{item_id}/image") + self.assertEqual(resp.status_code, 404) + + def test_get_image_returns_404_when_neither_path_nor_fileid(self): + item_id = ObjectId() + item = {"_id": item_id, "imagePath": None, "fileId": None} + with patch("controllers.ocr_queue_controller.get_ocr_queue_item", return_value=item): + resp = self.client.get(f"/api/ocr/queue/{item_id}/image") + self.assertEqual(resp.status_code, 404) + + class JobStageTests(_OcrQueueControllerTestBase): def test_stage_invalid_returns_400(self): resp = self.client.post(f"/api/ocr/jobs/{ObjectId()}/stage", json={"stage": "bogus"}) diff --git a/docker-compose-prod.yml b/docker-compose-prod.yml index 29e23d7..905550d 100644 --- a/docker-compose-prod.yml +++ b/docker-compose-prod.yml @@ -43,6 +43,12 @@ services: TWILIO_ACCOUNT_SID: ${TWILIO_ACCOUNT_SID} TWILIO_AUTH_TOKEN: ${TWILIO_AUTH_TOKEN} TWILIO_PHONE_NUMBER: ${TWILIO_PHONE_NUMBER} + FEATURE_PROMO_CLASSIFICATION: ${FEATURE_PROMO_CLASSIFICATION:-false} + FEATURE_OCR_AUTO_EXTRACT: ${FEATURE_OCR_AUTO_EXTRACT:-false} + IMAGE_STORE_DIR: /var/lib/avenu/images + IMAGE_RETENTION_HOURS: ${IMAGE_RETENTION_HOURS:-24} + volumes: + - mail-images:/var/lib/avenu/images ports: - 18000:8000 healthcheck: @@ -96,6 +102,7 @@ services: BACKEND_API_URL: http://backend:8000 SCHEDULER_INTERNAL_TOKEN: ${SCHEDULER_INTERNAL_TOKEN} SCHEDULER_CRON: ${SCHEDULER_CRON:-0 8 * * 1} + IMAGE_PRUNE_CRON: ${IMAGE_PRUNE_CRON:-0 3 * * *} SCHEDULER_TIMEZONE: ${SCHEDULER_TIMEZONE:-UTC} SCHEDULER_TICK_SECONDS: ${SCHEDULER_TICK_SECONDS:-20} restart: unless-stopped @@ -131,3 +138,9 @@ volumes: device: ${PROMETHEUS_CONFIG_DIR:-/mnt/Main/other/PrometheusMailEtc/} grafana-data: driver: local + mail-images: + driver: local + driver_opts: + type: none + o: bind + device: ${MAIL_IMAGES_DIR:-/mnt/Main/other/AvenuMailImages/} diff --git a/docker-compose.yml b/docker-compose.yml index c281fa3..5b8062f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -41,6 +41,12 @@ services: TWILIO_ACCOUNT_SID: ${TWILIO_ACCOUNT_SID} TWILIO_AUTH_TOKEN: ${TWILIO_AUTH_TOKEN} TWILIO_PHONE_NUMBER: ${TWILIO_PHONE_NUMBER} + FEATURE_PROMO_CLASSIFICATION: ${FEATURE_PROMO_CLASSIFICATION:-false} + FEATURE_OCR_AUTO_EXTRACT: ${FEATURE_OCR_AUTO_EXTRACT:-false} + IMAGE_STORE_DIR: /var/lib/avenu/images + IMAGE_RETENTION_HOURS: ${IMAGE_RETENTION_HOURS:-24} + volumes: + - mail-images:/var/lib/avenu/images healthcheck: test: ["CMD", "python", "-c", "import urllib.request; urllib.request.urlopen('http://localhost:8000/api/health', timeout=5)"] interval: 10s @@ -59,6 +65,7 @@ services: BACKEND_API_URL: http://backend:8000 SCHEDULER_INTERNAL_TOKEN: ${SCHEDULER_INTERNAL_TOKEN} SCHEDULER_CRON: ${SCHEDULER_CRON:-0 8 * * 1} + IMAGE_PRUNE_CRON: ${IMAGE_PRUNE_CRON:-0 3 * * *} SCHEDULER_TIMEZONE: ${SCHEDULER_TIMEZONE:-UTC} SCHEDULER_TICK_SECONDS: ${SCHEDULER_TICK_SECONDS:-20} restart: unless-stopped @@ -68,5 +75,8 @@ services: networks: - avenu-internal +volumes: + mail-images: + networks: avenu-internal: diff --git a/docs/01-architecture/data-model.md b/docs/01-architecture/data-model.md index 2b1a479..d0ddc26 100644 --- a/docs/01-architecture/data-model.md +++ b/docs/01-architecture/data-model.md @@ -202,9 +202,12 @@ Fields: - `receiverName: string | null` - `senderInfo: string | null` - `type: "letter" | "package"` +- `isPromotional: bool` (default `false`; gated behind `FEATURE_PROMO_CLASSIFICATION`. Carried into the created `mail` document on confirm when true.) - `rawText: string | null` (original OCR output) - `error: string | null` (if OCR failed) - `mailboxId: ObjectId | null` (assigned before confirm) +- `fileId: ObjectId | null` (legacy GridFS reference; nullable for new uploads) +- `imagePath: string | null` (relative path under `IMAGE_STORE_DIR`) - `confirmedAt: datetime | null` (when mail was created) - `createdAt: datetime` - `updatedAt: datetime` diff --git a/docs/01-architecture/diagrams/data-model.mmd b/docs/01-architecture/diagrams/data-model.mmd index f9e267b..eb48721 100644 --- a/docs/01-architecture/diagrams/data-model.mmd +++ b/docs/01-architecture/diagrams/data-model.mmd @@ -37,6 +37,7 @@ erDiagram enum type "letter | package" string receiverName "optional" string senderInfo "optional" + boolean isPromotional "optional; omitted when false" int count "optional legacy N pieces" datetime createdAt datetime updatedAt @@ -94,6 +95,9 @@ erDiagram string rawText "optional" string error "optional" ObjectId mailboxId FK "optional" + ObjectId fileId FK "optional; legacy GridFS reference, nullable for new uploads" + string imagePath "optional; relative path under IMAGE_STORE_DIR volume" + boolean isPromotional "default false; gated by FEATURE_PROMO_CLASSIFICATION" datetime confirmedAt "optional" datetime createdAt datetime updatedAt diff --git a/docs/02-plans/plan-mail-photo-staging-volume.md b/docs/02-plans/plan-mail-photo-staging-volume.md new file mode 100644 index 0000000..3b012f4 --- /dev/null +++ b/docs/02-plans/plan-mail-photo-staging-volume.md @@ -0,0 +1,144 @@ +# Open Questions +- None. + +# Locked Decisions +- Image bytes live on a host bind-mounted Docker volume at `IMAGE_STORE_DIR` (default `/var/lib/avenu/images`). Filename pattern: `.`, derived from upload content-type. +- OCR auto-extract is gated behind a new `FEATURE_OCR_AUTO_EXTRACT` env flag (default off). When off, uploads land as `pending` queue items with empty `receiverName` / `senderInfo`, and the existing review UI lets the admin fill them in by hand. When on, the existing background OCR thread runs unchanged. +- New `OCR_QUEUE_ITEM` documents store an optional `imagePath: str` (relative to `IMAGE_STORE_DIR`). Existing `fileId` (GridFS ObjectId) field stays for backwards-compat reads. +- Image read endpoint resolves in order: `imagePath` → filesystem; fallback `fileId` → GridFS. No data migration is performed; old uploads keep working untouched. +- Retention: hard 24h. A nightly cleanup deletes any image file with mtime older than 24h AND any `OCR_QUEUE_ITEM` doc with `createdAt` older than 24h regardless of status. No exceptions for "forgotten" pending items — that's the contract. +- Cleanup is triggered by the existing `scheduler` container hitting a new internal admin endpoint `POST /api/internal/jobs/prune-staged-images` authenticated with `SCHEDULER_INTERNAL_TOKEN`. Cron expression configurable via new `IMAGE_PRUNE_CRON` (default `0 3 * * *` local time). +- Scheduler now supports two independent cron schedules: existing weekly summary (`SCHEDULER_CRON`) and new image prune (`IMAGE_PRUNE_CRON`). Both share the tick loop; loop minute-key dedupe is per-job. +- Existing OCR queue UI label/route remains; no rename in this iteration. Admin enables the workflow with `FEATURE_ADMIN_OCR=true` + `FEATURE_OCR_QUEUE_V2=true` (already exists). `FEATURE_OCR_AUTO_EXTRACT` is the only new flag the operator needs to know about. + +# Task Checklist +## Phase 1 — Backend storage abstraction + flag +- ☑ Add `IMAGE_STORE_DIR` and `FEATURE_OCR_AUTO_EXTRACT` env handling in `backend/config.py`; expose `get_feature_flags()['ocrAutoExtract']` for parity with other flags. +- ☑ New `backend/services/image_store.py`: `save_bytes(data, content_type) -> str` (returns relative path), `open_path(rel_path) -> (stream, content_type)`, `delete_path(rel_path) -> bool`, `prune_older_than(seconds) -> int`. No Mongo; pure filesystem. +- ☑ Extend `OCR_QUEUE_ITEM` schema doc + `repositories/ocr_queue_repository.create_ocr_queue_items` to accept and persist `image_paths` alongside (or instead of) `file_ids`. +- ☑ Backend unit tests for `image_store` (round-trip, delete, prune-by-mtime) using `tmp_path`. + +## Phase 2 — Wire upload + read + skip-OCR path +- ☑ `controllers/ocr_queue_controller.create_job`: write each upload via `image_store.save_bytes` (not `fs.put`), pass `image_paths` to `create_ocr_queue_items`. Keep `fileId` writes for any legacy code path that already relies on GridFS (none today besides this controller — drop the GridFS write). +- ☑ `controllers/ocr_queue_controller.create_job`: skip the background OCR thread when `FEATURE_OCR_AUTO_EXTRACT` is false; mark items `pending` with no `rawText`. +- ☑ `controllers/ocr_queue_controller.get_item_image`: prefer `imagePath` via `image_store.open_path`; fall back to `fs.get(fileId)` when `imagePath` absent (legacy items). +- ☑ `_serialize_item` exposes `imagePath` (string) so the frontend has a stable shape; existing `fileId` field remains. +- ☑ Backend unit tests covering: skip-OCR upload path leaves items `pending`; image read prefers filesystem; fallback to GridFS works. + +## Phase 3 — Nightly prune endpoint + scheduler +- ☑ New `controllers/internal_jobs_controller` route `POST /api/internal/jobs/prune-staged-images`, internal-token guarded. Body optional `{"olderThanHours": int}` with default `IMAGE_RETENTION_HOURS=24`. Returns `{filesDeleted, itemsDeleted, errors}`. +- ☑ Service `services/image_pruner.py`: orchestrates filesystem prune + queue-item deletion in one transactional-best-effort sweep (per-doc tolerant of partial failures, returns counts). +- ☑ `scheduler/config.py` + `scheduler/main.py`: add second cron (`IMAGE_PRUNE_CRON`, default `0 3 * * *`); each tick checks both schedules independently; `last_processed_minute` becomes a per-job dict. +- ☑ `scheduler/client.py`: new `trigger_image_prune(token)` method analogous to `trigger_weekly_summary`. +- ☑ Backend unit test for prune service (creates fake old + new files, verifies only old removed; verifies queue items pruned by `createdAt`). +- ☑ Scheduler unit test for two-job tick dedupe (same minute fires once per job, different cron windows fire independently). + +## Phase 4 — Compose volume + docs +- ☑ `docker-compose.yml`: add `mail-images` named volume bound to host dir; mount at `/var/lib/avenu/images`; pass `IMAGE_STORE_DIR`, `IMAGE_RETENTION_HOURS`, `FEATURE_OCR_AUTO_EXTRACT` env vars to backend; pass `IMAGE_PRUNE_CRON` to scheduler. +- ☑ `docker-compose-prod.yml`: same, with `device: ${MAIL_IMAGES_DIR:-/mnt/Main/other/AvenuMailImages/}` to match the Prometheus pattern. +- ☑ `docs/01-architecture/diagrams/data-model.mmd`: add `imagePath` field on `OCR_QUEUE_ITEM`. + +--- + +## Phase 1: Backend storage abstraction + flag +Affected files and changes +- `backend/config.py` + - `IMAGE_STORE_DIR = os.getenv("IMAGE_STORE_DIR", "/var/lib/avenu/images")` (created on startup if missing). + - `IMAGE_RETENTION_HOURS = _env_positive_int("IMAGE_RETENTION_HOURS", 24)`. + - `FEATURE_OCR_AUTO_EXTRACT = _env_bool("FEATURE_OCR_AUTO_EXTRACT", False)`. + - Add `"ocrAutoExtract": FEATURE_OCR_AUTO_EXTRACT and FEATURE_ADMIN_OCR` to `get_feature_flags()`. +- `backend/services/image_store.py` (new) + - Pure-stdlib module. No Flask, no Mongo. Uses `pathlib`, `secrets.token_hex`, `mimetypes`. + - `save_bytes(data: bytes, content_type: str) -> str`: choose extension from content-type (`image/png` → `.png`, etc; default `.bin`); write to `IMAGE_STORE_DIR/.`; return relative path string. + - `open_path(rel_path: str) -> tuple[BinaryIO, str]`: validates the resolved path is inside `IMAGE_STORE_DIR` (no traversal); returns open binary stream and inferred content type. + - `delete_path(rel_path: str) -> bool`: best-effort unlink; returns success. + - `prune_older_than(seconds: int) -> int`: walks `IMAGE_STORE_DIR`, unlinks files where `mtime < now - seconds`; returns count. +- `backend/repositories/ocr_queue_repository.py` + - `create_ocr_queue_items(job_id, count, *, image_paths: list[str] | None = None, file_ids: list[ObjectId] | None = None)`: persist `imagePath` and/or `fileId` per item. + +Implementation notes +- Path validation in `open_path`: `Path(IMAGE_STORE_DIR).resolve() in resolved.parents` — refuse anything else with `APIError(404)`. +- `save_bytes` returns a *relative* path so storage dir can move without DB rewrites. +- No write fan-out to GridFS. New code path is one place: `save_bytes`. + +Unit tests (phase-local) +- `backend/tests/unit/test_image_store.py` + - `test_save_then_open_round_trips_bytes_and_content_type` + - `test_save_uses_content_type_extension` + - `test_open_path_rejects_traversal` + - `test_delete_path_returns_false_when_missing` + - `test_prune_older_than_only_deletes_stale_files` + +--- + +## Phase 2: Wire upload + read + skip-OCR path +Affected files and changes +- `backend/controllers/ocr_queue_controller.py` + - Replace the GridFS write loop with `image_paths = [save_bytes(data, ct) for data, ct in image_payloads]`. + - Pass `image_paths=image_paths` to `create_ocr_queue_items`. + - Skip background thread launch entirely when `not FEATURE_OCR_AUTO_EXTRACT`. Items are left `pending`. + - `get_item_image`: if `item.get("imagePath")`, stream via `image_store.open_path`; else fall back to existing `fs.get(item["fileId"])`. Raise `404` if neither present. + - `_serialize_item`: include `imagePath` field. +- `backend/config.py` + - Surface `IMAGE_RETENTION_HOURS` (used in Phase 3 but cheaper to add here). + +Implementation notes +- Keep `from config import fs` — it's only needed by the GridFS fallback branch and by any legacy admin page that surfaces ad-hoc uploads. +- `_process_ocr_job` is unchanged, just no longer started in skip-OCR mode. + +Unit tests (phase-local) +- `backend/tests/unit/test_ocr_queue_controller.py` + - `test_create_job_skips_ocr_thread_when_feature_off` (assert no thread started; items remain `pending`). + - `test_create_job_writes_image_paths_to_items` + - `test_get_item_image_prefers_filesystem_path` + - `test_get_item_image_falls_back_to_gridfs_when_no_path` + +--- + +## Phase 3: Nightly prune endpoint + scheduler +Affected files and changes +- `backend/services/image_pruner.py` (new) + - `prune_staged_images(*, older_than_hours: int) -> dict[str, int]`: + 1. Compute `cutoff = utcnow() - timedelta(hours=older_than_hours)`. + 2. Query `ocr_queue_items_collection.find({"createdAt": {"$lt": cutoff}}, {"imagePath": 1, "fileId": 1})`. + 3. For each, attempt `image_store.delete_path(imagePath)` if present; else `fs.delete(fileId)` if present. + 4. `delete_many({"createdAt": {"$lt": cutoff}})` for the items. + 5. Final sweep `image_store.prune_older_than(older_than_hours * 3600)` to catch orphans whose row was already deleted. + 6. Return `{filesDeleted, itemsDeleted, orphanFilesDeleted, errors}`. +- `backend/controllers/internal_jobs_controller.py` + - New route `POST /api/internal/jobs/prune-staged-images` mirroring the existing weekly-summary pattern: validates internal token, parses optional `olderThanHours` (defaults to `IMAGE_RETENTION_HOURS`), calls `prune_staged_images`, returns counts. +- `scheduler/config.py` + - Parse second cron `IMAGE_PRUNE_CRON` (default `"0 3 * * *"`). Keep existing `SCHEDULER_CRON` untouched. +- `scheduler/main.py` + - `last_processed_minute` becomes `dict[str, str]` keyed by job name (`"weekly_summary"`, `"image_prune"`). + - Each tick: for each `(job_name, schedule, action)` triple, dedupe + run. + - `action` for image prune: `client.trigger_image_prune(scheduler_token=...)`. +- `scheduler/client.py` + - New `trigger_image_prune(scheduler_token: str) -> dict`: `POST /api/internal/jobs/prune-staged-images` with the standard internal-token header. No body; backend uses default retention. + +Implementation notes +- Single-tick concurrency: the existing tick loop is single-threaded. Two jobs at the same minute boundary will both run sequentially. Acceptable. +- The orphan sweep at the end of `prune_staged_images` makes the contract idempotent: even if a row was deleted by hand and the file was orphaned, it'll get cleaned within 24h. + +Unit tests (phase-local) +- `backend/tests/unit/test_image_pruner.py` + - `test_prune_deletes_old_files_and_items` + - `test_prune_keeps_fresh_items_and_files` + - `test_prune_handles_missing_files_gracefully` + - `test_prune_orphan_sweep_catches_files_without_rows` +- `scheduler/tests/test_main.py` (new or extended) + - `test_two_jobs_dedupe_independently_in_same_tick` + - `test_image_prune_only_fires_at_03_00_local` + +--- + +## Phase 4: Compose volume + docs +Affected files and changes +- `docker-compose.yml` + - Backend service: add `volumes: - mail-images:/var/lib/avenu/images`; add `IMAGE_STORE_DIR`, `IMAGE_RETENTION_HOURS`, `FEATURE_OCR_AUTO_EXTRACT` to `environment`. + - Scheduler service: add `IMAGE_PRUNE_CRON` to `environment`. + - Top-level `volumes: { mail-images: { driver: local } }` (default named volume for dev). +- `docker-compose-prod.yml` + - Same as above, plus `mail-images` configured with `driver_opts: { type: none, o: bind, device: ${MAIL_IMAGES_DIR:-/mnt/Main/other/AvenuMailImages/} }` to match the Prometheus pattern. +- `docs/01-architecture/diagrams/data-model.mmd` + - Add `string imagePath "optional, relative to IMAGE_STORE_DIR"` to `OCR_QUEUE_ITEM`. diff --git a/docs/02-plans/plan-promotional-mail-classification-toggle.md b/docs/02-plans/plan-promotional-mail-classification-toggle.md new file mode 100644 index 0000000..1406630 --- /dev/null +++ b/docs/02-plans/plan-promotional-mail-classification-toggle.md @@ -0,0 +1,86 @@ +# Open Questions +- None. + +# Locked Decisions +- Classification is a boolean flag `isPromotional` on the `MAIL` document. Absent/false = not promotional. Omitted from the doc when false to keep storage consistent with existing optional fields (`receiverName`, `senderInfo`). +- Toggle is admin-only: set at mail-entry create and editable via patch. No member-facing preference in this iteration. +- Feature-gated behind `FEATURE_PROMO_CLASSIFICATION` env flag, default off. Surfaced via `/api/feature-flags` as `promoClassification`, consumed by the admin UI to show/hide the toggle. +- No new index, no backfill, no migration script. `ensure_indexes()` is unchanged. +- No changes to weekly summary aggregation, notifications, dashboard counts, or member views. Tag is stored and returned; downstream consumers are out of scope. +- API surface: `POST /api/mail` accepts optional `isPromotional: bool`; `PATCH /api/mail/:id` accepts partial `isPromotional`. `GET` responses include `isPromotional` only when true (mirrors `count` convention). +- OCR auto-sort is explicitly out of scope. Classification is entered by the admin at record time. + +# Task Checklist +## Phase 1 +- ☑ Add `FEATURE_PROMO_CLASSIFICATION` env flag and expose via `get_feature_flags()`. +- ☑ Extend `build_mail_create` / `build_mail_patch` to parse optional `isPromotional`. +- ☑ Preserve field in `to_api_doc` / response shaping (no-op if patch carries it through untouched). +- ☑ Backend unit tests for builder parsing and service round-trip. + +## Phase 2 +- ☑ Extend frontend `ApiFeatureFlags` and `ApiMailRecord` types. +- ☑ Extend `createMail` / `updateMail` API clients to send `isPromotional`. +- ☑ Add toggle to admin `RecordEntry.tsx` draft + edit forms, gated on `featureFlags.promoClassification`. +- ☑ Render a `Promo` badge on existing entries when `isPromotional === true`. +- ☑ Frontend unit tests for payload normalization (toggle true → field present; false → field omitted). + +--- + +## Phase 1: Backend field + feature flag +Affected files and changes +- `backend/config.py` + - Add `FEATURE_PROMO_CLASSIFICATION = _env_bool("FEATURE_PROMO_CLASSIFICATION", False)`. + - Add `"promoClassification": FEATURE_PROMO_CLASSIFICATION` to `get_feature_flags()`. +- `backend/models/builders.py` + - `build_mail_create`: if `isPromotional` key present, coerce via `optional_bool`; include `"isPromotional": True` in the doc only when true. Unknown/absent → omit. + - `build_mail_patch`: if `isPromotional` key present, coerce via `optional_bool` and include in patch. Patching to `false` sets the field to `False` (does not unset) — simplest behavior, still truthful for queries that check `== True`. Storage churn is negligible. +- `backend/repositories/mail_repository.py` + - `find_mail_for_mailboxes` projection: no change. (Classification is not consumed by summary aggregation in this phase.) +- No changes to `mail_service.py`, `mail_controller.py`, or `config.ensure_indexes`. + +Implementation notes +- Keep the storage shape truthful: omit `isPromotional` when false on create; preserve admin intent (explicit patch to `false`) by writing `False` on patch. This matches how the rest of the codebase treats optional booleans without over-engineering an unset path. +- No coupling to mail `type` (letter/package). Promo is orthogonal. + +Unit tests (phase-local) +- `backend/tests/unit/test_models.py` + - `test_build_mail_create_omits_is_promotional_when_absent` + - `test_build_mail_create_sets_is_promotional_true` + - `test_build_mail_create_omits_is_promotional_when_false` + - `test_build_mail_patch_accepts_is_promotional_true` + - `test_build_mail_patch_accepts_is_promotional_false_sets_field` + - `test_build_mail_patch_rejects_non_bool_is_promotional` +- `backend/tests/unit/test_mail_service_create.py` + - `test_create_mail_persists_is_promotional_when_true` + - `test_create_mail_omits_is_promotional_when_absent` + +--- + +## Phase 2: Admin UI toggle + API contract +Affected files and changes +- `frontend/src/lib/api/routes/featureFlags.ts` + - Add `promoClassification: boolean` to `ApiFeatureFlags`. +- `frontend/src/lib/api/contracts/types.ts` + - Add `isPromotional?: boolean` to `ApiMailRecord`. +- `frontend/src/lib/api/routes/mail.ts` + - `createMail` payload: accept optional `isPromotional`; include in body only when `true`. + - `updateMail` partial type: add `isPromotional?: boolean`; pass through as-is (true and false are both meaningful here). +- `frontend/src/lib/store.ts` + - Extend `featureFlags` default/shape with `promoClassification: false`. +- `frontend/src/pages/admin/RecordEntry.tsx` + - Extend `DraftEntry` with `isPromotional: boolean`; default `false` in `EMPTY_DRAFT`. + - Render a checkbox "Promotional" in the draft form and the edit form, only when `featureFlags.promoClassification` is true. + - Wire through `handleAddEntry` (pass `isPromotional: draft.isPromotional || undefined`) and `handleSaveEdit` (pass `isPromotional: editDraft.isPromotional`). + - In entry list rendering, show a `Promo` pill next to the `letter/package` pill when `entry.isPromotional === true`. + +Implementation notes +- Default `EMPTY_DRAFT.isPromotional = false`; when initializing `editDraft` from an entry, set `isPromotional: !!entry.isPromotional`. +- Payload normalization rule for `createMail`: omit when false (matches backend create semantics). For `updateMail`, include as-is — editing an entry from promo→not-promo must persist. +- No visual change when the flag is off (field hidden, payload unchanged). + +Unit tests (phase-local) +- `frontend/src/lib/api/routes/__tests__/mail.test.ts` (new or extended if file exists) + - `createMail omits isPromotional when false/undefined` + - `createMail includes isPromotional:true when toggled on` + - `updateMail includes isPromotional:false on explicit unset` +- No UI-level tests (per AGENTS.md: no UI/integration tests). diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index be7c572..d24b608 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -154,6 +154,8 @@ const AppRoutes = () => { adminOcr: flags.adminOcr ?? false, ocrQueueV2: flags.ocrQueueV2 ?? false, ocrShadowLaunch: flags.ocrShadowLaunch ?? false, + ocrAutoExtract: flags.ocrAutoExtract ?? false, + promoClassification: flags.promoClassification ?? false, }); } catch (err) { if (!alive) return; @@ -161,6 +163,8 @@ const AppRoutes = () => { adminOcr: false, ocrQueueV2: false, ocrShadowLaunch: false, + ocrAutoExtract: false, + promoClassification: false, }); } finally { if (alive) setFeatureFlagsHydrating(false); diff --git a/frontend/src/lib/api/contracts/types.ts b/frontend/src/lib/api/contracts/types.ts index 1a257d1..1fca8dd 100644 --- a/frontend/src/lib/api/contracts/types.ts +++ b/frontend/src/lib/api/contracts/types.ts @@ -19,6 +19,8 @@ export interface ApiMailRecord { count?: number; receiverName?: string | null; senderInfo?: string | null; + /** Admin-set classification. Absent on server means false. */ + isPromotional?: boolean; createdAt: string; updatedAt: string; } diff --git a/frontend/src/lib/api/routes/featureFlags.ts b/frontend/src/lib/api/routes/featureFlags.ts index 58b1220..8d86915 100644 --- a/frontend/src/lib/api/routes/featureFlags.ts +++ b/frontend/src/lib/api/routes/featureFlags.ts @@ -5,6 +5,13 @@ export interface ApiFeatureFlags { adminOcr: boolean; ocrQueueV2: boolean; ocrShadowLaunch: boolean; + /** + * When true, uploaded queue images are OCR'd automatically; when false they land as + * `pending` for manual review. Implies `adminOcr`. + */ + ocrAutoExtract: boolean; + /** When true, admin mail entry form shows the "Promotional" classification toggle. */ + promoClassification: boolean; } export function fetchFeatureFlags(): Promise { diff --git a/frontend/src/lib/api/routes/mail.ts b/frontend/src/lib/api/routes/mail.ts index e33dee8..6dec230 100644 --- a/frontend/src/lib/api/routes/mail.ts +++ b/frontend/src/lib/api/routes/mail.ts @@ -21,6 +21,8 @@ export function createMail(payload: { senderInfo?: string; /** Simple count mode: multiple pieces in one row (stored as count when > 1) */ count?: number; + /** Admin classification: only sent when true. */ + isPromotional?: boolean; idempotencyKey: string; }): Promise { const body: Record = { @@ -37,6 +39,9 @@ export function createMail(payload: { if (payload.senderInfo !== undefined && payload.senderInfo !== "") { body.senderInfo = payload.senderInfo; } + if (payload.isPromotional === true) { + body.isPromotional = true; + } return apiFetch("/mail", { method: "POST", headers: { @@ -55,6 +60,7 @@ export function updateMail( count: number; receiverName: string; senderInfo: string; + isPromotional: boolean; }> ): Promise { return apiFetch(`/mail/${mailId}`, { diff --git a/frontend/src/lib/api/routes/ocrQueue.ts b/frontend/src/lib/api/routes/ocrQueue.ts index faf6493..8fbdbd2 100644 --- a/frontend/src/lib/api/routes/ocrQueue.ts +++ b/frontend/src/lib/api/routes/ocrQueue.ts @@ -26,6 +26,8 @@ export interface ApiOcrQueueItem { error?: string | null; mailboxId?: string | null; fileId?: string | null; + imagePath?: string | null; + isPromotional?: boolean; confirmedAt?: string | null; } @@ -84,6 +86,7 @@ export async function updateOcrQueueItem( senderInfo: string; type: "letter" | "package"; mailboxId: string | null; + isPromotional: boolean; }> ): Promise<{ item: ApiOcrQueueItem }> { return apiFetch<{ item: ApiOcrQueueItem }>(`/ocr/queue/${itemId}`, { diff --git a/frontend/src/lib/store.ts b/frontend/src/lib/store.ts index 5393276..7b71f4b 100644 --- a/frontend/src/lib/store.ts +++ b/frontend/src/lib/store.ts @@ -15,6 +15,8 @@ export interface FeatureFlags { adminOcr: boolean; ocrQueueV2: boolean; ocrShadowLaunch: boolean; + ocrAutoExtract: boolean; + promoClassification: boolean; } interface AppState { @@ -41,6 +43,8 @@ export const useAppStore = create((set) => ({ adminOcr: false, ocrQueueV2: false, ocrShadowLaunch: false, + ocrAutoExtract: false, + promoClassification: false, }, isHydratingFeatureFlags: true, setSessionUser: (user) => set({ sessionUser: user }), diff --git a/frontend/src/pages/admin/OcrQueue.tsx b/frontend/src/pages/admin/OcrQueue.tsx index 86d3bd6..49929c8 100644 --- a/frontend/src/pages/admin/OcrQueue.tsx +++ b/frontend/src/pages/admin/OcrQueue.tsx @@ -250,6 +250,7 @@ const OcrQueue = () => { senderInfo: editingItem.senderInfo ?? item.senderInfo ?? "", type: editingItem.type ?? item.type ?? "letter", mailboxId: editingItem.mailboxId ?? item.mailboxId, + isPromotional: editingItem.isPromotional ?? item.isPromotional ?? false, } : null; @@ -555,7 +556,7 @@ const OcrQueue = () => {
{/* Left Column: Image */}
- {effectiveItem?.fileId ? ( + {effectiveItem && (effectiveItem.imagePath || effectiveItem.fileId) ? ( Mail Item {
+ {featureFlags.promoClassification && ( + + )}
{showMailboxPicker ? ( diff --git a/frontend/src/pages/admin/RecordEntry.tsx b/frontend/src/pages/admin/RecordEntry.tsx index 8e3ba5f..be3bda8 100644 --- a/frontend/src/pages/admin/RecordEntry.tsx +++ b/frontend/src/pages/admin/RecordEntry.tsx @@ -140,9 +140,10 @@ type DraftEntry = { receiverName: string; senderInfo: string; type: MailType; + isPromotional: boolean; }; -const EMPTY_DRAFT: DraftEntry = { receiverName: "", senderInfo: "", type: "letter" }; +const EMPTY_DRAFT: DraftEntry = { receiverName: "", senderInfo: "", type: "letter", isPromotional: false }; function mailPieceQty(entry: ApiMailRecord): number { const c = entry.count; @@ -157,6 +158,7 @@ const RecordEntry = () => { const logout = useAppStore((s) => s.logout); const adminOcrEnabled = useAppStore((s) => s.featureFlags.adminOcr); const ocrQueueV2 = useAppStore((s) => s.featureFlags.ocrQueueV2); + const promoClassificationEnabled = useAppStore((s) => s.featureFlags.promoClassification); const { toast } = useToast(); const [mailboxName, setMailboxName] = useState(""); @@ -373,6 +375,7 @@ const RecordEntry = () => { type: draft.type, receiverName: draft.receiverName.trim() || undefined, senderInfo: draft.senderInfo.trim() || undefined, + isPromotional: promoClassificationEnabled && draft.isPromotional ? true : undefined, idempotencyKey: makeIdempotencyKey(), }); setDraft({ ...EMPTY_DRAFT }); @@ -468,6 +471,7 @@ const RecordEntry = () => { receiverName: entry.receiverName || "", senderInfo: entry.senderInfo || "", type: entry.type, + isPromotional: !!entry.isPromotional, }); }; @@ -479,6 +483,7 @@ const RecordEntry = () => { type: editDraft.type, receiverName: editDraft.receiverName.trim(), senderInfo: editDraft.senderInfo.trim(), + ...(promoClassificationEnabled ? { isPromotional: editDraft.isPromotional } : {}), }); setEditingId(null); toast({ title: "Entry updated" }); @@ -620,6 +625,17 @@ const RecordEntry = () => { placeholder="Sender (optional)" className="w-full h-9 rounded-lg border border-input bg-background px-3 text-sm" /> + {promoClassificationEnabled && ( + + )}
{entry.receiverName && (

{entry.receiverName}

@@ -796,6 +817,17 @@ const RecordEntry = () => { className="w-full h-10 rounded-lg border border-input bg-background px-3 text-sm text-foreground focus:outline-none focus:ring-2 focus:ring-ring" />
+ {promoClassificationEnabled && ( + + )}