From b14270436c49e00375d3d0e28fa0e2a615f1dd0f Mon Sep 17 00:00:00 2001 From: dawn Date: Wed, 14 Oct 2026 04:30:36 +0300 Subject: [PATCH] knot2: name the change in write answers and list cob versions newest first Signed-off-by: dawn --- api/tangled/repolistCobVersions.go | 12 +- docs/src/content/docs/reference/xrpc.md | 2 +- knot2/crates/knot-xrpc/src/social.rs | 70 +++++++++--- knot2/crates/knot-xrpc/src/tests.rs | 108 ++++++++++++++++++ lexicons/repo/listCobVersions.json | 14 ++- .../types/sh/tangled/repo/listCobVersions.ts | 14 ++- 6 files changed, 199 insertions(+), 21 deletions(-) diff --git a/api/tangled/repolistCobVersions.go b/api/tangled/repolistCobVersions.go index 2b0a337d0..3d3c8f282 100644 --- a/api/tangled/repolistCobVersions.go +++ b/api/tangled/repolistCobVersions.go @@ -16,8 +16,10 @@ const ( // RepoListCobVersions_Output is the output of a sh.tangled.repo.listCobVersions call. type RepoListCobVersions_Output struct { - // cursor: Left out when the page already reaches the newest version. - Cursor *string `json:"cursor,omitempty" cborgen:"cursor,omitempty"` + // cursor: Left out when the page already reaches the last version in the requested order. + Cursor *string `json:"cursor,omitempty" cborgen:"cursor,omitempty"` + // total: How many versions the record has, whatever the page. + Total *int64 `json:"total,omitempty" cborgen:"total,omitempty"` Uri string `json:"uri" cborgen:"uri"` Versions []*RepoListCobVersions_Version `json:"versions" cborgen:"versions"` } @@ -37,8 +39,9 @@ type RepoListCobVersions_Version struct { // RepoListCobVersions calls the XRPC method "sh.tangled.repo.listCobVersions". // // cursor: Pass the cursor from one page into the next query to keep walking versions. +// order: Which end the walk starts from. `desc` puts the newest version first, which is where the version you just wrote is. // uri: AT-URI of a record in a repository this knot hosts, its repository authority a DID. -func RepoListCobVersions(ctx context.Context, c util.LexClient, cursor string, limit int64, uri string) (*RepoListCobVersions_Output, error) { +func RepoListCobVersions(ctx context.Context, c util.LexClient, cursor string, limit int64, order string, uri string) (*RepoListCobVersions_Output, error) { var out RepoListCobVersions_Output params := map[string]interface{}{} @@ -48,6 +51,9 @@ func RepoListCobVersions(ctx context.Context, c util.LexClient, cursor string, l if limit != 0 { params["limit"] = limit } + if order != "" { + params["order"] = order + } params["uri"] = uri if err := c.LexDo(ctx, util.Query, "", "sh.tangled.repo.listCobVersions", params, nil, &out); err != nil { return nil, err diff --git a/docs/src/content/docs/reference/xrpc.md b/docs/src/content/docs/reference/xrpc.md index f08e77e4a..08758c6ff 100644 --- a/docs/src/content/docs/reference/xrpc.md +++ b/docs/src/content/docs/reference/xrpc.md @@ -81,7 +81,7 @@ All **none**: `sh.tangled.repo.tree`, `log`, `branches`, `branch`, | --------------------------------- | ------------------------------------ | --------------------------------------------------------------------------------- | | `sh.tangled.repo.pull.merge` | service auth (owner or collaborator) | Merge a pull's latest patch into its target branch and mark the ticket completed. | | `sh.tangled.repo.mergeCheck` | service auth | Check whether a patch applies. | -| `sh.tangled.repo.listCobVersions` | none | COB change IDs behind a record, used for attestations. | +| `sh.tangled.repo.listCobVersions` | none | Versions of a COB record, `order=desc` for newest first, for attestations. | ### PDS surface diff --git a/knot2/crates/knot-xrpc/src/social.rs b/knot2/crates/knot-xrpc/src/social.rs index c6f76f0b0..102e14742 100644 --- a/knot2/crates/knot-xrpc/src/social.rs +++ b/knot2/crates/knot-xrpc/src/social.rs @@ -528,6 +528,18 @@ struct Written { commit: CommitHead, #[serde(rename = "validationStatus")] validation_status: ValidationStatus, + // missing when the answer replays a version instead of writing one + #[serde(rename = "x-tngl-change", skip_serializing_if = "Option::is_none")] + change: Option, +} + +impl Written { + fn by(self, change: ChangeId) -> Self { + Self { + change: Some(change), + ..self + } + } } #[derive(Serialize)] @@ -541,6 +553,7 @@ fn wrote(repo: &RepoDid, address: &RecordAddress, cid: RecordCid, head: &CommitH cid: cid.into(), commit: head.clone(), validation_status: ValidationStatus::Valid, + change: None, } } @@ -1182,8 +1195,8 @@ where }, body, }; - let (_, _, head) = ledger.write(cx, &address, change, Doing::Open { body: &record })?; - Ok(wrote(ledger.repo, &address, produces, &head)) + let (_, tip, head) = ledger.write(cx, &address, change, Doing::Open { body: &record })?; + Ok(wrote(ledger.repo, &address, produces, &head).by(tip)) }) .await } @@ -1679,7 +1692,7 @@ async fn ticket_put( }, body, }; - let (_, _, head) = ledger.write( + let (_, tip, head) = ledger.write( cx, &address, change, @@ -1689,7 +1702,7 @@ async fn ticket_put( body: &record, }, )?; - Ok(wrote(ledger.repo, &address, produces, &head)) + Ok(wrote(ledger.repo, &address, produces, &head).by(tip)) }) .await } @@ -1757,7 +1770,7 @@ async fn comment_put( }, body: opened, }; - let (_, _, head) = ledger.write( + let (_, tip, head) = ledger.write( cx, &address, change, @@ -1767,7 +1780,7 @@ async fn comment_put( body: &record, }, )?; - Ok(wrote(ledger.repo, &address, produces, &head)) + Ok(wrote(ledger.repo, &address, produces, &head).by(tip)) }) .await } @@ -1930,7 +1943,7 @@ async fn label_def_put( }, body: LabelDef, }; - let (_, _, head) = ledger.write( + let (_, tip, head) = ledger.write( cx, &address, change, @@ -1940,7 +1953,7 @@ async fn label_def_put( body: &record, }, )?; - Ok(wrote(ledger.repo, &address, produces, &head)) + Ok(wrote(ledger.repo, &address, produces, &head).by(tip)) }) .await } @@ -2124,6 +2137,24 @@ const COB_VERSIONS_DEFAULT: usize = 100; const COB_VERSIONS_MAX: usize = 500; +// not `query::Order`, which defaults to desc while this list reads oldest first +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum VersionOrder { + #[default] + OldestFirst, + NewestFirst, +} + +impl<'de> Deserialize<'de> for VersionOrder { + fn deserialize>(deserializer: D) -> Result { + match String::deserialize(deserializer)?.as_str() { + "" | "asc" => Ok(Self::OldestFirst), + "desc" => Ok(Self::NewestFirst), + _ => Err(serde::de::Error::custom("order must be 'asc' or 'desc'")), + } + } +} + #[derive(Deserialize)] pub(crate) struct VersionsQuery { pub(crate) uri: AtUri, @@ -2131,6 +2162,8 @@ pub(crate) struct VersionsQuery { pub(crate) limit: crate::query::Limit, #[serde(default)] pub(crate) cursor: crate::query::Offset, + #[serde(default)] + pub(crate) order: VersionOrder, } #[derive(Serialize)] @@ -2149,6 +2182,7 @@ struct Versions { uri: AtUri, #[serde(skip_serializing_if = "Option::is_none")] cursor: Option, + total: usize, versions: Vec, } @@ -2171,6 +2205,7 @@ pub(crate) async fn list_cob_versions( let listing = address.at_uri(&repo); let response_limit = state.byte_limits.response.get(); let (limit, cursor) = (query.limit, query.cursor); + let newest_first = query.order == VersionOrder::NewestFirst; let type_name = knot_types::TypeName::new(collection.as_str()) .map_err(|_| XrpcError::invalid_request("The URI's collection isn't a type name"))?; let (versions, total) = run_blocking(move || { @@ -2178,12 +2213,18 @@ pub(crate) async fn list_cob_versions( let store = CobStore::new(&git); let delta = store.changes_of(&type_name, object, knot_cob::HistoryModel::Convergent, None)?; - let total = crate::query::Total::new(delta.changes.len()); - delta - .changes - .iter() - .skip(cursor.get()) - .take(limit.get()) + let changes = &delta.changes; + let total = crate::query::Total::new(changes.len()); + let end = cursor.get().saturating_add(limit.get()).min(changes.len()); + let nth = |at: usize| { + if newest_first { + changes.len() - 1 - at + } else { + at + } + }; + (cursor.get()..end) + .map(|at| &changes[nth(at)]) .map(|change| { let payload = knot_cobs::decode_social(change.payload()).map_err(CobError::from)?; let emission = knot_cobs::Social::emission(&payload); @@ -2202,6 +2243,7 @@ pub(crate) async fn list_cob_versions( Versions { uri: listing, cursor: crate::query::next_cursor(cursor, limit, total), + total: total.get(), versions, }, response_limit, diff --git a/knot2/crates/knot-xrpc/src/tests.rs b/knot2/crates/knot-xrpc/src/tests.rs index b12042633..6666e3d4f 100644 --- a/knot2/crates/knot-xrpc/src/tests.rs +++ b/knot2/crates/knot-xrpc/src/tests.rs @@ -1526,6 +1526,17 @@ async fn versions_page( world: &World, uri: &AtUri, paging: Option<(usize, usize)>, +) -> serde_json::Value { + versions_ordered(world, uri, paging, Default::default()).await +} + +const NEWEST_FIRST: crate::social::VersionOrder = crate::social::VersionOrder::NewestFirst; + +async fn versions_ordered( + world: &World, + uri: &AtUri, + paging: Option<(usize, usize)>, + order: crate::social::VersionOrder, ) -> serde_json::Value { let (limit, cursor) = paging.map_or_else(Default::default, |(limit, cursor)| { ( @@ -1540,6 +1551,7 @@ async fn versions_page( uri: uri.clone(), limit, cursor, + order, }), ) .await, @@ -1547,6 +1559,15 @@ async fn versions_page( .await } +async fn names_its_change(world: &World, written: &serde_json::Value) { + let newest = versions_ordered(world, &uri_of(written), Some((1, 0)), NEWEST_FIRST).await; + assert!(written["x-tngl-change"].is_string(), "{written}"); + assert_eq!( + written["x-tngl-change"], newest["versions"][0]["changeId"], + "a write ought to name the change the chain lists for the version it made" + ); +} + fn replayed_blocks(world: &World) -> BTreeSet { world .state @@ -1771,6 +1792,45 @@ async fn stock_routes_dispatch_on_collection_and_reject_foreign_collections() { message_contains(&edit_state, &["never edited"]); } +#[tokio::test] +async fn a_record_with_one_version_lists_the_same_either_way() { + let (world, repo) = ticket_world().await; + let written = ok(Ticket::on(&world, &repo) + .open(&world.stranger, "kelp", "a forest") + .await); + let uri = uri_of(&written); + + let oldest = versions_page(&world, &uri, None).await; + let newest = versions_ordered(&world, &uri, None, NEWEST_FIRST).await; + + assert_eq!(oldest["versions"], newest["versions"]); + assert_eq!(oldest["versions"].as_array().unwrap().len(), 1); + assert_eq!( + (oldest["total"].clone(), newest["total"].clone()), + (1.into(), 1.into()) + ); + assert!(oldest.get("cursor").is_none() && newest.get("cursor").is_none()); + assert_eq!(oldest["versions"][0]["changeId"], written["x-tngl-change"]); +} + +#[test] +fn versions_query_reads_oldest_first_unless_told_newest() { + use crate::social::VersionOrder::{NewestFirst, OldestFirst}; + let order_of = |query: &str| { + let uri = format!( + "http://knot/xrpc/sh.tangled.repo.listCobVersions?uri=at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/{TICKET}/3labc{query}" + ); + axum::extract::Query::::try_from_uri(&uri.parse().unwrap()) + .map(|parsed| parsed.0.order) + }; + + assert_eq!(order_of("").unwrap(), OldestFirst); + assert_eq!(order_of("&order=asc").unwrap(), OldestFirst); + assert_eq!(order_of("&order=").unwrap(), OldestFirst); + assert_eq!(order_of("&order=desc").unwrap(), NewestFirst); + assert!(order_of("&order=sideways").is_err()); +} + #[tokio::test] async fn rewrites_chain_editors_and_erasure_tombstones_record_and_bytes() { let (world, repo) = ticket_world().await; @@ -1819,6 +1879,10 @@ async fn rewrites_chain_editors_and_erasure_tombstones_record_and_bytes() { "Without writing, an edit that doesn't change anything oughtta answer the standing version" ); assert_eq!(same["commit"], second["commit"]); + assert!( + same.get("x-tngl-change").is_none(), + "An answer that replays a version, not writes one, has no change to name" + ); let third = ok(ticket .edit(&world.member, &uri, &v2, "kelp, by the owner") @@ -1898,6 +1962,42 @@ async fn rewrites_chain_editors_and_erasure_tombstones_record_and_bytes() { assert_eq!(chain[2]["editor"], world.member.did.as_str()); assert!(chain[3].get("cid").is_none()); assert_eq!(chain[3]["editor"], world.member.did.as_str()); + assert_eq!(versions["total"], 4); + for (written, listed) in [ + (&first, &chain[0]), + (&second, &chain[1]), + (&third, &chain[2]), + ] { + assert_eq!( + written["x-tngl-change"], listed["changeId"], + "A write ought to name the change the chain lists for the version it made" + ); + } + + let newest = versions_ordered(&world, &uri, Some((2, 0)), NEWEST_FIRST).await; + let newest_chain = newest["versions"].as_array().unwrap(); + assert_eq!( + (newest_chain[0].clone(), newest_chain[1].clone()), + (chain[3].clone(), chain[2].clone()), + "desc starts at the tombstone and walks back" + ); + assert_eq!( + (newest["total"].clone(), newest["cursor"].clone()), + (4.into(), "2".into()) + ); + let oldest = versions_ordered(&world, &uri, Some((2, 2)), NEWEST_FIRST).await; + assert_eq!(oldest["versions"][1], chain[0]); + assert!(oldest.get("cursor").is_none()); + let beyond = versions_ordered(&world, &uri, Some((2, 9)), NEWEST_FIRST).await; + assert_eq!( + ( + beyond["versions"].as_array().unwrap().len(), + beyond["total"].clone() + ), + (0, 4.into()), + "a cursor past the end is an empty page that still says how many there are" + ); + assert!(beyond.get("cursor").is_none()); let first = versions_page(&world, &uri, Some((2, 0))).await; assert_eq!(first["versions"].as_array().unwrap().len(), 2); @@ -6390,6 +6490,7 @@ async fn comment_opens_edits_and_deletes_along_its_version_chain() { let issue = contribution.opened(&world.stranger, "kelp").await; let written = ok(issue.comment(&world.stranger, "a drifter").await); + names_its_change(&world, &written).await; let comment_uri = uri_of(&written); let v1 = cid_of(&written); assert!( @@ -6432,6 +6533,7 @@ async fn comment_opens_edits_and_deletes_along_its_version_chain() { let edited = ok(issue .edit(&world.stranger, &comment_uri, &v1, "the drifter") .await); + names_its_change(&world, &edited).await; let v2 = cid_of(&edited); assert_ne!(v1, v2); @@ -6646,6 +6748,7 @@ async fn reaction_answers_one_live_record_per_actor_subject_and_kind() { let issue = contribution.opened(&world.stranger, "kelp").await; let liked = ok(issue.react(&world.stranger, "❤️").await); + names_its_change(&world, &liked).await; let like_uri = uri_of(&liked); let like_v1 = cid_of(&liked); assert!( @@ -6663,6 +6766,10 @@ async fn reaction_answers_one_live_record_per_actor_subject_and_kind() { ); let again = ok(issue.react(&world.stranger, "❤️").await); + assert!( + again.get("x-tngl-change").is_none(), + "a deduped reaction replays the first one, so no change was made" + ); assert_eq!(again["uri"], like_uri.as_str()); assert_eq!(again["cid"], like_v1.to_string()); assert_eq!( @@ -7105,6 +7212,7 @@ async fn define_label( def_body(name, color), ) .await); + names_its_change(world, &written).await; (uri_of(&written), cid_of(&written)) } diff --git a/lexicons/repo/listCobVersions.json b/lexicons/repo/listCobVersions.json index b5c701f1d..08cbe3be1 100644 --- a/lexicons/repo/listCobVersions.json +++ b/lexicons/repo/listCobVersions.json @@ -4,7 +4,7 @@ "defs": { "main": { "type": "query", - "description": "Every version of a COB record! Give a record's URI and get back every saved version of that record, oldest to newest. Fetch a version's CID through `com.atproto.sync.getBlocks` for as long as the knot still has it.", + "description": "Every version of a COB record! Give a record's URI and get back every saved version of that record, oldest to newest unless `order` says otherwise. Fetch a version's CID through `com.atproto.sync.getBlocks` for as long as the knot still has it.", "parameters": { "type": "params", "required": [ @@ -25,6 +25,12 @@ "cursor": { "type": "string", "description": "Pass the cursor from one page into the next query to keep walking versions." + }, + "order": { + "type": "string", + "knownValues": ["asc", "desc"], + "default": "asc", + "description": "Which end the walk starts from. `desc` puts the newest version first, which is where the version you just wrote is." } } }, @@ -43,7 +49,11 @@ }, "cursor": { "type": "string", - "description": "Left out when the page already reaches the newest version." + "description": "Left out when the page already reaches the last version in the requested order." + }, + "total": { + "type": "integer", + "description": "How many versions the record has, whatever the page." }, "versions": { "type": "array", diff --git a/web/src/lib/api/lexicons/types/sh/tangled/repo/listCobVersions.ts b/web/src/lib/api/lexicons/types/sh/tangled/repo/listCobVersions.ts index 2db57121d..47879ff52 100644 --- a/web/src/lib/api/lexicons/types/sh/tangled/repo/listCobVersions.ts +++ b/web/src/lib/api/lexicons/types/sh/tangled/repo/listCobVersions.ts @@ -19,6 +19,14 @@ const _mainSchema = /*#__PURE__*/ v.query("sh.tangled.repo.listCobVersions", { ]), 100, ), + /** + * Which end the walk starts from. `desc` puts the newest version first, which is where the version you just wrote is. + * @default "asc" + */ + order: /*#__PURE__*/ v.optional( + /*#__PURE__*/ v.string<"asc" | "desc" | (string & {})>(), + "asc", + ), /** * AT-URI of a record in a repository this knot hosts, its repository authority a DID. */ @@ -28,9 +36,13 @@ const _mainSchema = /*#__PURE__*/ v.query("sh.tangled.repo.listCobVersions", { type: "lex", schema: /*#__PURE__*/ v.object({ /** - * Left out when the page already reaches the newest version. + * Left out when the page already reaches the last version in the requested order. */ cursor: /*#__PURE__*/ v.optional(/*#__PURE__*/ v.string()), + /** + * How many versions the record has, whatever the page. + */ + total: /*#__PURE__*/ v.optional(/*#__PURE__*/ v.integer()), uri: /*#__PURE__*/ v.resourceUriString(), get versions() { return /*#__PURE__*/ v.array(versionSchema); -- 2.51.2