Skip to content

feat(api): serve retained document content and versions - #860

Open
iuiu-py wants to merge 1 commit into
deeplethe:devfrom
iuiu-py:feat/document-content-api
Open

iuiu-py wants to merge 1 commit into
deeplethe:devfrom
iuiu-py:feat/document-content-api

Conversation

@iuiu-py

@iuiu-py iuiu-py commented Sep 21, 2026

Copy link
Copy Markdown

What

Adds a versioned read contract for retained originals:

  • GET /api/v1/documents/{id}/content serves the current or requested ?version=N bytes from the document ledger.
  • GET /api/v1/documents/{id}/versions lists version, sha256, size_bytes, and ingested_at.
  • Purged documents answer 410 Gone; soft-deleted retained documents remain readable until purge.
  • Responses carry the ledger MIME type, exact byte length, a quoted SHA-256 ETag, and RFC 5987-safe Content-Disposition.
  • Authentication accepts browser sessions and Viewer-or-higher utp_pat_ personal access tokens scoped to the knowledge base; source ingest tokens are rejected.

The content transaction takes SELECT ... FOR NO KEY UPDATE and holds it through the blob read, so replacement or purge cannot move or remove the selected blob after the ledger check. Commit happens after the blob read is complete. A ledger entry whose blob is unavailable returns 500 as an invariant failure rather than masking it as a normal client error.

ADR 0052 records the API/lifecycle boundary and why the ledger is the source of truth.

Why

The document API currently exposes metadata while versions and original bytes stay internal. This makes auditable retention observable to clients, lets callers retrieve the exact recorded original, and closes #859.

Closes #859

Testing

Against Postgres 17 with UTOPIA_TEST_REQUIRE_DB=1 and UTOPIA_DATABASE_URL set:

cargo fmt --all
cargo clippy --locked --workspace --all-targets -- -D warnings
cargo test --locked -p utopia-server 'api::documents_routes::tests' -- --nocapture

Results:

  • cargo fmt completed without changes.
  • cargo clippy passed with warnings denied.
  • 10/10 document-route tests passed, including version selection, deleted-versus-purged lifecycle, knowledge-base/PAT authorization, source-token rejection, transactional race protection, and missing-blob behavior.

Signed-off-by: wangzifei <wangzifei@cit.group.hk>
@WaylandYang

Copy link
Copy Markdown
Contributor

Read it end to end. CI approved — fork PRs need that here after every push.

This is careful work, and the parts I went looking for trouble in are the parts you got right.

Authentication. A new route that serves raw bytes is where I expected to find the hole. Instead there are two independent checks: access::require_kb(..., Role::Viewer) for the person, and pat.covers(kb_id) for the token, so a valid token for base A cannot read base B even if its owner is a member of both. Building a separate extractor rather than reusing AuthUser is the right call for the reason your comment gives — AuthUser would interpret any bearer string as a session and there would be nowhere to enforce scope.

Ingest tokens are rejected structurally rather than by a check that could be forgotten: utp_ tokens are Uuid::simple() hex, and pat_ is not hex, so an ingest token cannot be mistaken for a PAT and falls through to session decoding, which fails. That holds without anyone remembering to maintain it.

A scoped token gets the same 404 as an inaccessible document. Worth having written down in the record, because the tempting 403 leaks which document ids exist.

The transaction commits before the response is built. tx.commit() precedes (StatusCode::OK, headers, bytes).into_response(), so a slow client does not hold a row lock or a pool connection for the length of its download. That is the failure mode I went looking for after reviewing #833, and it is already avoided here.

version.size_bytes != bytes.len() is checked against the ledger and raised as an invariant failure rather than dressed up as a client error. Right instinct — a blob that disagrees with its ledger entry is not a 404.

Two things.

1. docs/decisions/README.md has no row for 0052. The record is added but appears in neither index table. Every other record this week (0048 through 0051) carries rows in both. 0052 is the correct next number — 0049 belongs to #839, which is open — so only the index rows are missing.

2. One sentence missing from the Limits section, which is otherwise the right section. You say the route "buffers within the existing upload cap", and MAX_UPLOAD_BYTES is 100 MB, so a single response is bounded. What is not bounded is how many of those happen at once: N concurrent downloads of large retained originals is N × up to 100 MB resident, and nothing in the route or the record caps N. Excluding Range requests and streaming is a defensible scope decision and I am not asking you to reopen it — but the concurrency multiplier is the thing whoever first serves a base full of large scans will meet, and the record is already the right place to say it.

Neither blocks. Once CI reports, and with the index rows added, this looks good to me.

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.

No route serves a document's original bytes — the export's digests are unverifiable

2 participants