URLs unfurl on their own, and a lone link becomes the note
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 9s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 37s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m6s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m16s
Desktop (Tauri) / Update manifest (push) Successful in 6s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 2s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 9s
CI & Build / integration (push) Successful in 14s
CI & Build / Build & push image (push) Successful in 37s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m6s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 4m16s
Desktop (Tauri) / Update manifest (push) Successful in 6s
Operator: *"I'd like for URLs to unfurl. To be the whole note when the note is a single URL, and to be a compact slot on the bottom of the note when the URL is inline. We also need to support multiple URLs in a single note."* Less new machinery than it sounds: `unfurl.py` already fetched and parsed OG tags, SSRF-hardened, and `note_link_previews` was already `UNIQUE(note_id, url)` — so several URLs per note has worked at the storage layer all along. What was missing was that it needed a button, had one size, and drew that size in the wrong place. **Automatic, and never in the way.** New `unfurl_queue.py` detects a body's URLs and fetches them on a background task AFTER the note is committed. Capture speed is the product: an unfurl is a five-second timeout against a host nobody controls, and a note has to persist the instant someone stops typing. Scheduled from create, from a body edit, and from a synced push — so a linked desktop or Android client gets previews too, on its next pull. An unlinked one has no server to ask and simply has none, which is the honest consequence of being offline. Safe to call on every save: it re-reads what's cached and does nothing when nothing is new. Capped at five URLs per note, silent on every failure (a link that won't fetch isn't an error the person needs — the note is fine, the link is still there), and it re-checks before storing, so a slow fetch can't resurrect a preview for a URL that was deleted while it was in flight. **Two presentations.** A note whose body is nothing but a URL renders as its preview and nothing else — printing the raw URL under a card that already says where it goes is saying the same thing twice, badly. Until the fetch lands, or if it never does, the URL stands in, so the card is never blank. Anything else gets a compact strip. **And the strip moved.** Previews were rendered ABOVE the body, which put a stranger's headline where the note's own first line should be — worse now that the first line IS the note's name. They sit at the foot of the card now, under the note's own words. The editor's "Preview example.com" button is gone with the manual path; removing an unwanted preview stays, and stays editor-only. Nine tests: three on detection (order, dedupe, sentence-punctuation trimming, non-http rejection) in the unit lane, and three in the integration lane for what only a real database shows — the upsert landing on the right row, a second pass fetching nothing, and a preview NOT being stored for a URL that left the body.
This commit is contained in:
@@ -1,12 +1,24 @@
|
||||
<script setup lang="ts">
|
||||
import type { LinkPreview } from "../stores/notes";
|
||||
|
||||
defineProps<{ preview: LinkPreview; removable?: boolean }>();
|
||||
/**
|
||||
* A fetched link preview, in one of two sizes.
|
||||
*
|
||||
* `compact` is a single row — favicon-less, one line of title, the site name — for a
|
||||
* URL mentioned *inside* a note that has its own text. The note is the thing; the
|
||||
* link is a footnote to it.
|
||||
*
|
||||
* Full size is for a note that is NOTHING but a URL. There the link IS the note, and
|
||||
* a compact strip would be a card with nothing on it.
|
||||
*/
|
||||
defineProps<{ preview: LinkPreview; removable?: boolean; compact?: boolean }>();
|
||||
defineEmits<{ (e: "remove"): void }>();
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<div class="group/lp relative overflow-hidden rounded-lg border border-neutral-200 dark:border-neutral-700">
|
||||
<div
|
||||
class="group/lp relative overflow-hidden rounded-lg border border-neutral-200 dark:border-neutral-700"
|
||||
>
|
||||
<a
|
||||
:href="preview.url"
|
||||
target="_blank"
|
||||
@@ -19,16 +31,28 @@ defineEmits<{ (e: "remove"): void }>();
|
||||
alt=""
|
||||
loading="lazy"
|
||||
decoding="async"
|
||||
class="h-auto w-24 shrink-0 self-stretch object-cover"
|
||||
class="h-auto shrink-0 self-stretch object-cover"
|
||||
:class="compact ? 'w-12' : 'w-24'"
|
||||
/>
|
||||
<div class="min-w-0 flex-1 px-3 py-2">
|
||||
<p v-if="preview.site_name" class="truncate text-[11px] uppercase tracking-wide text-neutral-400">
|
||||
<div class="min-w-0 flex-1" :class="compact ? 'px-2 py-1.5' : 'px-3 py-2'">
|
||||
<p
|
||||
v-if="preview.site_name"
|
||||
class="truncate uppercase tracking-wide text-neutral-400"
|
||||
:class="compact ? 'text-[10px]' : 'text-[11px]'"
|
||||
>
|
||||
{{ preview.site_name }}
|
||||
</p>
|
||||
<p class="truncate text-sm font-medium text-neutral-800 dark:text-neutral-100">
|
||||
<p
|
||||
class="truncate font-medium text-neutral-800 dark:text-neutral-100"
|
||||
:class="compact ? 'text-xs' : 'text-sm'"
|
||||
>
|
||||
{{ preview.title || preview.url }}
|
||||
</p>
|
||||
<p v-if="preview.description" class="mt-0.5 line-clamp-2 text-xs text-neutral-500 dark:text-neutral-400">
|
||||
<!-- The description is the first thing to go when there is no room for it. -->
|
||||
<p
|
||||
v-if="preview.description && !compact"
|
||||
class="mt-0.5 line-clamp-2 text-xs text-neutral-500 dark:text-neutral-400"
|
||||
>
|
||||
{{ preview.description }}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
@@ -68,6 +68,26 @@ const otherAttachments = computed(() => props.note.attachments.filter((a) => !a.
|
||||
// exactly, and skips parsing a body the card was never going to show.
|
||||
const PREVIEW_LINES = 8;
|
||||
|
||||
// --- Links ------------------------------------------------------------------
|
||||
//
|
||||
// A note that is NOTHING but a URL is a link, and its preview is the whole card —
|
||||
// showing the raw URL underneath a card that already says where it goes is saying the
|
||||
// same thing twice, badly. A URL mentioned *inside* a note is a footnote to it, and
|
||||
// gets a compact strip at the bottom instead.
|
||||
//
|
||||
// Whitespace either side still counts as lone: someone pasting a link rarely trims it.
|
||||
const LONE_URL_RE = /^\s*(https?:\/\/[^\s<>"'\])]+)\s*$/;
|
||||
|
||||
const isLoneUrl = computed(() => LONE_URL_RE.test(props.note.body) && !props.note.items.length);
|
||||
|
||||
/** The preview for a lone-URL note — null while it is still being fetched, or if it
|
||||
* could never be fetched at all. */
|
||||
const loneUrlPreview = computed(() => {
|
||||
if (!isLoneUrl.value) return null;
|
||||
const url = props.note.body.trim();
|
||||
return props.note.previews.find((p) => p.url === url) ?? null;
|
||||
});
|
||||
|
||||
const bodyPreview = computed(() => {
|
||||
const lines = props.note.body.split("\n");
|
||||
if (lines.length <= PREVIEW_LINES) return props.note.body;
|
||||
@@ -226,9 +246,6 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown))
|
||||
</span>
|
||||
</div>
|
||||
|
||||
<div v-if="note.previews.length" class="mb-2 flex flex-col gap-2">
|
||||
<LinkPreview v-for="p in note.previews" :key="p.id" :preview="p" />
|
||||
</div>
|
||||
|
||||
<!-- One render path: every note is a body plus, maybe, checkable items.
|
||||
A focusable div rather than a <button>, because a checklist nests interactive
|
||||
@@ -241,7 +258,11 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown))
|
||||
@click="emit('open', note)"
|
||||
@keydown.enter="emit('open', note)"
|
||||
>
|
||||
<div v-if="note.body" class="text-sm text-neutral-700 dark:text-neutral-300">
|
||||
<!-- A lone URL renders as its preview and nothing else. Until the fetch lands
|
||||
— or if it never does — the URL itself stands in, so the card is never
|
||||
blank and the link is never unreachable. -->
|
||||
<LinkPreview v-if="loneUrlPreview" :preview="loneUrlPreview" />
|
||||
<div v-else-if="note.body" class="text-sm text-neutral-700 dark:text-neutral-300">
|
||||
<MarkdownText :text="bodyPreview" />
|
||||
</div>
|
||||
<p
|
||||
@@ -251,6 +272,13 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown))
|
||||
Empty note
|
||||
</p>
|
||||
</div>
|
||||
<!-- Inline links: a compact strip at the FOOT of the card, under the note's own
|
||||
words rather than stacked on top of them. They were above the body until
|
||||
M13 — which put a stranger's headline where the note's first line should be. -->
|
||||
<div v-if="!isLoneUrl && note.previews.length" class="mt-2 flex flex-col gap-1">
|
||||
<LinkPreview v-for="p in note.previews" :key="p.id" :preview="p" compact />
|
||||
</div>
|
||||
|
||||
<NoteChecklist
|
||||
v-if="note.items.length"
|
||||
:class="note.body ? 'mt-2' : ''"
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
<script setup lang="ts">
|
||||
import { computed, nextTick, onMounted, ref, watch } from "vue";
|
||||
import { useNotesStore } from "../stores/notes";
|
||||
import { useConfigStore } from "../stores/config";
|
||||
import ColorPicker from "./ColorPicker.vue";
|
||||
import Icon from "./Icon.vue";
|
||||
import LabelPicker from "./LabelPicker.vue";
|
||||
@@ -24,7 +23,6 @@ const props = withDefaults(defineProps<{ note?: Note | null; initialBody?: strin
|
||||
});
|
||||
const emit = defineEmits<{ (e: "close"): void; (e: "navigate", id: string): void }>();
|
||||
const notes = useNotesStore();
|
||||
const config = useConfigStore();
|
||||
|
||||
const noteId = ref<string | null>(props.note?.id ?? null);
|
||||
const body = ref(props.note?.body ?? props.initialBody);
|
||||
@@ -323,41 +321,12 @@ async function uploadFile(file: File) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---- link previews (URL unfurl) ----
|
||||
const unfurling = ref<string | null>(null); // the URL currently being fetched
|
||||
const unfurlError = ref("");
|
||||
// Bare http(s) URLs in the body; trailing sentence punctuation trimmed.
|
||||
const URL_RE = /(https?:\/\/[^\s<>"'\])]+)/g;
|
||||
const detectedUrls = computed(() => {
|
||||
const out: string[] = [];
|
||||
for (const m of body.value.matchAll(URL_RE)) {
|
||||
const u = m[1].replace(/[.,;:!?]+$/, "");
|
||||
if (!out.includes(u)) out.push(u);
|
||||
}
|
||||
return out;
|
||||
});
|
||||
const previewedUrls = computed(() => new Set(liveNote.value.previews.map((p) => p.url)));
|
||||
const unpreviewedUrls = computed(() => detectedUrls.value.filter((u) => !previewedUrls.value.has(u)));
|
||||
async function addPreview(url: string) {
|
||||
const id = await ensureDraft();
|
||||
if (!id) return;
|
||||
unfurling.value = url;
|
||||
unfurlError.value = "";
|
||||
try {
|
||||
await notes.unfurl(id, url);
|
||||
} catch (e) {
|
||||
unfurlError.value = (e as { error?: string }).error ?? "Couldn't fetch a preview for that link.";
|
||||
} finally {
|
||||
unfurling.value = null;
|
||||
}
|
||||
}
|
||||
function shortUrl(url: string): string {
|
||||
try {
|
||||
return new URL(url).hostname.replace(/^www\./, "");
|
||||
} catch {
|
||||
return url;
|
||||
}
|
||||
}
|
||||
// ---- link previews ----
|
||||
//
|
||||
// Nothing to trigger any more: the server unfurls a note's URLs in the background
|
||||
// after each save (`unfurl_queue.py`) and the preview arrives on a later read. What
|
||||
// is left here is removing one you don't want — the editor is the only place with
|
||||
// room to offer that, and the card deliberately doesn't.
|
||||
async function onFileChange(e: Event) {
|
||||
const input = e.target as HTMLInputElement;
|
||||
const file = input.files?.[0];
|
||||
@@ -494,7 +463,7 @@ function revPreview(rev: NoteRevision): string {
|
||||
</div>
|
||||
<p v-if="uploadError" class="text-xs text-red-600 dark:text-red-400">{{ uploadError }}</p>
|
||||
|
||||
<!-- Link previews: stored preview cards + one "Preview <domain>" per detected URL -->
|
||||
<!-- Fetched automatically after each save; removable here and nowhere else. -->
|
||||
<div v-if="liveNote.previews.length" class="flex flex-col gap-2">
|
||||
<LinkPreview
|
||||
v-for="p in liveNote.previews"
|
||||
@@ -504,23 +473,6 @@ function revPreview(rev: NoteRevision): string {
|
||||
@remove="notes.deletePreview(liveNote.id, p.id)"
|
||||
/>
|
||||
</div>
|
||||
<div
|
||||
v-if="config.enableUrlUnfurl && !liveNote.trashed && unpreviewedUrls.length"
|
||||
class="flex flex-wrap gap-1.5"
|
||||
>
|
||||
<button
|
||||
v-for="u in unpreviewedUrls"
|
||||
:key="u"
|
||||
type="button"
|
||||
class="inline-flex items-center gap-1 rounded-full border border-neutral-200 px-2 py-0.5 text-xs text-neutral-500 hover:bg-neutral-50 focus:outline-none focus-visible:ring-2 focus-visible:ring-brand disabled:opacity-60 dark:border-neutral-700 dark:hover:bg-neutral-800"
|
||||
:disabled="unfurling === u"
|
||||
@click="addPreview(u)"
|
||||
>
|
||||
<Icon name="link" />
|
||||
{{ unfurling === u ? "Fetching…" : `Preview ${shortUrl(u)}` }}
|
||||
</button>
|
||||
</div>
|
||||
<p v-if="unfurlError" class="text-xs text-red-600 dark:text-red-400">{{ unfurlError }}</p>
|
||||
|
||||
<textarea
|
||||
ref="bodyInput"
|
||||
|
||||
@@ -37,6 +37,7 @@ from ..models.note_revision import NoteRevision
|
||||
from ..responses import json_error, not_found, parse_uuid
|
||||
from ..retention import purge_note
|
||||
from ..settings import get_setting
|
||||
from ..unfurl_queue import schedule as schedule_unfurls
|
||||
from ..unfurl import UnfurlError, unfurl
|
||||
from ._bp import bp
|
||||
from .helpers import (
|
||||
@@ -454,6 +455,9 @@ async def create_note():
|
||||
await _reconcile_tags(db, note)
|
||||
await db.commit()
|
||||
await db.refresh(note)
|
||||
# After the commit, never before it: the note is saved and the response is
|
||||
# about to go out. Any link previews arrive on a later read.
|
||||
schedule_unfurls(note.id, note.body)
|
||||
return jsonify(await _serialize_note(db, note)), 201
|
||||
|
||||
|
||||
@@ -509,6 +513,8 @@ async def update_note(note_id: str):
|
||||
db.add(NoteRevision(note_id=note.id, body=old_body))
|
||||
await db.commit()
|
||||
await db.refresh(note)
|
||||
if note.body != old_body:
|
||||
schedule_unfurls(note.id, note.body)
|
||||
return jsonify(await _serialize_note(db, note))
|
||||
|
||||
|
||||
|
||||
@@ -35,6 +35,7 @@ from .notes import (
|
||||
)
|
||||
from .retention import purge_note
|
||||
from .serialize import serialize_label_sync
|
||||
from .unfurl_queue import schedule as schedule_unfurls
|
||||
|
||||
bp = Blueprint("sync", __name__, url_prefix="/api/sync")
|
||||
|
||||
@@ -325,6 +326,11 @@ async def _apply_note(db, ch: dict) -> dict:
|
||||
await _apply_note_manual_labels(db, note, ch)
|
||||
await db.flush()
|
||||
await db.refresh(note, ["sync_revision"])
|
||||
# A note pushed from a linked client gets the same link previews as one typed into
|
||||
# the web app — the client picks them up on its next pull. Scheduled rather than
|
||||
# awaited: a push batch must not wait on somebody else's website.
|
||||
if creating or note.body != old_body:
|
||||
schedule_unfurls(note.id, note.body)
|
||||
return {
|
||||
"id": str(nid),
|
||||
"entity": "note",
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
"""Unfurling a note's URLs in the background, after the note is already saved.
|
||||
|
||||
## Why this is not done inline
|
||||
|
||||
Capture speed is the product. Unfurling is a 5-second-timeout network call to a host
|
||||
nobody controls, and a note must persist the instant someone stops typing — so the
|
||||
save returns first and the preview catches up. A person who pastes a link and closes
|
||||
the composer has already done the thing they came to do.
|
||||
|
||||
## Why it is on the server rather than in each client
|
||||
|
||||
The server sees every note that reaches it, from all three surfaces, so detection and
|
||||
fetching live in one place instead of three. A linked desktop or Android client pushes
|
||||
its note and picks the preview up on the next pull; an unlinked one has no server to
|
||||
ask and simply has no preview until it links, which is the honest consequence of being
|
||||
offline rather than a gap to paper over.
|
||||
|
||||
## What it deliberately does not do
|
||||
|
||||
Fail loudly. A preview that could not be fetched is not an error the person needs —
|
||||
the note is fine, it just has no card. The link is still in the body, still clickable,
|
||||
still searchable.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import re
|
||||
import uuid
|
||||
|
||||
from sqlalchemy import select
|
||||
|
||||
from .db import session_scope
|
||||
from .models.note import Note
|
||||
from .models.note_link_preview import NoteLinkPreview
|
||||
from .settings import get_setting
|
||||
from .unfurl import UnfurlError, unfurl
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
# Matches the frontend's detector (NoteEditor.vue) so both surfaces agree on what
|
||||
# counts as a link. Trailing sentence punctuation is stripped below rather than in the
|
||||
# pattern — a URL can legitimately end in most of these characters, just not when the
|
||||
# sentence does.
|
||||
_URL_RE = re.compile(r"(https?://[^\s<>\"'\])]+)")
|
||||
|
||||
# Per note, per save. A body pasted full of links should not turn into a burst of
|
||||
# outbound requests; nobody is reading forty preview cards on one card anyway.
|
||||
MAX_URLS_PER_NOTE = 5
|
||||
|
||||
# Background tasks are only weakly referenced by the event loop, so without a strong
|
||||
# reference here a task can be garbage-collected mid-flight. Discarded on completion.
|
||||
_running: set[asyncio.Task] = set()
|
||||
|
||||
|
||||
def detect_urls(body: str | None) -> list[str]:
|
||||
"""Distinct http(s) URLs in a note body, in order, trailing punctuation trimmed."""
|
||||
out: list[str] = []
|
||||
for match in _URL_RE.finditer(body or ""):
|
||||
url = match.group(1).rstrip(".,;:!?")
|
||||
if url and url not in out:
|
||||
out.append(url)
|
||||
return out
|
||||
|
||||
|
||||
async def _fetch_and_store(note_id: uuid.UUID, url: str) -> None:
|
||||
"""Unfurl one URL and cache it against the note. Silent on every failure."""
|
||||
try:
|
||||
preview = await unfurl(url)
|
||||
except UnfurlError as e:
|
||||
# Expected and uninteresting: a dead link, a private address, a non-page.
|
||||
logger.debug("no preview for %s: %s", url, e)
|
||||
return
|
||||
except Exception:
|
||||
logger.warning("unexpected failure unfurling %s", url, exc_info=True)
|
||||
return
|
||||
|
||||
async with session_scope() as db:
|
||||
# The note may have been deleted or the URL removed while the fetch was in
|
||||
# flight, so re-check rather than assuming the world held still.
|
||||
note = await db.scalar(select(Note).where(Note.id == note_id, Note.deleted_at.is_(None)))
|
||||
if note is None or url not in detect_urls(note.body):
|
||||
return
|
||||
row = await db.scalar(
|
||||
select(NoteLinkPreview).where(NoteLinkPreview.note_id == note_id, NoteLinkPreview.url == url)
|
||||
)
|
||||
if row is None:
|
||||
row = NoteLinkPreview(note_id=note_id, url=url)
|
||||
db.add(row)
|
||||
row.title = preview["title"]
|
||||
row.description = preview["description"]
|
||||
row.image_url = preview["image_url"]
|
||||
row.site_name = preview["site_name"]
|
||||
await db.commit()
|
||||
|
||||
|
||||
async def _unfurl_new_urls(note_id: uuid.UUID, body: str) -> None:
|
||||
async with session_scope() as db:
|
||||
if not await get_setting(db, "enable_url_unfurl"):
|
||||
return
|
||||
cached = set(
|
||||
(
|
||||
await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note_id))
|
||||
).all()
|
||||
)
|
||||
fresh = [u for u in detect_urls(body) if u not in cached][:MAX_URLS_PER_NOTE]
|
||||
for url in fresh:
|
||||
await _fetch_and_store(note_id, url)
|
||||
|
||||
|
||||
def schedule(note_id: uuid.UUID, body: str | None) -> None:
|
||||
"""Queue an unfurl pass for a note that was just written. Returns immediately.
|
||||
|
||||
Safe to call on every save: it re-reads what is already cached and does nothing
|
||||
when there is nothing new, so an edit that doesn't touch the links costs one
|
||||
cheap query on a background task rather than a fetch.
|
||||
"""
|
||||
if not body or not detect_urls(body):
|
||||
return
|
||||
try:
|
||||
task = asyncio.create_task(_unfurl_new_urls(note_id, body))
|
||||
except RuntimeError:
|
||||
# No running loop — a script or a test calling the write path directly. The
|
||||
# note is saved either way; only the preview is skipped.
|
||||
return
|
||||
_running.add(task)
|
||||
task.add_done_callback(_running.discard)
|
||||
@@ -26,13 +26,15 @@ from thoughtsync.models.note import Note
|
||||
from thoughtsync.models.note_item import NoteItem
|
||||
from thoughtsync.models.user import User
|
||||
from thoughtsync.notes.helpers import derive_display_title
|
||||
from thoughtsync.models.note_link_preview import NoteLinkPreview
|
||||
from thoughtsync.sync import _apply_note_items
|
||||
from thoughtsync.unfurl_queue import _unfurl_new_urls, detect_urls
|
||||
|
||||
pytestmark = pytest.mark.integration
|
||||
|
||||
# Every table the tests touch, child-first so FKs never block the truncate.
|
||||
# RESTART IDENTITY + CASCADE keeps this honest if a table gains children later.
|
||||
_TABLES = "notes, note_items, note_revisions, note_labels, labels, users"
|
||||
_TABLES = "notes, note_items, note_revisions, note_labels, note_link_previews, labels, users"
|
||||
|
||||
|
||||
@pytest_asyncio.fixture
|
||||
@@ -201,3 +203,81 @@ async def test_a_note_with_only_items_still_has_a_name(db, owner):
|
||||
await db.commit()
|
||||
|
||||
assert (await db.scalar(select(Note.display_title).where(Note.id == note.id))) == "milk"
|
||||
|
||||
|
||||
async def test_auto_unfurl_stores_a_preview_and_skips_what_is_cached(db, owner, monkeypatch):
|
||||
"""The background pass, run inline so the assertions are deterministic.
|
||||
|
||||
The network is stubbed — this is about what reaches the DATABASE, not about
|
||||
parsing someone's OpenGraph tags (unfurl.py's own tests cover that). What matters
|
||||
here is the part only a real database can show: the unique constraint holding, the
|
||||
upsert going to the right row, and a second pass not re-fetching.
|
||||
"""
|
||||
note = Note(
|
||||
owner_id=owner.id,
|
||||
body="read https://example.com/a and https://example.com/b",
|
||||
display_title="read https://example.com/a and https://example.com/b",
|
||||
)
|
||||
db.add(note)
|
||||
await db.commit()
|
||||
|
||||
calls: list[str] = []
|
||||
|
||||
async def fake_unfurl(url):
|
||||
calls.append(url)
|
||||
return {"url": url, "title": f"T {url}", "description": None, "image_url": None, "site_name": "example.com"}
|
||||
|
||||
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
||||
|
||||
await _unfurl_new_urls(note.id, note.body)
|
||||
assert sorted(calls) == ["https://example.com/a", "https://example.com/b"]
|
||||
|
||||
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
|
||||
assert {r.url for r in rows} == {"https://example.com/a", "https://example.com/b"}
|
||||
assert all(r.title.startswith("T ") for r in rows)
|
||||
|
||||
# A second pass over an unchanged body fetches nothing — the whole reason
|
||||
# `schedule` is safe to call on every save.
|
||||
calls.clear()
|
||||
await _unfurl_new_urls(note.id, note.body)
|
||||
assert calls == []
|
||||
|
||||
|
||||
async def test_auto_unfurl_drops_a_preview_whose_url_left_the_body(db, owner, monkeypatch):
|
||||
"""A slow fetch must not resurrect a link the person deleted mid-flight."""
|
||||
note = Note(owner_id=owner.id, body="https://example.com/gone", display_title="x")
|
||||
db.add(note)
|
||||
await db.commit()
|
||||
|
||||
async def fake_unfurl(url):
|
||||
# Simulate the body changing while the request was in the air.
|
||||
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
|
||||
|
||||
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
||||
note.body = "changed my mind"
|
||||
await db.commit()
|
||||
|
||||
await _unfurl_new_urls(note.id, "https://example.com/gone")
|
||||
rows = (await db.scalars(select(NoteLinkPreview).where(NoteLinkPreview.note_id == note.id))).all()
|
||||
assert rows == [], "a preview was stored for a URL the note no longer contains"
|
||||
|
||||
|
||||
async def test_detection_agrees_with_what_gets_stored(db, owner, monkeypatch):
|
||||
"""The detector and the storage path read the same body the same way."""
|
||||
body = "one https://example.com/x. two (https://example.com/y) three"
|
||||
assert detect_urls(body) == ["https://example.com/x", "https://example.com/y"]
|
||||
|
||||
note = Note(owner_id=owner.id, body=body, display_title="one")
|
||||
db.add(note)
|
||||
await db.commit()
|
||||
|
||||
async def fake_unfurl(url):
|
||||
return {"url": url, "title": "T", "description": None, "image_url": None, "site_name": None}
|
||||
|
||||
monkeypatch.setattr("thoughtsync.unfurl_queue.unfurl", fake_unfurl)
|
||||
await _unfurl_new_urls(note.id, body)
|
||||
|
||||
stored = {
|
||||
r for r in (await db.scalars(select(NoteLinkPreview.url).where(NoteLinkPreview.note_id == note.id))).all()
|
||||
}
|
||||
assert stored == set(detect_urls(body))
|
||||
|
||||
@@ -5,6 +5,7 @@ import pytest
|
||||
from thoughtsync.app import create_app
|
||||
from thoughtsync.common import coerce_bool, parse_dt
|
||||
from thoughtsync.models.note import NOTE_COLORS, Note
|
||||
from thoughtsync.unfurl_queue import detect_urls
|
||||
from thoughtsync.notes import (
|
||||
_attachment_ext,
|
||||
_header_filename,
|
||||
@@ -386,3 +387,22 @@ def test_native_spec_roundtrip_fields():
|
||||
assert spec["created_at"].year == 2026
|
||||
assert spec["labels"] == ["x"]
|
||||
assert spec["attachments"] == [{"file": "attachments/ab/img.png", "mime": "image/png"}]
|
||||
|
||||
|
||||
def test_detect_urls_finds_each_link_once_in_order():
|
||||
body = "see https://example.com/a and https://example.com/b\nand https://example.com/a again"
|
||||
assert detect_urls(body) == ["https://example.com/a", "https://example.com/b"]
|
||||
|
||||
|
||||
def test_detect_urls_trims_sentence_punctuation():
|
||||
# A URL can end in most punctuation; a SENTENCE containing one usually doesn't.
|
||||
assert detect_urls("read https://example.com/page.") == ["https://example.com/page"]
|
||||
assert detect_urls("(see https://example.com/x)") == ["https://example.com/x"]
|
||||
# …but a path that legitimately ends in a slash or a dash keeps it.
|
||||
assert detect_urls("https://example.com/dir/") == ["https://example.com/dir/"]
|
||||
|
||||
|
||||
def test_detect_urls_ignores_non_http():
|
||||
assert detect_urls("ftp://example.com and mailto:a@b.c and bare example.com") == []
|
||||
assert detect_urls(None) == []
|
||||
assert detect_urls("") == []
|
||||
|
||||
Reference in New Issue
Block a user