From 3533f07ecb31527c872545fe47e88f72fe0b3b98 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Wed, 6 May 2026 21:55:47 -0500 Subject: [PATCH] minor bug fixes, add ability see starred repos --- pkg/appview/db/queries.go | 88 +++++++++++++++++++ pkg/appview/db/queries_test.go | 84 ++++++++++++++++++ pkg/appview/handlers/user.go | 81 +++++++++++++++++ pkg/appview/jetstream/processor.go | 13 +-- pkg/appview/jetstream/processor_test.go | 44 ++++++++++ pkg/appview/routes/routes.go | 4 + pkg/appview/storage/tag_store.go | 9 ++ .../templates/components/nav-user.html | 1 + pkg/appview/templates/pages/user-starred.html | 58 ++++++++++++ pkg/appview/ui.go | 9 +- pkg/appview/ui_test.go | 31 +++++++ 11 files changed, 411 insertions(+), 11 deletions(-) create mode 100644 pkg/appview/templates/pages/user-starred.html diff --git a/pkg/appview/db/queries.go b/pkg/appview/db/queries.go index 6266571..df4c56f 100644 --- a/pkg/appview/db/queries.go +++ b/pkg/appview/db/queries.go @@ -2360,6 +2360,94 @@ func GetUserRepoCards(db DBTX, userDID string, currentUserDID string) ([]RepoCar return cards, nil } +// GetStarredRepoCards fetches repository cards for repositories starred by +// starrerDID. Stars whose target repo no longer has a manifest, lives on a +// hold the viewer can't access, or is currently taken down are silently +// dropped via the joins and filters. Ordered by star creation time DESC. +func GetStarredRepoCards(db DBTX, starrerDID string, currentUserDID string) ([]RepoCardData, error) { + query := ` + WITH starred AS ( + SELECT owner_did AS did, repository, created_at AS starred_at + FROM stars + WHERE starrer_did = ? + ), + latest_manifests AS ( + SELECT m.did, m.repository, MAX(m.id) as latest_id + FROM manifests m + JOIN starred st ON m.did = st.did AND m.repository = st.repository + WHERE m.hold_endpoint IN ` + accessibleHoldsSubquery + ` + GROUP BY m.did, m.repository + ) + SELECT + m.did, + u.handle, + COALESCE(u.avatar, ''), + m.repository, + COALESCE((SELECT value FROM repository_annotations WHERE did = m.did AND repository = m.repository AND key = 'org.opencontainers.image.title'), ''), + COALESCE((SELECT value FROM repository_annotations WHERE did = m.did AND repository = m.repository AND key = 'org.opencontainers.image.description'), ''), + COALESCE((SELECT value FROM repository_annotations WHERE did = m.did AND repository = m.repository AND key = 'io.atcr.icon'), ''), + COALESCE((SELECT COUNT(*) FROM stars WHERE owner_did = m.did AND repository = m.repository), 0), + COALESCE(rs.pull_count, 0), + COALESCE((SELECT COUNT(*) FROM stars WHERE starrer_did = ? AND owner_did = m.did AND repository = m.repository), 0), + COALESCE(m.artifact_type, 'container-image'), + COALESCE((SELECT tag FROM tags WHERE did = m.did AND repository = m.repository ORDER BY created_at DESC LIMIT 1), ''), + COALESCE(m.digest, ''), + MAX(rs.last_push, m.created_at), + COALESCE(rp.avatar_cid, ''), + st.starred_at + FROM latest_manifests lm + JOIN manifests m ON lm.latest_id = m.id + JOIN users u ON m.did = u.did + JOIN starred st ON st.did = m.did AND st.repository = m.repository + LEFT JOIN repository_stats rs ON m.did = rs.did AND m.repository = rs.repository + LEFT JOIN repo_pages rp ON m.did = rp.did AND m.repository = rp.repository + WHERE ` + activeTakedownClause("m") + ` + ORDER BY st.starred_at DESC + ` + + rows, err := db.Query(query, starrerDID, currentUserDID, currentUserDID, currentUserDID) + if err != nil { + return nil, err + } + defer rows.Close() + + var cards []RepoCardData + for rows.Next() { + var c RepoCardData + var ownerDID string + var isStarredInt int + var avatarCID string + var lastUpdatedStr sql.NullString + var starredAtStr sql.NullString + + if err := rows.Scan(&ownerDID, &c.OwnerHandle, &c.OwnerAvatarURL, &c.Repository, &c.Title, &c.Description, &c.IconURL, + &c.StarCount, &c.PullCount, &isStarredInt, &c.ArtifactType, &c.Tag, &c.Digest, &lastUpdatedStr, &avatarCID, &starredAtStr); err != nil { + return nil, err + } + c.IsStarred = isStarredInt > 0 + if lastUpdatedStr.Valid { + if t, err := parseTimestamp(lastUpdatedStr.String); err == nil { + c.LastUpdated = t + } + } + if avatarCID != "" { + c.IconURL = BlobCDNURL(ownerDID, avatarCID) + } + + cards = append(cards, c) + } + + if err := rows.Err(); err != nil { + return nil, err + } + + if err := PopulateRepoCardTags(db, cards); err != nil { + return nil, err + } + + return cards, nil +} + // RepoPage represents a repository page record cached from PDS type RepoPage struct { DID string diff --git a/pkg/appview/db/queries_test.go b/pkg/appview/db/queries_test.go index 29e2e91..31510e3 100644 --- a/pkg/appview/db/queries_test.go +++ b/pkg/appview/db/queries_test.go @@ -1744,6 +1744,90 @@ func TestGetUserRepositories_BulkGrouping(t *testing.T) { } } +// TestGetStarredRepoCards verifies the listing of repos starred by a user: +// stars whose target repo no longer has a manifest are silently dropped (the +// "still exists" filter the feature relies on), and results are ordered by +// star creation time DESC. +func TestGetStarredRepoCards(t *testing.T) { + db, err := InitDB("file:TestGetStarredRepoCards?mode=memory&cache=shared", LibsqlConfig{}) + if err != nil { + t.Fatalf("init db: %v", err) + } + defer db.Close() + + now := time.Now().UTC().Truncate(time.Second) + mediaType := "application/vnd.oci.image.manifest.v1+json" + + starrer := &User{DID: "did:plc:starrer", Handle: "starrer.test", PDSEndpoint: "https://pds.example", LastSeen: now} + if err := UpsertUser(db, starrer); err != nil { + t.Fatalf("upsert starrer: %v", err) + } + owner := &User{DID: "did:plc:owner", Handle: "owner.test", PDSEndpoint: "https://pds.example", LastSeen: now} + if err := UpsertUser(db, owner); err != nil { + t.Fatalf("upsert owner: %v", err) + } + // Owner of a deleted repo (still has a users row, just no manifests). + ghost := &User{DID: "did:plc:ghost", Handle: "ghost.test", PDSEndpoint: "https://pds.example", LastSeen: now} + if err := UpsertUser(db, ghost); err != nil { + t.Fatalf("upsert ghost: %v", err) + } + + if err := UpsertCaptainRecord(db, &HoldCaptainRecord{ + HoldDID: "did:web:hold.example", OwnerDID: "did:plc:holdowner", Public: true, + }); err != nil { + t.Fatalf("seed captain: %v", err) + } + + // Two existing repos owned by `owner`. + if _, err := InsertManifest(db, &Manifest{ + DID: owner.DID, Repository: "repo-old", Digest: "sha256:old", + HoldEndpoint: "did:web:hold.example", SchemaVersion: 2, MediaType: mediaType, + CreatedAt: now, + }); err != nil { + t.Fatalf("insert old manifest: %v", err) + } + if _, err := InsertManifest(db, &Manifest{ + DID: owner.DID, Repository: "repo-new", Digest: "sha256:new", + HoldEndpoint: "did:web:hold.example", SchemaVersion: 2, MediaType: mediaType, + CreatedAt: now.Add(5 * time.Second), + }); err != nil { + t.Fatalf("insert new manifest: %v", err) + } + + // Three stars: two for existing repos, one for a deleted repo. + if err := UpsertStar(db, starrer.DID, owner.DID, "repo-old", now); err != nil { + t.Fatalf("upsert star repo-old: %v", err) + } + if err := UpsertStar(db, starrer.DID, owner.DID, "repo-new", now.Add(10*time.Second)); err != nil { + t.Fatalf("upsert star repo-new: %v", err) + } + if err := UpsertStar(db, starrer.DID, ghost.DID, "deleted-repo", now.Add(20*time.Second)); err != nil { + t.Fatalf("upsert star deleted-repo: %v", err) + } + + cards, err := GetStarredRepoCards(db, starrer.DID, starrer.DID) + if err != nil { + t.Fatalf("GetStarredRepoCards: %v", err) + } + + if len(cards) != 2 { + t.Fatalf("expected 2 cards (deleted repo dropped), got %d: %+v", len(cards), cards) + } + + // Newest star first. + if cards[0].Repository != "repo-new" || cards[1].Repository != "repo-old" { + t.Errorf("expected order [repo-new, repo-old] (newest star first), got [%s, %s]", cards[0].Repository, cards[1].Repository) + } + + // IsStarred should reflect the viewer's perspective. Viewer == starrer here + // so every returned row is starred-by-viewer. + for _, c := range cards { + if !c.IsStarred { + t.Errorf("card %s/%s expected IsStarred=true for self-viewer", c.OwnerHandle, c.Repository) + } + } +} + // TestGetUserRepositories_Empty verifies the bulk-fetch path short-circuits // cleanly when the summary query returns no rows (no extra queries issued, // nil slice returned). diff --git a/pkg/appview/handlers/user.go b/pkg/appview/handlers/user.go index 8f610af..ffc1e79 100644 --- a/pkg/appview/handlers/user.go +++ b/pkg/appview/handlers/user.go @@ -107,3 +107,84 @@ func (h *UserPageHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } } + +// StarredRepositoriesHandler renders /u/{handle}/starred — the list of repos +// the viewed user has starred. Stars whose target repo no longer exists or is +// inaccessible are silently dropped by the underlying query. +type StarredRepositoriesHandler struct { + BaseUIHandler +} + +func (h *StarredRepositoriesHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { + identifier := chi.URLParam(r, "handle") + + did, resolvedHandle, pdsEndpoint, err := atproto.ResolveIdentity(r.Context(), identifier) + if err != nil { + RenderNotFound(w, r, &h.BaseUIHandler) + return + } + + viewedUser, err := db.GetUserByDID(h.ReadOnlyDB, did) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + hasProfile := true + if viewedUser == nil { + hasProfile = false + viewedUser = &db.User{ + DID: did, + Handle: resolvedHandle, + PDSEndpoint: pdsEndpoint, + } + } else if viewedUser.Handle != resolvedHandle { + _ = db.UpdateUserHandle(h.DB, did, resolvedHandle) + viewedUser.Handle = resolvedHandle + } + + var currentUserDID string + if user := middleware.GetUser(r); user != nil { + currentUserDID = user.DID + } + + var cardsErr bool + cards, err := db.GetStarredRepoCards(h.ReadOnlyDB, viewedUser.DID, currentUserDID) + if err != nil { + slog.Error("starred: fetch repo cards", "did", viewedUser.DID, "err", err) + cards = []db.RepoCardData{} + cardsErr = true + } + db.SetRegistryURL(cards, h.RegistryURL) + + pageData := NewPageData(r, &h.BaseUIHandler) + db.SetOciClient(cards, pageData.OciClient) + + meta := NewPageMeta( + viewedUser.Handle+"'s stars - "+h.ClientShortName, + "Repositories starred by "+viewedUser.Handle+" on "+h.ClientShortName+", the decentralized container registry", + ). + WithCanonical("https://" + h.SiteURL + "/u/" + viewedUser.Handle + "/starred"). + WithSiteName(h.ClientShortName) + + data := struct { + PageData + Meta *PageMeta + ViewedUser *db.User + Repositories []db.RepoCardData + HasProfile bool + HasError bool + }{ + PageData: pageData, + Meta: meta, + ViewedUser: viewedUser, + Repositories: cards, + HasProfile: hasProfile, + HasError: cardsErr, + } + + if err := h.Templates.ExecuteTemplate(w, "user-starred", data); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } +} diff --git a/pkg/appview/jetstream/processor.go b/pkg/appview/jetstream/processor.go index 3423f57..6aa2596 100644 --- a/pkg/appview/jetstream/processor.go +++ b/pkg/appview/jetstream/processor.go @@ -496,18 +496,21 @@ func (p *Processor) ProcessTag(ctx context.Context, did string, recordData []byt }) } -// ProcessStar processes a star record and stores it in the database +// ProcessStar processes a star record and stores it in the database. +// Records that don't match the current schema (bad JSON or unparseable +// subject AT URI) are skipped with a warning rather than failing the event, +// matching the backfill behavior. func (p *Processor) ProcessStar(ctx context.Context, did string, recordData []byte) error { - // Unmarshal star record (handles both old object and new AT URI subject formats) var starRecord atproto.StarRecord if err := json.Unmarshal(recordData, &starRecord); err != nil { - return fmt.Errorf("failed to unmarshal star: %w", err) + slog.Warn("Skipping invalid star record", "component", "processor", "did", did, "error", err) + return nil } - // Extract owner DID and repository from subject AT URI ownerDID, repository, err := starRecord.GetSubjectDIDAndRepository() if err != nil { - return fmt.Errorf("failed to parse star subject: %w", err) + slog.Warn("Skipping star with bad subject", "component", "processor", "did", did, "error", err) + return nil } // Ensure the starred repository's owner exists in the users table diff --git a/pkg/appview/jetstream/processor_test.go b/pkg/appview/jetstream/processor_test.go index 1196c47..05c3d80 100644 --- a/pkg/appview/jetstream/processor_test.go +++ b/pkg/appview/jetstream/processor_test.go @@ -489,6 +489,50 @@ func TestProcessStar(t *testing.T) { } } +// TestProcessStar_InvalidRecord verifies that star records that don't match +// the current schema (bad JSON or unparseable subject AT URI) are skipped +// with a warning rather than failing the firehose event. Regression guard +// for the behavior change from "return error" to "log warn + return nil". +func TestProcessStar_InvalidRecord(t *testing.T) { + database := setupTestDB(t) + defer database.Close() + + if _, err := database.Exec( + `INSERT INTO users (did, handle, pds_endpoint, last_seen) VALUES (?, ?, ?, ?)`, + "did:plc:starrer123", "starrer.test", "https://pds.example.com", time.Now()); err != nil { + t.Fatalf("Failed to insert starrer: %v", err) + } + + p := NewProcessor(database, false, nil) + ctx := context.Background() + + cases := []struct { + name string + body []byte + }{ + {name: "garbage JSON", body: []byte("not json at all")}, + {name: "missing subject", body: []byte(`{"$type":"io.atcr.sailor.star","createdAt":"2025-01-01T00:00:00Z"}`)}, + {name: "non-AT-URI subject", body: []byte(`{"$type":"io.atcr.sailor.star","subject":"https://example.com/notaturi","createdAt":"2025-01-01T00:00:00Z"}`)}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if err := p.ProcessStar(ctx, "did:plc:starrer123", tc.body); err != nil { + t.Errorf("ProcessStar with %s: expected nil error (skip-with-warning), got %v", tc.name, err) + } + }) + } + + // No rows should have been inserted. + var count int + if err := database.QueryRow("SELECT COUNT(*) FROM stars WHERE starrer_did = ?", "did:plc:starrer123").Scan(&count); err != nil { + t.Fatalf("query stars: %v", err) + } + if count != 0 { + t.Errorf("expected 0 stars after invalid records, got %d", count) + } +} + func TestProcessManifest_Duplicate(t *testing.T) { database := setupTestDB(t) defer database.Close() diff --git a/pkg/appview/routes/routes.go b/pkg/appview/routes/routes.go index 35fdac5..8dfa135 100644 --- a/pkg/appview/routes/routes.go +++ b/pkg/appview/routes/routes.go @@ -155,6 +155,10 @@ func RegisterUIRoutes(router chi.Router, deps UIDependencies) { &uihandlers.UserPageHandler{BaseUIHandler: base}, ).ServeHTTP) + router.Get("/u/{handle}/starred", middleware.OptionalAuth(deps.SessionStore, deps.Database)( + &uihandlers.StarredRepositoriesHandler{BaseUIHandler: base}, + ).ServeHTTP) + // OpenGraph image generation (public, cacheable) router.Get("/og/home", (&uihandlers.DefaultOGHandler{BaseUIHandler: base}).ServeHTTP) router.Get("/og/u/{handle}", (&uihandlers.UserOGHandler{BaseUIHandler: base}).ServeHTTP) diff --git a/pkg/appview/storage/tag_store.go b/pkg/appview/storage/tag_store.go index d4156de..7beb8ad 100644 --- a/pkg/appview/storage/tag_store.go +++ b/pkg/appview/storage/tag_store.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "atcr.io/pkg/atproto" "github.com/distribution/distribution/v3" @@ -121,6 +122,7 @@ func (s *TagStore) List(ctx context.Context, limit int, last string) ([]string, var result []string past := last == "" + reachedLimit := false for _, tag := range all { if !past { if tag == last { @@ -130,9 +132,16 @@ func (s *TagStore) List(ctx context.Context, limit int, last string) ([]string, } result = append(result, tag) if limit > 0 && len(result) >= limit { + reachedLimit = true break } } + // Signal end-of-list to the upstream handler so it knows there are no + // more entries to paginate over. Without io.EOF the handler indexes + // filled[len(filled)-1] and panics on an empty slice. + if !reachedLimit { + return result, io.EOF + } return result, nil } diff --git a/pkg/appview/templates/components/nav-user.html b/pkg/appview/templates/components/nav-user.html index 740d9d2..43c857c 100644 --- a/pkg/appview/templates/components/nav-user.html +++ b/pkg/appview/templates/components/nav-user.html @@ -21,6 +21,7 @@