server: one save path for a note's text, so a restored link gets its preview
CI & Build / Build now, or wait for Android? (push) Successful in 3s
Android / Build, or is the channel already serving this? (push) Successful in 3s
Android / Kotlin + Rust (APK) (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 11s
Desktop (Tauri) / Build, or is the channel already serving this? (push) Successful in 3s
Desktop (Tauri) / Clippy, tests and rustfmt (push) Skipped
Desktop (Tauri) / Tauri desktop (Linux) (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Skipped
Desktop (Tauri) / Update manifest (push) Skipped
CI & Build / Python tests (push) Successful in 16s
CI & Build / integration (push) Successful in 41s
CI & Build / Build & push image (push) Successful in 46s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
Android / Build, or is the channel already serving this? (push) Successful in 3s
Android / Kotlin + Rust (APK) (push) Skipped
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 11s
Desktop (Tauri) / Build, or is the channel already serving this? (push) Successful in 3s
Desktop (Tauri) / Clippy, tests and rustfmt (push) Skipped
Desktop (Tauri) / Tauri desktop (Linux) (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Skipped
Desktop (Tauri) / Update manifest (push) Skipped
CI & Build / Python tests (push) Successful in 16s
CI & Build / integration (push) Successful in 41s
CI & Build / Build & push image (push) Successful in 46s
A body edit is a sequence: keep a revision, rename the note, lift #tags,
commit, queue link previews. It was written out in PATCH, the item routes,
restore and sync push, and the copies had drifted. Now they all call
`notes/body.py: write_body`, which says how the old text is kept ("session",
"always" for restore, "never" for a new note) and returns whether the text
changed. The routes commit through `_commit_note`, which queues previews after
the commit.
Fixes, both red on 1a2f71e (run 8441):
- restoring a revision queues previews for its links (#5164, audit B5);
- a pushed note keeps the client's edit time when a standalone #tag is lifted.
The lift's extra flush used to let `onupdate` stamp the server clock over it.
Sync push also queues its previews after the batch commits, not mid-batch,
where a fast fetch could look for a note that wasn't committed yet.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -33,7 +33,6 @@ from ..models.note import Note
|
||||
from ..models.note_attachment import NoteAttachment
|
||||
from ..models.note_link_preview import NoteLinkPreview
|
||||
from ..models.note_revision import NoteRevision
|
||||
from ..revisions import should_snapshot
|
||||
from .checklist import (
|
||||
append_item,
|
||||
parse_items,
|
||||
@@ -47,6 +46,7 @@ from ..settings import get_setting
|
||||
from ..unfurl_queue import schedule as schedule_unfurls
|
||||
from ..unfurl import UnfurlError, unfurl
|
||||
from ._bp import bp
|
||||
from .body import write_body
|
||||
from .helpers import (
|
||||
ALLOWED_IMAGE_MIMES,
|
||||
VALID_FILTERS,
|
||||
@@ -402,22 +402,11 @@ async def create_note():
|
||||
# are folded into the body, which is where a checklist lives now (M304).
|
||||
for text in item_texts:
|
||||
body = append_item(body, text)
|
||||
note = Note(
|
||||
owner_id=g.user_id,
|
||||
display_title=derive_display_title(body),
|
||||
body=body,
|
||||
position=int(max_pos) + 1,
|
||||
)
|
||||
note = Note(owner_id=g.user_id, body="", display_title="", position=int(max_pos) + 1)
|
||||
db.add(note)
|
||||
await db.flush() # assign note.id before writing links
|
||||
# The FOLDED body: an item can carry a #tag too.
|
||||
await _lift_and_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
|
||||
changed = await write_body(db, note, body, snapshot="never")
|
||||
return await _commit_note(db, note, changed), 201
|
||||
|
||||
|
||||
@bp.get("/<note_id>")
|
||||
@@ -443,9 +432,12 @@ async def update_note(note_id: str):
|
||||
note = await _get_owned(db, note_id)
|
||||
if note is None:
|
||||
return not_found()
|
||||
old_body = note.body
|
||||
if "body" in data and isinstance(data["body"], str):
|
||||
note.body = data["body"]
|
||||
changed = False
|
||||
if isinstance(data.get("body"), str):
|
||||
# Version history: the first write of an editing session snapshots, not
|
||||
# every write — see revisions.should_snapshot. Writing often is what lets
|
||||
# a client autosave instead of hoarding text until it closes.
|
||||
changed = await write_body(db, note, data["body"])
|
||||
if "pinned" in data:
|
||||
note.pinned = bool(data["pinned"])
|
||||
if "archived" in data:
|
||||
@@ -462,19 +454,21 @@ async def update_note(note_id: str):
|
||||
note.remind_at = remind_dt
|
||||
if "recurrence" in data:
|
||||
note.recurrence = normalize_recurrence(data["recurrence"])
|
||||
if "body" in data:
|
||||
note.display_title = derive_display_title(note.body)
|
||||
await _lift_and_reconcile_tags(db, note)
|
||||
# Version history: snapshot the PRE-edit body, once per editing session
|
||||
# rather than once per write — see revisions.should_snapshot. Writing often
|
||||
# is what lets a client autosave instead of hoarding text until it closes.
|
||||
if await should_snapshot(db, note.id, old_body, note.body):
|
||||
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))
|
||||
return await _commit_note(db, note, changed)
|
||||
|
||||
|
||||
async def _commit_note(db, note: Note, text_changed: bool):
|
||||
"""Commit a note write and answer with the note.
|
||||
|
||||
Link previews are queued here, AFTER the commit and never before it: the fetch
|
||||
reads the note in a session of its own, so a note it cannot see yet gets no
|
||||
preview. `text_changed` is what `write_body` returned.
|
||||
"""
|
||||
await db.commit()
|
||||
await db.refresh(note)
|
||||
if text_changed:
|
||||
schedule_unfurls(note.id, note.body)
|
||||
return jsonify(await _serialize_note(db, note))
|
||||
|
||||
|
||||
def _serialize_revision(rev: NoteRevision) -> dict:
|
||||
@@ -518,15 +512,11 @@ async def restore_revision(note_id: str, rev_id: str):
|
||||
return not_found()
|
||||
if note.body == rev.body:
|
||||
return jsonify(await _serialize_note(db, note)) # already at this version — no-op
|
||||
# Snapshot the CURRENT state first, so restoring is itself undoable, then apply
|
||||
# the revision — with the same body ripple as a normal edit.
|
||||
db.add(NoteRevision(note_id=note.id, body=note.body))
|
||||
note.body = rev.body
|
||||
note.display_title = derive_display_title(note.body)
|
||||
await _lift_and_reconcile_tags(db, note)
|
||||
await db.commit()
|
||||
await db.refresh(note)
|
||||
return jsonify(await _serialize_note(db, note))
|
||||
# Snapshot the CURRENT text unconditionally, so restoring is itself undoable,
|
||||
# then apply the revision through the same path as any edit — which is what
|
||||
# gives a restored link its preview back.
|
||||
changed = await write_body(db, note, rev.body, snapshot="always")
|
||||
return await _commit_note(db, note, changed)
|
||||
|
||||
|
||||
@bp.put("/<note_id>/labels")
|
||||
@@ -565,24 +555,8 @@ def _item_index(item_id: str) -> int | None:
|
||||
|
||||
|
||||
async def _rewrite_body(db, note: Note, body: str):
|
||||
"""Every item mutation is a body edit, so all of them land here.
|
||||
|
||||
One place means one place that snapshots a revision, re-derives `#tags`, recomputes
|
||||
the name and queues link unfurls — rather than three routes each remembering to.
|
||||
Deliberately the same sequence the PATCH route runs for a body change, because it
|
||||
IS a body change.
|
||||
"""
|
||||
old_body = note.body
|
||||
if await should_snapshot(db, note.id, old_body, body):
|
||||
db.add(NoteRevision(note_id=note.id, body=old_body))
|
||||
note.body = body
|
||||
note.display_title = derive_display_title(body)
|
||||
await _lift_and_reconcile_tags(db, note)
|
||||
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))
|
||||
"""Every item mutation is a body edit, so it is saved exactly like one."""
|
||||
return await _commit_note(db, note, await write_body(db, note, body))
|
||||
|
||||
|
||||
@bp.post("/<note_id>/items")
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
"""Writing a note's text — the one path every surface goes through.
|
||||
|
||||
A body edit is never only the body. It may keep a revision of the old text, it
|
||||
renames the note, it lifts and reconciles `#tags`, and once committed it queues link
|
||||
previews. That sequence used to be written out at each place a body could change —
|
||||
PATCH, the item routes, restoring a revision and sync push — and the copies drifted:
|
||||
restoring a revision never queued previews, so a restored link stayed a bare URL
|
||||
(#5164). Now a caller states how the old text should be kept and this does the rest.
|
||||
|
||||
Previews are the one part that cannot happen here. They are queued AFTER the commit
|
||||
(`unfurl_queue.schedule`), because the fetch reads the note in a session of its own
|
||||
and a note it cannot see yet gets no preview. So this returns whether the text
|
||||
changed, and the caller queues them once its transaction is committed.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Literal
|
||||
|
||||
from ..models.note import Note
|
||||
from ..models.note_revision import NoteRevision
|
||||
from ..revisions import should_snapshot
|
||||
from .helpers import derive_display_title
|
||||
from .tags import _lift_and_reconcile_tags
|
||||
|
||||
# How the text being replaced is kept:
|
||||
# "session" the first write of an editing session snapshots (revisions.should_snapshot)
|
||||
# "always" snapshot unconditionally — restoring a revision must itself be undoable
|
||||
# "never" a note being created has no earlier text to keep
|
||||
Snapshot = Literal["session", "always", "never"]
|
||||
|
||||
|
||||
async def write_body(db, note: Note, body: str, *, snapshot: Snapshot = "session") -> bool:
|
||||
"""Replace `note`'s text and everything derived from it. Returns whether the text
|
||||
changed, which is what decides whether the caller queues previews after commit.
|
||||
|
||||
Compared against the text AFTER tags are lifted, not the text passed in: a body
|
||||
that only gained a standalone `#tag` ends up as it was, with a label attached.
|
||||
"""
|
||||
old_body = note.body or ""
|
||||
if snapshot == "always" or (snapshot == "session" and await should_snapshot(db, note.id, old_body, body)):
|
||||
db.add(NoteRevision(note_id=note.id, body=old_body))
|
||||
note.body = body
|
||||
note.display_title = derive_display_title(body)
|
||||
await db.flush() # a new note needs its id before its tag labels can point at it
|
||||
await _lift_and_reconcile_tags(db, note)
|
||||
return note.body != old_body
|
||||
+27
-25
@@ -24,15 +24,12 @@ from .db import session_scope
|
||||
from .labeling import reconcile_manual_labels, resolve_owned_label_ids
|
||||
from .models.label import Label, NoteLabel
|
||||
from .models.note import Note
|
||||
from .models.note_revision import NoteRevision
|
||||
from .revisions import should_snapshot
|
||||
from .notes import (
|
||||
_lift_and_reconcile_tags,
|
||||
_serialize_notes,
|
||||
derive_display_title,
|
||||
normalize_color,
|
||||
normalize_recurrence,
|
||||
)
|
||||
from .notes.body import write_body
|
||||
from .retention import purge_note
|
||||
from .serialize import serialize_label_sync
|
||||
from .unfurl_queue import schedule as schedule_unfurls
|
||||
@@ -205,8 +202,8 @@ def client_wins(client_edited_at: datetime | None, server_edited_at: datetime |
|
||||
|
||||
def _assign_note_fields(note: Note, ch: dict) -> None:
|
||||
"""Overwrite a note's scalar fields from a client's FULL-state change (sync is
|
||||
whole-note, not a partial patch — the client sends its authoritative version)."""
|
||||
note.body = ch["body"] if isinstance(ch.get("body"), str) else ""
|
||||
whole-note, not a partial patch — the client sends its authoritative version).
|
||||
The body is not one of them: it goes through `write_body`, like every edit."""
|
||||
note.pinned = bool(ch.get("pinned"))
|
||||
note.archived = bool(ch.get("archived"))
|
||||
if ch.get("trashed"):
|
||||
@@ -244,7 +241,9 @@ async def _apply_note_manual_labels(db, note: Note, ch: dict) -> None:
|
||||
await reconcile_manual_labels(db, note, owned)
|
||||
|
||||
|
||||
async def _apply_note(db, ch: dict) -> dict:
|
||||
async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> dict:
|
||||
"""Apply one pushed note. A note whose text changed is appended to `previews` as
|
||||
(id, final body), for the caller to queue link previews once the batch commits."""
|
||||
raw_id = ch.get("id")
|
||||
try:
|
||||
nid = uuid.UUID(str(raw_id))
|
||||
@@ -283,29 +282,25 @@ async def _apply_note(db, ch: dict) -> dict:
|
||||
elif note.purged_at is not None:
|
||||
note.purged_at = None # client re-created/edited → clear the tombstone
|
||||
|
||||
old_body = note.body
|
||||
_assign_note_fields(note, ch)
|
||||
note.display_title = derive_display_title(note.body)
|
||||
# Non-destructive LWW: the overwritten server body snapshots into history, subject
|
||||
# to the same session window as a direct edit (revisions.should_snapshot). This
|
||||
# path is why the window is a time rule rather than a flag on the wire: a client
|
||||
# autosaving every second pushes a body change every second, and without the
|
||||
# check the SERVER would snapshot each one no matter how restrained the client's
|
||||
# own store was being.
|
||||
body = ch["body"] if isinstance(ch.get("body"), str) else ""
|
||||
if await write_body(db, note, body, snapshot="never" if creating else "session"):
|
||||
# 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.
|
||||
previews.append((note.id, note.body))
|
||||
# AFTER write_body, whose flush lets the column's onupdate stamp the server's
|
||||
# clock. Set here, the client's edit time is what the next flush writes.
|
||||
if edited_at is not None:
|
||||
note.updated_at = edited_at
|
||||
# Non-destructive LWW: snapshot the overwritten server body into history —
|
||||
# subject to the same session window as a direct edit (revisions.should_snapshot).
|
||||
# This path is why the window is a time rule rather than a flag on the wire: a
|
||||
# client autosaving every second pushes a body change every second, and without
|
||||
# the check the SERVER would snapshot each one no matter how restrained the
|
||||
# client's own store was being.
|
||||
if not creating and await should_snapshot(db, note.id, old_body, note.body):
|
||||
db.add(NoteRevision(note_id=note.id, body=old_body))
|
||||
await db.flush() # assign note.id before items/labels/links
|
||||
await _lift_and_reconcile_tags(db, note)
|
||||
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",
|
||||
@@ -391,6 +386,7 @@ async def push():
|
||||
return jsonify({"error": f"too many changes in one push (max {MAX_PUSH})"}), 400
|
||||
|
||||
results = []
|
||||
previews: list[tuple[uuid.UUID, str]] = []
|
||||
async with session_scope() as db:
|
||||
for ch in changes:
|
||||
if not isinstance(ch, dict):
|
||||
@@ -398,10 +394,16 @@ async def push():
|
||||
continue
|
||||
entity = ch.get("entity")
|
||||
if entity == "note":
|
||||
results.append(await _apply_note(db, ch))
|
||||
results.append(await _apply_note(db, ch, previews))
|
||||
elif entity == "label":
|
||||
results.append(await _apply_label(db, ch))
|
||||
else:
|
||||
results.append({"id": ch.get("id"), "status": "rejected", "error": "unknown entity"})
|
||||
await db.commit()
|
||||
# After the batch commits, never inside it: the fetch reads each note in a
|
||||
# session of its own, and these were scheduled mid-batch until #5164, where a
|
||||
# fast fetch could look for a note that was not committed yet. Scheduled rather
|
||||
# than awaited — a push must not wait on somebody else's website.
|
||||
for note_id, text in previews:
|
||||
schedule_unfurls(note_id, text)
|
||||
return jsonify({"results": results})
|
||||
|
||||
Reference in New Issue
Block a user