appview/db: split star subjects #824

+80 -34

10 changed files

Diff round #0

expand all

unified split

appview

db.go

star.go

timeline.go

ingester.go

models

star.go

notify

db.go

posthog

notifier.go

pages

templates

timeline

fragments

timeline.html

state

profile.go

star.go

+41

appview/db/db.go

··· 1128 return err 1129 }) 1130 1131 return &DB{ 1132 db, 1133 logger,

··· 1128 return err 1129 }) 1130 1131 + // remove the foreign key constraints from stars. 1132 + // using same column names for backwards compatibility, but now `repo_at` 1133 + // column can represent literally anything. 1134 + runMigration(conn, logger, "generalize-stars-subject", func(tx *sql.Tx) error { 1135 + _, err := tx.Exec(` 1136 + create table stars_new ( 1137 + id integer primary key autoincrement, 1138 + did text not null, 1139 + rkey text not null, 1140 + 1141 + repo_at text not null, 1142 + 1143 + created text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), 1144 + unique(did, rkey), 1145 + unique(did, repo_at) 1146 + ); 1147 + 1148 + insert into stars_new ( 1149 + id, 1150 + did, 1151 + rkey, 1152 + repo_at, 1153 + created 1154 + ) 1155 + select 1156 + id, 1157 + starred_by_did, 1158 + rkey, 1159 + repo_at, 1160 + created 1161 + from stars; 1162 + 1163 + drop table stars; 1164 + alter table stars_new rename to stars; 1165 + 1166 + create index if not exists idx_stars_created on stars(created); 1167 + create index if not exists idx_stars_repo_at_created on stars(repo_at, created); 1168 + `) 1169 + return err 1170 + }) 1171 + 1172 return &DB{ 1173 db, 1174 logger,

+17 -17

appview/db/star.go

··· 14 ) 15 16 func AddStar(e Execer, star *models.Star) error { 17 - query := `insert or ignore into stars (starred_by_did, repo_at, rkey) values (?, ?, ?)` 18 _, err := e.Exec( 19 query, 20 - star.StarredByDid, 21 star.RepoAt.String(), 22 star.Rkey, 23 ) ··· 25 } 26 27 // Get a star record 28 - func GetStar(e Execer, starredByDid string, repoAt syntax.ATURI) (*models.Star, error) { 29 query := ` 30 - select starred_by_did, repo_at, created, rkey 31 from stars 32 - where starred_by_did = ? and repo_at = ?` 33 - row := e.QueryRow(query, starredByDid, repoAt) 34 35 var star models.Star 36 var created string 37 - err := row.Scan(&star.StarredByDid, &star.RepoAt, &created, &star.Rkey) 38 if err != nil { 39 return nil, err 40 } ··· 51 } 52 53 // Remove a star 54 - func DeleteStar(e Execer, starredByDid string, repoAt syntax.ATURI) error { 55 - _, err := e.Exec(`delete from stars where starred_by_did = ? and repo_at = ?`, starredByDid, repoAt) 56 return err 57 } 58 59 // Remove a star 60 - func DeleteStarByRkey(e Execer, starredByDid string, rkey string) error { 61 - _, err := e.Exec(`delete from stars where starred_by_did = ? and rkey = ?`, starredByDid, rkey) 62 return err 63 } 64 65 func GetStarCount(e Execer, repoAt syntax.ATURI) (int, error) { 66 stars := 0 67 err := e.QueryRow( 68 - `select count(starred_by_did) from stars where repo_at = ?`, repoAt).Scan(&stars) 69 if err != nil { 70 return 0, err 71 } ··· 91 query := fmt.Sprintf(` 92 SELECT repo_at 93 FROM stars 94 - WHERE starred_by_did = ? AND repo_at IN (%s) 95 `, strings.Join(placeholders, ",")) 96 97 rows, err := e.Query(query, args...) ··· 149 } 150 151 repoQuery := fmt.Sprintf( 152 - `select starred_by_did, repo_at, created, rkey 153 from stars 154 %s 155 order by created desc ··· 166 for rows.Next() { 167 var star models.Star 168 var created string 169 - err := rows.Scan(&star.StarredByDid, &star.RepoAt, &created, &star.Rkey) 170 if err != nil { 171 return nil, err 172 } ··· 252 253 rows, err := e.Query(` 254 select 255 - s.starred_by_did, 256 s.repo_at, 257 s.rkey, 258 s.created, ··· 276 var starCreatedAt, repoCreatedAt string 277 278 if err := rows.Scan( 279 - &star.StarredByDid, 280 &star.RepoAt, 281 &star.Rkey, 282 &starCreatedAt,

