Jul 14 - Using CDN - Phase 1
This commit is contained in:
@@ -1349,19 +1349,21 @@ timeout = 30
|
|||||||
- [ ] Record the baseline number: **"N photos must still resolve after cutover."**
|
- [ ] 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.
|
- [ ] 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)
|
### Phase 1 — Storage abstraction + local backend (NO-OP, safe to deploy) — ✅ delivered
|
||||||
- [ ] `app/utils/storage.py` — interface: `save(file_obj, subfolder) -> key`, `read(key) -> bytes`, `delete(key)`, `exists(key) -> bool`, `media_url(key) -> str`.
|
- [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`.
|
||||||
- [ ] `LocalBackend` = byte-for-byte current behavior (`static/uploads` write; `media_url` = `url_for('static', filename=key)`).
|
- [x] `LocalBackend` = byte-for-byte current behavior (`static/uploads` write; `media_url` = `url_for('static', filename=key)`; `abs_local_path` = `static_folder/key`).
|
||||||
- [ ] Config: `STORAGE_BACKEND=local|s3` (default `local`).
|
- [x] Config: `STORAGE_BACKEND=local|s3` (default `local`) in `config.py`.
|
||||||
- [ ] Route `_save_photo()` in `inspections.py` + `issues.py`, and `api/photos.py`, through `storage.save()` (keep magic-byte check in the caller).
|
- [x] Routed `_save_photo()` (inspections.py) + `api/photos.py::upload_photo` through `storage.save()` (magic-byte / ext validation stays in the callers).
|
||||||
- [ ] Templates: swap `url_for('static', filename=path)` → `media_url(path)` in `issues/view.html`, `inspections/view.html`, and any other photo render sites.
|
- [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.
|
- [ ] 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)
|
### Phase 2 — R2 backend (build, do NOT flip yet)
|
||||||
- [ ] `pip install boto3`; add to `requirements.txt`.
|
- [ ] `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`.
|
- [ ] `.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`.
|
- [ ] `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.
|
- [ ] 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.
|
- [ ] Leave `STORAGE_BACKEND=local` in production for now.
|
||||||
|
|
||||||
### Phase 3 — Verified migration sync
|
### Phase 3 — Verified migration sync
|
||||||
|
|||||||
@@ -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_remaining'] = sla_hours_remaining
|
||||||
app.jinja_env.globals['SLA_HOURS'] = SLA_HOURS
|
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 ──────
|
# ── Inject unread notification count into every template context ──────
|
||||||
# This powers the red badge on the navbar bell icon without requiring
|
# This powers the red badge on the navbar bell icon without requiring
|
||||||
# individual routes to pass the count manually.
|
# individual routes to pass the count manually.
|
||||||
|
|||||||
+4
-10
@@ -85,16 +85,10 @@ def upload_photo():
|
|||||||
else:
|
else:
|
||||||
subfolder = 'inspection_photos'
|
subfolder = 'inspection_photos'
|
||||||
|
|
||||||
ext = file_obj.filename.rsplit('.', 1)[-1].lower()
|
# Write via the active storage backend (local disk or R2). Key format
|
||||||
filename = f'{uuid.uuid4().hex}.{ext}'
|
# 'uploads/<subfolder>/<uuid>.<ext>' is unchanged across backends.
|
||||||
|
from app.utils import storage
|
||||||
dest_dir = os.path.join(current_app.config['UPLOAD_FOLDER'], subfolder)
|
server_path = storage.save(file_obj, 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}'
|
|
||||||
|
|
||||||
logger.info('API PHOTOS | uploaded | entity_type=%s | path=%s | user=%s',
|
logger.info('API PHOTOS | uploaded | entity_type=%s | path=%s | user=%s',
|
||||||
entity_type, server_path, user.username)
|
entity_type, server_path, user.username)
|
||||||
|
|||||||
@@ -58,11 +58,10 @@ def _save_photo(file_obj, subfolder='inspection_photos'):
|
|||||||
file_obj.seek(0)
|
file_obj.seek(0)
|
||||||
if not any(header.startswith(m) for m in _IMAGE_MAGIC):
|
if not any(header.startswith(m) for m in _IMAGE_MAGIC):
|
||||||
return None
|
return None
|
||||||
filename = f"{uuid.uuid4().hex}.{ext}"
|
# Write via the active storage backend (local disk or R2). Key format
|
||||||
dest_dir = os.path.join(current_app.config['UPLOAD_FOLDER'], subfolder)
|
# 'uploads/<subfolder>/<uuid>.<ext>' is unchanged across backends.
|
||||||
os.makedirs(dest_dir, exist_ok=True)
|
from app.utils import storage
|
||||||
file_obj.save(os.path.join(dest_dir, filename))
|
return storage.save(file_obj, subfolder)
|
||||||
return f"uploads/{subfolder}/{filename}"
|
|
||||||
|
|
||||||
|
|
||||||
def _collect_form_responses(form_fields, existing_responses=None):
|
def _collect_form_responses(form_fields, existing_responses=None):
|
||||||
|
|||||||
@@ -439,7 +439,7 @@
|
|||||||
</label>
|
</label>
|
||||||
{# Thumbnail shown after AJAX upload or when a saved path exists #}
|
{# Thumbnail shown after AJAX upload or when a saved path exists #}
|
||||||
{% if saved %}
|
{% if saved %}
|
||||||
<img src="{{ url_for('static', filename=saved) }}"
|
<img src="{{ media_url(saved) }}"
|
||||||
id="thumb_{{ fid }}"
|
id="thumb_{{ fid }}"
|
||||||
alt="Photo"
|
alt="Photo"
|
||||||
style="max-height:60px;max-width:100%;border-radius:4px;margin-top:.3rem;object-fit:cover;">
|
style="max-height:60px;max-width:100%;border-radius:4px;margin-top:.3rem;object-fit:cover;">
|
||||||
|
|||||||
@@ -688,7 +688,7 @@
|
|||||||
<div style="display:flex;align-items:center;flex:1;min-height:0;">
|
<div style="display:flex;align-items:center;flex:1;min-height:0;">
|
||||||
{% if val %}
|
{% if val %}
|
||||||
<button type="button" class="btn-view-media"
|
<button type="button" class="btn-view-media"
|
||||||
onclick="openMedia('{{ url_for('static', filename=val) }}','{{ field.label | e }}')">
|
onclick="openMedia('{{ media_url(val) }}','{{ field.label | e }}')">
|
||||||
<i class="bi bi-image"></i> View Photo
|
<i class="bi bi-image"></i> View Photo
|
||||||
</button>
|
</button>
|
||||||
{% else %}
|
{% else %}
|
||||||
|
|||||||
@@ -132,14 +132,14 @@
|
|||||||
<h6>Photo Evidence</h6>
|
<h6>Photo Evidence</h6>
|
||||||
<div class="d-flex flex-wrap gap-2">
|
<div class="d-flex flex-wrap gap-2">
|
||||||
{% if issue.photo_path %}
|
{% if issue.photo_path %}
|
||||||
<a href="{{ url_for('static', filename=issue.photo_path) }}" target="_blank">
|
<a href="{{ media_url(issue.photo_path) }}" target="_blank">
|
||||||
<img src="{{ url_for('static', filename=issue.photo_path) }}"
|
<img src="{{ media_url(issue.photo_path) }}"
|
||||||
class="img-fluid rounded" style="max-height:300px; max-width:100%;">
|
class="img-fluid rounded" style="max-height:300px; max-width:100%;">
|
||||||
</a>
|
</a>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
{% for photo in (issue.mobile_photo_paths or []) %}
|
{% for photo in (issue.mobile_photo_paths or []) %}
|
||||||
<a href="{{ url_for('static', filename=photo) }}" target="_blank">
|
<a href="{{ media_url(photo) }}" target="_blank">
|
||||||
<img src="{{ url_for('static', filename=photo) }}"
|
<img src="{{ media_url(photo) }}"
|
||||||
class="rounded border" style="max-height:300px; max-width:100%; object-fit:cover;">
|
class="rounded border" style="max-height:300px; max-width:100%; object-fit:cover;">
|
||||||
</a>
|
</a>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
@@ -155,8 +155,8 @@
|
|||||||
{% if issue.result_photos %}
|
{% if issue.result_photos %}
|
||||||
<div class="d-flex flex-wrap gap-2 mt-2">
|
<div class="d-flex flex-wrap gap-2 mt-2">
|
||||||
{% for photo in issue.result_photos %}
|
{% for photo in issue.result_photos %}
|
||||||
<a href="{{ url_for('static', filename=photo) }}" target="_blank">
|
<a href="{{ media_url(photo) }}" target="_blank">
|
||||||
<img src="{{ url_for('static', filename=photo) }}"
|
<img src="{{ media_url(photo) }}"
|
||||||
class="rounded border" style="max-height:120px; max-width:160px; object-fit:cover;"
|
class="rounded border" style="max-height:120px; max-width:160px; object-fit:cover;"
|
||||||
alt="Result photo">
|
alt="Result photo">
|
||||||
</a>
|
</a>
|
||||||
|
|||||||
@@ -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/<subfolder>/<file>`` —
|
||||||
|
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 == <root>/app/static/uploads
|
||||||
|
return current_app.config['UPLOAD_FOLDER']
|
||||||
|
|
||||||
|
def _static_folder(self):
|
||||||
|
# <root>/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)
|
||||||
@@ -37,6 +37,11 @@ class Config:
|
|||||||
MAX_CONTENT_LENGTH = 50 * 1024 * 1024 # 50 MB
|
MAX_CONTENT_LENGTH = 50 * 1024 * 1024 # 50 MB
|
||||||
ALLOWED_EXTENSIONS = {'png', 'jpg', 'jpeg', 'gif'}
|
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 ───────────────────────────────────────────────────
|
# ── Session / cookies ───────────────────────────────────────────────────
|
||||||
PERMANENT_SESSION_LIFETIME = timedelta(hours=24)
|
PERMANENT_SESSION_LIFETIME = timedelta(hours=24)
|
||||||
# Secure by default — subclasses must explicitly opt out for local dev.
|
# Secure by default — subclasses must explicitly opt out for local dev.
|
||||||
|
|||||||
Reference in New Issue
Block a user