peter
80ea1918e3
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.
76 lines
2.8 KiB
Python
76 lines
2.8 KiB
Python
"""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, User
|
|
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
|
|
|
|
|
|
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
|