diff --git a/internal/core/votes/service.go b/internal/core/votes/service.go deleted file mode 100644 index 5bf1446..0000000 --- a/internal/core/votes/service.go +++ /dev/null @@ -1,399 +0,0 @@ -package votes - -import ( - "Coves/internal/core/posts" - "bytes" - "context" - "encoding/json" - "fmt" - "io" - "log" - "net/http" - "strings" - "time" -) - -type voteService struct { - repo Repository - postRepo posts.Repository - pdsURL string -} - -// NewVoteService creates a new vote service -func NewVoteService( - repo Repository, - postRepo posts.Repository, - pdsURL string, -) Service { - return &voteService{ - repo: repo, - postRepo: postRepo, - pdsURL: pdsURL, - } -} - -// CreateVote creates a new vote or toggles an existing vote -// Toggle logic: -// - No vote -> Create vote -// - Same direction -> Delete vote (toggle off) -// - Different direction -> Delete old + Create new (toggle direction) -func (s *voteService) CreateVote(ctx context.Context, voterDID string, userAccessToken string, req CreateVoteRequest) (*CreateVoteResponse, error) { - // 1. Validate input - if voterDID == "" { - return nil, NewValidationError("voterDid", "required") - } - if userAccessToken == "" { - return nil, NewValidationError("userAccessToken", "required") - } - if req.Subject == "" { - return nil, NewValidationError("subject", "required") - } - if req.Direction != "up" && req.Direction != "down" { - return nil, ErrInvalidDirection - } - - // 2. Validate subject URI format (should be at://...) - if !strings.HasPrefix(req.Subject, "at://") { - return nil, ErrInvalidSubject - } - - // 3. Get subject post/comment to verify it exists and get its CID (for strong reference) - // For now, we assume the subject is a post. In the future, we'll support comments too. - post, err := s.postRepo.GetByURI(ctx, req.Subject) - if err != nil { - if err == posts.ErrNotFound { - return nil, ErrSubjectNotFound - } - return nil, fmt.Errorf("failed to get subject post: %w", err) - } - - // 4. Check for existing vote on PDS (source of truth for toggle logic) - // IMPORTANT: We query the user's PDS directly instead of AppView to avoid race conditions. - // AppView is eventually consistent (updated via Jetstream), so querying it can cause - // duplicate vote records if the user toggles before Jetstream catches up. - existingVoteRecord, err := s.findVoteOnPDS(ctx, voterDID, userAccessToken, req.Subject) - if err != nil { - return nil, fmt.Errorf("failed to check existing vote on PDS: %w", err) - } - - // 5. Handle toggle logic - var existingVoteURI *string - - if existingVoteRecord != nil { - // Vote exists on PDS - implement toggle logic - if existingVoteRecord.Direction == req.Direction { - // Same direction -> Delete vote (toggle off) - log.Printf("[VOTE-CREATE] Toggle off: deleting existing %s vote on %s", req.Direction, req.Subject) - - // Delete from user's PDS - if err := s.deleteRecordOnPDSAs(ctx, voterDID, "social.coves.interaction.vote", existingVoteRecord.RKey, userAccessToken); err != nil { - return nil, fmt.Errorf("failed to delete vote on PDS: %w", err) - } - - // Return empty response (vote was deleted, not created) - return &CreateVoteResponse{ - URI: "", - CID: "", - }, nil - } - - // Different direction -> Delete old vote first, then create new one below - log.Printf("[VOTE-CREATE] Toggle direction: %s -> %s on %s", existingVoteRecord.Direction, req.Direction, req.Subject) - - if err := s.deleteRecordOnPDSAs(ctx, voterDID, "social.coves.interaction.vote", existingVoteRecord.RKey, userAccessToken); err != nil { - return nil, fmt.Errorf("failed to delete old vote on PDS: %w", err) - } - - existingVoteURI = &existingVoteRecord.URI - } - - // 6. Build vote record with strong reference - voteRecord := map[string]interface{}{ - "$type": "social.coves.interaction.vote", - "subject": map[string]interface{}{ - "uri": req.Subject, - "cid": post.CID, - }, - "direction": req.Direction, - "createdAt": time.Now().Format(time.RFC3339), - } - - // 7. Write to user's PDS repository - recordURI, recordCID, err := s.createRecordOnPDSAs(ctx, voterDID, "social.coves.interaction.vote", "", voteRecord, userAccessToken) - if err != nil { - return nil, fmt.Errorf("failed to create vote on PDS: %w", err) - } - - log.Printf("[VOTE-CREATE] Created %s vote: %s (CID: %s)", req.Direction, recordURI, recordCID) - - // 8. Return response - return &CreateVoteResponse{ - URI: recordURI, - CID: recordCID, - Existing: existingVoteURI, - }, nil -} - -// DeleteVote removes a vote from a post/comment -func (s *voteService) DeleteVote(ctx context.Context, voterDID string, userAccessToken string, req DeleteVoteRequest) error { - // 1. Validate input - if voterDID == "" { - return NewValidationError("voterDid", "required") - } - if userAccessToken == "" { - return NewValidationError("userAccessToken", "required") - } - if req.Subject == "" { - return NewValidationError("subject", "required") - } - - // 2. Find existing vote on PDS (source of truth) - // IMPORTANT: Query PDS directly to avoid race conditions with AppView indexing - existingVoteRecord, err := s.findVoteOnPDS(ctx, voterDID, userAccessToken, req.Subject) - if err != nil { - return fmt.Errorf("failed to check existing vote on PDS: %w", err) - } - - if existingVoteRecord == nil { - return ErrVoteNotFound - } - - // 3. Delete from user's PDS - if err := s.deleteRecordOnPDSAs(ctx, voterDID, "social.coves.interaction.vote", existingVoteRecord.RKey, userAccessToken); err != nil { - return fmt.Errorf("failed to delete vote on PDS: %w", err) - } - - log.Printf("[VOTE-DELETE] Deleted vote: %s", existingVoteRecord.URI) - - return nil -} - -// GetVote retrieves a user's vote on a specific subject -func (s *voteService) GetVote(ctx context.Context, voterDID string, subjectURI string) (*Vote, error) { - return s.repo.GetByVoterAndSubject(ctx, voterDID, subjectURI) -} - -// Helper methods for PDS operations - -// createRecordOnPDSAs creates a record on the PDS using the user's access token -func (s *voteService) createRecordOnPDSAs(ctx context.Context, repoDID, collection, rkey string, record map[string]interface{}, accessToken string) (string, string, error) { - endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.createRecord", strings.TrimSuffix(s.pdsURL, "/")) - - payload := map[string]interface{}{ - "repo": repoDID, - "collection": collection, - "record": record, - } - - if rkey != "" { - payload["rkey"] = rkey - } - - return s.callPDSWithAuth(ctx, "POST", endpoint, payload, accessToken) -} - -// deleteRecordOnPDSAs deletes a record from the PDS using the user's access token -func (s *voteService) deleteRecordOnPDSAs(ctx context.Context, repoDID, collection, rkey, accessToken string) error { - endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.deleteRecord", strings.TrimSuffix(s.pdsURL, "/")) - - payload := map[string]interface{}{ - "repo": repoDID, - "collection": collection, - "rkey": rkey, - } - - _, _, err := s.callPDSWithAuth(ctx, "POST", endpoint, payload, accessToken) - return err -} - -// callPDSWithAuth makes a PDS call with a specific access token -func (s *voteService) callPDSWithAuth(ctx context.Context, method, endpoint string, payload map[string]interface{}, accessToken string) (string, string, error) { - jsonData, err := json.Marshal(payload) - if err != nil { - return "", "", fmt.Errorf("failed to marshal payload: %w", err) - } - - req, err := http.NewRequestWithContext(ctx, method, endpoint, bytes.NewBuffer(jsonData)) - if err != nil { - return "", "", fmt.Errorf("failed to create request: %w", err) - } - req.Header.Set("Content-Type", "application/json") - - // Add authentication with provided access token - if accessToken != "" { - req.Header.Set("Authorization", "Bearer "+accessToken) - } - - // Use 30 second timeout for write operations - timeout := 30 * time.Second - client := &http.Client{Timeout: timeout} - resp, err := client.Do(req) - if err != nil { - return "", "", fmt.Errorf("failed to call PDS: %w", err) - } - defer func() { - if closeErr := resp.Body.Close(); closeErr != nil { - log.Printf("Failed to close response body: %v", closeErr) - } - }() - - body, err := io.ReadAll(resp.Body) - if err != nil { - return "", "", fmt.Errorf("failed to read response: %w", err) - } - - if resp.StatusCode < 200 || resp.StatusCode >= 300 { - return "", "", fmt.Errorf("PDS returned error %d: %s", resp.StatusCode, string(body)) - } - - // Parse response to extract URI and CID - var result struct { - URI string `json:"uri"` - CID string `json:"cid"` - } - if err := json.Unmarshal(body, &result); err != nil { - return "", "", fmt.Errorf("failed to parse PDS response: %w", err) - } - - return result.URI, result.CID, nil -} - -// Helper functions - -// PDSVoteRecord represents a vote record returned from PDS listRecords -type PDSVoteRecord struct { - URI string - RKey string - Direction string - Subject struct { - URI string - CID string - } -} - -// findVoteOnPDS queries the user's PDS to find an existing vote on a specific subject -// This is the source of truth for toggle logic (avoiding AppView race conditions) -// -// IMPORTANT: This function paginates through ALL user votes with reverse=true (newest first) -// to handle users with >100 votes. Without pagination, votes on older posts would not be found, -// causing duplicate vote records and 404 errors on delete operations. -func (s *voteService) findVoteOnPDS(ctx context.Context, voterDID, accessToken, subjectURI string) (*PDSVoteRecord, error) { - const maxPages = 50 // Safety limit: prevent infinite loops (50 pages * 100 = 5000 votes max) - var cursor string - pageCount := 0 - - client := &http.Client{Timeout: 10 * time.Second} - - for { - pageCount++ - if pageCount > maxPages { - log.Printf("[VOTE-PDS] Reached max pagination limit (%d pages) searching for vote on %s", maxPages, subjectURI) - break - } - - // Build endpoint with pagination cursor and reverse=true (newest first) - endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.listRecords?repo=%s&collection=social.coves.interaction.vote&limit=100&reverse=true", - strings.TrimSuffix(s.pdsURL, "/"), voterDID) - - if cursor != "" { - endpoint += fmt.Sprintf("&cursor=%s", cursor) - } - - req, err := http.NewRequestWithContext(ctx, "GET", endpoint, nil) - if err != nil { - return nil, fmt.Errorf("failed to create request: %w", err) - } - - req.Header.Set("Authorization", "Bearer "+accessToken) - - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to query PDS: %w", err) - } - - if resp.StatusCode != http.StatusOK { - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - return nil, fmt.Errorf("PDS returned error %d: %s", resp.StatusCode, string(body)) - } - - var result struct { - Records []struct { - URI string `json:"uri"` - Value struct { - Subject struct { - URI string `json:"uri"` - CID string `json:"cid"` - } `json:"subject"` - Direction string `json:"direction"` - } `json:"value"` - } `json:"records"` - Cursor string `json:"cursor,omitempty"` // Pagination cursor for next page - } - - if err := json.NewDecoder(resp.Body).Decode(&result); err != nil { - resp.Body.Close() - return nil, fmt.Errorf("failed to decode PDS response: %w", err) - } - resp.Body.Close() - - // Find vote on this specific subject in current page - for _, record := range result.Records { - if record.Value.Subject.URI == subjectURI { - rkey := extractRKeyFromURI(record.URI) - log.Printf("[VOTE-PDS] Found existing vote on page %d: %s (direction: %s)", pageCount, record.URI, record.Value.Direction) - return &PDSVoteRecord{ - URI: record.URI, - RKey: rkey, - Direction: record.Value.Direction, - Subject: struct { - URI string - CID string - }{ - URI: record.Value.Subject.URI, - CID: record.Value.Subject.CID, - }, - }, nil - } - } - - // No more pages to check - if result.Cursor == "" { - log.Printf("[VOTE-PDS] No existing vote found after checking %d page(s)", pageCount) - break - } - - // Move to next page - cursor = result.Cursor - } - - // No vote found on this subject after paginating through all records - return nil, nil -} - -// extractRKeyFromURI extracts the rkey from an AT-URI (at://did/collection/rkey) -func extractRKeyFromURI(uri string) string { - parts := strings.Split(uri, "/") - if len(parts) >= 4 { - return parts[len(parts)-1] - } - return "" -} - -// ValidationError represents a validation error -type ValidationError struct { - Field string - Message string -} - -func (e *ValidationError) Error() string { - return fmt.Sprintf("validation error for field '%s': %s", e.Field, e.Message) -} - -// NewValidationError creates a new validation error -func NewValidationError(field, message string) error { - return &ValidationError{ - Field: field, - Message: message, - } -} diff --git a/internal/core/votes/service_test.go b/internal/core/votes/service_test.go deleted file mode 100644 index c344115..0000000 --- a/internal/core/votes/service_test.go +++ /dev/null @@ -1,344 +0,0 @@ -package votes - -import ( - "Coves/internal/core/posts" - "context" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" -) - -// Mock repositories for testing -type mockVoteRepository struct { - mock.Mock -} - -func (m *mockVoteRepository) Create(ctx context.Context, vote *Vote) error { - args := m.Called(ctx, vote) - return args.Error(0) -} - -func (m *mockVoteRepository) GetByURI(ctx context.Context, uri string) (*Vote, error) { - args := m.Called(ctx, uri) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*Vote), args.Error(1) -} - -func (m *mockVoteRepository) GetByVoterAndSubject(ctx context.Context, voterDID string, subjectURI string) (*Vote, error) { - args := m.Called(ctx, voterDID, subjectURI) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*Vote), args.Error(1) -} - -func (m *mockVoteRepository) Delete(ctx context.Context, uri string) error { - args := m.Called(ctx, uri) - return args.Error(0) -} - -func (m *mockVoteRepository) ListBySubject(ctx context.Context, subjectURI string, limit, offset int) ([]*Vote, error) { - args := m.Called(ctx, subjectURI, limit, offset) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).([]*Vote), args.Error(1) -} - -func (m *mockVoteRepository) ListByVoter(ctx context.Context, voterDID string, limit, offset int) ([]*Vote, error) { - args := m.Called(ctx, voterDID, limit, offset) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).([]*Vote), args.Error(1) -} - -type mockPostRepository struct { - mock.Mock -} - -func (m *mockPostRepository) GetByURI(ctx context.Context, uri string) (*posts.Post, error) { - args := m.Called(ctx, uri) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*posts.Post), args.Error(1) -} - -func (m *mockPostRepository) Create(ctx context.Context, post *posts.Post) error { - args := m.Called(ctx, post) - return args.Error(0) -} - -func (m *mockPostRepository) GetByRkey(ctx context.Context, communityDID, rkey string) (*posts.Post, error) { - args := m.Called(ctx, communityDID, rkey) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).(*posts.Post), args.Error(1) -} - -func (m *mockPostRepository) ListByCommunity(ctx context.Context, communityDID string, limit, offset int) ([]*posts.Post, error) { - args := m.Called(ctx, communityDID, limit, offset) - if args.Get(0) == nil { - return nil, args.Error(1) - } - return args.Get(0).([]*posts.Post), args.Error(1) -} - -func (m *mockPostRepository) Delete(ctx context.Context, uri string) error { - args := m.Called(ctx, uri) - return args.Error(0) -} - -// TestVoteService_CreateVote_NoExistingVote tests creating a vote when no vote exists -// NOTE: This test is skipped because we need to refactor service to inject HTTP client -// for testing PDS writes. The full flow is covered by E2E tests. -func TestVoteService_CreateVote_NoExistingVote(t *testing.T) { - t.Skip("Skipping because we need to refactor service to inject HTTP client for testing PDS writes - covered by E2E tests") - - // This test would verify: - // - Post exists check - // - No existing vote - // - PDS write succeeds - // - Response contains vote URI and CID -} - -// TestVoteService_ValidateInput tests input validation -func TestVoteService_ValidateInput(t *testing.T) { - mockVoteRepo := new(mockVoteRepository) - mockPostRepo := new(mockPostRepository) - - service := &voteService{ - repo: mockVoteRepo, - postRepo: mockPostRepo, - pdsURL: "http://mock-pds.test", - } - - ctx := context.Background() - - tests := []struct { - name string - voterDID string - accessToken string - req CreateVoteRequest - expectedError string - }{ - { - name: "missing voter DID", - voterDID: "", - accessToken: "token123", - req: CreateVoteRequest{Subject: "at://test", Direction: "up"}, - expectedError: "voterDid", - }, - { - name: "missing access token", - voterDID: "did:plc:test", - accessToken: "", - req: CreateVoteRequest{Subject: "at://test", Direction: "up"}, - expectedError: "userAccessToken", - }, - { - name: "missing subject", - voterDID: "did:plc:test", - accessToken: "token123", - req: CreateVoteRequest{Subject: "", Direction: "up"}, - expectedError: "subject", - }, - { - name: "invalid direction", - voterDID: "did:plc:test", - accessToken: "token123", - req: CreateVoteRequest{Subject: "at://test", Direction: "invalid"}, - expectedError: "invalid vote direction", - }, - { - name: "invalid subject format", - voterDID: "did:plc:test", - accessToken: "token123", - req: CreateVoteRequest{Subject: "http://not-at-uri", Direction: "up"}, - expectedError: "invalid subject URI", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := service.CreateVote(ctx, tt.voterDID, tt.accessToken, tt.req) - require.Error(t, err) - assert.Contains(t, err.Error(), tt.expectedError) - }) - } -} - -// TestVoteService_GetVote tests retrieving a vote -func TestVoteService_GetVote(t *testing.T) { - mockVoteRepo := new(mockVoteRepository) - mockPostRepo := new(mockPostRepository) - - service := &voteService{ - repo: mockVoteRepo, - postRepo: mockPostRepo, - pdsURL: "http://mock-pds.test", - } - - ctx := context.Background() - voterDID := "did:plc:voter123" - subjectURI := "at://did:plc:community/social.coves.post.record/abc123" - - expectedVote := &Vote{ - ID: 1, - URI: "at://did:plc:voter123/social.coves.interaction.vote/xyz789", - VoterDID: voterDID, - SubjectURI: subjectURI, - Direction: "up", - CreatedAt: time.Now(), - } - - mockVoteRepo.On("GetByVoterAndSubject", ctx, voterDID, subjectURI).Return(expectedVote, nil) - - result, err := service.GetVote(ctx, voterDID, subjectURI) - assert.NoError(t, err) - assert.Equal(t, expectedVote.URI, result.URI) - assert.Equal(t, expectedVote.Direction, result.Direction) - - mockVoteRepo.AssertExpectations(t) -} - -// TestVoteService_GetVote_NotFound tests getting a non-existent vote -func TestVoteService_GetVote_NotFound(t *testing.T) { - mockVoteRepo := new(mockVoteRepository) - mockPostRepo := new(mockPostRepository) - - service := &voteService{ - repo: mockVoteRepo, - postRepo: mockPostRepo, - pdsURL: "http://mock-pds.test", - } - - ctx := context.Background() - voterDID := "did:plc:voter123" - subjectURI := "at://did:plc:community/social.coves.post.record/noexist" - - mockVoteRepo.On("GetByVoterAndSubject", ctx, voterDID, subjectURI).Return(nil, ErrVoteNotFound) - - result, err := service.GetVote(ctx, voterDID, subjectURI) - assert.ErrorIs(t, err, ErrVoteNotFound) - assert.Nil(t, result) - - mockVoteRepo.AssertExpectations(t) -} - -// TestVoteService_SubjectNotFound tests voting on non-existent post -func TestVoteService_SubjectNotFound(t *testing.T) { - mockVoteRepo := new(mockVoteRepository) - mockPostRepo := new(mockPostRepository) - - service := &voteService{ - repo: mockVoteRepo, - postRepo: mockPostRepo, - pdsURL: "http://mock-pds.test", - } - - ctx := context.Background() - voterDID := "did:plc:voter123" - subjectURI := "at://did:plc:community/social.coves.post.record/noexist" - - // Mock post not found - mockPostRepo.On("GetByURI", ctx, subjectURI).Return(nil, posts.ErrNotFound) - - req := CreateVoteRequest{ - Subject: subjectURI, - Direction: "up", - } - - _, err := service.CreateVote(ctx, voterDID, "token123", req) - assert.ErrorIs(t, err, ErrSubjectNotFound) - - mockPostRepo.AssertExpectations(t) -} - -// NOTE: Testing toggle logic (same direction, different direction) requires mocking HTTP client -// These tests are covered by integration tests in tests/integration/vote_e2e_test.go -// To add unit tests for toggle logic, we would need to: -// 1. Refactor voteService to accept an HTTP client interface -// 2. Mock the PDS createRecord and deleteRecord calls -// 3. Verify the correct sequence of operations - -// Example of what toggle tests would look like (requires refactoring): -/* -func TestVoteService_ToggleSameDirection(t *testing.T) { - // Setup - mockVoteRepo := new(mockVoteRepository) - mockPostRepo := new(mockPostRepository) - mockPDSClient := new(mockPDSClient) - - service := &voteService{ - repo: mockVoteRepo, - postRepo: mockPostRepo, - pdsClient: mockPDSClient, // Would need to refactor to inject this - } - - ctx := context.Background() - voterDID := "did:plc:voter123" - subjectURI := "at://did:plc:community/social.coves.post.record/abc123" - - // Mock existing upvote - existingVote := &Vote{ - URI: "at://did:plc:voter123/social.coves.interaction.vote/existing", - VoterDID: voterDID, - SubjectURI: subjectURI, - Direction: "up", - } - mockVoteRepo.On("GetByVoterAndSubject", ctx, voterDID, subjectURI).Return(existingVote, nil) - - // Mock post exists - mockPostRepo.On("GetByURI", ctx, subjectURI).Return(&posts.Post{ - URI: subjectURI, - CID: "bafyreigpost123", - }, nil) - - // Mock PDS delete - mockPDSClient.On("DeleteRecord", voterDID, "social.coves.interaction.vote", "existing").Return(nil) - - // Execute: Click upvote when already upvoted -> should delete - req := CreateVoteRequest{ - Subject: subjectURI, - Direction: "up", // Same direction - } - - response, err := service.CreateVote(ctx, voterDID, "token123", req) - - // Assert - assert.NoError(t, err) - assert.Equal(t, "", response.URI, "Should return empty URI when toggled off") - mockPDSClient.AssertCalled(t, "DeleteRecord", voterDID, "social.coves.interaction.vote", "existing") - mockVoteRepo.AssertExpectations(t) - mockPostRepo.AssertExpectations(t) -} - -func TestVoteService_ToggleDifferentDirection(t *testing.T) { - // Similar test but existing vote is "up" and new vote is "down" - // Should delete old vote and create new vote - // Would verify: - // 1. DeleteRecord called for old vote - // 2. CreateRecord called for new vote - // 3. Response contains new vote URI -} -*/ - -// Documentation test to explain toggle logic (verified by E2E tests) -func TestVoteService_ToggleLogicDocumentation(t *testing.T) { - t.Log("Toggle Logic (verified by E2E tests in tests/integration/vote_e2e_test.go):") - t.Log("1. No existing vote + upvote clicked → Create upvote") - t.Log("2. Upvote exists + upvote clicked → Delete upvote (toggle off)") - t.Log("3. Upvote exists + downvote clicked → Delete upvote + Create downvote (switch)") - t.Log("4. Downvote exists + downvote clicked → Delete downvote (toggle off)") - t.Log("5. Downvote exists + upvote clicked → Delete downvote + Create upvote (switch)") - t.Log("") - t.Log("To add unit tests for toggle logic, refactor service to accept HTTP client interface") -}