fix(downloads): derive title clamp from the column + real-Postgres regression
Address review findings on the title-clamp: TITLE_MAX_LEN now reads the length straight from Notification.title so the clamp can't drift from the column a migration would change (single source of truth). Add a db-lane test that actually commits a >255-char title into Postgres (end-to-end proof the truncation is gone, not just the clamp logic) and one that asserts the constant tracks the column width.
This commit is contained in:
@@ -14,12 +14,13 @@ from app.models import Notification
|
||||
# rows are never pruned (the user hasn't seen them yet).
|
||||
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
|
||||
# Hard limit of the Notification.title column, read straight from the model so the clamp
|
||||
# can never drift from the actual column width (a later migration is the single source of
|
||||
# truth). 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 = Notification.__table__.c.title.type.length
|
||||
|
||||
|
||||
def create_notification(
|
||||
|
||||
@@ -5,7 +5,7 @@ clamp that string went straight into Notification.title (String(255)) and, at th
|
||||
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.models import Notification, User
|
||||
from app.notifications import TITLE_MAX_LEN, create_notification
|
||||
|
||||
|
||||
@@ -53,3 +53,23 @@ 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
|
||||
|
||||
|
||||
def test_limit_tracks_the_actual_column_width():
|
||||
# The clamp is derived from the model, so it can't silently drift from the column a
|
||||
# migration would change (which is exactly what re-introduces the truncation bug).
|
||||
assert TITLE_MAX_LEN == Notification.__table__.c.title.type.length
|
||||
|
||||
|
||||
def test_over_long_title_actually_commits_on_postgres(db):
|
||||
# The stub tests above prove the clamp *logic*; this proves the real fix end-to-end —
|
||||
# a caption-length title now COMMITS into varchar(TITLE_MAX_LEN) instead of raising
|
||||
# StringDataRightTruncation. Uses the opt-in Postgres lane (skipped with no DB).
|
||||
user = User(email="reel@example.com")
|
||||
db.add(user)
|
||||
db.flush()
|
||||
notif = create_notification(
|
||||
db, user_id=user.id, type="download_expiring", title="z" * 2000
|
||||
)
|
||||
db.refresh(notif)
|
||||
assert len(notif.title) == TITLE_MAX_LEN # persisted, not truncated by the DB
|
||||
|
||||
Reference in New Issue
Block a user