From 6bc929f2dc1e53420ed939e6a5abfb2d945ddbf8 Mon Sep 17 00:00:00 2001 From: Evan Jarrett Date: Sun, 26 Oct 2025 09:36:12 -0500 Subject: [PATCH] fix up ser creation logic when user doesn't have a bluesky profile record --- cmd/appview/serve.go | 23 ++++------- pkg/appview/db/queries.go | 23 +++++++++++ pkg/appview/jetstream/processor.go | 63 ++++++++++-------------------- 3 files changed, 52 insertions(+), 57 deletions(-) diff --git a/cmd/appview/serve.go b/cmd/appview/serve.go index 1bce3a8..0b31183 100644 --- a/cmd/appview/serve.go +++ b/cmd/appview/serve.go @@ -253,26 +253,19 @@ func serveRegistry(cmd *cobra.Command, args []string) error { profileRecord, err := client.GetProfileRecord(ctx, did) if err != nil { slog.Warn("Failed to fetch profile record", "component", "appview/callback", "did", did, "error", err) - // Still update user without avatar - _ = db.UpsertUser(uiDatabase, &db.User{ - DID: did, - Handle: handle, - PDSEndpoint: pdsEndpoint, - Avatar: "", - LastSeen: time.Now(), - }) - return nil // Non-fatal + // Continue without avatar - set profileRecord to nil to skip avatar extraction + profileRecord = nil } - // Construct avatar URL from blob CID using imgs.blue CDN - var avatarURL string - if profileRecord.Avatar != nil && profileRecord.Avatar.Ref.Link != "" { + // Construct avatar URL from blob CID using imgs.blue CDN (if profile record was fetched successfully) + avatarURL := "" + if profileRecord != nil && profileRecord.Avatar != nil && profileRecord.Avatar.Ref.Link != "" { avatarURL = atproto.BlobCDNURL(did, profileRecord.Avatar.Ref.Link) slog.Debug("Constructed avatar URL", "component", "appview/callback", "avatar_url", avatarURL) } - // Store user with avatar in database - err = db.UpsertUser(uiDatabase, &db.User{ + // Store user in database (with or without avatar) + err = db.UpsertUserIgnoreAvatar(uiDatabase, &db.User{ DID: did, Handle: handle, PDSEndpoint: pdsEndpoint, @@ -284,7 +277,7 @@ func serveRegistry(cmd *cobra.Command, args []string) error { return nil // Non-fatal } - slog.Debug("Stored user with avatar", "component", "appview/callback", "did", did) + slog.Debug("Stored user", "component", "appview/callback", "did", did, "has_avatar", avatarURL != "") // Migrate profile URL→DID if needed profile, err := storage.GetProfile(ctx, client) diff --git a/pkg/appview/db/queries.go b/pkg/appview/db/queries.go index 26c99d0..8a4bbdc 100644 --- a/pkg/appview/db/queries.go +++ b/pkg/appview/db/queries.go @@ -351,6 +351,29 @@ func UpsertUser(db *sql.DB, user *User) error { return err } +// UpsertUserIgnoreAvatar inserts or updates a user record, but preserves existing avatar on update +// This is useful when avatar fetch fails, and we don't want to overwrite an existing avatar with empty string +func UpsertUserIgnoreAvatar(db *sql.DB, user *User) error { + _, err := db.Exec(` + INSERT INTO users (did, handle, pds_endpoint, avatar, last_seen) + VALUES (?, ?, ?, ?, ?) + ON CONFLICT(did) DO UPDATE SET + handle = excluded.handle, + pds_endpoint = excluded.pds_endpoint, + last_seen = excluded.last_seen + `, user.DID, user.Handle, user.PDSEndpoint, user.Avatar, user.LastSeen) + return err +} + +// UpdateUserLastSeen updates only the last_seen timestamp for a user +// This is more efficient than UpsertUser when only updating activity timestamp +func UpdateUserLastSeen(db *sql.DB, did string) error { + _, err := db.Exec(` + UPDATE users SET last_seen = ? WHERE did = ? + `, time.Now(), did) + return err +} + // GetManifestDigestsForDID returns all manifest digests for a DID func GetManifestDigestsForDID(db *sql.DB, did string) ([]string, error) { rows, err := db.Query(` diff --git a/pkg/appview/jetstream/processor.go b/pkg/appview/jetstream/processor.go index 615bc93..b3fb1ff 100644 --- a/pkg/appview/jetstream/processor.go +++ b/pkg/appview/jetstream/processor.go @@ -48,74 +48,53 @@ func NewProcessor(database *sql.DB, useCache bool) *Processor { func (p *Processor) EnsureUser(ctx context.Context, did string) error { // Check cache first (if enabled) if p.useCache && p.userCache != nil { - if user, ok := p.userCache.cache[did]; ok { - // Update last seen - user.LastSeen = time.Now() - return db.UpsertUser(p.db, user) + if _, ok := p.userCache.cache[did]; ok { + // User in cache - just update last seen timestamp + return db.UpdateUserLastSeen(p.db, did) } } else if !p.useCache { // No cache - check if user already exists in DB existingUser, err := db.GetUserByDID(p.db, did) if err == nil && existingUser != nil { - // Update last seen - existingUser.LastSeen = time.Now() - return db.UpsertUser(p.db, existingUser) + // User exists - just update last seen timestamp + return db.UpdateUserLastSeen(p.db, did) } } // Resolve DID to get handle and PDS endpoint didParsed, err := syntax.ParseDID(did) if err != nil { - // Fallback: use DID as handle - user := &db.User{ - DID: did, - Handle: did, - PDSEndpoint: "https://bsky.social", - LastSeen: time.Now(), - } - if p.useCache { - p.userCache.cache[did] = user - } - return db.UpsertUser(p.db, user) + return fmt.Errorf("failed to parse DID: %w", err) } ident, err := p.directory.LookupDID(ctx, didParsed) if err != nil { - // Fallback: use DID as handle - user := &db.User{ - DID: did, - Handle: did, - PDSEndpoint: "https://bsky.social", - LastSeen: time.Now(), - } - if p.useCache { - p.userCache.cache[did] = user - } - return db.UpsertUser(p.db, user) + return fmt.Errorf("failed to lookup DID: %w", err) } resolvedDID := ident.DID.String() handle := ident.Handle.String() pdsEndpoint := ident.PDSEndpoint() - // If handle is invalid or PDS is missing, use defaults + // If handle is invalid, use DID as display name if handle == "handle.invalid" || handle == "" { handle = resolvedDID } + + // PDS endpoint is required - we can't make XRPC calls without it if pdsEndpoint == "" { - pdsEndpoint = "https://bsky.social" + return fmt.Errorf("no PDS endpoint found for DID: %s", resolvedDID) } - // Fetch user's Bluesky profile (including avatar) - // Use public Bluesky AppView API (doesn't require auth for public profiles) - avatar := "" - publicClient := atproto.NewClient("https://public.api.bsky.app", "", "") - profile, err := publicClient.GetActorProfile(ctx, resolvedDID) + // Fetch user's Bluesky profile record from their PDS (including avatar) + avatarURL := "" + client := atproto.NewClient(pdsEndpoint, "", "") + profileRecord, err := client.GetProfileRecord(ctx, resolvedDID) if err != nil { - slog.Warn("Failed to fetch profile", "component", "processor", "did", resolvedDID, "error", err) + slog.Warn("Failed to fetch profile record", "component", "processor", "did", resolvedDID, "error", err) // Continue without avatar - } else { - avatar = profile.Avatar + } else if profileRecord.Avatar != nil && profileRecord.Avatar.Ref.Link != "" { + avatarURL = atproto.BlobCDNURL(resolvedDID, profileRecord.Avatar.Ref.Link) } // Create user record @@ -123,7 +102,7 @@ func (p *Processor) EnsureUser(ctx context.Context, did string) error { DID: resolvedDID, Handle: handle, PDSEndpoint: pdsEndpoint, - Avatar: avatar, + Avatar: avatarURL, LastSeen: time.Now(), } @@ -132,8 +111,8 @@ func (p *Processor) EnsureUser(ctx context.Context, did string) error { p.userCache.cache[did] = user } - // Upsert to database - return db.UpsertUser(p.db, user) + // Upsert to database - preserve existing avatar if fetch failed + return db.UpsertUserIgnoreAvatar(p.db, user) } // ProcessManifest processes a manifest record and stores it in the database