refactor(app): share keyset paging and repository lookup, cap commit page skips
pageAfter replaces three copies of the keyset loop (refs, repositories, tokens), pageSize and decodeToken move to paging.go, lookupRepo is the single metadata read of Get, Update, Delete and Browse.authorize. A commit page token may skip at most 100,000 commits. Update writes the metadata before moving HEAD and publishes once, after both.
- commit
- 1e629682a5479c018bc675065224b47b1d1fad76
- author
- brice · · 2026-10-10 10:45 UTC
- parent
- 1b88622
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +95 -83
M internal/app/browse.go +9 -48
| @@ -17,14 +17,6 @@ import ( | |||
| 17 | 17 | "bnema.dev/ermite/internal/ports" | |
| 18 | 18 | ) | |
| 19 | 19 | ||
| 20 | - | // Page sizes of the listings. | |
| 21 | - | const ( | |
| 22 | - | // DefaultPageSize is the page size when the caller gives none. | |
| 23 | - | DefaultPageSize = 50 | |
| 24 | - | // MaxPageSize is the largest page; a larger request is lowered to it. | |
| 25 | - | MaxPageSize = 100 | |
| 26 | - | ) | |
| 27 | - | ||
| 28 | 20 | // Browse are the read use cases over the Git data of a repository: refs, | |
| 29 | 21 | // trees, blobs, history and commits. Every method applies the access rule | |
| 30 | 22 | // first: a repository the principal may not read, or that does not exist, | |
| @@ -41,10 +33,7 @@ func NewBrowse(meta ports.RepoMetaStore, reader ports.RepoReader) *Browse { | |||
| 41 | 33 | ||
| 42 | 34 | // authorize returns nil if p may read the repository. | |
| 43 | 35 | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) error { | |
| 44 | - | if name.IsZero() { | |
| 45 | - | return fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |
| 46 | - | } | |
| 47 | - | repo, err := b.meta.Get(ctx, name) | |
| 36 | + | repo, err := lookupRepo(ctx, b.meta, name) | |
| 48 | 37 | if err != nil { | |
| 49 | 38 | return err | |
| 50 | 39 | } | |
| @@ -54,17 +43,6 @@ func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain. | |||
| 54 | 43 | return nil | |
| 55 | 44 | } | |
| 56 | 45 | ||
| 57 | - | // pageSize applies the default and the maximum. | |
| 58 | - | func pageSize(n int) (int, error) { | |
| 59 | - | switch { | |
| 60 | - | case n < 0: | |
| 61 | - | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | |
| 62 | - | case n == 0: | |
| 63 | - | return DefaultPageSize, nil | |
| 64 | - | } | |
| 65 | - | return min(n, MaxPageSize), nil | |
| 66 | - | } | |
| 67 | - | ||
| 68 | 46 | // ListRefsParams selects the refs to list. | |
| 69 | 47 | type ListRefsParams struct { | |
| 70 | 48 | // Kinds restricts the result; empty means every kind. | |
| @@ -117,21 +95,16 @@ func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.R | |||
| 117 | 95 | return RefPage{}, err | |
| 118 | 96 | } | |
| 119 | 97 | ||
| 120 | - | var page RefPage | |
| 98 | + | var wanted []domain.Ref | |
| 121 | 99 | for _, r := range all { | |
| 122 | - | if len(params.Kinds) > 0 && !slices.Contains(params.Kinds, r.Kind) { | |
| 123 | - | continue | |
| 100 | + | if len(params.Kinds) == 0 || slices.Contains(params.Kinds, r.Kind) { | |
| 101 | + | wanted = append(wanted, r) | |
| 124 | 102 | } | |
| 125 | - | if after != nil && !refAfter(r, *after) { | |
| 126 | - | continue | |
| 127 | - | } | |
| 128 | - | if len(page.Refs) == size { | |
| 129 | - | last := page.Refs[len(page.Refs)-1] | |
| 130 | - | page.NextPageToken = encodeRefToken(last) | |
| 131 | - | break | |
| 132 | - | } | |
| 133 | - | page.Refs = append(page.Refs, r) | |
| 134 | 103 | } | |
| 104 | + | var page RefPage | |
| 105 | + | page.Refs, page.NextPageToken = pageAfter(wanted, size, | |
| 106 | + | func(r domain.Ref) bool { return after == nil || refAfter(r, *after) }, | |
| 107 | + | encodeRefToken) | |
| 135 | 108 | return page, nil | |
| 136 | 109 | } | |
| 137 | 110 | ||
| @@ -152,7 +125,6 @@ type refCursor struct { | |||
| 152 | 125 | const ( | |
| 153 | 126 | refTokenPrefix = "r1:" | |
| 154 | 127 | commitTokenPrefix = "c1:" | |
| 155 | - | maxTokenLen = 1024 | |
| 156 | 128 | ) | |
| 157 | 129 | ||
| 158 | 130 | func encodeRefToken(last domain.Ref) string { | |
| @@ -176,17 +148,6 @@ func decodeRefToken(tok string) (*refCursor, error) { | |||
| 176 | 148 | return &refCursor{kind: k, name: name}, nil | |
| 177 | 149 | } | |
| 178 | 150 | ||
| 179 | - | func decodeToken(tok, prefix string) (string, bool) { | |
| 180 | - | if len(tok) > maxTokenLen { | |
| 181 | - | return "", false | |
| 182 | - | } | |
| 183 | - | raw, err := base64.RawURLEncoding.DecodeString(tok) | |
| 184 | - | if err != nil { | |
| 185 | - | return "", false | |
| 186 | - | } | |
| 187 | - | return strings.CutPrefix(string(raw), prefix) | |
| 188 | - | } | |
| 189 | - | ||
| 190 | 151 | // ResolveRevision turns a revision into the commit it designates. | |
| 191 | 152 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | |
| 192 | 153 | if err := b.authorize(ctx, p, repo); err != nil { | |
| @@ -296,7 +257,7 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int, | |||
| 296 | 257 | } | |
| 297 | 258 | head, err := domain.ParseObjectID(parts[0]) | |
| 298 | 259 | skip, err2 := strconv.Atoi(parts[1]) | |
| 299 | - | if err != nil || err2 != nil || skip < 0 || parts[2] != pathDigest(path) { | |
| 260 | + | if err != nil || err2 != nil || skip < 0 || skip > MaxCommitSkip || parts[2] != pathDigest(path) { | |
| 300 | 261 | return domain.ObjectID{}, 0, bad | |
| 301 | 262 | } | |
| 302 | 263 | return head, skip, nil | |
| @@ -17,14 +17,6 @@ import ( | |||||
| 17 | "bnema.dev/ermite/internal/ports" | 17 | "bnema.dev/ermite/internal/ports" | ||
| 18 | ) | 18 | ) | ||
| 19 | 19 | ||||
| 20 | - | // Page sizes of the listings. | |||
| 21 | - | const ( | |||
| 22 | - | // DefaultPageSize is the page size when the caller gives none. | |||
| 23 | - | DefaultPageSize = 50 | |||
| 24 | - | // MaxPageSize is the largest page; a larger request is lowered to it. | |||
| 25 | - | MaxPageSize = 100 | |||
| 26 | - | ) | |||
| 27 | - | ||||
| 28 | // Browse are the read use cases over the Git data of a repository: refs, | 20 | // Browse are the read use cases over the Git data of a repository: refs, | ||
| 29 | // trees, blobs, history and commits. Every method applies the access rule | 21 | // trees, blobs, history and commits. Every method applies the access rule | ||
| 30 | // first: a repository the principal may not read, or that does not exist, | 22 | // first: a repository the principal may not read, or that does not exist, | ||
| @@ -41,10 +33,7 @@ func NewBrowse(meta ports.RepoMetaStore, reader ports.RepoReader) *Browse { | |||||
| 41 | 33 | ||||
| 42 | // authorize returns nil if p may read the repository. | 34 | // authorize returns nil if p may read the repository. | ||
| 43 | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) error { | 35 | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) error { | ||
| 44 | - | if name.IsZero() { | 36 | + | repo, err := lookupRepo(ctx, b.meta, name) |
| 45 | - | return fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |||
| 46 | - | } | |||
| 47 | - | repo, err := b.meta.Get(ctx, name) | |||
| 48 | if err != nil { | 37 | if err != nil { | ||
| 49 | return err | 38 | return err | ||
| 50 | } | 39 | } | ||
| @@ -54,17 +43,6 @@ func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain. | |||||
| 54 | return nil | 43 | return nil | ||
| 55 | } | 44 | } | ||
| 56 | 45 | ||||
| 57 | - | // pageSize applies the default and the maximum. | |||
| 58 | - | func pageSize(n int) (int, error) { | |||
| 59 | - | switch { | |||
| 60 | - | case n < 0: | |||
| 61 | - | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | |||
| 62 | - | case n == 0: | |||
| 63 | - | return DefaultPageSize, nil | |||
| 64 | - | } | |||
| 65 | - | return min(n, MaxPageSize), nil | |||
| 66 | - | } | |||
| 67 | - | ||||
| 68 | // ListRefsParams selects the refs to list. | 46 | // ListRefsParams selects the refs to list. | ||
| 69 | type ListRefsParams struct { | 47 | type ListRefsParams struct { | ||
| 70 | // Kinds restricts the result; empty means every kind. | 48 | // Kinds restricts the result; empty means every kind. | ||
| @@ -117,21 +95,16 @@ func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.R | |||||
| 117 | return RefPage{}, err | 95 | return RefPage{}, err | ||
| 118 | } | 96 | } | ||
| 119 | 97 | ||||
| 120 | - | var page RefPage | 98 | + | var wanted []domain.Ref |
| 121 | for _, r := range all { | 99 | for _, r := range all { | ||
| 122 | - | if len(params.Kinds) > 0 && !slices.Contains(params.Kinds, r.Kind) { | 100 | + | if len(params.Kinds) == 0 || slices.Contains(params.Kinds, r.Kind) { |
| 123 | - | continue | 101 | + | wanted = append(wanted, r) |
| 124 | } | 102 | } | ||
| 125 | - | if after != nil && !refAfter(r, *after) { | |||
| 126 | - | continue | |||
| 127 | - | } | |||
| 128 | - | if len(page.Refs) == size { | |||
| 129 | - | last := page.Refs[len(page.Refs)-1] | |||
| 130 | - | page.NextPageToken = encodeRefToken(last) | |||
| 131 | - | break | |||
| 132 | - | } | |||
| 133 | - | page.Refs = append(page.Refs, r) | |||
| 134 | } | 103 | } | ||
| 104 | + | var page RefPage | |||
| 105 | + | page.Refs, page.NextPageToken = pageAfter(wanted, size, | |||
| 106 | + | func(r domain.Ref) bool { return after == nil || refAfter(r, *after) }, | |||
| 107 | + | encodeRefToken) | |||
| 135 | return page, nil | 108 | return page, nil | ||
| 136 | } | 109 | } | ||
| 137 | 110 | ||||
| @@ -152,7 +125,6 @@ type refCursor struct { | |||||
| 152 | const ( | 125 | const ( | ||
| 153 | refTokenPrefix = "r1:" | 126 | refTokenPrefix = "r1:" | ||
| 154 | commitTokenPrefix = "c1:" | 127 | commitTokenPrefix = "c1:" | ||
| 155 | - | maxTokenLen = 1024 | |||
| 156 | ) | 128 | ) | ||
| 157 | 129 | ||||
| 158 | func encodeRefToken(last domain.Ref) string { | 130 | func encodeRefToken(last domain.Ref) string { | ||
| @@ -176,17 +148,6 @@ func decodeRefToken(tok string) (*refCursor, error) { | |||||
| 176 | return &refCursor{kind: k, name: name}, nil | 148 | return &refCursor{kind: k, name: name}, nil | ||
| 177 | } | 149 | } | ||
| 178 | 150 | ||||
| 179 | - | func decodeToken(tok, prefix string) (string, bool) { | |||
| 180 | - | if len(tok) > maxTokenLen { | |||
| 181 | - | return "", false | |||
| 182 | - | } | |||
| 183 | - | raw, err := base64.RawURLEncoding.DecodeString(tok) | |||
| 184 | - | if err != nil { | |||
| 185 | - | return "", false | |||
| 186 | - | } | |||
| 187 | - | return strings.CutPrefix(string(raw), prefix) | |||
| 188 | - | } | |||
| 189 | - | ||||
| 190 | // ResolveRevision turns a revision into the commit it designates. | 151 | // ResolveRevision turns a revision into the commit it designates. | ||
| 191 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | 152 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | ||
| 192 | if err := b.authorize(ctx, p, repo); err != nil { | 153 | if err := b.authorize(ctx, p, repo); err != nil { | ||
| @@ -296,7 +257,7 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int, | |||||
| 296 | } | 257 | } | ||
| 297 | head, err := domain.ParseObjectID(parts[0]) | 258 | head, err := domain.ParseObjectID(parts[0]) | ||
| 298 | skip, err2 := strconv.Atoi(parts[1]) | 259 | skip, err2 := strconv.Atoi(parts[1]) | ||
| 299 | - | if err != nil || err2 != nil || skip < 0 || parts[2] != pathDigest(path) { | 260 | + | if err != nil || err2 != nil || skip < 0 || skip > MaxCommitSkip || parts[2] != pathDigest(path) { |
| 300 | return domain.ObjectID{}, 0, bad | 261 | return domain.ObjectID{}, 0, bad | ||
| 301 | } | 262 | } | ||
| 302 | return head, skip, nil | 263 | return head, skip, nil | ||
M internal/app/paging.go +60 -20
| @@ -16,8 +16,62 @@ import ( | |||
| 16 | 16 | const ( | |
| 17 | 17 | repoTokenPrefix = "rp1:" | |
| 18 | 18 | tokenTokenPrefix = "tp1:" | |
| 19 | + | ||
| 20 | + | // maxTokenLen bounds a page token before it is decoded. | |
| 21 | + | maxTokenLen = 1024 | |
| 22 | + | ) | |
| 23 | + | ||
| 24 | + | // Page sizes of the listings. | |
| 25 | + | const ( | |
| 26 | + | // DefaultPageSize is the page size when the caller gives none. | |
| 27 | + | DefaultPageSize = 50 | |
| 28 | + | // MaxPageSize is the largest page; a larger request is lowered to it. | |
| 29 | + | MaxPageSize = 100 | |
| 19 | 30 | ) | |
| 20 | 31 | ||
| 32 | + | // MaxCommitSkip bounds how deep into a history a page token may point: each | |
| 33 | + | // page walks the history from its start with `git log --skip`. | |
| 34 | + | const MaxCommitSkip = 100_000 | |
| 35 | + | ||
| 36 | + | // pageSize applies the default and the maximum. | |
| 37 | + | func pageSize(n int) (int, error) { | |
| 38 | + | switch { | |
| 39 | + | case n < 0: | |
| 40 | + | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | |
| 41 | + | case n == 0: | |
| 42 | + | return DefaultPageSize, nil | |
| 43 | + | } | |
| 44 | + | return min(n, MaxPageSize), nil | |
| 45 | + | } | |
| 46 | + | ||
| 47 | + | func decodeToken(tok, prefix string) (string, bool) { | |
| 48 | + | if len(tok) > maxTokenLen { | |
| 49 | + | return "", false | |
| 50 | + | } | |
| 51 | + | raw, err := base64.RawURLEncoding.DecodeString(tok) | |
| 52 | + | if err != nil { | |
| 53 | + | return "", false | |
| 54 | + | } | |
| 55 | + | return strings.CutPrefix(string(raw), prefix) | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | // pageAfter returns the first size items of the ordered items that satisfy | |
| 59 | + | // after (nil keeps all of them) and, when more follow, the token of the last | |
| 60 | + | // one returned. Tokens name the last item, so items that appear or disappear | |
| 61 | + | // between two pages never shift the others. | |
| 62 | + | func pageAfter[T any](items []T, size int, after func(T) bool, token func(T) string) (page []T, next string) { | |
| 63 | + | for _, it := range items { | |
| 64 | + | if after != nil && !after(it) { | |
| 65 | + | continue | |
| 66 | + | } | |
| 67 | + | if len(page) == size { | |
| 68 | + | return page, token(page[size-1]) | |
| 69 | + | } | |
| 70 | + | page = append(page, it) | |
| 71 | + | } | |
| 72 | + | return page, "" | |
| 73 | + | } | |
| 74 | + | ||
| 21 | 75 | // RepoPage is a page of repositories ordered by name. | |
| 22 | 76 | type RepoPage struct { | |
| 23 | 77 | Repos []domain.Repo | |
| @@ -43,16 +97,9 @@ func (r *Repos) ListPage(ctx context.Context, p domain.Principal, size int, toke | |||
| 43 | 97 | return RepoPage{}, err | |
| 44 | 98 | } | |
| 45 | 99 | var page RepoPage | |
| 46 | - | for _, repo := range all { | |
| 47 | - | if after != "" && repo.Name.String() <= after { | |
| 48 | - | continue | |
| 49 | - | } | |
| 50 | - | if len(page.Repos) == n { | |
| 51 | - | page.NextPageToken = encodeRepoToken(page.Repos[n-1].Name) | |
| 52 | - | break | |
| 53 | - | } | |
| 54 | - | page.Repos = append(page.Repos, repo) | |
| 55 | - | } | |
| 100 | + | page.Repos, page.NextPageToken = pageAfter(all, n, | |
| 101 | + | func(repo domain.Repo) bool { return after == "" || repo.Name.String() > after }, | |
| 102 | + | func(repo domain.Repo) string { return encodeRepoToken(repo.Name) }) | |
| 56 | 103 | return page, nil | |
| 57 | 104 | } | |
| 58 | 105 | ||
| @@ -102,16 +149,9 @@ func (t *Tokens) ListPage(ctx context.Context, p domain.Principal, size int, tok | |||
| 102 | 149 | return strings.Compare(b.ID, a.ID) | |
| 103 | 150 | }) | |
| 104 | 151 | var page TokenPage | |
| 105 | - | for _, tok := range all { | |
| 106 | - | if after != nil && !tokenAfter(tok, *after) { | |
| 107 | - | continue | |
| 108 | - | } | |
| 109 | - | if len(page.Tokens) == n { | |
| 110 | - | page.NextPageToken = encodeTokenCursor(page.Tokens[n-1]) | |
| 111 | - | break | |
| 112 | - | } | |
| 113 | - | page.Tokens = append(page.Tokens, tok) | |
| 114 | - | } | |
| 152 | + | page.Tokens, page.NextPageToken = pageAfter(all, n, | |
| 153 | + | func(tok domain.Token) bool { return after == nil || tokenAfter(tok, *after) }, | |
| 154 | + | encodeTokenCursor) | |
| 115 | 155 | return page, nil | |
| 116 | 156 | } | |
| 117 | 157 | ||
| @@ -16,8 +16,62 @@ import ( | |||||
| 16 | const ( | 16 | const ( | ||
| 17 | repoTokenPrefix = "rp1:" | 17 | repoTokenPrefix = "rp1:" | ||
| 18 | tokenTokenPrefix = "tp1:" | 18 | tokenTokenPrefix = "tp1:" | ||
| 19 | + | ||||
| 20 | + | // maxTokenLen bounds a page token before it is decoded. | |||
| 21 | + | maxTokenLen = 1024 | |||
| 22 | + | ) | |||
| 23 | + | ||||
| 24 | + | // Page sizes of the listings. | |||
| 25 | + | const ( | |||
| 26 | + | // DefaultPageSize is the page size when the caller gives none. | |||
| 27 | + | DefaultPageSize = 50 | |||
| 28 | + | // MaxPageSize is the largest page; a larger request is lowered to it. | |||
| 29 | + | MaxPageSize = 100 | |||
| 19 | ) | 30 | ) | ||
| 20 | 31 | ||||
| 32 | + | // MaxCommitSkip bounds how deep into a history a page token may point: each | |||
| 33 | + | // page walks the history from its start with `git log --skip`. | |||
| 34 | + | const MaxCommitSkip = 100_000 | |||
| 35 | + | ||||
| 36 | + | // pageSize applies the default and the maximum. | |||
| 37 | + | func pageSize(n int) (int, error) { | |||
| 38 | + | switch { | |||
| 39 | + | case n < 0: | |||
| 40 | + | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | |||
| 41 | + | case n == 0: | |||
| 42 | + | return DefaultPageSize, nil | |||
| 43 | + | } | |||
| 44 | + | return min(n, MaxPageSize), nil | |||
| 45 | + | } | |||
| 46 | + | ||||
| 47 | + | func decodeToken(tok, prefix string) (string, bool) { | |||
| 48 | + | if len(tok) > maxTokenLen { | |||
| 49 | + | return "", false | |||
| 50 | + | } | |||
| 51 | + | raw, err := base64.RawURLEncoding.DecodeString(tok) | |||
| 52 | + | if err != nil { | |||
| 53 | + | return "", false | |||
| 54 | + | } | |||
| 55 | + | return strings.CutPrefix(string(raw), prefix) | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | // pageAfter returns the first size items of the ordered items that satisfy | |||
| 59 | + | // after (nil keeps all of them) and, when more follow, the token of the last | |||
| 60 | + | // one returned. Tokens name the last item, so items that appear or disappear | |||
| 61 | + | // between two pages never shift the others. | |||
| 62 | + | func pageAfter[T any](items []T, size int, after func(T) bool, token func(T) string) (page []T, next string) { | |||
| 63 | + | for _, it := range items { | |||
| 64 | + | if after != nil && !after(it) { | |||
| 65 | + | continue | |||
| 66 | + | } | |||
| 67 | + | if len(page) == size { | |||
| 68 | + | return page, token(page[size-1]) | |||
| 69 | + | } | |||
| 70 | + | page = append(page, it) | |||
| 71 | + | } | |||
| 72 | + | return page, "" | |||
| 73 | + | } | |||
| 74 | + | ||||
| 21 | // RepoPage is a page of repositories ordered by name. | 75 | // RepoPage is a page of repositories ordered by name. | ||
| 22 | type RepoPage struct { | 76 | type RepoPage struct { | ||
| 23 | Repos []domain.Repo | 77 | Repos []domain.Repo | ||
| @@ -43,16 +97,9 @@ func (r *Repos) ListPage(ctx context.Context, p domain.Principal, size int, toke | |||||
| 43 | return RepoPage{}, err | 97 | return RepoPage{}, err | ||
| 44 | } | 98 | } | ||
| 45 | var page RepoPage | 99 | var page RepoPage | ||
| 46 | - | for _, repo := range all { | 100 | + | page.Repos, page.NextPageToken = pageAfter(all, n, |
| 47 | - | if after != "" && repo.Name.String() <= after { | 101 | + | func(repo domain.Repo) bool { return after == "" || repo.Name.String() > after }, |
| 48 | - | continue | 102 | + | func(repo domain.Repo) string { return encodeRepoToken(repo.Name) }) |
| 49 | - | } | |||
| 50 | - | if len(page.Repos) == n { | |||
| 51 | - | page.NextPageToken = encodeRepoToken(page.Repos[n-1].Name) | |||
| 52 | - | break | |||
| 53 | - | } | |||
| 54 | - | page.Repos = append(page.Repos, repo) | |||
| 55 | - | } | |||
| 56 | return page, nil | 103 | return page, nil | ||
| 57 | } | 104 | } | ||
| 58 | 105 | ||||
| @@ -102,16 +149,9 @@ func (t *Tokens) ListPage(ctx context.Context, p domain.Principal, size int, tok | |||||
| 102 | return strings.Compare(b.ID, a.ID) | 149 | return strings.Compare(b.ID, a.ID) | ||
| 103 | }) | 150 | }) | ||
| 104 | var page TokenPage | 151 | var page TokenPage | ||
| 105 | - | for _, tok := range all { | 152 | + | page.Tokens, page.NextPageToken = pageAfter(all, n, |
| 106 | - | if after != nil && !tokenAfter(tok, *after) { | 153 | + | func(tok domain.Token) bool { return after == nil || tokenAfter(tok, *after) }, |
| 107 | - | continue | 154 | + | encodeTokenCursor) |
| 108 | - | } | |||
| 109 | - | if len(page.Tokens) == n { | |||
| 110 | - | page.NextPageToken = encodeTokenCursor(page.Tokens[n-1]) | |||
| 111 | - | break | |||
| 112 | - | } | |||
| 113 | - | page.Tokens = append(page.Tokens, tok) | |||
| 114 | - | } | |||
| 115 | return page, nil | 155 | return page, nil | ||
| 116 | } | 156 | } | ||
| 117 | 157 | ||||
M internal/app/repos.go +11 -1
| @@ -109,10 +109,20 @@ func (r *Repos) discard(ctx context.Context, id ports.StagedID) { | |||
| 109 | 109 | } | |
| 110 | 110 | } | |
| 111 | 111 | ||
| 112 | + | // lookupRepo reads the metadata of a repository: domain.ErrInvalidName for the | |
| 113 | + | // zero name, domain.ErrNotFound for a missing repository. Access rules are the | |
| 114 | + | // caller's. | |
| 115 | + | func lookupRepo(ctx context.Context, meta ports.RepoMetaStore, name domain.RepoName) (domain.Repo, error) { | |
| 116 | + | if name.IsZero() { | |
| 117 | + | return domain.Repo{}, fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |
| 118 | + | } | |
| 119 | + | return meta.Get(ctx, name) | |
| 120 | + | } | |
| 121 | + | ||
| 112 | 122 | // Get returns a repository. A repository the principal may not read answers | |
| 113 | 123 | // domain.ErrNotFound, like a missing one. | |
| 114 | 124 | func (r *Repos) Get(ctx context.Context, p domain.Principal, name domain.RepoName) (domain.Repo, error) { | |
| 115 | - | repo, err := r.meta.Get(ctx, name) | |
| 125 | + | repo, err := lookupRepo(ctx, r.meta, name) | |
| 116 | 126 | if err != nil { | |
| 117 | 127 | return domain.Repo{}, err | |
| 118 | 128 | } | |
| @@ -109,10 +109,20 @@ func (r *Repos) discard(ctx context.Context, id ports.StagedID) { | |||||
| 109 | } | 109 | } | ||
| 110 | } | 110 | } | ||
| 111 | 111 | ||||
| 112 | + | // lookupRepo reads the metadata of a repository: domain.ErrInvalidName for the | |||
| 113 | + | // zero name, domain.ErrNotFound for a missing repository. Access rules are the | |||
| 114 | + | // caller's. | |||
| 115 | + | func lookupRepo(ctx context.Context, meta ports.RepoMetaStore, name domain.RepoName) (domain.Repo, error) { | |||
| 116 | + | if name.IsZero() { | |||
| 117 | + | return domain.Repo{}, fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |||
| 118 | + | } | |||
| 119 | + | return meta.Get(ctx, name) | |||
| 120 | + | } | |||
| 121 | + | ||||
| 112 | // Get returns a repository. A repository the principal may not read answers | 122 | // Get returns a repository. A repository the principal may not read answers | ||
| 113 | // domain.ErrNotFound, like a missing one. | 123 | // domain.ErrNotFound, like a missing one. | ||
| 114 | func (r *Repos) Get(ctx context.Context, p domain.Principal, name domain.RepoName) (domain.Repo, error) { | 124 | func (r *Repos) Get(ctx context.Context, p domain.Principal, name domain.RepoName) (domain.Repo, error) { | ||
| 115 | - | repo, err := r.meta.Get(ctx, name) | 125 | + | repo, err := lookupRepo(ctx, r.meta, name) |
| 116 | if err != nil { | 126 | if err != nil { | ||
| 117 | return domain.Repo{}, err | 127 | return domain.Repo{}, err | ||
| 118 | } | 128 | } | ||
M internal/app/repos_write.go +15 -14
| @@ -62,12 +62,11 @@ type UpdateParams struct { | |||
| 62 | 62 | // repository, and publishes RepoUpdated. The principal needs repo:admin on it | |
| 63 | 63 | // (see adminAccess for what the others learn). At least one field must be set, | |
| 64 | 64 | // else domain.ErrInvalid. A default branch that does not exist gives | |
| 65 | - | // domain.ErrPreconditionFailed; HEAD is only moved after every check passed. | |
| 65 | + | // domain.ErrPreconditionFailed; nothing is written before every check passed. | |
| 66 | + | // The metadata is written first, then HEAD, and RepoUpdated is published after | |
| 67 | + | // both (see the comment on the writes for the partial state). | |
| 66 | 68 | func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.RepoName, params UpdateParams) (domain.Repo, error) { | |
| 67 | - | if name.IsZero() { | |
| 68 | - | return domain.Repo{}, fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |
| 69 | - | } | |
| 70 | - | repo, err := r.meta.Get(ctx, name) | |
| 69 | + | repo, err := lookupRepo(ctx, r.meta, name) | |
| 71 | 70 | if err != nil { | |
| 72 | 71 | return domain.Repo{}, err | |
| 73 | 72 | } | |
| @@ -100,16 +99,21 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo | |||
| 100 | 99 | } | |
| 101 | 100 | } | |
| 102 | 101 | ||
| 103 | - | if params.DefaultBranch != nil { | |
| 104 | - | if err := r.git.SetDefaultBranch(ctx, name, *params.DefaultBranch); err != nil { | |
| 105 | - | return domain.Repo{}, fmt.Errorf("set default branch of %q: %w", name, err) | |
| 106 | - | } | |
| 107 | - | } | |
| 102 | + | // Metadata first, then HEAD. The two writes are not atomic: when moving | |
| 103 | + | // HEAD fails after the metadata was stored, the call fails with the | |
| 104 | + | // description and visibility already changed, and no RepoUpdated is | |
| 105 | + | // published for that partial state. The branch was checked above, so the | |
| 106 | + | // remaining causes are a vanished repository or an I/O failure. | |
| 108 | 107 | if params.Description != nil || params.Visibility != nil { | |
| 109 | 108 | if repo, err = r.updateMeta(ctx, name, params); err != nil { | |
| 110 | 109 | return domain.Repo{}, err | |
| 111 | 110 | } | |
| 112 | 111 | } | |
| 112 | + | if params.DefaultBranch != nil { | |
| 113 | + | if err := r.git.SetDefaultBranch(ctx, name, *params.DefaultBranch); err != nil { | |
| 114 | + | return domain.Repo{}, fmt.Errorf("set default branch of %q: %w", name, err) | |
| 115 | + | } | |
| 116 | + | } | |
| 113 | 117 | ||
| 114 | 118 | log := zerowrap.FromCtx(ctx) | |
| 115 | 119 | log.Info(). | |
| @@ -154,10 +158,7 @@ func (r *Repos) updateMeta(ctx context.Context, name domain.RepoName, params Upd | |||
| 154 | 158 | // adminAccess). A repository whose directory is already gone, left by a | |
| 155 | 159 | // delete that failed half way, loses its metadata. | |
| 156 | 160 | func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.RepoName) error { | |
| 157 | - | if name.IsZero() { | |
| 158 | - | return fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |
| 159 | - | } | |
| 160 | - | repo, err := r.meta.Get(ctx, name) | |
| 161 | + | repo, err := lookupRepo(ctx, r.meta, name) | |
| 161 | 162 | if err != nil { | |
| 162 | 163 | return err | |
| 163 | 164 | } | |
| @@ -62,12 +62,11 @@ type UpdateParams struct { | |||||
| 62 | // repository, and publishes RepoUpdated. The principal needs repo:admin on it | 62 | // repository, and publishes RepoUpdated. The principal needs repo:admin on it | ||
| 63 | // (see adminAccess for what the others learn). At least one field must be set, | 63 | // (see adminAccess for what the others learn). At least one field must be set, | ||
| 64 | // else domain.ErrInvalid. A default branch that does not exist gives | 64 | // else domain.ErrInvalid. A default branch that does not exist gives | ||
| 65 | - | // domain.ErrPreconditionFailed; HEAD is only moved after every check passed. | 65 | + | // domain.ErrPreconditionFailed; nothing is written before every check passed. |
| 66 | + | // The metadata is written first, then HEAD, and RepoUpdated is published after | |||
| 67 | + | // both (see the comment on the writes for the partial state). | |||
| 66 | func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.RepoName, params UpdateParams) (domain.Repo, error) { | 68 | func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.RepoName, params UpdateParams) (domain.Repo, error) { | ||
| 67 | - | if name.IsZero() { | 69 | + | repo, err := lookupRepo(ctx, r.meta, name) |
| 68 | - | return domain.Repo{}, fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |||
| 69 | - | } | |||
| 70 | - | repo, err := r.meta.Get(ctx, name) | |||
| 71 | if err != nil { | 70 | if err != nil { | ||
| 72 | return domain.Repo{}, err | 71 | return domain.Repo{}, err | ||
| 73 | } | 72 | } | ||
| @@ -100,16 +99,21 @@ func (r *Repos) Update(ctx context.Context, p domain.Principal, name domain.Repo | |||||
| 100 | } | 99 | } | ||
| 101 | } | 100 | } | ||
| 102 | 101 | ||||
| 103 | - | if params.DefaultBranch != nil { | 102 | + | // Metadata first, then HEAD. The two writes are not atomic: when moving |
| 104 | - | if err := r.git.SetDefaultBranch(ctx, name, *params.DefaultBranch); err != nil { | 103 | + | // HEAD fails after the metadata was stored, the call fails with the |
| 105 | - | return domain.Repo{}, fmt.Errorf("set default branch of %q: %w", name, err) | 104 | + | // description and visibility already changed, and no RepoUpdated is |
| 106 | - | } | 105 | + | // published for that partial state. The branch was checked above, so the |
| 107 | - | } | 106 | + | // remaining causes are a vanished repository or an I/O failure. |
| 108 | if params.Description != nil || params.Visibility != nil { | 107 | if params.Description != nil || params.Visibility != nil { | ||
| 109 | if repo, err = r.updateMeta(ctx, name, params); err != nil { | 108 | if repo, err = r.updateMeta(ctx, name, params); err != nil { | ||
| 110 | return domain.Repo{}, err | 109 | return domain.Repo{}, err | ||
| 111 | } | 110 | } | ||
| 112 | } | 111 | } | ||
| 112 | + | if params.DefaultBranch != nil { | |||
| 113 | + | if err := r.git.SetDefaultBranch(ctx, name, *params.DefaultBranch); err != nil { | |||
| 114 | + | return domain.Repo{}, fmt.Errorf("set default branch of %q: %w", name, err) | |||
| 115 | + | } | |||
| 116 | + | } | |||
| 113 | 117 | ||||
| 114 | log := zerowrap.FromCtx(ctx) | 118 | log := zerowrap.FromCtx(ctx) | ||
| 115 | log.Info(). | 119 | log.Info(). | ||
| @@ -154,10 +158,7 @@ func (r *Repos) updateMeta(ctx context.Context, name domain.RepoName, params Upd | |||||
| 154 | // adminAccess). A repository whose directory is already gone, left by a | 158 | // adminAccess). A repository whose directory is already gone, left by a | ||
| 155 | // delete that failed half way, loses its metadata. | 159 | // delete that failed half way, loses its metadata. | ||
| 156 | func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.RepoName) error { | 160 | func (r *Repos) Delete(ctx context.Context, p domain.Principal, name domain.RepoName) error { | ||
| 157 | - | if name.IsZero() { | 161 | + | repo, err := lookupRepo(ctx, r.meta, name) |
| 158 | - | return fmt.Errorf("%w: empty name", domain.ErrInvalidName) | |||
| 159 | - | } | |||
| 160 | - | repo, err := r.meta.Get(ctx, name) | |||
| 161 | if err != nil { | 162 | if err != nil { | ||
| 162 | return err | 163 | return err | ||
| 163 | } | 164 | } | ||