Skip to content

A client reads a document back by the external_id it pushed it under - #1280

Merged
WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/read-a-document-by-its-external-id
Oct 21, 2026
Merged

WaylandYang merged 2 commits into
deeplethe:devfrom
gedaiu:feat/read-a-document-by-its-external-id

Conversation

@gedaiu

@gedaiu gedaiu commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Part of #1276, in the shape agreed there. Related: #1268 / #1275 (a declared external_id stays the document's identity), #1262 and #1264 (token reads).

What happened

A client that pushes to an api or statements source names each document by its own external_id, but could only read it back by Utopia's document id, which it never kept. So it could check what landed only by searching for its own text.

What changed

GET /kbs/{id}/sources/{source_id}/documents/by-external-id?external_id=… returns the document's metadata, its current version, its text as content, and versions (the list /versions gives; older text through GET /documents/{id}/content?version=N). The id is a query parameter because ids can contain /.

  • Credentials as for document content (0052): Viewer, with a session or any personal token whose kb_ids reach the base, through require_reader_kb. The source's push key is not accepted, so a leaked key stays push-only.
  • The source must be in the base (source_in_kb, now pub(crate)) and be api or statements; otherwise 404.
  • The key is built by one helper, pushed_key, which the API push, the statements push and this read all call, so the three cannot drift apart.
  • The content route's lock-then-read-the-blob body becomes read_retained, which both routes call. It takes the caller's conditions on the row, so the source, key, trash and tombstone checks are judged on the row it locks, not on the one read before the lock. A document in the trash, a tombstoned id and one never pushed are all 404; purge clears external_key, so a purged id is 404 as well.

Docs: web/src/docs/ingest.md gains "Reading a pushed document back". 0052 gets a dated revision note instead of an edit to its decision. In 0014 the route is a short paragraph inside decision 7, next to the six graph reads, and decision 7's revision line now reads "2026-10-19: the graph overview and neighbourhood join them, and so does reading a pushed document back by its external_id (#1276)." access-and-audit.md and the SessionOrToken comment in auth.rs name the route.

Tests

In api::sources_statements_tests, on its existing fixture:

  • a read token reads an id pushed twice back with its text, version 2 and versions [1, 2], and a token limited to another base gets 404;
  • a tombstoned id and a document in the trash are both 404;
  • an id tombstoned after the lookup but before the read takes its row lock is 404, which shows the check is made on the locked row.

Rebased on dev at fa6cd6cc. Run against Postgres with UTOPIA_TEST_REQUIRE_DB=1: cargo fmt --all -- --check and cargo clippy -p utopia-server --all-targets -- -D warnings are clean, and cargo test -p utopia-server gives 688 passed, 0 failed, 2 ignored (the two live-network tests).

🤖 Generated with Claude Code

@gedaiu
gedaiu marked this pull request as draft October 18, 2026 06:34
@gedaiu

gedaiu commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Moving this to draft until the endpoint is agreed in #1276, since it adds a public read. When I pick it up again I'll also stop editing 0052's decision in place and add a dated revision note instead, and line its 0014 and auth.rs wording up with #1263. Sorry for opening it early.

@gedaiu
gedaiu force-pushed the feat/read-a-document-by-its-external-id branch from 58c0385 to 5fd5fbc Compare October 19, 2026 18:53
@gedaiu
gedaiu marked this pull request as ready for review October 19, 2026 18:53
@gedaiu

gedaiu commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Thank you, WaylandYang, for the reviews and merges of #1264, #1318, #1319 and #1320, and for the kind words on them. Thank you too for agreeing the shape on #1276, which made this rework simple to scope. I've rebased this onto dev (fa6cd6cc), and it's still one commit. Here is how it covers your three points:

  • One key helper: pushed_key in sources_routes.rs builds the api:… / statements:… key. The API push, the statements push and this read all call it, so nothing else builds the key by hand.
  • Checks on the locked row: the content route's lock-and-read body is now read_retained, which takes the caller's conditions on the row. The source, key, trash and tombstone checks run on the row it locks with FOR NO KEY UPDATE, not on the row found before the lock. A new test tombstones the id between the lookup and the read and gets 404.
  • Docs: the route is a short paragraph inside 0014's decision 7, after the six graph reads that A token reads a base's graph overview and neighbourhood #1264 brought in. It sits apart from them because it is a document-content read, not a graph read. Decision 7's revision line is now a single sentence: "2026-10-19: the graph overview and neighbourhood join them, and so does reading a pushed document back by its external_id (A source's documents cannot be read back by the external_id they were pushed under #1276)." 0052 gets a dated revision note, and its decision text is unchanged. access-and-audit.md and the SessionOrToken comment in auth.rs keep A token reads a base's graph overview and neighbourhood #1264's wording and add this route.

cargo fmt and clippy -D warnings are clean, and cargo test -p utopia-server passes against Postgres (688 passed, 2 live-network tests ignored). Ready for a look whenever you have a moment. Thank you again.

@gedaiu

gedaiu commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

Hi @WaylandYang, a gentle ping whenever you have a moment, with no rush at all. This one is rebased on today's dev with #1264 in, and CI is green. I'm holding #1265, #1266 and the #1271 search rework until it lands, so that only one open PR touches 0014's decision 7 at a time, as you suggested. Thank you so much for all the careful reviews today. It's been a real pleasure working through these with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants