A deleted tag is a tombstone on every path, and the web skips tombstones
The web deleted a tag's row outright (delete, and merge's source), so the change feed never carried it and linked devices kept the tag. A device's delete left a tombstone that the web still listed, matched by name on create and rename, minted #tags onto, and accepted in a picker. - labeling.tombstone_label is the one way a tag is deleted: drop its links, set purged_at. REST delete, merge and sync's op=delete all use it. - labeling.live(owner) is the one definition of a tag that exists; every catalog read uses it (list, lookup, create/rename matching, #tag minting, picker ids, export, and sync's name-clash check). - 0040: (owner_id, name) is unique among live tags only, so a tombstone gives its name back and #grocery can be made again, on the web or from a device. Fixes #5382. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,41 @@
|
||||
"""labels: a tag's name is unique among LIVE tags only
|
||||
|
||||
Revision ID: 0040
|
||||
Revises: 0039
|
||||
Create Date: 2026-10-08
|
||||
|
||||
Deleting a tag now leaves a tombstone row (`purged_at` set) so the change feed can
|
||||
tell linked devices about it (#5382). Before, the web deleted the row outright and
|
||||
devices never heard. A tombstone must not hold its name: creating `#grocery` again
|
||||
after deleting it has to make a live tag, on the web and from a device alike. So
|
||||
`(owner_id, name)` stays unique only where `purged_at IS NULL`.
|
||||
|
||||
## Downgrade
|
||||
|
||||
Restores the plain unique constraint. Tombstones are deleted first, since one may
|
||||
share its name with a live tag; devices that have not pulled since keep the tag.
|
||||
"""
|
||||
import sqlalchemy as sa
|
||||
from alembic import op
|
||||
|
||||
revision = "0040"
|
||||
down_revision = "0039"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.drop_constraint("uq_labels_owner_name", "labels", type_="unique")
|
||||
op.create_index(
|
||||
"uq_labels_owner_name_live",
|
||||
"labels",
|
||||
["owner_id", "name"],
|
||||
unique=True,
|
||||
postgresql_where=sa.text("purged_at IS NULL"),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_index("uq_labels_owner_name_live", table_name="labels")
|
||||
op.execute("DELETE FROM labels WHERE purged_at IS NOT NULL")
|
||||
op.create_unique_constraint("uq_labels_owner_name", "labels", ["owner_id", "name"])
|
||||
+31
-6
@@ -2,24 +2,49 @@
|
||||
labels, leave the tag-sourced ones alone" logic was duplicated line-for-line between
|
||||
the labels-picker API (notes.set_note_labels) and sync push (sync._apply_note_manual_labels).
|
||||
Single home so both stay in lockstep. via_tag=True rows track the body #tags and are
|
||||
governed by _lift_and_reconcile_tags — this function never touches them."""
|
||||
governed by _lift_and_reconcile_tags — this function never touches them.
|
||||
|
||||
Also the one definition of a LIVE tag and of deleting one (#5382)."""
|
||||
from __future__ import annotations
|
||||
|
||||
from datetime import datetime, timezone
|
||||
|
||||
from sqlalchemy import delete as sa_delete
|
||||
from sqlalchemy import select
|
||||
|
||||
from .models.label import Label, NoteLabel
|
||||
from .models.note import Note
|
||||
|
||||
|
||||
def live(owner_id):
|
||||
"""`owner_id`'s tags that exist: not tombstoned. Every read of the catalog goes
|
||||
through this. A tombstone is kept only for the change feed, and one that leaked
|
||||
into a listing, a name match or a picker would bring a deleted tag back."""
|
||||
return (Label.owner_id == owner_id) & Label.purged_at.is_(None)
|
||||
|
||||
|
||||
async def tombstone_label(db, label: Label, edited_at: datetime | None = None) -> None:
|
||||
"""Delete a tag: detach it from every note and mark the row purged.
|
||||
|
||||
The ONE way a tag is deleted, from the web (delete, merge) and from a device's
|
||||
push alike. The row stays, with a new `sync_revision`, so the change feed tells
|
||||
every linked device. A hard delete sends nothing, and the devices keep the tag
|
||||
forever. Dropping the links bumps each note too (the 0015 parent trigger).
|
||||
`edited_at` is a device's edit time, kept as the row's LWW clock."""
|
||||
await db.execute(sa_delete(NoteLabel).where(NoteLabel.label_id == label.id))
|
||||
label.purged_at = datetime.now(timezone.utc)
|
||||
if edited_at is not None:
|
||||
label.updated_at = edited_at
|
||||
await db.flush()
|
||||
|
||||
|
||||
async def resolve_owned_label_ids(db, label_ids, owner_id) -> set:
|
||||
"""Of `label_ids` (an iterable of UUIDs), the subset actually owned by `owner_id`.
|
||||
Callers parse/validate the raw ids first; this just enforces ownership."""
|
||||
"""Of `label_ids` (an iterable of UUIDs), the subset actually owned by `owner_id`
|
||||
and live. Callers parse/validate the raw ids first; this just enforces ownership."""
|
||||
ids = list(label_ids)
|
||||
if not ids:
|
||||
return set()
|
||||
return set(
|
||||
(await db.scalars(select(Label.id).where(Label.owner_id == owner_id, Label.id.in_(ids)))).all()
|
||||
)
|
||||
return set((await db.scalars(select(Label.id).where(live(owner_id), Label.id.in_(ids)))).all())
|
||||
|
||||
|
||||
async def reconcile_manual_labels(db, note: Note, owned_label_ids: set) -> None:
|
||||
|
||||
@@ -6,6 +6,7 @@ from sqlalchemy import func, select
|
||||
from .auth import login_required
|
||||
from .colors import normalize_color
|
||||
from .db import session_scope
|
||||
from .labeling import live, tombstone_label
|
||||
from .models.label import Label, NoteLabel
|
||||
from .responses import json_error, not_found, parse_uuid
|
||||
from .serialize import serialize_label
|
||||
@@ -28,7 +29,7 @@ async def _label_note_count(db, label_id) -> int:
|
||||
|
||||
|
||||
async def _merge_into(db, source: Label, target: Label) -> None:
|
||||
"""Move every note tagged with `source` onto `target`, then delete `source`.
|
||||
"""Move every note tagged with `source` onto `target`, then tombstone `source`.
|
||||
|
||||
Shared by the explicit `/merge` route and by a rename that lands on a name
|
||||
another tag already holds — those are the same operation, and having one body
|
||||
@@ -53,22 +54,21 @@ async def _merge_into(db, source: Label, target: Label) -> None:
|
||||
for note_id, via_tag in by_note.items():
|
||||
if note_id not in target_notes:
|
||||
db.add(NoteLabel(note_id=note_id, label_id=target.id, via_tag=via_tag))
|
||||
await db.delete(source)
|
||||
await db.flush()
|
||||
await tombstone_label(db, source)
|
||||
|
||||
|
||||
async def _get_owned_label(db, label_id: str) -> Label | None:
|
||||
lid = parse_uuid(label_id)
|
||||
if lid is None:
|
||||
return None
|
||||
return await db.scalar(select(Label).where(Label.id == lid, Label.owner_id == g.user_id))
|
||||
return await db.scalar(select(Label).where(Label.id == lid, live(g.user_id)))
|
||||
|
||||
|
||||
@bp.get("")
|
||||
@login_required
|
||||
async def list_labels():
|
||||
async with session_scope() as db:
|
||||
labels = (await db.scalars(select(Label).where(Label.owner_id == g.user_id).order_by(Label.name))).all()
|
||||
labels = (await db.scalars(select(Label).where(live(g.user_id)).order_by(Label.name))).all()
|
||||
# One grouped query for all usage counts (0 for labels attached to nothing).
|
||||
counts = dict(
|
||||
(
|
||||
@@ -95,7 +95,7 @@ async def create_label():
|
||||
# the path that MINTED the "Groceries" beside "groceries" pair that no synced
|
||||
# client can hold, since their `labels` index is unique on `lower(name)`.
|
||||
existing = await db.scalar(
|
||||
select(Label).where(Label.owner_id == g.user_id, func.lower(Label.name) == name.lower())
|
||||
select(Label).where(live(g.user_id), func.lower(Label.name) == name.lower())
|
||||
)
|
||||
if existing is not None:
|
||||
return jsonify(_serialize_label(existing)), 200
|
||||
@@ -129,7 +129,7 @@ async def update_label(label_id: str):
|
||||
# one be made is letting a pull fail later on a phone.
|
||||
clash = await db.scalar(
|
||||
select(Label).where(
|
||||
Label.owner_id == g.user_id,
|
||||
live(g.user_id),
|
||||
func.lower(Label.name) == name.lower(),
|
||||
Label.id != label.id,
|
||||
)
|
||||
@@ -162,7 +162,7 @@ async def delete_label(label_id: str):
|
||||
label = await _get_owned_label(db, label_id)
|
||||
if label is None:
|
||||
return not_found()
|
||||
await db.delete(label) # note_labels rows cascade
|
||||
await tombstone_label(db, label)
|
||||
await db.commit()
|
||||
return jsonify({"ok": True})
|
||||
|
||||
@@ -171,7 +171,7 @@ async def delete_label(label_id: str):
|
||||
@login_required
|
||||
async def merge_label(label_id: str):
|
||||
"""Merge `label_id` (source) INTO the label given by body {"into": <id>}: move
|
||||
every note tagged with the source onto the target, then delete the source. Both
|
||||
every note tagged with the source onto the target, then tombstone the source. Both
|
||||
must be owned by the caller. Note-body `#tags` are NOT rewritten, so a note whose
|
||||
body still literally contains the source #tag will re-mint that label on its next
|
||||
edit — retire a tag by editing it out of the text (a known, documented nuance)."""
|
||||
|
||||
@@ -3,7 +3,7 @@ from __future__ import annotations
|
||||
import uuid
|
||||
from datetime import datetime
|
||||
|
||||
from sqlalchemy import BigInteger, Boolean, DateTime, ForeignKey, Text, UniqueConstraint, func
|
||||
from sqlalchemy import BigInteger, Boolean, DateTime, ForeignKey, Index, Text, func, text
|
||||
from sqlalchemy.dialects.postgresql import UUID
|
||||
from sqlalchemy.orm import Mapped, mapped_column
|
||||
|
||||
@@ -12,7 +12,17 @@ from . import Base
|
||||
|
||||
class Label(Base):
|
||||
__tablename__ = "labels"
|
||||
__table_args__ = (UniqueConstraint("owner_id", "name", name="uq_labels_owner_name"),)
|
||||
# A name is unique among LIVE tags only: a tombstone keeps its row for the change
|
||||
# feed but gives its name back, so `#grocery` can be made again (#5382, 0040).
|
||||
__table_args__ = (
|
||||
Index(
|
||||
"uq_labels_owner_name_live",
|
||||
"owner_id",
|
||||
"name",
|
||||
unique=True,
|
||||
postgresql_where=text("purged_at IS NULL"),
|
||||
),
|
||||
)
|
||||
|
||||
id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4)
|
||||
owner_id: Mapped[uuid.UUID] = mapped_column(
|
||||
@@ -29,7 +39,8 @@ class Label(Base):
|
||||
# Sync (M8): monotonic per-row revision (from sync_revision_seq via DB trigger) so a
|
||||
# label rename/recolor/merge/delete propagates to native clients independently of notes.
|
||||
sync_revision: Mapped[int | None] = mapped_column(BigInteger(), nullable=True)
|
||||
# Hard-delete tombstone (content-less) so a deleted label is removed on clients.
|
||||
# Tombstone: set means deleted. The row stays so the change feed can carry the
|
||||
# delete to devices; every read of the catalog skips it (`labeling.live`).
|
||||
purged_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True), nullable=True)
|
||||
|
||||
|
||||
|
||||
@@ -25,7 +25,7 @@ from ..colors import normalize_color
|
||||
from ..common import coerce_bool, iso, parse_dt
|
||||
from ..config import Config
|
||||
from ..db import session_scope
|
||||
from ..labeling import reconcile_manual_labels, resolve_owned_label_ids
|
||||
from ..labeling import live, reconcile_manual_labels, resolve_owned_label_ids
|
||||
from ..models.label import Label, NoteLabel
|
||||
from ..models.note import Note
|
||||
from ..models.note_attachment import NoteAttachment
|
||||
@@ -245,7 +245,7 @@ async def export_notes():
|
||||
for a in att_rows:
|
||||
att_by_note.setdefault(a.note_id, []).append(a)
|
||||
all_labels = (
|
||||
await db.scalars(select(Label).where(Label.owner_id == g.user_id).order_by(Label.name))
|
||||
await db.scalars(select(Label).where(live(g.user_id)).order_by(Label.name))
|
||||
).all()
|
||||
|
||||
payload: dict = {
|
||||
|
||||
@@ -18,6 +18,7 @@ import re
|
||||
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from ..labeling import live
|
||||
from ..models.label import Label, NoteLabel
|
||||
from ..models.note import Note
|
||||
from .helpers import derive_display_title
|
||||
@@ -120,9 +121,10 @@ def split_body_tags(body: str | None) -> tuple[list[str], list[str], str]:
|
||||
|
||||
|
||||
async def _find_or_create_label(db, owner_id, name: str):
|
||||
"""Owner's label id for `name` (case-insensitive match), creating it if absent."""
|
||||
"""Owner's live label id for `name` (case-insensitive match), creating it if absent.
|
||||
A deleted tag's tombstone never matches: tagging a note with it again makes a new tag."""
|
||||
existing = await db.scalar(
|
||||
select(Label.id).where(Label.owner_id == owner_id, func.lower(Label.name) == name.lower())
|
||||
select(Label.id).where(live(owner_id), func.lower(Label.name) == name.lower())
|
||||
)
|
||||
if existing is not None:
|
||||
return existing
|
||||
|
||||
+5
-10
@@ -25,7 +25,7 @@ from .acl import visible_to_user
|
||||
from .auth import login_required
|
||||
from .common import iso, parse_dt
|
||||
from .db import session_scope
|
||||
from .labeling import reconcile_manual_labels, resolve_owned_label_ids
|
||||
from .labeling import live, reconcile_manual_labels, resolve_owned_label_ids, tombstone_label
|
||||
from .models.label import Label, NoteLabel
|
||||
from .models.note import Note
|
||||
from .models.note_attachment import NoteAttachment
|
||||
@@ -395,11 +395,7 @@ async def _apply_label(db, ch: dict) -> dict:
|
||||
return _result(str(lid), "label", "noop")
|
||||
if not client_wins(edited_at, label.updated_at):
|
||||
return _result(str(lid), "label", "kept", sync_revision=label.sync_revision)
|
||||
await db.execute(sa_delete(NoteLabel).where(NoteLabel.label_id == label.id))
|
||||
label.purged_at = datetime.now(timezone.utc)
|
||||
if edited_at is not None:
|
||||
label.updated_at = edited_at
|
||||
await db.flush()
|
||||
await tombstone_label(db, label, edited_at)
|
||||
await db.refresh(label, ["sync_revision"])
|
||||
return _result(str(lid), "label", "applied", sync_revision=label.sync_revision)
|
||||
|
||||
@@ -407,12 +403,11 @@ async def _apply_label(db, ch: dict) -> dict:
|
||||
creating = label is None
|
||||
if not creating and not client_wins(edited_at, label.updated_at):
|
||||
return _result(str(lid), "label", "kept", sync_revision=label.sync_revision)
|
||||
# Names are unique per owner — a same-name clash on a DIFFERENT id can't be an insert.
|
||||
# Names are unique among an owner's LIVE tags — a same-name clash on a DIFFERENT
|
||||
# id can't be an insert. A tombstone holds no name (0040), so it never clashes.
|
||||
if name:
|
||||
clash = await db.scalar(
|
||||
select(Label.id).where(
|
||||
Label.owner_id == g.user_id, func.lower(Label.name) == name.lower(), Label.id != lid
|
||||
)
|
||||
select(Label.id).where(live(g.user_id), func.lower(Label.name) == name.lower(), Label.id != lid)
|
||||
)
|
||||
if clash is not None:
|
||||
return _result(str(lid), "label", "rejected", error="name in use")
|
||||
|
||||
@@ -646,6 +646,91 @@ async def test_creating_a_tag_that_differs_only_in_case_returns_the_existing_one
|
||||
assert len(listing) == 1
|
||||
|
||||
|
||||
# --- A deleted tag is a tombstone on every path (#5382) ---------------------------
|
||||
#
|
||||
# The web deleted the row outright, so the change feed never carried the delete and
|
||||
# devices kept the tag. A device's delete left a tombstone the web still listed and
|
||||
# still matched by name. One definition now: `labeling.tombstone_label`, and every
|
||||
# read of the catalog goes through `labeling.live`.
|
||||
|
||||
|
||||
async def _label_in_feed(app_client, lid: str) -> dict | None:
|
||||
feed = await (await app_client.get("/api/sync/changes?since=0")).get_json()
|
||||
return next((lb for lb in feed["labels"] if lb["id"] == lid), None)
|
||||
|
||||
|
||||
async def _push_label(app_client, lid: str, op: str, edited_at: str, name: str = "grocery") -> dict:
|
||||
change = {"entity": "label", "id": lid, "op": op, "name": name, "edited_at": edited_at}
|
||||
resp = await app_client.post("/api/sync/push", json={"changes": [change]})
|
||||
return (await resp.get_json())["results"][0]
|
||||
|
||||
|
||||
async def test_a_tag_deleted_on_the_web_reaches_devices_as_a_tombstone(app_client, db):
|
||||
await _signed_in(app_client, "webdelete")
|
||||
tag = await (await app_client.post("/api/labels", json={"name": "grocery"})).get_json()
|
||||
note = await (await app_client.post("/api/notes", json={"body": "milk"})).get_json()
|
||||
await app_client.put(f"/api/notes/{note['id']}/labels", json={"label_ids": [tag["id"]]})
|
||||
|
||||
assert (await app_client.delete(f"/api/labels/{tag['id']}")).status_code == 200
|
||||
|
||||
row = await _label_in_feed(app_client, tag["id"])
|
||||
assert row is not None and row["purged_at"] is not None, "the feed carries the delete"
|
||||
assert (await (await app_client.get(f"/api/notes/{note['id']}")).get_json())["labels"] == []
|
||||
assert (await (await app_client.get("/api/labels")).get_json())["labels"] == []
|
||||
assert (await app_client.delete(f"/api/labels/{tag['id']}")).status_code == 404, "gone, not hidden"
|
||||
|
||||
|
||||
async def test_a_merge_tombstones_the_tag_it_folds_away(app_client, db):
|
||||
await _signed_in(app_client, "mergedelete")
|
||||
source = await (await app_client.post("/api/labels", json={"name": "groceries"})).get_json()
|
||||
target = await (await app_client.post("/api/labels", json={"name": "shopping"})).get_json()
|
||||
|
||||
resp = await app_client.post(f"/api/labels/{source['id']}/merge", json={"into": target["id"]})
|
||||
assert resp.status_code == 200
|
||||
|
||||
row = await _label_in_feed(app_client, source["id"])
|
||||
assert row is not None and row["purged_at"] is not None
|
||||
listing = (await (await app_client.get("/api/labels")).get_json())["labels"]
|
||||
assert [lb["id"] for lb in listing] == [target["id"]]
|
||||
|
||||
|
||||
async def test_a_tag_deleted_on_a_device_is_gone_from_the_web_and_its_name_is_free(app_client, db):
|
||||
await _signed_in(app_client, "devicedelete")
|
||||
lid = str(uuid.uuid4())
|
||||
assert (await _push_label(app_client, lid, "upsert", "2026-01-01T00:00:00Z"))["status"] == "created"
|
||||
assert (await _push_label(app_client, lid, "delete", "2026-01-02T00:00:00Z"))["status"] == "applied"
|
||||
|
||||
assert (await (await app_client.get("/api/labels")).get_json())["labels"] == []
|
||||
|
||||
# The web makes the name again: a NEW live tag, not the tombstone handed back.
|
||||
again = await app_client.post("/api/labels", json={"name": "Grocery"})
|
||||
assert again.status_code == 201
|
||||
assert (await again.get_json())["id"] != lid
|
||||
|
||||
# A body #tag finds that live one, not the tombstone.
|
||||
note = await (await app_client.post("/api/notes", json={"body": "milk #grocery"})).get_json()
|
||||
note = await (await app_client.get(f"/api/notes/{note['id']}")).get_json()
|
||||
assert [lb["id"] for lb in note["labels"]] == [(await again.get_json())["id"]]
|
||||
|
||||
# A picker naming the tombstone attaches nothing.
|
||||
await app_client.put(f"/api/notes/{note['id']}/labels", json={"label_ids": [lid]})
|
||||
labels = (await (await app_client.get(f"/api/notes/{note['id']}")).get_json())["labels"]
|
||||
assert lid not in [lb["id"] for lb in labels]
|
||||
|
||||
|
||||
async def test_a_device_can_make_a_deleted_tags_name_again(app_client, db):
|
||||
"""The tombstone gives its name back (0040), so another device's fresh tag of
|
||||
the same name is created rather than refused as "name in use"."""
|
||||
await _signed_in(app_client, "devicerename")
|
||||
old, new = str(uuid.uuid4()), str(uuid.uuid4())
|
||||
await _push_label(app_client, old, "upsert", "2026-01-01T00:00:00Z")
|
||||
await _push_label(app_client, old, "delete", "2026-01-02T00:00:00Z")
|
||||
|
||||
assert (await _push_label(app_client, new, "upsert", "2026-01-03T00:00:00Z"))["status"] == "created"
|
||||
listing = (await (await app_client.get("/api/labels")).get_json())["labels"]
|
||||
assert [lb["id"] for lb in listing] == [new]
|
||||
|
||||
|
||||
# --- One save path for a note's text (#5164) ---------------------------------
|
||||
#
|
||||
# A body edit is a sequence: keep a revision, rename the note, lift #tags, commit,
|
||||
|
||||
Reference in New Issue
Block a user