From bab564c23e8e1d568afe896e63ec42995da8dc91 Mon Sep 17 00:00:00 2001 From: NguyenND Date: Tue, 14 Jul 2026 11:49:25 -0400 Subject: [PATCH] Jul 14 - Using CDN - Phase 1 --- CLAUDE.md | 16 +-- app/__init__.py | 5 + app/api/photos.py | 14 +-- app/routes/inspections.py | 9 +- app/templates/inspections/execute.html | 2 +- app/templates/inspections/view.html | 2 +- app/templates/issues/view.html | 12 +- app/utils/storage.py | 145 +++++++++++++++++++++++++ config.py | 5 + 9 files changed, 180 insertions(+), 30 deletions(-) create mode 100644 app/utils/storage.py diff --git a/CLAUDE.md b/CLAUDE.md index 8b21089..5bd7b59 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1349,19 +1349,21 @@ timeout = 30 - [ ] Record the baseline number: **"N photos must still resolve after cutover."** - [ ] Investigate any `missing_on_disk` entries — these are PRE-EXISTING broken references, surfaced now so they can't be blamed on the move. -### Phase 1 — Storage abstraction + local backend (NO-OP, safe to deploy) -- [ ] `app/utils/storage.py` — interface: `save(file_obj, subfolder) -> key`, `read(key) -> bytes`, `delete(key)`, `exists(key) -> bool`, `media_url(key) -> str`. -- [ ] `LocalBackend` = byte-for-byte current behavior (`static/uploads` write; `media_url` = `url_for('static', filename=key)`). -- [ ] Config: `STORAGE_BACKEND=local|s3` (default `local`). -- [ ] Route `_save_photo()` in `inspections.py` + `issues.py`, and `api/photos.py`, through `storage.save()` (keep magic-byte check in the caller). -- [ ] Templates: swap `url_for('static', filename=path)` → `media_url(path)` in `issues/view.html`, `inspections/view.html`, and any other photo render sites. -- [ ] Deploy with `STORAGE_BACKEND=local` → identical behavior; regression check that web + iPad photos still load. +### Phase 1 — Storage abstraction + local backend (NO-OP, safe to deploy) — ✅ delivered +- [x] `app/utils/storage.py` — interface: `save(file_obj, subfolder) -> key`, `read(key) -> bytes`, `delete(key)`, `exists(key) -> bool`, `abs_local_path(key)`, `media_url(key) -> str`. Backend chosen by `STORAGE_BACKEND`, one instance cached in `app.extensions`. +- [x] `LocalBackend` = byte-for-byte current behavior (`static/uploads` write; `media_url` = `url_for('static', filename=key)`; `abs_local_path` = `static_folder/key`). +- [x] Config: `STORAGE_BACKEND=local|s3` (default `local`) in `config.py`. +- [x] Routed `_save_photo()` (inspections.py) + `api/photos.py::upload_photo` through `storage.save()` (magic-byte / ext validation stays in the callers). +- [x] `media_url` registered as a Jinja global in `app/__init__.py`; swapped all 8 photo render sites: `issues/view.html` (×6: primary, mobile, result), `inspections/view.html` (×1: form image), `inspections/execute.html` (×1). +- [ ] Deploy with `STORAGE_BACKEND=local` → identical behavior; regression check that web + iPad photos still load and new uploads still save. +- **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. ### Phase 3 — Verified migration sync diff --git a/app/__init__.py b/app/__init__.py index d936422..2408f03 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -106,6 +106,11 @@ def create_app(config_name='default'): app.jinja_env.globals['sla_hours_remaining'] = sla_hours_remaining app.jinja_env.globals['SLA_HOURS'] = SLA_HOURS + # Photo URL resolver — routes through the active storage backend so templates + # work unchanged when the backend flips from local to R2 (see utils/storage.py). + from app.utils import storage as _storage + app.jinja_env.globals['media_url'] = _storage.media_url + # ── Inject unread notification count into every template context ────── # This powers the red badge on the navbar bell icon without requiring # individual routes to pass the count manually. diff --git a/app/api/photos.py b/app/api/photos.py index 852874d..1903e20 100644 --- a/app/api/photos.py +++ b/app/api/photos.py @@ -85,16 +85,10 @@ def upload_photo(): else: subfolder = 'inspection_photos' - ext = file_obj.filename.rsplit('.', 1)[-1].lower() - filename = f'{uuid.uuid4().hex}.{ext}' - - dest_dir = os.path.join(current_app.config['UPLOAD_FOLDER'], subfolder) - os.makedirs(dest_dir, exist_ok=True) - - dest_path = os.path.join(dest_dir, filename) - file_obj.save(dest_path) - - server_path = f'uploads/{subfolder}/{filename}' + # Write via the active storage backend (local disk or R2). Key format + # 'uploads//.' is unchanged across backends. + from app.utils import storage + server_path = storage.save(file_obj, subfolder) logger.info('API PHOTOS | uploaded | entity_type=%s | path=%s | user=%s', entity_type, server_path, user.username) diff --git a/app/routes/inspections.py b/app/routes/inspections.py index 601b41d..79582c8 100644 --- a/app/routes/inspections.py +++ b/app/routes/inspections.py @@ -58,11 +58,10 @@ def _save_photo(file_obj, subfolder='inspection_photos'): file_obj.seek(0) if not any(header.startswith(m) for m in _IMAGE_MAGIC): return None - filename = f"{uuid.uuid4().hex}.{ext}" - dest_dir = os.path.join(current_app.config['UPLOAD_FOLDER'], subfolder) - os.makedirs(dest_dir, exist_ok=True) - file_obj.save(os.path.join(dest_dir, filename)) - return f"uploads/{subfolder}/{filename}" + # Write via the active storage backend (local disk or R2). Key format + # 'uploads//.' is unchanged across backends. + from app.utils import storage + return storage.save(file_obj, subfolder) def _collect_form_responses(form_fields, existing_responses=None): diff --git a/app/templates/inspections/execute.html b/app/templates/inspections/execute.html index 4f4dcbd..42ab574 100644 --- a/app/templates/inspections/execute.html +++ b/app/templates/inspections/execute.html @@ -439,7 +439,7 @@ {# Thumbnail shown after AJAX upload or when a saved path exists #} {% if saved %} - Photo diff --git a/app/templates/inspections/view.html b/app/templates/inspections/view.html index 84c815e..ae407f5 100644 --- a/app/templates/inspections/view.html +++ b/app/templates/inspections/view.html @@ -688,7 +688,7 @@
{% if val %} {% else %} diff --git a/app/templates/issues/view.html b/app/templates/issues/view.html index 79ce957..0949950 100644 --- a/app/templates/issues/view.html +++ b/app/templates/issues/view.html @@ -132,14 +132,14 @@
Photo Evidence
{% if issue.photo_path %} - - + {% endif %} {% for photo in (issue.mobile_photo_paths or []) %} - - + {% endfor %} @@ -155,8 +155,8 @@ {% if issue.result_photos %}
{% for photo in issue.result_photos %} - - + Result photo diff --git a/app/utils/storage.py b/app/utils/storage.py new file mode 100644 index 0000000..e3d9488 --- /dev/null +++ b/app/utils/storage.py @@ -0,0 +1,145 @@ +""" +app/utils/storage.py +--------------------- +Storage abstraction for uploaded photo files (R2 object-storage migration). + +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. + +The stored **key** is always the relative path ``uploads//`` — +the exact string persisted in the DB (`Issue.photo_path`, `result_photos[]`, +`mobile_photo_paths[]`, `Inspection.form_data` image values, +`InspectionResult.photo_path`). It never changes between backends, so there is +**no schema migration**. + +Public module-level API (delegates to the active backend): + + save(file_obj, subfolder) -> key # write an uploaded file, return its key + media_url(key) -> str # browser/API URL for a key ('' if falsy) + read(key) -> bytes # raw bytes (e.g. PDF export image embed) + exists(key) -> bool + delete(key) -> None # best-effort file removal + abs_local_path(key) -> str # physical path under static/ (local backend) + +NOTE: extension / magic-byte validation stays in the CALLER (e.g. `_save_photo`, +`api/photos.upload_photo`) — this module only moves bytes and builds URLs. +""" + +import os +import uuid +import logging + +from flask import current_app, url_for + +logger = logging.getLogger(__name__) + + +def _ext_of(filename): + """Lower-case extension of a filename, defaulting to 'jpg' when absent.""" + name = filename or '' + return name.rsplit('.', 1)[-1].lower() if '.' in name else 'jpg' + + +# ── Local filesystem backend (current behavior) ─────────────────────────────── + +class LocalBackend: + """Files under ``app/static/uploads``, served by Flask/Nginx static route.""" + + name = 'local' + + def _uploads_root(self): + # config UPLOAD_FOLDER == /app/static/uploads + return current_app.config['UPLOAD_FOLDER'] + + def _static_folder(self): + # /app/static — key 'uploads/...' resolves under here + return os.path.join(current_app.root_path, 'static') + + def save(self, file_obj, subfolder): + """Write ``file_obj`` under ``subfolder``; return the ``uploads/...`` key. + + Replicates the previous inline logic in `_save_photo` / `upload_photo` + exactly: random uuid filename, `os.makedirs(exist_ok=True)`, same key. + """ + ext = _ext_of(file_obj.filename) + filename = f'{uuid.uuid4().hex}.{ext}' + dest_dir = os.path.join(self._uploads_root(), subfolder) + os.makedirs(dest_dir, exist_ok=True) + file_obj.save(os.path.join(dest_dir, filename)) + return f'uploads/{subfolder}/{filename}' + + def abs_local_path(self, key): + return os.path.normpath(os.path.join(self._static_folder(), key)) + + def media_url(self, key): + if not key: + return '' + return url_for('static', filename=key) + + def read(self, key): + with open(self.abs_local_path(key), 'rb') as fh: + return fh.read() + + def exists(self, key): + return bool(key) and os.path.isfile(self.abs_local_path(key)) + + def delete(self, key): + if not key: + return + try: + path = self.abs_local_path(key) + if os.path.isfile(path): + os.remove(path) + except OSError: + pass + + +# S3Backend (Cloudflare R2) is added in Phase 2 and registered below. + +_BACKENDS = { + 'local': LocalBackend, +} + + +def get_backend(): + """Return the active storage backend (one cached instance per app).""" + name = (current_app.config.get('STORAGE_BACKEND') or 'local').lower() + cls = _BACKENDS.get(name) + if cls is None: + logger.warning('Unknown STORAGE_BACKEND=%r — falling back to local', name) + cls = LocalBackend + cache_key = f'_storage_backend_{cls.name}' + inst = current_app.extensions.get(cache_key) + if inst is None: + inst = cls() + current_app.extensions[cache_key] = inst + return inst + + +# ── Module-level convenience delegates ──────────────────────────────────────── + +def save(file_obj, subfolder): + return get_backend().save(file_obj, subfolder) + + +def media_url(key): + return get_backend().media_url(key) + + +def read(key): + return get_backend().read(key) + + +def exists(key): + return get_backend().exists(key) + + +def delete(key): + return get_backend().delete(key) + + +def abs_local_path(key): + return get_backend().abs_local_path(key) diff --git a/config.py b/config.py index 3713795..b4fe2a9 100644 --- a/config.py +++ b/config.py @@ -37,6 +37,11 @@ class Config: MAX_CONTENT_LENGTH = 50 * 1024 * 1024 # 50 MB ALLOWED_EXTENSIONS = {'png', 'jpg', 'jpeg', 'gif'} + # ── Storage backend (R2 migration) ────────────────────────────────────── + # 'local' (default) = files under app/static/uploads, served via url_for('static'). + # 's3' = Cloudflare R2 / S3-compatible (added Phase 2). Flip via env only. + STORAGE_BACKEND = os.environ.get('STORAGE_BACKEND', 'local') + # ── Session / cookies ─────────────────────────────────────────────────── PERMANENT_SESSION_LIFETIME = timedelta(hours=24) # Secure by default — subclasses must explicitly opt out for local dev.