From be79b49fa38e4dc79c4d64b01a29505d24af6c3e Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Fri, 31 Jul 2026 10:48:21 -0700 Subject: [PATCH 1/5] Serve createRecord from MockPDS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same guards and body handling as `putRecord`, but the PDS mints the record key instead of taking it from the input — a TID, since that is what the key callers split back out of `uri` has to parse as. Production paths that create records (blocking, for one) 400'd against the mock and so could not be tested at all. Co-Authored-By: Claude Opus 5 --- .changeset/mock-create-record.md | 5 + .../AtprotoClientMocks/MockPDS/MockPDS.swift | 73 +++++++++++ .../MockPDSCreateRecordTests.swift | 115 ++++++++++++++++++ 3 files changed, 193 insertions(+) create mode 100644 .changeset/mock-create-record.md create mode 100644 Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift diff --git a/.changeset/mock-create-record.md b/.changeset/mock-create-record.md new file mode 100644 index 0000000..be3e729 --- /dev/null +++ b/.changeset/mock-create-record.md @@ -0,0 +1,5 @@ +--- +"@germ-network/atprotoclient": minor +--- + +Serve `com.atproto.repo.createRecord` from MockPDS. Same guards and body handling as `putRecord`, but the PDS mints the record key — a TID, so the key callers recover from `uri` parses as one — instead of taking it from the input. Production paths that create records (blocking, for one) were previously untestable against the mock: it 400'd on the way in. diff --git a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift index f764eb1..120443b 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift @@ -7,6 +7,7 @@ import AtprotoClient import AtprotoTypes +import AtprotoTypesMocks import Foundation import GermConvenience @@ -105,6 +106,15 @@ public actor MockPDS { return try await listRecords(queryItems: queryItems) // case Lexicon.Com.Atproto.Sync.GetBlob.nsid: // break + case Lexicon.Com.Atproto.Repo.CreateRecordNSID.nsid: + guard let authedDid else { + return try .mock(error: "Unauthorized", status: 401) + } + + return try await createRecord( + authedDid: authedDid, bodyData: body.tryUnwrap + ) + case Lexicon.Com.Atproto.Repo.PutRecordNSID.nsid: guard let authedDid else { return try .mock(error: "Unauthorized", status: 401) @@ -223,6 +233,69 @@ public actor MockPDS { let collection: Atproto.NSID } + //Same guards and body handling as `putRecord`; the difference is the record + //key. Create mints one — a TID, since that is what the record keys the app + //then reads back out of `uri` have to parse as — where put takes one from the + //input. The lexicon still allows an explicit rkey, so honor it when sent. + private func createRecord( + authedDid: Atproto.DID, + bodyData: Data + ) async throws -> HTTPDataResponse { + let protoSchema = try JSONDecoder().decode(ProtoSchema.self, from: bodyData) + + guard case .did(let did) = protoSchema.repo else { + return try .mock(error: "Invalid Request", status: 400) + } + + guard did == authedDid else { + return try .mock(error: "Unauthorized", status: 401) + } + + guard let repo = repos[authedDid] else { + return try .mock(error: "Invalid Request", status: 400) + } + + //hacky, but type-erases the record type + let input = try JSONSerialization.jsonObject(with: bodyData) + let inputDict = try (input as? [String: Any]).tryUnwrap + let rkey = (inputDict["rkey"] as? String) ?? Atproto.TID.mock().rawValue + + let encodedRecord = + try JSONSerialization + .data(withJSONObject: inputDict["record"].tryUnwrap) + + try await repo.createRecord( + collection: protoSchema.collection, + rkey: rkey, + encodedRecord: encodedRecord + ) + + //unlike put, the caller did not choose the key, so the uri is the only + //way it learns which record it just wrote + let returnVal = Lexicon.Com.Atproto.Repo + .PutRecordOutput( + uri: + "at://\(authedDid.rawValue)/\(protoSchema.collection.rawValue)/\(rkey)", + cid: "mock", + commit: try .mock(), + validationStatus: .valid + ) + return .init( + data: try JSONEncoder().encode(returnVal), + response: .init( + status: .ok, + headerFields: .init( + [ + .init( + name: .contentType, + value: HTTPContentType.json.rawValue + ) + ] + ) + ) + ) + } + private func putRecord( authedDid: Atproto.DID, bodyData: Data diff --git a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift new file mode 100644 index 0000000..68930ca --- /dev/null +++ b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift @@ -0,0 +1,115 @@ +// +// MockPDSCreateRecordTests.swift +// AtprotoClientTests +// +// `com.atproto.repo.createRecord` against `MockPDS`. Before it was served, any +// production path that creates a record — blocking someone, say — could not be +// tested against the mock at all: it 400'd on the way in. +// +// What distinguishes create from put is that the PDS, not the caller, picks the +// record key, so both properties below are about the key: it comes back as a +// usable TID, and a second create does not land on top of the first. +// + +import AtprotoClient +import AtprotoClientMocks +import AtprotoTypes +import Foundation +import Testing + +struct MockPDSCreateRecordTests { + let mockPDS: MockPDS + + init() throws { + self.mockPDS = try .init() + } + + /// The record key the caller gets back has to be a real TID: callers recover + /// it by splitting `uri`, then feed it to `deleteRecord` as an `Atproto.TID`, + /// which validates. A UUID or any other filler would store fine and fail there. + private func rkey(of output: Lexicon.Com.Atproto.Repo.PutRecordOutput) throws + -> Atproto.TID + { + try .init(string: .init(output.uri.split(separator: "/").last ?? "")) + } + + @Test("a created record reads back at the key the PDS minted") + func createdRecordReadsBack() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + let subject = Atproto.DID.mock() + + let output = try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: subject, createdAt: .now) + ) + + let readBack = try await authAgent.getRecord( + Lexicon.App.Bsky.Graph.Block.self, + rkey: try rkey(of: output), + cid: nil + ) + + #expect(readBack?.subject == subject) + } + + /// The property that separates create from put: put twice at one key leaves + /// one record, create twice leaves two. A handler that reused a fixed key — + /// or read a key out of an input that carries none — would pass the test + /// above and fail this one. + @Test("two creates write two distinct records") + func twoCreatesWriteTwoRecords() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + let first = Atproto.DID.mock() + let second = Atproto.DID.mock() + + let firstOutput = try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: first, createdAt: .now) + ) + let secondOutput = try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: second, createdAt: .now) + ) + + #expect(try rkey(of: firstOutput) != rkey(of: secondOutput)) + + let (records, _) = try await authAgent.listRecords( + Lexicon.App.Bsky.Graph.Block.self, + limit: nil, + cursor: nil, + reverse: nil + ) + #expect(Set(records.map(\.subject)) == [first, second]) + } + + /// Unauthenticated callers get 401 rather than a write, same as put. + @Test("creating without a session is rejected") + func createWithoutAuthIsRejected() async throws { + let did = Atproto.DID.mock() + let _ = try await mockPDS.host(did: did) + let publicAgent = try await mockPDS.publicAgent(did: did) + + await #expect(throws: (any Error).self) { + try await publicAgent.call( + Lexicon.Com.Atproto.Repo.CreateRecord< + Lexicon.App.Bsky.Graph.Block + >.self, + input: .init( + schema: .init( + repo: .did(did), + rkey: nil, + record: .init(subject: .mock(), createdAt: .now) + ) + ) + ) + } + + let (records, _) = try await mockPDS.authAgent(did: did) + .listRecords( + Lexicon.App.Bsky.Graph.Block.self, + limit: nil, + cursor: nil, + reverse: nil + ) + #expect(records.isEmpty) + } +} From 9a1be1aec477b55644c6abcd509588ff1ae605a8 Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Fri, 31 Jul 2026 11:15:42 -0700 Subject: [PATCH 2/5] Address review: one uri builder, no rkey passthrough, assert the 401 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes from review of the createRecord handler: Record uris were built in two places and had drifted. Reads reported `at://did:web:example.com/NSID(rawValue: "app.bsky.graph.block")/` — a hardcoded authority, and `collection` interpolated as a struct instead of its rawValue — so the uri create returned and the uri a later read reported were different strings for the same record. `MockRepo` now owns the one builder and knows its own DID. The lexicon's optional rkey is no longer honored. Accepting it without also modeling the already-exists failure was putRecord wearing create's name, and that branch had no test. It is refused with a 400 that says to use putRecord. `createWithoutAuthIsRejected` asserted only that something threw, which an unserved endpoint's 400 satisfies just as well — it passed with the handler deleted. It now asserts the 401. Co-Authored-By: Claude Opus 5 --- .changeset/mock-create-record.md | 6 +- .../AtprotoClientMocks/MockPDS/MockPDS.swift | 28 +++++-- .../AtprotoClientMocks/MockPDS/MockRepo.swift | 26 +++++- .../MockPDSCreateRecordTests.swift | 80 ++++++++++++++++++- .../MockRepoResilienceTests.swift | 2 +- 5 files changed, 127 insertions(+), 15 deletions(-) diff --git a/.changeset/mock-create-record.md b/.changeset/mock-create-record.md index be3e729..ab53abe 100644 --- a/.changeset/mock-create-record.md +++ b/.changeset/mock-create-record.md @@ -2,4 +2,8 @@ "@germ-network/atprotoclient": minor --- -Serve `com.atproto.repo.createRecord` from MockPDS. Same guards and body handling as `putRecord`, but the PDS mints the record key — a TID, so the key callers recover from `uri` parses as one — instead of taking it from the input. Production paths that create records (blocking, for one) were previously untestable against the mock: it 400'd on the way in. +Serve `com.atproto.repo.createRecord` from MockPDS. Same guards and body handling as `putRecord`, but the PDS mints the record key — a TID, so the key callers recover from `uri` parses as one. A caller-supplied `rkey` is refused with a 400 rather than honored: without the already-exists failure modeled too, honoring it would just be `putRecord` under another name. Production paths that create records (blocking, for one) were previously untestable against the mock: it 400'd on the way in. + +Also fixes the record `uri` the mock reports on reads, which was `at://did:web:example.com/NSID(rawValue: "app.bsky.graph.block")/` — a hardcoded authority, and `collection` interpolated as a struct rather than its `rawValue`. Every uri is now built in one place from the repo's own DID, so create and read agree. + +Breaking: `MockRepo.init` takes the repo's `did`. Callers that construct a `MockRepo` directly need updating; `MockPDS.host(did:bskyProfile:)` is unchanged. diff --git a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift index 120443b..f3c4283 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift @@ -30,7 +30,7 @@ public actor MockPDS { throw Errors.didAlreadyHostedHere } - repos[did] = try .init(bskyProfile: bskyProfile) + repos[did] = try .init(did: did, bskyProfile: bskyProfile) return .init(did: did, pds: self) } @@ -236,7 +236,7 @@ public actor MockPDS { //Same guards and body handling as `putRecord`; the difference is the record //key. Create mints one — a TID, since that is what the record keys the app //then reads back out of `uri` have to parse as — where put takes one from the - //input. The lexicon still allows an explicit rkey, so honor it when sent. + //input. private func createRecord( authedDid: Atproto.DID, bodyData: Data @@ -258,7 +258,23 @@ public actor MockPDS { //hacky, but type-erases the record type let input = try JSONSerialization.jsonObject(with: bodyData) let inputDict = try (input as? [String: Any]).tryUnwrap - let rkey = (inputDict["rkey"] as? String) ?? Atproto.TID.mock().rawValue + + //The lexicon's optional rkey is deliberately NOT modeled. Honoring it + //without also modeling the already-exists failure would just be putRecord + //wearing create's name, and minting a different key anyway would strand a + //caller that asked for a specific one. Refuse it, loudly: a test that wants + //to choose the key wants `putRecord(_:input:)`. + guard inputDict["rkey"] as? String == nil else { + return try .mock( + errorObject: .init( + error: "InvalidRequest", + message: + "MockPDS mints record keys; use putRecord to choose one" + ), + status: .badRequest + ) + } + let rkey = Atproto.TID.mock().rawValue let encodedRecord = try JSONSerialization @@ -274,8 +290,10 @@ public actor MockPDS { //way it learns which record it just wrote let returnVal = Lexicon.Com.Atproto.Repo .PutRecordOutput( - uri: - "at://\(authedDid.rawValue)/\(protoSchema.collection.rawValue)/\(rkey)", + uri: repo.recordUri( + collection: protoSchema.collection, + rkey: rkey + ), cid: "mock", commit: try .mock(), validationStatus: .valid diff --git a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift index 10f5977..8788e6b 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift @@ -26,7 +26,15 @@ public actor MockRepo { typealias Cursor = UUID private var paginationCache: [UUID: [(EncodedRecordKey, Data)]] = [:] - public init(bskyProfile: Lexicon.App.Bsky.Actor.Profile? = nil) throws { + //the repo this is, so record uris carry the right authority + public nonisolated let did: Atproto.DID + + public init( + did: Atproto.DID, + bskyProfile: Lexicon.App.Bsky.Actor.Profile? = nil + ) throws { + self.did = did + guard let bskyProfile else { untypedRepo = [:] return @@ -44,6 +52,17 @@ public actor MockRepo { print(untypedRepo) } + //Every record uri is built here, so what `createRecord` hands back and what + //`getRecord`/`listRecords` report for that same record cannot drift apart. + //They did: reads hardcoded a `did:web:example.com` authority and interpolated + //`collection` as a struct, which put `NSID(rawValue: "...")` in the path. + nonisolated func recordUri( + collection: Atproto.NSID, + rkey: EncodedRecordKey + ) -> String { + "at://\(did.rawValue)/\(collection.rawValue)/\(rkey)" + } + enum Errors: Error { case badParameters case cursorNotFound @@ -96,7 +115,7 @@ extension MockRepo { // TODO: Mock CID return [ - "uri": "at://did:web:example.com/\(collection)/\(encodedRkey)", + "uri": recordUri(collection: collection, rkey: encodedRkey), "cid": Atproto.CID.mock().string, "value": try JSONSerialization.jsonObject(with: record), ] @@ -178,8 +197,7 @@ extension MockRepo { try pending.prefix(pageSize) .map { (key, encodedRecord) in [ - "uri": - "at://did:web:example.com/\(collection)/\(key)", + "uri": recordUri(collection: collection, rkey: key), "cid": Atproto.CID.mock().string, "value": try JSONSerialization diff --git a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift index 68930ca..cde80ec 100644 --- a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift +++ b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift @@ -7,8 +7,12 @@ // tested against the mock at all: it 400'd on the way in. // // What distinguishes create from put is that the PDS, not the caller, picks the -// record key, so both properties below are about the key: it comes back as a -// usable TID, and a second create does not land on top of the first. +// record key, so most of what follows is about the key: it comes back as a usable +// TID, a second create does not land on top of the first, and a caller who tries +// to choose one is refused rather than quietly handed a different key. +// +// The uri test is here for the same reason — minting means `uri` is the only +// channel back, so it has to agree with what a later read reports. // import AtprotoClient @@ -81,14 +85,76 @@ struct MockPDSCreateRecordTests { #expect(Set(records.map(\.subject)) == [first, second]) } - /// Unauthenticated callers get 401 rather than a write, same as put. + /// The uri create hands back has to be the uri the same record reports when + /// read: they are built from different code paths, and they diverged — reads + /// carried a hardcoded authority and a `NSID(rawValue:)` reflection dump where + /// the collection belongs, so a caller comparing the two got nonsense. + @Test("create's uri is the uri the record reports when read") + func createUriMatchesTheReadUri() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + + let output = try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now) + ) + + let listed = try await authAgent.call( + Lexicon.Com.Atproto.Repo.ListRecords + .self, + parameters: .init( + repo: .did(did), limit: nil, cursor: nil, reverse: nil) + ) + + #expect(listed.records.map(\.uri.rawValue) == [output.uri]) + #expect( + output.uri + == "at://\(did.rawValue)/app.bsky.graph.block/" + + "\(try rkey(of: output).rawValue)" + ) + } + + /// Create mints the key, so a caller that supplies one is refused rather than + /// quietly given a different key. The alternative — honoring it — would need + /// the already-exists failure modeled too, and without that it is just + /// `putRecord` under another name. + @Test("creating at a caller-chosen key is refused") + func createWithAnExplicitKeyIsRefused() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + + let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { + try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now), + rkey: try .init(string: "3kabcdefghij2") + ) + } + + guard case .xrpcError(let status, let error) = thrown else { + Issue.record("expected an xrpc error, got \(String(describing: thrown))") + return + } + #expect(status == .badRequest) + #expect(error.error == "InvalidRequest") + + let (records, _) = try await authAgent.listRecords( + Lexicon.App.Bsky.Graph.Block.self, + limit: nil, + cursor: nil, + reverse: nil + ) + #expect(records.isEmpty, "and nothing was written") + } + + /// Unauthenticated callers get 401 rather than a write, same as put. Asserted + /// on the status, not merely that something threw: an unserved endpoint 400s, + /// which throws too — so a looser assertion passes with the handler deleted. @Test("creating without a session is rejected") func createWithoutAuthIsRejected() async throws { let did = Atproto.DID.mock() let _ = try await mockPDS.host(did: did) let publicAgent = try await mockPDS.publicAgent(did: did) - await #expect(throws: (any Error).self) { + let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { try await publicAgent.call( Lexicon.Com.Atproto.Repo.CreateRecord< Lexicon.App.Bsky.Graph.Block @@ -103,6 +169,12 @@ struct MockPDSCreateRecordTests { ) } + guard case .xrpcError(let status, _) = thrown else { + Issue.record("expected a 401, got \(String(describing: thrown))") + return + } + #expect(status == .unauthorized) + let (records, _) = try await mockPDS.authAgent(did: did) .listRecords( Lexicon.App.Bsky.Graph.Block.self, diff --git a/Tests/AtprotoClientTests/MockRepoResilienceTests.swift b/Tests/AtprotoClientTests/MockRepoResilienceTests.swift index 68fb730..a1f0b90 100644 --- a/Tests/AtprotoClientTests/MockRepoResilienceTests.swift +++ b/Tests/AtprotoClientTests/MockRepoResilienceTests.swift @@ -18,7 +18,7 @@ import Testing struct MockRepoResilienceTests { @Test func graphReadAndUnfollowSkipUndecodableFollowRecords() async throws { - let repo = try MockRepo() + let repo = try MockRepo(did: .mock()) let collection = Lexicon.App.Bsky.Graph.Follow.Collection.nsid let keep = Atproto.DID.mock() From a1a38eceff11dea4c99a27d2c4ad4be6fc09ffab Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Fri, 31 Jul 2026 11:24:25 -0700 Subject: [PATCH 3/5] Return a real cid and uri, and build the JSON envelope once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cid` was the literal "mock" on both createRecord and putRecord, which does not parse as a CID — no `b` prefix, not base32 — so anything feeding it back failed at the boundary. `PutRecordOutput.cid` is typed String, so nothing catches that at decode. It is now `Atproto.CID.mock().string`, which is what reads already returned. putRecord's uri was the placeholder "example.com". It now goes through the same builder as create and read, so all three name the record the same way. The 200-with-JSON envelope was hand-built at five sites across MockPDS and MockRepo. `HTTPDataResponse.mock(json:)` / `.mock(encoding:)` join the existing `.mock(error:status:)` family; net 51 fewer lines. Co-Authored-By: Claude Opus 5 --- .changeset/mock-create-record.md | 6 +- .../MockPDS/HTTPDataResponse+Mock.swift | 24 ++++++++ .../AtprotoClientMocks/MockPDS/MockPDS.swift | 60 ++++--------------- .../AtprotoClientMocks/MockPDS/MockRepo.swift | 39 +++--------- .../MockPDSCreateRecordTests.swift | 49 +++++++++++++++ 5 files changed, 97 insertions(+), 81 deletions(-) diff --git a/.changeset/mock-create-record.md b/.changeset/mock-create-record.md index ab53abe..c910f20 100644 --- a/.changeset/mock-create-record.md +++ b/.changeset/mock-create-record.md @@ -4,6 +4,8 @@ Serve `com.atproto.repo.createRecord` from MockPDS. Same guards and body handling as `putRecord`, but the PDS mints the record key — a TID, so the key callers recover from `uri` parses as one. A caller-supplied `rkey` is refused with a 400 rather than honored: without the already-exists failure modeled too, honoring it would just be `putRecord` under another name. Production paths that create records (blocking, for one) were previously untestable against the mock: it 400'd on the way in. -Also fixes the record `uri` the mock reports on reads, which was `at://did:web:example.com/NSID(rawValue: "app.bsky.graph.block")/` — a hardcoded authority, and `collection` interpolated as a struct rather than its `rawValue`. Every uri is now built in one place from the repo's own DID, so create and read agree. +Fixes the record `uri` the mock reports. Reads returned `at://did:web:example.com/NSID(rawValue: "app.bsky.graph.block")/` — a hardcoded authority, and `collection` interpolated as a struct rather than its `rawValue` — and `putRecord` returned the bare placeholder `"example.com"`. Every uri is now built in one place from the repo's own DID, so create, put and read all name the same record the same way. -Breaking: `MockRepo.init` takes the repo's `did`. Callers that construct a `MockRepo` directly need updating; `MockPDS.host(did:bskyProfile:)` is unchanged. +Fixes the `cid` that `createRecord` and `putRecord` return: it was the literal `"mock"`, which does not parse as a CID (no `b` prefix, not base32), so feeding it back to anything that takes one failed at the boundary. It is now `Atproto.CID.mock().string`, matching what reads already returned. + +Breaking: `MockRepo.init` takes the repo's `did`. Callers that construct a `MockRepo` directly need updating; `MockPDS.host(did:bskyProfile:)` is unchanged. Anything asserting on the old `uri` or `cid` values needs updating too. diff --git a/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift b/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift index 6c97c49..5194731 100644 --- a/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift +++ b/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift @@ -32,4 +32,28 @@ extension HTTPDataResponse { status: status ) } + + //Every success the mock returns carries the same envelope — 200, JSON content + //type — and five sites were building it by hand. One builder means a change to + //the envelope reaches all of them. + static func mock(json: Data) -> Self { + .init( + data: json, + response: .init( + status: .ok, + headerFields: .init( + [ + .init( + name: .contentType, + value: HTTPContentType.json.rawValue + ) + ] + ) + ) + ) + } + + static func mock(encoding value: some Encodable) throws -> Self { + .mock(json: try JSONEncoder().encode(value)) + } } diff --git a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift index f3c4283..ba52ad7 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift @@ -288,29 +288,16 @@ public actor MockPDS { //unlike put, the caller did not choose the key, so the uri is the only //way it learns which record it just wrote - let returnVal = Lexicon.Com.Atproto.Repo - .PutRecordOutput( + return try .mock( + encoding: Lexicon.Com.Atproto.Repo.PutRecordOutput( uri: repo.recordUri( collection: protoSchema.collection, rkey: rkey ), - cid: "mock", + cid: Atproto.CID.mock().string, commit: try .mock(), validationStatus: .valid ) - return .init( - data: try JSONEncoder().encode(returnVal), - response: .init( - status: .ok, - headerFields: .init( - [ - .init( - name: .contentType, - value: HTTPContentType.json.rawValue - ) - ] - ) - ) ) } @@ -347,26 +334,16 @@ public actor MockPDS { encodedRecord: encodedRecord ) - let returnVal = Lexicon.Com.Atproto.Repo - .PutRecordOutput( - uri: "example.com", - cid: "mock", + return try .mock( + encoding: Lexicon.Com.Atproto.Repo.PutRecordOutput( + uri: repo.recordUri( + collection: protoSchema.collection, + rkey: inputRkey + ), + cid: Atproto.CID.mock().string, commit: try .mock(), validationStatus: .valid ) - return .init( - data: try JSONEncoder().encode(returnVal), - response: .init( - status: .ok, - headerFields: .init( - [ - .init( - name: .contentType, - value: HTTPContentType.json.rawValue - ) - ] - ) - ) ) } @@ -406,26 +383,13 @@ public actor MockPDS { rkey: input.rkey ) - let returnVal = Lexicon.Com.Atproto.Repo - .DeleteRecordOutput( + return try .mock( + encoding: Lexicon.Com.Atproto.Repo.DeleteRecordOutput( commit: .init( cid: .mock(), rev: .mock() ) ) - return .init( - data: try JSONEncoder().encode(returnVal), - response: .init( - status: .ok, - headerFields: .init( - [ - .init( - name: .contentType, - value: HTTPContentType.json.rawValue - ) - ] - ) - ) ) } diff --git a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift index 8788e6b..2154045 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift @@ -135,18 +135,8 @@ extension MockRepo { guard let resultObject else { return try .mock(error: "RecordNotFound", status: 400) } - return .init( - data: try JSONSerialization.data(withJSONObject: resultObject), - response: .init( - status: .ok, - headerFields: .init( - [ - .init( - name: .contentType, - value: HTTPContentType.json.rawValue) - ] - ) - ) + return .mock( + json: try JSONSerialization.data(withJSONObject: resultObject) ) } @@ -240,25 +230,12 @@ extension MockRepo { } else { nil } - let result = try listRecords( - collection: collection, - limit: limitInt, - cursor: cursor, - reverse: reverseBool - ) - - return .init( - data: result, - response: .init( - status: .ok, - headerFields: .init( - [ - .init( - name: .contentType, - value: HTTPContentType.json.rawValue - ) - ] - ) + return .mock( + json: try listRecords( + collection: collection, + limit: limitInt, + cursor: cursor, + reverse: reverseBool ) ) } diff --git a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift index cde80ec..3423271 100644 --- a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift +++ b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift @@ -113,6 +113,55 @@ struct MockPDSCreateRecordTests { ) } + /// `cid` was the literal string "mock", which is not a CID — no `b` prefix, not + /// base32 — so anything that fed it back (a swapRecord round trip, say) failed + /// at the boundary. `PutRecordOutput.cid` is typed `String`, so nothing catches + /// it at decode; this does. Put returns one too, and it was equally broken. + @Test("the cid create and put return parses as a CID") + func returnedCidIsAWellFormedCID() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + + let created = try await authAgent.createRecord( + Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now) + ) + #expect(throws: Never.self) { try Atproto.CID(string: created.cid) } + + let put = try await authAgent.putRecord( + Lexicon.App.Bsky.Graph.Block.self, + input: .init( + schema: .init( + repo: .did(did), + rkey: try .init(string: "3kabcdefghij2"), + record: .init(subject: .mock(), createdAt: .now) + ) + ) + ) + #expect(throws: Never.self) { try Atproto.CID(string: put.cid) } + } + + /// Put chose the key, so its uri is not the only channel back the way create's + /// is — but it still has to name the record that was written. It returned the + /// bare placeholder "example.com". + @Test("put's uri names the record too") + func putUriNamesTheRecord() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + + let put = try await authAgent.putRecord( + Lexicon.App.Bsky.Graph.Block.self, + input: .init( + schema: .init( + repo: .did(did), + rkey: try .init(string: "3kabcdefghij2"), + record: .init(subject: .mock(), createdAt: .now) + ) + ) + ) + + #expect(put.uri == "at://\(did.rawValue)/app.bsky.graph.block/3kabcdefghij2") + } + /// Create mints the key, so a caller that supplies one is refused rather than /// quietly given a different key. The alternative — honoring it — would need /// the already-exists failure modeled too, and without that it is just From 862b71816ef6049ebfed6e189476f7539bf426cc Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Fri, 31 Jul 2026 11:49:28 -0700 Subject: [PATCH 4/5] Take GermConvenience 0.3.0 and align the mock's errors with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.3.0 consolidates every failure onto `HTTPResponseError.unsuccessful` and reads the body through `bodyString`; `.unsuccessfulString` is no longer thrown from that module, so `getRecord`'s catch of that case would have gone dead. It matches the type and uses the accessors now. The mock's generic 400s carried "Invalid Request" — with a space, which is not an atproto error name — so `parse` matched nothing in `badRequestErrors` and every one of them surfaced as an opaque `.unrecognized(400 )`. They are "InvalidRequest" now, so consumers get a typed `.xrpcError`. "Unauthorized" (401 is a recognized status) and "RecordNotFound" (declared by GetRecord) were already fine and are untouched. Co-Authored-By: Claude Opus 5 --- .changeset/mock-create-record.md | 6 ++- Package.resolved | 6 +-- Package.swift | 2 +- .../AtprotoClientMocks/MockPDS/MockPDS.swift | 41 +++++++++++-------- .../MockPDSCreateRecordTests.swift | 31 ++++++++++++++ 5 files changed, 64 insertions(+), 22 deletions(-) diff --git a/.changeset/mock-create-record.md b/.changeset/mock-create-record.md index c910f20..fe3db93 100644 --- a/.changeset/mock-create-record.md +++ b/.changeset/mock-create-record.md @@ -8,4 +8,8 @@ Fixes the record `uri` the mock reports. Reads returned `at://did:web:example.co Fixes the `cid` that `createRecord` and `putRecord` return: it was the literal `"mock"`, which does not parse as a CID (no `b` prefix, not base32), so feeding it back to anything that takes one failed at the boundary. It is now `Atproto.CID.mock().string`, matching what reads already returned. -Breaking: `MockRepo.init` takes the repo's `did`. Callers that construct a `MockRepo` directly need updating; `MockPDS.host(did:bskyProfile:)` is unchanged. Anything asserting on the old `uri` or `cid` values needs updating too. +Raises the GermConvenience floor to 0.3.0 and aligns the mock's errors with its cleaned-up handling. The mock's generic 400s said `"Invalid Request"` — with a space, matching no atproto error name — so `parse` fell through and every one reached the caller as an opaque `.unrecognized(400 )`. They are `InvalidRequest` now, which is in `defaultErrors`, so consumers get a typed `.xrpcError` they can match on. `getRecord`'s `catch` moved off `HTTPResponseError.unsuccessfulString`, which 0.3.0 no longer throws, onto the type plus its `code` / `bodyString` accessors. + +Breaking: `MockRepo.init` takes the repo's `did`. Callers that construct a `MockRepo` directly need updating; `MockPDS.host(did:bskyProfile:)` is unchanged. Anything asserting on the old `uri` or `cid` values, or matching a mock 400 as `.unrecognized`, needs updating too. + +The GermConvenience floor is the one to watch downstream: SwiftPM's `from:` is `upToNextMajor` on 0.x, so this drags a consumer's whole graph onto 0.3.0, which is source-breaking for anything that mutates a `BundledHTTPRequest`. oauth4swift ≥ 0.6.0 carries its companion change; first-party mutation sites migrate to `settingHeader(_:for:)`. diff --git a/Package.resolved b/Package.resolved index 50880ed..e9ee7bb 100644 --- a/Package.resolved +++ b/Package.resolved @@ -1,5 +1,5 @@ { - "originHash" : "7f245233df40425cfc09bccb988b86e09600d40c1ffc004b1f93d98b860bb347", + "originHash" : "accc41766bc60949b6f42220db80f9c7347e9229d9274204e7c73b74e3b22c82", "pins" : [ { "identity" : "atprototypes", @@ -15,8 +15,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/germ-network/GermConvenience.git", "state" : { - "revision" : "5ec8573dbceb2e03b9a32c3e84a53af471df5b89", - "version" : "0.2.4" + "revision" : "bfe84a1678c6d7a21af3190c00bc78430befe36c", + "version" : "0.3.0" } }, { diff --git a/Package.swift b/Package.swift index 34be44d..0f5a84d 100644 --- a/Package.swift +++ b/Package.swift @@ -21,7 +21,7 @@ let package = Package( ), .package( url: "https://github.com/germ-network/GermConvenience.git", - from: "0.2.4" + from: "0.3.0" ), .package( url: "https://github.com/apple/swift-crypto.git", diff --git a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift index ba52ad7..f2535bf 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockPDS.swift @@ -87,7 +87,7 @@ public actor MockPDS { case ".well-known": return try await handleWellKnown(path: .init(pathComponents[2...])) default: - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } //here is where a directory of types would be handy @@ -133,13 +133,13 @@ public actor MockPDS { authedDid: authedDid, bodyData: body.tryUnwrap ) default: - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } } private func handleWellKnown(path: [String]) async throws -> HTTPDataResponse { guard let component = path.first, path.count == 1 else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } switch component { case "oauth-protected-resource": @@ -153,7 +153,7 @@ public actor MockPDS { response: .init(status: .ok) ) default: - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } } @@ -189,7 +189,7 @@ public actor MockPDS { }() guard let repo = try repos[.init(string: repoParam)] else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } do { @@ -198,11 +198,18 @@ public actor MockPDS { encodedRkey: encodedRkey, cid: typedCid ) - } catch HTTPResponseError.unsuccessfulString(let code, let error) { - return .init( - data: try JSONEncoder().encode( - Atproto.XRPC.ErrorResponse(error: error, message: error)), - response: .init(status: .init(code: code)) + //GermConvenience 0.3.0 reports every failure as `.unsuccessful` and reads + //the body through `bodyString`; `.unsuccessfulString` is no longer thrown + //from that module. Match the type and use the accessors, so this keeps + //converting whichever case a caller hands us. The body is the message — + //the name stays a code the response parser can recognize. + } catch let failure as HTTPResponseError { + return try .mock( + errorObject: .init( + error: "InvalidRequest", + message: failure.bodyString ?? "Mock Error" + ), + status: .init(code: failure.code) ) } } @@ -217,7 +224,7 @@ public actor MockPDS { let reverse = queryItems?["reverse"] guard let repo = try repos[.init(string: repoParam)] else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } return try await repo.listRecordsResponse( @@ -244,7 +251,7 @@ public actor MockPDS { let protoSchema = try JSONDecoder().decode(ProtoSchema.self, from: bodyData) guard case .did(let did) = protoSchema.repo else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } guard did == authedDid else { @@ -252,7 +259,7 @@ public actor MockPDS { } guard let repo = repos[authedDid] else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } //hacky, but type-erases the record type @@ -308,7 +315,7 @@ public actor MockPDS { let protoSchema = try JSONDecoder().decode(ProtoSchema.self, from: bodyData) guard case .did(let did) = protoSchema.repo else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } guard did == authedDid else { @@ -316,7 +323,7 @@ public actor MockPDS { } guard let repo = repos[authedDid] else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } //hacky, but type-erases the record type @@ -362,7 +369,7 @@ public actor MockPDS { let protoSchema = try JSONDecoder().decode(ProtoSchema.self, from: bodyData) guard case .did(let did) = protoSchema.repo else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } guard did == authedDid else { @@ -370,7 +377,7 @@ public actor MockPDS { } guard let repo = repos[authedDid] else { - return try .mock(error: "Invalid Request", status: 400) + return try .mock(error: "InvalidRequest", status: 400) } let input = try JSONDecoder().decode( diff --git a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift index 3423271..acc6406 100644 --- a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift +++ b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift @@ -194,6 +194,37 @@ struct MockPDSCreateRecordTests { #expect(records.isEmpty, "and nothing was written") } + /// The mock's generic 400s said `"Invalid Request"` — with a space, which is not + /// an atproto error name, so `parse` matched nothing and every one of them + /// reached the caller as an opaque `.unrecognized(400 )`. They are `InvalidRequest` + /// now, which is in `defaultErrors`, so a consumer can actually match on them. + @Test("a rejected request arrives as a typed error, not .unrecognized") + func rejectedRequestsCarryARecognizableErrorName() async throws { + let did = Atproto.DID.mock() + let authAgent = try await mockPDS.host(did: did) + + //repo that is not the authed one: the "Invalid Request" guard in putRecord + let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { + try await authAgent.putRecord( + Lexicon.App.Bsky.Graph.Block.self, + input: .init( + schema: .init( + repo: .handle(try .init(string: "example.com")), + rkey: try .init(string: "3kabcdefghij2"), + record: .init(subject: .mock(), createdAt: .now) + ) + ) + ) + } + + guard case .xrpcError(let status, let error) = thrown else { + Issue.record("expected a typed error, got \(String(describing: thrown))") + return + } + #expect(status == .badRequest) + #expect(error.error == "InvalidRequest") + } + /// Unauthenticated callers get 401 rather than a write, same as put. Asserted /// on the status, not merely that something threw: an unserved endpoint 400s, /// which throws too — so a looser assertion passes with the handler deleted. From 638240526c94306ffed31ecd29ce75b324f77d3f Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Fri, 31 Jul 2026 12:00:31 -0700 Subject: [PATCH 5/5] Close the Data footgun, split the suites by endpoint, cover getRecord's uri MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups. `mock(encoding:)` accepted an already-encoded `Data` and re-encoded it as a base64 string. It passes Data through now, the way GermConvenience's `Data.decode` special-cases the same type on the way in. Note the obvious guard does NOT work: an `@available(*, unavailable)` Data overload still loses resolution to the generic one and the misuse compiles silently — verified, and the pass-through is pinned by a test instead. The create suite had grown put's response-shape tests and a general error-vocabulary test. Split into MockPDSPutRecordTests and MockPDSErrorTests, with the repeated hosting/rkey setup in MockPDSFixture. `getRecord` embeds a uri on a path `listRecords` does not share, and nothing asserted it — the typed accessor drops everything but `value`. Asserted through `callExpectingOptional`. Also: `MockRepo.did` is internal rather than public (nothing outside needs it), and the stale `// TODO: Mock CID` is gone — that cid was always a real mock CID. Co-Authored-By: Claude Opus 5 --- .../MockPDS/HTTPDataResponse+Mock.swift | 10 +- .../AtprotoClientMocks/MockPDS/MockRepo.swift | 3 +- .../HTTPDataResponseMockTests.swift | 44 ++++ .../MockPDSCreateRecordTests.swift | 192 +++++------------- .../MockPDSErrorTests.swift | 67 ++++++ .../MockPDSPutRecordTests.swift | 62 ++++++ Tests/AtprotoClientTests/MockPDSSupport.swift | 53 +++++ 7 files changed, 282 insertions(+), 149 deletions(-) create mode 100644 Tests/AtprotoClientTests/HTTPDataResponseMockTests.swift create mode 100644 Tests/AtprotoClientTests/MockPDSErrorTests.swift create mode 100644 Tests/AtprotoClientTests/MockPDSPutRecordTests.swift create mode 100644 Tests/AtprotoClientTests/MockPDSSupport.swift diff --git a/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift b/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift index 5194731..2017313 100644 --- a/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift +++ b/Sources/AtprotoClientMocks/MockPDS/HTTPDataResponse+Mock.swift @@ -53,7 +53,15 @@ extension HTTPDataResponse { ) } + //`Data` is itself Encodable, so an already-encoded body handed to this would + //otherwise be re-encoded as a base64 string. Pass it through instead, the way + //GermConvenience's `Data.decode` special-cases the same type on the way in. An + //`@available(*, unavailable)` overload does NOT close this: the generic is + //still available, so it wins resolution and the call compiles silently. static func mock(encoding value: some Encodable) throws -> Self { - .mock(json: try JSONEncoder().encode(value)) + if let json = value as? Data { + return .mock(json: json) + } + return .mock(json: try JSONEncoder().encode(value)) } } diff --git a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift index 2154045..d18ccf0 100644 --- a/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift +++ b/Sources/AtprotoClientMocks/MockPDS/MockRepo.swift @@ -27,7 +27,7 @@ public actor MockRepo { private var paginationCache: [UUID: [(EncodedRecordKey, Data)]] = [:] //the repo this is, so record uris carry the right authority - public nonisolated let did: Atproto.DID + nonisolated let did: Atproto.DID public init( did: Atproto.DID, @@ -113,7 +113,6 @@ extension MockRepo { return nil } - // TODO: Mock CID return [ "uri": recordUri(collection: collection, rkey: encodedRkey), "cid": Atproto.CID.mock().string, diff --git a/Tests/AtprotoClientTests/HTTPDataResponseMockTests.swift b/Tests/AtprotoClientTests/HTTPDataResponseMockTests.swift new file mode 100644 index 0000000..ce41884 --- /dev/null +++ b/Tests/AtprotoClientTests/HTTPDataResponseMockTests.swift @@ -0,0 +1,44 @@ +// +// HTTPDataResponseMockTests.swift +// AtprotoClientTests +// +// The shared success envelope every mock endpoint returns through. +// + +import AtprotoTypes +import Foundation +import GermConvenience +import Testing + +@testable import AtprotoClientMocks + +struct HTTPDataResponseMockTests { + /// `Data` conforms to `Encodable`, so handing an already-encoded body to + /// `mock(encoding:)` would JSON-encode it a second time — into a base64 string + /// — and the caller would get a body that decodes as nothing it expected. The + /// overload passes Data through instead. + /// + /// Worth pinning because the obvious guard does not work: an + /// `@available(*, unavailable)` `Data` overload still loses to the generic one, + /// and the misuse compiles silently. + @Test("an already-encoded body is not encoded twice") + func dataIsPassedThroughRatherThanReEncoded() throws { + let body = Data(#"{"already":"json"}"#.utf8) + + let response = try HTTPDataResponse.mock(encoding: body) + + #expect(response.data == body) + #expect(response.data != (try JSONEncoder().encode(body))) + } + + @Test("the envelope is a 200 carrying JSON") + func envelopeIsJSONOK() throws { + let response = try HTTPDataResponse.mock(encoding: ["a": 1]) + + #expect(response.response.status == .ok) + #expect( + response.response.headerFields[.contentType] + == HTTPContentType.json.rawValue + ) + } +} diff --git a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift index acc6406..e2f21d5 100644 --- a/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift +++ b/Tests/AtprotoClientTests/MockPDSCreateRecordTests.swift @@ -11,8 +11,9 @@ // TID, a second create does not land on top of the first, and a caller who tries // to choose one is refused rather than quietly handed a different key. // -// The uri test is here for the same reason — minting means `uri` is the only -// channel back, so it has to agree with what a later read reports. +// Minting is also why `uri` matters here — it is the only channel back — so it +// has to agree with what a later read reports. Put's own response shape lives in +// `MockPDSPutRecordTests`. // import AtprotoClient @@ -22,34 +23,18 @@ import Foundation import Testing struct MockPDSCreateRecordTests { - let mockPDS: MockPDS - - init() throws { - self.mockPDS = try .init() - } - - /// The record key the caller gets back has to be a real TID: callers recover - /// it by splitting `uri`, then feed it to `deleteRecord` as an `Atproto.TID`, - /// which validates. A UUID or any other filler would store fine and fail there. - private func rkey(of output: Lexicon.Com.Atproto.Repo.PutRecordOutput) throws - -> Atproto.TID - { - try .init(string: .init(output.uri.split(separator: "/").last ?? "")) - } - @Test("a created record reads back at the key the PDS minted") func createdRecordReadsBack() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) + let (_, _, agent) = try await MockPDSFixture.hosted() let subject = Atproto.DID.mock() - let output = try await authAgent.createRecord( + let output = try await agent.createRecord( Lexicon.App.Bsky.Graph.Block(subject: subject, createdAt: .now) ) - let readBack = try await authAgent.getRecord( + let readBack = try await agent.getRecord( Lexicon.App.Bsky.Graph.Block.self, - rkey: try rkey(of: output), + rkey: try MockPDSFixture.rkey(of: output), cid: nil ) @@ -62,104 +47,67 @@ struct MockPDSCreateRecordTests { /// above and fail this one. @Test("two creates write two distinct records") func twoCreatesWriteTwoRecords() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) + let (_, _, agent) = try await MockPDSFixture.hosted() let first = Atproto.DID.mock() let second = Atproto.DID.mock() - let firstOutput = try await authAgent.createRecord( + let firstOutput = try await agent.createRecord( Lexicon.App.Bsky.Graph.Block(subject: first, createdAt: .now) ) - let secondOutput = try await authAgent.createRecord( + let secondOutput = try await agent.createRecord( Lexicon.App.Bsky.Graph.Block(subject: second, createdAt: .now) ) - #expect(try rkey(of: firstOutput) != rkey(of: secondOutput)) - - let (records, _) = try await authAgent.listRecords( - Lexicon.App.Bsky.Graph.Block.self, - limit: nil, - cursor: nil, - reverse: nil + #expect( + try MockPDSFixture.rkey(of: firstOutput) + != MockPDSFixture.rkey(of: secondOutput) + ) + #expect( + Set(try await MockPDSFixture.blocks(agent).map(\.subject)) + == [first, second] ) - #expect(Set(records.map(\.subject)) == [first, second]) } /// The uri create hands back has to be the uri the same record reports when /// read: they are built from different code paths, and they diverged — reads /// carried a hardcoded authority and a `NSID(rawValue:)` reflection dump where - /// the collection belongs, so a caller comparing the two got nonsense. + /// the collection belongs, so a caller comparing the two got nonsense. Both + /// read paths are checked, since each embeds a uri of its own. @Test("create's uri is the uri the record reports when read") func createUriMatchesTheReadUri() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) + let (_, did, agent) = try await MockPDSFixture.hosted() + + let output = try await agent.createRecord(MockPDSFixture.block()) + let rkey = try MockPDSFixture.rkey(of: output) - let output = try await authAgent.createRecord( - Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now) + #expect( + output.uri == "at://\(did.rawValue)/app.bsky.graph.block/\(rkey.rawValue)" ) - let listed = try await authAgent.call( + let listed = try await agent.call( Lexicon.Com.Atproto.Repo.ListRecords .self, parameters: .init( repo: .did(did), limit: nil, cursor: nil, reverse: nil) ) - #expect(listed.records.map(\.uri.rawValue) == [output.uri]) - #expect( - output.uri - == "at://\(did.rawValue)/app.bsky.graph.block/" - + "\(try rkey(of: output).rawValue)" - ) - } - - /// `cid` was the literal string "mock", which is not a CID — no `b` prefix, not - /// base32 — so anything that fed it back (a swapRecord round trip, say) failed - /// at the boundary. `PutRecordOutput.cid` is typed `String`, so nothing catches - /// it at decode; this does. Put returns one too, and it was equally broken. - @Test("the cid create and put return parses as a CID") - func returnedCidIsAWellFormedCID() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) - - let created = try await authAgent.createRecord( - Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now) - ) - #expect(throws: Never.self) { try Atproto.CID(string: created.cid) } - let put = try await authAgent.putRecord( - Lexicon.App.Bsky.Graph.Block.self, - input: .init( - schema: .init( - repo: .did(did), - rkey: try .init(string: "3kabcdefghij2"), - record: .init(subject: .mock(), createdAt: .now) - ) - ) + //getRecord embeds a uri too, on a path listRecords does not share + let fetched = try await agent.callExpectingOptional( + Lexicon.Com.Atproto.Repo.GetRecord + .self, + parameters: .init(repo: .did(did), rkey: rkey, cid: nil) ) - #expect(throws: Never.self) { try Atproto.CID(string: put.cid) } + #expect(fetched?.uri.rawValue == output.uri) } - /// Put chose the key, so its uri is not the only channel back the way create's - /// is — but it still has to name the record that was written. It returned the - /// bare placeholder "example.com". - @Test("put's uri names the record too") - func putUriNamesTheRecord() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) + @Test("the cid create returns parses as a CID") + func createReturnsAWellFormedCID() async throws { + let (_, _, agent) = try await MockPDSFixture.hosted() - let put = try await authAgent.putRecord( - Lexicon.App.Bsky.Graph.Block.self, - input: .init( - schema: .init( - repo: .did(did), - rkey: try .init(string: "3kabcdefghij2"), - record: .init(subject: .mock(), createdAt: .now) - ) - ) - ) + let output = try await agent.createRecord(MockPDSFixture.block()) - #expect(put.uri == "at://\(did.rawValue)/app.bsky.graph.block/3kabcdefghij2") + #expect(throws: Never.self) { try Atproto.CID(string: output.cid) } } /// Create mints the key, so a caller that supplies one is refused rather than @@ -168,13 +116,12 @@ struct MockPDSCreateRecordTests { /// `putRecord` under another name. @Test("creating at a caller-chosen key is refused") func createWithAnExplicitKeyIsRefused() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) + let (_, _, agent) = try await MockPDSFixture.hosted() let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { - try await authAgent.createRecord( - Lexicon.App.Bsky.Graph.Block(subject: .mock(), createdAt: .now), - rkey: try .init(string: "3kabcdefghij2") + try await agent.createRecord( + MockPDSFixture.block(), + rkey: try .init(string: MockPDSFixture.chosenKey) ) } @@ -184,45 +131,7 @@ struct MockPDSCreateRecordTests { } #expect(status == .badRequest) #expect(error.error == "InvalidRequest") - - let (records, _) = try await authAgent.listRecords( - Lexicon.App.Bsky.Graph.Block.self, - limit: nil, - cursor: nil, - reverse: nil - ) - #expect(records.isEmpty, "and nothing was written") - } - - /// The mock's generic 400s said `"Invalid Request"` — with a space, which is not - /// an atproto error name, so `parse` matched nothing and every one of them - /// reached the caller as an opaque `.unrecognized(400 )`. They are `InvalidRequest` - /// now, which is in `defaultErrors`, so a consumer can actually match on them. - @Test("a rejected request arrives as a typed error, not .unrecognized") - func rejectedRequestsCarryARecognizableErrorName() async throws { - let did = Atproto.DID.mock() - let authAgent = try await mockPDS.host(did: did) - - //repo that is not the authed one: the "Invalid Request" guard in putRecord - let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { - try await authAgent.putRecord( - Lexicon.App.Bsky.Graph.Block.self, - input: .init( - schema: .init( - repo: .handle(try .init(string: "example.com")), - rkey: try .init(string: "3kabcdefghij2"), - record: .init(subject: .mock(), createdAt: .now) - ) - ) - ) - } - - guard case .xrpcError(let status, let error) = thrown else { - Issue.record("expected a typed error, got \(String(describing: thrown))") - return - } - #expect(status == .badRequest) - #expect(error.error == "InvalidRequest") + #expect(try await MockPDSFixture.blocks(agent).isEmpty, "and nothing was written") } /// Unauthenticated callers get 401 rather than a write, same as put. Asserted @@ -230,9 +139,8 @@ struct MockPDSCreateRecordTests { /// which throws too — so a looser assertion passes with the handler deleted. @Test("creating without a session is rejected") func createWithoutAuthIsRejected() async throws { - let did = Atproto.DID.mock() - let _ = try await mockPDS.host(did: did) - let publicAgent = try await mockPDS.publicAgent(did: did) + let (pds, did, agent) = try await MockPDSFixture.hosted() + let publicAgent = try await pds.publicAgent(did: did) let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { try await publicAgent.call( @@ -243,7 +151,7 @@ struct MockPDSCreateRecordTests { schema: .init( repo: .did(did), rkey: nil, - record: .init(subject: .mock(), createdAt: .now) + record: MockPDSFixture.block() ) ) ) @@ -254,14 +162,6 @@ struct MockPDSCreateRecordTests { return } #expect(status == .unauthorized) - - let (records, _) = try await mockPDS.authAgent(did: did) - .listRecords( - Lexicon.App.Bsky.Graph.Block.self, - limit: nil, - cursor: nil, - reverse: nil - ) - #expect(records.isEmpty) + #expect(try await MockPDSFixture.blocks(agent).isEmpty) } } diff --git a/Tests/AtprotoClientTests/MockPDSErrorTests.swift b/Tests/AtprotoClientTests/MockPDSErrorTests.swift new file mode 100644 index 0000000..e424c71 --- /dev/null +++ b/Tests/AtprotoClientTests/MockPDSErrorTests.swift @@ -0,0 +1,67 @@ +// +// MockPDSErrorTests.swift +// AtprotoClientTests +// +// What a rejection from the mock looks like to the caller. This is a property of +// the mock's error *vocabulary*, not of any one endpoint: `parse` matches the +// `error` name against the endpoint's `badRequestErrors`, so a name that is not +// in that set collapses every distinct failure into one opaque +// `.unrecognized(400 )` — which is what a consumer debugging against the mock +// used to see. +// + +import AtprotoClient +import AtprotoClientMocks +import AtprotoTypes +import Foundation +import Testing + +struct MockPDSErrorTests { + /// The generic 400s said `"Invalid Request"` — with a space, which is not an + /// atproto error name — so none of them matched `defaultErrors`. + @Test("a rejected request arrives as a typed error, not .unrecognized") + func rejectedRequestsCarryARecognizableErrorName() async throws { + let (_, _, agent) = try await MockPDSFixture.hosted() + + //a repo that is not the authed one: putRecord's generic 400 guard + let thrown = await #expect(throws: Atproto.XRPC.ParseError.self) { + try await agent.putRecord( + Lexicon.App.Bsky.Graph.Block.self, + input: .init( + schema: .init( + repo: .handle(try .init(string: "example.com")), + rkey: try .init(string: MockPDSFixture.chosenKey), + record: MockPDSFixture.block() + ) + ) + ) + } + + guard case .xrpcError(let status, let error) = thrown else { + Issue.record("expected a typed error, got \(String(describing: thrown))") + return + } + #expect(status == .badRequest) + #expect(error.error == "InvalidRequest") + } + + /// A miss is `RecordNotFound`, which `GetRecord` declares in its + /// `notFoundCodes` — so unlike the generic 400s it always parsed, and the + /// optional-result path turns it into `nil` rather than an error. + @Test("a missing record reads back as nil, not an error") + func missingRecordIsNil() async throws { + let (_, did, agent) = try await MockPDSFixture.hosted() + + let fetched = try await agent.callExpectingOptional( + Lexicon.Com.Atproto.Repo.GetRecord + .self, + parameters: .init( + repo: .did(did), + rkey: try .init(string: MockPDSFixture.chosenKey), + cid: nil + ) + ) + + #expect(fetched == nil) + } +} diff --git a/Tests/AtprotoClientTests/MockPDSPutRecordTests.swift b/Tests/AtprotoClientTests/MockPDSPutRecordTests.swift new file mode 100644 index 0000000..2bac712 --- /dev/null +++ b/Tests/AtprotoClientTests/MockPDSPutRecordTests.swift @@ -0,0 +1,62 @@ +// +// MockPDSPutRecordTests.swift +// AtprotoClientTests +// +// `com.atproto.repo.putRecord`'s response shape. The endpoint itself predates +// these tests and is exercised throughout the mock suites; what was never +// asserted is what it hands *back*, and both fields were wrong: a placeholder +// uri that named no record, and a cid that was not a CID. +// + +import AtprotoClient +import AtprotoClientMocks +import AtprotoTypes +import Foundation +import Testing + +struct MockPDSPutRecordTests { + private func put( + _ agent: MockPDS.AuthAgent, + did: Atproto.DID + ) async throws -> Lexicon.Com.Atproto.Repo.PutRecordOutput { + try await agent.putRecord( + Lexicon.App.Bsky.Graph.Block.self, + input: .init( + schema: .init( + repo: .did(did), + rkey: try .init(string: MockPDSFixture.chosenKey), + record: MockPDSFixture.block() + ) + ) + ) + } + + /// Put chose the key, so its uri is not the only channel back the way create's + /// is — but it still has to name the record that was written. It returned the + /// bare placeholder "example.com". + @Test("put's uri names the record") + func putUriNamesTheRecord() async throws { + let (_, did, agent) = try await MockPDSFixture.hosted() + + let output = try await put(agent, did: did) + + #expect( + output.uri + == "at://\(did.rawValue)/app.bsky.graph.block/" + + MockPDSFixture.chosenKey + ) + } + + /// `cid` was the literal string "mock", which is not a CID — no `b` prefix, not + /// base32 — so anything that fed it back (a swapRecord round trip, say) failed + /// at the boundary. `PutRecordOutput.cid` is typed `String`, so nothing catches + /// it at decode; this does. + @Test("the cid put returns parses as a CID") + func putReturnsAWellFormedCID() async throws { + let (_, did, agent) = try await MockPDSFixture.hosted() + + let output = try await put(agent, did: did) + + #expect(throws: Never.self) { try Atproto.CID(string: output.cid) } + } +} diff --git a/Tests/AtprotoClientTests/MockPDSSupport.swift b/Tests/AtprotoClientTests/MockPDSSupport.swift new file mode 100644 index 0000000..ee95e6c --- /dev/null +++ b/Tests/AtprotoClientTests/MockPDSSupport.swift @@ -0,0 +1,53 @@ +// +// MockPDSSupport.swift +// AtprotoClientTests +// +// Shared setup for the MockPDS suites: a hosted repo, and the record-key +// recovery every write assertion needs. +// + +import AtprotoClient +import AtprotoClientMocks +import AtprotoTypes +import Foundation + +enum MockPDSFixture { + //a valid TID: 13 characters from the base32-sortable alphabet, leading + //character from the narrower prefix set. Anything else fails `Atproto.TID` + //validation before it reaches the repo. + static let chosenKey = "3kabcdefghij2" + + /// A PDS hosting one repo, and an agent authed as it. + static func hosted() async throws -> ( + pds: MockPDS, did: Atproto.DID, agent: MockPDS.AuthAgent + ) { + let pds = try MockPDS() + let did = Atproto.DID.mock() + return (pds, did, try await pds.host(did: did)) + } + + /// The record key out of a write's `uri`. It has to survive `Atproto.TID` + /// validation, because that is exactly what callers do with it: recover it by + /// splitting the uri, then hand it to `deleteRecord` as a typed key. A UUID or + /// any other filler would store fine here and fail there. + static func rkey( + of output: Lexicon.Com.Atproto.Repo.PutRecordOutput + ) throws -> Atproto.TID { + try .init(string: .init(output.uri.split(separator: "/").last ?? "")) + } + + static func blocks( + _ agent: MockPDS.AuthAgent + ) async throws -> [Lexicon.App.Bsky.Graph.Block] { + try await agent.listRecords( + Lexicon.App.Bsky.Graph.Block.self, + limit: nil, + cursor: nil, + reverse: nil + ).0 + } + + static func block() -> Lexicon.App.Bsky.Graph.Block { + .init(subject: .mock(), createdAt: .now) + } +}