diff --git a/backend/README.md b/backend/README.md index f97a7ab..7727cf5 100644 --- a/backend/README.md +++ b/backend/README.md @@ -822,10 +822,13 @@ preview card fetched from that page's Open Graph tags (`og:title`, `Content-Type` is `text/html`. - **Cached by URL, not by message** (`link_previews` table, unique on `url`) — a URL posted by five different people in five different rooms - fetches once. A row also gets written on a *failed* fetch - (`fetch_failed=True`) so a URL that genuinely doesn't unfurl (SSRF - rejection, timeout, no usable title) isn't re-attempted on every message - that references it; both kinds expire after 7 days (`_CACHE_TTL`). + within the same short window fetches once. A row also gets written on a + *failed* fetch (`fetch_failed=True`) so a URL that genuinely doesn't + unfurl (SSRF rejection, timeout, no usable title) isn't re-attempted on + every message that references it; both kinds expire after 5 minutes + (`_CACHE_TTL` — #70: was 7 days, confirmed live as far too long, a + re-posted URL whose title/content had genuinely changed kept showing + the stale first-fetch preview for up to a week). - Parsed with stdlib `html.parser.HTMLParser`, not a new dependency — only meta-tag scraping is needed, not general HTML parsing. - Editing a message re-extracts the URL; if it changed or was removed, the diff --git a/backend/app/services/link_preview_service.py b/backend/app/services/link_preview_service.py index 64331ca..cad4a6e 100644 --- a/backend/app/services/link_preview_service.py +++ b/backend/app/services/link_preview_service.py @@ -27,8 +27,18 @@ _TRAILING_PUNCTUATION = ".,;:!?)'\">" _FETCH_TIMEOUT_SECONDS = 5.0 _MAX_BYTES = 512 * 1024 _MAX_REDIRECTS = 3 +# #70: was 7 days -- confirmed live as too long for how this app actually +# gets used: re-posting a URL whose title/content had genuinely changed +# kept showing the stale first-fetch preview for up to a week. Short +# enough that it's effectively "always fresh" for any realistic human +# posting cadence, while still doing the one thing a cache here is +# actually for -- collapsing a burst of near-simultaneous fetches of the +# same URL (several people pasting the same link within moments of each +# other, or the same person's message history being loaded repeatedly) +# into one, and not hammering a URL that just failed on every message +# that references it. _USER_AGENT = "ds-chat-link-preview/1.0" -_CACHE_TTL = timedelta(days=7) +_CACHE_TTL = timedelta(minutes=5) def extract_first_url(content: str | None) -> str | None: diff --git a/backend/tests/test_link_previews.py b/backend/tests/test_link_previews.py index f7086ef..93e60aa 100644 --- a/backend/tests/test_link_previews.py +++ b/backend/tests/test_link_previews.py @@ -1,9 +1,13 @@ import asyncio import uuid +from datetime import datetime, timedelta, timezone import pytest_asyncio +from sqlalchemy import select +from app.database import async_session_factory from app.database import engine as _link_preview_engine +from app.models import LinkPreview from app.schemas.user import UserCreate from app.services.auth_service import register_user from app.services.link_preview_service import extract_first_url @@ -294,3 +298,66 @@ async def test_link_preview_reused_across_messages_with_same_url(client, db_sess history = (await client.get(f"/api/rooms/{room['id']}/messages")).json() assert len(history) == 2 assert all(m["link_preview"]["title"] == "Example Article" for m in history) + + +async def test_link_preview_refetches_after_cache_expires(client, db_session, monkeypatch): + # #70: a real report -- re-posting a URL whose title had genuinely + # changed kept showing the stale first-fetch preview, because the + # cache TTL used to be 7 days. Simulates that expiry directly (rather + # than actually sleeping 5+ minutes) by backdating the cached row's + # fetched_at past the TTL, then confirms a second post of the same URL + # picks up new content instead of the stale cached title. + captured_tasks: list[asyncio.Task] = [] + real_create_task = asyncio.create_task + + def fake_create_task(coro): + task = real_create_task(coro) + captured_tasks.append(task) + return task + + monkeypatch.setattr("app.services.message_events.asyncio.create_task", fake_create_task) + calls: list[str] = [] + monkeypatch.setattr( + "app.services.link_preview_service.httpx.AsyncClient", + _fake_client_factory(call_log=calls), + ) + url = _unique_url() + + await register_and_login(client, db_session, username="alice") + room = (await client.post("/api/rooms", json={"name": "general"})).json() + webhook = (await client.post(f"/api/rooms/{room['id']}/webhooks/incoming", json={})).json() + + resp1 = await client.post( + f"/api/webhooks/incoming/{webhook['token']}", json={"content": f"see {url}"} + ) + assert resp1.status_code == 204 + await asyncio.gather(*captured_tasks) + captured_tasks.clear() + + async with async_session_factory() as session: + row = ( + await session.execute(select(LinkPreview).where(LinkPreview.url == url)) + ).scalar_one() + row.fetched_at = datetime.now(timezone.utc) - timedelta(minutes=10) + await session.commit() + + updated_html = _OG_HTML.replace(b"Example Article", b"Updated Article") + monkeypatch.setattr( + "app.services.link_preview_service.httpx.AsyncClient", + _fake_client_factory(html=updated_html, call_log=calls), + ) + + resp2 = await client.post( + f"/api/webhooks/incoming/{webhook['token']}", json={"content": f"again: {url}"} + ) + assert resp2.status_code == 204 + await asyncio.gather(*captured_tasks) + + assert len(calls) == 2 # the expired cache forced a second real fetch + + # Cached by URL, not by message (see link_preview_service.py) -- the + # row was refreshed in place, so *both* messages referencing this URL + # now show the new title on a history reload, not one each. + history = (await client.get(f"/api/rooms/{room['id']}/messages")).json() + assert len(history) == 2 + assert all(m["link_preview"]["title"] == "Updated Article" for m in history)