diff --git a/internal/api/handlers/user/update_profile_test.go b/internal/api/handlers/user/update_profile_test.go index 099429d..665d033 100644 --- a/internal/api/handlers/user/update_profile_test.go +++ b/internal/api/handlers/user/update_profile_test.go @@ -26,6 +26,14 @@ type mockPDSClient struct { putRecordError error putRecordURI string putRecordCID string + + // The arguments the handler actually passed to PutRecord. Captured because + // the RECORD is the wire contract with the firehose consumer that indexes + // it: a 200 from this handler says nothing about whether the thing written + // to the repo is the thing internal/atproto/jetstream can read back. + putRecordCollection string + putRecordRKey string + putRecordValue any } func (m *mockPDSClient) CreateRecord(_ context.Context, _ string, _ string, _ any) (string, string, error) { @@ -44,7 +52,8 @@ func (m *mockPDSClient) GetRecord(_ context.Context, _ string, _ string) (*pds.R return nil, nil } -func (m *mockPDSClient) PutRecord(_ context.Context, _ string, _ string, _ any, _ string) (string, string, error) { +func (m *mockPDSClient) PutRecord(_ context.Context, collection string, rkey string, record any, _ string) (string, string, error) { + m.putRecordCollection, m.putRecordRKey, m.putRecordValue = collection, rkey, record if m.putRecordError != nil { return "", "", m.putRecordError } @@ -1338,3 +1347,105 @@ func TestUpdateProfileHandler_EmptyRequestSuccess(t *testing.T) { assert.Equal(t, "at://did:plc:test123/social.coves.actor.profile/self", resp.URI) assert.Equal(t, "bafyreifake", resp.CID) } + +// TestUpdateProfileHandler_WritesTheRecordTheConsumerReadsBack asserts the SHAPE +// of the profile record the handler puts in the user's repo — not that the +// request succeeded, which every other test here already covers. +// +// # WHY THIS IS THE ASSERTION THAT WAS MISSING +// +// The handler's job ends at a repo write; everything a user sees afterwards +// comes from the firehose consumer reading that record back +// (jetstream.handleProfileUpdate → extractBlobCID → users.avatar_cid → a +// hydrated URL on getProfile). The two sides agree on nothing but a JSON shape, +// and neither has the other in scope: this package's tests mocked PutRecord and +// discarded the record, and the consumer's tests build their own record +// literals. So a handler that uploaded a blob correctly and then embedded the +// reference under the wrong key, or flattened it to a bare CID string, produced +// a 200 here, a valid-looking record on the PDS, and an avatar that silently +// never appeared — with no failing test anywhere. +// +// tests/integration/user_profile_avatar_e2e_test.go was the only thing covering +// it, at 1,022 lines and four hand-dialled websockets, and it covered it by +// accident: it watched the real firehose event go past and then re-implemented +// the consumer's extraction inside the test body. This is that claim, stated +// directly. Its other half — that a record of this shape really does reach +// getProfile as a working image URL — is tests/e2e/user_contract_test.go. +func TestUpdateProfileHandler_WritesTheRecordTheConsumerReadsBack(t *testing.T) { + const avatarCID = "bafyavatartest" + const bannerCID = "bafybannertest" + + mockClient := &mockPDSClient{ + uploadBlobRef: &blobs.BlobRef{ + Type: "blob", + Ref: map[string]string{"$link": avatarCID}, + MimeType: "image/png", + Size: 1000, + }, + putRecordURI: "at://did:plc:testuser123/social.coves.actor.profile/self", + putRecordCID: "bafyreifake", + } + handler := NewUpdateProfileHandlerWithFactory(createMockFactory(mockClient, nil)) + + body, _ := json.Marshal(UpdateProfileRequest{ + DisplayName: strPtr("Written Through"), + Bio: strPtr("and read back by the consumer"), + AvatarBlob: []byte("fake image data"), + AvatarMimeType: "image/png", + }) + req := httptest.NewRequest(http.MethodPost, "/xrpc/social.coves.actor.updateProfile", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + const testDID = "did:plc:testuser123" + req = setTestOAuthSession(req, testDID, createTestOAuthSession(testDID)) + + w := httptest.NewRecorder() + handler.ServeHTTP(w, req) + assert.Equal(t, http.StatusOK, w.Code) + + // Where it was written. rkey "self" is not a convention the handler is free + // to change: the profile is a singleton record and the consumer, the + // backfill path and every other client all address it by that key. + assert.Equal(t, "social.coves.actor.profile", mockClient.putRecordCollection) + assert.Equal(t, "self", mockClient.putRecordRKey) + + // What was written. Round-tripped through JSON rather than type-asserted, + // because JSON is what the PDS stores and what the consumer decodes — a + // field with a Go name that marshals to the wrong key would pass a + // type-assertion and fail in production. + encoded, err := json.Marshal(mockClient.putRecordValue) + assert.NoError(t, err) + var record map[string]any + assert.NoError(t, json.Unmarshal(encoded, &record)) + + assert.Equal(t, "social.coves.actor.profile", record["$type"]) + assert.Equal(t, "Written Through", record["displayName"]) + assert.Equal(t, "and read back by the consumer", record["description"], + "the bio is called `bio` on the request and `description` in the record, and the "+ + "consumer reads `description` back into the bio column: three names for one field, "+ + "and this is the only place all three are in scope at once") + + // The blob reference, in the shape jetstream.extractBlobCID insists on: + // $type == "blob" and a string at ref.$link. Anything else and the consumer + // declines the ref — silently, because a malformed picture is not worth + // failing a profile event over. + avatar, ok := record["avatar"].(map[string]any) + assert.True(t, ok, "the avatar must be an object; a bare CID string is not a blob ref and "+ + "the consumer would ignore it") + assert.Equal(t, "blob", avatar["$type"]) + assert.Equal(t, "image/png", avatar["mimeType"]) + ref, ok := avatar["ref"].(map[string]any) + assert.True(t, ok, "the blob ref's `ref` must be an object holding $link") + assert.Equal(t, avatarCID, ref["$link"], + "the record must name the CID the PDS returned from uploadBlob: any other value points "+ + "at bytes that do not exist and the image URL 502s") + + // A field the request did not set must be absent, not present and empty. + // The consumer treats an ABSENT key as "leave it alone" and an empty string + // as "clear it" (handleProfileUpdate builds a nil pointer for the former), + // so an empty banner emitted here would wipe a banner the user still has. + _, hasBanner := record["banner"] + assert.False(t, hasBanner, + "a request that did not touch the banner emitted a banner key: the consumer reads an "+ + "empty value as an instruction to clear the stored one") + assert.NotContains(t, record, bannerCID) +} diff --git a/internal/atproto/jetstream/community_consumer_test.go b/internal/atproto/jetstream/community_consumer_test.go new file mode 100644 index 0000000..b353725 --- /dev/null +++ b/internal/atproto/jetstream/community_consumer_test.go @@ -0,0 +1,283 @@ +package jetstream + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Unit coverage for the community consumer's record parsing. +// +// community_consumer.go is the largest consumer in the package and had no test +// file of its own until this one; what coverage it had arrived incidentally, +// through rev-gate and bridged-stats tests that drove it to reach some other +// conclusion. These are the two pure decoders it makes on every event, and both +// were previously exercised only through Postgres: +// tests/integration/subscription_indexing_test.go spent seven database +// round-trips per run establishing what the clamping table below states +// directly, and tests/integration/community_avatar_e2e_test.go spent 993 lines +// and three websocket dials on the blob-ref extraction. +// +// Untagged, because neither function touches anything out of process. The +// pipeline proofs they underpin are tests/e2e/community_contract_test.go (the +// avatar reaching social.coves.community.get as a hydrated URL) and +// tests/e2e/subscription_contract_test.go. + +func TestExtractContentVisibility(t *testing.T) { + t.Parallel() + + // contentVisibility is the subscriber's content-maturity preference for one + // community, valid at 1-5. It arrives from a REMOTE repo — anyone's PDS can + // write any number into a subscription record — so the consumer clamps + // rather than validates: a subscription is not worth rejecting over a + // preference field, but an out-of-range value stored as-is would leak into + // filtering comparisons downstream. + // + // JSON numbers decode as float64, which is why the interesting cases are + // float64 and the int cases exist only because the consumer accepts them + // defensively. + for _, tc := range []struct { + name string + record map[string]interface{} + want int + }{ + {"missing field defaults to 3", map[string]interface{}{}, 3}, + {"nil value defaults to 3", map[string]interface{}{"contentVisibility": nil}, 3}, + {"a string defaults to 3", map[string]interface{}{"contentVisibility": "4"}, 3}, + {"in range is kept", map[string]interface{}{"contentVisibility": float64(4)}, 4}, + {"the bottom of the range is kept", map[string]interface{}{"contentVisibility": float64(1)}, 1}, + {"the top of the range is kept", map[string]interface{}{"contentVisibility": float64(5)}, 5}, + {"zero clamps up to 1", map[string]interface{}{"contentVisibility": float64(0)}, 1}, + {"negative clamps up to 1", map[string]interface{}{"contentVisibility": float64(-5)}, 1}, + {"above range clamps down to 5", map[string]interface{}{"contentVisibility": float64(10)}, 5}, + {"far above range clamps down to 5", map[string]interface{}{"contentVisibility": float64(100)}, 5}, + {"an int is accepted and clamped", map[string]interface{}{"contentVisibility": 99}, 5}, + {"an in-range int is kept", map[string]interface{}{"contentVisibility": 2}, 2}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, extractContentVisibility(tc.record)) + }) + } +} + +func TestCommunityProfileBlobRefs(t *testing.T) { + t.Parallel() + + // The avatar and banner on a community profile are blob REFERENCES: the + // bytes live in the community's PDS and the record carries only a CID. What + // the consumer stores is that CID, and what the serving endpoint returns is + // a URL built from it (blobs.HydrateImageURL) — so an extraction that picks + // up the wrong field, or silently picks up nothing, produces a community + // with no picture and no error anywhere. + // + // What is tested here is the COMPOSITION createCommunity performs — + // parseCommunityProfile, then extractBlobCID over the field it produced — + // rather than either half alone. extractBlobCID's own edge cases already + // have a table in user_consumer_test.go (TestExtractBlobCID), and repeating + // them would say nothing new. The join is what has no coverage, and it is + // where the realistic mistake lives: a json tag that does not match the + // lexicon's field name leaves Avatar nil, extractBlobCID declines a nil map + // exactly as it should, and every community silently loses its picture with + // no error on any path. + const avatarCID = "bafkreiavatarcidaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + const bannerCID = "bafkreibannercidaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + + blobRef := func(cid string) map[string]interface{} { + return map[string]interface{}{ + "$type": "blob", + "ref": map[string]interface{}{"$link": cid}, + "mimeType": "image/png", + "size": float64(1234), + } + } + + base := func() map[string]interface{} { + return map[string]interface{}{ + "$type": "social.coves.community.profile", + "name": "blobtest", + "handle": "c-blobtest.coves.local", + "displayName": "Blob Test", + "createdBy": "did:plc:aaaaaaaaaaaaaaaaaaaaaaaa", + "hostedBy": "did:web:coves.local", + "createdAt": "2026-03-01T00:00:00Z", + } + } + + // cidsOf is the composition createCommunity and updateCommunity both apply. + cidsOf := func(t *testing.T, record map[string]interface{}) (string, string) { + t.Helper() + profile := mustParse(t, record) + avatar, _ := extractBlobCID(profile.Avatar) + banner, _ := extractBlobCID(profile.Banner) + return avatar, banner + } + + t.Run("both images are extracted, and not from each other", func(t *testing.T) { + t.Parallel() + record := base() + record["avatar"] = blobRef(avatarCID) + record["banner"] = blobRef(bannerCID) + + avatar, banner := cidsOf(t, record) + assert.Equal(t, avatarCID, avatar) + assert.Equal(t, bannerCID, banner, + "the banner must not be read out of the avatar's ref, or replacing one changes both") + }) + + t.Run("an absent image is empty rather than an error", func(t *testing.T) { + t.Parallel() + profile, err := parseCommunityProfile(base()) + require.NoError(t, err, + "a community without a picture is ordinary, not a malformed record") + avatar, banner := cidsOf(t, base()) + assert.Empty(t, avatar) + assert.Empty(t, banner) + assert.Nil(t, profile.Avatar) + }) + + t.Run("only one of the two present leaves the other alone", func(t *testing.T) { + t.Parallel() + record := base() + record["avatar"] = blobRef(avatarCID) + + avatar, banner := cidsOf(t, record) + assert.Equal(t, avatarCID, avatar) + assert.Empty(t, banner, + "a record with an avatar and no banner must not produce a banner CID: the consumer "+ + "only assigns when extraction succeeds, which is what stops an update that omits "+ + "the banner from blanking a stored one") + }) + + t.Run("a malformed ref yields nothing rather than a partial value", func(t *testing.T) { + t.Parallel() + // The realistic remote-record shape: a peer that wrote the CID inline + // instead of as a blob ref. It unmarshals into the map field fine (it is + // an object) and must produce no CID at all — a stored value of + // "map[...]" would render a URL that 502s forever. + record := base() + record["avatar"] = map[string]interface{}{"cid": avatarCID} + + avatar, _ := cidsOf(t, record) + assert.Empty(t, avatar) + }) + + t.Run("an omitted image does not blank a stored one", func(t *testing.T) { + t.Parallel() + // THE `if ok` GUARDS, stated as the invariant they exist for. + // + // Both the create and update paths assign conditionally: + // + // if avatarCID, ok := extractBlobCID(profile.Avatar); ok { … = avatarCID } + // + // so a record that omits a picture leaves the stored CID untouched + // rather than clearing it. That matters because UpdateCommunity rebuilds + // the profile record from scratch and only sets `avatar` when a NEW blob + // was uploaded — so a display-name-only edit ships a record with no + // avatar key at all, and the guard is the only thing that stops every + // such edit from wiping the community's picture. + // + // Modelled here as the decision the consumer makes (extraction + // succeeded, or it did not), which is what the guard branches on; the + // end-to-end version is the community contract's update step, and the + // erasure risk on the WRITE side is + // TestService_CreateAndUpdateWriteBlobRefsIntoTheProfileRecord. + withImage := base() + withImage["avatar"] = blobRef(avatarCID) + stored, ok := extractBlobCID(mustParse(t, withImage).Avatar) + require.True(t, ok) + require.Equal(t, avatarCID, stored) + + // The follow-up edit carries no avatar at all. + _, ok = extractBlobCID(mustParse(t, base()).Avatar) + require.False(t, ok, + "extraction must FAIL for an omitted image, because failing is what makes the "+ + "consumer's `if ok` skip the assignment and keep the stored CID. If this ever "+ + "returned ok with an empty string, every display-name-only update would blank "+ + "the community's avatar") + }) + + t.Run("a picture that is not an object fails the whole record", func(t *testing.T) { + t.Parallel() + // Worth pinning because it is the one malformed-image case that is NOT + // tolerated: a bare string where the lexicon says object cannot unmarshal + // into map[string]interface{}, so parseCommunityProfile rejects the + // profile outright — permanently, taking the community's name and + // description down with the picture. Tolerating it would be defensible; + // what is not defensible is not knowing which way it goes. + record := base() + record["avatar"] = avatarCID + + _, err := parseCommunityProfile(record) + require.Error(t, err) + assert.ErrorIs(t, err, ErrPermanentEvent) + }) +} + +// mustParse decodes a community profile record or fails the test. +func mustParse(t *testing.T, record map[string]interface{}) *CommunityProfile { + t.Helper() + profile, err := parseCommunityProfile(record) + require.NoError(t, err) + return profile +} + +func TestCommunityConsumer_IgnoresUnrelatedCollections(t *testing.T) { + t.Parallel() + + // The consumer subscribes to three collections and shares its feed with + // every other consumer's traffic, so the common case by volume is an event + // it must do nothing with. A nil repository is the assertion: anything that + // reached a repo call from here would panic rather than pass. + c := NewCommunityEventConsumer(nil, "did:web:test.local", true, nil) + ctx := context.Background() + + for _, collection := range []string{ + "social.coves.community.post", + "social.coves.actor.profile", + "social.coves.feed.vote", + "app.bsky.feed.post", + } { + require.NoErrorf(t, c.HandleEvent(ctx, taxonomyEvent( + "did:plc:somebody", collection, "create", "rk1", map[string]interface{}{"foo": "bar"})), + "an event for %s must be ignored, not handled", collection) + } + + // Non-commit kinds too: identity and account events arrive on this feed + // regardless of wantedCollections. + require.NoError(t, c.HandleEvent(ctx, &JetstreamEvent{Kind: "identity", Did: "did:plc:somebody"})) + require.NoError(t, c.HandleEvent(ctx, &JetstreamEvent{Kind: "account", Did: "did:plc:somebody"})) + require.NoError(t, c.HandleEvent(ctx, &JetstreamEvent{Kind: "commit", Did: "did:plc:somebody"}), + "a commit event with no commit body must not dereference it") +} + +func TestCommunityConsumer_SubscriptionAndBlockIgnoreUpdates(t *testing.T) { + t.Parallel() + + // Subscriptions and blocks are create-or-delete records: there is nothing in + // one to edit except contentVisibility, and no client produces an update. + // The consumer therefore handles create and delete and logs anything else, + // which is worth pinning because the failure mode is silent — an update + // operation the consumer decided to ignore looks exactly like an event that + // never arrived. + // + // It is also the shape of a real defect in a neighbouring consumer: the vote + // consumer drops update commits the same way, and there a client CAN + // legitimately produce one (see tests/e2e/vote_contract_test.go). The + // difference is worth being able to point at. + c := NewCommunityEventConsumer(nil, "did:web:test.local", true, nil) + ctx := context.Background() + + for _, collection := range []string{ + "social.coves.community.subscription", + "social.coves.community.block", + } { + require.NoErrorf(t, c.HandleEvent(ctx, taxonomyEvent( + "did:plc:somebody", collection, "update", "rk1", + map[string]interface{}{"subject": "did:plc:community", "contentVisibility": float64(2)})), + "an update on %s must be ignored without reaching the repository (a nil repo here "+ + "means anything that did would panic)", collection) + } +} diff --git a/internal/atproto/jetstream/user_consumer_test.go b/internal/atproto/jetstream/user_consumer_test.go index d5ad732..422e64c 100644 --- a/internal/atproto/jetstream/user_consumer_test.go +++ b/internal/atproto/jetstream/user_consumer_test.go @@ -7,6 +7,9 @@ import ( "errors" "testing" "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // mockUserService is a test double for users.UserService @@ -845,3 +848,118 @@ func TestExtractBlobCID(t *testing.T) { } }) } + +// fakeSessionHandleUpdater records the OAuth-session fan-out the identity path +// triggers, and can fail on demand. +type fakeSessionHandleUpdater struct { + calls []struct{ did, handle string } + err error +} + +func (f *fakeSessionHandleUpdater) UpdateHandleByDID(_ context.Context, did, newHandle string) (int64, error) { + f.calls = append(f.calls, struct{ did, handle string }{did, newHandle}) + if f.err != nil { + return 0, f.err + } + return int64(len(f.calls)), nil +} + +// TestUserConsumer_IdentityEvent_SyncsSessionHandles covers the consumer's half +// of the handle-rename fan-out: that an identity event for a known user whose +// handle really changed reaches SessionHandleUpdater, and that nothing else +// does. +// +// The store's half — which sessions the fan-out touches — is +// TestPostgresOAuthStore_UpdateHandleByDID in internal/atproto/oauth. Together +// they replace tests/integration/oauth_session_handle_sync_test.go, which +// proved both against real Postgres in 370 lines and paid for a live websocket +// dial and a vacuous three-t.Log "E2E" test function to do it. +// +// Kept as a unit test rather than folded into the store's: the interesting +// cases here are the ones where the updater must NOT be called, and those are +// invisible when the collaborator is real. +func TestUserConsumer_IdentityEvent_SyncsSessionHandles(t *testing.T) { + const did = "did:plc:identitysyncuser" + + identityEvent := func(handle string) *JetstreamEvent { + return &JetstreamEvent{Kind: "identity", Did: did, Identity: &IdentityEvent{Did: did, Handle: handle}} + } + + newConsumer := func(currentHandle string) (*UserEventConsumer, *fakeSessionHandleUpdater, *mockUserService) { + service := newMockUserService() + service.users[did] = &users.User{DID: did, Handle: currentHandle} + updater := &fakeSessionHandleUpdater{} + return NewUserEventConsumer(service, &mockIdentityResolverForUser{}, + WithSessionHandleUpdater(updater)), updater, service + } + + t.Run("a real rename fans out to the sessions", func(t *testing.T) { + consumer, updater, _ := newConsumer("old.example.com") + + require.NoError(t, consumer.HandleEvent(context.Background(), identityEvent("new.example.com"))) + + require.Len(t, updater.calls, 1, + "a handle change must reach the OAuth sessions, or every device the user is signed "+ + "in on keeps showing the old handle until its session expires") + assert.Equal(t, did, updater.calls[0].did) + assert.Equal(t, "new.example.com", updater.calls[0].handle, + "the sessions must be given the NEW handle, not the one they already hold") + }) + + t.Run("an unchanged handle does not touch the sessions", func(t *testing.T) { + // Identity events are re-emitted for reasons other than renames (a + // rotation key change, a PDS migration), and they arrive on an + // unfiltered stream. Writing every session row on each of them would be + // a steady stream of pointless UPDATEs against a hot table. + consumer, updater, _ := newConsumer("same.example.com") + + require.NoError(t, consumer.HandleEvent(context.Background(), identityEvent("same.example.com"))) + assert.Empty(t, updater.calls) + }) + + t.Run("an unknown user does not touch the sessions", func(t *testing.T) { + // The consumer indexes only identities it has seen ("this prevents us + // from indexing millions of Bluesky users we don't care about"), and the + // session store cannot hold a session for one of them anyway. + service := newMockUserService() + updater := &fakeSessionHandleUpdater{} + consumer := NewUserEventConsumer(service, &mockIdentityResolverForUser{}, + WithSessionHandleUpdater(updater)) + + require.NoError(t, consumer.HandleEvent(context.Background(), + &JetstreamEvent{Kind: "identity", Did: "did:plc:strangernobodyknows", + Identity: &IdentityEvent{Did: "did:plc:strangernobodyknows", Handle: "stranger.example.com"}})) + assert.Empty(t, updater.calls) + }) + + t.Run("a failed fan-out is logged, not returned", func(t *testing.T) { + // PINS A DELIBERATE SILENT FAILURE, which is worth doing precisely + // because silent failures are usually bugs and this one is a choice. + // + // The users row has already been updated and the identity cache already + // purged by the time the fan-out runs. Returning the error here would + // dead-letter the event and, on redrive, re-run those two steps for a + // rename that has already been applied — to fix a stale string in a + // session row that expires on its own. The cost of the failure is a user + // seeing their old handle in one client; the cost of treating it as + // fatal is a consumer that stops. + consumer, updater, _ := newConsumer("old.example.com") + updater.err = errors.New("the session store is unreachable") + + require.NoError(t, consumer.HandleEvent(context.Background(), identityEvent("new.example.com")), + "a session-store failure must not fail the identity event: the handle change itself "+ + "has already been applied and a redrive would only repeat it") + require.Len(t, updater.calls, 1, "the fan-out was attempted") + }) + + t.Run("a consumer with no updater configured does not panic", func(t *testing.T) { + // The option is optional, and a nil interface value dereferenced here + // would take down the whole users consumer on the first rename anybody + // performed. + service := newMockUserService() + service.users[did] = &users.User{DID: did, Handle: "old.example.com"} + consumer := NewUserEventConsumer(service, &mockIdentityResolverForUser{}) + + require.NoError(t, consumer.HandleEvent(context.Background(), identityEvent("new.example.com"))) + }) +} diff --git a/internal/atproto/jetstream/vote_consumer.go b/internal/atproto/jetstream/vote_consumer.go index abc8a56..6370f75 100644 --- a/internal/atproto/jetstream/vote_consumer.go +++ b/internal/atproto/jetstream/vote_consumer.go @@ -480,7 +480,26 @@ func (c *VoteEventConsumer) indexVoteAndUpdateCounts(ctx context.Context, vote * return false, fmt.Errorf("failed to check update result: %w", err) } - // If subject doesn't exist or is deleted, that's OK (vote still indexed) + // KNOWN DEFECT — zero rows here is NOT OK, which is what this comment used + // to say. See + // ~/Code/claude-skills/issues/2026-07-29-vote-before-subject-lost-then-subtracts.md. + // + // Two distinct cases reach this branch and only one of them is benign: + // + // - the subject was DELETED. Nothing to count; the vote row is harmless. + // - the subject has NOT BEEN INDEXED YET. A vote and its subject always + // live in different repos and Jetstream parallelises across repos, so + // this is ordinary, not exotic. The vote is then counted by nobody, + // forever — the post consumer INSERTs fresh zeroed counters when the + // subject finally arrives and never consults this table. And because the + // row below is live, deleteVote will later decrement the subject for it, + // subtracting a vote it never added. + // + // The fix is a must-exist gate on the subject returning a TRANSIENT error + // (as post_consumer.go and createSubscription already do) so the redrive + // succeeds once the subject lands. Pinned meanwhile by + // TestVoteOutOfOrderIsLostAndSubtracts (tests/e2e/vote_contract_test.go), + // which fails when this is fixed. if rowsAffected == 0 { log.Printf("Warning: Vote subject not found or deleted: %s (vote indexed anyway)", vote.SubjectURI) } diff --git a/internal/atproto/oauth/store_test.go b/internal/atproto/oauth/store_test.go index 306b543..6f72df0 100644 --- a/internal/atproto/oauth/store_test.go +++ b/internal/atproto/oauth/store_test.go @@ -490,3 +490,119 @@ func TestPostgresOAuthStore_MultipleSessions(t *testing.T) { _, err = store.GetSession(ctx, did, "mobile_app") assert.NoError(t, err) } + +// TestPostgresOAuthStore_UpdateHandleByDID covers the fan-out that keeps signed-in +// sessions honest when a user renames themselves. +// +// # WHY IT MATTERS AND WHY IT HAD NO TEST +// +// A handle is mutable. When one changes, the identity event reaches the user +// consumer, which updates the users row and then calls this method so that +// every device the person is signed in on stops showing the old name. Nothing +// else revisits a session row, so a session missed here shows the stale handle +// until it expires — days, on the default TTL. +// +// UpdateHandleByDID had zero direct coverage before this. What coverage existed +// was tests/integration/oauth_session_handle_sync_test.go, which drove the +// consumer and then read the column back with raw SQL; it is deleted with this +// commit, having also carried a websocket-dialling second test function whose +// entire body was three t.Logs. The two behaviours worth keeping from it are +// this test (the store's fan-out) and the consumer's call into it +// (TestUserConsumer_IdentityEvent_SyncsSessionHandles, in +// internal/atproto/jetstream). +// +// The three properties are all about SCOPE, because a fan-out's failure modes +// are all "too many" or "too few": every live session for this DID, no session +// belonging to anyone else, and — the clause nothing tested and the one a +// refactor would most easily drop — no EXPIRED session, which the query +// excludes with `expires_at > NOW()`. +func TestPostgresOAuthStore_UpdateHandleByDID(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + + // The concrete type, because UpdateHandleByDID is ours rather than part of + // indigo's ClientAuthStore interface: it exists for the identity-event path + // and is reached through jetstream.SessionHandleUpdater, not through the + // OAuth machinery. + store := NewPostgresOAuthStore(db, 0).(*PostgresOAuthStore) + ctx := context.Background() + + renamed, err := syntax.ParseDID("did:plc:handlesyncrenamed") + require.NoError(t, err) + bystander, err := syntax.ParseDID("did:plc:handlesyncbystandr") + require.NoError(t, err) + + session := func(did syntax.DID, id string) oauth.ClientSessionData { + return oauth.ClientSessionData{ + AccountDID: did, + SessionID: id, + HostURL: "https://pds.example.com", + AuthServerURL: "https://auth.example.com", + Scopes: []string{"atproto"}, + AccessToken: "at_" + id, + RefreshToken: "rt_" + id, + DPoPPrivateKeyMultibase: "z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", + } + } + + // Three live sessions for the renamed account — a browser, a phone and a + // tablet is the ordinary case, and "all of them" is the whole point. + for _, id := range []string{"browser", "phone", "tablet"} { + require.NoError(t, store.SaveSession(ctx, session(renamed, id))) + } + // One that has already lapsed. SaveSession always writes a future expiry, so + // it is backdated directly: the store has no API for creating an expired + // session, and this clause is only observable against one. + require.NoError(t, store.SaveSession(ctx, session(renamed, "lapsed"))) + _, err = db.ExecContext(ctx, + `UPDATE oauth_sessions SET expires_at = NOW() - INTERVAL '1 hour' WHERE did = $1 AND session_id = $2`, + renamed.String(), "lapsed") + require.NoError(t, err) + + // And somebody else, signed in throughout. + require.NoError(t, store.SaveSession(ctx, session(bystander, "browser"))) + + const newHandle = "renamed.example.com" + updated, err := store.UpdateHandleByDID(ctx, renamed.String(), newHandle) + require.NoError(t, err) + assert.EqualValues(t, 3, updated, + "every LIVE session for the renamed DID must be updated, and only those: three live, "+ + "one lapsed, one belonging to somebody else") + + handleOf := func(did syntax.DID, sessionID string) string { + t.Helper() + var handle string + require.NoError(t, db.QueryRowContext(ctx, + `SELECT handle FROM oauth_sessions WHERE did = $1 AND session_id = $2`, + did.String(), sessionID).Scan(&handle)) + return handle + } + + for _, id := range []string{"browser", "phone", "tablet"} { + assert.Equalf(t, newHandle, handleOf(renamed, id), + "the %s session still shows the old handle: a device signed in at rename time keeps "+ + "displaying the previous name until its session expires", id) + } + assert.NotEqual(t, newHandle, handleOf(renamed, "lapsed"), + "an expired session was rewritten. Harmless today, but the query's expires_at clause is "+ + "what keeps this statement's cost proportional to a user's LIVE sessions rather than "+ + "to every session they have ever held") + assert.NotEqual(t, newHandle, handleOf(bystander, "browser"), + "another account's session took the renamed account's handle, which is a session-integrity "+ + "failure and not merely a cosmetic one") +} + +func TestPostgresOAuthStore_UpdateHandleByDID_NoSessions(t *testing.T) { + t.Parallel() + db := testkit.DB(t) + + // A rename for someone who is not signed in anywhere. This is the common + // case in production — most identity events are for accounts with no + // session here at all — so it must be a cheap zero rather than an error the + // consumer has to decide how to treat. + store := NewPostgresOAuthStore(db, 0).(*PostgresOAuthStore) + updated, err := store.UpdateHandleByDID( + context.Background(), "did:plc:handlesyncnosession", "nobody.example.com") + require.NoError(t, err) + assert.EqualValues(t, 0, updated) +} diff --git a/internal/core/communities/service_provisioning_test.go b/internal/core/communities/service_provisioning_test.go index c9a2083..89251b5 100644 --- a/internal/core/communities/service_provisioning_test.go +++ b/internal/core/communities/service_provisioning_test.go @@ -4,6 +4,7 @@ package communities_test import ( "Coves/internal/atproto/pds" + "Coves/internal/core/blobs" "Coves/internal/core/communities" "Coves/internal/db/postgres" "Coves/tests/testkit" @@ -50,8 +51,15 @@ const ( // It is wired the way cmd/server wires it, with one substitution: the PDS // client factory is password auth rather than OAuth/DPoP, so a test can hold a // user session without an authorization-code flow. Everything on the community -// side — the provisioner, the credential storage, the write-forwards — is the -// production path. +// side — the provisioner, the credential storage, the blob uploads, the +// write-forwards — is the production path. +// +// The blob service is REAL (blobs.NewBlobService against the test PDS, exactly +// as cmd/server/wiring.go builds it). It used to be nil here, which was +// invisible for as long as no test uploaded an image: the service's avatar path +// fails closed with "blob service not configured", so a nil one does not +// silently skip the upload, it refuses the request. Passing the real one is +// what lets a test assert on what an avatar becomes. func newCommunityService(t *testing.T) (communities.Service, communities.Repository, *testkit.PDS) { t.Helper() @@ -64,7 +72,7 @@ func newCommunityService(t *testing.T) (communities.Service, communities.Reposit instanceDomain, communities.NewPDSAccountProvisioner(instanceDomain, pdsServer.URL()), testkit.PasswordAuthFactory(pds.NewFromAccessToken), - nil, + blobs.NewBlobService(pdsServer.URL()), ) return service, repo, pdsServer } @@ -130,3 +138,141 @@ func TestService_CreateProvisionsAResolvableAccount(t *testing.T) { "hostedBy is stamped from the instance configuration, never from the request") assert.Equal(t, "did:plc:provisioningtest", record.Value["createdBy"]) } + +// TestService_CreateAndUpdateWriteBlobRefsIntoTheProfileRecord covers the +// community half of the avatar/banner write-forward: that an uploaded image +// reaches the community's PROFILE RECORD in the shape the firehose consumer +// reads back. +// +// # WHY IT IS ASSERTED HERE AND NOT WHERE IT LOOKS LIKE IT BELONGS +// +// The service uploads the blob to the community's PDS and then embeds the +// returned reference in the profile record. Those are two calls, and only the +// second one matters to anybody downstream: a correct upload followed by a +// reference embedded under the wrong key, or flattened to a bare CID string, +// leaves a record that stores fine, indexes fine, and renders no picture. +// jetstream.extractBlobCID declines anything that is not {$type:"blob", +// ref:{$link:...}} — silently, because a malformed image is not worth failing a +// whole profile event over — so there is no error anywhere on that path. +// +// Nothing covered it. tests/integration/community_avatar_e2e_test.go (deleted +// with this commit) came closest and could not see it: it read the AVATAR_CID +// COLUMN back through the repo, which the service writes synchronously itself +// on create, so its assertions held with the consumer switched off entirely — +// the §3.4 false-pass in its purest form. Its own consumer call had the error +// swallowed into a t.Logf with a comment saying the conflict was expected. +// +// The two neighbours this joins: the user-side equivalent is +// internal/api/handlers/user's +// TestUpdateProfileHandler_WritesTheRecordTheConsumerReadsBack, and the +// pipeline proof that a record of this shape becomes a working image URL is +// tests/e2e/community_contract_test.go. +func TestService_CreateAndUpdateWriteBlobRefsIntoTheProfileRecord(t *testing.T) { + t.Parallel() + + service, _, pdsServer := newCommunityService(t) + ctx := context.Background() + + name := testkit.UniqueIDWithPrefix(t, "b") + require.LessOrEqualf(t, len("c-"+name), testkit.MaxIDLength, + "the generated community name %q makes a handle label the PDS will refuse", name) + + // Created WITH an avatar and WITHOUT a banner, so the update below exercises + // the nil→value transition as well as the replacement one. + community, err := service.CreateCommunity(ctx, communities.CreateCommunityRequest{ + Name: name, + DisplayName: "Blob Write Forward", + Description: "an avatar that has to survive the trip", + Visibility: "public", + CreatedByDID: "did:plc:blobwriteforward", + AvatarBlob: testkit.TestPNG(64, 64), + AvatarMimeType: "image/png", + }) + require.NoError(t, err) + + account := pdsServer.Login(t, community.Handle, community.PDSPassword) + + // blobCIDOf reads a picture out of the profile record exactly the way + // jetstream.extractBlobCID does, and fails with the reason it would have + // declined rather than with a bare type-assertion panic. + blobCIDOf := func(t *testing.T, record map[string]any, field string) string { + t.Helper() + raw, present := record[field] + require.Truef(t, present, "the profile record has no %q field at all", field) + blob, ok := raw.(map[string]any) + require.Truef(t, ok, "the record's %q is a %T, not an object: a bare CID is not a blob "+ + "ref and the consumer ignores it", field, raw) + require.Equalf(t, "blob", blob["$type"], + "the %q ref must carry $type \"blob\" or extractBlobCID declines it", field) + ref, ok := blob["ref"].(map[string]any) + require.Truef(t, ok, "the %q ref has no `ref` object holding $link", field) + link, ok := ref["$link"].(string) + require.Truef(t, ok, "the %q ref's $link is a %T, not a string", field, ref["$link"]) + require.NotEmptyf(t, link, "the %q ref's $link is empty", field) + return link + } + + created := account.GetRecord(t, "social.coves.community.profile", "self") + avatarCID := blobCIDOf(t, created.Value, "avatar") + assert.Equal(t, avatarCID, community.AvatarCID, + "the AppView row and the PDS record must name the same avatar blob; if they diverge, "+ + "the hydrated URL points at bytes the record does not reference") + _, hasBanner := created.Value["banner"] + assert.False(t, hasBanner, + "a community created without a banner emitted a banner key: the consumer reads a "+ + "present-but-unparseable value differently from an absent one") + + // ---- update: replace the avatar and add a banner ------------------------ + displayName := "Blob Write Forward v2" + updated, err := service.UpdateCommunity(ctx, communities.UpdateCommunityRequest{ + CommunityDID: community.DID, + UpdatedByDID: "did:plc:blobwriteforward", + DisplayName: &displayName, + AvatarBlob: testkit.TestPNG(48, 48), + AvatarMimeType: "image/png", + BannerBlob: testkit.TestJPEG(96, 32), + BannerMimeType: "image/jpeg", + }) + require.NoError(t, err) + + afterUpdate := account.GetRecord(t, "social.coves.community.profile", "self") + newAvatarCID := blobCIDOf(t, afterUpdate.Value, "avatar") + bannerCID := blobCIDOf(t, afterUpdate.Value, "banner") + + assert.NotEqual(t, avatarCID, newAvatarCID, + "different image bytes must produce a different CID in the record: an unchanged CID here "+ + "means the update re-embedded the old reference and the community keeps its old picture") + assert.NotEqual(t, newAvatarCID, bannerCID, + "the avatar and banner must reference different blobs; the same CID in both means one "+ + "upload's result was embedded twice") + + // THE CREATE/UPDATE ASYMMETRY, which this test discovered by asserting the + // wrong thing first and is worth stating rather than quietly accommodating. + // + // CreateCommunity writes the AppView row SYNCHRONOUSLY (service.go's + // repo.Create), which is why the create-side assertion above could compare + // community.AvatarCID against the record. UpdateCommunity does NOT: it + // uploads, writes the record to the PDS, and returns — leaving the row to + // the firehose consumer. So the value returned here still carries the + // PREVIOUS avatar and no banner at all, and that is correct behaviour, not + // a stale read. + // + // It is also exactly the §3.4 distinction the whole test tier is built + // around: the create path can be verified end-to-end in-process and + // therefore proves nothing about the pipeline, while the update path can + // only be completed by the consumer. The update's visible effect is + // asserted where it becomes visible — tests/e2e/community_contract_test.go, + // through social.coves.community.get, after the firehose has delivered it. + assert.Equal(t, avatarCID, updated.AvatarCID, + "UpdateCommunity does not index; the returned row should still show the pre-update "+ + "avatar. If this now matches the NEW CID, the update path started writing Postgres "+ + "synchronously — which would make the community contract's update step a false pass, "+ + "because it could then be satisfied with the consumer dead") + assert.Empty(t, updated.BannerCID, + "the banner added by this update reaches the row through the consumer, not through the "+ + "service; a non-empty value here means the same synchronous-indexing change") + + assert.Equal(t, "image/jpeg", afterUpdate.Value["banner"].(map[string]any)["mimeType"], + "the blob's declared MIME type must survive into the record: the image proxy serves it "+ + "back as the response content type") +} diff --git a/internal/core/votes/service_impl.go b/internal/core/votes/service_impl.go index dda538a..9b08260 100644 --- a/internal/core/votes/service_impl.go +++ b/internal/core/votes/service_impl.go @@ -118,9 +118,23 @@ func (s *voteService) CreateVote(ctx context.Context, session *oauth.ClientSessi } // Note: We intentionally don't validate subject existence here. - // The vote record goes to the user's PDS regardless. The Jetstream consumer - // handles orphaned votes correctly by only updating counts for non-deleted subjects. - // This avoids race conditions and eventual consistency issues. + // The vote record goes to the user's PDS regardless, which avoids race + // conditions and eventual consistency issues on the write path. + // + // KNOWN DEFECT — the consumer does NOT handle the resulting orphan + // correctly, and this comment used to claim it did. See + // ~/Code/claude-skills/issues/2026-07-29-vote-before-subject-lost-then-subtracts.md: + // a vote indexed before its subject is never counted (the count UPDATE + // matches zero rows and is only logged, and nothing recomputes afterwards), + // and worse, withdrawing that orphan later DECREMENTS the subject — + // subtracting a vote it never added and taking a different voter's with it. + // Pinned end-to-end by TestVoteOutOfOrderIsLostAndSubtracts + // (tests/e2e/vote_contract_test.go). + // + // Leaving this write path as-is is still probably right; the fix belongs in + // the consumer (a must-exist gate returning a TRANSIENT error, matching + // post_consumer.go and createSubscription, so the redrive succeeds once the + // subject lands). // Check for existing vote using cache with PDS fallback // First check populates cache from PDS, subsequent checks are O(1) lookups diff --git a/internal/core/votes/service_impl_test.go b/internal/core/votes/service_impl_test.go new file mode 100644 index 0000000..a0ffcfa --- /dev/null +++ b/internal/core/votes/service_impl_test.go @@ -0,0 +1,446 @@ +package votes_test + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/bluesky-social/indigo/atproto/auth/oauth" + "github.com/bluesky-social/indigo/atproto/syntax" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "Coves/internal/atproto/pds" + "Coves/internal/core/blobs" + "Coves/internal/core/votes" +) + +// The vote service's write path: what a tap on an arrow actually does to the +// voter's repository. +// +// # WHY THIS FILE EXISTS, AND WHY IT IS NOT AN INTEGRATION TEST +// +// It replaces the write-path half of tests/integration/vote_e2e_test.go, whose +// four XRPC tests each stood up a real PDS account, a real chi router and a +// fake OAuth middleware in order to assert — in the two places they asserted +// anything about the service at all — that a POST returned 200. The decisions +// this file covers are the ones those tests described in their comments and +// then simulated instead of observing: +// +// - TestVoteE2E_ToggleSameDirection asserted the second POST was 200, and +// produced the AppView state change by hand-writing the delete event it +// expected the service to have caused. Whether the service deleted anything +// was never checked. +// - TestVoteE2E_ToggleDifferentDirection went further: its comment stated that +// the service deletes the old record and creates a new one under a fresh +// rkey, and then the test simulated exactly that. It did not even assert +// that the returned URI differed from the first. +// +// Both decisions live entirely in voteService.CreateVote and are visible from a +// fake pds.Client, which is what this file uses. No PDS, no Postgres, no +// router: this is T0, and it runs in milliseconds. +// +// The pipeline half of that file — a real record reaching the AppView's own +// consumers and moving a post's counts — is tests/e2e/vote_contract_test.go. + +// fakePDS is a pds.Client that records what the service asked it to do. +// +// Only the four methods the vote service calls do anything; the rest satisfy +// the interface and fail loudly if the service starts using them, because a new +// call to the real PDS from this path is exactly the kind of change these tests +// should notice. +type fakePDS struct { + t *testing.T + did string + + // records is the repo's contents, keyed by rkey. + records map[string]votes.VoteRecord + + // ops is ONE ordered log of every write, because the interesting claim in + // this file is a claim about SEQUENCE: a direction change must delete the + // old record before creating the new one, or there is a window in which the + // repo holds two live votes for one subject and the consumer's stale-vote + // cleanup decides arbitrarily which survives. + // + // It was two slices (created []string, deleted []string) in the first draft, + // which cannot express interleaving at all — the ordering comment sat above + // three final-state assertions that would pass just as happily with the + // calls reversed. Review caught it. One log, so the assertion can be the one + // the comment claims. + ops []voteOp + + // createErr and deleteErr let a test drive the failure branches. + createErr error + deleteErr error +} + +// voteOp is one write against the fake repo. +type voteOp struct { + kind string // "create" or "delete" + rkey string +} + +func (o voteOp) String() string { return o.kind + "(" + o.rkey + ")" } + +// creates returns the rkeys created, in order. +func (f *fakePDS) creates() []string { return f.rkeysOf("create") } + +// deletes returns the rkeys deleted, in order. +func (f *fakePDS) deletes() []string { return f.rkeysOf("delete") } + +func (f *fakePDS) rkeysOf(kind string) []string { + var out []string + for _, op := range f.ops { + if op.kind == kind { + out = append(out, op.rkey) + } + } + return out +} + +func newFakePDS(t *testing.T, did string) *fakePDS { + t.Helper() + return &fakePDS{t: t, did: did, records: map[string]votes.VoteRecord{}} +} + +func (f *fakePDS) CreateRecord(_ context.Context, collection, rkey string, record any) (string, string, error) { + if f.createErr != nil { + return "", "", f.createErr + } + require.Equal(f.t, "social.coves.feed.vote", collection, + "the vote service wrote to a collection other than the vote one") + vote, ok := record.(votes.VoteRecord) + require.Truef(f.t, ok, "the vote service passed a %T to CreateRecord rather than a votes.VoteRecord", record) + f.records[rkey] = vote + f.ops = append(f.ops, voteOp{kind: "create", rkey: rkey}) + return "at://" + f.did + "/" + collection + "/" + rkey, "bafycid" + rkey, nil +} + +func (f *fakePDS) DeleteRecord(_ context.Context, collection, rkey string) error { + if f.deleteErr != nil { + return f.deleteErr + } + require.Equal(f.t, "social.coves.feed.vote", collection) + delete(f.records, rkey) + f.ops = append(f.ops, voteOp{kind: "delete", rkey: rkey}) + return nil +} + +func (f *fakePDS) ListRecords(_ context.Context, collection string, _ int, cursor string) (*pds.ListRecordsResponse, error) { + require.Equal(f.t, "social.coves.feed.vote", collection) + require.Empty(f.t, cursor, "the fake repo is a single page; a cursor means the service is paginating past the end") + out := &pds.ListRecordsResponse{} + for rkey, rec := range f.records { + out.Records = append(out.Records, pds.RecordEntry{ + URI: "at://" + f.did + "/" + collection + "/" + rkey, + CID: "bafycid" + rkey, + Value: map[string]any{ + "$type": rec.Type, + "direction": rec.Direction, + "subject": map[string]any{"uri": rec.Subject.URI, "cid": rec.Subject.CID}, + "createdAt": rec.CreatedAt, + }, + }) + } + return out, nil +} + +func (f *fakePDS) DID() string { return f.did } +func (f *fakePDS) HostURL() string { return "https://pds.invalid" } + +func (f *fakePDS) GetRecord(context.Context, string, string) (*pds.RecordResponse, error) { + f.t.Fatal("the vote service called GetRecord, which it did not before: add coverage for the new path") + return nil, nil +} + +func (f *fakePDS) PutRecord(context.Context, string, string, any, string) (string, string, error) { + f.t.Fatal("the vote service called PutRecord. Votes are created and deleted, never updated in " + + "place — and the vote consumer ignores update commits entirely (see " + + "tests/e2e/vote_contract_test.go), so a vote written this way would never reach the index") + return "", "", nil +} + +func (f *fakePDS) UploadBlob(context.Context, []byte, string) (*blobs.BlobRef, error) { + f.t.Fatal("the vote service uploaded a blob, which makes no sense for a vote") + return nil, nil +} + +// voter is the session the service votes on behalf of. +func voter(t *testing.T, did string) *oauth.ClientSessionData { + t.Helper() + parsed, err := syntax.ParseDID(did) + require.NoError(t, err) + return &oauth.ClientSessionData{AccountDID: parsed, AccessToken: "test-access-token"} +} + +// newService wires the service to a fake PDS. +// +// The cache is nil on purpose in most tests: with a cache the service answers +// "does a vote already exist" from memory, and these tests are about the +// decision made from that answer, not about the cache. The one test that cares +// which source the answer came from builds its own. +func newService(t *testing.T, fake *fakePDS, cache *votes.VoteCache) votes.Service { + t.Helper() + return votes.NewServiceWithPDSFactory(nil, cache, nil, + func(context.Context, *oauth.ClientSessionData) (pds.Client, error) { return fake, nil }) +} + +const ( + testVoterDID = "did:plc:aaaaaaaaaaaaaaaaaaaaaaaa" + testSubject = "at://did:plc:bbbbbbbbbbbbbbbbbbbbbbbb/social.coves.community.post/3kabc" + testSubjectCI = "bafyreiasubjectcid" +) + +func subject() votes.StrongRef { + return votes.StrongRef{URI: testSubject, CID: testSubjectCI} +} + +func TestCreateVote_FirstVoteWritesTheRecord(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + + resp, err := svc.CreateVote(context.Background(), voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + + require.Len(t, fake.ops, 1, "a first vote is one CreateRecord and nothing else") + require.Equal(t, "create", fake.ops[0].kind) + + rkey := fake.ops[0].rkey + require.Equal(t, "at://"+testVoterDID+"/social.coves.feed.vote/"+rkey, resp.URI, + "the response must name the record the service actually wrote") + require.NotEmpty(t, resp.CID) + + // The record's SHAPE is the wire contract with the consumer: the same fields + // internal/atproto/jetstream's parseVoteRecord reads back out. Asserted here + // because it is the one place both halves are in scope — the ingestion + // contract writes its own records and so cannot check what the service + // writes. + written := fake.records[rkey] + assert.Equal(t, "social.coves.feed.vote", written.Type) + assert.Equal(t, "up", written.Direction) + assert.Equal(t, testSubject, written.Subject.URI) + assert.Equal(t, testSubjectCI, written.Subject.CID, + "the subject CID makes the reference STRONG; dropping it would let a vote follow a post "+ + "through an edit") + assert.NotEmpty(t, written.CreatedAt) + + // The rkey is a TID, which is what makes vote records sortable and what the + // consumer builds the vote's URI from. + _, err = syntax.ParseTID(rkey) + assert.NoErrorf(t, err, "the vote service used %q as an rkey, which is not a TID", rkey) +} + +func TestCreateVote_SameDirectionTogglesOff(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + ctx := context.Background() + + first, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + existing := fake.creates()[0] + + // The same tap again. This is the decision tests/integration's + // TestVoteE2E_ToggleSameDirection was named after and never checked. + second, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + + require.Equal(t, []string{existing}, fake.deletes(), + "re-voting the same direction must DELETE the existing record from the voter's repo") + require.Len(t, fake.creates(), 1, + "toggling off must not also create a replacement record: the repo would then hold two "+ + "votes for one subject, and the consumer's stale-vote cleanup would silently pick one") + require.Empty(t, fake.records, "the voter's repo has no vote for this subject any more") + + // The empty response is how the handler tells a client the vote was + // withdrawn rather than recorded — there is no separate status for it. + require.Equal(t, "", second.URI, "a toggled-off vote answers with an empty URI") + require.Equal(t, "", second.CID) + require.NotEqual(t, first.URI, second.URI) +} + +func TestCreateVote_DifferentDirectionReplacesUnderANewRKey(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + ctx := context.Background() + + up, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + oldRKey := fake.creates()[0] + + down, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "down"}) + require.NoError(t, err) + + require.Equal(t, []string{oldRKey}, fake.deletes(), + "changing direction must remove the old record, not leave two votes in the repo") + require.Len(t, fake.creates(), 2) + newRKey := fake.creates()[1] + + // THE LOAD-BEARING CLAIM, and the one the deleted integration test asserted + // nowhere: the replacement is a NEW record under a NEW key, not an update of + // the old one. It matters beyond tidiness — the vote consumer handles create + // and delete commits only, so a same-rkey update would never reach the + // AppView at all (pinned from the outside in tests/e2e/vote_contract_test.go). + require.NotEqual(t, oldRKey, newRKey, + "the service reused the old rkey. A putRecord-shaped update is invisible to the vote "+ + "consumer, so the AppView would keep serving the previous direction forever") + require.NotEqual(t, up.URI, down.URI) + require.Equal(t, "at://"+testVoterDID+"/social.coves.feed.vote/"+newRKey, down.URI) + + // ORDERING, asserted rather than described. The old record must be deleted + // BEFORE the new one is created: the reverse leaves a window in which the + // repo holds two live votes for one subject, and if the firehose carries + // them in that order the consumer's stale-vote cleanup picks a winner by + // whichever arrives second rather than by what the user chose. + // + // Stated as the whole op log rather than as two positions, so a third write + // appearing between them also fails. + require.Equal(t, []voteOp{ + {kind: "create", rkey: oldRKey}, + {kind: "delete", rkey: oldRKey}, + {kind: "create", rkey: newRKey}, + }, fake.ops, + "a direction change must be exactly delete-old-then-create-new against the voter's repo") + + require.Equal(t, "down", fake.records[newRKey].Direction) + require.Len(t, fake.records, 1, "exactly one vote for this subject survives") +} + +func TestCreateVote_LeavesOtherSubjectsAlone(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + ctx := context.Background() + + other := votes.StrongRef{ + URI: "at://did:plc:bbbbbbbbbbbbbbbbbbbbbbbb/social.coves.community.post/3kother", + CID: "bafyreiotherc", + } + _, err := svc.CreateVote(ctx, voter(t, testVoterDID), votes.CreateVoteRequest{Subject: other, Direction: "up"}) + require.NoError(t, err) + _, err = svc.CreateVote(ctx, voter(t, testVoterDID), votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + + // Toggling this subject off must not disturb the vote on the other one. The + // lookup is by subject URI, and a lookup that ignored it would pass every + // other test in this file. + _, err = svc.CreateVote(ctx, voter(t, testVoterDID), votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + + require.Len(t, fake.records, 1) + for _, rec := range fake.records { + require.Equal(t, other.URI, rec.Subject.URI, + "withdrawing a vote on one post removed the voter's vote on a different one") + } +} + +func TestDeleteVote(t *testing.T) { + t.Parallel() + + t.Run("removes the record from the voter's repo", func(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + ctx := context.Background() + + _, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + rkey := fake.creates()[0] + + require.NoError(t, svc.DeleteVote(ctx, voter(t, testVoterDID), + votes.DeleteVoteRequest{Subject: subject()})) + require.Equal(t, []string{rkey}, fake.deletes()) + require.Empty(t, fake.records) + }) + + t.Run("is not found when there is no vote to delete", func(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + + err := svc.DeleteVote(context.Background(), voter(t, testVoterDID), + votes.DeleteVoteRequest{Subject: subject()}) + require.ErrorIs(t, err, votes.ErrVoteNotFound, + "deleting a vote that was never cast must be distinguishable from deleting one that was: "+ + "the handler maps this to a 404 and a silent success would tell a client its "+ + "un-vote worked when nothing happened") + require.Empty(t, fake.ops) + }) +} + +func TestCreateVote_RejectsMalformedInput(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + req votes.CreateVoteRequest + wantErr error + }{ + {"sideways direction", votes.CreateVoteRequest{Subject: subject(), Direction: "sideways"}, votes.ErrInvalidDirection}, + {"empty direction", votes.CreateVoteRequest{Subject: subject(), Direction: ""}, votes.ErrInvalidDirection}, + {"empty subject URI", votes.CreateVoteRequest{ + Subject: votes.StrongRef{CID: testSubjectCI}, Direction: "up"}, votes.ErrInvalidSubject}, + {"subject URI that is not an AT-URI", votes.CreateVoteRequest{ + Subject: votes.StrongRef{URI: "https://example.com/post/1", CID: testSubjectCI}, + Direction: "up"}, votes.ErrInvalidSubject}, + {"subject with no CID", votes.CreateVoteRequest{ + Subject: votes.StrongRef{URI: testSubject}, Direction: "up"}, votes.ErrInvalidSubject}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + + _, err := svc.CreateVote(context.Background(), voter(t, testVoterDID), tc.req) + require.ErrorIs(t, err, tc.wantErr) + require.Empty(t, fake.ops, + "a rejected vote must not reach the PDS: validation runs before any repo write") + }) + } +} + +func TestCreateVote_PDSFailureIsReportedRatherThanSwallowed(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + fake.createErr = errors.New("pds is down") + svc := newService(t, fake, nil) + + _, err := svc.CreateVote(context.Background(), voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.Error(t, err, "a vote the PDS refused must not be reported to the client as cast — "+ + "nothing else will ever write that record, so a swallowed error is a vote that silently "+ + "never existed") + require.Contains(t, strings.ToLower(err.Error()), "pds is down", + "the underlying failure must survive into the error the handler maps") +} + +func TestCreateVote_FailedToggleOffDoesNotReportSuccess(t *testing.T) { + t.Parallel() + fake := newFakePDS(t, testVoterDID) + svc := newService(t, fake, nil) + ctx := context.Background() + + _, err := svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.NoError(t, err) + + // The withdrawal fails at the PDS. The dangerous outcome would be the + // service returning the empty "toggled off" response anyway: the client + // would render the arrow as un-pressed while the record — and therefore the + // AppView's count — still says otherwise, with nothing to correct it. + fake.deleteErr = errors.New("pds refused the delete") + _, err = svc.CreateVote(ctx, voter(t, testVoterDID), + votes.CreateVoteRequest{Subject: subject(), Direction: "up"}) + require.Error(t, err) + require.Len(t, fake.records, 1, "the vote is still in the repo, and the caller was told so") + require.Len(t, fake.creates(), 1, "a failed toggle-off must not fall through into creating a second vote") +} diff --git a/loop_state.md b/loop_state.md index ec110ec..0ab715f 100644 --- a/loop_state.md +++ b/loop_state.md @@ -50,8 +50,8 @@ Stop the loop when every task is done, or on any blocked task. | 11 | Contracts: community (community.profile ingestion + API) — strangler: behavior inventory of community_e2e_test.go (1820 LOC) → down-tier T1s → contract → delete old | 4 | S | done | (see git log) | TEMPLATE PROVEN. 22-behavior inventory; 2,168 LOC deleted, +14 net tests; 2/9 serial firehose files gone. Ingestion contract SELF-REGISTERS the community's PDS repo (stronger than arming — no sync write exists; consumer has NO must-know-first gate, verified). FOUND+FILED prod defect: unverifiable handles → handle.invalid UNIQUE squat → federated communities silently dropped; second symptom pds_url permanently empty → BridgeTrust denies bridged votes (issue extended). STANDING TIER LIMIT (spec §3.4b amended): sealed sessions mint only in browser OAuth — T2 covers auth boundary + reads; authenticated writes proven at T1; test-only mint = phase-5 pre-work. Review: Codex 1 high (update-handler boundary died — restored w/ 8 tests) + Opus audit 17/20 equal-or-stronger, 3 gaps all closed. make ci GREEN ×2 3496/0 @2:12 | | 12 | Contracts: post (community.post) + post_delete + decompose post god-files | 4 | S | done | (see git log) | post_e2e (662) + post_delete (841) deleted, net −1402 w/ more coverage; serial firehose files 7→5. TWO MORE PROD DEFECTS FILED (tally 4): p1 deleted posts served in full by getComments to anon callers forever (repo missing deleted_at filter); p3 DeletePost idempotency branch unreachable (PDS 400 ≠ ErrNotFound — pinned w/ loud require.Errorf). Spoof negative REWRITTEN intra-repo after Opus traced indigo parallel scheduler (cross-repo FIFO doesn't exist; 5s hold was the real bound; relay would break silently) — mutation-tested. Consumer gates pinned: author-not-found=transient+replay-accepted. Journey flake (2/6 gates, note-[C] starvation) mitigated: 60s/75s ordered waits + starvation-vs-dead tally; real fix task 16. make ci GREEN 3496/0 @2:21 | | 13 | Contracts: comment (community.comment) + comment god-files (1821+1443+1229+999 LOC) | 4 | S | done | (see git log) | comment_e2e (1120) deleted; 4,263 LOC of T1-shaped satellites KEPT deliberately (right call — §3.4 rule 3 breadth). JOURNEY PULLED FORWARD from task 16: worker bisected its own deletion breaking the journey deterministically (serial tests were accidental SPACERS vs the note-[C] storm) → rebuilt as the §3.4 rule-5 saga in tests/e2e (3 actors, 3 repos, 4 read paths; old file was 2-real-of-11-steps, SQL-insert fallbacks, fakecid). Serial firehose files 5→3; ONE subscriber helper left tree-wide (vote_e2e — task 14 kills it). Comment consumer has NO must-exist gates (measured); comments live in AUTHOR repo; delete = placeholder. getComments capped 20/min → withReadCadence(2.5s) + FreshReadQuota + Holds@1s. Defect #5 filed (malformed URI → 500). Reviews: kill-list EMPTY. make ci GREEN ×2 3532/0 @~2:45; e2e 56/0/0; audit 473 | -| 14 | Contracts: vote (feed.vote) + user (actor.profile incl. avatar blob path) + subscription (community.subscription) | 4 | S | in-progress | | vote re-tap idempotency invariant | -| 15 | Contracts: blocks (actor.block + community.block) + aggregators (aggregator.service + aggregator.authorization) | 4 | S | pending | | collections revision-1 inventory MISSED — see spec §3.4a | +| 14 | Contracts: vote (feed.vote) + user (actor.profile incl. avatar blob path) + subscription (community.subscription) | 4 | S | review | | THE LAST HAND-ROLLED SUBSCRIBER IS DEAD (tree-wide dialer count = 1, and it is production connector.go). 5 files / 3,908 LOC deleted; 3 contracts + community-avatar step added; 5 T1/T0 files gained. TWO DEFECTS FILED (tally 7), both spike-confirmed: #6 vote-before-subject is lost AND its later delete SUBTRACTS a real vote (unbounded downward drift, floored at 0); #7 same-rkey putRecord vote flip silently ignored (consumer handles create/delete only) — third-party/federated clients only, our own client does delete+create. Both PINNED, not asserted-as-intended. make ci GREEN x2 3547/0 @3:55; e2e 68/0/0 x2 kept-stack; audit 473->403. REVIEW BATCH (6 items) applied: item-6 verdict CODEX RIGHT (fake's two slices could not express ordering and the comment claimed they did — unified op log, mutation-proven); vacuous Hold dropped, p1 pin hardened with a second Holds + issue names in both pins; GetBinary added to testkit (Get accepts any 2xx and discards the body — a 204 passed the must-200 claim); community banner + nil->value transition + blob write-forward test, which FOUND the create/update indexing asymmetry. Post-review: make ci GREEN 3549/0 @4:06; e2e 68/0/0 | +| 15 | Contracts: blocks (actor.block + community.block) + aggregators (aggregator.service + aggregator.authorization) | 4 | S | in-progress | | collections revision-1 inventory MISSED — see spec §3.4a | | 16 | Reliability suite (§3.4c: cursor resume, replay-once, rev-gate no-resurrection via Holds, dead-letter, 2-feed overlap) + user_journey rebuild + rm -rf tests/integration | 4 ⛩ | F | pending | | FULL PANEL on the whole Phase-4 output | | 17 | Unit-coverage debt A: communities, votes, identity (repo tests for their repos too) | 5 | S | pending | | behavior matrices at T0, repo seams at T1 | | 18 | Unit-coverage debt B: routes, timeline, discover, communityFeeds + remaining untested repos | 5 | S | pending | | | @@ -299,3 +299,90 @@ Stop the loop when every task is done, or on any blocked task. serial files re-packs the schedule — prefer deleting the fragile subscriber over re-spacing (proven). Saga leaves ~1 dead letter/run too (hijack) — task 16 dead-letter accounting. +- **From task 14 (for task 15-16)**: THE SUBSCRIBER ERA IS OVER — no test in + the tree dials a websocket; `test-audit.sh`'s dialer count of 1 is + production `connector.go` and will never reach 0 (task 20 must exempt it, + not chase it). SPIKE FINDINGS worth not re-deriving: vote re-tap has THREE + distinct paths keyed on three different things (same-rkey update = dropped + by HandleEvent's switch; duplicate delivery = rev gate + ON CONFLICT(uri), + reachable only from task 16's reliability suite; new-rkey re-tap = the + (voter_did, subject_uri) stale-vote cleanup — this last one is what a real + client produces, because votes.voteService always deletes and re-creates + under a fresh TID). Deleting an already-superseded vote does NOT + double-decrement. AVATAR OBSERVABLE: getProfile/community.get serve a + HYDRATED URL, never the CID — `{proxy}/img/{preset}/plain/{did}/{cid}` with + IMAGE_PROXY_ENABLED=true in .env.ci, the PDS' com.atproto.sync.getBlob URL + without it. Assert `Contains(url, cid)`, which holds in both modes; pinning + either shape makes the contract a test of one config value. A bogus CID on + that path is a 502, so `AppView.Get(path, nil)` is a real end-to-end blob + assertion. actor.profile DELETE clears the fields and keeps the row + (getProfile still 200s) — the OPPOSITE of community.profile, which hard- + deletes and 404s; the two look alike from outside and getting it backwards + costs a debugging session. Profile updates are PARTIAL: an absent key means + "leave alone", not "clear", so a record is not a snapshot. SUBSCRIPTIONS + have two observables that drift apart on purpose — community.get's + subscriberCount is a STORED COLUMN, actor.getProfile's stats.communityCount + is a live COUNT(*); assert both, it is the cheapest detector for a missed + increment. Duplicate subscribe under a NEW rkey re-points record_uri and + does NOT increment (ON CONFLICT keys on (user_did, community_did), decided + by `xmax = 0`). Subscription FAN-OUT is unreachable at T2: getTimeline is + the only endpoint that joins posts to subscriptions and it is RequireAuth; + getDiscover explicitly does not filter, communityFeed filters by community, + community.list?subscribed=true 401s. Documented in the contract, T1 covers + it (timeline_test.go), Phase-5 mint unlocks it. DEAD LETTERS for task 16's + accounting: an unknown-community subscribe and an invalid-direction vote + each leave ONE retired dead letter (permanent → redrive budget exhausted at + birth); neither is in a contract, deliberately — direction validation is + already at T1 (error_taxonomy_test.go:113) and was not worth a per-run DLQ + row. Steady state after a full tier run is communities/posts/comments = 1 + each, users/aggregators/votes = 0. SCOPE JUDGMENTS: comment_vote_test.go + KEPT as T1 (task-13 rule-3 breadth — it holds the ONLY real-SQL coverage of + GetVoteStateForComments; its one vacuous `if len(...)>0` subtest was + de-vacuumed); community_avatar_e2e_test.go handled HERE not task 15 (its + create-path DB assertions were a §3.4 false-pass — CreateCommunity writes + Postgres synchronously — and splitting the file three ways across tasks + would have left the coverage in limbo). COST: the e2e tier went 65s → 131s; + TestVoteIngestion alone is 29s, of which 25s is five Holds windows, and + every one of them is load-bearing (a "did not double" claim cannot be made + by an eventually-check). make ci 2:45 → 3:55, all of it here. +- **From task 14's review (rules tasks 15-16 inherit)**: PIN PLACEMENT is now a + written rule in tests/e2e/contracts_test.go's package doc, not a per-task + judgment — a pin MAY sit inside a marked contract provided the contract still + passes once the defect is fixed (vote's same-rkey pin qualifies; the + out-of-order one does not and lives in its own unmarked function). Two + obligations either way: name the ISSUE FILE in the assertion message so a red + run says which defect got fixed, and pin the wrong-but-current value with + HOLDS as well as Await — an Await-only pin is satisfied by an ASYNCHRONOUS + fix (reconciliation pass, lazy repair) on its way to the right answer and + then passes forever against corrected code. NORMALIZATION HAZARD, worth a + standing check: two production comments asserted the opposite of what the p1 + pin proves ("the Jetstream consumer handles orphaned votes correctly", + "zero rows is OK") — a pin whose subject is documented as correct behaviour + will be "cleaned up" by the next reader, so replacing those comments with + KNOWN DEFECT notes naming the issue file is part of filing, not optional + polish. TESTKIT: XRPCClient.Get asserts only 2xx and DISCARDS the body, so it + cannot support a "this URL serves an image" claim (a 204 passes) — use + GetBinary, which returns status + content-type + bounded body; the image + assertions want 200 AND non-empty AND image/*, and deliberately do NOT + compare bytes because the proxy re-encodes per preset. FOUND WHILE FIXING: + the communities test harness passed a nil blobService, invisible until a test + uploaded an image (the service fails closed, "blob service not configured"); + it is now the real one, as cmd/server wires it. And CreateCommunity writes + the AppView row SYNCHRONOUSLY while UpdateCommunity does NOT — asserted + explicitly in service_provisioning_test.go, because if update ever starts + indexing synchronously the community contract's update step silently becomes + a false pass. +- **From task 14 (for tasks 15-16, 20)**: MARKER-PIN RULE in + contracts_test.go doc: pins may live inside marked contracts iff the + contract passes once fixed; pins carry issue IDs + "IF THIS FAILED the + defect is FIXED" in assertion strings; production comments adjacent to + pinned defects carry KNOWN DEFECT notes (normalization hazard). + CreateCommunity indexes synchronously, UpdateCommunity does NOT (by + design — it's what makes the update step honest pipeline proof; a + tripwire T1 asserts the asymmetry). requireServesImage = host-check + + 200 + non-empty + image/* via GetBinary (any-2xx Get was a false-200). + Dead-letter steady state after full tier: communities/posts/comments=1 + each, users/aggregators/votes=0 (task 16 accounting). e2e tier 131s, + gate ~4:00 — watch at task 15 but §3.1 budget fine. Task 20: audit + dialer count floor is 1 (production connector.go:318 — exempt, don't + chase). oauth UpdateHandleByDID + expires_at now covered (first time). diff --git a/tests/ci/pending_contracts.txt b/tests/ci/pending_contracts.txt index 8d57565..ee4f20a 100644 --- a/tests/ci/pending_contracts.txt +++ b/tests/ci/pending_contracts.txt @@ -19,17 +19,6 @@ # Task 20 empties this file and flips cmd/contract-manifest to # -allow-pending=false, after which "has a contract" is the only passing state. -social.coves.feed.vote # task 14: vote ingestion contract (the re-tap idempotency invariant lives here) -# -# social.coves.actor.profile carries a trap worth restating where task 14 will -# read it: users.maybeBackfillProfile fetches the profile record straight from -# the PDS in a detached goroutine at signup, so the FIRST profile a contract -# writes can reach the AppView without touching the firehose. Backfill only -# touches a completely empty profile, so the contract must make the row -# non-empty first and assert on a SECOND update. TestPipelineSmoke shows the -# shape; see the RECONCILIATION PATHS hazard in tests/e2e/contracts_test.go. -social.coves.actor.profile # task 14: user ingestion contract, including the avatar blob path; MUST assert a second update on a non-empty profile (backfill false-pass) -social.coves.community.subscription # task 14: subscription ingestion contract social.coves.actor.block # task 15: user-block ingestion contract social.coves.community.block # task 15: community-block ingestion contract social.coves.aggregator.service # task 15: aggregator service-record ingestion contract diff --git a/tests/e2e/community_contract_test.go b/tests/e2e/community_contract_test.go index a2107a6..8ce401e 100644 --- a/tests/e2e/community_contract_test.go +++ b/tests/e2e/community_contract_test.go @@ -162,6 +162,14 @@ type communityView struct { HostedBy string `json:"hostedBy"` Visibility string `json:"visibility"` SubscriberCount int `json:"subscriberCount"` + + // Avatar and Banner are HYDRATED image URLs, not the CIDs the record + // carries — the same shape actor.getProfile serves and for the same reason + // (blobs.HydrateImageURL). user_contract_test.go's opening note traces the + // whole blob path; the only thing these fields add is that communities take + // it too, through a different consumer. + Avatar string `json:"avatar"` + Banner string `json:"banner"` } // Community reads a community from the AppView by any identifier the endpoint @@ -229,6 +237,17 @@ func communityProfile(c provisionedCommunity, creatorDID, displayName, descripti } } +// withCommunityImage attaches a blob reference to a community profile record. +// +// Separate from communityProfile rather than a parameter on it because most +// callers want a community to hang other records on and do not care about +// images; only the ingestion contract exercises the blob path, and it should +// read as the extra step it is. +func withCommunityImage(record map[string]any, field string, ref testkit.BlobRef) map[string]any { + record[field] = blobRefValue(ref) + return record +} + // TestCommunityProfileIngestion is the pipeline proof for community profiles. // // coves:ingestion-contract social.coves.community.profile @@ -273,12 +292,34 @@ func TestCommunityProfileIngestion(t *testing.T) { } // ---- create ----------------------------------------------------------- + // With an avatar, so the blob path is proven on the same arc rather than in + // a file of its own. It is the community half of what + // user_contract_test.go's opening note traces for actors: bytes uploaded to + // the repo's PDS, a reference embedded in the record, a CID extracted by the + // consumer, and a hydrated URL served back. The consumer is a different one + // (community_consumer.go, not user_consumer.go) and the serving endpoint is + // a different one, so neither contract covers the other. + // The avatar only — NO banner, on purpose. The update step below adds one, + // which is what exercises the nil→value transition the consumer's `if ok` + // guards (community_consumer.go's create and update paths) actually govern: + // a profile that gains a picture it did not have. + avatar := community.UploadBlob(t, testkit.TestPNG(64, 64), "image/png") community.PutRecord(t, communityProfileCollection, "self", - communityProfile(community, creator.DID, created, "a community the AppView never provisioned", "public")) + withCommunityImage( + communityProfile(community, creator.DID, created, "a community the AppView never provisioned", "public"), + "avatar", avatar)) view := observe("the directly-written community profile to reach social.coves.community.get via the consumers", func(v communityView) bool { return v.DisplayName == created }) + require.Containsf(t, view.Avatar, avatar.CID(), + "the community's avatar URL %q does not name the CID of the blob that was uploaded (%s): "+ + "the community consumer either failed to extract the blob ref or extracted the wrong one", + view.Avatar, avatar.CID()) + require.Emptyf(t, view.Banner, + "the community was created with no banner and the endpoint served one anyway: %q", view.Banner) + requireServesImage(t, p, "community avatar", view.Avatar) + require.Equal(t, community.DID, view.DID, "the AppView served a different community than the one that owns the repo") require.Equal(t, community.Handle, view.Handle, @@ -296,8 +337,26 @@ func TestCommunityProfileIngestion(t *testing.T) { // Same rkey, so this is an update commit rather than a second create — the // consumer's updateCommunity path, which reads the existing row and writes // the changed fields back. + // The avatar is replaced in the same commit: different bytes, so necessarily + // a different CID, so necessarily a different URL. This is what catches an + // avatar_cid the update path failed to refresh — the old URL keeps being + // served and looks entirely healthy. + // Two image changes in one commit, each exercising a different transition: + // the avatar is REPLACED (value→different value) and the banner is ADDED + // (nil→value, on a community that had none). + replacement := community.UploadBlob(t, testkit.TestPNG(48, 48), "image/png") + banner := community.UploadBlob(t, testkit.TestJPEG(96, 32), "image/jpeg") + require.NotEqual(t, avatar.CID(), replacement.CID(), + "the replacement image must differ from the original, or this step proves nothing") + require.NotEqual(t, replacement.CID(), banner.CID(), + "the banner must differ from the avatar, or neither assertion below can tell them apart") + community.PutRecord(t, communityProfileCollection, "self", - communityProfile(community, creator.DID, updated, "edited through the firehose", "unlisted")) + withCommunityImage( + withCommunityImage( + communityProfile(community, creator.DID, updated, "edited through the firehose", "unlisted"), + "avatar", replacement), + "banner", banner)) view = observe("the updated profile to reach social.coves.community.get", func(v communityView) bool { return v.DisplayName == updated }) @@ -306,6 +365,15 @@ func TestCommunityProfileIngestion(t *testing.T) { require.Equal(t, "unlisted", view.Visibility, "the update path must carry every changed field, not only the display name") require.Equal(t, community.DID, view.DID) + require.Containsf(t, view.Avatar, replacement.CID(), + "the update did not refresh the community's avatar: still serving %q", view.Avatar) + require.NotContains(t, view.Avatar, avatar.CID(), + "the community still names the previous avatar's CID after the update") + require.Containsf(t, view.Banner, banner.CID(), + "the banner added by the update never reached the endpoint (%q): this is the nil→value "+ + "transition the consumer's `if ok` blob guards govern", view.Banner) + requireServesImage(t, p, "community avatar after replacement", view.Avatar) + requireServesImage(t, p, "community banner", view.Banner) // ---- delete ----------------------------------------------------------- // DeleteExistingRecord rather than DeleteRecord: deleting a key that is not diff --git a/tests/e2e/contracts_test.go b/tests/e2e/contracts_test.go index b92ea7c..1595b80 100644 --- a/tests/e2e/contracts_test.go +++ b/tests/e2e/contracts_test.go @@ -91,6 +91,39 @@ // collection has no such marker and no entry in tests/ci/pending_contracts.txt. // The inventory is therefore generated, never curated: adding a collection to a // consumer breaks the build until it is proven. +// +// # PINNING A DEFECT INSIDE A MARKED CONTRACT +// +// Contracts keep finding bugs, and a bug this phase is not fixing gets PINNED: +// an assertion of what the shipped code currently does, written so it fails +// loudly the moment somebody fixes it. The question that comes up each time is +// whether a pin may live inside a function carrying an ingestion marker, since +// a marker is a claim that the collection is proven and a pin is a record that +// something about it is broken. +// +// THE RULE: a pin may share a marked contract's arc, provided the contract +// STILL PASSES once the defect is fixed — everything except the pinned step, +// which is expected to fail and is what announces the fix. If a fix would also +// break the surrounding proof, the pin belongs in its own unmarked test +// function beside the contract, because the marker would otherwise be +// advertising a proof that no longer runs. +// +// Both shapes are in the tree as worked examples, and the difference is +// instructive. vote_contract_test.go pins a same-rkey update being dropped +// INSIDE TestVoteIngestion: fixing it changes one step's expected counts and +// leaves create, re-tap, direction change and delete proving exactly what they +// prove today. The out-of-order defect is pinned OUTSIDE, in +// TestVoteOutOfOrderIsLostAndSubtracts, because its whole arc is the defect — +// there is no residual pipeline proof left if the behaviour changes, so it +// carries no marker and claims nothing about the collection. +// +// Two obligations either way. Name the issue file in the assertion message, so +// a red run says which defect got fixed rather than merely which line moved. +// And state the wrong-but-current value with Holds, not only Await, wherever an +// asynchronous fix — a reconciliation pass, a lazy repair on read — could +// satisfy an eventually-check on its way to the right answer and leave the pin +// silently passing against corrected code. A pin that cannot detect its own +// obsolescence is worse than no pin, because it reads as coverage. package e2e import ( diff --git a/tests/e2e/subscription_contract_test.go b/tests/e2e/subscription_contract_test.go new file mode 100644 index 0000000..a79c513 --- /dev/null +++ b/tests/e2e/subscription_contract_test.go @@ -0,0 +1,255 @@ +//go:build e2e + +package e2e + +import ( + "context" + "net/url" + "testing" + "time" + + "Coves/tests/testkit" + + "github.com/stretchr/testify/require" +) + +// The subscription domain's pipeline contract: the ingestion proof for +// social.coves.community.subscription. +// +// # A SUBSCRIPTION IS A RECORD IN THE SUBSCRIBER'S REPO, NAMING A COMMUNITY +// +// Like a vote and unlike a post, the record lives in the actor's own +// repository, and the consumer takes the subscriber's DID from the commit's +// repo rather than from the record. The record's only payload is `subject` — +// the community DID — plus a contentVisibility level. The XRPC procedures +// social.coves.community.subscribe/unsubscribe are just endpoints that create +// and delete these records; the collection is what the AppView indexes. +// +// # IT IS OBSERVED ON TWO ENDPOINTS, AND THAT IS DELIBERATE +// +// A subscription moves two numbers, maintained in two completely different +// ways, and asserting both is what makes this contract more than a round-trip: +// +// - social.coves.community.get's subscriberCount is a STORED COLUMN. +// SubscribeWithCount does `UPDATE communities SET subscriber_count = +// subscriber_count + 1` in the same transaction as the row insert, and +// UnsubscribeWithCount decrements it with a GREATEST(0, …) floor. +// - social.coves.actor.getProfile's stats.communityCount is a LIVE COUNT(*) +// over community_subscriptions (user_repo.go). +// +// A stored counter and a recomputed one can drift apart, and the stored one is +// the one that can be wrong: a missed increment, a double decrement, or an +// unsubscribe that removes the row without adjusting the column all leave the +// COUNT(*) correct and the column silently off. The floor at zero then hides +// the drift from anyone reading the number alone. Checking the two together is +// the cheapest possible detector for that whole class, and it costs one extra +// request. +// +// # THE FAN-OUT THIS CONTRACT CANNOT REACH, STATED PLAINLY +// +// The interesting thing a subscription DOES — a post from a subscribed +// community appearing in the subscriber's feed — is served by exactly one +// endpoint, social.coves.feed.getTimeline, and it is behind RequireAuth. §3.4b's +// standing limitation applies: nothing outside the browser OAuth callback mints +// a credential RequireAuth accepts, so T2 cannot call it at all. Every other +// public surface was checked before writing that sentence: +// social.coves.feed.getDiscover explicitly does not filter by subscription +// ("show ALL posts from ALL communities", discover_repo.go), +// communityFeed.getCommunity filters by community and never by subscriber, and +// community.list's ?subscribed=true filter 401s without a session. +// +// So the fan-out is covered at T1 (tests/integration/timeline_test.go, against +// the repo's own join) and becomes reachable here when the Phase-5 test-only +// session mint lands. It is named here rather than quietly omitted, because +// "the subscription contract covers the timeline" would otherwise stay true in +// everyone's memory and false in the code — the same note journey_test.go makes +// about the step it had to substitute. +// +// # THE 401 MATRIX IS NOT REPEATED HERE +// +// social.coves.community.subscribe and .unsubscribe are the write endpoints for +// this collection, and both are already asserted to answer 401 to a +// session-less client in TestCommunityAPIContract's boundary matrix, which +// enumerates every NSID RegisterCommunityRoutes puts behind RequireAuth. Adding +// a second copy here would make that matrix's completeness harder to see, not +// easier — the value of listing them in one place is that a route added without +// middleware has exactly one test to escape. +const subscriptionCollection = "social.coves.community.subscription" + +// subscriptionRecord builds a social.coves.community.subscription record in the +// shape internal/core/communities writes it (service.go's Subscribe), so the +// consumer parses exactly what production hands it. +// +// contentVisibility is the subscriber's preferred content level for this +// community, clamped by the consumer to 1-5 with a default of 3. The clamping +// is behavioural breadth and belongs at T1 (§3.4 rule 3); what a contract needs +// is a value the record carries through unchanged. +func subscriptionRecord(communityDID string, contentVisibility int) map[string]any { + return map[string]any{ + "$type": subscriptionCollection, + "subject": communityDID, + "contentVisibility": contentVisibility, + "createdAt": time.Now().UTC().Format(time.RFC3339), + } +} + +// subscriptionCounts is the pair of numbers this contract watches: the +// community's stored subscriber count and the subscriber's recomputed community +// count, read from two different endpoints. +type subscriptionCounts struct { + Subscribers int // social.coves.community.get → subscriberCount (stored column) + Communities int // social.coves.actor.getProfile → stats.communityCount (COUNT(*)) +} + +// counts reads both numbers. Two requests, so the pair is very slightly +// non-atomic — which is not a problem for the assertions here, because every +// one of them is made after a wait has already settled one of the two. +func (p *pipeline) counts(ctx context.Context, communityDID, subscriberDID string) (subscriptionCounts, error) { + community, err := p.Community(ctx, communityDID) + if err != nil { + return subscriptionCounts{}, err + } + var profile struct { + Stats struct { + CommunityCount int `json:"communityCount"` + } `json:"stats"` + } + if err := p.AppView.Query(ctx, "social.coves.actor.getProfile", + url.Values{"actor": {subscriberDID}}, &profile); err != nil { + return subscriptionCounts{}, err + } + return subscriptionCounts{ + Subscribers: community.SubscriberCount, + Communities: profile.Stats.CommunityCount, + }, nil +} + +// TestCommunitySubscriptionIngestion is the pipeline proof for subscriptions. +// +// coves:ingestion-contract social.coves.community.subscription +// +// Every record is written straight into the subscriber's own repo with the +// subscriber's session, and every observation is made through two serving +// endpoints at once (see the file's opening note): +// +// subscribe → both counts reach exactly one +// duplicate rkey → a second subscription record for the same community does NOT +// double either count, and STAYS undoubled (Holds) +// unsubscribe → both counts return to zero, and STAY there (Holds, §3.4a) +// stale unsubscribe→ deleting the superseded record does not drive the count negative +func TestCommunitySubscriptionIngestion(t *testing.T) { + p := newPipeline(t) + + creator := p.IndexedAccount(t, "sc") + subscriber := p.IndexedAccount(t, "ss") + community := indexedCommunity(t, p, "sb", creator.DID) + + ctx := context.Background() + + // The community is fresh — provisioned and indexed by this test — so an + // exact count is safe even on a kept stack where earlier runs have left + // their own communities behind. That is the whole reason the fixture is + // per-contract rather than shared. + before, err := p.counts(ctx, community.DID, subscriber.DID) + require.NoError(t, err) + require.Equal(t, subscriptionCounts{}, before, + "a newly indexed community has no subscribers and a fresh account subscribes to nothing; "+ + "non-zero here means the fixture is not as isolated as this contract assumes") + + awaitCounts := func(description string, want subscriptionCounts) { + t.Helper() + p.Await(t, description, func() (bool, error) { + got, err := p.counts(context.Background(), community.DID, subscriber.DID) + if err != nil { + return false, err + } + return got == want, nil + }) + } + holdCounts := func(description string, want subscriptionCounts) { + t.Helper() + p.Holds(t, description, func() (bool, error) { + got, err := p.counts(context.Background(), community.DID, subscriber.DID) + if err != nil { + return false, err + } + return got == want, nil + }) + } + + one := subscriptionCounts{Subscribers: 1, Communities: 1} + + // ---- subscribe ----------------------------------------------------------- + first := testkit.TID() + subscriber.PutRecord(t, subscriptionCollection, first, subscriptionRecord(community.DID, 4)) + awaitCounts("the directly-written subscription to reach both count surfaces via the consumers", one) + + // ---- a second subscription record, new rkey ------------------------------ + // The idempotency case that matters, and the one no test in the tree covered + // before this: SubscribeWithCount's ON CONFLICT keys on (user_did, + // community_did), NOT on the record URI, and it decides whether to increment + // from the `xmax = 0` discriminator — "did this statement actually insert a + // row". So a second record for the same community re-points the stored + // record_uri at the newer record and does not touch the count. + // + // Why a client produces one at all: nothing stops a user subscribing from + // two devices, and nothing in atProto makes an rkey a natural key. The + // re-pointing is deliberate — it is what makes a redriven delete of the OLD + // record a no-op instead of a silent unsubscribe — and the step after next + // asserts that half. + // + // Holds rather than a single read, because "the count did not double" is a + // claim about an event that has already been processed: an eventually-check + // would pass in the window before the second increment landed. + second := testkit.TID() + subscriber.PutRecord(t, subscriptionCollection, second, subscriptionRecord(community.DID, 2)) + holdCounts("a second subscription record for the same community to leave both counts at one", one) + + // ---- unsubscribe --------------------------------------------------------- + // The NEWER record, which is the one the stored row now points at. A delete + // commit carries no record body, so the consumer finds the community by + // looking the subscription up by URI — which is why deleting this one works + // and deleting the older one (below) does not. + subscriber.DeleteExistingRecord(t, subscriptionCollection, second) + awaitCounts("the deleted subscription to leave both count surfaces", subscriptionCounts{}) + holdCounts("the unsubscribe to stay unsubscribed", subscriptionCounts{}) + + // ---- the superseded record's delete is a no-op --------------------------- + // `first` was superseded when `second` re-pointed the row, and the row is + // gone entirely now. Its delete must not decrement anything. The GREATEST(0, + // …) floor would hide a spurious decrement from zero, so this is asserted + // where it can be seen: with a second, unrelated subscriber's live + // subscription present, which a spurious decrement has something to take + // away from. + // + // This is the redrive-safety property the consumer's own comment claims + // ("the redriven delete of the old URI then finds no row here and is skipped + // instead of tearing down the valid subscription"), reached from the outside. + other := p.IndexedAccount(t, "so") + other.PutRecord(t, subscriptionCollection, testkit.TID(), subscriptionRecord(community.DID, 3)) + p.Await(t, "a second subscriber, so a spurious decrement is visible", func() (bool, error) { + view, err := p.Community(context.Background(), community.DID) + if err != nil { + return false, err + } + return view.SubscriberCount == 1, nil + }) + + subscriber.DeleteExistingRecord(t, subscriptionCollection, first) + p.Holds(t, "deleting the superseded subscription record to leave the other subscriber alone", + func() (bool, error) { + view, err := p.Community(context.Background(), community.DID) + if err != nil { + return false, err + } + return view.SubscriberCount == 1, nil + }) + + // And the first subscriber is still at zero: the delete did not resurrect + // anything either. + final, err := p.counts(ctx, community.DID, subscriber.DID) + require.NoError(t, err) + require.Equal(t, subscriptionCounts{Subscribers: 1, Communities: 0}, final, + "after unsubscribing, the community keeps the other subscriber's count and the "+ + "unsubscribed actor's own community count is zero") +} diff --git a/tests/e2e/user_contract_test.go b/tests/e2e/user_contract_test.go new file mode 100644 index 0000000..c8e26a2 --- /dev/null +++ b/tests/e2e/user_contract_test.go @@ -0,0 +1,452 @@ +//go:build e2e + +package e2e + +import ( + "context" + "net/http" + "net/url" + "strings" + "testing" + + "Coves/tests/testkit" + + "github.com/stretchr/testify/require" +) + +// The user domain's pipeline contracts: the ingestion proof for +// social.coves.actor.profile, including the blob path an avatar travels, and +// the client-facing read surface. +// +// # THIS COLLECTION IS THE PACKAGE DOC'S RECONCILIATION HAZARD, IN PERSON +// +// contracts_test.go warns about code that reads the PDS by itself and can +// therefore satisfy a "did the firehose deliver it" wait with every consumer +// dead. actor.profile is the known instance: users.maybeBackfillProfile spawns +// a detached goroutine at IndexUser time that fetches +// social.coves.actor.profile/self straight from the user's PDS and writes it to +// Postgres. A contract that signs up, writes a profile and waits for it can be +// satisfied entirely by that goroutine. +// +// The guard is to falsify backfill's precondition before asserting anything. +// Backfill only touches a profile that is COMPLETELY empty, and it checks that +// twice — at the spawn site and again immediately before the write, after the +// fetch, specifically so that a firehose event arriving mid-fetch is not +// clobbered. So one arming write makes the row non-empty and disarms it for +// good, and every assertion this contract makes is on a LATER write. +// +// tests/ci/pending_contracts.txt carried that requirement as this collection's +// entry, and it is the reason the arming step below is not optional politeness. +// TestPipelineSmoke does the same thing with two display names; this contract +// goes further, because a display name is the one field that proves the least. +// +// # WHAT AN AVATAR ACTUALLY IS, END TO END +// +// The avatar is where this domain stops being a string round-trip, and it is +// worth writing down because five components have to agree: +// +// 1. The bytes are uploaded to the PDS with com.atproto.repo.uploadBlob, which +// answers with a blob ref — {$type: "blob", ref: {$link: }, mimeType, +// size}. The PDS holds the bytes; the record holds only the reference. +// 2. The profile record embeds that ref under "avatar" (and "banner"). +// 3. The user consumer pulls the CID back out with extractBlobCID +// (community_consumer.go), which insists on $type == "blob" and reads +// ref.$link, and stores it as users.avatar_cid. A ref it cannot parse is +// silently left alone — so a malformed blob does not clear an existing +// avatar, and does not fail the event either. +// 4. getProfile does NOT serve the CID. users.GetProfile hydrates it into a URL +// with blobs.HydrateImageURL, which in a stack with the image proxy enabled +// (.env.ci sets IMAGE_PROXY_ENABLED=true) is +// {proxy}/img/avatar/plain/{did}/{cid}, and with it disabled is the PDS' +// own com.atproto.sync.getBlob URL. Both forms contain the CID, which is +// what the assertions below key on — pinning the proxy form would make this +// contract a test of one config value. +// 5. Following that URL makes the AppView's image proxy fetch the blob back +// out of the PDS. That last hop is the only part of the chain a CID +// comparison cannot prove, and it is the part most likely to break silently +// (a proxy pointed at the wrong host answers a URL that looks perfect), so +// the contract follows the link it was given. +// +// The file this replaces (tests/integration/user_profile_avatar_e2e_test.go, +// deleted with this commit) advertised all of that across 1,022 lines and +// proved almost none of it: it watched a real firehose event arrive, then +// re-implemented extractBlobCID inside the test body and called +// userService.UpdateProfile itself — swallowing the error — so its final +// assertion was about its own transcription rather than about the consumer. It +// also asserted the com.atproto.sync.getBlob URL form, which the CI stack does +// not produce, and it fetched the avatar URL only to log the status code. +const profileCollection = "social.coves.actor.profile" + +// profileImages is the slice of getProfile's response carrying the hydrated +// blob URLs. Kept apart from ProfileView (contracts_test.go) rather than folded +// into it: ProfileView is read by every contract in the package and these two +// fields are this contract's business. +type profileImages struct { + Avatar string `json:"avatar"` + Banner string `json:"banner"` +} + +// profileWithImages reads the profile fields ProfileView models plus the image +// URLs, in one request. +func (p *pipeline) profileWithImages(ctx context.Context, actor string) (ProfileView, profileImages, error) { + var both struct { + ProfileView + profileImages + } + err := p.AppView.Query(ctx, "social.coves.actor.getProfile", url.Values{"actor": {actor}}, &both) + return both.ProfileView, both.profileImages, err +} + +// requireServesImage asserts that an image URL the AppView advertised actually +// serves that image, and is the only place in this package that follows a link +// out of a response body. +// +// # WHAT IT CHECKS AND WHY EACH PART IS THERE +// +// The claim being made is "a client rendering this profile would see a +// picture", and it decomposes into three facts that fail independently: +// +// - the URL points at THIS AppView. A hydrated URL is built from configured +// values (blobs.HydrateImageURL, IMAGE_PROXY_BASE_URL), so a misconfigured +// deployment serves URLs that are perfectly well-formed and point somewhere +// a client cannot reach. Checked before the fetch, because following it +// first would silently make the assertion about some other host. +// - the response is a 200 with a NON-EMPTY body of an image content type. +// Not merely "2xx": a 204, or a 200 with an empty body, satisfies "did not +// fail" while serving nothing, and that is a real outcome here rather than +// a hypothetical — the proxy fetches the blob from the PDS on demand, so +// everything about whether bytes exist is decided upstream of the status +// code it returns. +// - the bytes are reached through the AppView CLIENT, so the request carries +// this contract's synthetic rate-limit bucket like every other request the +// contract makes. +// +// What it deliberately does NOT check is that the bytes equal the bytes +// uploaded. The proxy re-encodes and resizes by preset, so a byte comparison +// would be asserting the image pipeline's output rather than its reachability, +// and would fail the day a preset changed. +func requireServesImage(t *testing.T, p *pipeline, kind, rawURL string) { + t.Helper() + + appview, err := url.Parse(testkit.Endpoints().AppView.BaseURL) + require.NoError(t, err) + parsed, err := url.Parse(rawURL) + require.NoErrorf(t, err, "the %s URL the AppView served is not a URL: %q", kind, rawURL) + require.Equalf(t, appview.Host, parsed.Host, + "the %s URL points at %q rather than at the AppView serving it (%q) — a client following "+ + "it would leave the deployment", kind, parsed.Host, appview.Host) + + resp, err := p.AppView.GetBinary(context.Background(), parsed.Path) + require.NoErrorf(t, err, + "the %s URL the AppView advertised does not serve: the profile names a blob the image "+ + "path cannot fetch back out of the PDS", kind) + require.Equalf(t, http.StatusOK, resp.Status, + "the %s URL answered %d rather than 200", kind, resp.Status) + require.NotEmptyf(t, resp.Body, + "the %s URL answered 200 with an EMPTY body: the status says the proxy is fine and the "+ + "client still renders a broken image", kind) + require.Truef(t, strings.HasPrefix(resp.ContentType, "image/"), + "the %s URL served content type %q rather than an image/*: a client will not render it, "+ + "and an HTML error page returned with a 200 looks exactly like this", kind, resp.ContentType) +} + +// blobRefValue renders a testkit blob reference the way a record embeds it. +// +// testkit.BlobRef marshals correctly on its own, but records here are built as +// map[string]any so that a contract can write a MALFORMED ref too — which is +// one of the cases below — and mixing a typed value into an otherwise untyped +// record makes the two look like different kinds of thing when they are not. +func blobRefValue(ref testkit.BlobRef) map[string]any { + return map[string]any{ + "$type": "blob", + "ref": map[string]any{"$link": ref.CID()}, + "mimeType": ref.MimeType, + "size": ref.Size, + } +} + +// TestActorProfileIngestion is the pipeline proof for user profiles. +// +// coves:ingestion-contract social.coves.actor.profile +// +// Every record is written straight into the account's own repo at rkey "self", +// and every observation is made through social.coves.actor.getProfile: +// +// arm → the row becomes non-empty, DISARMING profile backfill (proves nothing) +// update → the second write's fields are served, and only the firehose could have brought them +// blob → an uploaded avatar and banner reach the endpoint as URLs that carry their CIDs, +// and those URLs serve the bytes back through the AppView's image proxy +// replace→ a new avatar changes the URL, because the CID is the content +// delete → the profile fields are gone, and STAY gone (Holds, §3.4a) +func TestActorProfileIngestion(t *testing.T) { + p := newPipeline(t) + account := p.IndexedAccount(t, "up") + + // Every value is run-scoped, so a row left by an earlier run on a kept PDS + // volume cannot satisfy a wait for us. + arming := "arming " + testkit.UniqueID(t) + proving := "proving " + testkit.UniqueID(t) + provingBio := "written straight into the repo " + testkit.UniqueID(t) + + writeProfile := func(record map[string]any) { + t.Helper() + record["$type"] = profileCollection + account.PutRecord(t, profileCollection, "self", record) + } + + awaitProfile := func(description string, accept func(ProfileView, profileImages) bool) (ProfileView, profileImages) { + t.Helper() + var view ProfileView + var images profileImages + p.Await(t, description, func() (bool, error) { + v, i, err := p.profileWithImages(context.Background(), account.DID) + if done, err := testkit.PendingIfNotFound(err); !done || err != nil { + return done, err + } + view, images = v, i + return accept(v, i), nil + }) + return view, images + } + + // ---- arm ---------------------------------------------------------------- + // ASSERTS NOTHING about the pipeline. Its only job is to make the profile + // row non-empty so that backfill can never write again — this write may + // legitimately have been delivered by backfill itself, and the contract does + // not care which path brought it. + writeProfile(map[string]any{"displayName": arming, "description": "arming write"}) + awaitProfile("the profile row to become non-empty (disarming profile backfill)", + func(v ProfileView, _ profileImages) bool { return v.DisplayName == arming }) + + // ---- update: the first real assertion ------------------------------------ + // Backfill cannot write over a non-empty profile, so the only remaining path + // from the PDS to this endpoint is firehose → Jetstream → the AppView's own + // consumers → Postgres. + writeProfile(map[string]any{"displayName": proving, "description": provingBio}) + view, _ := awaitProfile( + "the second directly-written profile to reach social.coves.actor.getProfile via the consumers", + func(v ProfileView, _ profileImages) bool { return v.DisplayName == proving }) + + require.Equal(t, proving, view.DisplayName) + require.Equal(t, provingBio, view.Description, + "the update path must carry every changed field, not only the display name") + require.Equal(t, account.DID, view.DID, "the AppView served a different actor than the one that wrote") + require.Equal(t, account.Handle, view.Handle, + "the handle comes from the users row the signup created, not from the profile record — "+ + "a profile update must not disturb it") + + // ---- the blob path ------------------------------------------------------- + avatarBytes := testkit.TestPNG(64, 64) + avatar := account.UploadBlob(t, avatarBytes, "image/png") + banner := account.UploadBlob(t, testkit.TestJPEG(96, 32), "image/jpeg") + require.NotEqual(t, avatar.CID(), banner.CID(), + "two different images must have different CIDs, or the assertions below cannot tell "+ + "the avatar from the banner") + + withImages := "images " + testkit.UniqueID(t) + writeProfile(map[string]any{ + "displayName": withImages, + "description": provingBio, + "avatar": blobRefValue(avatar), + "banner": blobRefValue(banner), + }) + _, images := awaitProfile("the uploaded avatar to reach the profile endpoint", + func(_ ProfileView, i profileImages) bool { return i.Avatar != "" }) + + // The endpoint serves a URL, not a CID (see the file's opening note), so + // what is asserted is that the URL is ABOUT the blob that was uploaded. That + // holds in both hydration modes, which is the point: a contract that pinned + // the proxy path would fail the day IMAGE_PROXY_ENABLED changed, for no + // reason connected to the pipeline. + require.Containsf(t, images.Avatar, avatar.CID(), + "the avatar URL %q does not name the CID of the blob that was uploaded (%s): the "+ + "consumer either failed to extract the blob ref or extracted the wrong one", + images.Avatar, avatar.CID()) + require.Containsf(t, images.Banner, banner.CID(), + "the banner URL %q does not name the uploaded banner's CID (%s)", images.Banner, banner.CID()) + require.Containsf(t, images.Avatar, account.DID, + "the avatar URL must name the actor whose blob it is, or the proxy cannot fetch it "+ + "from the right repo: %q", images.Avatar) + require.NotEqual(t, images.Avatar, images.Banner) + + // ---- the URL is not merely well-formed, it serves ------------------------ + // The hop a CID comparison cannot prove: following the link makes the + // AppView's image proxy fetch the blob back out of the PDS. A proxy pointed + // at the wrong host, or a PDS that never stored the bytes, produces a URL + // that passes every assertion above and a 502 here. + // + // Requested through the AppView client so the request carries this + // contract's rate-limit bucket, and by PATH so the assertion cannot be + // satisfied by a host the tier did not configure — a URL naming some other + // service would fail here rather than being silently followed. That the host + // is this AppView is asserted separately, just below. + requireServesImage(t, p, "avatar", images.Avatar) + requireServesImage(t, p, "banner", images.Banner) + + // ---- replace: the CID is the content ------------------------------------ + // Different bytes, so necessarily a different CID, so necessarily a + // different URL. This is the assertion that a re-upload is really re-read + // rather than cached: an avatar_cid the consumer failed to update leaves the + // OLD URL being served, which looks entirely healthy. + replacement := account.UploadBlob(t, testkit.TestPNG(48, 48), "image/png") + require.NotEqual(t, avatar.CID(), replacement.CID(), + "the replacement image must differ from the original, or this step proves nothing") + + writeProfile(map[string]any{ + "displayName": withImages, + "description": provingBio, + "avatar": blobRefValue(replacement), + "banner": blobRefValue(banner), + }) + _, images = awaitProfile("the replacement avatar to reach the profile endpoint", + func(_ ProfileView, i profileImages) bool { return strings.Contains(i.Avatar, replacement.CID()) }) + + require.NotContains(t, images.Avatar, avatar.CID(), + "the profile still names the previous avatar's CID alongside the new one") + require.Containsf(t, images.Banner, banner.CID(), + "replacing the avatar cleared or changed the banner (%q), which the record did not ask for", + images.Banner) + + // The replacement must SERVE, not merely be named. A URL rebuilt around a + // CID whose bytes never reached the PDS is the exact failure a CID + // comparison cannot see, and a re-upload is where it would happen. + requireServesImage(t, p, "replacement avatar", images.Avatar) + + // ---- delete: the profile record goes, the account stays ------------------ + // handleProfileDelete does not delete the user — it clears the profile + // fields by writing empty strings — so the correct observation is that + // getProfile still answers 200 for the actor with the fields gone, NOT a + // 404. Getting this the wrong way round is the easy mistake: community + // profiles ARE hard-deleted and their endpoint does 404 (see + // TestCommunityProfileIngestion), and the two collections look alike from + // the outside. + account.DeleteExistingRecord(t, profileCollection, "self") + + cleared := func() (bool, error) { + v, i, err := p.profileWithImages(context.Background(), account.DID) + if err != nil { + return false, err + } + return v.DisplayName == "" && v.Description == "" && i.Avatar == "" && i.Banner == "", nil + } + p.Await(t, "the deleted profile record to clear the served profile", cleared) + p.Holds(t, "the cleared profile to stay cleared", cleared) + + final, _, err := p.profileWithImages(context.Background(), account.DID) + require.NoError(t, err, "deleting a profile RECORD must not delete the actor: getProfile "+ + "still answers for an account whose profile was cleared") + require.Equal(t, account.DID, final.DID) + require.Equal(t, account.Handle, final.Handle, + "the handle survives a profile delete — it belongs to the identity, not to the profile record") +} + +// TestActorProfileAPIContract covers the client-facing surface of the profile +// endpoints as a third-party client meets it. It carries NO ingestion marker — +// markers are for pipeline proofs (§3.4a). +// +// The authenticated half — social.coves.actor.updateProfile, which is how the +// mobile app writes the record this contract's sibling writes directly — is +// proven at T1 for the reason §3.4b records: nothing outside the browser OAuth +// callback mints a session RequireAuth accepts. That half lives in +// internal/api/handlers/user/update_profile_test.go, which asserts (among the +// size caps and MIME allowlist) that the record handed to the PDS embeds the +// uploaded blob ref in the shape the consumer above parses. What this adds is +// the part no handler test can see: that the shipped binary really routes these +// NSIDs, really guards the write one, and really serves an indexed profile back +// by every identifier a client holds. +func TestActorProfileAPIContract(t *testing.T) { + p := newPipeline(t) + account := p.IndexedAccount(t, "ua") + + displayName := "api contract " + testkit.UniqueID(t) + account.PutRecord(t, profileCollection, "self", map[string]any{ + "$type": profileCollection, "displayName": displayName, "description": "read back through the client surface", + }) + p.Await(t, "the profile to be indexed before the client surface is exercised", func() (bool, error) { + view, err := p.Profile(context.Background(), account.DID) + if done, err := testkit.PendingIfNotFound(err); !done || err != nil { + return done, err + } + return view.DisplayName == displayName, nil + }) + + ctx, cancel := context.WithTimeout(context.Background(), contractBudget) + defer cancel() + + t.Run("the write endpoint refuses an unauthenticated client", func(t *testing.T) { + // social.coves.actor.updateProfile is the only actor NSID behind + // RequireAuth. signup is deliberately NOT here: it is public by design + // (there is no session yet), and it has its own contract in + // user_signup_test.go. + err := p.AppView.Procedure(ctx, "social.coves.actor.updateProfile", + map[string]any{"displayName": "nope"}, nil) + require.Truef(t, testkit.IsStatus(err, http.StatusUnauthorized), + "social.coves.actor.updateProfile must answer 401 to a client with no session, answered: %v", err) + }) + + t.Run("a client reads the profile by DID and by handle", func(t *testing.T) { + // The two identifier forms take different paths: a DID goes straight to + // the lookup, a handle is resolved first. The handle case doubles as + // proof that resolution is served from the AppView's own index — the + // stack is egress-blocked, so a lookup that escaped to DNS could not + // have answered at all. + for _, actor := range []string{account.DID, account.Handle} { + view, err := p.Profile(ctx, actor) + require.NoErrorf(t, err, "social.coves.actor.getProfile rejected identifier %q", actor) + require.Equalf(t, account.DID, view.DID, "identifier %q resolved to the wrong actor", actor) + require.Equal(t, displayName, view.DisplayName) + } + }) + + t.Run("an unknown actor is an XRPC not-found", func(t *testing.T) { + // XRPC-shaped, which is what testkit.IsNotFound insists on: a plain 404 + // would mean the route is gone, and every wait in this tier that treats + // not-found as "not yet" depends on telling those apart. + // + // The DID is a literal at the full 24 characters of a real did:plc + // rather than a generated one, for the reason TestPostAPIContract gives: + // UniqueID does not promise the base32 alphabet the validator checks, + // and a malformed identifier would take the 400 path instead of the + // lookup path under test. + _, err := p.Profile(ctx, "did:plc:aaaaaaaaneverindexedactr") + require.Truef(t, testkit.IsNotFound(err), "expected an XRPC not-found, got: %v", err) + require.True(t, testkit.IsStatus(err, http.StatusNotFound)) + }) + + t.Run("a profile write that omits a field leaves it alone", func(t *testing.T) { + // PINS A DESIGN CHOICE THAT IS NOT OBVIOUS AND IS EASY TO GET WRONG. + // + // handleProfileUpdate builds users.UpdateProfileInput from whichever keys + // the record HAPPENS to carry: an absent displayName leaves the pointer + // nil, and a nil pointer means "do not touch" rather than "clear". So a + // record is not a snapshot of the profile — writing {description: "x"} + // to rkey self does not remove the display name, even though the record + // in the repo no longer has one. + // + // That makes the AppView's view and the PDS record disagree, which is + // worth knowing about rather than discovering. It is the correct choice + // for the AppView's own client (updateProfile always sends the full set) + // and the surprising one for a third-party client doing a partial + // putRecord. Clearing a field requires writing it EMPTY, which is + // exactly what handleProfileDelete does and what the ingestion + // contract's delete step observes. + onlyDescription := "description only " + testkit.UniqueID(t) + account.PutRecord(t, profileCollection, "self", map[string]any{ + "$type": profileCollection, "description": onlyDescription, + }) + p.Await(t, "the field-omitting update to be indexed", func() (bool, error) { + view, err := p.Profile(context.Background(), account.DID) + if err != nil { + return false, err + } + return view.Description == onlyDescription, nil + }) + + view, err := p.Profile(ctx, account.DID) + require.NoError(t, err) + require.Equal(t, displayName, view.DisplayName, + "a profile record with no displayName cleared the stored one: the consumer now treats "+ + "a record as a full snapshot, which silently erases fields for any client that "+ + "writes partial records") + }) +} diff --git a/tests/e2e/vote_contract_test.go b/tests/e2e/vote_contract_test.go new file mode 100644 index 0000000..064b789 --- /dev/null +++ b/tests/e2e/vote_contract_test.go @@ -0,0 +1,518 @@ +//go:build e2e + +package e2e + +import ( + "context" + "net/http" + "net/url" + "testing" + + "Coves/tests/testkit" + + "github.com/stretchr/testify/require" +) + +// The vote domain's pipeline contracts: the ingestion proof for +// social.coves.feed.vote, and the client-facing auth boundary. +// +// # VOTES LIVE IN THE VOTER'S REPO, AND THAT IS THE WHOLE SHAPE OF THE DOMAIN +// +// Unlike a post (which lives in the community's repo) or a comment (the +// author's), a vote's repo owner IS its subject-independent identity: the +// consumer takes the voter DID from the commit's repo and never reads one out of +// the record (vote_consumer.go createVote, "Vote comes from user's +// repository"). There is consequently no repo-ownership spoof to test here the +// way TestPostIngestion tests one — a repo cannot forge a vote as somebody else, +// because the repo is the identity. +// +// What that costs is the ordering guarantee the post contract leans on. A vote +// and its subject are necessarily in DIFFERENT repos, Jetstream parallelises +// across repos, and so "the vote arrives after the post" is topology luck rather +// than a protocol promise. The contract below does not pretend otherwise: it +// waits for the post to be INDEXED before writing a vote, and the out-of-order +// case is not an edge this contract avoids but the defect it pins (see +// TestVoteOutOfOrderIsLostAndSubtracts). +// +// # THERE IS NO RECONCILIATION PATH FOR VOTES (checked) +// +// Per the package doc's reconciliation hazard, the search for code that could +// satisfy a vote observation without the firehose: +// +// - votes.voteService.CreateVote/DeleteVote write NOTHING to Postgres. They +// resolve the subject, forward the record to the voter's PDS, and return — +// the AppView learns about its own users' votes only from the firehose, the +// same way it learns about a stranger's. +// - The only writer of upvote_count/downvote_count/score reachable in a +// running server is vote_consumer.go's transaction (plus the bridged_* +// columns, which the aggregator path owns and which this contract does not +// touch). +// - votes.cache is a read-through cache of the PDS for VIEWER state, not a +// count source, and nothing in this contract reads viewer state. +// +// So a single write → observe is honest here, with none of the arming the +// actor.profile contract needs. +// +// # WHAT "THE SAME VOTE AGAIN" MEANS, AND WHY IT IS TESTED THREE WAYS +// +// §3.4 rule 2's named invariant for this domain is that re-tapping must not +// double a count. "Re-tap" turns out to name three different code paths, keyed +// on three different things, and only asserting all three says the invariant +// holds: +// +// 1. THE SAME RECORD REWRITTEN AT THE SAME RKEY. This is an `update` commit, +// and the vote consumer handles exactly `create` and `delete` — an update is +// dropped on the floor by HandleEvent's switch. The count cannot double +// because nothing runs at all. (That is also a defect for the flipped- +// direction case; see the subtest that pins it.) +// 2. A DUPLICATE DELIVERY of one commit. Keyed on the record URI, twice over: +// the rev gate (tryAdvanceRecordRev, equal rev loses) and +// `INSERT … ON CONFLICT (uri) DO NOTHING RETURNING id`, whose no-rows result +// skips the count update. Not reachable from this tier — manufacturing a +// duplicate delivery is the reliability suite's job (§3.4c, task 16) — and +// it is covered at T1 by +// internal/atproto/jetstream/duplicate_delivery_test.go. +// 3. A SECOND VOTE RECORD UNDER A NEW RKEY. This is what a client actually +// produces: votes.voteService deletes the old record and creates a new one +// with a fresh TID, so the re-tap a user performs never reuses an rkey. +// Keyed on (voter_did, subject_uri, deleted_at IS NULL) — the consumer's +// "stale vote" cleanup soft-deletes the old row, decrements its direction, +// then inserts and increments the new one. Net zero for a same-direction +// re-tap, and a swing for a changed one. +// +// (1) and (3) are reachable from here and both are asserted below, each with a +// Holds, because "the count did not double" is a claim about a duplicate that +// has already been through the consumer — an eventually-check would pass while +// the second increment was still in flight. + +// voteURI renders the AT-URI a vote record has once committed. The VOTER's DID +// is the authority — see the file's opening note. +func voteURI(voterDID, rkey string) string { + return "at://" + voterDID + "/" + voteCollection + "/" + rkey +} + +// awaitStats waits for a post's stats to satisfy accept, and returns them. +// +// Every observation in this contract is a stats read on +// social.coves.community.post.get, so the shape is worth naming once: the +// endpoint answers 200 with a notFoundPost member for an unindexed post, which +// is not what any wait here is waiting for — the post is indexed before the +// first vote is written — so a notFound is a hard error rather than "not yet". +func awaitStats(t *testing.T, p *pipeline, uri, description string, accept func(postStats) bool) postStats { + t.Helper() + var observed postStats + p.Await(t, description, func() (bool, error) { + view, err := p.Post(context.Background(), uri) + if err != nil { + return false, err + } + if view.NotFound { + return false, errPostVanished(uri) + } + observed = view.Stats + return accept(view.Stats), nil + }) + return observed +} + +// holdStats asserts a post's stats stay exactly want for contractHoldWindow. +// +// The vote domain's destructive-and-duplicate assertions are all of this shape: +// a count that must not move. Stated as an exact equality on the whole struct +// rather than on one field, because the failures worth catching here are +// cross-field — a decrement that lands on the wrong direction leaves the field +// under test correct and the score wrong. +func holdStats(t *testing.T, p *pipeline, uri, description string, want postStats) { + t.Helper() + p.Holds(t, description, func() (bool, error) { + view, err := p.Post(context.Background(), uri) + if err != nil { + return false, err + } + if view.NotFound { + return false, errPostVanished(uri) + } + return view.Stats == want, nil + }) +} + +// errPostVanished is the terminal error both helpers above raise when the post +// they are measuring stops being served. A vote cannot delete its subject, so +// this means something outside the contract removed the post — and retrying +// would turn that into an opaque timeout about vote counts. +func errPostVanished(uri string) error { + return &voteSubjectGoneError{uri: uri} +} + +type voteSubjectGoneError struct{ uri string } + +func (e *voteSubjectGoneError) Error() string { + return "the post being voted on (" + e.uri + ") stopped being served by " + + "social.coves.community.post.get part-way through the contract: a vote cannot " + + "delete its subject, so the post was removed by something outside this test" +} + +// TestVoteIngestion is the pipeline proof for votes. +// +// coves:ingestion-contract social.coves.feed.vote +// +// Every record is written straight into the voter's own repo with the voter's +// session, and every observation is a stats read on +// social.coves.community.post.get — the vote itself has no serving endpoint of +// its own that an unauthenticated caller can reach, so the denormalised counts +// the consumer maintains ARE the observable: +// +// create → upvotes 1, score 1 +// same-rkey re-put → the count does not move, and STAYS put (Holds) +// new-rkey re-tap → still exactly one vote's worth, and STAYS (Holds) +// direction change → the up is withdrawn as the down lands (0/1/-1) +// delete → back to zero, and STAYS zero (Holds, §3.4a) +func TestVoteIngestion(t *testing.T) { + p := newPipeline(t) + + author := p.IndexedAccount(t, "vi") + voter := p.IndexedAccount(t, "vv") + community := indexedCommunity(t, p, "vi", author.DID) + post := indexedPost(t, p, community, author.DID, "vote target "+testkit.UniqueID(t)) + + // The voter is deliberately NOT the author: a self-vote and a stranger's + // vote take the same path (the consumer has no self-vote rule), but using + // two identities keeps the fixture honest about which repo the record is in. + require.NotEqual(t, author.DID, voter.DID) + + // ---- create ------------------------------------------------------------- + first := testkit.TID() + record := voter.PutRecord(t, voteCollection, first, voteRecord(post, "up")) + require.Equal(t, voteURI(voter.DID, first), record.URI, + "the PDS committed the vote under a different URI than the voter's repo and rkey imply, "+ + "which would make every assertion below measure a record this test did not write") + + stats := awaitStats(t, p, post.URI, "the directly-written upvote to reach the post's stats via the consumers", + func(s postStats) bool { return s.Upvotes == 1 }) + require.Equal(t, postStats{Upvotes: 1, Score: 1}, stats, + "an upvote raises upvotes and score by one and touches nothing else") + + // ---- PINNED DEFECT: a same-rkey update is silently dropped --------------- + // + // Rewriting the record at the SAME rkey is an `update` commit, and + // HandleEvent switches on create and delete only — so an update is + // discarded before any vote logic runs, whatever it says. + // + // For a BYTE-IDENTICAL rewrite that is the right outcome reached for the + // wrong reason, and it is not worth a step of its own: a first draft spent a + // full hold window on it, and review pointed out the assertion could not + // fail. Identical bytes produce an identical CID, so the PDS may emit no + // commit at all; if it does, the rev gate and `ON CONFLICT (uri) DO NOTHING` + // would each independently stop the count moving even if the switch did + // route updates. Three redundant guards and a coin-flip on whether an event + // is even emitted — a green result there said nothing. + // + // The FLIP is the same code path with a visible consequence, so it is the + // one worth holding. A third-party atProto client changing its vote in place + // — the obvious way to express "I changed my mind", and what + // com.atproto.repo.putRecord is FOR — leaves the record on the PDS saying + // "down" and the AppView saying "up", permanently: nothing ever revisits it. + // Coves' own client never hits this (votes.voteService always deletes and + // re-creates under a fresh rkey, path 3 in the file's opening note), so the + // bug is invisible from inside the product and appears the moment a + // federated peer or a third-party client votes. + // + // PINNED, not asserted-as-intended: the suite documents the shipped + // behaviour rather than failing over a bug this task is not fixing. When it + // is fixed, this block inverts to expect {Downvotes: 1, Score: -1}, and the + // Holds is what fails first — loudly, and naming the issue — which is the + // intent. + voter.PutRecord(t, voteCollection, first, voteRecord(post, "down")) + holdStats(t, p, post.URI, + "a same-rkey direction flip to STILL be ignored (pinning known defect "+ + "2026-07-29-vote-putrecord-update-silently-ignored: the vote consumer handles create "+ + "and delete only, so an update commit never reaches it. IF THIS FAILED, the defect is "+ + "FIXED — invert this step to expect {Downvotes:1, Score:-1} and close the issue)", + postStats{Upvotes: 1, Score: 1}) + + // ---- a second vote record, new rkey, same direction ---------------------- + // The shape a real re-tap produces. The consumer's stale-vote cleanup keys + // on (voter_did, subject_uri) rather than on the URI: it soft-deletes the + // first vote, decrements for it, then inserts the second and increments. + // Net zero, which is only observable as "the count did not become 2". + second := testkit.TID() + voter.PutRecord(t, voteCollection, second, voteRecord(post, "up")) + holdStats(t, p, post.URI, + "the count to stay at one upvote after the same voter votes again under a new rkey "+ + "(the consumer supersedes the old vote instead of adding a second)", + postStats{Upvotes: 1, Score: 1}) + + // ---- direction change ---------------------------------------------------- + // Same mechanism, opposite outcome: the superseded upvote is withdrawn and + // the downvote lands, so the swing is two points of score in one event. + third := testkit.TID() + voter.PutRecord(t, voteCollection, third, voteRecord(post, "down")) + stats = awaitStats(t, p, post.URI, "the voter's change of mind to swing the post's score", + func(s postStats) bool { return s.Downvotes == 1 }) + require.Equal(t, postStats{Downvotes: 1, Score: -1}, stats, + "changing direction must withdraw the upvote in the same transaction that records the "+ + "downvote — a stale upvote left behind shows up here as upvotes 1, score 0") + + // ---- delete -------------------------------------------------------------- + // DeleteExistingRecord, not DeleteRecord: deleting an absent rkey answers + // 200 and emits no commit, so a wrong rkey would become a timeout blaming + // the firehose (testkit/pds.go). + voter.DeleteExistingRecord(t, voteCollection, third) + stats = awaitStats(t, p, post.URI, "the withdrawn vote to leave the post's stats", + func(s postStats) bool { return s.Downvotes == 0 }) + require.Equal(t, postStats{}, stats) + holdStats(t, p, post.URI, "the withdrawn vote to stay withdrawn", postStats{}) + + // ---- deleting an already-superseded vote is a no-op ---------------------- + // `second` was soft-deleted by the stale-vote cleanup when `third` arrived, + // and its decrement was applied then. Its own delete commit must not + // decrement a second time. The floor (GREATEST(0, …)) would hide a double + // decrement from zero, so this is asserted where it can be seen: with a live + // vote from ANOTHER voter present, so a spurious decrement has something to + // take away. + other := p.IndexedAccount(t, "vo") + otherRKey := testkit.TID() + other.PutRecord(t, voteCollection, otherRKey, voteRecord(post, "up")) + awaitStats(t, p, post.URI, "a second voter's upvote, so a spurious decrement is visible", + func(s postStats) bool { return s.Upvotes == 1 }) + + voter.DeleteExistingRecord(t, voteCollection, second) + holdStats(t, p, post.URI, + "deleting an already-superseded vote to leave the other voter's upvote alone "+ + "(a second decrement for a vote whose count was already withdrawn)", + postStats{Upvotes: 1, Score: 1}) +} + +// TestVoteOutOfOrderIsLostAndSubtracts PINS A DEFECT. It is not an aspiration +// and it is not a contract — it carries no ingestion marker — it is the +// executable record of what the shipped pipeline does when a vote reaches the +// AppView before the thing it votes on. +// +// # WHY THIS IS REACHABLE IN PRODUCTION +// +// A vote lives in the voter's repo and its subject lives in another (a post is +// in the community's repo, a comment in its author's). Jetstream serialises a +// single repo's commits and PARALLELISES ACROSS REPOS, so nothing orders a vote +// against its subject. The window is small in a healthy stack and arbitrarily +// large in an unhealthy one — a redriven post, a consumer catching up after a +// restart, a slow blob fetch upstream — and Phase 5's relay topology widens it +// further. +// +// # WHAT THE CONSUMER DOES, AND WHY IT LOSES +// +// vote_consumer.go's createVote has NO must-exist gate on the subject: unlike +// the post consumer (which rejects a post whose community it has not seen, with +// a TRANSIENT error so the redrive succeeds later) and unlike the subscription +// consumer (same pattern), a vote whose subject is unknown is accepted. The row +// is inserted; the `UPDATE posts SET upvote_count = …` that follows matches +// zero rows; and the zero-row case is a log line: +// +// log.Printf("Warning: Vote subject not found or deleted: %s (vote indexed anyway)", …) +// +// Nothing reconciles afterwards. The post consumer, when the post finally +// arrives, INSERTs it with fresh zeroed counters and never looks at the votes +// table. So the vote is counted by nobody, forever. +// +// # AND IT IS WORSE THAN A LOST VOTE +// +// The row that was inserted is a live, undeleted vote. When it is eventually +// withdrawn — the user un-taps, or the record is tidied up — deleteVote loads +// that row, finds a direction and a subject, and DECREMENTS the post. It +// subtracts a vote it never added, taking a real vote from a real voter with it. +// The steady-state error is therefore not "one vote short", it is unbounded +// downward drift, floored at zero by GREATEST(0, …) so that it never even looks +// wrong in the data. +// +// Both halves are asserted below, because the second is the one that turns a +// missing-increment annoyance into a correctness bug, and it is the one a +// reader would not predict. +func TestVoteOutOfOrderIsLostAndSubtracts(t *testing.T) { + p := newPipeline(t) + + author := p.IndexedAccount(t, "vd") + early := p.IndexedAccount(t, "ve") + late := p.IndexedAccount(t, "vl") + community := indexedCommunity(t, p, "vd", author.DID) + + // The post's URI is knowable before the post exists — the community's DID + // and an rkey this test chooses are all it is made of — which is exactly why + // a vote can name a subject that has not been indexed. + rkey := testkit.TID() + uri := postURI(community.DID, rkey) + + // ---- the vote, before its subject --------------------------------------- + // The CID is a well-formed placeholder rather than the post's real one, + // which the test cannot know yet. The consumer stores subject_cid without + // checking it against anything (there is no CID validation on the vote path + // at all — worth knowing, and not this test's subject). + orphanRKey := testkit.TID() + early.PutRecord(t, voteCollection, orphanRKey, voteRecord( + strongRef{URI: uri, CID: "bafyreiaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"}, "up")) + + // Bounded INTRA-REPO, the way §3.4's negatives have to be: a second record + // in the SAME repo, whose arrival proves the first has already been through + // the consumer. Cross-repo ordering is the very thing this test is about, so + // it cannot also be the thing the test assumes. + // + // The bounding record is a vote on a DIFFERENT, already-indexed post, so its + // effect is observable — an unobservable bound is not a bound. + boundPost := indexedPost(t, p, community, author.DID, "ordering bound "+testkit.UniqueID(t)) + early.PutRecord(t, voteCollection, testkit.TID(), voteRecord(boundPost, "up")) + awaitStats(t, p, boundPost.URI, + "a later vote from the SAME repo to be indexed, proving the orphan vote's commit "+ + "has already passed through the vote consumer", + func(s postStats) bool { return s.Upvotes == 1 }) + + // ---- now the subject arrives -------------------------------------------- + community.PutRecord(t, postCollection, rkey, + postRecord(community.DID, author.DID, "voted on before it existed", "the late post")) + p.Await(t, "the post to be indexed after the vote that names it", func() (bool, error) { + view, err := p.Post(context.Background(), uri) + if err != nil { + return false, err + } + return !view.NotFound, nil + }) + + // ---- half one: the vote is lost, and stays lost ------------------------- + // Holds rather than a single read, because "no reconciliation happens" is a + // claim about the future: a repair pass that ran a second later would make + // this the wrong assertion, and Holds is what would notice. + holdStats(t, p, uri, + "the out-of-order vote to be INVISIBLE on the post it names, and to stay invisible "+ + "(pinning known defect 2026-07-29-vote-before-subject-lost-then-subtracts: createVote "+ + "has no must-exist gate on its subject, its count UPDATE matches zero rows, and nothing "+ + "recomputes counts afterwards. IF THIS FAILED, the defect is FIXED — invert this step "+ + "to expect {Upvotes:1, Score:1} and close the issue)", + postStats{}) + + // ---- half two: the lost vote can still subtract ------------------------- + // A real, correctly-ordered vote from another actor takes the count to one. + view, err := p.Post(context.Background(), uri) + require.NoError(t, err) + late.PutRecord(t, voteCollection, testkit.TID(), voteRecord(strongRef{URI: uri, CID: view.CID}, "up")) + awaitStats(t, p, uri, "a correctly-ordered vote to be counted normally", + func(s postStats) bool { return s.Upvotes == 1 }) + + // Withdrawing the ORPHAN — which never incremented anything — takes the + // other voter's upvote away with it. + early.DeleteExistingRecord(t, voteCollection, orphanRKey) + stats := awaitStats(t, p, + uri, "withdrawing the never-counted vote to DECREMENT the post anyway", + func(s postStats) bool { return s.Upvotes == 0 }) + require.Equal(t, postStats{}, stats, + "the orphaned vote's delete subtracted a vote it never added, and the count it took "+ + "belonged to a different voter whose vote is still live in the votes table") + + // AND IT STAYS WRONG. The Await above would be satisfied by a fix that + // repairs the count ASYNCHRONOUSLY — a reconciliation pass, a recount + // triggered by the delete — because it only has to observe zero once, on its + // way back up. This pin would then keep passing against code that no longer + // has the defect, which is the specific way a pin rots: it stops being a + // record of current behaviour and becomes a test of nothing. + // + // Holding the wrong-but-current value is what closes that. When the defect + // is fixed by any means, sync or async, THIS is the assertion that fails. + holdStats(t, p, uri, + "the stolen upvote to STAY stolen (pinning known defect "+ + "2026-07-29-vote-before-subject-lost-then-subtracts, second half: deleteVote decrements "+ + "unconditionally from the stored row and cannot tell an applied increment from a missed "+ + "one. IF THIS FAILED, the defect is FIXED — the surviving voter's upvote should read "+ + "{Upvotes:1, Score:1}; invert this step and close the issue)", + postStats{}) +} + +// TestVoteAPIContract covers the client-facing surface of the vote endpoints as +// a third-party client meets it. It carries NO ingestion marker — markers are +// for pipeline proofs (§3.4a) — and it is short, because votes have no public +// read endpoint of their own. +// +// A vote is only ever visible to an unauthenticated client as somebody else's +// count, which the ingestion contract above already reads through +// social.coves.community.post.get. Viewer state (did I vote, and which way) is +// the part a client asks about by identity, and it is behind OptionalAuth with +// no credential this tier can mint (§3.4b) — covered at T1 instead, in +// tests/integration/comment_vote_test.go for comments and +// internal/db/postgres/vote_repo_test.go for posts. +// +// So what is left, and what nothing else can see, is the auth boundary of the +// shipped router. +func TestVoteAPIContract(t *testing.T) { + p := newPipeline(t) + + author := p.IndexedAccount(t, "va") + community := indexedCommunity(t, p, "va", author.DID) + post := indexedPost(t, p, community, author.DID, "api contract "+testkit.UniqueID(t)) + + ctx, cancel := context.WithTimeout(context.Background(), contractBudget) + defer cancel() + + t.Run("the write endpoints refuse an unauthenticated client", func(t *testing.T) { + // One request each, no polling: this is the auth boundary of the shipped + // router and the answer does not become true later. + // + // Both NSIDs RegisterVoteRoutes puts behind RequireAuth are listed. + // Asserting it HERE rather than only in the handler tests is the point: a + // handler test proves the handler refuses an unauthenticated call and + // structurally cannot see a route registered without the middleware in + // front of it. Only a request to the running router can — and for votes + // that gap is the difference between a rate-limited, authenticated write + // and an open ballot box. + for _, endpoint := range []struct { + nsid string + input map[string]any + }{ + {"social.coves.feed.vote.create", map[string]any{ + "subject": map[string]any{"uri": post.URI, "cid": post.CID}, + "direction": "up", + }}, + {"social.coves.feed.vote.delete", map[string]any{"subject": post.URI}}, + } { + err := p.AppView.Procedure(ctx, endpoint.nsid, endpoint.input, nil) + require.Truef(t, testkit.IsStatus(err, http.StatusUnauthorized), + "%s must answer 401 to a client with no session, answered: %v", endpoint.nsid, err) + } + }) + + t.Run("a vote's effect is public even though the vote is not", func(t *testing.T) { + // The complement of the auth boundary: an anonymous client cannot cast a + // vote and cannot ask whether it voted, but it must still see the totals + // — that is what a score on a feed is. Read with no credential at all. + voter := p.IndexedAccount(t, "vp") + voter.PutRecord(t, voteCollection, testkit.TID(), voteRecord(post, "down")) + + stats := awaitStats(t, p, post.URI, "an anonymous client to see the vote totals", + func(s postStats) bool { return s.Downvotes == 1 }) + require.Equal(t, postStats{Downvotes: 1, Score: -1}, stats) + + // The same totals through the community feed, which is a different query + // over different joins: a count correct on post.get and wrong here is a + // hydration bug neither endpoint's own test would show. + require.Equal(t, stats, feedStats(t, p, community.DID, post.URI), + "the community feed disagreed with post.get about the same post's vote totals") + }) +} + +// feedStats reads one post's stats out of the community feed, which is the +// other public surface a vote total reaches a client through. +func feedStats(t *testing.T, p *pipeline, communityDID, postURI string) postStats { + t.Helper() + var feed struct { + Feed []struct { + Post postView `json:"post"` + } `json:"feed"` + } + ctx, cancel := context.WithTimeout(context.Background(), contractBudget) + defer cancel() + require.NoError(t, p.AppView.Query(ctx, "social.coves.communityFeed.getCommunity", + url.Values{"community": {communityDID}, "sort": {"new"}, "limit": {"25"}}, &feed)) + + for _, item := range feed.Feed { + if item.Post.URI == postURI { + return item.Post.Stats + } + } + t.Fatalf("the post %s was not in community %s's %d newest posts, so its feed stats could "+ + "not be compared with post.get's", postURI, communityDID, len(feed.Feed)) + return postStats{} +} diff --git a/tests/integration/comment_vote_test.go b/tests/integration/comment_vote_test.go index 44cc442..29fab4e 100644 --- a/tests/integration/comment_vote_test.go +++ b/tests/integration/comment_vote_test.go @@ -12,6 +12,8 @@ import ( "fmt" "testing" "time" + + "github.com/stretchr/testify/require" ) // TestCommentVote_CreateAndUpdate tests voting on comments and vote count updates @@ -540,7 +542,7 @@ func TestCommentVote_ViewerState(t *testing.T) { }) t.Run("Unauthenticated request has no viewer state", func(t *testing.T) { - // Query without authentication + // Query without authentication. // Use factory constructor with nil factory - this test only uses the read path (GetComments) commentService := comments.NewCommentServiceWithPDSFactory(commentRepo, userRepo, postRepo, communityRepo, nil, nil) response, err := commentService.GetComments(ctx, &comments.GetCommentsRequest{ @@ -554,11 +556,20 @@ func TestCommentVote_ViewerState(t *testing.T) { t.Fatalf("Failed to get comments: %v", err) } - if len(response.Comments) > 0 { - // Verify no viewer state - if response.Comments[0].Comment.Viewer != nil { - t.Error("Expected viewer = nil for unauthenticated request") - } + // The assertion used to be wrapped in `if len(response.Comments) > 0`, + // which made it vacuous whenever the thread was empty — and it was only + // ever non-empty because the sibling subtests above happened to run + // first and seed this post. A guard that turns "the thread was empty" + // into a silent pass hides exactly the regression it was written for, so + // the emptiness is now the failure. + require.NotEmpty(t, response.Comments, + "the sibling subtests seed this post's thread; an empty thread here means the "+ + "assertion below would have checked nothing") + for _, node := range response.Comments { + require.Nilf(t, node.Comment.Viewer, + "comment %s carried viewer state for a caller with no identity: viewer state is "+ + "per-actor, so serving it unauthenticated leaks one user's votes to everyone", + node.Comment.URI) } }) } diff --git a/tests/integration/community_avatar_e2e_test.go b/tests/integration/community_avatar_e2e_test.go deleted file mode 100644 index 976445d..0000000 --- a/tests/integration/community_avatar_e2e_test.go +++ /dev/null @@ -1,993 +0,0 @@ -//go:build integration - -package integration - -// SERIAL BY DESIGN — do not add t.Parallel() to this file. -// -// Its tests drive the Jetstream firehose through the hand-rolled -// subscribeToJetstream* helpers below rather than testkit's cursor-gated -// subscriber. Those helpers subscribe to one shared stream and match on the -// first event of a collection, so a concurrent test writing the same -// collection is delivered to them too and either steals the match or trips -// their timeout. Per-test database clones do not isolate a shared websocket. -// -// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). - -import ( - "Coves/internal/atproto/identity" - "Coves/internal/atproto/jetstream" - "Coves/internal/core/blobs" - "Coves/internal/core/communities" - "Coves/internal/db/postgres" - "Coves/tests/testkit" - "bytes" - "context" - "fmt" - "image" - "image/color" - "image/png" - "net/http" - "os" - "strings" - "testing" - "time" - - "github.com/gorilla/websocket" -) - -// createTestPNGImage creates a simple PNG image for testing -// Panics on encoding error since this is a test helper and encoding should never fail -// for simple in-memory images -func createTestPNGImage(width, height int, c color.Color) []byte { - img := image.NewRGBA(image.Rect(0, 0, width, height)) - for y := 0; y < height; y++ { - for x := 0; x < width; x++ { - img.Set(x, y, c) - } - } - var buf bytes.Buffer - if err := png.Encode(&buf, img); err != nil { - panic(fmt.Sprintf("createTestPNGImage: failed to encode PNG: %v", err)) - } - return buf.Bytes() -} - -// TestCommunityAvatarE2E_CreateWithAvatar tests creating a community with an avatar -// Flow: CreateCommunity(avatar) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView -func TestCommunityAvatarE2E_CreateWithAvatar(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.community.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - t.Logf("✅ Jetstream available at %s", jetstreamURL) - - ctx := context.Background() - instanceDID := "did:web:coves.social" - - // Setup identity resolver with local PLC - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - communityRepo := postgres.NewCommunityRepository(db) - provisioner := communities.NewPDSAccountProvisioner("coves.social", pdsURL) - blobService := blobs.NewBlobService(pdsURL) - - communityService := communities.NewCommunityServiceWithPDSFactory( - communityRepo, - pdsURL, - instanceDID, - "coves.social", - provisioner, - nil, // No custom PDS factory, uses built-in - blobService, - ) - - consumer := jetstream.NewCommunityEventConsumer(communityRepo, instanceDID, true, identityResolver) - - t.Run("create community with avatar via real Jetstream", func(t *testing.T) { - uniqueName := fmt.Sprintf("avt%s", uniqueTestID()) - creatorDID := "did:plc:avatar-create-test" - - // Create a test PNG image (100x100 red square) - avatarData := createTestPNGImage(100, 100, color.RGBA{255, 0, 0, 255}) - t.Logf("Created test avatar image: %d bytes", len(avatarData)) - - // Subscribe to Jetstream BEFORE creating the community - // This ensures we catch the create event - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, 30*time.Second) - defer cancelSubscribe() - - // We don't know the DID yet, so we'll filter by collection and match after - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - t.Logf("Failed to connect to Jetstream: %v", dialErr) - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { - return - } - - var event jetstream.JetstreamEvent - if readErr := conn.ReadJSON(&event); readErr != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - // Only process community profile create events - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.community.profile" && - event.Commit.Operation == "create" { - eventChan <- &event - } - } - } - }() - time.Sleep(500 * time.Millisecond) // Give subscriber time to connect - - t.Logf("\n📝 Creating community with avatar on PDS...") - community, createErr := communityService.CreateCommunity(ctx, communities.CreateCommunityRequest{ - Name: uniqueName, - DisplayName: "Community With Avatar", - Description: "Testing avatar upload on create", - Visibility: "public", - CreatedByDID: creatorDID, - HostedByDID: instanceDID, - AllowExternalDiscovery: true, - AvatarBlob: avatarData, - AvatarMimeType: "image/png", - }) - if createErr != nil { - close(done) - t.Fatalf("Failed to create community with avatar: %v", createErr) - } - - t.Logf("✅ Community created on PDS:") - t.Logf(" DID: %s", community.DID) - t.Logf(" RecordCID: %s", community.RecordCID) - t.Logf(" AvatarCID (from service): %s", community.AvatarCID) - - // Wait for REAL Jetstream event - t.Logf("\n⏳ Waiting for create event from Jetstream...") - var realEvent *jetstream.JetstreamEvent - timeout := time.After(jetstreamReadBudget) - - eventLoop: - for { - select { - case event := <-eventChan: - // Match by DID (we now know it) - if event.Did == community.DID { - realEvent = event - t.Logf("✅ Received REAL create event from Jetstream!") - t.Logf(" DID: %s", event.Did) - t.Logf(" Operation: %s", event.Commit.Operation) - t.Logf(" CID: %s", event.Commit.CID) - - // Log avatar info from real event - if event.Commit.Record != nil { - if avatar, hasAvatar := event.Commit.Record["avatar"]; hasAvatar { - t.Logf(" Avatar in event: %v", avatar) - } - } - break eventLoop - } - case <-timeout: - close(done) - t.Fatalf("Timeout waiting for Jetstream create event for DID %s", community.DID) - } - } - close(done) - - // Process the REAL event through consumer - // Note: The community already exists (service indexed it), so consumer will hit conflict - // But this tests that the real event has correct avatar data - t.Logf("\n🔄 Processing real Jetstream event through consumer...") - if handleErr := consumer.HandleEvent(ctx, realEvent); handleErr != nil { - t.Logf(" Note: Consumer conflict expected (already indexed): %v", handleErr) - } - - // Verify avatar CID matches what's in the database - final, err := communityRepo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get final community: %v", err) - } - - t.Logf("\n✅ Community avatar verification:") - t.Logf(" AvatarCID in DB: %s", final.AvatarCID) - - if final.AvatarCID == "" { - t.Errorf("Expected AvatarCID to be set after create with avatar") - } - - // Verify the avatar CID from the real Jetstream event matches what we stored - if realEvent.Commit.Record != nil { - if avatar, hasAvatar := realEvent.Commit.Record["avatar"].(map[string]interface{}); hasAvatar { - if ref, hasRef := avatar["ref"].(map[string]interface{}); hasRef { - if link, hasLink := ref["$link"].(string); hasLink { - t.Logf(" AvatarCID from Jetstream: %s", link) - if final.AvatarCID != link { - t.Errorf("AvatarCID mismatch: DB has %s, Jetstream has %s", final.AvatarCID, link) - } else { - t.Logf(" ✅ AvatarCID matches between DB and Jetstream event!") - } - } - } - } - } - - // Verify we can fetch the avatar from PDS - pdsResp, pdsErr := http.Get(fmt.Sprintf("%s/xrpc/com.atproto.repo.getRecord?repo=%s&collection=social.coves.community.profile&rkey=self", - pdsURL, community.DID)) - if pdsErr != nil { - t.Fatalf("Failed to fetch profile record from PDS: %v", pdsErr) - } - defer func() { _ = pdsResp.Body.Close() }() - - if pdsResp.StatusCode != http.StatusOK { - t.Fatalf("Profile record not found on PDS: status %d", pdsResp.StatusCode) - } - t.Logf(" ✅ Profile record with avatar exists on PDS") - - t.Logf("\n✅ TRUE E2E AVATAR CREATE FLOW COMPLETE:") - t.Logf(" Service → PDS uploadBlob → PDS putRecord → Jetstream → Verified ✓") - }) -} - -// TestCommunityAvatarE2E_UpdateWithAvatar tests updating a community's avatar -// Flow: UpdateCommunity(avatar) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView -func TestCommunityAvatarE2E_UpdateWithAvatar(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - REQUIRED for true E2E - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.community.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - t.Logf("✅ Jetstream available at %s", jetstreamURL) - - ctx := context.Background() - instanceDID := "did:web:coves.social" - - // Setup identity resolver with local PLC - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - communityRepo := postgres.NewCommunityRepository(db) - provisioner := communities.NewPDSAccountProvisioner("coves.social", pdsURL) - blobService := blobs.NewBlobService(pdsURL) - - communityService := communities.NewCommunityServiceWithPDSFactory( - communityRepo, - pdsURL, - instanceDID, - "coves.social", - provisioner, - nil, - blobService, - ) - - consumer := jetstream.NewCommunityEventConsumer(communityRepo, instanceDID, true, identityResolver) - - // Helper to wait for Jetstream update event and process it - waitForUpdateEvent := func(t *testing.T, communityDID string, timeout time.Duration) *jetstream.JetstreamEvent { - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, timeout) - defer cancelSubscribe() - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - t.Logf("Failed to connect to Jetstream: %v", dialErr) - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { - return - } - - var event jetstream.JetstreamEvent - if readErr := conn.ReadJSON(&event); readErr != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.community.profile" && - event.Commit.Operation == "update" && - event.Did == communityDID { - eventChan <- &event - } - } - } - }() - - select { - case event := <-eventChan: - close(done) - return event - case <-time.After(timeout): - close(done) - return nil - } - } - - t.Run("add avatar to community without one", func(t *testing.T) { - uniqueName := fmt.Sprintf("upa%s", uniqueTestID()) - creatorDID := "did:plc:avatar-update-test" - - // Create a community WITHOUT an avatar - t.Logf("\n📝 Creating community without avatar...") - community, createErr := communityService.CreateCommunity(ctx, communities.CreateCommunityRequest{ - Name: uniqueName, - DisplayName: "Community Without Avatar", - Description: "Will add avatar via update", - Visibility: "public", - CreatedByDID: creatorDID, - HostedByDID: instanceDID, - AllowExternalDiscovery: true, - }) - if createErr != nil { - t.Fatalf("Failed to create community: %v", createErr) - } - t.Logf("✅ Community created: DID=%s", community.DID) - - // Verify no avatar initially - initial, err := communityService.GetCommunity(ctx, community.DID) - if err != nil { - t.Fatalf("Community not indexed: %v", err) - } - if initial.AvatarCID != "" { - t.Fatalf("Expected no initial avatar, got: %s", initial.AvatarCID) - } - t.Logf(" Initial AvatarCID: '' (confirmed empty)") - - // Create test avatar image (100x100 blue square) - avatarData := createTestPNGImage(100, 100, color.RGBA{0, 0, 255, 255}) - t.Logf("\n📝 Updating community with avatar (%d bytes)...", len(avatarData)) - - // Start listening for Jetstream event - eventReceived := make(chan *jetstream.JetstreamEvent, 1) - go func() { - event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) - eventReceived <- event - }() - time.Sleep(500 * time.Millisecond) // Give subscriber time to connect - - // Perform the update with avatar - newDisplayName := "Community With New Avatar" - updated, updateErr := communityService.UpdateCommunity(ctx, communities.UpdateCommunityRequest{ - CommunityDID: community.DID, - UpdatedByDID: creatorDID, - DisplayName: &newDisplayName, - AvatarBlob: avatarData, - AvatarMimeType: "image/png", - }) - if updateErr != nil { - t.Fatalf("Failed to update community with avatar: %v", updateErr) - } - - t.Logf("✅ Community update written to PDS:") - t.Logf(" New RecordCID: %s", updated.RecordCID) - - // Wait for REAL Jetstream event - t.Logf("\n⏳ Waiting for update event from Jetstream...") - realEvent := <-eventReceived - if realEvent == nil { - t.Fatalf("Timeout waiting for Jetstream update event") - } - - t.Logf("✅ Received REAL update event from Jetstream!") - t.Logf(" Operation: %s", realEvent.Commit.Operation) - t.Logf(" CID: %s", realEvent.Commit.CID) - - // Extract avatar CID from real event - var avatarCIDFromEvent string - if realEvent.Commit.Record != nil { - if avatar, hasAvatar := realEvent.Commit.Record["avatar"].(map[string]interface{}); hasAvatar { - t.Logf(" Avatar in event: %v", avatar) - if ref, hasRef := avatar["ref"].(map[string]interface{}); hasRef { - if link, hasLink := ref["$link"].(string); hasLink { - avatarCIDFromEvent = link - t.Logf(" AvatarCID from Jetstream: %s", avatarCIDFromEvent) - } - } - } - } - - // Process the REAL event through consumer - t.Logf("\n🔄 Processing real Jetstream event through consumer...") - if handleErr := consumer.HandleEvent(ctx, realEvent); handleErr != nil { - t.Logf(" Consumer error: %v", handleErr) - } - - // Verify avatar CID is now set in DB - final, err := communityRepo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get final community: %v", err) - } - - t.Logf("\n✅ Community avatar update verified:") - t.Logf(" DisplayName: %s", final.DisplayName) - t.Logf(" AvatarCID in DB: %s", final.AvatarCID) - - if final.AvatarCID == "" { - t.Errorf("Expected AvatarCID to be set after update") - } - - // Verify DB matches Jetstream event - if avatarCIDFromEvent != "" && final.AvatarCID != avatarCIDFromEvent { - t.Errorf("AvatarCID mismatch: DB has %s, Jetstream has %s", final.AvatarCID, avatarCIDFromEvent) - } else if avatarCIDFromEvent != "" { - t.Logf(" ✅ AvatarCID matches between DB and Jetstream!") - } - - t.Logf("\n✅ TRUE E2E ADD AVATAR FLOW COMPLETE") - }) - - t.Run("replace existing avatar with new one", func(t *testing.T) { - uniqueName := fmt.Sprintf("rpa%s", uniqueTestID()) - creatorDID := "did:plc:avatar-replace-test" - - // Create a community WITH an initial avatar (red square) - initialAvatarData := createTestPNGImage(100, 100, color.RGBA{255, 0, 0, 255}) - t.Logf("\n📝 Creating community with initial avatar (red, %d bytes)...", len(initialAvatarData)) - - community, createErr := communityService.CreateCommunity(ctx, communities.CreateCommunityRequest{ - Name: uniqueName, - DisplayName: "Community With Initial Avatar", - Description: "Will replace avatar", - Visibility: "public", - CreatedByDID: creatorDID, - HostedByDID: instanceDID, - AllowExternalDiscovery: true, - AvatarBlob: initialAvatarData, - AvatarMimeType: "image/png", - }) - if createErr != nil { - t.Fatalf("Failed to create community with avatar: %v", createErr) - } - t.Logf("✅ Community created: DID=%s", community.DID) - - // Verify initial avatar is set - initial, err := communityService.GetCommunity(ctx, community.DID) - if err != nil { - t.Fatalf("Community not indexed: %v", err) - } - initialAvatarCID := initial.AvatarCID - if initialAvatarCID == "" { - t.Fatalf("Expected initial avatar to be set") - } - t.Logf(" Initial AvatarCID: %s", initialAvatarCID) - - // Create NEW avatar image (100x100 green square - different from initial red) - newAvatarData := createTestPNGImage(100, 100, color.RGBA{0, 255, 0, 255}) - t.Logf("\n📝 Replacing avatar with new one (green, %d bytes)...", len(newAvatarData)) - - // Start listening for Jetstream event - eventReceived := make(chan *jetstream.JetstreamEvent, 1) - go func() { - event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) - eventReceived <- event - }() - time.Sleep(500 * time.Millisecond) - - // Perform the update with NEW avatar - newDisplayName := "Community With Replaced Avatar" - updated, updateErr := communityService.UpdateCommunity(ctx, communities.UpdateCommunityRequest{ - CommunityDID: community.DID, - UpdatedByDID: creatorDID, - DisplayName: &newDisplayName, - AvatarBlob: newAvatarData, - AvatarMimeType: "image/png", - }) - if updateErr != nil { - t.Fatalf("Failed to update community with new avatar: %v", updateErr) - } - - t.Logf("✅ Community update written to PDS:") - t.Logf(" New RecordCID: %s", updated.RecordCID) - - // Wait for REAL Jetstream event - t.Logf("\n⏳ Waiting for update event from Jetstream...") - realEvent := <-eventReceived - if realEvent == nil { - t.Fatalf("Timeout waiting for Jetstream update event") - } - - t.Logf("✅ Received REAL update event from Jetstream!") - t.Logf(" Operation: %s", realEvent.Commit.Operation) - - // Extract new avatar CID from real event - var newAvatarCIDFromEvent string - if realEvent.Commit.Record != nil { - if avatar, hasAvatar := realEvent.Commit.Record["avatar"].(map[string]interface{}); hasAvatar { - if ref, hasRef := avatar["ref"].(map[string]interface{}); hasRef { - if link, hasLink := ref["$link"].(string); hasLink { - newAvatarCIDFromEvent = link - t.Logf(" New AvatarCID from Jetstream: %s", newAvatarCIDFromEvent) - } - } - } - } - - // Process the REAL event through consumer - t.Logf("\n🔄 Processing real Jetstream event through consumer...") - if handleErr := consumer.HandleEvent(ctx, realEvent); handleErr != nil { - t.Logf(" Consumer error: %v", handleErr) - } - - // Verify avatar CID has CHANGED - final, err := communityRepo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get final community: %v", err) - } - - t.Logf("\n✅ Community avatar replacement verified:") - t.Logf(" DisplayName: %s", final.DisplayName) - t.Logf(" Old AvatarCID: %s", initialAvatarCID) - t.Logf(" New AvatarCID: %s", final.AvatarCID) - - if final.AvatarCID == "" { - t.Errorf("Expected AvatarCID to be set after replacement") - } - - if final.AvatarCID == initialAvatarCID { - t.Errorf("AvatarCID should have changed after replacement! Old: %s, New: %s", initialAvatarCID, final.AvatarCID) - } else { - t.Logf(" ✅ AvatarCID successfully changed!") - } - - // Verify DB matches Jetstream event - if newAvatarCIDFromEvent != "" && final.AvatarCID != newAvatarCIDFromEvent { - t.Errorf("AvatarCID mismatch: DB has %s, Jetstream has %s", final.AvatarCID, newAvatarCIDFromEvent) - } else if newAvatarCIDFromEvent != "" { - t.Logf(" ✅ New AvatarCID matches between DB and Jetstream!") - } - - t.Logf("\n✅ TRUE E2E REPLACE AVATAR FLOW COMPLETE") - }) -} - -// TestCommunityAvatarE2E_UpdateWithBanner tests updating a community's banner -// Flow: UpdateCommunity(banner) → PDS uploadBlob + putRecord → Jetstream → Consumer → AppView -func TestCommunityAvatarE2E_UpdateWithBanner(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - REQUIRED for true E2E - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.community.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - t.Logf("✅ Jetstream available at %s", jetstreamURL) - - ctx := context.Background() - instanceDID := "did:web:coves.social" - - // Setup identity resolver with local PLC - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - communityRepo := postgres.NewCommunityRepository(db) - provisioner := communities.NewPDSAccountProvisioner("coves.social", pdsURL) - blobService := blobs.NewBlobService(pdsURL) - - communityService := communities.NewCommunityServiceWithPDSFactory( - communityRepo, - pdsURL, - instanceDID, - "coves.social", - provisioner, - nil, - blobService, - ) - - consumer := jetstream.NewCommunityEventConsumer(communityRepo, instanceDID, true, identityResolver) - - // Helper to wait for Jetstream update event and process it - waitForUpdateEvent := func(t *testing.T, communityDID string, timeout time.Duration) *jetstream.JetstreamEvent { - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, timeout) - defer cancelSubscribe() - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - t.Logf("Failed to connect to Jetstream: %v", dialErr) - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { - return - } - - var event jetstream.JetstreamEvent - if readErr := conn.ReadJSON(&event); readErr != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.community.profile" && - event.Commit.Operation == "update" && - event.Did == communityDID { - eventChan <- &event - } - } - } - }() - - select { - case event := <-eventChan: - close(done) - return event - case <-time.After(timeout): - close(done) - return nil - } - } - - t.Run("add banner to community without one", func(t *testing.T) { - uniqueName := fmt.Sprintf("ban%s", uniqueTestID()) - creatorDID := "did:plc:banner-add-test" - - // Create a community WITHOUT a banner - t.Logf("\n📝 Creating community without banner...") - community, createErr := communityService.CreateCommunity(ctx, communities.CreateCommunityRequest{ - Name: uniqueName, - DisplayName: "Community Without Banner", - Description: "Will add banner via update", - Visibility: "public", - CreatedByDID: creatorDID, - HostedByDID: instanceDID, - AllowExternalDiscovery: true, - }) - if createErr != nil { - t.Fatalf("Failed to create community: %v", createErr) - } - t.Logf("✅ Community created: DID=%s", community.DID) - - // Verify no banner initially - initial, err := communityService.GetCommunity(ctx, community.DID) - if err != nil { - t.Fatalf("Community not indexed: %v", err) - } - if initial.BannerCID != "" { - t.Fatalf("Expected no initial banner, got: %s", initial.BannerCID) - } - t.Logf(" Initial BannerCID: '' (confirmed empty)") - - // Create test banner image (300x100 green rectangle) - bannerData := createTestPNGImage(300, 100, color.RGBA{0, 255, 0, 255}) - t.Logf("\n📝 Updating community with banner (%d bytes)...", len(bannerData)) - - // Start listening for Jetstream event - eventReceived := make(chan *jetstream.JetstreamEvent, 1) - go func() { - event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) - eventReceived <- event - }() - time.Sleep(500 * time.Millisecond) // Give subscriber time to connect - - // Perform the update with banner - newDisplayName := "Community With New Banner" - updated, updateErr := communityService.UpdateCommunity(ctx, communities.UpdateCommunityRequest{ - CommunityDID: community.DID, - UpdatedByDID: creatorDID, - DisplayName: &newDisplayName, - BannerBlob: bannerData, - BannerMimeType: "image/png", - }) - if updateErr != nil { - t.Fatalf("Failed to update community with banner: %v", updateErr) - } - - t.Logf("✅ Community update written to PDS:") - t.Logf(" New RecordCID: %s", updated.RecordCID) - - // Wait for REAL Jetstream event - t.Logf("\n⏳ Waiting for update event from Jetstream...") - realEvent := <-eventReceived - if realEvent == nil { - t.Fatalf("Timeout waiting for Jetstream update event") - } - - t.Logf("✅ Received REAL update event from Jetstream!") - t.Logf(" Operation: %s", realEvent.Commit.Operation) - t.Logf(" CID: %s", realEvent.Commit.CID) - - // Extract banner CID from real event - var bannerCIDFromEvent string - if realEvent.Commit.Record != nil { - if banner, hasBanner := realEvent.Commit.Record["banner"].(map[string]interface{}); hasBanner { - t.Logf(" Banner in event: %v", banner) - if ref, hasRef := banner["ref"].(map[string]interface{}); hasRef { - if link, hasLink := ref["$link"].(string); hasLink { - bannerCIDFromEvent = link - t.Logf(" BannerCID from Jetstream: %s", bannerCIDFromEvent) - } - } - } - } - - // Process the REAL event through consumer - t.Logf("\n🔄 Processing real Jetstream event through consumer...") - if handleErr := consumer.HandleEvent(ctx, realEvent); handleErr != nil { - t.Logf(" Consumer error: %v", handleErr) - } - - // Verify banner CID is now set in DB - final, err := communityRepo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get final community: %v", err) - } - - t.Logf("\n✅ Community banner update verified:") - t.Logf(" DisplayName: %s", final.DisplayName) - t.Logf(" BannerCID in DB: %s", final.BannerCID) - - if final.BannerCID == "" { - t.Errorf("Expected BannerCID to be set after update") - } - - // Verify DB matches Jetstream event - if bannerCIDFromEvent != "" && final.BannerCID != bannerCIDFromEvent { - t.Errorf("BannerCID mismatch: DB has %s, Jetstream has %s", final.BannerCID, bannerCIDFromEvent) - } else if bannerCIDFromEvent != "" { - t.Logf(" ✅ BannerCID matches between DB and Jetstream!") - } - - t.Logf("\n✅ TRUE E2E ADD BANNER FLOW COMPLETE") - }) - - t.Run("replace existing banner with new one", func(t *testing.T) { - uniqueName := fmt.Sprintf("rpb%s", uniqueTestID()) - creatorDID := "did:plc:banner-replace-test" - - // Create a community WITH an initial banner (red rectangle) - initialBannerData := createTestPNGImage(300, 100, color.RGBA{255, 0, 0, 255}) - t.Logf("\n📝 Creating community with initial banner (red, %d bytes)...", len(initialBannerData)) - - community, createErr := communityService.CreateCommunity(ctx, communities.CreateCommunityRequest{ - Name: uniqueName, - DisplayName: "Community With Initial Banner", - Description: "Will replace banner", - Visibility: "public", - CreatedByDID: creatorDID, - HostedByDID: instanceDID, - AllowExternalDiscovery: true, - BannerBlob: initialBannerData, - BannerMimeType: "image/png", - }) - if createErr != nil { - t.Fatalf("Failed to create community with banner: %v", createErr) - } - t.Logf("✅ Community created: DID=%s", community.DID) - - // Verify initial banner is set - initial, err := communityService.GetCommunity(ctx, community.DID) - if err != nil { - t.Fatalf("Community not indexed: %v", err) - } - initialBannerCID := initial.BannerCID - if initialBannerCID == "" { - t.Fatalf("Expected initial banner to be set") - } - t.Logf(" Initial BannerCID: %s", initialBannerCID) - - // Create NEW banner image (300x100 blue rectangle - different from initial red) - newBannerData := createTestPNGImage(300, 100, color.RGBA{0, 0, 255, 255}) - t.Logf("\n📝 Replacing banner with new one (blue, %d bytes)...", len(newBannerData)) - - // Start listening for Jetstream event - eventReceived := make(chan *jetstream.JetstreamEvent, 1) - go func() { - event := waitForUpdateEvent(t, community.DID, jetstreamReadBudget) - eventReceived <- event - }() - time.Sleep(500 * time.Millisecond) - - // Perform the update with NEW banner - newDisplayName := "Community With Replaced Banner" - updated, updateErr := communityService.UpdateCommunity(ctx, communities.UpdateCommunityRequest{ - CommunityDID: community.DID, - UpdatedByDID: creatorDID, - DisplayName: &newDisplayName, - BannerBlob: newBannerData, - BannerMimeType: "image/png", - }) - if updateErr != nil { - t.Fatalf("Failed to update community with new banner: %v", updateErr) - } - - t.Logf("✅ Community update written to PDS:") - t.Logf(" New RecordCID: %s", updated.RecordCID) - - // Wait for REAL Jetstream event - t.Logf("\n⏳ Waiting for update event from Jetstream...") - realEvent := <-eventReceived - if realEvent == nil { - t.Fatalf("Timeout waiting for Jetstream update event") - } - - t.Logf("✅ Received REAL update event from Jetstream!") - t.Logf(" Operation: %s", realEvent.Commit.Operation) - - // Extract new banner CID from real event - var newBannerCIDFromEvent string - if realEvent.Commit.Record != nil { - if banner, hasBanner := realEvent.Commit.Record["banner"].(map[string]interface{}); hasBanner { - if ref, hasRef := banner["ref"].(map[string]interface{}); hasRef { - if link, hasLink := ref["$link"].(string); hasLink { - newBannerCIDFromEvent = link - t.Logf(" New BannerCID from Jetstream: %s", newBannerCIDFromEvent) - } - } - } - } - - // Process the REAL event through consumer - t.Logf("\n🔄 Processing real Jetstream event through consumer...") - if handleErr := consumer.HandleEvent(ctx, realEvent); handleErr != nil { - t.Logf(" Consumer error: %v", handleErr) - } - - // Verify banner CID has CHANGED - final, err := communityRepo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get final community: %v", err) - } - - t.Logf("\n✅ Community banner replacement verified:") - t.Logf(" DisplayName: %s", final.DisplayName) - t.Logf(" Old BannerCID: %s", initialBannerCID) - t.Logf(" New BannerCID: %s", final.BannerCID) - - if final.BannerCID == "" { - t.Errorf("Expected BannerCID to be set after replacement") - } - - if final.BannerCID == initialBannerCID { - t.Errorf("BannerCID should have changed after replacement! Old: %s, New: %s", initialBannerCID, final.BannerCID) - } else { - t.Logf(" ✅ BannerCID successfully changed!") - } - - // Verify DB matches Jetstream event - if newBannerCIDFromEvent != "" && final.BannerCID != newBannerCIDFromEvent { - t.Errorf("BannerCID mismatch: DB has %s, Jetstream has %s", final.BannerCID, newBannerCIDFromEvent) - } else if newBannerCIDFromEvent != "" { - t.Logf(" ✅ New BannerCID matches between DB and Jetstream!") - } - - t.Logf("\n✅ TRUE E2E REPLACE BANNER FLOW COMPLETE") - }) -} diff --git a/tests/integration/helpers.go b/tests/integration/helpers.go index 126fd6d..1d33c8d 100644 --- a/tests/integration/helpers.go +++ b/tests/integration/helpers.go @@ -492,8 +492,3 @@ func CommunityPasswordAuthPDSClientFactory() communities.PDSClientFactory { func UserBlockPasswordAuthPDSClientFactory() userblocks.PDSClientFactory { return passwordAuthPDSClient } - -// UserProfilePasswordAuthPDSClientFactory creates a PDSClientFactory for user profile E2E tests. -func UserProfilePasswordAuthPDSClientFactory() func(ctx context.Context, session *oauthlib.ClientSessionData) (pds.Client, error) { - return passwordAuthPDSClient -} diff --git a/tests/integration/oauth_session_handle_sync_test.go b/tests/integration/oauth_session_handle_sync_test.go deleted file mode 100644 index 8fa2f6b..0000000 --- a/tests/integration/oauth_session_handle_sync_test.go +++ /dev/null @@ -1,370 +0,0 @@ -//go:build integration - -package integration - -import ( - "context" - "errors" - "fmt" - "net/http" - "testing" - "time" - - oauthlib "github.com/bluesky-social/indigo/atproto/auth/oauth" - "github.com/bluesky-social/indigo/atproto/syntax" - "github.com/stretchr/testify/require" - - "Coves/internal/atproto/identity" - "Coves/internal/atproto/jetstream" - "Coves/internal/atproto/oauth" - "Coves/internal/core/users" - "Coves/internal/db/postgres" - "Coves/tests/testkit" -) - -// TestOAuthSessionHandleSync tests that OAuth session handles are updated -// when identity events indicate a handle change. -// -// This ensures mobile/web apps display the correct handle after a user -// changes their handle on their PDS. -// -// Run with `make test-integration`, or against an already-running dev stack: -// -// go test -tags integration ./tests/integration/ -run TestOAuthSessionHandleSync -func TestOAuthSessionHandleSync(t *testing.T) { - t.Parallel() - db := testkit.DB(t) - - ctx := context.Background() - - // Set up real infrastructure components - userRepo := postgres.NewUserRepository(db) - resolver := identity.NewResolver(db, identity.DefaultConfig()) - userService := users.NewUserService(userRepo, resolver, "http://localhost:3001", nil, "") - - // Create real OAuth store (with session handle updater capability) - baseOAuthStore := oauth.NewPostgresOAuthStore(db, 24*time.Hour) - - t.Run("Handle change syncs to active OAuth sessions", func(t *testing.T) { - testDID := "did:plc:oauthsync123" - oldHandle := "oldhandle.oauth.sync.test" - newHandle := "newhandle.oauth.sync.test" - sessionID := "test-session-oauth-sync-001" - - // 1. Create user with old handle - _, err := userService.CreateUser(ctx, users.CreateUserRequest{ - DID: testDID, - Handle: oldHandle, - PDSURL: "https://bsky.social", - }) - require.NoError(t, err, "Failed to create test user") - t.Logf("✅ Created user: %s (%s)", oldHandle, testDID) - - // 2. Create OAuth session with old handle - parsedDID, err := syntax.ParseDID(testDID) - require.NoError(t, err, "Failed to parse DID") - - session := oauthlib.ClientSessionData{ - AccountDID: parsedDID, - SessionID: sessionID, - HostURL: "https://bsky.social", - AccessToken: "test-access-token", - RefreshToken: "test-refresh-token", - Scopes: []string{"atproto"}, - } - err = baseOAuthStore.SaveSession(ctx, session) - require.NoError(t, err, "Failed to save OAuth session") - t.Logf("✅ Created OAuth session: %s", sessionID) - - // 3. Verify session was created with correct data - savedSession, err := baseOAuthStore.GetSession(ctx, parsedDID, sessionID) - require.NoError(t, err, "Failed to retrieve saved session") - require.NotNil(t, savedSession, "Session should exist") - t.Logf("✅ Verified session exists for DID: %s", testDID) - - // 4. Cast store to SessionHandleUpdater (what the consumer uses) - sessionUpdater, ok := baseOAuthStore.(jetstream.SessionHandleUpdater) - require.True(t, ok, "OAuth store should implement SessionHandleUpdater") - - // 5. Create consumer with session handle updater - consumer := jetstream.NewUserEventConsumer( - userService, - resolver, - jetstream.WithSessionHandleUpdater(sessionUpdater), - ) - - // 6. Simulate identity event with NEW handle (as if PDS sent handle change) - identityEvent := &jetstream.JetstreamEvent{ - Did: testDID, - Kind: "identity", - Identity: &jetstream.IdentityEvent{ - Did: testDID, - Handle: newHandle, - Seq: 999999, - Time: time.Now().Format(time.RFC3339), - }, - } - - t.Logf("📡 Simulating identity event: %s → %s", oldHandle, newHandle) - err = consumer.HandleIdentityEventPublic(ctx, identityEvent) - require.NoError(t, err, "Failed to handle identity event") - t.Logf("✅ Identity event processed") - - // 7. Verify users table was updated - user, err := userService.GetUserByDID(ctx, testDID) - require.NoError(t, err, "Failed to get user after handle change") - require.Equal(t, newHandle, user.Handle, "User handle should be updated in database") - t.Logf("✅ Users table updated: handle=%s", user.Handle) - - // 8. Verify OAuth session handle was updated - var sessionHandle string - err = db.QueryRowContext(ctx, - "SELECT handle FROM oauth_sessions WHERE did = $1 AND session_id = $2", - testDID, sessionID, - ).Scan(&sessionHandle) - require.NoError(t, err, "Failed to query session handle") - require.Equal(t, newHandle, sessionHandle, "OAuth session handle should be updated") - t.Logf("✅ OAuth session handle updated: %s", sessionHandle) - }) - - t.Run("Multiple sessions updated on handle change", func(t *testing.T) { - testDID := "did:plc:multisession456" - oldHandle := "multi.old.handle.test" - newHandle := "multi.new.handle.test" - - // 1. Create user - _, err := userService.CreateUser(ctx, users.CreateUserRequest{ - DID: testDID, - Handle: oldHandle, - PDSURL: "https://bsky.social", - }) - require.NoError(t, err) - - // 2. Create multiple OAuth sessions (simulating login from multiple devices) - parsedDID, _ := syntax.ParseDID(testDID) - for i := 1; i <= 3; i++ { - session := oauthlib.ClientSessionData{ - AccountDID: parsedDID, - SessionID: fmt.Sprintf("multi-session-%d", i), - HostURL: "https://bsky.social", - AccessToken: fmt.Sprintf("access-token-%d", i), - RefreshToken: fmt.Sprintf("refresh-token-%d", i), - Scopes: []string{"atproto"}, - } - err = baseOAuthStore.SaveSession(ctx, session) - require.NoError(t, err) - } - t.Logf("✅ Created 3 OAuth sessions for user") - - // 3. Process identity event with new handle - sessionUpdater := baseOAuthStore.(jetstream.SessionHandleUpdater) - consumer := jetstream.NewUserEventConsumer( - userService, resolver, - jetstream.WithSessionHandleUpdater(sessionUpdater), - ) - - identityEvent := &jetstream.JetstreamEvent{ - Did: testDID, - Kind: "identity", - Identity: &jetstream.IdentityEvent{ - Did: testDID, - Handle: newHandle, - Seq: 888888, - Time: time.Now().Format(time.RFC3339), - }, - } - - err = consumer.HandleIdentityEventPublic(ctx, identityEvent) - require.NoError(t, err) - - // 4. Verify ALL sessions were updated - var count int - err = db.QueryRowContext(ctx, - "SELECT COUNT(*) FROM oauth_sessions WHERE did = $1 AND handle = $2", - testDID, newHandle, - ).Scan(&count) - require.NoError(t, err) - require.Equal(t, 3, count, "All 3 sessions should have updated handles") - t.Logf("✅ All %d sessions updated with new handle", count) - }) - - t.Run("No sessions updated when user has no active sessions", func(t *testing.T) { - testDID := "did:plc:nosessions789" - oldHandle := "nosession.old.test" - newHandle := "nosession.new.test" - - // 1. Create user with no OAuth sessions - _, err := userService.CreateUser(ctx, users.CreateUserRequest{ - DID: testDID, - Handle: oldHandle, - PDSURL: "https://bsky.social", - }) - require.NoError(t, err) - - // 2. Process identity event - sessionUpdater := baseOAuthStore.(jetstream.SessionHandleUpdater) - consumer := jetstream.NewUserEventConsumer( - userService, resolver, - jetstream.WithSessionHandleUpdater(sessionUpdater), - ) - - identityEvent := &jetstream.JetstreamEvent{ - Did: testDID, - Kind: "identity", - Identity: &jetstream.IdentityEvent{ - Did: testDID, - Handle: newHandle, - Seq: 777777, - Time: time.Now().Format(time.RFC3339), - }, - } - - // Should not error even when no sessions exist - err = consumer.HandleIdentityEventPublic(ctx, identityEvent) - require.NoError(t, err, "Should handle event gracefully with no sessions") - - // 3. Verify user was still updated - user, err := userService.GetUserByDID(ctx, testDID) - require.NoError(t, err) - require.Equal(t, newHandle, user.Handle) - t.Logf("✅ User updated correctly even with no active sessions") - }) - - t.Run("Consumer works without session updater (backward compat)", func(t *testing.T) { - testDID := "did:plc:nosyncer000" - oldHandle := "nosyncer.old.test" - newHandle := "nosyncer.new.test" - - // 1. Create user - _, err := userService.CreateUser(ctx, users.CreateUserRequest{ - DID: testDID, - Handle: oldHandle, - PDSURL: "https://bsky.social", - }) - require.NoError(t, err) - - // 2. Create consumer WITHOUT session handle updater - consumer := jetstream.NewUserEventConsumer( - userService, resolver, - // No WithSessionHandleUpdater - testing backward compatibility - ) - - // 3. Process identity event - should work without error - identityEvent := &jetstream.JetstreamEvent{ - Did: testDID, - Kind: "identity", - Identity: &jetstream.IdentityEvent{ - Did: testDID, - Handle: newHandle, - Seq: 666666, - Time: time.Now().Format(time.RFC3339), - }, - } - - err = consumer.HandleIdentityEventPublic(ctx, identityEvent) - require.NoError(t, err, "Consumer should work without session updater") - - // 4. Verify user was updated - user, err := userService.GetUserByDID(ctx, testDID) - require.NoError(t, err) - require.Equal(t, newHandle, user.Handle) - t.Logf("✅ Consumer works correctly without session handle updater") - }) -} - -// TestOAuthSessionHandleSync_LiveJetstream tests the full flow with real Jetstream -// This requires the dev infrastructure to be running. -// -// Run with `make test-integration` (which brings the stack up), or directly -// against a running one: -// -// go test -tags integration ./tests/integration/ -run TestOAuthSessionHandleSync_LiveJetstream -// -// SERIAL BY DESIGN — no t.Parallel(): this is the one test in the file that -// runs a connector against the shared live stream rather than feeding a -// consumer in process. -func TestOAuthSessionHandleSync_LiveJetstream(t *testing.T) { - // Check if Jetstream is available - if !isServiceAvailable("http://localhost:6008") { - t.Skip("Jetstream not available at localhost:6008 - run 'docker-compose --profile jetstream up -d' first") - } - - // Check if PDS is available - if !isServiceAvailable("http://localhost:3001/xrpc/_health") { - t.Skip("PDS not available at localhost:3001 - run 'docker-compose up -d pds' first") - } - - db := testkit.DB(t) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - // Set up real infrastructure - userRepo := postgres.NewUserRepository(db) - resolver := identity.NewResolver(db, identity.DefaultConfig()) - userService := users.NewUserService(userRepo, resolver, "http://localhost:3001", nil, "") - baseOAuthStore := oauth.NewPostgresOAuthStore(db, 24*time.Hour) - sessionUpdater := baseOAuthStore.(jetstream.SessionHandleUpdater) - - // Start consumer connected to real Jetstream - consumer := jetstream.NewUserEventConsumer( - userService, - resolver, - jetstream.WithSessionHandleUpdater(sessionUpdater), - ) - connector := jetstream.NewConnector("users-test", "ws://localhost:6008/subscribe", consumer) - - // Start consumer in background, and JOIN IT before returning. - // - // Without the join this goroutine outlives the test, and its t.Logf then - // panics the whole binary with "Log in goroutine after test has completed". - // That is not theoretical here: the clone this consumer writes to is - // dropped WITH (FORCE) in testkit.DB's cleanup, so Start returns a database - // error rather than context.Canceled, sails past the guard, and logs. - consumerCtx, consumerCancel := context.WithCancel(ctx) - consumerStopped := make(chan error, 1) - go func() { consumerStopped <- connector.Start(consumerCtx) }() - t.Cleanup(func() { - consumerCancel() - select { - case err := <-consumerStopped: - // Reported after the join, so it is the test's own goroutine - // logging, and only when it is not the shutdown we asked for. - if err != nil && !errors.Is(err, context.Canceled) { - t.Logf("Consumer stopped: %v", err) - } - case <-time.After(10 * time.Second): - t.Errorf("consumer goroutine did not stop within 10s of cancellation") - } - }) - - // Give consumer time to connect - time.Sleep(500 * time.Millisecond) - - t.Run("Real Jetstream integration", func(t *testing.T) { - t.Log("🔌 Connected to live Jetstream - waiting for identity events...") - t.Log("Note: This test verifies the consumer is properly configured with session sync.") - t.Log("To fully test handle sync, create a user on the PDS and change their handle.") - - // For now, just verify the consumer is running with the session updater - // A full E2E test would require: - // 1. Create user on PDS - // 2. Create OAuth session - // 3. Update handle on PDS (via user credentials) - // 4. Wait for Jetstream to deliver identity event - // 5. Verify session handle updated - - t.Log("✅ Consumer running with OAuth session sync enabled") - }) -} - -// isServiceAvailable checks if an HTTP service is responding -func isServiceAvailable(url string) bool { - client := &http.Client{Timeout: 2 * time.Second} - resp, err := client.Get(url) - if err != nil { - return false - } - defer resp.Body.Close() - return resp.StatusCode < 500 -} diff --git a/tests/integration/subscription_indexing_test.go b/tests/integration/subscription_indexing_test.go deleted file mode 100644 index 520af31..0000000 --- a/tests/integration/subscription_indexing_test.go +++ /dev/null @@ -1,489 +0,0 @@ -//go:build integration - -package integration - -import ( - "Coves/internal/atproto/jetstream" - "Coves/internal/core/communities" - "Coves/tests/testkit" - "context" - "database/sql" - "fmt" - "testing" - "time" - - postgresRepo "Coves/internal/db/postgres" -) - -// TestSubscriptionIndexing_ContentVisibility tests that contentVisibility is properly indexed -// from Jetstream events and stored in the AppView database -func TestSubscriptionIndexing_ContentVisibility(t *testing.T) { - t.Parallel() - ctx := context.Background() - db := testkit.DB(t) - - repo := createTestCommunityRepo(t, db) - // Skip verification in tests - // Pass nil for identity resolver - not needed since consumer constructs handles from DIDs - consumer := jetstream.NewCommunityEventConsumer(repo, "did:web:coves.local", true, nil) - - // Create a test community first (with unique DID) - testDID := fmt.Sprintf("did:plc:test-community-%d", time.Now().UnixNano()) - community := createTestCommunity(t, repo, "test-community-visibility", testDID) - - t.Run("indexes subscription with contentVisibility=5", func(t *testing.T) { - userDID := "did:plc:test-user-123" - rkey := "test-sub-1" - uri := "at://" + userDID + "/social.coves.community.subscription/" + rkey - - // Simulate Jetstream CREATE event for subscription - event := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-1", - Operation: "create", - Collection: "social.coves.community.subscription", // CORRECT collection name - RKey: rkey, - CID: "bafytest123", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - "contentVisibility": float64(5), // JSON numbers decode as float64 - }, - }, - } - - // Process event through consumer - err := consumer.HandleEvent(ctx, event) - if err != nil { - t.Fatalf("Failed to handle subscription event: %v", err) - } - - // Verify subscription was indexed with correct contentVisibility - subscription, err := repo.GetSubscription(ctx, userDID, community.DID) - if err != nil { - t.Fatalf("Failed to get subscription: %v", err) - } - - if subscription.ContentVisibility != 5 { - t.Errorf("Expected contentVisibility=5, got %d", subscription.ContentVisibility) - } - - if subscription.UserDID != userDID { - t.Errorf("Expected userDID=%s, got %s", userDID, subscription.UserDID) - } - - if subscription.CommunityDID != community.DID { - t.Errorf("Expected communityDID=%s, got %s", community.DID, subscription.CommunityDID) - } - - if subscription.RecordURI != uri { - t.Errorf("Expected recordURI=%s, got %s", uri, subscription.RecordURI) - } - - t.Logf("✓ Subscription indexed with contentVisibility=5") - }) - - t.Run("defaults to contentVisibility=3 when not provided", func(t *testing.T) { - userDID := "did:plc:test-user-default" - rkey := "test-sub-default" - - // Simulate Jetstream CREATE event WITHOUT contentVisibility field - event := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-default", - Operation: "create", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "bafydefault", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - // contentVisibility NOT provided - }, - }, - } - - // Process event - err := consumer.HandleEvent(ctx, event) - if err != nil { - t.Fatalf("Failed to handle subscription event: %v", err) - } - - // Verify defaults to 3 - subscription, err := repo.GetSubscription(ctx, userDID, community.DID) - if err != nil { - t.Fatalf("Failed to get subscription: %v", err) - } - - if subscription.ContentVisibility != 3 { - t.Errorf("Expected contentVisibility=3 (default), got %d", subscription.ContentVisibility) - } - - t.Logf("✓ Subscription defaulted to contentVisibility=3") - }) - - t.Run("clamps contentVisibility to valid range (1-5)", func(t *testing.T) { - testCases := []struct { - name string - input float64 - expected int - }{ - {input: 0, expected: 1, name: "zero clamped to 1"}, - {input: -5, expected: 1, name: "negative clamped to 1"}, - {input: 10, expected: 5, name: "10 clamped to 5"}, - {input: 100, expected: 5, name: "100 clamped to 5"}, - {input: 1, expected: 1, name: "1 stays 1"}, - {input: 3, expected: 3, name: "3 stays 3"}, - {input: 5, expected: 5, name: "5 stays 5"}, - } - - for i, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - userDID := fmt.Sprintf("did:plc:test-clamp-%d", i) - rkey := fmt.Sprintf("test-sub-clamp-%d", i) - - event := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-clamp", - Operation: "create", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "bafyclamp", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - "contentVisibility": tc.input, - }, - }, - } - - err := consumer.HandleEvent(ctx, event) - if err != nil { - t.Fatalf("Failed to handle subscription event: %v", err) - } - - subscription, err := repo.GetSubscription(ctx, userDID, community.DID) - if err != nil { - t.Fatalf("Failed to get subscription: %v", err) - } - - if subscription.ContentVisibility != tc.expected { - t.Errorf("Input %.0f: expected %d, got %d", tc.input, tc.expected, subscription.ContentVisibility) - } - - t.Logf("✓ Input %.0f clamped to %d", tc.input, subscription.ContentVisibility) - }) - } - }) - - t.Run("idempotency: duplicate subscription events don't fail", func(t *testing.T) { - userDID := "did:plc:test-idempotent" - rkey := "test-sub-idempotent" - - event := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-idempotent", - Operation: "create", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "bafyidempotent", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - "contentVisibility": float64(4), - }, - }, - } - - // Process first time - err := consumer.HandleEvent(ctx, event) - if err != nil { - t.Fatalf("Failed to handle first subscription event: %v", err) - } - - // Process again (Jetstream replay scenario) - err = consumer.HandleEvent(ctx, event) - if err != nil { - t.Errorf("Idempotency failed: second event should not error, got: %v", err) - } - - // Verify only one subscription exists - subscription, err := repo.GetSubscription(ctx, userDID, community.DID) - if err != nil { - t.Fatalf("Failed to get subscription: %v", err) - } - - if subscription.ContentVisibility != 4 { - t.Errorf("Expected contentVisibility=4, got %d", subscription.ContentVisibility) - } - - t.Logf("✓ Duplicate events handled idempotently") - }) -} - -// TestSubscriptionIndexing_DeleteOperations tests unsubscribe (DELETE) event handling -func TestSubscriptionIndexing_DeleteOperations(t *testing.T) { - t.Parallel() - ctx := context.Background() - db := testkit.DB(t) - - repo := createTestCommunityRepo(t, db) - // Skip verification in tests - // Pass nil for identity resolver - not needed since consumer constructs handles from DIDs - consumer := jetstream.NewCommunityEventConsumer(repo, "did:web:coves.local", true, nil) - - // Create test community (with unique DID) - testDID := fmt.Sprintf("did:plc:test-unsub-%d", time.Now().UnixNano()) - community := createTestCommunity(t, repo, "test-unsubscribe", testDID) - - t.Run("deletes subscription when DELETE event received", func(t *testing.T) { - userDID := "did:plc:test-user-delete" - rkey := "test-sub-delete" - - // First, create a subscription - createEvent := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-create", - Operation: "create", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "bafycreate", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - "contentVisibility": float64(3), - }, - }, - } - - err := consumer.HandleEvent(ctx, createEvent) - if err != nil { - t.Fatalf("Failed to create subscription: %v", err) - } - - // Verify subscription exists - _, err = repo.GetSubscription(ctx, userDID, community.DID) - if err != nil { - t.Fatalf("Subscription should exist: %v", err) - } - - // Now send DELETE event (unsubscribe) - // IMPORTANT: DELETE operations don't include record data in Jetstream - deleteEvent := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-delete", - Operation: "delete", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "", // No CID on deletes - Record: nil, // No record data on deletes - }, - } - - err = consumer.HandleEvent(ctx, deleteEvent) - if err != nil { - t.Fatalf("Failed to handle delete event: %v", err) - } - - // Verify subscription was deleted - _, err = repo.GetSubscription(ctx, userDID, community.DID) - if err == nil { - t.Errorf("Subscription should have been deleted") - } - if !communities.IsNotFound(err) { - t.Errorf("Expected NotFound error, got: %v", err) - } - - t.Logf("✓ Subscription deleted successfully") - }) - - t.Run("idempotent delete: deleting non-existent subscription doesn't fail", func(t *testing.T) { - userDID := "did:plc:test-user-noexist" - rkey := "test-sub-noexist" - - // Try to delete a subscription that doesn't exist - deleteEvent := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-noexist", - Operation: "delete", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "", - Record: nil, - }, - } - - // Should not error (idempotent) - err := consumer.HandleEvent(ctx, deleteEvent) - if err != nil { - t.Errorf("Deleting non-existent subscription should not error, got: %v", err) - } - - t.Logf("✓ Idempotent delete handled gracefully") - }) -} - -// TestSubscriptionIndexing_SubscriberCount tests that subscriber counts are updated atomically -func TestSubscriptionIndexing_SubscriberCount(t *testing.T) { - t.Parallel() - ctx := context.Background() - db := testkit.DB(t) - - repo := createTestCommunityRepo(t, db) - // Skip verification in tests - // Pass nil for identity resolver - not needed since consumer constructs handles from DIDs - consumer := jetstream.NewCommunityEventConsumer(repo, "did:web:coves.local", true, nil) - - // Create test community (with unique DID) - testDID := fmt.Sprintf("did:plc:test-subcount-%d", time.Now().UnixNano()) - community := createTestCommunity(t, repo, "test-subscriber-count", testDID) - - // Verify initial subscriber count is 0 - comm, err := repo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get community: %v", err) - } - if comm.SubscriberCount != 0 { - t.Errorf("Initial subscriber count should be 0, got %d", comm.SubscriberCount) - } - - t.Run("increments subscriber count on subscribe", func(t *testing.T) { - userDID := "did:plc:test-user-count1" - rkey := "test-sub-count1" - - event := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-count", - Operation: "create", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "bafycount", - Record: map[string]interface{}{ - "$type": "social.coves.community.subscription", - "subject": community.DID, - "createdAt": time.Now().Format(time.RFC3339), - "contentVisibility": float64(3), - }, - }, - } - - err := consumer.HandleEvent(ctx, event) - if err != nil { - t.Fatalf("Failed to handle subscription: %v", err) - } - - // Check subscriber count incremented - comm, err := repo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get community: %v", err) - } - - if comm.SubscriberCount != 1 { - t.Errorf("Subscriber count should be 1, got %d", comm.SubscriberCount) - } - - t.Logf("✓ Subscriber count incremented to 1") - }) - - t.Run("decrements subscriber count on unsubscribe", func(t *testing.T) { - userDID := "did:plc:test-user-count1" // Same user from above - rkey := "test-sub-count1" - - // Send DELETE event - deleteEvent := &jetstream.JetstreamEvent{ - Did: userDID, - Kind: "commit", - TimeUS: time.Now().UnixMicro(), - Commit: &jetstream.CommitEvent{ - Rev: "test-rev-unsub", - Operation: "delete", - Collection: "social.coves.community.subscription", - RKey: rkey, - CID: "", - Record: nil, - }, - } - - err := consumer.HandleEvent(ctx, deleteEvent) - if err != nil { - t.Fatalf("Failed to handle unsubscribe: %v", err) - } - - // Check subscriber count decremented back to 0 - comm, err := repo.GetByDID(ctx, community.DID) - if err != nil { - t.Fatalf("Failed to get community: %v", err) - } - - if comm.SubscriberCount != 0 { - t.Errorf("Subscriber count should be 0, got %d", comm.SubscriberCount) - } - - t.Logf("✓ Subscriber count decremented to 0") - }) -} - -// Helper functions - -func createTestCommunity(t *testing.T, repo communities.Repository, name, did string) *communities.Community { - t.Helper() - - // Add timestamp to make handles unique across test runs - uniqueHandle := fmt.Sprintf("%s-%d.test.coves.social", name, time.Now().UnixNano()) - - community := &communities.Community{ - DID: did, - Handle: uniqueHandle, - Name: name, - DisplayName: "Test Community " + name, - Description: "Test community for subscription indexing", - OwnerDID: did, - CreatedByDID: "did:plc:test-creator", - HostedByDID: "did:plc:test-instance", - Visibility: "public", - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - } - - created, err := repo.Create(context.Background(), community) - if err != nil { - t.Fatalf("Failed to create test community: %v", err) - } - - return created -} - -func createTestCommunityRepo(t *testing.T, db interface{}) communities.Repository { - t.Helper() - // Import the postgres package to create a repo - return postgresRepo.NewCommunityRepository(db.(*sql.DB)) -} diff --git a/tests/integration/user_profile_avatar_e2e_test.go b/tests/integration/user_profile_avatar_e2e_test.go deleted file mode 100644 index 4dad196..0000000 --- a/tests/integration/user_profile_avatar_e2e_test.go +++ /dev/null @@ -1,1022 +0,0 @@ -//go:build integration - -package integration - -// SERIAL BY DESIGN — do not add t.Parallel() to this file. -// -// Its tests drive the Jetstream firehose through the hand-rolled -// subscribeToJetstream* helpers below rather than testkit's cursor-gated -// subscriber. Those helpers subscribe to one shared stream and match on the -// first event of a collection, so a concurrent test writing the same -// collection is delivered to them too and either steals the match or trips -// their timeout. Per-test database clones do not isolate a shared websocket. -// -// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). - -import ( - "Coves/internal/api/handlers/user" - "Coves/internal/api/routes" - "Coves/internal/atproto/identity" - "Coves/internal/atproto/jetstream" - "Coves/internal/core/users" - "Coves/internal/db/postgres" - "Coves/tests/testkit" - "bytes" - "context" - "encoding/json" - "fmt" - "image" - "image/color" - "image/png" - "net/http" - "net/http/httptest" - "net/url" - "os" - "strings" - "testing" - "time" - - "github.com/go-chi/chi/v5" - "github.com/gorilla/websocket" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// createTestAvatarPNG creates a simple PNG image for avatar testing -// Parameters: -// - width, height: image dimensions in pixels -// - c: fill color for the image -// Returns the PNG encoded as bytes -func createTestAvatarPNG(width, height int, c color.Color) []byte { - img := image.NewRGBA(image.Rect(0, 0, width, height)) - for y := 0; y < height; y++ { - for x := 0; x < width; x++ { - img.Set(x, y, c) - } - } - var buf bytes.Buffer - if err := png.Encode(&buf, img); err != nil { - panic(fmt.Sprintf("createTestAvatarPNG: failed to encode PNG: %v", err)) - } - return buf.Bytes() -} - -// TestUserProfileAvatarE2E_UpdateWithAvatar tests the full flow of updating a user profile with an avatar: -// 1. User updates profile via Coves API (POST /xrpc/social.coves.actor.updateProfile) -// 2. Profile record is written to PDS (social.coves.actor.profile) -// 3. Jetstream consumer receives and processes the event -// 4. GetProfile returns the correct avatar URL -func TestUserProfileAvatarE2E_UpdateWithAvatar(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - t.Logf("Jetstream available at %s", jetstreamURL) - - ctx := context.Background() - - // Setup identity resolver - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - userRepo := postgres.NewUserRepository(db) - userService := users.NewUserService(userRepo, identityResolver, pdsURL, nil, "") - - // Setup user consumer for processing Jetstream events - userConsumer := jetstream.NewUserEventConsumer(userService, identityResolver) - - // Setup HTTP server with all user routes using password-based PDS client for E2E tests - e2eAuth := NewE2EOAuthMiddleware() - r := chi.NewRouter() - routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ - PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), - }) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - // Cleanup old test data - testID := uniqueTestID() - - t.Run("update profile with avatar via real PDS and Jetstream", func(t *testing.T) { - // Create test user account on PDS - userHandle := fmt.Sprintf("avatartest%s.local.coves.dev", testID) - email := fmt.Sprintf("avatartest%s@test.com", testID) - password := "test-password-avatar-123" - - t.Logf("\n Creating test user account on PDS: %s", userHandle) - - userToken, userDID, err := createPDSAccount(pdsURL, userHandle, email, password) - require.NoError(t, err, "Failed to create test user account") - require.NotEmpty(t, userToken, "User should receive access token") - require.NotEmpty(t, userDID, "User should receive DID") - - t.Logf("User created: %s (%s)", userHandle, userDID) - - // Index user in AppView database - _ = createTestUser(t, db, userHandle, userDID) - - // Register user with OAuth middleware using real PDS token - userAPIToken := e2eAuth.AddUserWithPDSToken(userDID, userToken, pdsURL) - - // Verify user has no avatar initially - initialProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - assert.Empty(t, initialProfile.Avatar, "Initial avatar should be empty") - t.Logf("Initial profile verified - no avatar") - - // Create test avatar image (100x100 red square) - avatarData := createTestAvatarPNG(100, 100, color.RGBA{255, 0, 0, 255}) - t.Logf("\n Updating profile with avatar (%d bytes)...", len(avatarData)) - - // Subscribe to Jetstream BEFORE making the update - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, 30*time.Second) - defer cancelSubscribe() - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - t.Logf("Failed to connect to Jetstream: %v", dialErr) - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if deadlineErr := conn.SetReadDeadline(readDeadline); deadlineErr != nil { - return - } - - var event jetstream.JetstreamEvent - if readErr := conn.ReadJSON(&event); readErr != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - // Only process profile update events for our user - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.actor.profile" && - event.Did == userDID { - eventChan <- &event - } - } - } - }() - time.Sleep(500 * time.Millisecond) // Give subscriber time to connect - - // Build update profile request - displayName := "Avatar Test User" - bio := "Testing avatar upload E2E" - updateReq := user.UpdateProfileRequest{ - DisplayName: &displayName, - Bio: &bio, - AvatarBlob: avatarData, - AvatarMimeType: "image/png", - } - - reqBody, _ := json.Marshal(updateReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.actor.updateProfile", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+userAPIToken) - - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - defer func() { _ = resp.Body.Close() }() - - require.Equal(t, http.StatusOK, resp.StatusCode, "Update profile should succeed") - - var updateResp user.UpdateProfileResponse - require.NoError(t, json.NewDecoder(resp.Body).Decode(&updateResp)) - - t.Logf("Profile update written to PDS:") - t.Logf(" URI: %s", updateResp.URI) - t.Logf(" CID: %s", updateResp.CID) - - // Wait for REAL Jetstream event - t.Logf("\n Waiting for profile update event from Jetstream...") - var realEvent *jetstream.JetstreamEvent - timeout := time.After(jetstreamReadBudget) - - eventLoop: - for { - select { - case event := <-eventChan: - realEvent = event - t.Logf("Received REAL profile update event from Jetstream!") - t.Logf(" DID: %s", event.Did) - t.Logf(" Operation: %s", event.Commit.Operation) - t.Logf(" CID: %s", event.Commit.CID) - - // Log avatar info from real event - if event.Commit.Record != nil { - if avatar, hasAvatar := event.Commit.Record["avatar"]; hasAvatar { - t.Logf(" Avatar in event: %v", avatar) - } - } - break eventLoop - case <-timeout: - close(done) - t.Fatalf("Timeout waiting for Jetstream profile update event for DID %s", userDID) - } - } - close(done) - - // Process the REAL event through user consumer - t.Logf("\n Processing real Jetstream event through user consumer...") - if handleErr := userConsumer.HandleIdentityEventPublic(ctx, realEvent); handleErr != nil { - // HandleIdentityEventPublic is for identity events, use commit handling instead - t.Logf(" Note: Identity event handling result: %v", handleErr) - } - - // For profile updates, we need to manually process the commit event - // The consumer checks for social.coves.actor.profile commit events - if realEvent.Kind == "commit" && realEvent.Commit != nil { - // Extract profile data from the event and update the user - var displayNamePtr, bioPtr, avatarCIDPtr, bannerCIDPtr *string - - if dn, ok := realEvent.Commit.Record["displayName"].(string); ok { - displayNamePtr = &dn - } - if desc, ok := realEvent.Commit.Record["description"].(string); ok { - bioPtr = &desc - } - if avatarMap, ok := realEvent.Commit.Record["avatar"].(map[string]interface{}); ok { - if ref, ok := avatarMap["ref"].(map[string]interface{}); ok { - if link, ok := ref["$link"].(string); ok { - avatarCIDPtr = &link - t.Logf(" AvatarCID from Jetstream: %s", link) - } - } - } - - _, updateErr := userService.UpdateProfile(ctx, userDID, users.UpdateProfileInput{ - DisplayName: displayNamePtr, - Bio: bioPtr, - AvatarCID: avatarCIDPtr, - BannerCID: bannerCIDPtr, - }) - if updateErr != nil { - t.Logf(" Update profile from event error: %v", updateErr) - } - } - - // Verify profile now has avatar URL via GetProfile - t.Logf("\n Verifying profile via GetProfile...") - finalProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - - t.Logf("Final profile verification:") - t.Logf(" DisplayName: %s", finalProfile.DisplayName) - t.Logf(" Bio: %s", finalProfile.Bio) - t.Logf(" Avatar URL: %s", finalProfile.Avatar) - - assert.Equal(t, displayName, finalProfile.DisplayName, "DisplayName should match") - assert.Equal(t, bio, finalProfile.Bio, "Bio should match") - assert.NotEmpty(t, finalProfile.Avatar, "Avatar URL should be set") - - // Verify avatar URL format (should be PDS blob URL) - if finalProfile.Avatar != "" { - assert.Contains(t, finalProfile.Avatar, "/xrpc/com.atproto.sync.getBlob", - "Avatar URL should be a PDS blob URL") - // URL-decode the avatar URL before checking for DID (DIDs are URL-encoded in query params) - decodedAvatarURL, _ := url.QueryUnescape(finalProfile.Avatar) - assert.Contains(t, decodedAvatarURL, userDID, - "Avatar URL should contain user DID") - } - - // Optionally: Fetch avatar URL and verify blob is accessible - if finalProfile.Avatar != "" { - avatarResp, avatarErr := http.Get(finalProfile.Avatar) - if avatarErr != nil { - t.Logf(" Warning: Could not fetch avatar URL: %v", avatarErr) - } else { - defer func() { _ = avatarResp.Body.Close() }() - t.Logf(" Avatar fetch status: %d", avatarResp.StatusCode) - if avatarResp.StatusCode == http.StatusOK { - t.Logf(" Avatar blob is accessible!") - } - } - } - - t.Logf("\n TRUE E2E USER PROFILE AVATAR UPDATE COMPLETE") - t.Logf(" API -> PDS uploadBlob -> PDS putRecord -> Jetstream -> AppView") - }) -} - -// TestUserProfileAvatarE2E_UpdateWithBanner tests the full flow of updating a user profile with a banner -func TestUserProfileAvatarE2E_UpdateWithBanner(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - - ctx := context.Background() - - // Setup identity resolver - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - userRepo := postgres.NewUserRepository(db) - userService := users.NewUserService(userRepo, identityResolver, pdsURL, nil, "") - - // Setup HTTP server using password-based PDS client for E2E tests - e2eAuth := NewE2EOAuthMiddleware() - r := chi.NewRouter() - routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ - PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), - }) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - testID := uniqueTestID() - - t.Run("update profile with banner via real PDS and Jetstream", func(t *testing.T) { - // Create test user account on PDS - userHandle := fmt.Sprintf("bannertest%s.local.coves.dev", testID) - email := fmt.Sprintf("bannertest%s@test.com", testID) - password := "test-password-banner-123" - - t.Logf("\n Creating test user account on PDS: %s", userHandle) - - userToken, userDID, err := createPDSAccount(pdsURL, userHandle, email, password) - require.NoError(t, err, "Failed to create test user account") - - t.Logf("User created: %s (%s)", userHandle, userDID) - - // Index user in AppView database - _ = createTestUser(t, db, userHandle, userDID) - - // Register user with OAuth middleware - userAPIToken := e2eAuth.AddUserWithPDSToken(userDID, userToken, pdsURL) - - // Verify no banner initially - initialProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - assert.Empty(t, initialProfile.Banner, "Initial banner should be empty") - - // Create test banner image (300x100 blue rectangle) - bannerData := createTestAvatarPNG(300, 100, color.RGBA{0, 0, 255, 255}) - t.Logf("\n Updating profile with banner (%d bytes)...", len(bannerData)) - - // Subscribe to Jetstream - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, 30*time.Second) - defer cancelSubscribe() - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if err := conn.SetReadDeadline(readDeadline); err != nil { - return - } - - var event jetstream.JetstreamEvent - if err := conn.ReadJSON(&event); err != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.actor.profile" && - event.Did == userDID { - eventChan <- &event - } - } - } - }() - time.Sleep(500 * time.Millisecond) - - // Build update profile request with banner - displayName := "Banner Test User" - updateReq := user.UpdateProfileRequest{ - DisplayName: &displayName, - BannerBlob: bannerData, - BannerMimeType: "image/png", - } - - reqBody, _ := json.Marshal(updateReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.actor.updateProfile", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+userAPIToken) - - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - defer func() { _ = resp.Body.Close() }() - - require.Equal(t, http.StatusOK, resp.StatusCode, "Update profile should succeed") - - var updateResp user.UpdateProfileResponse - require.NoError(t, json.NewDecoder(resp.Body).Decode(&updateResp)) - - t.Logf("Profile update written to PDS: URI=%s, CID=%s", updateResp.URI, updateResp.CID) - - // Wait for Jetstream event - t.Logf("\n Waiting for profile update event from Jetstream...") - var realEvent *jetstream.JetstreamEvent - timeout := time.After(jetstreamReadBudget) - - eventLoop: - for { - select { - case event := <-eventChan: - realEvent = event - t.Logf("Received REAL profile update event!") - - if event.Commit.Record != nil { - if banner, hasBanner := event.Commit.Record["banner"]; hasBanner { - t.Logf(" Banner in event: %v", banner) - } - } - break eventLoop - case <-timeout: - close(done) - t.Fatalf("Timeout waiting for Jetstream event") - } - } - close(done) - - // Process the event and update user profile - if realEvent.Kind == "commit" && realEvent.Commit != nil { - var displayNamePtr, bioPtr, avatarCIDPtr, bannerCIDPtr *string - - if dn, ok := realEvent.Commit.Record["displayName"].(string); ok { - displayNamePtr = &dn - } - if bannerMap, ok := realEvent.Commit.Record["banner"].(map[string]interface{}); ok { - if ref, ok := bannerMap["ref"].(map[string]interface{}); ok { - if link, ok := ref["$link"].(string); ok { - bannerCIDPtr = &link - t.Logf(" BannerCID from Jetstream: %s", link) - } - } - } - - _, _ = userService.UpdateProfile(ctx, userDID, users.UpdateProfileInput{ - DisplayName: displayNamePtr, - Bio: bioPtr, - AvatarCID: avatarCIDPtr, - BannerCID: bannerCIDPtr, - }) - } - - // Verify profile now has banner URL - finalProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - - t.Logf("Final profile verification:") - t.Logf(" DisplayName: %s", finalProfile.DisplayName) - t.Logf(" Banner URL: %s", finalProfile.Banner) - - assert.Equal(t, displayName, finalProfile.DisplayName) - assert.NotEmpty(t, finalProfile.Banner, "Banner URL should be set") - - if finalProfile.Banner != "" { - assert.Contains(t, finalProfile.Banner, "/xrpc/com.atproto.sync.getBlob") - // URL-decode the banner URL before checking for DID (DIDs are URL-encoded in query params) - decodedBannerURL, _ := url.QueryUnescape(finalProfile.Banner) - assert.Contains(t, decodedBannerURL, userDID) - } - - t.Logf("\n TRUE E2E USER PROFILE BANNER UPDATE COMPLETE") - }) -} - -// TestUserProfileAvatarE2E_UpdateDisplayNameAndBio tests updating non-blob profile fields -func TestUserProfileAvatarE2E_UpdateDisplayNameAndBio(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - - ctx := context.Background() - - // Setup identity resolver - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - userRepo := postgres.NewUserRepository(db) - userService := users.NewUserService(userRepo, identityResolver, pdsURL, nil, "") - - // Setup HTTP server using password-based PDS client for E2E tests - e2eAuth := NewE2EOAuthMiddleware() - r := chi.NewRouter() - routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ - PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), - }) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - testID := uniqueTestID() - - t.Run("update display name and bio without blobs", func(t *testing.T) { - // Create test user account on PDS - userHandle := fmt.Sprintf("texttest%s.local.coves.dev", testID) - email := fmt.Sprintf("texttest%s@test.com", testID) - password := "test-password-text-123" - - userToken, userDID, err := createPDSAccount(pdsURL, userHandle, email, password) - require.NoError(t, err) - - t.Logf("User created: %s (%s)", userHandle, userDID) - - // Index user in AppView - _ = createTestUser(t, db, userHandle, userDID) - userAPIToken := e2eAuth.AddUserWithPDSToken(userDID, userToken, pdsURL) - - // Subscribe to Jetstream - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, 30*time.Second) - defer cancelSubscribe() - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if err := conn.SetReadDeadline(readDeadline); err != nil { - return - } - - var event jetstream.JetstreamEvent - if err := conn.ReadJSON(&event); err != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.actor.profile" && - event.Did == userDID { - eventChan <- &event - } - } - } - }() - time.Sleep(500 * time.Millisecond) - - // Update with only text fields - displayName := "Text Update Test User" - bio := "This is my test bio for E2E testing" - updateReq := user.UpdateProfileRequest{ - DisplayName: &displayName, - Bio: &bio, - } - - reqBody, _ := json.Marshal(updateReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.actor.updateProfile", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+userAPIToken) - - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - defer func() { _ = resp.Body.Close() }() - - require.Equal(t, http.StatusOK, resp.StatusCode) - - // Wait for Jetstream event - var realEvent *jetstream.JetstreamEvent - timeout := time.After(jetstreamReadBudget) - - eventLoop: - for { - select { - case event := <-eventChan: - realEvent = event - t.Logf("Received profile update event!") - break eventLoop - case <-timeout: - close(done) - t.Fatalf("Timeout waiting for Jetstream event") - } - } - close(done) - - // Process the event - if realEvent.Kind == "commit" && realEvent.Commit != nil { - var displayNamePtr, bioPtr *string - - if dn, ok := realEvent.Commit.Record["displayName"].(string); ok { - displayNamePtr = &dn - } - if desc, ok := realEvent.Commit.Record["description"].(string); ok { - bioPtr = &desc - } - - _, _ = userService.UpdateProfile(ctx, userDID, users.UpdateProfileInput{ - DisplayName: displayNamePtr, - Bio: bioPtr, - }) - } - - // Verify profile - finalProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - - assert.Equal(t, displayName, finalProfile.DisplayName) - assert.Equal(t, bio, finalProfile.Bio) - - t.Logf("Text-only profile update verified:") - t.Logf(" DisplayName: %s", finalProfile.DisplayName) - t.Logf(" Bio: %s", finalProfile.Bio) - - t.Logf("\n TRUE E2E TEXT-ONLY PROFILE UPDATE COMPLETE") - }) -} - -// TestUserProfileAvatarE2E_ReplaceAvatar tests replacing an existing avatar with a new one -func TestUserProfileAvatarE2E_ReplaceAvatar(t *testing.T) { - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - if err != nil { - t.Skipf("PDS not running at %s: %v. Run 'make dev-up' to start.", pdsURL, err) - } - _ = healthResp.Body.Close() - - // Check if Jetstream is running - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.actor.profile", pdsHostname) - - testConn, _, connErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if connErr != nil { - t.Skipf("Jetstream not available at %s: %v. Run 'make dev-up' to start.", jetstreamURL, connErr) - } - _ = testConn.Close() - - ctx := context.Background() - - // Setup identity resolver - plcURL := os.Getenv("PLC_DIRECTORY_URL") - if plcURL == "" { - plcURL = "http://localhost:3002" - } - identityConfig := identity.DefaultConfig() - identityConfig.PLCURL = plcURL - identityResolver := identity.NewResolver(db, identityConfig) - - // Setup services - userRepo := postgres.NewUserRepository(db) - userService := users.NewUserService(userRepo, identityResolver, pdsURL, nil, "") - - // Setup HTTP server using password-based PDS client for E2E tests - e2eAuth := NewE2EOAuthMiddleware() - r := chi.NewRouter() - routes.RegisterUserRoutesWithOptions(r, userService, e2eAuth.OAuthAuthMiddleware, nil, &routes.UserRouteOptions{ - PDSClientFactory: UserProfilePasswordAuthPDSClientFactory(), - }) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - testID := uniqueTestID() - - // subscribeForProfileEvent opens the Jetstream subscription and returns a wait - // function that blocks until a profile commit for userDID arrives (or times out), - // returning the avatar CID extracted from the commit record. - // - // It MUST be called BEFORE the PDS write. The firehose subscription is cursorless - // (see jetstreamURL), so it only streams commits emitted after the socket is - // established — there is no replay. Dialing after the write (the previous helper's - // behavior) races the PDS→firehose relay and silently drops the event under load. - subscribeForProfileEvent := func(t *testing.T, userDID string, timeout time.Duration) func() (string, *jetstream.JetstreamEvent) { - eventChan := make(chan *jetstream.JetstreamEvent, 10) - done := make(chan bool) - ready := make(chan struct{}) - subscribeCtx, cancelSubscribe := context.WithTimeout(ctx, timeout) - - go func() { - conn, _, dialErr := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if dialErr != nil { - t.Logf("Failed to connect to Jetstream: %v", dialErr) - close(ready) - return - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - close(ready) // socket dialed; safe for the caller to write - - for { - select { - case <-done: - return - case <-subscribeCtx.Done(): - return - default: - if err := conn.SetReadDeadline(readDeadline); err != nil { - return - } - - var event jetstream.JetstreamEvent - if err := conn.ReadJSON(&event); err != nil { - // Any read error ends this subscription. A gorilla connection is - // corrupt once its read deadline has expired, and looping on it - // is what reaches the panic that aborts the whole test binary. - // The caller's own timeout reports the missing event. - return - } - - if event.Kind == "commit" && event.Commit != nil && - event.Commit.Collection == "social.coves.actor.profile" && - event.Did == userDID { - eventChan <- &event - } - } - } - }() - - // Block until the socket is dialed, then give Jetstream a moment to register - // the subscription, so the caller's subsequent write is guaranteed to land - // after we are listening. - <-ready - time.Sleep(500 * time.Millisecond) - - return func() (string, *jetstream.JetstreamEvent) { - defer cancelSubscribe() - select { - case event := <-eventChan: - close(done) - var avatarCID string - if event.Commit.Record != nil { - if avatarMap, ok := event.Commit.Record["avatar"].(map[string]interface{}); ok { - if ref, ok := avatarMap["ref"].(map[string]interface{}); ok { - if link, ok := ref["$link"].(string); ok { - avatarCID = link - } - } - } - } - return avatarCID, event - case <-time.After(timeout): - close(done) - return "", nil - } - } - } - - t.Run("replace existing avatar with new one", func(t *testing.T) { - // Create test user account on PDS - userHandle := fmt.Sprintf("replaceav%s.local.coves.dev", testID) - email := fmt.Sprintf("replaceav%s@test.com", testID) - password := "test-password-replace-123" - - userToken, userDID, err := createPDSAccount(pdsURL, userHandle, email, password) - require.NoError(t, err) - - t.Logf("User created: %s (%s)", userHandle, userDID) - - // Index user in AppView - _ = createTestUser(t, db, userHandle, userDID) - userAPIToken := e2eAuth.AddUserWithPDSToken(userDID, userToken, pdsURL) - - // STEP 1: Create initial avatar (red square) - t.Logf("\n Step 1: Setting initial avatar (red)...") - - initialAvatarData := createTestAvatarPNG(100, 100, color.RGBA{255, 0, 0, 255}) - displayName := "Replace Avatar Test" - updateReq := user.UpdateProfileRequest{ - DisplayName: &displayName, - AvatarBlob: initialAvatarData, - AvatarMimeType: "image/png", - } - - // Subscribe to the firehose BEFORE the write (cursorless: no replay). - waitInitial := subscribeForProfileEvent(t, userDID, 30*time.Second) - - reqBody, _ := json.Marshal(updateReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.actor.updateProfile", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+userAPIToken) - - resp, err := http.DefaultClient.Do(req) - require.NoError(t, err) - _ = resp.Body.Close() - require.Equal(t, http.StatusOK, resp.StatusCode) - - // Wait for initial avatar event - initialAvatarCID, initialEvent := waitInitial() - require.NotNil(t, initialEvent, "Should receive initial avatar event") - require.NotEmpty(t, initialAvatarCID, "Initial avatar CID should not be empty") - - t.Logf(" Initial AvatarCID: %s", initialAvatarCID) - - // Update local user profile - _, _ = userService.UpdateProfile(ctx, userDID, users.UpdateProfileInput{ - DisplayName: &displayName, - AvatarCID: &initialAvatarCID, - }) - - // Verify initial avatar - profileAfterInitial, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - assert.NotEmpty(t, profileAfterInitial.Avatar) - - // Small delay between updates - time.Sleep(1 * time.Second) - - // STEP 2: Replace with new avatar (green square) - t.Logf("\n Step 2: Replacing avatar with new one (green)...") - - newAvatarData := createTestAvatarPNG(100, 100, color.RGBA{0, 255, 0, 255}) - updateReq2 := user.UpdateProfileRequest{ - AvatarBlob: newAvatarData, - AvatarMimeType: "image/png", - } - - // Subscribe BEFORE the replacement write (cursorless: no replay). - waitReplacement := subscribeForProfileEvent(t, userDID, 30*time.Second) - - reqBody2, _ := json.Marshal(updateReq2) - req2, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.actor.updateProfile", - bytes.NewBuffer(reqBody2)) - req2.Header.Set("Content-Type", "application/json") - req2.Header.Set("Authorization", "Bearer "+userAPIToken) - - resp2, err := http.DefaultClient.Do(req2) - require.NoError(t, err) - _ = resp2.Body.Close() - require.Equal(t, http.StatusOK, resp2.StatusCode) - - // Wait for replacement avatar event - newAvatarCID, newEvent := waitReplacement() - require.NotNil(t, newEvent, "Should receive replacement avatar event") - require.NotEmpty(t, newAvatarCID, "New avatar CID should not be empty") - - t.Logf(" New AvatarCID: %s", newAvatarCID) - - // Verify CIDs are different - assert.NotEqual(t, initialAvatarCID, newAvatarCID, - "New avatar CID should be different from initial") - - // Update local user profile with new avatar - _, _ = userService.UpdateProfile(ctx, userDID, users.UpdateProfileInput{ - AvatarCID: &newAvatarCID, - }) - - // Verify final profile - finalProfile, err := userService.GetProfile(ctx, userDID) - require.NoError(t, err) - - assert.NotEmpty(t, finalProfile.Avatar, "Final avatar URL should be set") - assert.Contains(t, finalProfile.Avatar, newAvatarCID, - "Avatar URL should contain new CID") - - t.Logf("\n Avatar replacement verified:") - t.Logf(" Old CID: %s", initialAvatarCID) - t.Logf(" New CID: %s", newAvatarCID) - t.Logf(" CIDs different: %v", initialAvatarCID != newAvatarCID) - - t.Logf("\n TRUE E2E AVATAR REPLACEMENT COMPLETE") - }) -} diff --git a/tests/integration/vote_e2e_test.go b/tests/integration/vote_e2e_test.go deleted file mode 100644 index 3b24f67..0000000 --- a/tests/integration/vote_e2e_test.go +++ /dev/null @@ -1,1034 +0,0 @@ -//go:build integration - -package integration - -// SERIAL BY DESIGN — do not add t.Parallel() to this file. -// -// Its tests drive the Jetstream firehose through the hand-rolled -// subscribeToJetstream* helpers below rather than testkit's cursor-gated -// subscriber. Those helpers subscribe to one shared stream and match on the -// first event of a collection, so a concurrent test writing the same -// collection is delivered to them too and either steals the match or trips -// their timeout. Per-test database clones do not isolate a shared websocket. -// -// docs/TEST_ARCHITECTURE.md §3.3 ("Parallelism is earned, not assumed"). - -import ( - "Coves/internal/api/routes" - "Coves/internal/atproto/jetstream" - "Coves/internal/atproto/utils" - "Coves/internal/core/votes" - "Coves/internal/db/postgres" - "Coves/tests/testkit" - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "net" - "net/http" - "net/http/httptest" - "os" - "strings" - "testing" - "time" - - "github.com/go-chi/chi/v5" - "github.com/gorilla/websocket" - - "github.com/stretchr/testify/require" -) - -// TestVoteE2E_CreateUpvote tests the full vote creation flow with a real local PDS -// Flow: Client → XRPC → PDS Write → Jetstream → Consumer → AppView -func TestVoteE2E_CreateUpvote(t *testing.T) { - - db := testkit.DB(t) - - // Check if PDS is running - pdsURL := os.Getenv("PDS_URL") - if pdsURL == "" { - pdsURL = "http://localhost:3001" - } - - healthResp, err := http.Get(pdsURL + "/xrpc/_health") - require.NoError(t, err, "PDS health check at %s (TestMain's RequirePDS should have caught this)", pdsURL) - func() { - if closeErr := healthResp.Body.Close(); closeErr != nil { - t.Logf("Failed to close health response: %v", closeErr) - } - }() - - ctx := context.Background() - - // Setup repositories - voteRepo := postgres.NewVoteRepository(db) - postRepo := postgres.NewPostRepository(db) - - // Setup services with password-based PDS client factory for E2E testing - voteService := votes.NewServiceWithPDSFactory(voteRepo, nil, nil, PasswordAuthPDSClientFactory()) - - // Create test user on PDS - testID := uniqueTestID() - testUserHandle := fmt.Sprintf("vot%s.local.coves.dev", testID) - testUserEmail := fmt.Sprintf("voter-%s@test.local", testID) - testUserPassword := "test-password-123" - - t.Logf("Creating test user on PDS: %s", testUserHandle) - pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - if err != nil { - t.Fatalf("Failed to create test user on PDS: %v", err) - } - t.Logf("Test user created: DID=%s", userDID) - - // Index user in AppView - testUser := createTestUser(t, db, testUserHandle, userDID) - - // Create test post to vote on - testCommunityDID, err := createFeedTestCommunity(db, ctx, "test-community", "owner.test") - if err != nil { - t.Fatalf("Failed to create test community: %v", err) - } - - postURI := createTestPost(t, db, testCommunityDID, testUser.DID, "Test Post", 0, time.Now()) - postCID := "bafypost123" - - // Setup OAuth middleware with real PDS access token - e2eAuth := NewE2EOAuthMiddleware() - token := e2eAuth.AddUserWithPDSToken(userDID, pdsAccessToken, pdsURL) - - // Setup HTTP server with XRPC routes - r := chi.NewRouter() - routes.RegisterVoteRoutes(r, voteService, e2eAuth.OAuthAuthMiddleware) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - // Setup Jetstream consumer - voteConsumer := jetstream.NewVoteEventConsumer(voteRepo, nil, db) - - // ==================================================================================== - // TEST: Create upvote on post - // ==================================================================================== - t.Logf("\n📝 Creating upvote via XRPC endpoint...") - - voteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - } - - reqBody, marshalErr := json.Marshal(voteReq) - if marshalErr != nil { - t.Fatalf("Failed to marshal request: %v", marshalErr) - } - - req, err := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody)) - if err != nil { - t.Fatalf("Failed to create request: %v", err) - } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+token) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("Failed to POST vote: %v", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - body, readErr := io.ReadAll(resp.Body) - if readErr != nil { - t.Fatalf("Expected 200, got %d (failed to read body: %v)", resp.StatusCode, readErr) - } - t.Logf("XRPC Vote Failed") - t.Logf(" Status: %d", resp.StatusCode) - t.Logf(" Response: %s", string(body)) - t.Fatalf("Expected 200, got %d: %s", resp.StatusCode, string(body)) - } - - var voteResp struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - - if decodeErr := json.NewDecoder(resp.Body).Decode(&voteResp); decodeErr != nil { - t.Fatalf("Failed to decode vote response: %v", decodeErr) - } - - t.Logf("✅ XRPC response received:") - t.Logf(" URI: %s", voteResp.URI) - t.Logf(" CID: %s", voteResp.CID) - - // Verify vote record was written to PDS - t.Logf("\n🔍 Verifying vote record on PDS...") - rkey := utils.ExtractRKeyFromURI(voteResp.URI) - collection := "social.coves.feed.vote" - - pdsResp, pdsErr := http.Get(fmt.Sprintf("%s/xrpc/com.atproto.repo.getRecord?repo=%s&collection=%s&rkey=%s", - pdsURL, userDID, collection, rkey)) - if pdsErr != nil { - t.Fatalf("Failed to fetch vote record from PDS: %v", pdsErr) - } - defer func() { - if closeErr := pdsResp.Body.Close(); closeErr != nil { - t.Logf("Failed to close PDS response: %v", closeErr) - } - }() - - if pdsResp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(pdsResp.Body) - t.Fatalf("Vote record not found on PDS: status %d, body: %s", pdsResp.StatusCode, string(body)) - } - - var pdsRecord struct { - Value map[string]interface{} `json:"value"` - CID string `json:"cid"` - } - if decodeErr := json.NewDecoder(pdsResp.Body).Decode(&pdsRecord); decodeErr != nil { - t.Fatalf("Failed to decode PDS record: %v", decodeErr) - } - - t.Logf("✅ Vote record found on PDS:") - t.Logf(" CID: %s", pdsRecord.CID) - t.Logf(" Direction: %v", pdsRecord.Value["direction"]) - - // Verify direction - if pdsRecord.Value["direction"] != "up" { - t.Errorf("Expected direction 'up', got %v", pdsRecord.Value["direction"]) - } - - // Simulate Jetstream consumer indexing the vote - t.Logf("\n🔄 Simulating Jetstream consumer indexing vote...") - voteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev", - Operation: "create", - Collection: "social.coves.feed.vote", - RKey: rkey, - CID: pdsRecord.CID, - Record: map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - "createdAt": time.Now().Format(time.RFC3339), - }, - }, - } - - if handleErr := voteConsumer.HandleEvent(ctx, &voteEvent); handleErr != nil { - t.Fatalf("Failed to handle vote event: %v", handleErr) - } - - // Verify vote was indexed in AppView - t.Logf("\n🔍 Verifying vote indexed in AppView...") - indexedVote, err := voteRepo.GetByURI(ctx, voteResp.URI) - if err != nil { - t.Fatalf("Vote not indexed in AppView: %v", err) - } - - t.Logf("✅ Vote indexed in AppView:") - t.Logf(" VoterDID: %s", indexedVote.VoterDID) - t.Logf(" SubjectURI: %s", indexedVote.SubjectURI) - t.Logf(" Direction: %s", indexedVote.Direction) - t.Logf(" URI: %s", indexedVote.URI) - - // Verify vote details - if indexedVote.VoterDID != userDID { - t.Errorf("Expected voter_did %s, got %s", userDID, indexedVote.VoterDID) - } - if indexedVote.SubjectURI != postURI { - t.Errorf("Expected subject_uri %s, got %s", postURI, indexedVote.SubjectURI) - } - if indexedVote.Direction != "up" { - t.Errorf("Expected direction 'up', got %s", indexedVote.Direction) - } - - // Verify post counts updated - t.Logf("\n🔍 Verifying post vote counts updated...") - updatedPost, err := postRepo.GetByURI(ctx, postURI) - if err != nil { - t.Fatalf("Failed to get updated post: %v", err) - } - - if updatedPost.UpvoteCount != 1 { - t.Errorf("Expected upvote_count = 1, got %d", updatedPost.UpvoteCount) - } - if updatedPost.Score != 1 { - t.Errorf("Expected score = 1, got %d", updatedPost.Score) - } - - t.Logf("✅ TRUE E2E UPVOTE FLOW COMPLETE:") - t.Logf(" Client → XRPC → PDS Write → Jetstream → Consumer → AppView ✓") - t.Logf(" ✓ Vote written to PDS") - t.Logf(" ✓ Vote indexed in AppView") - t.Logf(" ✓ Post vote counts updated") -} - -// TestVoteE2E_ToggleSameDirection tests voting twice in same direction (toggle off) -func TestVoteE2E_ToggleSameDirection(t *testing.T) { - db := testkit.DB(t) - - ctx := context.Background() - pdsURL := getTestPDSURL() - - // Setup repositories and services - voteRepo := postgres.NewVoteRepository(db) - postRepo := postgres.NewPostRepository(db) - - voteService := votes.NewServiceWithPDSFactory(voteRepo, nil, nil, PasswordAuthPDSClientFactory()) - - // Create test user - testID := uniqueTestID() - testUserHandle := fmt.Sprintf("tog%s.local.coves.dev", testID) - testUserEmail := fmt.Sprintf("toggle-%s@test.local", testID) - testUserPassword := "test-password-123" - - pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - require.NoError(t, err, "creating the test account on the PDS") - - testUser := createTestUser(t, db, testUserHandle, userDID) - - // Create test post - testCommunityDID, _ := createFeedTestCommunity(db, ctx, "toggle-community", "owner.test") - postURI := createTestPost(t, db, testCommunityDID, testUser.DID, "Test Post", 0, time.Now()) - postCID := "bafypost456" - - // Setup OAuth and HTTP server with real PDS access token - e2eAuth := NewE2EOAuthMiddleware() - token := e2eAuth.AddUserWithPDSToken(userDID, pdsAccessToken, pdsURL) - - r := chi.NewRouter() - routes.RegisterVoteRoutes(r, voteService, e2eAuth.OAuthAuthMiddleware) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - voteConsumer := jetstream.NewVoteEventConsumer(voteRepo, nil, db) - - // First upvote - t.Logf("\n📝 Creating first upvote...") - voteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - } - - reqBody, _ := json.Marshal(voteReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+token) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("Failed to create first vote: %v", err) - } - - var firstVoteResp struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if decodeErr := json.NewDecoder(resp.Body).Decode(&firstVoteResp); decodeErr != nil { - t.Fatalf("Failed to decode first vote response: %v", decodeErr) - } - if closeErr := resp.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - - t.Logf("✅ First vote created: %s", firstVoteResp.URI) - - // Index first vote - rkey := utils.ExtractRKeyFromURI(firstVoteResp.URI) - voteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev-1", - Operation: "create", - Collection: "social.coves.feed.vote", - RKey: rkey, - CID: firstVoteResp.CID, - Record: map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - "createdAt": time.Now().Format(time.RFC3339), - }, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &voteEvent); handleErr != nil { - t.Fatalf("Failed to handle first vote event: %v", handleErr) - } - - // Second upvote (same direction) - should toggle off (delete) - t.Logf("\n📝 Creating second upvote (toggle off)...") - req2, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody)) - req2.Header.Set("Content-Type", "application/json") - req2.Header.Set("Authorization", "Bearer "+token) - - resp2, err := http.DefaultClient.Do(req2) - if err != nil { - t.Fatalf("Failed to toggle vote: %v", err) - } - defer func() { - if closeErr := resp2.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - }() - - if resp2.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp2.Body) - t.Fatalf("Expected 200, got %d: %s", resp2.StatusCode, string(body)) - } - - t.Logf("✅ Second vote request completed (toggle)") - - // Simulate Jetstream DELETE event - t.Logf("\n🔄 Simulating Jetstream DELETE event...") - deleteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev-2", - Operation: "delete", - Collection: "social.coves.feed.vote", - RKey: rkey, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &deleteEvent); handleErr != nil { - t.Fatalf("Failed to handle delete event: %v", handleErr) - } - - // Verify vote was removed from AppView - t.Logf("\n🔍 Verifying vote removed from AppView...") - _, err = voteRepo.GetByURI(ctx, firstVoteResp.URI) - if err == nil { - t.Error("Expected vote to be deleted, but it still exists") - } - - // Verify post counts reset - updatedPost, _ := postRepo.GetByURI(ctx, postURI) - if updatedPost.UpvoteCount != 0 { - t.Errorf("Expected upvote_count = 0 after toggle, got %d", updatedPost.UpvoteCount) - } - - t.Logf("✅ TOGGLE SAME DIRECTION FLOW COMPLETE:") - t.Logf(" ✓ First vote created and indexed") - t.Logf(" ✓ Second vote toggled off (deleted)") - t.Logf(" ✓ Post counts updated correctly") -} - -// TestVoteE2E_ToggleDifferentDirection tests changing vote direction -func TestVoteE2E_ToggleDifferentDirection(t *testing.T) { - db := testkit.DB(t) - - ctx := context.Background() - pdsURL := getTestPDSURL() - - // Setup repositories and services - voteRepo := postgres.NewVoteRepository(db) - postRepo := postgres.NewPostRepository(db) - - voteService := votes.NewServiceWithPDSFactory(voteRepo, nil, nil, PasswordAuthPDSClientFactory()) - - // Create test user - testID := uniqueTestID() - testUserHandle := fmt.Sprintf("flp%s.local.coves.dev", testID) - testUserEmail := fmt.Sprintf("flip-%s@test.local", testID) - testUserPassword := "test-password-123" - - pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - require.NoError(t, err, "creating the test account on the PDS") - - testUser := createTestUser(t, db, testUserHandle, userDID) - - // Create test post - testCommunityDID, _ := createFeedTestCommunity(db, ctx, "flip-community", "owner.test") - postURI := createTestPost(t, db, testCommunityDID, testUser.DID, "Test Post", 0, time.Now()) - postCID := "bafypost789" - - // Setup OAuth and HTTP server with real PDS access token - e2eAuth := NewE2EOAuthMiddleware() - token := e2eAuth.AddUserWithPDSToken(userDID, pdsAccessToken, pdsURL) - - r := chi.NewRouter() - routes.RegisterVoteRoutes(r, voteService, e2eAuth.OAuthAuthMiddleware) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - voteConsumer := jetstream.NewVoteEventConsumer(voteRepo, nil, db) - - // Create upvote - t.Logf("\n📝 Creating upvote...") - upvoteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - } - - reqBody, _ := json.Marshal(upvoteReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+token) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("Failed to create upvote: %v", err) - } - var upvoteResp struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if decodeErr := json.NewDecoder(resp.Body).Decode(&upvoteResp); decodeErr != nil { - t.Fatalf("Failed to decode upvote response: %v", decodeErr) - } - if closeErr := resp.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - - // Index upvote - rkey := utils.ExtractRKeyFromURI(upvoteResp.URI) - upvoteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev-up", - Operation: "create", - Collection: "social.coves.feed.vote", - RKey: rkey, - CID: upvoteResp.CID, - Record: map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - "createdAt": time.Now().Format(time.RFC3339), - }, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &upvoteEvent); handleErr != nil { - t.Fatalf("Failed to handle upvote event: %v", handleErr) - } - - t.Logf("✅ Upvote created and indexed") - - // Change to downvote - t.Logf("\n📝 Changing to downvote...") - downvoteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "down", - } - - reqBody2, _ := json.Marshal(downvoteReq) - req2, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody2)) - req2.Header.Set("Content-Type", "application/json") - req2.Header.Set("Authorization", "Bearer "+token) - - resp2, err := http.DefaultClient.Do(req2) - if err != nil { - t.Fatalf("Failed to create downvote: %v", err) - } - var downvoteResp struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if decodeErr := json.NewDecoder(resp2.Body).Decode(&downvoteResp); decodeErr != nil { - t.Fatalf("Failed to decode downvote response: %v", decodeErr) - } - if closeErr := resp2.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - - // The service flow for direction change is: - // 1. DELETE old vote on PDS - // 2. CREATE new vote with NEW rkey on PDS - // So we simulate DELETE + CREATE events (not UPDATE) - - // Simulate Jetstream DELETE event for old vote - t.Logf("\n🔄 Simulating Jetstream DELETE event for old upvote...") - deleteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev-delete", - Operation: "delete", - Collection: "social.coves.feed.vote", - RKey: rkey, // Old upvote rkey - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &deleteEvent); handleErr != nil { - t.Fatalf("Failed to handle delete event: %v", handleErr) - } - - // Simulate Jetstream CREATE event for new downvote - t.Logf("\n🔄 Simulating Jetstream CREATE event for new downvote...") - newRkey := utils.ExtractRKeyFromURI(downvoteResp.URI) - createEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-rev-down", - Operation: "create", - Collection: "social.coves.feed.vote", - RKey: newRkey, // NEW rkey from downvote response - CID: downvoteResp.CID, - Record: map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "down", - "createdAt": time.Now().Format(time.RFC3339), - }, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &createEvent); handleErr != nil { - t.Fatalf("Failed to handle create event: %v", handleErr) - } - - // Verify old upvote was deleted - t.Logf("\n🔍 Verifying old upvote was deleted...") - _, err = voteRepo.GetByURI(ctx, upvoteResp.URI) - if err == nil { - t.Error("Expected old upvote to be deleted, but it still exists") - } - - // Verify new downvote was indexed - t.Logf("\n🔍 Verifying new downvote indexed in AppView...") - newVote, err := voteRepo.GetByURI(ctx, downvoteResp.URI) - if err != nil { - t.Fatalf("New downvote not found: %v", err) - } - - if newVote.Direction != "down" { - t.Errorf("Expected direction 'down', got %s", newVote.Direction) - } - - // Verify post counts updated - updatedPost, _ := postRepo.GetByURI(ctx, postURI) - if updatedPost.UpvoteCount != 0 { - t.Errorf("Expected upvote_count = 0, got %d", updatedPost.UpvoteCount) - } - if updatedPost.DownvoteCount != 1 { - t.Errorf("Expected downvote_count = 1, got %d", updatedPost.DownvoteCount) - } - if updatedPost.Score != -1 { - t.Errorf("Expected score = -1, got %d", updatedPost.Score) - } - - t.Logf("✅ TOGGLE DIFFERENT DIRECTION FLOW COMPLETE:") - t.Logf(" ✓ Upvote created (score: +1)") - t.Logf(" ✓ Changed to downvote (score: -1)") - t.Logf(" ✓ Post counts updated correctly") -} - -// TestVoteE2E_DeleteVote tests explicit vote deletion -func TestVoteE2E_DeleteVote(t *testing.T) { - db := testkit.DB(t) - - ctx := context.Background() - pdsURL := getTestPDSURL() - - // Setup repositories and services - voteRepo := postgres.NewVoteRepository(db) - postRepo := postgres.NewPostRepository(db) - - voteService := votes.NewServiceWithPDSFactory(voteRepo, nil, nil, PasswordAuthPDSClientFactory()) - - // Create test user - testID := uniqueTestID() - testUserHandle := fmt.Sprintf("dlt%s.local.coves.dev", testID) - testUserEmail := fmt.Sprintf("delete-%s@test.local", testID) - testUserPassword := "test-password-123" - - pdsAccessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - require.NoError(t, err, "creating the test account on the PDS") - - testUser := createTestUser(t, db, testUserHandle, userDID) - - // Create test post - testCommunityDID, _ := createFeedTestCommunity(db, ctx, "delete-community", "owner.test") - postURI := createTestPost(t, db, testCommunityDID, testUser.DID, "Test Post", 0, time.Now()) - postCID := "bafypost999" - - // Setup OAuth and HTTP server with real PDS access token - e2eAuth := NewE2EOAuthMiddleware() - token := e2eAuth.AddUserWithPDSToken(userDID, pdsAccessToken, pdsURL) - - r := chi.NewRouter() - routes.RegisterVoteRoutes(r, voteService, e2eAuth.OAuthAuthMiddleware) - httpServer := httptest.NewServer(r) - defer httpServer.Close() - - voteConsumer := jetstream.NewVoteEventConsumer(voteRepo, nil, db) - - // Create vote first - t.Logf("\n📝 Creating vote to delete...") - voteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - } - - reqBody, _ := json.Marshal(voteReq) - req, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.create", - bytes.NewBuffer(reqBody)) - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Bearer "+token) - - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("Failed to create vote: %v", err) - } - var voteResp struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if decodeErr := json.NewDecoder(resp.Body).Decode(&voteResp); decodeErr != nil { - t.Fatalf("Failed to decode vote response: %v", decodeErr) - } - if closeErr := resp.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - - // Index vote - rkey := utils.ExtractRKeyFromURI(voteResp.URI) - voteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-create", - Operation: "create", - Collection: "social.coves.feed.vote", - RKey: rkey, - CID: voteResp.CID, - Record: map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - "createdAt": time.Now().Format(time.RFC3339), - }, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &voteEvent); handleErr != nil { - t.Fatalf("Failed to handle vote event: %v", handleErr) - } - - t.Logf("✅ Vote created and indexed") - - // Delete vote via XRPC - t.Logf("\n📝 Deleting vote via XRPC...") - deleteReq := map[string]interface{}{ - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - } - - deleteBody, _ := json.Marshal(deleteReq) - deleteHttpReq, _ := http.NewRequest(http.MethodPost, - httpServer.URL+"/xrpc/social.coves.feed.vote.delete", - bytes.NewBuffer(deleteBody)) - deleteHttpReq.Header.Set("Content-Type", "application/json") - deleteHttpReq.Header.Set("Authorization", "Bearer "+token) - - deleteResp, err := http.DefaultClient.Do(deleteHttpReq) - if err != nil { - t.Fatalf("Failed to delete vote: %v", err) - } - defer func() { - if closeErr := deleteResp.Body.Close(); closeErr != nil { - t.Logf("Failed to close response body: %v", closeErr) - } - }() - - if deleteResp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(deleteResp.Body) - t.Fatalf("Delete failed: status %d, body: %s", deleteResp.StatusCode, string(body)) - } - - // Per lexicon, delete returns empty object {} - var deleteRespBody map[string]interface{} - if decodeErr := json.NewDecoder(deleteResp.Body).Decode(&deleteRespBody); decodeErr != nil { - t.Fatalf("Failed to decode delete response: %v", decodeErr) - } - - if len(deleteRespBody) != 0 { - t.Errorf("Expected empty object per lexicon, got %v", deleteRespBody) - } - - t.Logf("✅ Delete vote request succeeded") - - // Simulate Jetstream DELETE event - t.Logf("\n🔄 Simulating Jetstream DELETE event...") - deleteEvent := jetstream.JetstreamEvent{ - Did: userDID, - TimeUS: time.Now().UnixMicro(), - Kind: "commit", - Commit: &jetstream.CommitEvent{ - Rev: "test-vote-delete", - Operation: "delete", - Collection: "social.coves.feed.vote", - RKey: rkey, - }, - } - if handleErr := voteConsumer.HandleEvent(ctx, &deleteEvent); handleErr != nil { - t.Fatalf("Failed to handle delete event: %v", handleErr) - } - - // Verify vote removed from AppView - t.Logf("\n🔍 Verifying vote removed from AppView...") - _, err = voteRepo.GetByURI(ctx, voteResp.URI) - if err == nil { - t.Error("Expected vote to be deleted, but it still exists") - } - - // Verify post counts reset - updatedPost, _ := postRepo.GetByURI(ctx, postURI) - if updatedPost.UpvoteCount != 0 { - t.Errorf("Expected upvote_count = 0 after delete, got %d", updatedPost.UpvoteCount) - } - if updatedPost.Score != 0 { - t.Errorf("Expected score = 0 after delete, got %d", updatedPost.Score) - } - - t.Logf("✅ EXPLICIT DELETE FLOW COMPLETE:") - t.Logf(" ✓ Vote created and indexed") - t.Logf(" ✓ Vote deleted via XRPC") - t.Logf(" ✓ Vote removed from AppView") - t.Logf(" ✓ Post counts updated correctly") -} - -// TestVoteE2E_JetstreamIndexing tests real Jetstream firehose consumption -func TestVoteE2E_JetstreamIndexing(t *testing.T) { - db := testkit.DB(t) - - ctx := context.Background() - pdsURL := getTestPDSURL() - - // Setup repositories - voteRepo := postgres.NewVoteRepository(db) - - // Create test user on PDS - testID := uniqueTestID() - testUserHandle := fmt.Sprintf("jet%s.local.coves.dev", testID) - testUserEmail := fmt.Sprintf("jetstream-%s@test.local", testID) - testUserPassword := "test-password-123" - - accessToken, userDID, err := createPDSAccount(pdsURL, testUserHandle, testUserEmail, testUserPassword) - require.NoError(t, err, "creating the test account on the PDS") - - testUser := createTestUser(t, db, testUserHandle, userDID) - - // Create test post - testCommunityDID, _ := createFeedTestCommunity(db, ctx, "jetstream-community", "owner.test") - postURI := createTestPost(t, db, testCommunityDID, testUser.DID, "Test Post", 0, time.Now()) - postCID := "bafypostjetstream" - - // Capture the time before the PDS write: the Jetstream subscription below - // connects AFTER the write, so it must use a cursor from before the write - // to replay the event. A live-tail subscription races event propagation - // (PDS → relay → Jetstream) and loses whenever the pipeline is warm — - // this was a reliable failure in a full-suite run and a pass in - // isolation before the cursor was added. - subscribeCursorUS := time.Now().Add(-2 * time.Second).UnixMicro() - - // Write vote directly to PDS - t.Logf("\n📝 Writing vote to PDS...") - voteRecord := map[string]interface{}{ - "$type": "social.coves.feed.vote", - "subject": map[string]interface{}{ - "uri": postURI, - "cid": postCID, - }, - "direction": "up", - "createdAt": time.Now().Format(time.RFC3339), - } - - voteURI, voteCID, err := writePDSRecord(pdsURL, accessToken, userDID, "social.coves.feed.vote", "", voteRecord) - if err != nil { - t.Fatalf("Failed to write vote to PDS: %v", err) - } - - t.Logf("✅ Vote written to PDS:") - t.Logf(" URI: %s", voteURI) - t.Logf(" CID: %s", voteCID) - - // Setup Jetstream consumer - voteConsumer := jetstream.NewVoteEventConsumer(voteRepo, nil, db) - - // Subscribe to Jetstream - t.Logf("\n🔄 Subscribing to real Jetstream firehose...") - pdsHostname := strings.TrimPrefix(pdsURL, "http://") - pdsHostname = strings.TrimPrefix(pdsHostname, "https://") - pdsHostname = strings.Split(pdsHostname, ":")[0] - - jetstreamURL := fmt.Sprintf("ws://%s:6008/subscribe?wantedCollections=social.coves.feed.vote&cursor=%d", pdsHostname, subscribeCursorUS) - t.Logf(" Jetstream URL: %s", jetstreamURL) - t.Logf(" Looking for vote DID: %s", userDID) - - // Channels for event communication - eventChan := make(chan *jetstream.JetstreamEvent, 10) - errorChan := make(chan error, 1) - done := make(chan bool) - - // Start Jetstream consumer in background - go func() { - err := subscribeToJetstreamForVote(ctx, jetstreamURL, userDID, voteConsumer, eventChan, errorChan, done) - if err != nil { - errorChan <- err - } - }() - - // Wait for event or timeout - t.Logf("⏳ Waiting for Jetstream event (max 30 seconds)...") - - select { - case event := <-eventChan: - t.Logf("✅ Received real Jetstream event!") - t.Logf(" Event DID: %s", event.Did) - t.Logf(" Collection: %s", event.Commit.Collection) - t.Logf(" Operation: %s", event.Commit.Operation) - t.Logf(" RKey: %s", event.Commit.RKey) - - // Verify it's our vote - if event.Did != userDID { - t.Errorf("Expected DID %s, got %s", userDID, event.Did) - } - - // Verify indexed in AppView database - t.Logf("\n🔍 Querying AppView database...") - indexedVote, err := voteRepo.GetByURI(ctx, voteURI) - if err != nil { - t.Fatalf("Vote not indexed in AppView: %v", err) - } - - t.Logf("✅ Vote indexed in AppView:") - t.Logf(" VoterDID: %s", indexedVote.VoterDID) - t.Logf(" SubjectURI: %s", indexedVote.SubjectURI) - t.Logf(" Direction: %s", indexedVote.Direction) - t.Logf(" URI: %s", indexedVote.URI) - - // Signal to stop Jetstream consumer - close(done) - - case err := <-errorChan: - t.Fatalf("Jetstream error: %v", err) - - case <-time.After(30 * time.Second): - t.Fatalf("Timeout: No Jetstream event received within 30 seconds") - } - - t.Logf("\n✅ TRUE E2E JETSTREAM FLOW COMPLETE:") - t.Logf(" PDS → Jetstream → Consumer → AppView ✓") -} - -// subscribeToJetstreamForVote subscribes to real Jetstream firehose for vote events -func subscribeToJetstreamForVote( - ctx context.Context, - jetstreamURL string, - targetDID string, - consumer *jetstream.VoteEventConsumer, - eventChan chan<- *jetstream.JetstreamEvent, - errorChan chan<- error, - done <-chan bool, -) error { - conn, _, err := websocket.DefaultDialer.Dial(jetstreamURL, nil) - if err != nil { - return fmt.Errorf("failed to connect to Jetstream: %w", err) - } - defer func() { _ = conn.Close() }() - - // ONE deadline for the whole subscription, not one per read: the - // budget is what the caller is willing to wait in total, and a - // per-read deadline would let a busy stream extend it indefinitely. - readDeadline := time.Now().Add(jetstreamReadBudget) - - // Read messages until we find our event or receive done signal - for { - select { - case <-done: - return nil - case <-ctx.Done(): - return ctx.Err() - default: - // Set read deadline to avoid blocking forever - if err := conn.SetReadDeadline(readDeadline); err != nil { - return fmt.Errorf("failed to set read deadline: %w", err) - } - - var event jetstream.JetstreamEvent - err := conn.ReadJSON(&event) - if err != nil { - // Check if it's a timeout (expected) - if websocket.IsCloseError(err, websocket.CloseNormalClosure) { - return fmt.Errorf("Jetstream closed the subscription before the event arrived") - } - if netErr, ok := err.(net.Error); ok && netErr.Timeout() { - // The deadline is the whole budget, so its expiry is the answer: - // no matching event arrived. Reading on would be reading a - // connection gorilla has already marked failed. - return fmt.Errorf("no matching event within %s", jetstreamReadBudget) - } - return fmt.Errorf("failed to read Jetstream message: %w", err) - } - - // Check if this is the event we're looking for - if event.Did == targetDID && event.Kind == "commit" && event.Commit.Collection == "social.coves.feed.vote" { - // Process the event through the consumer - if err := consumer.HandleEvent(ctx, &event); err != nil { - return fmt.Errorf("failed to process event: %w", err) - } - - // Send to channel so test can verify - select { - case eventChan <- &event: - return nil - case <-time.After(1 * time.Second): - return fmt.Errorf("timeout sending event to channel") - } - } - } - } -} diff --git a/tests/testkit/appview.go b/tests/testkit/appview.go index 431d680..d81161d 100644 --- a/tests/testkit/appview.go +++ b/tests/testkit/appview.go @@ -333,6 +333,81 @@ func (c *XRPCClient) Get(ctx context.Context, path string, out any) error { return c.do(req, path, out) } +// BinaryResponse is a non-JSON response: what was served, and enough about it +// to assert the service really served content rather than merely not failing. +type BinaryResponse struct { + Status int + ContentType string + Body []byte +} + +// GetBinary fetches a plain path and returns the raw response. +// +// # WHY THIS EXISTS ALONGSIDE Get +// +// Get answers only "did this 2xx", and discards the body. That is the right +// shape for a health probe and the WRONG shape for asserting that an image URL +// serves an image: a 204 with no body satisfies "did not fail" while serving +// nothing at all, and so does a 200 whose body is an empty byte slice or an +// HTML error page the upstream returned with the wrong status. An image path is +// exactly where those distinctions matter, because the failure being guarded +// against — a proxy that cannot reach the blob store — is upstream of the +// status code the proxy chooses to report. +// +// So this returns the three facts a caller needs to make the real claim +// (status, content type, bytes) rather than folding them into a bool. The body +// is bounded: a test asserting an image is non-empty does not need to buffer an +// arbitrarily large one, and an unbounded read here would make a runaway +// response a hang instead of a failure. +// +// Unlike Get, a non-2xx is returned as a StatusError, so callers keep the +// familiar testkit.IsStatus handling. +func (c *XRPCClient) GetBinary(ctx context.Context, path string) (BinaryResponse, error) { + if !strings.HasPrefix(path, "/") { + return BinaryResponse{}, fmt.Errorf("testkit: path %q must start with \"/\"", path) + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.BaseURL+path, nil) + if err != nil { + return BinaryResponse{}, fmt.Errorf("%s: building request: %w", path, err) + } + for name, values := range c.Headers { + for _, value := range values { + req.Header.Add(name, value) + } + } + if c.Bearer != "" { + req.Header.Set("Authorization", "Bearer "+c.Bearer) + } + // Deliberately NOT "application/json": this path serves bytes, and a server + // content-negotiating on the header would be handed the wrong answer. + req.Header.Set("Accept", "*/*") + + resp, err := c.HTTP.Do(req) + if err != nil { + return BinaryResponse{}, fmt.Errorf("%s: %w", path, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return BinaryResponse{}, newStatusError(path, resp) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, maxBinaryBody)) + if err != nil { + return BinaryResponse{}, fmt.Errorf("%s: reading %d response: %w", path, resp.StatusCode, err) + } + return BinaryResponse{ + Status: resp.StatusCode, + ContentType: resp.Header.Get("Content-Type"), + Body: body, + }, nil +} + +// maxBinaryBody bounds how much of a binary response GetBinary buffers. Test +// fixtures are a few hundred bytes; this is generous enough that a truncation +// means something is wrong, and small enough that a runaway response fails +// rather than exhausts memory. +const maxBinaryBody = 8 << 20 + // Health calls the service's _health endpoint. // // It asserts only that the service answered 2xx. Both the AppView and the PDS