diff --git a/cmd/validate-lexicon/main.go b/cmd/validate-lexicon/main.go index 3791d88..5dc8fe4 100644 --- a/cmd/validate-lexicon/main.go +++ b/cmd/validate-lexicon/main.go @@ -11,6 +11,7 @@ import ( "path/filepath" "strings" + "github.com/bluesky-social/indigo/atproto/atdata" lexicon "github.com/bluesky-social/indigo/atproto/lexicon" ) @@ -57,7 +58,10 @@ func main() { // Validate test data unless schemas-only flag is set if !*schemasOnly { fmt.Printf("\n๐Ÿ“‹ Validating test data from: %s\n", *testDataPath) - allSchemas := extractAllSchemaIDs(*schemaPath) + allSchemas, err := extractAllSchemaIDs(*schemaPath) + if err != nil { + log.Fatalf("Failed to extract schema IDs: %v", err) + } if err := validateTestData(&catalog, *testDataPath, *verbose, *strict, allSchemas); err != nil { log.Fatalf("Test data validation failed: %v", err) } @@ -189,8 +193,11 @@ func loadSchemasWithDebug(catalog *lexicon.BaseCatalog, schemaPath string, verbo return catalog.LoadDirectory(schemaPath) } -// extractAllSchemaIDs walks the schema directory and returns all schema IDs -func extractAllSchemaIDs(schemaPath string) []string { +// extractAllSchemaIDs walks the schema directory and returns the schema IDs of +// all record lexicons โ€” schemas whose defs["main"].type is "record". Queries, +// procedures, and defs-only files are excluded because they never appear as a +// record $type in test data. +func extractAllSchemaIDs(schemaPath string) ([]string, error) { var schemaIDs []string if err := filepath.Walk(schemaPath, func(path string, info os.FileInfo, err error) error { @@ -205,6 +212,25 @@ func extractAllSchemaIDs(schemaPath string) []string { // Only process .json files if !info.IsDir() && filepath.Ext(path) == ".json" { + data, err := os.ReadFile(path) + if err != nil { + return fmt.Errorf("failed to read schema file %s: %w", path, err) + } + + var schema struct { + Defs map[string]struct { + Type string `json:"type"` + } `json:"defs"` + } + if err := json.Unmarshal(data, &schema); err != nil { + return fmt.Errorf("failed to parse schema file %s: %w", path, err) + } + + // Only include record schemas (not queries, procedures, or defs-only files) + if schema.Defs["main"].Type != "record" { + return nil + } + // Convert file path to schema ID relPath, err := filepath.Rel(schemaPath, path) if err != nil { @@ -213,36 +239,23 @@ func extractAllSchemaIDs(schemaPath string) []string { schemaID := filepath.ToSlash(relPath) schemaID = schemaID[:len(schemaID)-5] // Remove .json extension schemaID = strings.ReplaceAll(schemaID, "/", ".") - - // Only include record schemas (not procedures) - if strings.Contains(schemaID, ".record") || - strings.Contains(schemaID, ".postv2") || - strings.Contains(schemaID, ".acceptance") || - strings.Contains(schemaID, ".removal") || - strings.Contains(schemaID, ".profile") || - strings.Contains(schemaID, ".rules") || - strings.Contains(schemaID, ".wiki") || - strings.Contains(schemaID, ".subscription") || - strings.Contains(schemaID, ".membership") || - strings.Contains(schemaID, ".vote") || - strings.Contains(schemaID, ".tag") || - strings.Contains(schemaID, ".comment") || - strings.Contains(schemaID, ".share") || - strings.Contains(schemaID, ".tribunalVote") || - strings.Contains(schemaID, ".ruleProposal") || - strings.Contains(schemaID, ".ban") { - schemaIDs = append(schemaIDs, schemaID) - } + schemaIDs = append(schemaIDs, schemaID) } return nil }); err != nil { - log.Printf("Warning: failed to walk schema directory: %v", err) + return nil, fmt.Errorf("failed to walk schema directory: %w", err) } - return schemaIDs + return schemaIDs, nil } -// validateTestData validates test JSON data files against their corresponding schemas +// validateTestData validates test JSON data files against their corresponding schemas. +// +// NOTE: tests/lexicon_fixtures_test.go is the second consumer of the fixture +// conventions applied here โ€” the "-invalid-" filename marker, the +// UseNumber-then-narrow decoding (see convertNumbers), and the default +// AllowLenientDatetime validation mode. If any of these change, update both +// harnesses together, or the two will silently diverge in what they accept. func validateTestData(catalog *lexicon.BaseCatalog, testDataPath string, verbose, strict bool, allSchemas []string) error { // Check if test data directory exists if _, err := os.Stat(testDataPath); os.IsNotExist(err) { @@ -305,6 +318,17 @@ func validateTestData(catalog *lexicon.BaseCatalog, testDataPath string, verbose // Convert json.Number values to appropriate types recordData = convertNumbers(recordData).(map[string]interface{}) + // Convert blob-shaped objects to atdata.Blob: indigo's SchemaBlob + // validation type-asserts on atdata.Blob, so a blob left as a plain + // decoded map always fails with "expected a blob". Mirrors + // convertBlobs in tests/lexicon_fixtures_test.go (harness parity). + converted, blobErr := convertBlobs(recordData) + if blobErr != nil { + validationErrors = append(validationErrors, fmt.Sprintf("Failed to convert blobs in %s: %v", path, blobErr)) + return nil + } + recordData = converted.(map[string]interface{}) + // Extract $type field recordType, ok := recordData["$type"].(string) if !ok { @@ -479,6 +503,47 @@ func validateCrossReferences(catalog *lexicon.BaseCatalog, verbose bool) error { return nil } +// convertBlobs recursively replaces blob-shaped objects ($type == "blob") with +// atdata.Blob values, which indigo's SchemaBlob validation type-asserts on. +// Mirrors convertBlobs in tests/lexicon_fixtures_test.go (harness parity). +func convertBlobs(v interface{}) (interface{}, error) { + switch vv := v.(type) { + case map[string]interface{}: + if vv["$type"] == "blob" { + raw, err := json.Marshal(vv) + if err != nil { + return nil, fmt.Errorf("re-encoding blob: %w", err) + } + var blob atdata.Blob + if err := json.Unmarshal(raw, &blob); err != nil { + return nil, fmt.Errorf("parsing blob: %w", err) + } + return blob, nil + } + result := make(map[string]interface{}, len(vv)) + for k, val := range vv { + converted, err := convertBlobs(val) + if err != nil { + return nil, err + } + result[k] = converted + } + return result, nil + case []interface{}: + result := make([]interface{}, len(vv)) + for i, val := range vv { + converted, err := convertBlobs(val) + if err != nil { + return nil, err + } + result[i] = converted + } + return result, nil + default: + return v, nil + } +} + // convertNumbers recursively converts json.Number values to int64 or float64 func convertNumbers(v interface{}) interface{} { switch vv := v.(type) { diff --git a/docs/PRD_AUTHOR_OWNED_POSTS.md b/docs/PRD_AUTHOR_OWNED_POSTS.md index b614865..ded2bcf 100644 --- a/docs/PRD_AUTHOR_OWNED_POSTS.md +++ b/docs/PRD_AUTHOR_OWNED_POSTS.md @@ -15,7 +15,10 @@ of breaking `social.coves.community.post` in place (ยง3.0/ยง3.1); lexicon details audited against the atProto Lexicon spec, record-key spec, and the draft Lexicon style guide (bluesky-social/atproto discussion #4245) โ€” open value sets confirmed, deterministic rkeys specified per the record-key spec's -"(transformed) AT URI" pattern.** +"(transformed) AT URI" pattern. +Rev 2.2 (2026-08-07): rkey derivation switched from readable URI transform to +SHA-256/base32 digest (review catch: transform non-total over legal DID +space).** **Supersedes** the write-path architecture in `docs/federation-prd.md`: that document solves cross-instance posting by service-auth-forwarding the write to @@ -170,13 +173,17 @@ As specced in rev 1 (strongRef `subject` + `createdAt`, community implicit in the repo), with two hardening changes from review: - **Record key: deterministic, not TID.** `key` is `any`, and the rkey is the - subject post's AT-URI transformed into rkey-safe form (strip `at://`, - replace `/` with `:` โ€” e.g. - `did:plc:abcโ€ฆ:social.coves.community.postv2:3jzfcijpj2z2a`; well under the - 512-char rkey limit, and human-greppable). The record-key spec explicitly - blesses `any` for exactly this โ€” "de-duplication and known-URI lookups" - via "a (transformed) AT URI" โ€” and Bluesky's `threadgate` (rkey must equal - the subject post's rkey) is precedent for subject-derived keys. One post โ†’ + unpadded lowercase base32 encoding of the SHA-256 digest of the canonical + subject AT-URI โ€” a fixed 52 characters, always within the 512-byte rkey + limit and always drawn from the rkey-safe charset. Why a digest instead of + a readable URI transform: external review caught that DIDs may legally run + up to 2048 bytes and may contain percent-escapes, so the readable transform + (strip `at://`, swap `/` for `:`) is non-total over the legal DID space โ€” + a fixed-size digest is total and still deterministic. The record-key spec + explicitly blesses `any` for exactly this โ€” "de-duplication and known-URI + lookups" via "a (transformed) AT URI" โ€” and Bluesky's `threadgate` (rkey + must equal the subject post's rkey) is precedent for subject-derived keys. + One post โ†’ one acceptance rkey per community, forever. This makes the three independent acceptance writers (sync fast path ยง4.3, firehose engine ยง5.6, notify ยง7) **idempotent by construction** โ€” concurrent attempts converge on diff --git a/internal/atproto/lexicon/social/coves/community/acceptance.json b/internal/atproto/lexicon/social/coves/community/acceptance.json index 5f25c5d..8715a97 100644 --- a/internal/atproto/lexicon/social/coves/community/acceptance.json +++ b/internal/atproto/lexicon/social/coves/community/acceptance.json @@ -4,7 +4,7 @@ "defs": { "main": { "type": "record", - "description": "A community's attestation that it accepts a post. Written only by the community's key holder, automatically once admission checks pass - machine attestation, not human approval. The community is implicit in the repository this record lives in. The record key is deterministic: the subject post's AT-URI with \"at://\" stripped and \"/\" replaced by \":\", so a post has exactly one acceptance per community and concurrent writers converge instead of allocating duplicates.", + "description": "A community's attestation that it accepts a post. Written only by the community's key holder, automatically once admission checks pass - machine attestation, not human approval. The community is implicit in the repository this record lives in. The record key is deterministic: the unpadded lowercase base32 encoding of the SHA-256 digest of the canonical subject AT-URI (fixed 52 characters, always rkey-safe), so a post has exactly one acceptance rkey per community, concurrent writers converge idempotently instead of allocating duplicates, and re-acceptance after an author edit is an update of this same record in place. Repo placement attributes authorship as vouched for by a verifying relay or a direct DID-resolved PDS fetch; firehose events themselves carry no commit-signature proof. If the author deletes the subject post, the community host deletes this acceptance on observing the tombstone; consumers treat an acceptance whose subject is gone as inert.", "key": "any", "record": { "type": "object", diff --git a/internal/atproto/lexicon/social/coves/community/postv2.json b/internal/atproto/lexicon/social/coves/community/postv2.json index 9f3be26..c9adffb 100644 --- a/internal/atproto/lexicon/social/coves/community/postv2.json +++ b/internal/atproto/lexicon/social/coves/community/postv2.json @@ -4,7 +4,7 @@ "defs": { "main": { "type": "record", - "description": "A post authored by a user, living in the author's repository. Successor to the deprecated social.coves.community.post: authorship is derived from the repository the record lives in, so there is no in-record author field. A post claiming a community is not visible in it until the community writes a social.coves.community.acceptance for it - community surfaces render from acceptance records only.", + "description": "A post authored by a user, living in the author's repository. Successor to the deprecated social.coves.community.post: authorship is derived from the repository the record lives in, so there is no in-record author field. A post claiming a community is not visible in it until the community writes a social.coves.community.acceptance for it - community surfaces render from acceptance records only. Repo placement attributes authorship as vouched for by a verifying relay or a direct DID-resolved PDS fetch; firehose events themselves carry no commit-signature proof.", "key": "tid", "record": { "type": "object", @@ -13,7 +13,7 @@ "community": { "type": "string", "format": "did", - "description": "DID of the community this was submitted to. Immutable across updates: consumers MUST ignore an update event that changes it, since retargeting a post means writing a new post record." + "description": "DID of the community this was submitted to. Immutable across updates: consumers MUST ignore the entire update event that changes it - discard the whole event, not merely retain the old community value - since retargeting a post means writing a new post record." }, "title": { "type": "string", diff --git a/internal/atproto/lexicon/social/coves/community/removal.json b/internal/atproto/lexicon/social/coves/community/removal.json index c99a48b..a559e62 100644 --- a/internal/atproto/lexicon/social/coves/community/removal.json +++ b/internal/atproto/lexicon/social/coves/community/removal.json @@ -4,7 +4,7 @@ "defs": { "main": { "type": "record", - "description": "A community's record that a post has been removed from it. Written in the same com.atproto.repo.applyWrites commit that deletes the social.coves.community.acceptance, so the firehose never carries a half-completed moderation action. Removal is URI-scoped and terminal: it applies to the post URI across later author edits, not only to the version it pins. A post rejected at submission time was never accepted and writes no record. The record key is deterministic: the subject post's AT-URI with \"at://\" stripped and \"/\" replaced by \":\".", + "description": "A community's record that a post has been removed from it. Written in the same com.atproto.repo.applyWrites commit that deletes the social.coves.community.acceptance, so the firehose never carries a half-completed moderation action. Removal is URI-scoped and terminal: it applies to the post URI across later author edits, not only to the version it pins. A post rejected at submission time was never accepted and writes no record. Removed is exited only by an explicit moderator restore: an atomic commit deleting the removal record and writing a fresh acceptance. The record key is deterministic: the unpadded lowercase base32 encoding of the SHA-256 digest of the canonical subject AT-URI (fixed 52 characters, always rkey-safe), so each post has exactly one removal rkey per community and re-removal updates the same record.", "key": "any", "record": { "type": "object", diff --git a/tests/lexicon-test-data/README.md b/tests/lexicon-test-data/README.md index 0cc6372..308ec61 100644 --- a/tests/lexicon-test-data/README.md +++ b/tests/lexicon-test-data/README.md @@ -19,18 +19,30 @@ Test files follow a specific naming pattern to distinguish between valid and inv ``` lexicon-test-data/ +โ”œโ”€โ”€ acceptance/ +โ”‚ โ”œโ”€โ”€ acceptance-valid.json # Valid community acceptance +โ”‚ โ””โ”€โ”€ acceptance-invalid-missing-subject.json # Missing required field โ”œโ”€โ”€ actor/ โ”‚ โ”œโ”€โ”€ profile-valid.json # Valid actor profile โ”‚ โ””โ”€โ”€ profile-invalid-missing-handle.json # Missing required field โ”œโ”€โ”€ community/ โ”‚ โ””โ”€โ”€ profile-valid.json # Valid community profile +โ”œโ”€โ”€ feed/ +โ”‚ โ””โ”€โ”€ vote-valid.json # Valid feed vote record โ”œโ”€โ”€ interaction/ -โ”‚ โ””โ”€โ”€ vote-valid.json # Valid vote record +โ”‚ โ””โ”€โ”€ comment-valid-text.json # Valid comment record โ”œโ”€โ”€ moderation/ โ”‚ โ””โ”€โ”€ ban-valid.json # Valid ban record -โ””โ”€โ”€ post/ - โ”œโ”€โ”€ post-valid-text.json # Valid text post - โ””โ”€โ”€ post-invalid-enum-type.json # Invalid postType value +โ”œโ”€โ”€ post/ +โ”‚ โ”œโ”€โ”€ post-valid-text.json # Valid text post (deprecated family) +โ”‚ โ””โ”€โ”€ post-invalid-missing-community.json # Missing required field +โ”œโ”€โ”€ postv2/ +โ”‚ โ”œโ”€โ”€ postv2-valid-text.json # Valid authorless post +โ”‚ โ”œโ”€โ”€ postv2-valid-embed-images.json # Valid post with an images embed +โ”‚ โ””โ”€โ”€ postv2-invalid-community-not-a-did.json # Malformed community DID +โ””โ”€โ”€ removal/ + โ”œโ”€โ”€ removal-valid.json # Valid community removal + โ””โ”€โ”€ removal-invalid-missing-code.json # Missing required field ``` ## Running Tests diff --git a/tests/lexicon-test-data/postv2/postv2-invalid-community-not-a-did.json b/tests/lexicon-test-data/postv2/postv2-invalid-community-not-a-did.json new file mode 100644 index 0000000..a984597 --- /dev/null +++ b/tests/lexicon-test-data/postv2/postv2-invalid-community-not-a-did.json @@ -0,0 +1,9 @@ +{ + "$type": "social.coves.community.postv2", + "community": "not-a-did", + "title": "Best practices for error handling in Go", + "content": "The community field must be a syntactically valid DID; this one is not.", + "tags": ["golang"], + "langs": ["en"], + "createdAt": "2025-01-09T14:30:00Z" +} diff --git a/tests/lexicon-test-data/postv2/postv2-valid-embed-images.json b/tests/lexicon-test-data/postv2/postv2-valid-embed-images.json new file mode 100644 index 0000000..ee8eb0d --- /dev/null +++ b/tests/lexicon-test-data/postv2/postv2-valid-embed-images.json @@ -0,0 +1,40 @@ +{ + "$type": "social.coves.community.postv2", + "community": "did:plc:programming123", + "title": "Gallery: my home lab rack", + "content": "Two photos of the finished rack build.", + "embed": { + "$type": "social.coves.embed.images", + "images": [ + { + "image": { + "$type": "blob", + "ref": { + "$link": "bafkreibme22gw2h7y2h7tg2fhqotaqjucnbc24deqo72b6mkl2egezxhvy" + }, + "mimeType": "image/jpeg", + "size": 214390 + }, + "alt": "A 12U server rack with cables neatly routed", + "aspectRatio": { + "width": 4, + "height": 3 + } + }, + { + "image": { + "$type": "blob", + "ref": { + "$link": "bafkreih6kvpkpwzi6vlk7cx6xoauvterrn3fqp75lwvj6saow4dbotcm2q" + }, + "mimeType": "image/png", + "size": 98231 + }, + "alt": "Close-up of the patch panel labeling" + } + ] + }, + "tags": ["homelab"], + "langs": ["en"], + "createdAt": "2026-07-22T10:00:00Z" +} diff --git a/tests/lexicon-test-data/postv2/postv2-valid-full.json b/tests/lexicon-test-data/postv2/postv2-valid-full.json index a3fecf5..64a6bb5 100644 --- a/tests/lexicon-test-data/postv2/postv2-valid-full.json +++ b/tests/lexicon-test-data/postv2/postv2-valid-full.json @@ -3,6 +3,20 @@ "community": "did:plc:programming123", "title": "Bridged megathread: the full optional surface", "content": "Every optional field on postv2 is populated here so a ref that stops resolving fails this fixture.", + "facets": [ + { + "index": { + "byteStart": 0, + "byteEnd": 20 + }, + "features": [ + { + "$type": "social.coves.richtext.facet#heading", + "level": 2 + } + ] + } + ], "embed": { "$type": "social.coves.embed.external", "external": { diff --git a/tests/lexicon_fixtures_test.go b/tests/lexicon_fixtures_test.go index afd4253..3ffc07d 100644 --- a/tests/lexicon_fixtures_test.go +++ b/tests/lexicon_fixtures_test.go @@ -8,7 +8,8 @@ import ( "strings" "testing" - lexicon "github.com/bluesky-social/indigo/atproto/lexicon" + "github.com/bluesky-social/indigo/atproto/atdata" + "github.com/bluesky-social/indigo/atproto/lexicon" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,6 +25,53 @@ const fixtureDir = "lexicon-test-data" // into a should-pass case in both harnesses. const invalidFixtureMarker = "-invalid-" +// invalidFixtureExpectedErrors pins WHY each invalid fixture is rejected, keyed +// by basename. Without this, an invalid fixture that starts failing for an +// unrelated reason โ€” a typo in a field name, a schema that stopped resolving โ€” +// still "passes", and the case it was written for goes untested. Every +// "-invalid-" fixture MUST have an entry here; the walker fails on any that +// does not, so adding a fixture forces adding its expected error. +var invalidFixtureExpectedErrors = map[string]string{ + "acceptance-invalid-missing-subject.json": "subject", + "comment-invalid-content.json": "content", + "post-invalid-missing-community.json": "community", + "postv2-invalid-community-not-a-did.json": "DID", + "postv2-invalid-missing-community.json": "community", + "postv2-invalid-missing-createdat.json": "createdAt", + "profile-invalid-moderation-type.json": "expected a string", + "removal-invalid-missing-code.json": "code", + "removal-invalid-reason-too-long.json": "graphemes", + "rule-proposal-invalid-status.json": "enum", + "rule-proposal-invalid-threshold.json": "outside specified range", + "rule-proposal-invalid-type.json": "enum", + "rules-invalid-moderation.json": "$type", + "subscription-invalid-visibility.json": "outside specified range", + "tribunal-vote-invalid-decision.json": "AT-URI", + "vote-invalid-option.json": "enum", + "wiki-invalid-slug.json": "length outside specified range", +} + +// expectedFixtureFamilies is the closed list of top-level fixture directories +// as of today. A family losing its directory (or all of its valid fixtures) +// fails the coverage guard instead of silently shrinking the suite; a new +// family must be added here to count. +var expectedFixtureFamilies = []string{ + "acceptance", + "actor", + "community", + "feed", + "interaction", + "moderation", + "post", + "postv2", + "removal", +} + +// familiesRequiringInvalidFixtures lists the families that must also carry at +// least one rejection case: the records that gate community membership and +// moderation must prove the validator rejects their malformed forms. +var familiesRequiringInvalidFixtures = []string{"postv2", "acceptance", "removal"} + // loadFixtureCatalog builds the catalog every fixture is validated against. func loadFixtureCatalog(t *testing.T) *lexicon.BaseCatalog { t.Helper() @@ -39,7 +87,10 @@ func loadFixtureCatalog(t *testing.T) *lexicon.BaseCatalog { // decoded with UseNumber and then narrowed to int64 where possible. Plain // json.Unmarshal yields float64 for every number, which fails validation of // integer-typed fields โ€” a divergence between the two harnesses would mean a -// fixture passes in one and fails in the other. +// fixture passes in one and fails in the other. On top of that, blob-shaped +// objects are converted to atdata.Blob (see convertBlobs), which +// cmd/validate-lexicon does not do yet: blob-bearing fixtures currently +// validate only here. func decodeFixture(t *testing.T, path string) map[string]interface{} { t.Helper() @@ -50,8 +101,11 @@ func decodeFixture(t *testing.T, path string) map[string]interface{} { decoder := json.NewDecoder(bytes.NewReader(raw)) decoder.UseNumber() require.NoError(t, decoder.Decode(&record), "parsing fixture %s", path) + require.False(t, decoder.More(), + "fixture %s has trailing data after the JSON record", path) - return narrowNumbers(record).(map[string]interface{}) + record = narrowNumbers(record).(map[string]interface{}) + return convertBlobs(t, path, record).(map[string]interface{}) } // narrowNumbers walks a decoded record and turns every json.Number into the @@ -83,6 +137,38 @@ func narrowNumbers(value interface{}) interface{} { } } +// convertBlobs walks a decoded record and replaces every blob-shaped object +// ({"$type": "blob", ...}) with an atdata.Blob value. indigo's SchemaBlob +// validator type-asserts on atdata.Blob, so a blob left as a plain map fails +// validation with "expected a blob" no matter how well-formed it is. +func convertBlobs(t *testing.T, path string, value interface{}) interface{} { + t.Helper() + + switch typed := value.(type) { + case map[string]interface{}: + if typed["$type"] == "blob" { + raw, err := json.Marshal(typed) + require.NoError(t, err, "re-encoding blob in fixture %s", path) + var blob atdata.Blob + require.NoError(t, json.Unmarshal(raw, &blob), "parsing blob in fixture %s", path) + return blob + } + converted := make(map[string]interface{}, len(typed)) + for key, member := range typed { + converted[key] = convertBlobs(t, path, member) + } + return converted + case []interface{}: + converted := make([]interface{}, len(typed)) + for i, member := range typed { + converted[i] = convertBlobs(t, path, member) + } + return converted + default: + return value + } +} + // collectFixturePaths returns every fixture JSON under fixtureDir, keyed by its // path relative to fixtureDir so subtest names read as "postv2/postv2-valid-text.json". func collectFixturePaths(t *testing.T) []string { @@ -93,7 +179,7 @@ func collectFixturePaths(t *testing.T) []string { if err != nil { return err } - if info.IsDir() || !strings.HasSuffix(path, ".json") { + if info.IsDir() || !strings.EqualFold(filepath.Ext(path), ".json") { return nil } relPath, err := filepath.Rel(fixtureDir, path) @@ -115,27 +201,65 @@ func collectFixturePaths(t *testing.T) []string { // TestLexiconFixtures validates every record fixture against the published // schemas. The basename decides the expectation: "-invalid-" means the record -// must be rejected, anything else means it must be accepted. +// must be rejected โ€” for the reason pinned in invalidFixtureExpectedErrors โ€” +// anything else means it must be accepted. func TestLexiconFixtures(t *testing.T) { catalog := loadFixtureCatalog(t) + fixturePaths := collectFixturePaths(t) - for _, relPath := range collectFixturePaths(t) { + for _, relPath := range fixturePaths { t.Run(filepath.ToSlash(relPath), func(t *testing.T) { record := decodeFixture(t, filepath.Join(fixtureDir, relPath)) recordType, ok := record["$type"].(string) require.True(t, ok, "fixture %s has no top-level string $type", relPath) + // An unresolvable $type must fail loudly here, not surface as a + // generic validation error: an invalid fixture whose schema went + // missing would otherwise "fail validation" for the wrong reason + // and keep passing vacuously. + _, resolveErr := catalog.Resolve(recordType) + require.NoError(t, resolveErr, + "fixture %s names a schema that does not resolve", relPath) + // AllowLenientDatetime matches cmd/validate-lexicon's default mode. err := lexicon.ValidateRecord(catalog, record, recordType, lexicon.AllowLenientDatetime) - if strings.Contains(filepath.Base(relPath), invalidFixtureMarker) { - assert.Error(t, err, "expected validation failure but record validated as %s", recordType) + basename := filepath.Base(relPath) + if strings.Contains(basename, invalidFixtureMarker) { + expectedError, ok := invalidFixtureExpectedErrors[basename] + if !ok { + t.Fatalf("invalid fixture %s has no entry in invalidFixtureExpectedErrors; add its expected error substring", relPath) + } + require.ErrorContains(t, err, expectedError, + "expected %s to be rejected as %s for its declared reason", relPath, recordType) return } - assert.NoError(t, err, "expected %s to validate as %s", relPath, recordType) + require.NoError(t, err, "expected %s to validate as %s", relPath, recordType) }) } + + t.Run("family coverage", func(t *testing.T) { + validCountByFamily := make(map[string]int) + invalidCountByFamily := make(map[string]int) + for _, relPath := range fixturePaths { + family := strings.SplitN(filepath.ToSlash(relPath), "/", 2)[0] + if strings.Contains(filepath.Base(relPath), invalidFixtureMarker) { + invalidCountByFamily[family]++ + } else { + validCountByFamily[family]++ + } + } + + for _, family := range expectedFixtureFamilies { + assert.NotZero(t, validCountByFamily[family], + "fixture family %s has no valid fixture โ€” deleted directory or all cases renamed?", family) + } + for _, family := range familiesRequiringInvalidFixtures { + assert.NotZero(t, invalidCountByFamily[family], + "fixture family %s has no invalid fixture โ€” its rejection paths are untested", family) + } + }) } // resolveRecordSchema resolves an NSID and asserts it is a record schema. @@ -150,6 +274,40 @@ func resolveRecordSchema(t *testing.T, catalog *lexicon.BaseCatalog, nsid string return record } +// requireStringProperty asserts a record property exists and is a string +// schema, returning it for further shape assertions. +func requireStringProperty(t *testing.T, record lexicon.SchemaRecord, name string) lexicon.SchemaString { + t.Helper() + + property, ok := record.Record.Properties[name] + require.True(t, ok, "record has no %s property", name) + propertyString, ok := property.Inner.(lexicon.SchemaString) + require.True(t, ok, "%s is not a string (got %T)", name, property.Inner) + return propertyString +} + +// requireDatetimeCreatedAt asserts a record's createdAt is a datetime-format string. +func requireDatetimeCreatedAt(t *testing.T, record lexicon.SchemaRecord) { + t.Helper() + + createdAt := requireStringProperty(t, record, "createdAt") + require.NotNil(t, createdAt.Format, "createdAt must declare a format") + assert.Equal(t, "datetime", *createdAt.Format, "createdAt format") +} + +// requireStrongRefSubject asserts a record's subject is a ref to +// com.atproto.repo.strongRef, pinning the exact version it points at. +func requireStrongRefSubject(t *testing.T, record lexicon.SchemaRecord) { + t.Helper() + + subject, ok := record.Record.Properties["subject"] + require.True(t, ok, "record has no subject property") + subjectRef, ok := subject.Inner.(lexicon.SchemaRef) + require.True(t, ok, "subject is not a ref (got %T)", subject.Inner) + assert.Equal(t, "com.atproto.repo.strongRef", subjectRef.Ref, + "subject must pin the exact version of the record it references") +} + // TestLexiconRecordShapes pins schema shape rather than record content. // // atproto lexicons are OPEN: an unknown field on a record validates fine. That @@ -174,6 +332,13 @@ func TestLexiconRecordShapes(t *testing.T) { assert.ElementsMatch(t, []string{"community", "createdAt"}, record.Record.Required, "postv2 required fields") + + community := requireStringProperty(t, record, "community") + require.NotNil(t, community.Format, "postv2 community must declare a format") + assert.Equal(t, "did", *community.Format, + "postv2 community must be a DID, not a free-form name") + + requireDatetimeCreatedAt(t, record) }) t.Run("social.coves.community.acceptance", func(t *testing.T) { @@ -183,12 +348,8 @@ func TestLexiconRecordShapes(t *testing.T) { assert.ElementsMatch(t, []string{"subject", "createdAt"}, record.Record.Required, "acceptance required fields") - subject, ok := record.Record.Properties["subject"] - require.True(t, ok, "acceptance has no subject property") - subjectRef, ok := subject.Inner.(lexicon.SchemaRef) - require.True(t, ok, "acceptance subject is not a ref (got %T)", subject.Inner) - assert.Equal(t, "com.atproto.repo.strongRef", subjectRef.Ref, - "acceptance must pin the exact version of what it accepts") + requireStrongRefSubject(t, record) + requireDatetimeCreatedAt(t, record) }) t.Run("social.coves.community.removal", func(t *testing.T) { @@ -198,20 +359,30 @@ func TestLexiconRecordShapes(t *testing.T) { assert.ElementsMatch(t, []string{"subject", "code", "createdAt"}, record.Record.Required, "removal required fields") - code, ok := record.Record.Properties["code"] - require.True(t, ok, "removal has no code property") - codeString, ok := code.Inner.(lexicon.SchemaString) - require.True(t, ok, "removal code is not a string (got %T)", code.Inner) + requireStrongRefSubject(t, record) + requireDatetimeCreatedAt(t, record) + + code := requireStringProperty(t, record, "code") - require.NotNil(t, codeString.MaxLength, "removal code must bound its length") - assert.Equal(t, 64, *codeString.MaxLength, "removal code maxLength") + require.NotNil(t, code.MaxLength, "removal code must bound its length") + assert.Equal(t, 64, *code.MaxLength, "removal code maxLength") // knownValues, never enum: federated peers must be able to send removal // codes this AppView has not heard of yet without their records being // rejected outright. - assert.Empty(t, codeString.Enum, + assert.Empty(t, code.Enum, "removal code must stay an open knownValues set, not a closed enum") - assert.Contains(t, codeString.KnownValues, "spam", - "removal code knownValues must document the common codes") + assert.ElementsMatch(t, []string{ + "rule-violation", + "spam", + "off-topic", + "illegal-content", + "author-banned", + "moderator-discretion", + }, code.KnownValues, "removal code knownValues must document the common codes") + + reason := requireStringProperty(t, record, "reason") + require.NotNil(t, reason.MaxGraphemes, "removal reason must bound its grapheme length") + assert.Equal(t, 1000, *reason.MaxGraphemes, "removal reason maxGraphemes") }) }