appview/db: split star subjects #824

+118 -139

12 changed files

Diff round #4

expand all

unified split

appview

db.go

repos.go

star.go

timeline.go

ingester.go

models

star.go

timeline.go

notify

db.go

posthog

notifier.go

pages

templates

timeline

fragments

timeline.html

state

profile.go

star.go

+39 -2

appview/db/db.go

··· 569 569 -- indexes for better performance 570 570 create index if not exists idx_notifications_recipient_created on notifications(recipient_did, created desc); 571 571 create index if not exists idx_notifications_recipient_read on notifications(recipient_did, read); 572 - create index if not exists idx_stars_created on stars(created); 573 - create index if not exists idx_stars_repo_at_created on stars(repo_at, created); 574 572 `) 575 573 if err != nil { 576 574 return nil, err ··· 1128 1126 return err 1129 1127 }) 1130 1128 1129 + // remove the foreign key constraints from stars. 1130 + runMigration(conn, logger, "generalize-stars-subject", func(tx *sql.Tx) error { 1131 + _, err := tx.Exec(` 1132 + create table stars_new ( 1133 + id integer primary key autoincrement, 1134 + did text not null, 1135 + rkey text not null, 1136 + 1137 + subject_at text not null, 1138 + 1139 + created text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), 1140 + unique(did, rkey), 1141 + unique(did, subject_at) 1142 + ); 1143 + 1144 + insert into stars_new ( 1145 + id, 1146 + did, 1147 + rkey, 1148 + subject_at, 1149 + created 1150 + ) 1151 + select 1152 + id, 1153 + starred_by_did, 1154 + rkey, 1155 + repo_at, 1156 + created 1157 + from stars; 1158 + 1159 + drop table stars; 1160 + alter table stars_new rename to stars; 1161 + 1162 + create index if not exists idx_stars_created on stars(created); 1163 + create index if not exists idx_stars_subject_at_created on stars(subject_at, created); 1164 + `) 1165 + return err 1166 + }) 1167 + 1131 1168 return &DB{ 1132 1169 db, 1133 1170 logger,

+3 -3

appview/db/repos.go

··· 208 208 209 209 starCountQuery := fmt.Sprintf( 210 210 `select 211 - repo_at, count(1) 211 + subject_at, count(1) 212 212 from stars 213 - where repo_at in (%s) 214 - group by repo_at`, 213 + where subject_at in (%s) 214 + group by subject_at`, 215 215 inClause, 216 216 ) 217 217 rows, err = e.Query(starCountQuery, args...)

+39 -99

appview/db/star.go

