appview/db: split star subjects #824

+56 -121

12 changed files

Interdiff #0 → #1

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

+4 -6

appview/db/db.go

··· 1129 1129 }) 1130 1130 1131 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 1132 runMigration(conn, logger, "generalize-stars-subject", func(tx *sql.Tx) error { 1135 1133 _, err := tx.Exec(` 1136 1134 create table stars_new ( ··· 1138 1136 did text not null, 1139 1137 rkey text not null, 1140 1138 1141 - repo_at text not null, 1139 + subject_at text not null, 1142 1140 1143 1141 created text not null default (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')), 1144 1142 unique(did, rkey), 1145 - unique(did, repo_at) 1143 + unique(did, subject_at) 1146 1144 ); 1147 1145 1148 1146 insert into stars_new ( 1149 1147 id, 1150 1148 did, 1151 1149 rkey, 1152 - repo_at, 1150 + subject_at, 1153 1151 created 1154 1152 ) 1155 1153 select ··· 1164 1162 alter table stars_new rename to stars; 1165 1163 1166 1164 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); 1165 + create index if not exists idx_stars_subject_at_created on stars(subject_at, created); 1168 1166 `) 1169 1167 return err 1170 1168 })

+34 -94

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 (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 20 star.Did, ··· 25 25 } 26 26 27 27 // Get a star record 28 - func GetStar(e Execer, did 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 did, repo_at, created, rkey 30 + select did, subject_at, created, rkey 31 31 from stars 32 - where did = ? and repo_at = ?` 33 - row := e.QueryRow(query, did, 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 ··· 51 51 } 52 52 53 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) 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 ··· 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(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 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 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 ··· 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.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.Did, 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

+2 -12

appview/db/timeline.go

··· 149 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,

appview/ingester.go

This file has not been changed.

+10 -1

appview/models/star.go

··· 11 11 RepoAt syntax.ATURI 12 12 Created time.Time 13 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 + }

appview/notify/db/db.go

This file has not been changed.

appview/notify/posthog/notifier.go

This file has not been changed.

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

This file has not been changed.

+2 -4

appview/state/profile.go

··· 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("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{

appview/state/star.go

This file has not been changed.

+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...)

+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

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

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!