··· 14 ) 15 16 func AddStar(e Execer, star *models.Star) error { 17 + query := `insert or ignore into stars (did, repo_at, rkey) values (?, ?, ?)` 18 _, err := e.Exec( 19 query, 20 + star.Did, 21 star.RepoAt.String(), 22 star.Rkey, 23 ) ··· 25 } 26 27 // Get a star record 28 + func GetStar(e Execer, did string, repoAt syntax.ATURI) (*models.Star, error) { 29 query := ` 30 + select did, repo_at, created, rkey 31 from stars 32 + where did = ? and repo_at = ?` 33 + row := e.QueryRow(query, did, repoAt) 34 35 var star models.Star 36 var created string 37 + err := row.Scan(&star.Did, &star.RepoAt, &created, &star.Rkey) 38 if err != nil { 39 return nil, err 40 } ··· 51 } 52 53 // Remove a star 54 + func DeleteStar(e Execer, did string, repoAt syntax.ATURI) error { 55 + _, err := e.Exec(`delete from stars where did = ? and repo_at = ?`, did, repoAt) 56 return err 57 } 58 59 // Remove a star 60 + func DeleteStarByRkey(e Execer, did string, rkey string) error { 61 + _, err := e.Exec(`delete from stars where did = ? and rkey = ?`, did, rkey) 62 return err 63 } 64 65 func GetStarCount(e Execer, repoAt syntax.ATURI) (int, error) { 66 stars := 0 67 err := e.QueryRow( 68 + `select count(did) from stars where repo_at = ?`, repoAt).Scan(&stars) 69 if err != nil { 70 return 0, err 71 } ··· 91 query := fmt.Sprintf(` 92 SELECT repo_at 93 FROM stars 94 + WHERE did = ? AND repo_at IN (%s) 95 `, strings.Join(placeholders, ",")) 96 97 rows, err := e.Query(query, args...) ··· 149 } 150 151 repoQuery := fmt.Sprintf( 152 + `select did, repo_at, created, rkey 153 from stars 154 %s 155 order by created desc ··· 166 for rows.Next() { 167 var star models.Star 168 var created string 169 + err := rows.Scan(&star.Did, &star.RepoAt, &created, &star.Rkey) 170 if err != nil { 171 return nil, err 172 } ··· 252 253 rows, err := e.Query(` 254 select 255 + s.did, 256 s.repo_at, 257 s.rkey, 258 s.created, ··· 276 var starCreatedAt, repoCreatedAt string 277 278 if err := rows.Scan( 279 + &star.Did, 280 &star.RepoAt, 281 &star.Rkey, 282 &starCreatedAt,

+1 -1

appview/db/timeline.go

··· 146 func getTimelineStars(e Execer, limit int, loggedInUserDid string, userIsFollowing []string) ([]models.TimelineEvent, error) { 147 filters := make([]filter, 0) 148 if userIsFollowing != nil { 149 - filters = append(filters, FilterIn("starred_by_did", userIsFollowing)) 150 } 151 152 stars, err := GetStars(e, limit, filters...)

··· 146 func getTimelineStars(e Execer, limit int, loggedInUserDid string, userIsFollowing []string) ([]models.TimelineEvent, error) { 147 filters := make([]filter, 0) 148 if userIsFollowing != nil { 149 + filters = append(filters, FilterIn("did", userIsFollowing)) 150 } 151 152 stars, err := GetStars(e, limit, filters...)

+3 -3

appview/ingester.go

··· 121 return err 122 } 123 err = db.AddStar(i.Db, &models.Star{ 124 - StarredByDid: did, 125 - RepoAt: subjectUri, 126 - Rkey: e.Commit.RKey, 127 }) 128 case jmodels.CommitOperationDelete: 129 err = db.DeleteStarByRkey(i.Db, did, e.Commit.RKey)

··· 121 return err 122 } 123 err = db.AddStar(i.Db, &models.Star{ 124 + Did: did, 125 + RepoAt: subjectUri, 126 + Rkey: e.Commit.RKey, 127 }) 128 case jmodels.CommitOperationDelete: 129 err = db.DeleteStarByRkey(i.Db, did, e.Commit.RKey)

