From cdadaf47d34113cdcf9a65f718e3258d9cf885ad Mon Sep 17 00:00:00 2001 From: Michael Black Date: Tue, 23 Jun 2026 11:51:50 -0500 Subject: [PATCH] optimize network calls --- bskyweb/cmd/bskyweb/jsonld.go | 24 +++++++++ bskyweb/cmd/bskyweb/jsonld_test.go | 59 +++++++++++++++++++++ bskyweb/cmd/bskyweb/labels.go | 17 ++++++ bskyweb/cmd/bskyweb/labels_test.go | 73 +++++++++++++++++++++++++ bskyweb/cmd/bskyweb/server.go | 85 +++++++++++++++++------------- 5 files changed, 220 insertions(+), 38 deletions(-) diff --git a/bskyweb/cmd/bskyweb/jsonld.go b/bskyweb/cmd/bskyweb/jsonld.go index dc31f8f78d..a5ab849c35 100644 --- a/bskyweb/cmd/bskyweb/jsonld.go +++ b/bskyweb/cmd/bskyweb/jsonld.go @@ -358,6 +358,30 @@ func threadRootURI(pv *appbsky.FeedDefs_PostView) string { return rec.Reply.Root.Uri } +// findRootPostInParents walks tv's parent chain upward and returns the +// PostView whose URI matches rootURI, or nil if the root is not present. +// The root is absent when the chain is truncated by parentHeight (reply +// deeper than the fetched height) or broken by a blocked/not-found parent. +// Avoids a separate FeedGetPosts call when the thread response already +// contains the root. +func findRootPostInParents(tv *appbsky.FeedDefs_ThreadViewPost, rootURI string) *appbsky.FeedDefs_PostView { + if rootURI == "" { + return nil + } + for node := tv; node != nil; { + if node.Post != nil && node.Post.Uri == rootURI { + return node.Post + } + if node.Parent == nil { + return nil + } + // Only threadViewPost parents continue the chain; a blocked or + // not-found parent breaks it before reaching the root. + node = node.Parent.FeedDefs_ThreadViewPost + } + return nil +} + // buildAuthor constructs a Person. Organization classification for // custom-domain accounts is a future enhancement. func buildAuthor(author *appbsky.ActorDefs_ProfileViewBasic) *personOrOrg { diff --git a/bskyweb/cmd/bskyweb/jsonld_test.go b/bskyweb/cmd/bskyweb/jsonld_test.go index 09123356af..d3da1a0570 100644 --- a/bskyweb/cmd/bskyweb/jsonld_test.go +++ b/bskyweb/cmd/bskyweb/jsonld_test.go @@ -513,6 +513,65 @@ func TestThreadRootURI(t *testing.T) { } } +func TestFindRootPostInParents(t *testing.T) { + rootPost := makePostView("root.bsky.social", "did:plc:root", "rootrkey", "root") + rootURI := rootPost.Uri + + // tvp wraps a PostView, optionally chaining to a parent thread node. + tvp := func(pv *appbsky.FeedDefs_PostView, parent *appbsky.FeedDefs_ThreadViewPost) *appbsky.FeedDefs_ThreadViewPost { + node := &appbsky.FeedDefs_ThreadViewPost{Post: pv} + if parent != nil { + node.Parent = &appbsky.FeedDefs_ThreadViewPost_Parent{FeedDefs_ThreadViewPost: parent} + } + return node + } + + t.Run("direct reply, parent is root", func(t *testing.T) { + leaf := tvp(makePostView("alice.bsky.social", "did:plc:alice", "leaf", "reply"), tvp(rootPost, nil)) + if got := findRootPostInParents(leaf, rootURI); got != rootPost { + t.Errorf("expected root post, got %v", got) + } + }) + + t.Run("multi-level chain", func(t *testing.T) { + mid := tvp(makePostView("bob.bsky.social", "did:plc:bob", "mid", "mid"), tvp(rootPost, nil)) + leaf := tvp(makePostView("alice.bsky.social", "did:plc:alice", "leaf", "reply"), mid) + if got := findRootPostInParents(leaf, rootURI); got != rootPost { + t.Errorf("expected root post in chain, got %v", got) + } + }) + + t.Run("root absent, chain truncated", func(t *testing.T) { + // Topmost parent is not the root (e.g. parentHeight cut off the chain). + topmost := makePostView("bob.bsky.social", "did:plc:bob", "mid", "mid") + leaf := tvp(makePostView("alice.bsky.social", "did:plc:alice", "leaf", "reply"), tvp(topmost, nil)) + if got := findRootPostInParents(leaf, rootURI); got != nil { + t.Errorf("expected nil when root absent, got %v", got) + } + }) + + t.Run("chain broken by blocked parent", func(t *testing.T) { + // A blocked/not-found parent yields a nil FeedDefs_ThreadViewPost, + // breaking the walk before the root. + leaf := &appbsky.FeedDefs_ThreadViewPost{ + Post: makePostView("alice.bsky.social", "did:plc:alice", "leaf", "reply"), + Parent: &appbsky.FeedDefs_ThreadViewPost_Parent{ + FeedDefs_BlockedPost: &appbsky.FeedDefs_BlockedPost{Uri: rootURI}, + }, + } + if got := findRootPostInParents(leaf, rootURI); got != nil { + t.Errorf("expected nil when chain broken by blocked parent, got %v", got) + } + }) + + t.Run("empty rootURI", func(t *testing.T) { + leaf := tvp(makePostView("alice.bsky.social", "did:plc:alice", "leaf", "reply"), tvp(rootPost, nil)) + if got := findRootPostInParents(leaf, ""); got != nil { + t.Errorf("expected nil for empty rootURI, got %v", got) + } + }) +} + func TestBuildPostJSONLD_ReplyCommentsNoIsPartOf(t *testing.T) { // Replies surfaced under the main post as comment[] are Comment nodes and // never carry isPartOf, even when the main post has one. diff --git a/bskyweb/cmd/bskyweb/labels.go b/bskyweb/cmd/bskyweb/labels.go index 4055d4b9c8..2a1c77e3e0 100644 --- a/bskyweb/cmd/bskyweb/labels.go +++ b/bskyweb/cmd/bskyweb/labels.go @@ -20,3 +20,20 @@ func profileRequiresAuth(pv *appbsky.ActorDefs_ProfileViewDetailed) bool { } return false } + +// postAuthorRequiresAuth reports whether the post author self-applied the +// !no-unauthenticated label, read from the author view embedded in a +// getPostThread response. The appview surfaces the account's profile-record +// self-labels on the post author (src == author DID), so this mirrors +// profileRequiresAuth without a separate ActorGetProfile call. +func postAuthorRequiresAuth(pv *appbsky.FeedDefs_PostView) bool { + if pv == nil || pv.Author == nil { + return false + } + for _, label := range pv.Author.Labels { + if label.Src == pv.Author.Did && label.Val == "!no-unauthenticated" { + return true + } + } + return false +} diff --git a/bskyweb/cmd/bskyweb/labels_test.go b/bskyweb/cmd/bskyweb/labels_test.go index d55e31b207..5d385b7ad2 100644 --- a/bskyweb/cmd/bskyweb/labels_test.go +++ b/bskyweb/cmd/bskyweb/labels_test.go @@ -77,3 +77,76 @@ func TestProfileRequiresAuth(t *testing.T) { }) } } + +func TestPostAuthorRequiresAuth(t *testing.T) { + negTrue := true + + authorPV := func(labels []*comatprototypes.LabelDefs_Label) *appbsky.FeedDefs_PostView { + return &appbsky.FeedDefs_PostView{ + Author: &appbsky.ActorDefs_ProfileViewBasic{ + Did: "did:plc:alice", + Handle: "alice.bsky.social", + Labels: labels, + }, + } + } + + tests := []struct { + name string + pv *appbsky.FeedDefs_PostView + want bool + }{ + { + name: "nil post view", + pv: nil, + want: false, + }, + { + name: "nil author", + pv: &appbsky.FeedDefs_PostView{}, + want: false, + }, + { + name: "no labels", + pv: authorPV(nil), + want: false, + }, + { + name: "self-applied !no-unauthenticated", + pv: authorPV([]*comatprototypes.LabelDefs_Label{ + {Src: "did:plc:alice", Val: "!no-unauthenticated"}, + }), + want: true, + }, + { + name: "label from a different src does not gate", + pv: authorPV([]*comatprototypes.LabelDefs_Label{ + {Src: "did:plc:labeler", Val: "!no-unauthenticated"}, + }), + want: false, + }, + { + name: "different label value does not gate", + pv: authorPV([]*comatprototypes.LabelDefs_Label{ + {Src: "did:plc:alice", Val: "spam"}, + }), + want: false, + }, + { + // Negation isn't honored - matches profileRequiresAuth behavior. + name: "negated label still triggers (matches profile behavior)", + pv: authorPV([]*comatprototypes.LabelDefs_Label{ + {Src: "did:plc:alice", Val: "!no-unauthenticated", Neg: &negTrue}, + }), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := postAuthorRequiresAuth(tt.pv); got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} diff --git a/bskyweb/cmd/bskyweb/server.go b/bskyweb/cmd/bskyweb/server.go index 7c36b72560..7373dc5cdc 100644 --- a/bskyweb/cmd/bskyweb/server.go +++ b/bskyweb/cmd/bskyweb/server.go @@ -620,23 +620,39 @@ func (srv *Server) WebPost(c echo.Context) error { identifier := handleOrDID.Normalize().String() - // requires two fetches: first fetch profile (!) - pv, err := appbsky.ActorGetProfile(ctx, srv.xrpcc, identifier) - if err != nil { - log.Warnf("failed to fetch profile for: %s\t%v", identifier, err) - return c.Render(http.StatusOK, "post.html", data) - } - unauthedViewingOkay := !profileRequiresAuth(pv) - req := c.Request() requestURI := fmt.Sprintf("https://%s%s", req.Host, req.URL.Path) + // Fetch the post thread directly. The AT-URI authority accepts either a + // handle or a DID (the appview resolves it), so we skip the separate + // ActorGetProfile call and source identity, the canonical URL, and the + // auth gate from the thread response's author view instead. + // parentHeight=80 (the lexicon default) pulls the reply's ancestor chain + // up to the root in nearly all threads, letting isPartOf resolve from this + // response without a separate FeedGetPosts call. + uri := fmt.Sprintf("at://%s/app.bsky.feed.post/%s", identifier, rkey) + tpv, err := appbsky.FeedGetPostThread(ctx, srv.xrpcc, 1, 80, uri) + if err != nil { + log.Warnf("failed to fetch post: %s\t%v", uri, err) + return c.Render(http.StatusOK, "post.html", data) + } + + threadView := tpv.Thread.FeedDefs_ThreadViewPost + if threadView == nil || threadView.Post == nil || threadView.Post.Author == nil { + return c.Render(http.StatusOK, "post.html", data) + } + postView := threadView.Post + // Always prefer the handle-form URL so JSON-LD `url` and // match. Falls back to requestURI when the // handle is unusable (template strips query/fragment). - canonicalURL := bskyPostURL(pv.Handle, rkey.String()) + canonicalURL := bskyPostURL(postView.Author.Handle, rkey.String()) - if !unauthedViewingOkay { + // Gate before populating any post content into the template so that + // !no-unauthenticated posts never leak text/media. The appview returns the + // post (with the author self-label) to unauthed callers, so we detect the + // label here rather than via a profile fetch. + if postAuthorRequiresAuth(postView) { // Provide minimal OpenGraph data for auth-required posts data["requestURI"] = requestURI if canonicalURL != "" { @@ -645,26 +661,13 @@ func (srv *Server) WebPost(c echo.Context) error { data["requiresAuth"] = true data["noindex"] = true data["nofollow"] = true - data["profileHandle"] = pv.Handle - if pv.DisplayName != nil { - data["profileDisplayName"] = *pv.DisplayName + data["profileHandle"] = postView.Author.Handle + if postView.Author.DisplayName != nil { + data["profileDisplayName"] = *postView.Author.DisplayName } return c.Render(http.StatusOK, "post.html", data) } - // then fetch the post thread (with extra context) - uri := fmt.Sprintf("at://%s/app.bsky.feed.post/%s", pv.Did, rkey) - tpv, err := appbsky.FeedGetPostThread(ctx, srv.xrpcc, 1, 0, uri) - if err != nil { - log.Warnf("failed to fetch post: %s\t%v", uri, err) - return c.Render(http.StatusOK, "post.html", data) - } - - threadView := tpv.Thread.FeedDefs_ThreadViewPost - if threadView == nil || threadView.Post == nil { - return c.Render(http.StatusOK, "post.html", data) - } - postView := threadView.Post data["postView"] = postView data["requestURI"] = requestURI if canonicalURL != "" { @@ -696,21 +699,27 @@ func (srv *Server) WebPost(c echo.Context) error { } // Best-effort: resolve a reply's thread root to its handle-form canonical - // URL for isPartOf. Non-critical and bounded by a short timeout - on - // timeout, error, or an unresolvable root (deleted/taken-down) we omit - // isPartOf rather than point at a non-indexable page. + // URL for isPartOf. Prefer the root already present in the thread response + // (parentHeight=80); fall back to a bounded FeedGetPosts only when the + // chain is truncated (very deep thread) or broken by a blocked/not-found + // ancestor. On timeout, error, or an unresolvable root we omit isPartOf + // rather than point at a non-indexable page. isPartOfURL := "" if rootURI := threadRootURI(postView); rootURI != "" { - pctx, cancel := context.WithTimeout(ctx, 1*time.Second) - if posts, perr := appbsky.FeedGetPosts(pctx, srv.xrpcc, []string{rootURI}); perr != nil { - log.Warnf("failed to resolve thread root post for isPartOf: %s\t%v", rootURI, perr) - } else if len(posts.Posts) > 0 && posts.Posts[0].Author != nil { - // Handle-form only (no DID fallback): isPartOf must match the - // root page's handle-form canonical, so an unusable handle omits - // isPartOf rather than point at a non-canonical DID-form URL. - isPartOfURL = bskyPostURLFromATURI(posts.Posts[0].Author.Handle, rootURI) + if rootPost := findRootPostInParents(threadView, rootURI); rootPost != nil && rootPost.Author != nil { + isPartOfURL = bskyPostURLFromATURI(rootPost.Author.Handle, rootURI) + } else { + pctx, cancel := context.WithTimeout(ctx, 1*time.Second) + if posts, perr := appbsky.FeedGetPosts(pctx, srv.xrpcc, []string{rootURI}); perr != nil { + log.Warnf("failed to resolve thread root post for isPartOf: %s\t%v", rootURI, perr) + } else if len(posts.Posts) > 0 && posts.Posts[0].Author != nil { + // Handle-form only (no DID fallback): isPartOf must match the + // root page's handle-form canonical, so an unusable handle omits + // isPartOf rather than point at a non-canonical DID-form URL. + isPartOfURL = bskyPostURLFromATURI(posts.Posts[0].Author.Handle, rootURI) + } + cancel() } - cancel() } if jsonld, err := buildPostJSONLD(postView, threadView.Replies, jsonldURL, isPartOfURL, hideEmbedLabels, hideReplyLabels); err == nil {