··· 14 14 ) 15 15 16 16 func AddStar(e Execer, star *models.Star) error { 17 - query := `insert or ignore into stars (starred_by_did, repo_at, rkey) values (?, ?, ?)` 17 + query := `insert or ignore into stars (did, subject_at, rkey) values (?, ?, ?)` 18 18 _, err := e.Exec( 19 19 query, 20 - star.StarredByDid, 20 + star.Did, 21 21 star.RepoAt.String(), 22 22 star.Rkey, 23 23 ) ··· 25 25 } 26 26 27 27 // Get a star record 28 - func GetStar(e Execer, starredByDid string, repoAt syntax.ATURI) (*models.Star, error) { 28 + func GetStar(e Execer, did string, subjectAt syntax.ATURI) (*models.Star, error) { 29 29 query := ` 30 - select starred_by_did, repo_at, created, rkey 30 + select did, subject_at, created, rkey 31 31 from stars 32 - where starred_by_did = ? and repo_at = ?` 33 - row := e.QueryRow(query, starredByDid, repoAt) 32 + where did = ? and subject_at = ?` 33 + row := e.QueryRow(query, did, subjectAt) 34 34 35 35 var star models.Star 36 36 var created string 37 - err := row.Scan(&star.StarredByDid, &star.RepoAt, &created, &star.Rkey) 37 + err := row.Scan(&star.Did, &star.RepoAt, &created, &star.Rkey) 38 38 if err != nil { 39 39 return nil, err 40 40 } ··· 51 51 } 52 52 53 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) 54 + func DeleteStar(e Execer, did string, subjectAt syntax.ATURI) error { 55 + _, err := e.Exec(`delete from stars where did = ? and subject_at = ?`, did, subjectAt) 56 56 return err 57 57 } 58 58 59 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) 60 + func DeleteStarByRkey(e Execer, did string, rkey string) error { 61 + _, err := e.Exec(`delete from stars where did = ? and rkey = ?`, did, rkey) 62 62 return err 63 63 } 64 64 65 - func GetStarCount(e Execer, repoAt syntax.ATURI) (int, error) { 65 + func GetStarCount(e Execer, subjectAt syntax.ATURI) (int, error) { 66 66 stars := 0 67 67 err := e.QueryRow( 68 - `select count(starred_by_did) from stars where repo_at = ?`, repoAt).Scan(&stars) 68 + `select count(did) from stars where subject_at = ?`, subjectAt).Scan(&stars) 69 69 if err != nil { 70 70 return 0, err 71 71 } ··· 89 89 } 90 90 91 91 query := fmt.Sprintf(` 92 - SELECT repo_at 92 + SELECT subject_at 93 93 FROM stars 94 - WHERE starred_by_did = ? AND repo_at IN (%s) 94 + WHERE did = ? AND subject_at IN (%s) 95 95 `, strings.Join(placeholders, ",")) 96 96 97 97 rows, err := e.Query(query, args...) ··· 118 118 return result, nil 119 119 } 120 120 121 - func GetStarStatus(e Execer, userDid string, repoAt syntax.ATURI) bool { 122 - statuses, err := getStarStatuses(e, userDid, []syntax.ATURI{repoAt}) 121 + func GetStarStatus(e Execer, userDid string, subjectAt syntax.ATURI) bool { 122 + statuses, err := getStarStatuses(e, userDid, []syntax.ATURI{subjectAt}) 123 123 if err != nil { 124 124 return false 125 125 } 126 - return statuses[repoAt.String()] 126 + return statuses[subjectAt.String()] 127 127 } 128 128 129 129 // GetStarStatuses returns a map of repo URIs to star status for a given user 130 - func GetStarStatuses(e Execer, userDid string, repoAts []syntax.ATURI) (map[string]bool, error) { 131 - return getStarStatuses(e, userDid, repoAts) 130 + func GetStarStatuses(e Execer, userDid string, subjectAts []syntax.ATURI) (map[string]bool, error) { 131 + return getStarStatuses(e, userDid, subjectAts) 132 132 } 133 - func GetStars(e Execer, limit int, filters ...filter) ([]models.Star, error) { 133 + 134 + // GetRepoStars return a list of stars each holding target repository. 135 + // If there isn't known repo with starred at-uri, those stars will be ignored. 136 + func GetRepoStars(e Execer, limit int, filters ...filter) ([]models.RepoStar, error) { 134 137 var conditions []string 135 138 var args []any 136 139 for _, filter := range filters { ··· 149 152 } 150 153 151 154 repoQuery := fmt.Sprintf( 152 - `select starred_by_did, repo_at, created, rkey 155 + `select did, subject_at, created, rkey 153 156 from stars 154 157 %s 155 158 order by created desc ··· 166 169 for rows.Next() { 167 170 var star models.Star 168 171 var created string 169 - err := rows.Scan(&star.StarredByDid, &star.RepoAt, &created, &star.Rkey) 172 + err := rows.Scan(&star.Did, &star.RepoAt, &created, &star.Rkey) 170 173 if err != nil { 171 174 return nil, err 172 175 } ··· 197 200 return nil, err 198 201 } 199 202 203 + var repoStars []models.RepoStar 200 204 for _, r := range repos { 201 205 if stars, ok := starMap[string(r.RepoAt())]; ok { 202 - for i := range stars { 203 - stars[i].Repo = &r 206 + for _, star := range stars { 207 + repoStars = append(repoStars, models.RepoStar{ 208 + Star: star, 209 + Repo: &r, 210 + }) 204 211 } 205 212 } 206 213 } 207 214 208 - var stars []models.Star 209 - for _, s := range starMap { 210 - stars = append(stars, s...) 211 - } 212 - 213 - slices.SortFunc(stars, func(a, b models.Star) int { 215 + slices.SortFunc(repoStars, func(a, b models.RepoStar) int { 214 216 if a.Created.After(b.Created) { 215 217 return -1 216 218 } ··· 220 222 return 0 221 223 }) 222 224 223 - return stars, nil 225 + return repoStars, nil 224 226 } 225 227 226 228 func CountStars(e Execer, filters ...filter) (int64, error) { ··· 247 249 return count, nil 248 250 } 249 251 250 - func GetAllStars(e Execer, limit int) ([]models.Star, error) { 251 - var stars []models.Star 252 - 253 - rows, err := e.Query(` 254 - select 255 - s.starred_by_did, 256 - s.repo_at, 257 - s.rkey, 258 - s.created, 259 - r.did, 260 - r.name, 261 - r.knot, 262 - r.rkey, 263 - r.created 264 - from stars s 265 - join repos r on s.repo_at = r.at_uri 266 - `) 267 - 268 - if err != nil { 269 - return nil, err 270 - } 271 - defer rows.Close() 272 - 273 - for rows.Next() { 274 - var star models.Star 275 - var repo models.Repo 276 - var starCreatedAt, repoCreatedAt string 277 - 278 - if err := rows.Scan( 279 - &star.StarredByDid, 280 - &star.RepoAt, 281 - &star.Rkey, 282 - &starCreatedAt, 283 - &repo.Did, 284 - &repo.Name, 285 - &repo.Knot, 286 - &repo.Rkey, 287 - &repoCreatedAt, 288 - ); err != nil { 289 - return nil, err 290 - } 291 - 292 - star.Created, err = time.Parse(time.RFC3339, starCreatedAt) 293 - if err != nil { 294 - star.Created = time.Now() 295 - } 296 - repo.Created, err = time.Parse(time.RFC3339, repoCreatedAt) 297 - if err != nil { 298 - repo.Created = time.Now() 299 - } 300 - star.Repo = &repo 301 - 302 - stars = append(stars, star) 303 - } 304 - 305 - if err := rows.Err(); err != nil { 306 - return nil, err 307 - } 308 - 309 - return stars, nil 310 - } 311 - 312 252 // GetTopStarredReposLastWeek returns the top 8 most starred repositories from the last week 313 253 func GetTopStarredReposLastWeek(e Execer) ([]models.Repo, error) { 314 254 // first, get the top repo URIs by star count from the last week 315 255 query := ` 316 256 with recent_starred_repos as ( 317 - select distinct repo_at 257 + select distinct subject_at 318 258 from stars 319 259 where created >= datetime('now', '-7 days') 320 260 ), 321 261 repo_star_counts as ( 322 262 select 323 - s.repo_at, 263 + s.subject_at, 324 264 count(*) as stars_gained_last_week 325 265 from stars s 326 - join recent_starred_repos rsr on s.repo_at = rsr.repo_at 266 + join recent_starred_repos rsr on s.subject_at = rsr.subject_at 327 267 where s.created >= datetime('now', '-7 days') 328 - group by s.repo_at 268 + group by s.subject_at 329 269 ) 330 - select rsc.repo_at 270 + select rsc.subject_at 331 271 from repo_star_counts rsc 332 272 order by rsc.stars_gained_last_week desc 333 273 limit 8

