fix(storage): #106.5 — serve_blob בודק קיום ב-S3 ספציפית (לא dual disk-OR-S3) #201
13
web/app.py
13
web/app.py
@@ -2789,14 +2789,21 @@ async def serve_blob(
|
|||||||
from pathlib import Path as _Path
|
from pathlib import Path as _Path
|
||||||
backend = storage.get_storage()
|
backend = storage.get_storage()
|
||||||
if backend.name != "filesystem":
|
if backend.name != "filesystem":
|
||||||
|
# Presign ONLY when the object is actually in S3. Under ``dual`` the
|
||||||
|
# generic ``exists`` is disk-OR-S3, so a disk-only file (failed mirror /
|
||||||
|
# outside the migration set) would otherwise be redirected to a presigned
|
||||||
|
# URL that 404s on MinIO. Probe the S3 sub-backend specifically
|
||||||
|
# (DualBackend.s3 — or the S3 backend itself under ``s3``) so disk-only
|
||||||
|
# files fall back to a real disk read.
|
||||||
|
s3 = getattr(backend, "s3", backend)
|
||||||
try:
|
try:
|
||||||
key = storage.normalize_key(path)
|
key = storage.normalize_key(path)
|
||||||
if await backend.exists(key, bucket=bucket):
|
if await s3.exists(key, bucket=bucket):
|
||||||
url = await backend.presign_get(key, bucket=bucket, download_name=filename)
|
url = await s3.presign_get(key, bucket=bucket, download_name=filename)
|
||||||
return RedirectResponse(url, status_code=302)
|
return RedirectResponse(url, status_code=302)
|
||||||
except Exception: # noqa: BLE001 — never 500 on a key/presign hiccup; fall back to disk
|
except Exception: # noqa: BLE001 — never 500 on a key/presign hiccup; fall back to disk
|
||||||
logger.exception("serve_blob: presign failed for %s; falling back to disk", path)
|
logger.exception("serve_blob: presign failed for %s; falling back to disk", path)
|
||||||
# not in object storage (dual fallback) — serve the disk copy if present
|
# not in object storage (dual disk-fallback) — serve the disk copy if present
|
||||||
if not _Path(path).exists():
|
if not _Path(path).exists():
|
||||||
raise HTTPException(404, "קובץ לא נמצא")
|
raise HTTPException(404, "קובץ לא נמצא")
|
||||||
return FileResponse(path, media_type=media_type, filename=filename)
|
return FileResponse(path, media_type=media_type, filename=filename)
|
||||||
|
|||||||
@@ -78,3 +78,34 @@ def test_s3_missing_no_disk_404(monkeypatch):
|
|||||||
with pytest.raises(HTTPException) as ei:
|
with pytest.raises(HTTPException) as ei:
|
||||||
_serve(monkeypatch, "s3", False, DATA_DIR / "audit" / "_nope.txt")
|
_serve(monkeypatch, "s3", False, DATA_DIR / "audit" / "_nope.txt")
|
||||||
assert ei.value.status_code == 404
|
assert ei.value.status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
class _FakeDual:
|
||||||
|
"""DualBackend stand-in: generic exists() is disk-OR-S3 (always True here),
|
||||||
|
but the .s3 sub-backend is S3-only. The fix must probe .s3, not exists()."""
|
||||||
|
name = "dual"
|
||||||
|
|
||||||
|
def __init__(self, s3_has: bool) -> None:
|
||||||
|
self.s3 = _FakeBackend("s3", s3_has)
|
||||||
|
|
||||||
|
async def exists(self, key, *, bucket) -> bool: # noqa: ANN001 — disk-or-S3 → True
|
||||||
|
return True
|
||||||
|
|
||||||
|
async def presign_get(self, key, *, bucket, download_name=None) -> str: # noqa: ANN001
|
||||||
|
return f"https://s3.example/WRONG/{key}" # must NOT be used (proves .s3 probed)
|
||||||
|
|
||||||
|
|
||||||
|
def test_dual_probes_s3_subbackend_not_generic_exists(blob, monkeypatch):
|
||||||
|
"""Regression (DualBackend.exists is disk-OR-S3): a file on disk but NOT in
|
||||||
|
S3 must fall back to a disk FileResponse, never a presigned URL that 404s."""
|
||||||
|
monkeypatch.setattr(storage, "get_storage", lambda: _FakeDual(s3_has=False))
|
||||||
|
r = asyncio.new_event_loop().run_until_complete(
|
||||||
|
app.serve_blob(str(blob), media_type="text/plain", filename="x.txt"))
|
||||||
|
assert isinstance(r, FileResponse) # disk fallback, NOT a (broken) redirect
|
||||||
|
|
||||||
|
|
||||||
|
def test_dual_in_s3_uses_s3_subbackend(blob, monkeypatch):
|
||||||
|
monkeypatch.setattr(storage, "get_storage", lambda: _FakeDual(s3_has=True))
|
||||||
|
r = asyncio.new_event_loop().run_until_complete(
|
||||||
|
app.serve_blob(str(blob), media_type="text/plain", filename="x.txt"))
|
||||||
|
assert isinstance(r, RedirectResponse)
|
||||||
|
|||||||
Reference in New Issue
Block a user