From f4a09133a636451c495fa96c98b99379b7692497 Mon Sep 17 00:00:00 2001 From: Seongmin Lee Date: Mon, 19 Jan 2026 15:15:20 +0000 Subject: [PATCH] appview/{db,state}: delete all duplicate records on delete Signed-off-by: Seongmin Lee --- appview/db/follow.go | 49 +++++++++++++++++++++++-------------------------- appview/db/reaction.go | 27 ++++++++++++++++++++++++--- appview/db/star.go | 52 ++++++++++++++++++++++------------------------------ appview/state/follow.go | 47 ++++++++++++++++++++++++++++++++--------------- appview/state/reaction.go | 36 ++++++++++++++++++++++++------------ appview/state/star.go | 46 +++++++++++++++++++++++++++++++--------------- 6 file(s) changed, 156 insertion(s)(+), 101 deletion(s)(-) diff --git a/appview/db/follow.go b/appview/db/follow.go --- a/appview/db/follow.go +++ b/appview/db/follow.go @@ -6,6 +6,7 @@ "strings" "time" + "github.com/bluesky-social/indigo/atproto/syntax" "tangled.org/core/appview/models" "tangled.org/core/orm" ) @@ -16,33 +17,29 @@ return err } -// Get a follow record -func GetFollow(e Execer, userDid, subjectDid string) (*models.Follow, error) { - query := `select did, subject_did, created, rkey from follows where did = ? and subject_did = ?` - row := e.QueryRow(query, userDid, subjectDid) - - var follow models.Follow - var followedAt string - err := row.Scan(&follow.UserDid, &follow.SubjectDid, &followedAt, &follow.Rkey) - if err != nil { - return nil, err - } - - followedAtTime, err := time.Parse(time.RFC3339, followedAt) - if err != nil { - log.Println("unable to determine followed at time") - follow.FollowedAt = time.Now() - } else { - follow.FollowedAt = followedAtTime - } - - return &follow, nil -} - // Remove a follow -func DeleteFollow(e Execer, userDid, subjectDid string) error { - _, err := e.Exec(`delete from follows where did = ? and subject_did = ?`, userDid, subjectDid) - return err +func DeleteFollow(e Execer, did, subjectDid syntax.DID) ([]syntax.ATURI, error) { + var deleted []syntax.ATURI + rows, err := e.Query( + `delete from follows + where did = ? and subject_did = ? + returning at_uri`, + did, + subjectDid, + ) + if err != nil { + return nil, fmt.Errorf("deleting stars: %w", err) + } + defer rows.Close() + + for rows.Next() { + var aturi syntax.ATURI + if err := rows.Scan(&aturi); err != nil { + return nil, fmt.Errorf("scanning at_uri: %w", err) + } + deleted = append(deleted, aturi) + } + return deleted, nil } // Remove a follow diff --git a/appview/db/reaction.go b/appview/db/reaction.go --- a/appview/db/reaction.go +++ b/appview/db/reaction.go @@ -1,6 +1,7 @@ package db import ( + "fmt" "log" "time" @@ -41,9 +42,29 @@ } // Remove a reaction -func DeleteReaction(e Execer, did string, subjectAt syntax.ATURI, kind models.ReactionKind) error { - _, err := e.Exec(`delete from reactions where did = ? and subject_at = ? and kind = ?`, did, subjectAt, kind) - return err +func DeleteReaction(e Execer, did syntax.DID, subjectAt syntax.ATURI, kind models.ReactionKind) ([]syntax.ATURI, error) { + var deleted []syntax.ATURI + rows, err := e.Query( + `delete from reactions + where did = ? and subject_at = ? and kind = ? + returning at_uri`, + did, + subjectAt, + kind, + ) + if err != nil { + return nil, fmt.Errorf("deleting stars: %w", err) + } + defer rows.Close() + + for rows.Next() { + var aturi syntax.ATURI + if err := rows.Scan(&aturi); err != nil { + return nil, fmt.Errorf("scanning at_uri: %w", err) + } + deleted = append(deleted, aturi) + } + return deleted, nil } // Remove a reaction diff --git a/appview/db/star.go b/appview/db/star.go --- a/appview/db/star.go +++ b/appview/db/star.go @@ -4,7 +4,6 @@ "database/sql" "errors" "fmt" - "log" "slices" "strings" "time" @@ -25,36 +24,29 @@ return err } -// Get a star record -func GetStar(e Execer, did string, subjectAt syntax.ATURI) (*models.Star, error) { - query := ` - select did, subject_at, created, rkey - from stars - where did = ? and subject_at = ?` - row := e.QueryRow(query, did, subjectAt) - - var star models.Star - var created string - err := row.Scan(&star.Did, &star.RepoAt, &created, &star.Rkey) - if err != nil { - return nil, err - } - - createdAtTime, err := time.Parse(time.RFC3339, created) - if err != nil { - log.Println("unable to determine followed at time") - star.Created = time.Now() - } else { - star.Created = createdAtTime - } - - return &star, nil -} - // Remove a star -func DeleteStar(e Execer, did string, subjectAt syntax.ATURI) error { - _, err := e.Exec(`delete from stars where did = ? and subject_at = ?`, did, subjectAt) - return err +func DeleteStar(tx *sql.Tx, did syntax.DID, subjectAt syntax.ATURI) ([]syntax.ATURI, error) { + var deleted []syntax.ATURI + rows, err := tx.Query( + `delete from stars + where did = ? and subject_at = ? + returning at_uri`, + did, + subjectAt, + ) + if err != nil { + return nil, fmt.Errorf("deleting stars: %w", err) + } + defer rows.Close() + + for rows.Next() { + var aturi syntax.ATURI + if err := rows.Scan(&aturi); err != nil { + return nil, fmt.Errorf("scanning at_uri: %w", err) + } + deleted = append(deleted, aturi) + } + return deleted, nil } // Remove a star diff --git a/appview/state/follow.go b/appview/state/follow.go --- a/appview/state/follow.go +++ b/appview/state/follow.go @@ -6,6 +6,7 @@ "time" comatproto "github.com/bluesky-social/indigo/api/atproto" + "github.com/bluesky-social/indigo/atproto/syntax" lexutil "github.com/bluesky-social/indigo/lex/util" "tangled.org/core/api/tangled" "tangled.org/core/appview/db" @@ -88,29 +89,47 @@ return case http.MethodDelete: - // find the record in the db - follow, err := db.GetFollow(s.db, currentUser.Active.Did, subjectIdent.DID.String()) + tx, err := s.db.BeginTx(r.Context(), nil) if err != nil { - log.Println("failed to get follow relationship") + s.logger.Error("failed to start transaction", "err", err) + } + defer tx.Rollback() + + follows, err := db.DeleteFollow(tx, syntax.DID(currentUser.Active.Did), subjectIdent.DID) + if err != nil { + s.logger.Error("failed to delete follows from db", "err", err) return } - _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ - Collection: tangled.GraphFollowNSID, - Repo: currentUser.Active.Did, - Rkey: follow.Rkey, + var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem + for _, followAt := range follows { + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ + RepoApplyWrites_Delete: &comatproto.RepoApplyWrites_Delete{ + Collection: tangled.GraphFollowNSID, + Rkey: followAt.RecordKey().String(), + }, + }) + } + _, err = comatproto.RepoApplyWrites(r.Context(), client, &comatproto.RepoApplyWrites_Input{ + Repo: currentUser.Active.Did, + Writes: writes, }) - if err != nil { - log.Println("failed to unfollow") + s.logger.Error("failed to delete follows from PDS", "err", err) return } - err = db.DeleteFollowByRkey(s.db, currentUser.Active.Did, follow.Rkey) - if err != nil { - log.Println("failed to delete follow from DB") - // this is not an issue, the firehose event might have already done this + if err := tx.Commit(); err != nil { + s.logger.Error("failed to commit transaction", "err", err) + // DB op failed but record is created in PDS. Ingester will backfill the missed operation } + + s.notifier.DeleteFollow(r.Context(), &models.Follow{ + UserDid: currentUser.Active.Did, + SubjectDid: subjectIdent.DID.String(), + // Rkey + // FollowedAt + }) followStats, err := db.GetFollowerFollowingCount(s.db, subjectIdent.DID.String()) if err != nil { @@ -122,8 +141,6 @@ FollowStatus: models.IsNotFollowing, FollowersCount: followStats.Followers, }) - - s.notifier.DeleteFollow(r.Context(), follow) return } diff --git a/appview/state/reaction.go b/appview/state/reaction.go --- a/appview/state/reaction.go +++ b/appview/state/reaction.go @@ -87,27 +87,39 @@ return case http.MethodDelete: - reaction, err := db.GetReaction(s.db, currentUser.Active.Did, subjectUri, reactionKind) + tx, err := s.db.BeginTx(r.Context(), nil) if err != nil { - log.Println("failed to get reaction relationship for", currentUser.Active.Did, subjectUri) + s.logger.Error("failed to start transaction", "err", err) + } + defer tx.Rollback() + + reactions, err := db.DeleteReaction(tx, syntax.DID(currentUser.Active.Did), subjectUri, reactionKind) + if err != nil { + s.logger.Error("failed to delete reactions from db", "err", err) return } - _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ - Collection: tangled.FeedReactionNSID, - Repo: currentUser.Active.Did, - Rkey: reaction.Rkey, + var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem + for _, reactionAt := range reactions { + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ + RepoApplyWrites_Delete: &comatproto.RepoApplyWrites_Delete{ + Collection: tangled.FeedReactionNSID, + Rkey: reactionAt.RecordKey().String(), + }, + }) + } + _, err = comatproto.RepoApplyWrites(r.Context(), client, &comatproto.RepoApplyWrites_Input{ + Repo: currentUser.Active.Did, + Writes: writes, }) - if err != nil { - log.Println("failed to remove reaction") + s.logger.Error("failed to delete reactions from PDS", "err", err) return } - err = db.DeleteReactionByRkey(s.db, currentUser.Active.Did, reaction.Rkey) - if err != nil { - log.Println("failed to delete reaction from DB") - // this is not an issue, the firehose event might have already done this + if err := tx.Commit(); err != nil { + s.logger.Error("failed to commit transaction", "err", err) + // DB op failed but record is created in PDS. Ingester will backfill the missed operation } reactionMap, err := db.GetReactionMap(s.db, 20, subjectUri) diff --git a/appview/state/star.go b/appview/state/star.go --- a/appview/state/star.go +++ b/appview/state/star.go @@ -83,37 +83,53 @@ return case http.MethodDelete: - // find the record in the db - star, err := db.GetStar(s.db, currentUser.Active.Did, subjectUri) + tx, err := s.db.BeginTx(r.Context(), nil) if err != nil { - log.Println("failed to get star relationship") + s.logger.Error("failed to start transaction", "err", err) + } + defer tx.Rollback() + + stars, err := db.DeleteStar(tx, syntax.DID(currentUser.Active.Did), subjectUri) + if err != nil { + s.logger.Error("failed to delete stars from db", "err", err) return } - _, err = comatproto.RepoDeleteRecord(r.Context(), client, &comatproto.RepoDeleteRecord_Input{ - Collection: tangled.FeedStarNSID, - Repo: currentUser.Active.Did, - Rkey: star.Rkey, + var writes []*comatproto.RepoApplyWrites_Input_Writes_Elem + for _, starAt := range stars { + writes = append(writes, &comatproto.RepoApplyWrites_Input_Writes_Elem{ + RepoApplyWrites_Delete: &comatproto.RepoApplyWrites_Delete{ + Collection: tangled.FeedStarNSID, + Rkey: starAt.RecordKey().String(), + }, + }) + } + _, err = comatproto.RepoApplyWrites(r.Context(), client, &comatproto.RepoApplyWrites_Input{ + Repo: currentUser.Active.Did, + Writes: writes, }) - if err != nil { - log.Println("failed to unstar") + s.logger.Error("failed to delete stars from PDS", "err", err) return } - err = db.DeleteStarByRkey(s.db, currentUser.Active.Did, star.Rkey) - if err != nil { - log.Println("failed to delete star from DB") - // this is not an issue, the firehose event might have already done this + if err := tx.Commit(); err != nil { + s.logger.Error("failed to commit transaction", "err", err) + // DB op failed but record is created in PDS. Ingester will backfill the missed operation } + + s.notifier.DeleteStar(r.Context(), &models.Star{ + Did: currentUser.Active.Did, + RepoAt: subjectUri, + // Rkey + // Created + }) starCount, err := db.GetStarCount(s.db, subjectUri) if err != nil { log.Println("failed to get star count for ", subjectUri) return } - - s.notifier.DeleteStar(r.Context(), star) s.pages.StarBtnFragment(w, pages.StarBtnFragmentParams{ IsStarred: false, -- tangled.sh