+3 -13

appview/db/timeline.go

··· 146 146 func getTimelineStars(e Execer, limit int, loggedInUserDid string, userIsFollowing []string) ([]models.TimelineEvent, error) { 147 147 filters := make([]filter, 0) 148 148 if userIsFollowing != nil { 149 - filters = append(filters, FilterIn("starred_by_did", userIsFollowing)) 149 + filters = append(filters, FilterIn("did", userIsFollowing)) 150 150 } 151 151 152 - stars, err := GetStars(e, limit, filters...) 152 + stars, err := GetRepoStars(e, limit, filters...) 153 153 if err != nil { 154 154 return nil, err 155 155 } 156 156 157 - // filter star records without a repo 158 - n := 0 159 - for _, s := range stars { 160 - if s.Repo != nil { 161 - stars[n] = s 162 - n++ 163 - } 164 - } 165 - stars = stars[:n] 166 - 167 157 var repos []models.Repo 168 158 for _, s := range stars { 169 159 repos = append(repos, *s.Repo) ··· 179 169 isStarred, starCount := getRepoStarInfo(s.Repo, starStatuses) 180 170 181 171 events = append(events, models.TimelineEvent{ 182 - Star: &s, 172 + RepoStar: &s, 183 173 EventAt: s.Created, 184 174 IsStarred: isStarred, 185 175 StarCount: starCount,

+3 -3

appview/ingester.go

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

+14 -5

appview/models/star.go

··· 7 7 ) 8 8 9 9 type Star struct { 10 - StarredByDid string 11 - RepoAt syntax.ATURI 12 - Created time.Time 13 - Rkey string 10 + Did string 11 + RepoAt syntax.ATURI 12 + Created time.Time 13 + Rkey string 14 + } 14 15 15 - // optionally, populate this when querying for reverse mappings 16 + // RepoStar is used for reverse mapping to repos 17 + type RepoStar struct { 18 + Star 16 19 Repo *Repo 17 20 } 21 + 22 + // StringStar is used for reverse mapping to strings 23 + type StringStar struct { 24 + Star 25 + String *String 26 + }

