From 7f2e5466a3d53e0a5d87a8ed05cc499d3871b97e Mon Sep 17 00:00:00 2001 From: Bretton Date: Mon, 8 Jun 2026 16:49:56 -0700 Subject: [PATCH] feat(posts): add social.coves.community.post.get batch endpoint Batch-fetch post views by AT-URI for feed hydration and permalink/cold-load rendering. Returns posts in request order; missing or soft-deleted posts come back as notFoundPost markers. - posts.Service.GetPosts + Repository.GetViewsByURIs (deduped batch fetch, canonical DID-URI validation, request-order assembly) - GET handler at /xrpc/social.coves.community.post.get with per-URI length caps and the shared batch-size limit - viewer vote-state enrichment, blob-ref and embed transforms on found posts Baseline reviewed via /second-opinion; review fixes follow on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) --- cmd/server/main.go | 2 +- internal/api/handlers/actor/get_posts_test.go | 4 + internal/api/handlers/post/get.go | 112 ++++++++++ internal/api/handlers/post/get_test.go | 120 +++++++++++ internal/api/routes/post.go | 23 +- .../core/comments/comment_service_test.go | 5 + internal/core/posts/interfaces.go | 17 +- internal/core/posts/post.go | 31 +++ internal/core/posts/service.go | 157 +++++++++++--- .../core/posts/service_author_posts_test.go | 10 +- internal/core/posts/service_get_posts_test.go | 197 ++++++++++++++++++ internal/db/postgres/post_repo.go | 65 +++++- internal/db/postgres/post_repo_cursor_test.go | 4 + tests/integration/user_journey_e2e_test.go | 2 +- 14 files changed, 710 insertions(+), 39 deletions(-) create mode 100644 internal/api/handlers/post/get.go create mode 100644 internal/api/handlers/post/get_test.go create mode 100644 internal/core/posts/service_get_posts_test.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 2cf56be..c340549 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -842,7 +842,7 @@ func main() { routes.RegisterCommunityRoutes(r, communityService, communityRepo, authMiddleware, allowedCommunityCreators) log.Println("Community XRPC endpoints registered with OAuth authentication") - routes.RegisterPostRoutes(r, postService, dualAuth) + routes.RegisterPostRoutes(r, postService, voteService, blueskyService, dualAuth, authMiddleware) log.Println("Post XRPC endpoints registered with dual auth (OAuth + service JWT for aggregators)") routes.RegisterVoteRoutes(r, voteService, authMiddleware) diff --git a/internal/api/handlers/actor/get_posts_test.go b/internal/api/handlers/actor/get_posts_test.go index 79a18b3..a3b6df8 100644 --- a/internal/api/handlers/actor/get_posts_test.go +++ b/internal/api/handlers/actor/get_posts_test.go @@ -38,6 +38,10 @@ func (m *mockPostService) DeletePost(ctx context.Context, session *oauthlib.Clie return nil } +func (m *mockPostService) GetPosts(ctx context.Context, req posts.GetPostsRequest) ([]*posts.PostResult, error) { + return nil, nil +} + // mockUserService implements users.UserService for testing type mockUserService struct { resolveHandleToDIDFunc func(ctx context.Context, handle string) (string, error) diff --git a/internal/api/handlers/post/get.go b/internal/api/handlers/post/get.go new file mode 100644 index 0000000..c90a483 --- /dev/null +++ b/internal/api/handlers/post/get.go @@ -0,0 +1,112 @@ +package post + +import ( + "encoding/json" + "fmt" + "log" + "net/http" + + "Coves/internal/api/handlers/common" + "Coves/internal/api/middleware" + "Coves/internal/core/blueskypost" + "Coves/internal/core/posts" + "Coves/internal/core/votes" +) + +// maxURILength caps each URI to prevent abuse via oversized query params. +// The batch size limit is shared with the service layer via posts.MaxGetPostsURIs. +const maxURILength = 512 + +// GetHandler handles batch post retrieval by AT-URI. +// Implements social.coves.community.post.get (feed hydration + permalink/cold-load). +type GetHandler struct { + service posts.Service + voteService votes.Service + blueskyService blueskypost.Service +} + +// NewGetHandler creates a new get handler. +// voteService is used to populate the viewer's vote state (may be nil). +// blueskyService is used to resolve embedded Bluesky posts (may be nil). +func NewGetHandler(service posts.Service, voteService votes.Service, blueskyService blueskypost.Service) *GetHandler { + return &GetHandler{ + service: service, + voteService: voteService, + blueskyService: blueskyService, + } +} + +// HandleGet handles GET /xrpc/social.coves.community.post.get?uris=at://...&uris=at://... +// Batch-fetches post views by AT-URI for feed hydration and permalink rendering. +// Posts are returned in the same order as the input URIs; missing or deleted posts +// are returned as notFoundPost markers ({uri, notFound: true}). +func (h *GetHandler) HandleGet(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodGet { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + return + } + + // Parse and validate the repeated `uris` query parameter + uris := r.URL.Query()["uris"] + if len(uris) == 0 { + writeError(w, http.StatusBadRequest, "InvalidRequest", "uris parameter is required") + return + } + if len(uris) > posts.MaxGetPostsURIs { + writeError(w, http.StatusBadRequest, "InvalidRequest", fmt.Sprintf("too many URIs (max %d)", posts.MaxGetPostsURIs)) + return + } + for _, uri := range uris { + if len(uri) > maxURILength { + writeError(w, http.StatusBadRequest, "InvalidRequest", "URI exceeds maximum length") + return + } + } + + // Optional viewer DID (set by OptionalAuth) for viewer-specific state + viewerDID := middleware.GetUserDID(r) + + results, err := h.service.GetPosts(r.Context(), posts.GetPostsRequest{ + URIs: uris, + ViewerDID: viewerDID, + }) + if err != nil { + handleServiceError(w, err) + return + } + + // Enrich found posts with the authenticated viewer's vote state (no-op if unauthenticated) + common.PopulateViewerVoteState(r.Context(), r, h.voteService, results) + + // Transform blob refs to URLs and resolve embedded Bluesky posts for found posts + for _, res := range results { + if res.Post != nil { + posts.TransformBlobRefsToURLs(res.Post) + posts.TransformPostEmbeds(r.Context(), res.Post, h.blueskyService) + } + } + + // Build the union output array in request order: postView or notFoundPost per slot + out := make([]interface{}, len(results)) + for i, res := range results { + if res.Post != nil { + out[i] = res.Post + } else { + out[i] = res.NotFound + } + } + + // Pre-encode to a buffer so an encoding failure still yields a proper error response + responseBytes, err := json.Marshal(map[string]interface{}{"posts": out}) + if err != nil { + log.Printf("ERROR: Failed to encode getPosts response: %v", err) + writeError(w, http.StatusInternalServerError, "InternalServerError", "Failed to encode response") + return + } + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if _, err := w.Write(responseBytes); err != nil { + log.Printf("ERROR: Failed to write getPosts response: %v", err) + } +} diff --git a/internal/api/handlers/post/get_test.go b/internal/api/handlers/post/get_test.go new file mode 100644 index 0000000..f63d825 --- /dev/null +++ b/internal/api/handlers/post/get_test.go @@ -0,0 +1,120 @@ +package post + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "Coves/internal/core/posts" + + oauthlib "github.com/bluesky-social/indigo/atproto/auth/oauth" +) + +// mockGetPostService implements posts.Service for testing the get handler. +type mockGetPostService struct { + getPostsFunc func(ctx context.Context, req posts.GetPostsRequest) ([]*posts.PostResult, error) +} + +func (m *mockGetPostService) CreatePost(ctx context.Context, req posts.CreatePostRequest) (*posts.CreatePostResponse, error) { + return nil, nil +} + +func (m *mockGetPostService) GetAuthorPosts(ctx context.Context, req posts.GetAuthorPostsRequest) (*posts.GetAuthorPostsResponse, error) { + return nil, nil +} + +func (m *mockGetPostService) DeletePost(ctx context.Context, session *oauthlib.ClientSessionData, req posts.DeletePostRequest) error { + return nil +} + +func (m *mockGetPostService) GetPosts(ctx context.Context, req posts.GetPostsRequest) ([]*posts.PostResult, error) { + if m.getPostsFunc != nil { + return m.getPostsFunc(ctx, req) + } + return nil, nil +} + +func TestHandleGet_MissingURIs(t *testing.T) { + h := NewGetHandler(&mockGetPostService{}, nil, nil) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/xrpc/social.coves.community.post.get", nil) + + h.HandleGet(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHandleGet_TooManyURIs(t *testing.T) { + h := NewGetHandler(&mockGetPostService{}, nil, nil) + rec := httptest.NewRecorder() + + q := "" + for i := 0; i < posts.MaxGetPostsURIs+1; i++ { + if i > 0 { + q += "&" + } + q += "uris=at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.post/r" + string(rune('a'+i%26)) + } + req := httptest.NewRequest(http.MethodGet, "/xrpc/social.coves.community.post.get?"+q, nil) + + h.HandleGet(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400", rec.Code) + } +} + +func TestHandleGet_Success_UnionOrder(t *testing.T) { + foundURI := "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.post/found" + missingURI := "at://did:plc:ewvi7nxzyoun6zhxrhs64oiz/social.coves.community.post/missing" + + svc := &mockGetPostService{ + getPostsFunc: func(ctx context.Context, req posts.GetPostsRequest) ([]*posts.PostResult, error) { + return []*posts.PostResult{ + {Post: &posts.PostView{URI: foundURI, CID: "cid1"}}, + {NotFound: &posts.NotFoundPost{URI: missingURI, NotFound: true}}, + }, nil + }, + } + h := NewGetHandler(svc, nil, nil) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, + "/xrpc/social.coves.community.post.get?uris="+foundURI+"&uris="+missingURI, nil) + + h.HandleGet(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (body: %s)", rec.Code, rec.Body.String()) + } + + var resp struct { + Posts []map[string]interface{} `json:"posts"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("failed to decode response: %v (body: %s)", err, rec.Body.String()) + } + if len(resp.Posts) != 2 { + t.Fatalf("expected 2 posts, got %d", len(resp.Posts)) + } + + // [0] is a postView (has uri, no notFound marker) + if got := resp.Posts[0]["uri"]; got != foundURI { + t.Errorf("posts[0].uri = %v, want %q", got, foundURI) + } + if _, hasNotFound := resp.Posts[0]["notFound"]; hasNotFound { + t.Errorf("posts[0] should be a postView, but has a notFound marker") + } + + // [1] is a notFoundPost + if got := resp.Posts[1]["notFound"]; got != true { + t.Errorf("posts[1].notFound = %v, want true", got) + } + if got := resp.Posts[1]["uri"]; got != missingURI { + t.Errorf("posts[1].uri = %v, want %q", got, missingURI) + } +} diff --git a/internal/api/routes/post.go b/internal/api/routes/post.go index d39909d..e2b96a9 100644 --- a/internal/api/routes/post.go +++ b/internal/api/routes/post.go @@ -3,18 +3,30 @@ package routes import ( "Coves/internal/api/handlers/post" "Coves/internal/api/middleware" + "Coves/internal/core/blueskypost" "Coves/internal/core/posts" + "Coves/internal/core/votes" "github.com/go-chi/chi/v5" ) // RegisterPostRoutes registers post-related XRPC endpoints on the router // Implements social.coves.community.post.* lexicon endpoints -// authMiddleware can be either OAuthAuthMiddleware or DualAuthMiddleware -func RegisterPostRoutes(r chi.Router, service posts.Service, authMiddleware middleware.AuthMiddleware) { +// authMiddleware can be either OAuthAuthMiddleware or DualAuthMiddleware (used for +// write procedures via RequireAuth). oauthMiddleware is the OAuth middleware whose +// OptionalAuth is used for public reads so authenticated viewers get viewer state. +func RegisterPostRoutes( + r chi.Router, + service posts.Service, + voteService votes.Service, + blueskyService blueskypost.Service, + authMiddleware middleware.AuthMiddleware, + oauthMiddleware *middleware.OAuthAuthMiddleware, +) { // Initialize handlers createHandler := post.NewCreateHandler(service) deleteHandler := post.NewDeleteHandler(service) + getHandler := post.NewGetHandler(service, voteService, blueskyService) // Procedure endpoints (POST) - require authentication // social.coves.community.post.create - create a new post in a community @@ -25,8 +37,13 @@ func RegisterPostRoutes(r chi.Router, service posts.Service, authMiddleware midd // Only post authors can delete their own posts r.With(authMiddleware.RequireAuth).Post("/xrpc/social.coves.community.post.delete", deleteHandler.HandleDelete) + // Query endpoints (GET) + // social.coves.community.post.get - batch fetch post views by AT-URI. + // Public endpoint with optional auth so authenticated viewers receive their vote state. + // Used for feed-skeleton hydration and permalink / cold-load rendering. + r.With(oauthMiddleware.OptionalAuth).Get("/xrpc/social.coves.community.post.get", getHandler.HandleGet) + // Future endpoints (Beta): - // r.Get("/xrpc/social.coves.community.post.get", getHandler.HandleGet) // r.With(authMiddleware.RequireAuth).Post("/xrpc/social.coves.community.post.update", updateHandler.HandleUpdate) // r.Get("/xrpc/social.coves.community.post.list", listHandler.HandleList) } diff --git a/internal/core/comments/comment_service_test.go b/internal/core/comments/comment_service_test.go index 9a6be3b..0f7934f 100644 --- a/internal/core/comments/comment_service_test.go +++ b/internal/core/comments/comment_service_test.go @@ -259,6 +259,11 @@ func (m *mockPostRepo) GetByAuthor(ctx context.Context, req posts.GetAuthorPosts return nil, nil, nil } +func (m *mockPostRepo) GetViewsByURIs(ctx context.Context, uris []string) (map[string]*posts.PostView, error) { + // Mock implementation - returns empty for tests + return map[string]*posts.PostView{}, nil +} + func (m *mockPostRepo) SoftDelete(ctx context.Context, uri string) error { // Mock implementation - just delete from map delete(m.posts, uri) diff --git a/internal/core/posts/interfaces.go b/internal/core/posts/interfaces.go index 39f698b..d5f2f8e 100644 --- a/internal/core/posts/interfaces.go +++ b/internal/core/posts/interfaces.go @@ -23,13 +23,20 @@ type Service interface { // Returns paginated feed with cursor GetAuthorPosts(ctx context.Context, req GetAuthorPostsRequest) (*GetAuthorPostsResponse, error) + // GetPosts batch-fetches post views by AT-URI for feed hydration and permalink + // (cold-load) rendering. Implements social.coves.community.post.get. + // URIs must be canonical DID-based AT-URIs; malformed or handle-based URIs are + // rejected with a validation error (handles are mutable and would break on rename). + // Results are returned in the same order as req.URIs; valid URIs whose post is + // missing or deleted come back as NotFoundPost markers. + GetPosts(ctx context.Context, req GetPostsRequest) ([]*PostResult, error) + // DeletePost deletes a post from the community's PDS repository // SECURITY: Only the post author can delete their own posts // Flow: Validate URI -> Fetch community -> Verify author -> Delete from PDS DeletePost(ctx context.Context, session *oauth.ClientSessionData, req DeletePostRequest) error // Future methods (Beta): - // GetPost(ctx context.Context, uri string, viewerDID *string) (*Post, error) // UpdatePost(ctx context.Context, req UpdatePostRequest) (*Post, error) // ListCommunityPosts(ctx context.Context, communityDID string, limit, offset int) ([]*Post, error) } @@ -42,9 +49,15 @@ type Repository interface { Create(ctx context.Context, post *Post) error // GetByURI retrieves a post by its AT-URI - // Used for E2E test verification and future GET endpoint + // Used for E2E test verification and single-record lookups (returns the raw + // record without author/community joins) GetByURI(ctx context.Context, uri string) (*Post, error) + // GetViewsByURIs retrieves full post views (with author + community joins) for a + // set of canonical DID-based AT-URIs. Returns a map keyed by URI; missing or + // soft-deleted posts are simply absent from the map. Backs social.coves.community.post.get. + GetViewsByURIs(ctx context.Context, uris []string) (map[string]*PostView, error) + // GetByAuthor retrieves posts authored by a specific user // Supports filtering by post type and community // Returns posts, cursor for pagination, and error diff --git a/internal/core/posts/post.go b/internal/core/posts/post.go index 846e041..21775da 100644 --- a/internal/core/posts/post.go +++ b/internal/core/posts/post.go @@ -70,6 +70,37 @@ type DeletePostRequest struct { URI string `json:"uri"` // AT-URI of the post to delete } +// GetPostsRequest represents input for batch-fetching post views by AT-URI. +// Matches social.coves.community.post.get parameters (plus viewer context). +// URIs must be canonical DID-based AT-URIs (at:///.../); +// handle-based authorities are rejected (handles are mutable and would break on rename). +type GetPostsRequest struct { + URIs []string // 1..25 canonical DID-based post AT-URIs to hydrate + ViewerDID string // Optional viewer DID (from OptionalAuth) for viewer state +} + +// NotFoundPost is a union member of the social.coves.community.post.get output, +// emitted when a requested URI cannot be resolved (deleted, never indexed, or an +// unresolvable/invalid authority). Matches social.coves.community.post.get#notFoundPost. +type NotFoundPost struct { + URI string `json:"uri"` + NotFound bool `json:"notFound"` // Always true (const per lexicon); discriminates the union on the wire +} + +// PostResult is one ordered element of a GetPosts response: exactly one of Post or +// NotFound is set. Post is set when the post was found and is visible; NotFound +// otherwise. (A blockedPost variant is defined in the lexicon but not yet produced.) +type PostResult struct { + Post *PostView + NotFound *NotFoundPost +} + +// GetPost returns the underlying PostView (nil for not-found results), satisfying +// the viewer-state enrichment helper's FeedPostProvider interface. +func (r *PostResult) GetPost() *PostView { + return r.Post +} + // PostRecord represents the actual atProto record structure written to PDS // This is the data structure that gets stored in the community's repository type PostRecord struct { diff --git a/internal/core/posts/service.go b/internal/core/posts/service.go index a35f3e4..8a6548e 100644 --- a/internal/core/posts/service.go +++ b/internal/core/posts/service.go @@ -620,6 +620,127 @@ func (s *postService) GetAuthorPosts(ctx context.Context, req GetAuthorPostsRequ }, nil } +// Bounds for the social.coves.community.post.get endpoint. +const ( + postCollection = "social.coves.community.post" + // MaxGetPostsURIs is the maximum number of URIs accepted by a single + // social.coves.community.post.get request (matches the lexicon maxLength). + // Exported so the handler layer reuses the same bound (single source of truth). + MaxGetPostsURIs = 25 +) + +// GetPosts batch-fetches post views by AT-URI for feed hydration and permalink +// (cold-load) rendering. Implements social.coves.community.post.get. +// +// URIs must be canonical DID-based AT-URIs (at:///social.coves.community.post/). +// Handle-based authorities are rejected: handles are mutable, so a handle-based URI +// would break (or, if the handle is later reassigned, mis-resolve to the wrong community) +// after a rename. Resolving a human-readable handle to a DID is the caller's job, done +// once at the edge. DIDs are permanent, so DID-based URIs stay valid forever. +// +// Flow: +// 1. Validate the URI count (1..25) and that every URI is a well-formed DID-based URI. +// A malformed or handle-based URI is a client error -> InvalidRequest, not a silent miss. +// 2. Batch fetch views for the (deduped) URIs. +// 3. Assemble results in request order; valid-but-absent URIs become notFoundPost. +// +// Viewer state (vote) and embed/blob transforms are applied by the handler layer. +func (s *postService) GetPosts(ctx context.Context, req GetPostsRequest) ([]*PostResult, error) { + // 1. Validate batch size + if len(req.URIs) == 0 { + return nil, NewValidationError("uris", "at least one URI is required") + } + if len(req.URIs) > MaxGetPostsURIs { + return nil, NewValidationError("uris", fmt.Sprintf("too many URIs (max %d)", MaxGetPostsURIs)) + } + + // Validate every URI up front and dedup for the batch fetch. A malformed or + // handle-based URI fails the whole request with a clear error rather than silently + // degrading to notFound, which would hide client bugs. + uniqueSet := make(map[string]struct{}, len(req.URIs)) + for _, uri := range req.URIs { + if err := validatePostURI(uri); err != nil { + return nil, err + } + uniqueSet[uri] = struct{}{} + } + + // 2. Batch fetch the (deduped) URIs + unique := make([]string, 0, len(uniqueSet)) + for uri := range uniqueSet { + unique = append(unique, uri) + } + views, err := s.repo.GetViewsByURIs(ctx, unique) + if err != nil { + return nil, fmt.Errorf("failed to fetch post views: %w", err) + } + + // 3. Assemble results in request order; valid-but-absent URIs become notFoundPost + results := make([]*PostResult, len(req.URIs)) + for i, uri := range req.URIs { + if view := views[uri]; view != nil { + results[i] = &PostResult{Post: view} + } else { + results[i] = &PostResult{NotFound: &NotFoundPost{URI: uri, NotFound: true}} + } + } + + return results, nil +} + +// parsePostURIParts splits a post AT-URI into its authority and rkey, validating the +// scheme, structure, collection, and that both authority and rkey are present. The +// authority may be a DID or a handle; callers enforce the DID requirement (if any) via +// requireDIDAuthority. field names the request parameter for error attribution (e.g. +// "uri" or "uris"). This is the single source of truth for post-URI structure rules, +// shared by validatePostURI (get) and parsePostURI (delete). Pure (no I/O), so unit-testable. +func parsePostURIParts(uri, field string) (authority string, rkey string, err error) { + if !strings.HasPrefix(uri, "at://") { + return "", "", NewValidationError(field, "invalid AT-URI: must start with at://") + } + parts := strings.Split(strings.TrimPrefix(uri, "at://"), "/") + if len(parts) != 3 { + return "", "", NewValidationError(field, "invalid post URI format: expected at://authority/"+postCollection+"/rkey") + } + authority, collection, rkey := parts[0], parts[1], parts[2] + if authority == "" { + return "", "", NewValidationError(field, "invalid post URI: missing authority") + } + if collection != postCollection { + return "", "", NewValidationError(field, fmt.Sprintf("invalid collection in URI: expected %s, got %s", postCollection, collection)) + } + if rkey == "" { + return "", "", NewValidationError(field, "invalid post URI: missing rkey") + } + return authority, rkey, nil +} + +// requireDIDAuthority enforces that a parsed post-URI authority is a DID (not a handle). +// Handles are mutable, so a handle-based URI would break after a community rename, or +// mis-resolve if the handle is later reassigned. field names the request parameter for errors. +func requireDIDAuthority(authority, field string) error { + if !strings.HasPrefix(authority, "did:") { + return NewValidationError(field, fmt.Sprintf("post URI authority must be a DID, got handle %q (resolve the community handle to its DID before calling)", authority)) + } + if err := validateDIDFormat(authority); err != nil { + return NewValidationError(field, fmt.Sprintf("invalid community DID in URI: %s", err.Error())) + } + return nil +} + +// validatePostURI verifies that uri is a well-formed, canonical (DID-based) post AT-URI: +// +// at:///social.coves.community.post/ +// +// Used by GetPosts; callers must resolve handles to DIDs before calling. +func validatePostURI(uri string) error { + authority, _, err := parsePostURIParts(uri, "uris") + if err != nil { + return err + } + return requireDIDAuthority(authority, "uris") +} + // validateGetAuthorPostsRequest validates the GetAuthorPosts request func (s *postService) validateGetAuthorPostsRequest(req *GetAuthorPostsRequest) error { // Validate actor DID is set @@ -805,37 +926,17 @@ func (s *postService) validateDeleteRequest(req *DeletePostRequest) error { // Format: at://community_did/social.coves.community.post/rkey // Returns community DID, rkey, and error func (s *postService) parsePostURI(uri string) (communityDID string, rkey string, err error) { - // Remove at:// prefix - withoutScheme := strings.TrimPrefix(uri, "at://") - parts := strings.Split(withoutScheme, "/") - - // Expected format: [community_did, collection, rkey] - if len(parts) != 3 { - return "", "", NewValidationError("uri", "invalid post URI format: expected at://did/collection/rkey") - } - - communityDID = parts[0] - collection := parts[1] - rkey = parts[2] - - // Validate collection type - if collection != "social.coves.community.post" { - return "", "", NewValidationError("uri", fmt.Sprintf("invalid collection in URI: expected social.coves.community.post, got %s", collection)) - } - - // Validate DID format - if err := validateDIDFormat(communityDID); err != nil { - return "", "", NewValidationError("uri", fmt.Sprintf("invalid community DID in URI: %s", err.Error())) + // Structure + DID-authority validation is shared with the get path (single source of truth). + communityDID, rkey, err = parsePostURIParts(uri, "uri") + if err != nil { + return "", "", err } - - // Validate rkey is not empty - if rkey == "" { - return "", "", NewValidationError("uri", "missing rkey in post URI") + if err := requireDIDAuthority(communityDID, "uri"); err != nil { + return "", "", err } - // Also verify with utils helper for consistency - extractedRkey := utils.ExtractRKeyFromURI(uri) - if extractedRkey != rkey { + // Defense-in-depth: verify rkey extraction is consistent with the utils helper. + if extractedRkey := utils.ExtractRKeyFromURI(uri); extractedRkey != rkey { return "", "", NewValidationError("uri", "URI parsing inconsistency") } diff --git a/internal/core/posts/service_author_posts_test.go b/internal/core/posts/service_author_posts_test.go index 225c8c4..363ff87 100644 --- a/internal/core/posts/service_author_posts_test.go +++ b/internal/core/posts/service_author_posts_test.go @@ -7,7 +7,8 @@ import ( // mockRepository implements Repository for testing type mockRepository struct { - getByAuthorFunc func(ctx context.Context, req GetAuthorPostsRequest) ([]*PostView, *string, error) + getByAuthorFunc func(ctx context.Context, req GetAuthorPostsRequest) ([]*PostView, *string, error) + getViewsByURIsFunc func(ctx context.Context, uris []string) (map[string]*PostView, error) } func (m *mockRepository) Create(ctx context.Context, post *Post) error { @@ -18,6 +19,13 @@ func (m *mockRepository) GetByURI(ctx context.Context, uri string) (*Post, error return nil, nil } +func (m *mockRepository) GetViewsByURIs(ctx context.Context, uris []string) (map[string]*PostView, error) { + if m.getViewsByURIsFunc != nil { + return m.getViewsByURIsFunc(ctx, uris) + } + return map[string]*PostView{}, nil +} + func (m *mockRepository) GetByAuthor(ctx context.Context, req GetAuthorPostsRequest) ([]*PostView, *string, error) { if m.getByAuthorFunc != nil { return m.getByAuthorFunc(ctx, req) diff --git a/internal/core/posts/service_get_posts_test.go b/internal/core/posts/service_get_posts_test.go new file mode 100644 index 0000000..b6bc5f1 --- /dev/null +++ b/internal/core/posts/service_get_posts_test.go @@ -0,0 +1,197 @@ +package posts + +import ( + "context" + "testing" +) + +const testCommunityDID = "did:plc:ewvi7nxzyoun6zhxrhs64oiz" + +func didPostURI(rkey string) string { + return "at://" + testCommunityDID + "/" + postCollection + "/" + rkey +} + +func TestParsePostURIParts(t *testing.T) { + // parsePostURIParts is the pure splitter: it validates scheme/structure/collection/rkey + // but does NOT check whether the authority is a DID or a handle. + tests := []struct { + name string + uri string + wantAuthority string + wantRKey string + wantErr bool + }{ + { + name: "valid DID authority", + uri: didPostURI("abc123"), + wantAuthority: testCommunityDID, + wantRKey: "abc123", + }, + { + name: "handle authority parses (DID check happens in validatePostURI)", + uri: "at://c-test-community/" + postCollection + "/abc123", + wantAuthority: "c-test-community", + wantRKey: "abc123", + }, + { + name: "missing at:// scheme", + uri: testCommunityDID + "/" + postCollection + "/abc123", + wantErr: true, + }, + { + name: "too few segments", + uri: "at://" + testCommunityDID + "/" + postCollection, + wantErr: true, + }, + { + name: "wrong collection", + uri: "at://" + testCommunityDID + "/app.bsky.feed.post/abc123", + wantErr: true, + }, + { + name: "missing rkey", + uri: "at://" + testCommunityDID + "/" + postCollection + "/", + wantErr: true, + }, + { + name: "empty authority", + uri: "at:///" + postCollection + "/abc123", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + authority, rkey, err := parsePostURIParts(tt.uri, "uris") + if tt.wantErr { + if err == nil { + t.Fatalf("parsePostURIParts(%q) = nil error, want error", tt.uri) + } + return + } + if err != nil { + t.Fatalf("parsePostURIParts(%q) unexpected error: %v", tt.uri, err) + } + if authority != tt.wantAuthority { + t.Errorf("authority = %q, want %q", authority, tt.wantAuthority) + } + if rkey != tt.wantRKey { + t.Errorf("rkey = %q, want %q", rkey, tt.wantRKey) + } + }) + } +} + +func TestValidatePostURI(t *testing.T) { + tests := []struct { + name string + uri string + wantErr bool + }{ + {name: "valid DID-based URI", uri: didPostURI("abc123"), wantErr: false}, + {name: "handle authority is rejected", uri: "at://c-test-community/" + postCollection + "/abc123", wantErr: true}, + {name: "missing scheme", uri: testCommunityDID + "/" + postCollection + "/abc", wantErr: true}, + {name: "wrong collection", uri: "at://" + testCommunityDID + "/app.bsky.feed.post/abc", wantErr: true}, + {name: "missing rkey", uri: "at://" + testCommunityDID + "/" + postCollection + "/", wantErr: true}, + {name: "malformed DID authority", uri: "at://did:plc:UPPERCASE/" + postCollection + "/abc", wantErr: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validatePostURI(tt.uri) + if tt.wantErr && err == nil { + t.Fatalf("validatePostURI(%q) = nil, want error", tt.uri) + } + if !tt.wantErr && err != nil { + t.Fatalf("validatePostURI(%q) = %v, want nil", tt.uri, err) + } + }) + } +} + +func TestGetPosts_Validation(t *testing.T) { + s := &postService{} + + t.Run("empty uris is rejected", func(t *testing.T) { + _, err := s.GetPosts(context.Background(), GetPostsRequest{URIs: nil}) + if err == nil || !IsValidationError(err) { + t.Fatalf("expected validation error for empty uris, got %v", err) + } + }) + + t.Run("too many uris is rejected", func(t *testing.T) { + uris := make([]string, MaxGetPostsURIs+1) + for i := range uris { + uris[i] = didPostURI("rkey") + } + _, err := s.GetPosts(context.Background(), GetPostsRequest{URIs: uris}) + if err == nil || !IsValidationError(err) { + t.Fatalf("expected validation error for too many uris, got %v", err) + } + }) +} + +// TestGetPosts_RejectsNonCanonicalURI verifies that malformed and handle-based URIs are +// rejected with a clear error (rather than silently degrading to notFound, which would +// hide client bugs and break on community handle changes). +func TestGetPosts_RejectsNonCanonicalURI(t *testing.T) { + s := &postService{} // repo not reached: validation happens before the fetch + + t.Run("handle-based URI is rejected", func(t *testing.T) { + _, err := s.GetPosts(context.Background(), GetPostsRequest{ + URIs: []string{"at://c-test-community/" + postCollection + "/abc"}, + }) + if err == nil || !IsValidationError(err) { + t.Fatalf("expected validation error for handle-based URI, got %v", err) + } + }) + + t.Run("malformed URI is rejected", func(t *testing.T) { + _, err := s.GetPosts(context.Background(), GetPostsRequest{URIs: []string{"not-a-uri"}}) + if err == nil || !IsValidationError(err) { + t.Fatalf("expected validation error for malformed URI, got %v", err) + } + }) +} + +// TestGetPosts_OrderingAndNotFound verifies request order is preserved and that valid +// DID-based URIs whose post is absent come back as notFoundPost markers. +func TestGetPosts_OrderingAndNotFound(t *testing.T) { + found := didPostURI("found1") + missing := didPostURI("missing1") + + repo := &mockRepository{ + getViewsByURIsFunc: func(ctx context.Context, uris []string) (map[string]*PostView, error) { + // Only the "found" URI exists in the AppView + return map[string]*PostView{ + found: {URI: found, CID: "cid-found"}, + }, nil + }, + } + s := &postService{repo: repo} + + results, err := s.GetPosts(context.Background(), GetPostsRequest{ + URIs: []string{found, missing}, + }) + if err != nil { + t.Fatalf("GetPosts returned error: %v", err) + } + if len(results) != 2 { + t.Fatalf("expected 2 results (input order preserved), got %d", len(results)) + } + + // [0] found -> postView + if results[0].Post == nil || results[0].NotFound != nil { + t.Fatalf("results[0] expected a Post, got %+v", results[0]) + } + if results[0].Post.URI != found { + t.Errorf("results[0].Post.URI = %q, want %q", results[0].Post.URI, found) + } + + // [1] missing (valid URI, absent in repo) -> notFoundPost echoing the requested URI + if results[1].Post != nil || results[1].NotFound == nil { + t.Fatalf("results[1] expected NotFound, got %+v", results[1]) + } + if results[1].NotFound.URI != missing || !results[1].NotFound.NotFound { + t.Errorf("results[1].NotFound = %+v, want {URI:%q, NotFound:true}", results[1].NotFound, missing) + } +} diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index 9c4f411..450c75c 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -136,6 +136,64 @@ func (r *postgresPostRepo) GetByURI(ctx context.Context, uri string) (*posts.Pos return &post, nil } +// GetViewsByURIs retrieves full post views for a set of canonical (DID-based) AT-URIs. +// Returns a map keyed by URI; URIs that are missing or soft-deleted are simply absent +// from the map (the caller emits notFoundPost markers for those). +// Reuses scanPostView for row scanning, so the SELECT column order must match GetByAuthor. +// Backs the social.coves.community.post.get endpoint (feed hydration + permalinks). +func (r *postgresPostRepo) GetViewsByURIs(ctx context.Context, uris []string) (map[string]*posts.PostView, error) { + result := make(map[string]*posts.PostView, len(uris)) + if len(uris) == 0 { + return result, nil + } + + // Build parameterized IN clause ($1, $2, ...) - bounded to 25 URIs by the handler + placeholders := make([]string, len(uris)) + args := make([]interface{}, len(uris)) + for i, uri := range uris { + placeholders[i] = fmt.Sprintf("$%d", i+1) + args[i] = uri + } + + query := fmt.Sprintf(` + SELECT + p.uri, p.cid, p.rkey, + p.author_did, u.handle as author_handle, + p.community_did, c.handle as community_handle, c.name as community_name, c.avatar_cid as community_avatar, c.pds_url as community_pds_url, + p.title, p.content, p.content_facets, p.embed, p.content_labels, + p.created_at, p.edited_at, p.indexed_at, + p.upvote_count, p.downvote_count, p.score, p.comment_count + FROM posts p + INNER JOIN users u ON p.author_did = u.did + INNER JOIN communities c ON p.community_did = c.did + WHERE p.uri IN (%s) AND p.deleted_at IS NULL + `, strings.Join(placeholders, ", ")) + + rows, err := r.db.QueryContext(ctx, query, args...) + if err != nil { + return nil, fmt.Errorf("failed to query posts by URIs: %w", err) + } + defer func() { + if err := rows.Close(); err != nil { + slog.Warn("failed to close rows", "error", err) + } + }() + + for rows.Next() { + postView, err := r.scanPostView(rows) + if err != nil { + return nil, fmt.Errorf("failed to scan post: %w", err) + } + result[postView.URI] = postView + } + + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("error iterating posts results: %w", err) + } + + return result, nil +} + // GetByAuthor retrieves posts by author with filtering and pagination // Supports filter options: posts_with_replies (default), posts_no_replies, posts_with_media // Uses cursor-based pagination with created_at + uri for stable ordering @@ -224,7 +282,7 @@ func (r *postgresPostRepo) GetByAuthor(ctx context.Context, req posts.GetAuthorP // Scan results var postViews []*posts.PostView for rows.Next() { - postView, err := r.scanAuthorPost(rows) + postView, err := r.scanPostView(rows) if err != nil { return nil, nil, fmt.Errorf("failed to scan author post: %w", err) } @@ -318,8 +376,9 @@ func (r *postgresPostRepo) SoftDelete(ctx context.Context, uri string) error { return nil } -// scanAuthorPost scans a database row into a PostView for author posts query -func (r *postgresPostRepo) scanAuthorPost(rows *sql.Rows) (*posts.PostView, error) { +// scanPostView scans a database row into a PostView. Shared by GetByAuthor and +// GetViewsByURIs; the SELECT column order in those queries must match the Scan below. +func (r *postgresPostRepo) scanPostView(rows *sql.Rows) (*posts.PostView, error) { var ( postView posts.PostView authorView posts.AuthorView diff --git a/internal/db/postgres/post_repo_cursor_test.go b/internal/db/postgres/post_repo_cursor_test.go index 1b32921..6275cfe 100644 --- a/internal/db/postgres/post_repo_cursor_test.go +++ b/internal/db/postgres/post_repo_cursor_test.go @@ -231,6 +231,10 @@ func (m *mockPostRepository) GetByAuthor(ctx context.Context, req posts.GetAutho return nil, nil, nil } +func (m *mockPostRepository) GetViewsByURIs(ctx context.Context, uris []string) (map[string]*posts.PostView, error) { + return map[string]*posts.PostView{}, nil +} + func (m *mockPostRepository) SoftDelete(ctx context.Context, uri string) error { return nil } diff --git a/tests/integration/user_journey_e2e_test.go b/tests/integration/user_journey_e2e_test.go index dfc683b..4e26f9a 100644 --- a/tests/integration/user_journey_e2e_test.go +++ b/tests/integration/user_journey_e2e_test.go @@ -144,7 +144,7 @@ func TestFullUserJourney_E2E(t *testing.T) { e2eAuth := NewE2EOAuthMiddleware() r := chi.NewRouter() routes.RegisterCommunityRoutes(r, communityService, communityRepo, e2eAuth.OAuthAuthMiddleware, nil) // nil = allow all community creators - routes.RegisterPostRoutes(r, postService, e2eAuth.OAuthAuthMiddleware) + routes.RegisterPostRoutes(r, postService, nil, nil, e2eAuth.OAuthAuthMiddleware, e2eAuth.OAuthAuthMiddleware) routes.RegisterTimelineRoutes(r, timelineService, nil, nil, e2eAuth.OAuthAuthMiddleware) httpServer := httptest.NewServer(r) defer httpServer.Close() -- 2.51.2