fix(downloads): clamp notification title to column limit so download_gc can't wedge
A downloaded FB/IG reel's "title" is the whole caption (up to ~1900 chars). It flowed untrimmed into Notification.title (String(255)); at the batched download_gc commit that raised psycopg StringDataRightTruncation in pass 1, aborting the whole run so expiry deletion, LRU eviction and orphan reap never executed (13/110 prod assets already exceed the limit). Clamp the title to 255 (with an ellipsis) centrally in create_notification — defending all six callers — since the full title already reaches the client via `data` and the column is only an English fallback. Add a regression test.
This commit is contained in:
@@ -14,6 +14,13 @@ from app.models import Notification
|
|||||||
# rows are never pruned (the user hasn't seen them yet).
|
# rows are never pruned (the user hasn't seen them yet).
|
||||||
READ_SOFT_CAP = 200
|
READ_SOFT_CAP = 200
|
||||||
|
|
||||||
|
# Hard limit of the Notification.title column (String(255)). Titles are only an English
|
||||||
|
# fallback — the client renders the translated message from `data` — so clamping an
|
||||||
|
# over-long one (e.g. a FB/IG reel whose "title" is the whole caption, hundreds of chars)
|
||||||
|
# is harmless to the UI and prevents a StringDataRightTruncation that would abort the
|
||||||
|
# caller's whole transaction (the batched download_gc run in particular).
|
||||||
|
TITLE_MAX_LEN = 255
|
||||||
|
|
||||||
|
|
||||||
def create_notification(
|
def create_notification(
|
||||||
db: Session,
|
db: Session,
|
||||||
@@ -27,6 +34,8 @@ def create_notification(
|
|||||||
) -> Notification:
|
) -> Notification:
|
||||||
"""Insert one notification for a user. Callers that batch many inserts in a single
|
"""Insert one notification for a user. Callers that batch many inserts in a single
|
||||||
transaction can pass commit=False and commit once themselves."""
|
transaction can pass commit=False and commit once themselves."""
|
||||||
|
if title is not None and len(title) > TITLE_MAX_LEN:
|
||||||
|
title = title[: TITLE_MAX_LEN - 1] + "…" # clamp to the column limit, add an ellipsis
|
||||||
notif = Notification(
|
notif = Notification(
|
||||||
user_id=user_id, type=type, title=title, body=body, data=data
|
user_id=user_id, type=type, title=title, body=body, data=data
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
"""create_notification title-clamp regression.
|
||||||
|
|
||||||
|
A downloaded FB/IG reel's "title" is the whole caption (hundreds to ~1900 chars). Before the
|
||||||
|
clamp that string went straight into Notification.title (String(255)) and, at the batched
|
||||||
|
download_gc commit, raised psycopg StringDataRightTruncation — aborting the entire GC run so
|
||||||
|
no expiry deletion / LRU eviction / orphan reap ever ran. The clamp trims the title to the
|
||||||
|
column limit centrally, defending all callers regardless of DB backend."""
|
||||||
|
from app.models import Notification
|
||||||
|
from app.notifications import TITLE_MAX_LEN, create_notification
|
||||||
|
|
||||||
|
|
||||||
|
class _CaptureDB:
|
||||||
|
"""Captures the added row without a real DB, so the clamp is verified on every lane
|
||||||
|
(the truncation only reproduces on Postgres; the clamp logic is backend-agnostic)."""
|
||||||
|
|
||||||
|
def __init__(self):
|
||||||
|
self.added: list[Notification] = []
|
||||||
|
|
||||||
|
def add(self, obj):
|
||||||
|
self.added.append(obj)
|
||||||
|
|
||||||
|
def commit(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def refresh(self, _obj):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
def test_title_clamped_to_column_limit():
|
||||||
|
long_title = "x" * 2000 # e.g. a reel caption
|
||||||
|
notif = create_notification(
|
||||||
|
_CaptureDB(), user_id=1, type="download_expiring", title=long_title
|
||||||
|
)
|
||||||
|
assert len(notif.title) == TITLE_MAX_LEN
|
||||||
|
assert notif.title.endswith("…")
|
||||||
|
# The untruncated title still reaches the client via `data`, which is never clamped.
|
||||||
|
notif2 = create_notification(
|
||||||
|
_CaptureDB(),
|
||||||
|
user_id=1,
|
||||||
|
type="download_expiring",
|
||||||
|
title=long_title,
|
||||||
|
data={"kind": "download_expiring", "title": long_title},
|
||||||
|
)
|
||||||
|
assert notif2.data["title"] == long_title
|
||||||
|
|
||||||
|
|
||||||
|
def test_short_title_is_untouched():
|
||||||
|
notif = create_notification(_CaptureDB(), user_id=1, type="t", title="short")
|
||||||
|
assert notif.title == "short"
|
||||||
|
|
||||||
|
|
||||||
|
def test_title_at_exactly_the_limit_is_untouched():
|
||||||
|
exact = "y" * TITLE_MAX_LEN
|
||||||
|
notif = create_notification(_CaptureDB(), user_id=1, type="t", title=exact)
|
||||||
|
assert notif.title == exact
|
||||||
Reference in New Issue
Block a user