+1 -1

appview/models/timeline.go

··· 5 5 type TimelineEvent struct { 6 6 *Repo 7 7 *Follow 8 - *Star 8 + *RepoStar 9 9 10 10 EventAt time.Time 11 11

+6 -1

appview/notify/db/db.go

··· 7 7 "slices" 8 8 9 9 "github.com/bluesky-social/indigo/atproto/syntax" 10 + "tangled.org/core/api/tangled" 10 11 "tangled.org/core/appview/db" 11 12 "tangled.org/core/appview/models" 12 13 "tangled.org/core/appview/notify" ··· 36 37 } 37 38 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 + } 39 44 var err error 40 45 repo, err := db.GetRepo(n.db, db.FilterEq("at_uri", string(star.RepoAt))) 41 46 if err != nil { ··· 43 48 return 44 49 } 45 50 46 - actorDid := syntax.DID(star.StarredByDid) 51 + actorDid := syntax.DID(star.Did) 47 52 recipients := []syntax.DID{syntax.DID(repo.Did)} 48 53 eventType := models.NotificationTypeRepoStarred 49 54 entityType := "repo"

+2 -2

appview/notify/posthog/notifier.go

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

+2 -2

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

··· 59 59 {{ define "timeline/fragments/starEvent" }} 60 60 {{ $root := index . 0 }} 61 61 {{ $event := index . 1 }} 62 - {{ $star := $event.Star }} 62 + {{ $star := $event.RepoStar }} 63 63 {{ with $star }} 64 - {{ $starrerHandle := resolve .StarredByDid }} 64 + {{ $starrerHandle := resolve .Did }} 65 65 {{ $repoOwnerHandle := resolve .Repo.Did }} 66 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 67 {{ template "user/fragments/picHandleLink" $starrerHandle }}

+3 -5

appview/state/profile.go

··· 66 66 return nil, fmt.Errorf("failed to get string count: %w", err) 67 67 } 68 68 69 - starredCount, err := db.CountStars(s.db, db.FilterEq("starred_by_did", did)) 69 + starredCount, err := db.CountStars(s.db, db.FilterEq("did", did)) 70 70 if err != nil { 71 71 return nil, fmt.Errorf("failed to get starred repo count: %w", err) 72 72 } ··· 211 211 } 212 212 l = l.With("profileDid", profile.UserDid, "profileHandle", profile.UserHandle) 213 213 214 - stars, err := db.GetStars(s.db, 0, db.FilterEq("starred_by_did", profile.UserDid)) 214 + stars, err := db.GetRepoStars(s.db, 0, db.FilterEq("did", profile.UserDid)) 215 215 if err != nil { 216 216 l.Error("failed to get stars", "err", err) 217 217 s.pages.Error500(w) ··· 219 219 } 220 220 var repos []models.Repo 221 221 for _, s := range stars { 222 - if s.Repo != nil { 223 - repos = append(repos, *s.Repo) 224 - } 222 + repos = append(repos, *s.Repo) 225 223 } 226 224 227 225 err = s.pages.ProfileStarred(w, pages.ProfileStarredParams{

+3 -3

appview/state/star.go

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

History

5 rounds 3 comments

boltless.me submitted #4 3mo

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

diff

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!