Private
Public Access
Shorten link preview cache TTL from 7 days to 5 minutes (#70)
Re-posting a URL whose title/content had genuinely changed kept showing the stale first-fetch preview for up to a week. 5 minutes is effectively "always fresh" for any realistic re-share cadence, while still collapsing a burst of near-simultaneous fetches of the same URL into one and not re-hammering a URL that just failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+7
-4
@@ -822,10 +822,13 @@ preview card fetched from that page's Open Graph tags (`og:title`,
|
|||||||
`Content-Type` is `text/html`.
|
`Content-Type` is `text/html`.
|
||||||
- **Cached by URL, not by message** (`link_previews` table, unique on
|
- **Cached by URL, not by message** (`link_previews` table, unique on
|
||||||
`url`) — a URL posted by five different people in five different rooms
|
`url`) — a URL posted by five different people in five different rooms
|
||||||
fetches once. A row also gets written on a *failed* fetch
|
within the same short window fetches once. A row also gets written on a
|
||||||
(`fetch_failed=True`) so a URL that genuinely doesn't unfurl (SSRF
|
*failed* fetch (`fetch_failed=True`) so a URL that genuinely doesn't
|
||||||
rejection, timeout, no usable title) isn't re-attempted on every message
|
unfurl (SSRF rejection, timeout, no usable title) isn't re-attempted on
|
||||||
that references it; both kinds expire after 7 days (`_CACHE_TTL`).
|
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
|
- Parsed with stdlib `html.parser.HTMLParser`, not a new dependency — only
|
||||||
meta-tag scraping is needed, not general HTML parsing.
|
meta-tag scraping is needed, not general HTML parsing.
|
||||||
- Editing a message re-extracts the URL; if it changed or was removed, the
|
- Editing a message re-extracts the URL; if it changed or was removed, the
|
||||||
|
|||||||
@@ -27,8 +27,18 @@ _TRAILING_PUNCTUATION = ".,;:!?)'\">"
|
|||||||
_FETCH_TIMEOUT_SECONDS = 5.0
|
_FETCH_TIMEOUT_SECONDS = 5.0
|
||||||
_MAX_BYTES = 512 * 1024
|
_MAX_BYTES = 512 * 1024
|
||||||
_MAX_REDIRECTS = 3
|
_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"
|
_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:
|
def extract_first_url(content: str | None) -> str | None:
|
||||||
|
|||||||
@@ -1,9 +1,13 @@
|
|||||||
import asyncio
|
import asyncio
|
||||||
import uuid
|
import uuid
|
||||||
|
from datetime import datetime, timedelta, timezone
|
||||||
|
|
||||||
import pytest_asyncio
|
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.database import engine as _link_preview_engine
|
||||||
|
from app.models import LinkPreview
|
||||||
from app.schemas.user import UserCreate
|
from app.schemas.user import UserCreate
|
||||||
from app.services.auth_service import register_user
|
from app.services.auth_service import register_user
|
||||||
from app.services.link_preview_service import extract_first_url
|
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()
|
history = (await client.get(f"/api/rooms/{room['id']}/messages")).json()
|
||||||
assert len(history) == 2
|
assert len(history) == 2
|
||||||
assert all(m["link_preview"]["title"] == "Example Article" for m in history)
|
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)
|
||||||
|
|||||||
Reference in New Issue
Block a user