From 9edece27266590537ade8e7a72239cbc2a228b34 Mon Sep 17 00:00:00 2001 From: NguyenND Date: Tue, 14 Jul 2026 12:34:28 -0400 Subject: [PATCH] Jul 14 - Using CDN - Phase 2a --- CLAUDE.md | 20 ++++--- app/routes/inspections.py | 13 ++--- app/routes/issues.py | 11 ++-- app/utils/storage.py | 108 +++++++++++++++++++++++++++++++++++++- config.py | 11 ++++ requirements.txt | 1 + 6 files changed, 140 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5bd7b59..6535e7b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1359,12 +1359,20 @@ timeout = 30 - **Deferred to Phase 2 (not yet routed through storage):** photo *deletes* (issues.py ~1013, inspections.py ~1340) and PDF-export image *reads* (`utils/pdf_export.py`). These still hit local disk directly — fine while `local`, must be routed before the `s3` flip. **Bug to fix when routing:** the inspection-delete cleanup builds `os.path.join(UPLOAD_FOLDER, '..', 'static', rel_path)` which normalizes to `app/static/static/...` (double `static`) and never deletes — routing it through `storage.delete(key)` fixes it. The issue-delete path (`root_path + 'static' + rel_path`) is already correct. ### Phase 2 — R2 backend (build, do NOT flip yet) -- [ ] `pip install boto3`; add to `requirements.txt`. -- [ ] `.env`: `STORAGE_BACKEND`, `R2_ENDPOINT_URL`, `R2_ACCESS_KEY_ID`, `R2_SECRET_ACCESS_KEY`, `R2_BUCKET`, `R2_PRESIGN_TTL=86400`. -- [ ] `S3Backend`: boto3 client (`endpoint_url=R2_ENDPOINT_URL`, `region_name='auto'`). `save` → `put_object` (set `ContentType`); `media_url` → `generate_presigned_url('get_object', ..., ExpiresIn=TTL)`; `read` → `get_object`; `exists` → `head_object`. -- [ ] Fallback: if `head_object` 404s, `S3Backend.read`/`media_url` fall back to the local static path. -- [ ] Route the deferred callers through storage (required before the flip): photo *deletes* (issues.py, inspections.py) → `storage.delete(key)`; PDF-export image *reads* (`utils/pdf_export.py`, `os.path.join(static_folder, v)`) → `storage.read(key)`. Fix the inspection-delete double-`static` path in the process. -- [ ] Leave `STORAGE_BACKEND=local` in production for now. +**2a — delivered:** +- [x] `boto3>=1.34` added to `requirements.txt` (imported **lazily** in `S3Backend` — a `local` deploy doesn't need it installed). +- [x] R2 config keys in `config.py`: `R2_ENDPOINT_URL`, `R2_ACCESS_KEY_ID`, `R2_SECRET_ACCESS_KEY`, `R2_BUCKET`, `R2_PRESIGN_TTL` (default 86400), `R2_MEDIA_FALLBACK` (default false). +- [x] `S3Backend` in `storage.py`: private bucket; `save` → `put_object` (ContentType from ext); `media_url` → presigned GET (TTL); `read` → `get_object`; `exists`/`_head` → `head_object`; `delete` → `delete_object`. Client cached in `app.extensions`, `region_name='auto'`, SigV4. +- [x] Transition fallback: `read()` falls back to the local copy on a missing object; `media_url()` falls back to a local static URL **only when `R2_MEDIA_FALLBACK=true`** (off by default — avoids a HEAD per image; cutover is gated on full verification anyway). +- [x] Routed photo **deletes** (issues.py, inspections.py) → `storage.delete(key)`; **fixed** the inspection-delete double-`static` bug in the process. + +**2b — remaining before cutover:** +- [ ] Route PDF-export image **reads** through storage. `pdf_export.py` resolves `os.path.join(static_folder, key)` at several sites (`_form_fields_section`, `generate_issue_pdf`, `_compress_image`). Lowest-risk approach: a `storage.materialize_to_dir(keys)` helper — local backend returns the real static folder (no-op); s3 downloads referenced keys to a temp dir mirroring `key` layout, and the two entry points (`generate_inspection_pdf`, `generate_issue_pdf`) point `static_folder` at it (try/finally cleanup). Leaves all render sites untouched. + +**Operator setup (before flip, not code):** +- [ ] Create R2 bucket (e.g. `jqc-media`), US region; create an R2 API token → Access Key + Secret + endpoint. +- [ ] `.env`: `R2_ENDPOINT_URL`, `R2_ACCESS_KEY_ID`, `R2_SECRET_ACCESS_KEY`, `R2_BUCKET=jqc-media`, `R2_PRESIGN_TTL=86400`. Keep `STORAGE_BACKEND=local` for now. +- [ ] `pip install boto3` into the venv. ### Phase 3 — Verified migration sync - [ ] `scripts/migrate_photos_to_r2.py` — idempotent, resumable, checksummed. diff --git a/app/routes/inspections.py b/app/routes/inspections.py index 79582c8..f7cb167 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -1335,15 +1335,12 @@ def delete(inspection_id): db.session.delete(inspection) db.session.commit() + # Remove orphaned photo files from the active storage backend — best-effort. + # (Also fixes the previous double-'static' path that normalized to + # app/static/static/... and never actually deleted anything.) + from app.utils import storage for rel_path in photo_paths: - abs_path = os.path.normpath( - os.path.join(current_app.config['UPLOAD_FOLDER'], '..', 'static', rel_path) - ) - try: - if os.path.isfile(abs_path): - os.remove(abs_path) - except OSError: - pass + storage.delete(rel_path) current_app.logger.info( 'INSPECTION DELETED | id=%s | facility="%s" | template="%s" | ' diff --git a/app/routes/issues.py b/app/routes/issues.py index ae1b5a8..f78c90d 100644 --- a/app/routes/issues.py +++ b/app/routes/issues.py @@ -1009,15 +1009,10 @@ def delete(issue_id): db.session.delete(issue) db.session.commit() - # Remove orphaned photo files — best-effort, never block on failure - static_folder = current_app.root_path + # Remove orphaned photo files from the active storage backend — best-effort. + from app.utils import storage for rel_path in photo_paths: - abs_path = os.path.normpath(os.path.join(static_folder, 'static', rel_path)) - try: - if os.path.isfile(abs_path): - os.remove(abs_path) - except OSError: - pass + storage.delete(rel_path) current_app.logger.info( 'ISSUE DELETED | id=%s | severity=%s | area=%s | facility=%s | deleted_by=%s', diff --git a/app/utils/storage.py b/app/utils/storage.py index e3d9488..19dc328 100644 --- a/app/utils/storage.py +++ b/app/utils/storage.py @@ -7,7 +7,9 @@ One interface, backend selected by config ``STORAGE_BACKEND``: - ``local`` (default): files under ``app/static/uploads``, served via ``url_for('static', ...)``. **Byte-for-byte identical** to the behavior before this abstraction existed — Phase 1 is a no-op. - - ``s3`` (added in Phase 2): Cloudflare R2 / any S3-compatible store. + - ``s3``: Cloudflare R2 / any S3-compatible store. Private bucket; browser/API + URLs are short-lived presigned GETs. Requires ``boto3`` and the ``R2_*`` + config keys. boto3 is imported lazily, so a ``local`` deploy needs neither. The stored **key** is always the relative path ``uploads//`` — the exact string persisted in the DB (`Issue.photo_path`, `result_photos[]`, @@ -97,10 +99,112 @@ class LocalBackend: pass -# S3Backend (Cloudflare R2) is added in Phase 2 and registered below. +# ── Cloudflare R2 / S3-compatible backend ───────────────────────────────────── + +_CONTENT_TYPES = { + 'jpg': 'image/jpeg', 'jpeg': 'image/jpeg', + 'png': 'image/png', 'gif': 'image/gif', +} + + +class S3Backend: + """ + Cloudflare R2 (or any S3-compatible store). Objects are private; browser/API + URLs are short-lived presigned GETs. The object key == the DB path string. + + boto3 is imported lazily so a 'local' deployment doesn't require it installed. + """ + + name = 's3' + + def __init__(self): + import boto3 # lazy — only when s3 is active + from botocore.config import Config as _BotoConfig + + cfg = current_app.config + missing = [k for k in ('R2_ENDPOINT_URL', 'R2_ACCESS_KEY_ID', + 'R2_SECRET_ACCESS_KEY', 'R2_BUCKET') if not cfg.get(k)] + if missing: + raise RuntimeError(f'STORAGE_BACKEND=s3 but missing config: {", ".join(missing)}') + + self.bucket = cfg['R2_BUCKET'] + self.ttl = int(cfg.get('R2_PRESIGN_TTL', 86400)) + self.fallback = bool(cfg.get('R2_MEDIA_FALLBACK', False)) + self._client = boto3.client( + 's3', + endpoint_url = cfg['R2_ENDPOINT_URL'], + aws_access_key_id = cfg['R2_ACCESS_KEY_ID'], + aws_secret_access_key = cfg['R2_SECRET_ACCESS_KEY'], + region_name = 'auto', + config = _BotoConfig(signature_version='s3v4'), + ) + # A LocalBackend for transition fallback (read/media_url when object absent). + self._local = LocalBackend() + + def _content_type(self, key): + return _CONTENT_TYPES.get(_ext_of(key), 'application/octet-stream') + + def save(self, file_obj, subfolder): + ext = _ext_of(file_obj.filename) + key = f'uploads/{subfolder}/{uuid.uuid4().hex}.{ext}' + file_obj.stream.seek(0) + body = file_obj.stream.read() + self._client.put_object( + Bucket=self.bucket, Key=key, Body=body, + ContentType=_CONTENT_TYPES.get(ext, 'application/octet-stream'), + ) + return key + + def abs_local_path(self, key): + # No local path for an S3 object; expose the local mirror path (may or may + # not exist) so transition code / callers can still reference it. + return self._local.abs_local_path(key) + + def media_url(self, key): + if not key: + return '' + if self.fallback and not self._head(key): + # Object not in R2 yet — serve the local copy during transition. + if self._local.exists(key): + return self._local.media_url(key) + return self._client.generate_presigned_url( + 'get_object', + Params={'Bucket': self.bucket, 'Key': key}, + ExpiresIn=self.ttl, + ) + + def read(self, key): + try: + resp = self._client.get_object(Bucket=self.bucket, Key=key) + return resp['Body'].read() + except Exception: + # Transition safety: fall back to a local copy if present. + if self._local.exists(key): + return self._local.read(key) + raise + + def _head(self, key): + try: + self._client.head_object(Bucket=self.bucket, Key=key) + return True + except Exception: + return False + + def exists(self, key): + return bool(key) and self._head(key) + + def delete(self, key): + if not key: + return + try: + self._client.delete_object(Bucket=self.bucket, Key=key) + except Exception as e: + logger.warning('S3 delete failed for %s: %s', key, e) + _BACKENDS = { 'local': LocalBackend, + 's3': S3Backend, } diff --git a/config.py b/config.py index b4fe2a9..44a526e 100644 --- a/config.py +++ b/config.py @@ -42,6 +42,17 @@ class Config: # 's3' = Cloudflare R2 / S3-compatible (added Phase 2). Flip via env only. STORAGE_BACKEND = os.environ.get('STORAGE_BACKEND', 'local') + # Cloudflare R2 — only used when STORAGE_BACKEND=s3. + R2_ENDPOINT_URL = os.environ.get('R2_ENDPOINT_URL') + R2_ACCESS_KEY_ID = os.environ.get('R2_ACCESS_KEY_ID') + R2_SECRET_ACCESS_KEY = os.environ.get('R2_SECRET_ACCESS_KEY') + R2_BUCKET = os.environ.get('R2_BUCKET') + R2_PRESIGN_TTL = int(os.environ.get('R2_PRESIGN_TTL', '86400')) # seconds (24h) + # When true, media_url HEADs the object and falls back to a local static URL + # if it's missing (belt-and-suspenders during an early/partial cutover). + # Off by default — cutover is gated on full verification, so objects exist. + R2_MEDIA_FALLBACK = os.environ.get('R2_MEDIA_FALLBACK', 'false').lower() == 'true' + # ── Session / cookies ─────────────────────────────────────────────────── PERMANENT_SESSION_LIFETIME = timedelta(hours=24) # Secure by default — subclasses must explicitly opt out for local dev. diff --git a/requirements.txt b/requirements.txt index 3d0918e..ac67033 100644 --- a/requirements.txt +++ b/requirements.txt @@ -23,3 +23,4 @@ pyJWT openpyxl groq qrcode +boto3>=1.34