+4 -4

appview/models/star.go

··· 7 ) 8 9 type Star struct { 10 - StarredByDid string 11 - RepoAt syntax.ATURI 12 - Created time.Time 13 - Rkey string 14 15 // optionally, populate this when querying for reverse mappings 16 Repo *Repo

··· 7 ) 8 9 type Star struct { 10 + Did string 11 + RepoAt syntax.ATURI 12 + Created time.Time 13 + Rkey string 14 15 // optionally, populate this when querying for reverse mappings 16 Repo *Repo

+6 -1

appview/notify/db/db.go

··· 7 "slices" 8 9 "github.com/bluesky-social/indigo/atproto/syntax" 10 "tangled.org/core/appview/db" 11 "tangled.org/core/appview/models" 12 "tangled.org/core/appview/notify" ··· 36 } 37 38 func (n *databaseNotifier) NewStar(ctx context.Context, star *models.Star) { 39 var err error 40 repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(star.RepoAt))) 41 if err != nil { ··· 43 return 44 } 45 46 - actorDid := syntax.DID(star.StarredByDid) 47 recipients := []syntax.DID{syntax.DID(repo.Did)} 48 eventType := models.NotificationTypeRepoStarred 49 entityType := "repo"

··· 7 "slices" 8 9 "github.com/bluesky-social/indigo/atproto/syntax" 10 + "tangled.org/core/api/tangled" 11 "tangled.org/core/appview/db" 12 "tangled.org/core/appview/models" 13 "tangled.org/core/appview/notify" ··· 37 } 38 39 func (n *databaseNotifier) NewStar(ctx context.Context, star *models.Star) { 40 + if star.RepoAt.Collection().String() != tangled.RepoNSID { 41 + // skip string stars for now 42 + return 43 + } 44 var err error 45 repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(star.RepoAt))) 46 if err != nil { ··· 48 return 49 } 50 51 + actorDid := syntax.DID(star.Did) 52 recipients := []syntax.DID{syntax.DID(repo.Did)} 53 eventType := models.NotificationTypeRepoStarred 54 entityType := "repo"

+2 -2

appview/notify/posthog/notifier.go

··· 37 38 func (n *posthogNotifier) NewStar(ctx context.Context, star *models.Star) { 39 err := n.client.Enqueue(posthog.Capture{ 40 - DistinctId: star.StarredByDid, 41 Event: "star", 42 Properties: posthog.Properties{"repo_at": star.RepoAt.String()}, 43 }) ··· 48 49 func (n *posthogNotifier) DeleteStar(ctx context.Context, star *models.Star) { 50 err := n.client.Enqueue(posthog.Capture{ 51 - DistinctId: star.StarredByDid, 52 Event: "unstar", 53 Properties: posthog.Properties{"repo_at": star.RepoAt.String()}, 54 })

··· 37 38 func (n *posthogNotifier) NewStar(ctx context.Context, star *models.Star) { 39 err := n.client.Enqueue(posthog.Capture{ 40 + DistinctId: star.Did, 41 Event: "star", 42 Properties: posthog.Properties{"repo_at": star.RepoAt.String()}, 43 }) ··· 48 49 func (n *posthogNotifier) DeleteStar(ctx context.Context, star *models.Star) { 50 err := n.client.Enqueue(posthog.Capture{ 51 + DistinctId: star.Did, 52 Event: "unstar", 53 Properties: posthog.Properties{"repo_at": star.RepoAt.String()}, 54 })

+1 -1

appview/pages/templates/timeline/fragments/timeline.html

··· 61 {{ $event := index . 1 }} 62 {{ $star := $event.Star }} 63 {{ with $star }} 64 - {{ $starrerHandle := resolve .StarredByDid }} 65 {{ $repoOwnerHandle := resolve .Repo.Did }} 66 <div class="pl-6 py-2 bg-white dark:bg-gray-800 text-gray-600 dark:text-gray-300 flex flex-wrap items-center gap-2 text-sm"> 67 {{ template "user/fragments/picHandleLink" $starrerHandle }}

··· 61 {{ $event := index . 1 }} 62 {{ $star := $event.Star }} 63 {{ with $star }} 64 + {{ $starrerHandle := resolve .Did }} 65 {{ $repoOwnerHandle := resolve .Repo.Did }} 66 <div class="pl-6 py-2 bg-white dark:bg-gray-800 text-gray-600 dark:text-gray-300 flex flex-wrap items-center gap-2 text-sm"> 67 {{ template "user/fragments/picHandleLink" $starrerHandle }}

+2 -2

appview/state/profile.go

··· 66 return nil, fmt.Errorf("failed to get string count: %w", err) 67 } 68 69 - starredCount, err := db.CountStars(s.db, db.FilterEq("starred_by_did", did)) 70 if err != nil { 71 return nil, fmt.Errorf("failed to get starred repo count: %w", err) 72 } ··· 211 } 212 l = l.With("profileDid", profile.UserDid, "profileHandle", profile.UserHandle) 213 214 - stars, err := db.GetStars(s.db, 0, db.FilterEq("starred_by_did", profile.UserDid)) 215 if err != nil { 216 l.Error("failed to get stars", "err", err) 217 s.pages.Error500(w)

··· 66 return nil, fmt.Errorf("failed to get string count: %w", err) 67 } 68 69 + starredCount, err := db.CountStars(s.db, db.FilterEq("did", did)) 70 if err != nil { 71 return nil, fmt.Errorf("failed to get starred repo count: %w", err) 72 } ··· 211 } 212 l = l.With("profileDid", profile.UserDid, "profileHandle", profile.UserHandle) 213 214 + stars, err := db.GetStars(s.db, 0, db.FilterEq("did", profile.UserDid)) 215 if err != nil { 216 l.Error("failed to get stars", "err", err) 217 s.pages.Error500(w)

+3 -3

appview/state/star.go

··· 57 log.Println("created atproto record: ", resp.Uri) 58 59 star := &models.Star{ 60 - StarredByDid: currentUser.Did, 61 - RepoAt: subjectUri, 62 - Rkey: rkey, 63 } 64 65 err = db.AddStar(s.db, star)

··· 57 log.Println("created atproto record: ", resp.Uri) 58 59 star := &models.Star{ 60 + Did: currentUser.Did, 61 + RepoAt: subjectUri, 62 + Rkey: rkey, 63 } 64 65 err = db.AddStar(s.db, star)

History

5 rounds 3 comments

boltless.me submitted #4 3mo

diff interdiff

1 commit

expand

17e92c3c

appview/db: split star subjects

3/3 success

expand

expand 0 comments

pull request successfully merged

boltless.me submitted #3 3mo

diff interdiff

1 commit

expand

17e92c3c

appview/db: split star subjects

3/3 success

expand

expand 0 comments

boltless.me submitted #2 3mo

diff interdiff

1 commit

expand

c6167de1

appview/db: split star subjects

3/3 success

expand

expand 0 comments

boltless.me submitted #1 3mo

diff interdiff

1 commit

expand

10e3b844

appview/db: split star subjects

3/3 success

expand

expand 0 comments

boltless.me submitted #0 3mo

1 commit

expand

fb8b603e

appview/db: split star subjects

1/3 failed, 2/3 success

expand

expand 3 comments

oppi.li 3mo

bit confusing that the repo_at col of the stars table can now refer to anything. can we rename that column too? to something more generic like target?

boltless.me 3mo

Actually that was my first intention, renaming repo_at to subject_at. I just didn't do that here because I was afraid to find all .repo_at usages in raw sql queries. And I thought that will also harder to review without tests.

Do you think it is worth renaming? If so, I can do that.

oppi.li 3mo

IIUC we'd only need to find repo_at for all queries that mention the "stars" table. searching for from stars might cover that. I'd recommend that we do fix this!