perf(web): list repos and refs in one call, share live refs fragments, trim blob copies
- repo and ref listings for in-process callers use pages of up to 1000 - live refs fragments are built once per repo, event and sign-in class - blob view presizes its buffer and checks highlighter tokens without joining - speculation rules skip heavy tree, blob, commit, compare and refs pages
- commit
- c505b30612a0ee0c37136dc237cae9cb03114431
- author
- brice · · 2026-10-11 18:57 UTC
- parent
- ed6aa64
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
14 files · +521 -32
M internal/adapters/web/ASSETS.md +1 -1
| @@ -14,7 +14,7 @@ content-hashed names. Nothing is fetched at runtime. | |||
| 14 | 14 | | `static/fonts/atkinson-next.woff2`, `atkinson-next-italic.woff2` | Atkinson Hyperlegible Next, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-next.txt`) | Google Fonts | `18b2a1a39a2fa298b0ba5390aca68462669826c90925656f1c1f6796e0e1bbaf`, `4a5037bfaf6680f40147407407ec09fa42925774bde809a579283d27f9f08106` | | |
| 15 | 15 | | `static/fonts/atkinson-mono.woff2` | Atkinson Hyperlegible Mono, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-mono.txt`) | Google Fonts | `2706b1ee4f452e744ea91f7e4908cbde9c5d35521bf5ffffc71a382a2de89613` | | |
| 16 | 16 | | `static/icons/*.svg` | interface icons, used as CSS masks; `brand.svg` is a grey picture shown as is | written here | | | |
| 17 | - | | `static/rules.json` | speculation rules: prefetch same-origin pages on hover (`Speculation-Rules` header) | written here | | | |
| 17 | + | | `static/rules.json` | speculation rules: prefetch the light same-origin pages on hover (`Speculation-Rules` header); the heavy ones (tree, blob, commit, commits, compare, refs, raw, MR "since") are excluded, each costs several Git reads on the server | written here | | | |
| 18 | 18 | ||
| 19 | 19 | The three htmx files are the exact files the approved mockup was built with. | |
| 20 | 20 | To upgrade, replace the file, update the checksum here, and run the web tests | |
| @@ -14,7 +14,7 @@ content-hashed names. Nothing is fetched at runtime. | |||||
| 14 | | `static/fonts/atkinson-next.woff2`, `atkinson-next-italic.woff2` | Atkinson Hyperlegible Next, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-next.txt`) | Google Fonts | `18b2a1a39a2fa298b0ba5390aca68462669826c90925656f1c1f6796e0e1bbaf`, `4a5037bfaf6680f40147407407ec09fa42925774bde809a579283d27f9f08106` | | 14 | | `static/fonts/atkinson-next.woff2`, `atkinson-next-italic.woff2` | Atkinson Hyperlegible Next, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-next.txt`) | Google Fonts | `18b2a1a39a2fa298b0ba5390aca68462669826c90925656f1c1f6796e0e1bbaf`, `4a5037bfaf6680f40147407407ec09fa42925774bde809a579283d27f9f08106` | | ||
| 15 | | `static/fonts/atkinson-mono.woff2` | Atkinson Hyperlegible Mono, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-mono.txt`) | Google Fonts | `2706b1ee4f452e744ea91f7e4908cbde9c5d35521bf5ffffc71a382a2de89613` | | 15 | | `static/fonts/atkinson-mono.woff2` | Atkinson Hyperlegible Mono, variable weight, latin subset (SIL OFL 1.1, `static/fonts/OFL-mono.txt`) | Google Fonts | `2706b1ee4f452e744ea91f7e4908cbde9c5d35521bf5ffffc71a382a2de89613` | | ||
| 16 | | `static/icons/*.svg` | interface icons, used as CSS masks; `brand.svg` is a grey picture shown as is | written here | | | 16 | | `static/icons/*.svg` | interface icons, used as CSS masks; `brand.svg` is a grey picture shown as is | written here | | | ||
| 17 | - | | `static/rules.json` | speculation rules: prefetch same-origin pages on hover (`Speculation-Rules` header) | written here | | | 17 | + | | `static/rules.json` | speculation rules: prefetch the light same-origin pages on hover (`Speculation-Rules` header); the heavy ones (tree, blob, commit, commits, compare, refs, raw, MR "since") are excluded, each costs several Git reads on the server | written here | | |
| 18 | 18 | ||||
| 19 | The three htmx files are the exact files the approved mockup was built with. | 19 | The three htmx files are the exact files the approved mockup was built with. | ||
| 20 | To upgrade, replace the file, update the checksum here, and run the web tests | 20 | To upgrade, replace the file, update the checksum here, and run the web tests | ||
M internal/adapters/web/blob.go +4 -0
| @@ -78,6 +78,10 @@ func (b *blobStream) close() { _ = b.stream.Close() } | |||
| 78 | 78 | ||
| 79 | 79 | // fill reads from the stream until buf holds n bytes or the content ends. | |
| 80 | 80 | func (b *blobStream) fill(n int) error { | |
| 81 | + | // One allocation for the whole content, not the doublings of append. | |
| 82 | + | if cap(b.buf) < n { | |
| 83 | + | b.buf = append(make([]byte, 0, n), b.buf...) | |
| 84 | + | } | |
| 81 | 85 | for len(b.buf) < n && b.stream.Receive() { | |
| 82 | 86 | b.buf = append(b.buf, b.stream.Msg().GetChunk()...) | |
| 83 | 87 | } | |
| @@ -78,6 +78,10 @@ func (b *blobStream) close() { _ = b.stream.Close() } | |||||
| 78 | 78 | ||||
| 79 | // fill reads from the stream until buf holds n bytes or the content ends. | 79 | // fill reads from the stream until buf holds n bytes or the content ends. | ||
| 80 | func (b *blobStream) fill(n int) error { | 80 | func (b *blobStream) fill(n int) error { | ||
| 81 | + | // One allocation for the whole content, not the doublings of append. | |||
| 82 | + | if cap(b.buf) < n { | |||
| 83 | + | b.buf = append(make([]byte, 0, n), b.buf...) | |||
| 84 | + | } | |||
| 81 | for len(b.buf) < n && b.stream.Receive() { | 85 | for len(b.buf) < n && b.stream.Receive() { | ||
| 82 | b.buf = append(b.buf, b.stream.Msg().GetChunk()...) | 86 | b.buf = append(b.buf, b.stream.Msg().GetChunk()...) | ||
| 83 | } | 87 | } | ||
A internal/adapters/web/fanout_test.go +213 -0
| @@ -0,0 +1,213 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package web | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "errors" | |
| 8 | + | "net/http" | |
| 9 | + | "net/http/httptest" | |
| 10 | + | "sync" | |
| 11 | + | "sync/atomic" | |
| 12 | + | "testing" | |
| 13 | + | ||
| 14 | + | "connectrpc.com/connect" | |
| 15 | + | "github.com/stretchr/testify/assert" | |
| 16 | + | "github.com/stretchr/testify/require" | |
| 17 | + | ||
| 18 | + | pb "bnema.dev/ermite-api/gen/ermite" | |
| 19 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 20 | + | ) | |
| 21 | + | ||
| 22 | + | func frag(s string) []fragment { return []fragment{{event: "refs-list", html: s}} } | |
| 23 | + | ||
| 24 | + | func TestFanoutBuildsOncePerKey(t *testing.T) { | |
| 25 | + | srvA, srvB := &Server{}, &Server{} | |
| 26 | + | base := fanKey{srv: srvA, repo: "x", event: "b-1"} | |
| 27 | + | for _, tt := range []struct { | |
| 28 | + | name string | |
| 29 | + | keys []fanKey | |
| 30 | + | built int32 | |
| 31 | + | }{ | |
| 32 | + | {"same key", []fanKey{base, base, base}, 1}, | |
| 33 | + | {"owner and anonymous never share", []fanKey{base, {srv: srvA, repo: "x", event: "b-1", owner: true}}, 2}, | |
| 34 | + | {"another event", []fanKey{base, {srv: srvA, repo: "x", event: "b-2"}}, 2}, | |
| 35 | + | {"another repository", []fanKey{base, {srv: srvA, repo: "y", event: "b-1"}}, 2}, | |
| 36 | + | {"another server", []fanKey{base, {srv: srvB, repo: "x", event: "b-1"}}, 2}, | |
| 37 | + | {"no event id: nothing to share by", []fanKey{{srv: srvA, repo: "x"}, {srv: srvA, repo: "x"}}, 2}, | |
| 38 | + | } { | |
| 39 | + | t.Run(tt.name, func(t *testing.T) { | |
| 40 | + | var f fanout | |
| 41 | + | var built atomic.Int32 | |
| 42 | + | for _, k := range tt.keys { | |
| 43 | + | got, err := f.do(context.Background(), k, func() ([]fragment, error) { | |
| 44 | + | built.Add(1) | |
| 45 | + | return frag("for-owner=" + map[bool]string{true: "yes", false: "no"}[k.owner]), nil | |
| 46 | + | }) | |
| 47 | + | require.NoError(t, err) | |
| 48 | + | assert.Equal(t, "for-owner="+map[bool]string{true: "yes", false: "no"}[k.owner], got[0].html, | |
| 49 | + | "a watcher never gets the output built for the other class") | |
| 50 | + | } | |
| 51 | + | assert.Equal(t, tt.built, built.Load()) | |
| 52 | + | }) | |
| 53 | + | } | |
| 54 | + | } | |
| 55 | + | ||
| 56 | + | func TestFanoutConcurrentWatchersShareOneBuild(t *testing.T) { | |
| 57 | + | var f fanout | |
| 58 | + | var built atomic.Int32 | |
| 59 | + | release := make(chan struct{}) | |
| 60 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 61 | + | var wg sync.WaitGroup | |
| 62 | + | results := make([][]fragment, 50) | |
| 63 | + | for i := range results { | |
| 64 | + | wg.Add(1) | |
| 65 | + | go func() { | |
| 66 | + | defer wg.Done() | |
| 67 | + | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { | |
| 68 | + | built.Add(1) | |
| 69 | + | <-release | |
| 70 | + | return frag("once"), nil | |
| 71 | + | }) | |
| 72 | + | }() | |
| 73 | + | } | |
| 74 | + | close(release) | |
| 75 | + | wg.Wait() | |
| 76 | + | assert.Equal(t, int32(1), built.Load()) | |
| 77 | + | for _, r := range results { | |
| 78 | + | assert.Equal(t, frag("once"), r) | |
| 79 | + | } | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | func TestFanoutFailureIsNotKept(t *testing.T) { | |
| 83 | + | var f fanout | |
| 84 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 85 | + | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return nil, errors.New("boom") }) | |
| 86 | + | require.Error(t, err) | |
| 87 | + | got, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("ok"), nil }) | |
| 88 | + | require.NoError(t, err) | |
| 89 | + | assert.Equal(t, frag("ok"), got, "the next watcher builds again") | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | |
| 93 | + | var f fanout | |
| 94 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 95 | + | started, release := make(chan struct{}), make(chan struct{}) | |
| 96 | + | go func() { | |
| 97 | + | _, _ = f.do(context.Background(), key, func() ([]fragment, error) { | |
| 98 | + | close(started) | |
| 99 | + | <-release | |
| 100 | + | return frag("late"), nil | |
| 101 | + | }) | |
| 102 | + | }() | |
| 103 | + | <-started | |
| 104 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 105 | + | cancel() | |
| 106 | + | _, err := f.do(ctx, key, func() ([]fragment, error) { return frag("never"), nil }) | |
| 107 | + | assert.ErrorIs(t, err, context.Canceled) | |
| 108 | + | close(release) | |
| 109 | + | } | |
| 110 | + | ||
| 111 | + | func TestFanoutIsBounded(t *testing.T) { | |
| 112 | + | var f fanout | |
| 113 | + | srv := &Server{} | |
| 114 | + | for i := range fanMax + 10 { | |
| 115 | + | key := fanKey{srv: srv, repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} | |
| 116 | + | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("x"), nil }) | |
| 117 | + | require.NoError(t, err) | |
| 118 | + | } | |
| 119 | + | assert.LessOrEqual(t, len(f.m), fanMax) | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | // refsAPI counts the calls the refs fragment makes to the API. | |
| 123 | + | type refsAPI struct { | |
| 124 | + | ermiteconnect.UnimplementedRepoServiceHandler | |
| 125 | + | ermiteconnect.UnimplementedGitServiceHandler | |
| 126 | + | repo, refs, commits atomic.Int32 | |
| 127 | + | pageSizes []int32 | |
| 128 | + | mu sync.Mutex | |
| 129 | + | } | |
| 130 | + | ||
| 131 | + | func (a *refsAPI) GetRepo(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { | |
| 132 | + | a.repo.Add(1) | |
| 133 | + | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func (a *refsAPI) ListRefs(_ context.Context, req *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |
| 137 | + | a.refs.Add(1) | |
| 138 | + | a.mu.Lock() | |
| 139 | + | a.pageSizes = append(a.pageSizes, req.Msg.GetPageSize()) | |
| 140 | + | a.mu.Unlock() | |
| 141 | + | id := "0123456789abcdef0123456789abcdef01234567" | |
| 142 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |
| 143 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |
| 144 | + | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |
| 145 | + | }}), nil | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | func (a *refsAPI) ListCommits(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |
| 149 | + | a.commits.Add(1) | |
| 150 | + | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |
| 151 | + | } | |
| 152 | + | ||
| 153 | + | func refsServer(t *testing.T, api *refsAPI) *Server { | |
| 154 | + | t.Helper() | |
| 155 | + | mux := http.NewServeMux() | |
| 156 | + | mux.Handle(ermiteconnect.NewRepoServiceHandler(api)) | |
| 157 | + | mux.Handle(ermiteconnect.NewGitServiceHandler(api)) | |
| 158 | + | backend := httptest.NewServer(mux) | |
| 159 | + | t.Cleanup(backend.Close) | |
| 160 | + | s, err := New(Config{ | |
| 161 | + | Repos: ermiteconnect.NewRepoServiceClient(backend.Client(), backend.URL), | |
| 162 | + | Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), | |
| 163 | + | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | |
| 164 | + | }) | |
| 165 | + | require.NoError(t, err) | |
| 166 | + | return s | |
| 167 | + | } | |
| 168 | + | ||
| 169 | + | // A push is rendered once for the watchers of the same class, and the owner | |
| 170 | + | // and the anonymous visitors never share what was built. | |
| 171 | + | func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | |
| 172 | + | api := &refsAPI{} | |
| 173 | + | s := refsServer(t, api) | |
| 174 | + | owner := context.WithValue(context.Background(), signedInKey{}, struct{}{}) | |
| 175 | + | anon := context.Background() | |
| 176 | + | ||
| 177 | + | for _, tt := range []struct { | |
| 178 | + | name string | |
| 179 | + | ctxs []context.Context | |
| 180 | + | wantRepo, wantCommits int32 | |
| 181 | + | }{ | |
| 182 | + | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1, 1}, | |
| 183 | + | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2, 2}, | |
| 184 | + | } { | |
| 185 | + | t.Run(tt.name, func(t *testing.T) { | |
| 186 | + | api.repo.Store(0) | |
| 187 | + | api.commits.Store(0) | |
| 188 | + | ev := &pb.Event{Id: "b-" + tt.name, Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} | |
| 189 | + | for _, ctx := range tt.ctxs { | |
| 190 | + | st := &liveStream{s: s, repo: "x", view: "refs"} | |
| 191 | + | frags, err := st.render(ctx, ev) | |
| 192 | + | require.NoError(t, err) | |
| 193 | + | require.Len(t, frags, 2) | |
| 194 | + | assert.Equal(t, "refs-list", frags[0].event) | |
| 195 | + | assert.Contains(t, frags[0].html, `id="ref-rows"`) | |
| 196 | + | } | |
| 197 | + | assert.Equal(t, tt.wantRepo, api.repo.Load(), "loadRepo") | |
| 198 | + | assert.Equal(t, tt.wantCommits, api.commits.Load(), "one ListCommits per distinct commit and class") | |
| 199 | + | }) | |
| 200 | + | } | |
| 201 | + | } | |
| 202 | + | ||
| 203 | + | // The refs are asked in one page of up to maxRefs. | |
| 204 | + | func TestLoadRepoAsksTheRefsInOnePage(t *testing.T) { | |
| 205 | + | api := &refsAPI{} | |
| 206 | + | s := refsServer(t, api) | |
| 207 | + | d, err := s.loadRepo(context.Background(), "x") | |
| 208 | + | require.NoError(t, err) | |
| 209 | + | assert.Len(t, d.branches, 1) | |
| 210 | + | assert.Len(t, d.tags, 1) | |
| 211 | + | assert.Equal(t, int32(1), api.refs.Load()) | |
| 212 | + | assert.Equal(t, []int32{maxRefs}, api.pageSizes) | |
| 213 | + | } | |
| @@ -0,0 +1,213 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package web | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "errors" | |||
| 8 | + | "net/http" | |||
| 9 | + | "net/http/httptest" | |||
| 10 | + | "sync" | |||
| 11 | + | "sync/atomic" | |||
| 12 | + | "testing" | |||
| 13 | + | ||||
| 14 | + | "connectrpc.com/connect" | |||
| 15 | + | "github.com/stretchr/testify/assert" | |||
| 16 | + | "github.com/stretchr/testify/require" | |||
| 17 | + | ||||
| 18 | + | pb "bnema.dev/ermite-api/gen/ermite" | |||
| 19 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |||
| 20 | + | ) | |||
| 21 | + | ||||
| 22 | + | func frag(s string) []fragment { return []fragment{{event: "refs-list", html: s}} } | |||
| 23 | + | ||||
| 24 | + | func TestFanoutBuildsOncePerKey(t *testing.T) { | |||
| 25 | + | srvA, srvB := &Server{}, &Server{} | |||
| 26 | + | base := fanKey{srv: srvA, repo: "x", event: "b-1"} | |||
| 27 | + | for _, tt := range []struct { | |||
| 28 | + | name string | |||
| 29 | + | keys []fanKey | |||
| 30 | + | built int32 | |||
| 31 | + | }{ | |||
| 32 | + | {"same key", []fanKey{base, base, base}, 1}, | |||
| 33 | + | {"owner and anonymous never share", []fanKey{base, {srv: srvA, repo: "x", event: "b-1", owner: true}}, 2}, | |||
| 34 | + | {"another event", []fanKey{base, {srv: srvA, repo: "x", event: "b-2"}}, 2}, | |||
| 35 | + | {"another repository", []fanKey{base, {srv: srvA, repo: "y", event: "b-1"}}, 2}, | |||
| 36 | + | {"another server", []fanKey{base, {srv: srvB, repo: "x", event: "b-1"}}, 2}, | |||
| 37 | + | {"no event id: nothing to share by", []fanKey{{srv: srvA, repo: "x"}, {srv: srvA, repo: "x"}}, 2}, | |||
| 38 | + | } { | |||
| 39 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 40 | + | var f fanout | |||
| 41 | + | var built atomic.Int32 | |||
| 42 | + | for _, k := range tt.keys { | |||
| 43 | + | got, err := f.do(context.Background(), k, func() ([]fragment, error) { | |||
| 44 | + | built.Add(1) | |||
| 45 | + | return frag("for-owner=" + map[bool]string{true: "yes", false: "no"}[k.owner]), nil | |||
| 46 | + | }) | |||
| 47 | + | require.NoError(t, err) | |||
| 48 | + | assert.Equal(t, "for-owner="+map[bool]string{true: "yes", false: "no"}[k.owner], got[0].html, | |||
| 49 | + | "a watcher never gets the output built for the other class") | |||
| 50 | + | } | |||
| 51 | + | assert.Equal(t, tt.built, built.Load()) | |||
| 52 | + | }) | |||
| 53 | + | } | |||
| 54 | + | } | |||
| 55 | + | ||||
| 56 | + | func TestFanoutConcurrentWatchersShareOneBuild(t *testing.T) { | |||
| 57 | + | var f fanout | |||
| 58 | + | var built atomic.Int32 | |||
| 59 | + | release := make(chan struct{}) | |||
| 60 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |||
| 61 | + | var wg sync.WaitGroup | |||
| 62 | + | results := make([][]fragment, 50) | |||
| 63 | + | for i := range results { | |||
| 64 | + | wg.Add(1) | |||
| 65 | + | go func() { | |||
| 66 | + | defer wg.Done() | |||
| 67 | + | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { | |||
| 68 | + | built.Add(1) | |||
| 69 | + | <-release | |||
| 70 | + | return frag("once"), nil | |||
| 71 | + | }) | |||
| 72 | + | }() | |||
| 73 | + | } | |||
| 74 | + | close(release) | |||
| 75 | + | wg.Wait() | |||
| 76 | + | assert.Equal(t, int32(1), built.Load()) | |||
| 77 | + | for _, r := range results { | |||
| 78 | + | assert.Equal(t, frag("once"), r) | |||
| 79 | + | } | |||
| 80 | + | } | |||
| 81 | + | ||||
| 82 | + | func TestFanoutFailureIsNotKept(t *testing.T) { | |||
| 83 | + | var f fanout | |||
| 84 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |||
| 85 | + | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return nil, errors.New("boom") }) | |||
| 86 | + | require.Error(t, err) | |||
| 87 | + | got, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("ok"), nil }) | |||
| 88 | + | require.NoError(t, err) | |||
| 89 | + | assert.Equal(t, frag("ok"), got, "the next watcher builds again") | |||
| 90 | + | } | |||
| 91 | + | ||||
| 92 | + | func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | |||
| 93 | + | var f fanout | |||
| 94 | + | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |||
| 95 | + | started, release := make(chan struct{}), make(chan struct{}) | |||
| 96 | + | go func() { | |||
| 97 | + | _, _ = f.do(context.Background(), key, func() ([]fragment, error) { | |||
| 98 | + | close(started) | |||
| 99 | + | <-release | |||
| 100 | + | return frag("late"), nil | |||
| 101 | + | }) | |||
| 102 | + | }() | |||
| 103 | + | <-started | |||
| 104 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 105 | + | cancel() | |||
| 106 | + | _, err := f.do(ctx, key, func() ([]fragment, error) { return frag("never"), nil }) | |||
| 107 | + | assert.ErrorIs(t, err, context.Canceled) | |||
| 108 | + | close(release) | |||
| 109 | + | } | |||
| 110 | + | ||||
| 111 | + | func TestFanoutIsBounded(t *testing.T) { | |||
| 112 | + | var f fanout | |||
| 113 | + | srv := &Server{} | |||
| 114 | + | for i := range fanMax + 10 { | |||
| 115 | + | key := fanKey{srv: srv, repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} | |||
| 116 | + | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("x"), nil }) | |||
| 117 | + | require.NoError(t, err) | |||
| 118 | + | } | |||
| 119 | + | assert.LessOrEqual(t, len(f.m), fanMax) | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | // refsAPI counts the calls the refs fragment makes to the API. | |||
| 123 | + | type refsAPI struct { | |||
| 124 | + | ermiteconnect.UnimplementedRepoServiceHandler | |||
| 125 | + | ermiteconnect.UnimplementedGitServiceHandler | |||
| 126 | + | repo, refs, commits atomic.Int32 | |||
| 127 | + | pageSizes []int32 | |||
| 128 | + | mu sync.Mutex | |||
| 129 | + | } | |||
| 130 | + | ||||
| 131 | + | func (a *refsAPI) GetRepo(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { | |||
| 132 | + | a.repo.Add(1) | |||
| 133 | + | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func (a *refsAPI) ListRefs(_ context.Context, req *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |||
| 137 | + | a.refs.Add(1) | |||
| 138 | + | a.mu.Lock() | |||
| 139 | + | a.pageSizes = append(a.pageSizes, req.Msg.GetPageSize()) | |||
| 140 | + | a.mu.Unlock() | |||
| 141 | + | id := "0123456789abcdef0123456789abcdef01234567" | |||
| 142 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |||
| 143 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |||
| 144 | + | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |||
| 145 | + | }}), nil | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | func (a *refsAPI) ListCommits(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |||
| 149 | + | a.commits.Add(1) | |||
| 150 | + | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |||
| 151 | + | } | |||
| 152 | + | ||||
| 153 | + | func refsServer(t *testing.T, api *refsAPI) *Server { | |||
| 154 | + | t.Helper() | |||
| 155 | + | mux := http.NewServeMux() | |||
| 156 | + | mux.Handle(ermiteconnect.NewRepoServiceHandler(api)) | |||
| 157 | + | mux.Handle(ermiteconnect.NewGitServiceHandler(api)) | |||
| 158 | + | backend := httptest.NewServer(mux) | |||
| 159 | + | t.Cleanup(backend.Close) | |||
| 160 | + | s, err := New(Config{ | |||
| 161 | + | Repos: ermiteconnect.NewRepoServiceClient(backend.Client(), backend.URL), | |||
| 162 | + | Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), | |||
| 163 | + | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | |||
| 164 | + | }) | |||
| 165 | + | require.NoError(t, err) | |||
| 166 | + | return s | |||
| 167 | + | } | |||
| 168 | + | ||||
| 169 | + | // A push is rendered once for the watchers of the same class, and the owner | |||
| 170 | + | // and the anonymous visitors never share what was built. | |||
| 171 | + | func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | |||
| 172 | + | api := &refsAPI{} | |||
| 173 | + | s := refsServer(t, api) | |||
| 174 | + | owner := context.WithValue(context.Background(), signedInKey{}, struct{}{}) | |||
| 175 | + | anon := context.Background() | |||
| 176 | + | ||||
| 177 | + | for _, tt := range []struct { | |||
| 178 | + | name string | |||
| 179 | + | ctxs []context.Context | |||
| 180 | + | wantRepo, wantCommits int32 | |||
| 181 | + | }{ | |||
| 182 | + | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1, 1}, | |||
| 183 | + | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2, 2}, | |||
| 184 | + | } { | |||
| 185 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 186 | + | api.repo.Store(0) | |||
| 187 | + | api.commits.Store(0) | |||
| 188 | + | ev := &pb.Event{Id: "b-" + tt.name, Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} | |||
| 189 | + | for _, ctx := range tt.ctxs { | |||
| 190 | + | st := &liveStream{s: s, repo: "x", view: "refs"} | |||
| 191 | + | frags, err := st.render(ctx, ev) | |||
| 192 | + | require.NoError(t, err) | |||
| 193 | + | require.Len(t, frags, 2) | |||
| 194 | + | assert.Equal(t, "refs-list", frags[0].event) | |||
| 195 | + | assert.Contains(t, frags[0].html, `id="ref-rows"`) | |||
| 196 | + | } | |||
| 197 | + | assert.Equal(t, tt.wantRepo, api.repo.Load(), "loadRepo") | |||
| 198 | + | assert.Equal(t, tt.wantCommits, api.commits.Load(), "one ListCommits per distinct commit and class") | |||
| 199 | + | }) | |||
| 200 | + | } | |||
| 201 | + | } | |||
| 202 | + | ||||
| 203 | + | // The refs are asked in one page of up to maxRefs. | |||
| 204 | + | func TestLoadRepoAsksTheRefsInOnePage(t *testing.T) { | |||
| 205 | + | api := &refsAPI{} | |||
| 206 | + | s := refsServer(t, api) | |||
| 207 | + | d, err := s.loadRepo(context.Background(), "x") | |||
| 208 | + | require.NoError(t, err) | |||
| 209 | + | assert.Len(t, d.branches, 1) | |||
| 210 | + | assert.Len(t, d.tags, 1) | |||
| 211 | + | assert.Equal(t, int32(1), api.refs.Load()) | |||
| 212 | + | assert.Equal(t, []int32{maxRefs}, api.pageSizes) | |||
| 213 | + | } | |||
M internal/adapters/web/fragments.go +121 -12
| @@ -5,7 +5,10 @@ package web | |||
| 5 | 5 | import ( | |
| 6 | 6 | "bytes" | |
| 7 | 7 | "context" | |
| 8 | + | "errors" | |
| 8 | 9 | "strconv" | |
| 10 | + | "sync" | |
| 11 | + | "time" | |
| 9 | 12 | ||
| 10 | 13 | "connectrpc.com/connect" | |
| 11 | 14 | "github.com/a-h/templ" | |
| @@ -121,7 +124,7 @@ func (l *liveStream) repoPage(ctx context.Context, ev *pb.Event) ([]fragment, er | |||
| 121 | 124 | } | |
| 122 | 125 | switch l.view { | |
| 123 | 126 | case "refs": | |
| 124 | - | return l.refs(ctx) | |
| 127 | + | return l.refs(ctx, ev.GetId()) | |
| 125 | 128 | case "commits": | |
| 126 | 129 | return l.banner(ctx, k.Pushed) | |
| 127 | 130 | } | |
| @@ -140,19 +143,125 @@ func (l *liveStream) lastPush(ctx context.Context) ([]fragment, error) { | |||
| 140 | 143 | return []fragment{f}, err | |
| 141 | 144 | } | |
| 142 | 145 | ||
| 143 | - | // refs is the table of the refs page and the facts of its rail. | |
| 144 | - | func (l *liveStream) refs(ctx context.Context) ([]fragment, error) { | |
| 145 | - | d, err := l.s.loadRepo(ctx, l.repo) | |
| 146 | - | if err != nil { | |
| 147 | - | return nil, err | |
| 146 | + | // refs is the table of the refs page and the facts of its rail. The fragments | |
| 147 | + | // of one event are built once for all the watchers of the repository that | |
| 148 | + | // see the same things (see fanout); eventID names the event. | |
| 149 | + | func (l *liveStream) refs(ctx context.Context, eventID string) ([]fragment, error) { | |
| 150 | + | key := fanKey{srv: l.s, repo: l.repo, event: eventID, owner: SignedIn(ctx)} | |
| 151 | + | return refsFan.do(ctx, key, func() ([]fragment, error) { | |
| 152 | + | d, err := l.s.loadRepo(ctx, l.repo) | |
| 153 | + | if err != nil { | |
| 154 | + | return nil, err | |
| 155 | + | } | |
| 156 | + | p := l.s.refsPage(ctx, d) | |
| 157 | + | body, err := renderFragment(ctx, "refs-list", RefsBody(p)) | |
| 158 | + | if err != nil { | |
| 159 | + | return nil, err | |
| 160 | + | } | |
| 161 | + | kv, err := renderFragment(ctx, "refs-list", RefsKV(p)) | |
| 162 | + | return []fragment{body, kv}, err | |
| 163 | + | }) | |
| 164 | + | } | |
| 165 | + | ||
| 166 | + | const ( | |
| 167 | + | // fanTTL is how long the fragments of an event are kept for the watchers | |
| 168 | + | // that are told of it a little later than the first. | |
| 169 | + | fanTTL = 2 * time.Second | |
| 170 | + | // fanMax bounds the events kept; past it an event is built for each | |
| 171 | + | // watcher, as without sharing. | |
| 172 | + | fanMax = 64 | |
| 173 | + | ) | |
| 174 | + | ||
| 175 | + | // fanKey says which watchers may share the fragments of an event: those of | |
| 176 | + | // the same repository, for the same event, and of the same class. The class | |
| 177 | + | // is anonymous or owner (a session is the owner's alone, ADR 0018): what is | |
| 178 | + | // built as the owner can show what an anonymous visitor may not see, so the | |
| 179 | + | // two never share an entry. | |
| 180 | + | type fanKey struct { | |
| 181 | + | srv *Server // events of two servers never share | |
| 182 | + | repo, event string | |
| 183 | + | owner bool | |
| 184 | + | } | |
| 185 | + | ||
| 186 | + | type fanEntry struct { | |
| 187 | + | done chan struct{} // closed once frags or err is set | |
| 188 | + | frags []fragment | |
| 189 | + | err error | |
| 190 | + | at time.Time // when it was built; zero while it is | |
| 191 | + | } | |
| 192 | + | ||
| 193 | + | // refsFan shares the fragments of the refs page between the watchers of a | |
| 194 | + | // push. It is a variable of the package, not a field of Server, only because | |
| 195 | + | // keys carry the server; entries are few and short-lived. | |
| 196 | + | var refsFan fanout | |
| 197 | + | ||
| 198 | + | // fanout builds the fragments of an event once for the many watchers that | |
| 199 | + | // receive it: a push to a repository that 256 pages watch would otherwise | |
| 200 | + | // load the refs and the commits they designate 256 times. The zero value is | |
| 201 | + | // ready. A failure is never kept: the next watcher builds it again. | |
| 202 | + | type fanout struct { | |
| 203 | + | mu sync.Mutex | |
| 204 | + | m map[fanKey]*fanEntry | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | // do returns the fragments of key, built by build when nobody has or is | |
| 208 | + | // building them. build runs with the context of the watcher that calls it, | |
| 209 | + | // which is of the class of the key; a watcher whose builder failed (its | |
| 210 | + | // request was cancelled, say) does not fail the others, they build again. | |
| 211 | + | func (f *fanout) do(ctx context.Context, key fanKey, build func() ([]fragment, error)) ([]fragment, error) { | |
| 212 | + | if key.event == "" { | |
| 213 | + | return build() // no event to share by | |
| 148 | 214 | } | |
| 149 | - | p := l.s.refsPage(ctx, d) | |
| 150 | - | body, err := renderFragment(ctx, "refs-list", RefsBody(p)) | |
| 151 | - | if err != nil { | |
| 152 | - | return nil, err | |
| 215 | + | for range 3 { | |
| 216 | + | f.mu.Lock() | |
| 217 | + | now := time.Now() | |
| 218 | + | for k, e := range f.m { | |
| 219 | + | if !e.at.IsZero() && now.Sub(e.at) > fanTTL { | |
| 220 | + | delete(f.m, k) | |
| 221 | + | } | |
| 222 | + | } | |
| 223 | + | e, shared := f.m[key] | |
| 224 | + | if !shared { | |
| 225 | + | if len(f.m) >= fanMax { | |
| 226 | + | f.mu.Unlock() | |
| 227 | + | return build() | |
| 228 | + | } | |
| 229 | + | if f.m == nil { | |
| 230 | + | f.m = make(map[fanKey]*fanEntry) | |
| 231 | + | } | |
| 232 | + | e = &fanEntry{done: make(chan struct{})} | |
| 233 | + | f.m[key] = e | |
| 234 | + | } | |
| 235 | + | f.mu.Unlock() | |
| 236 | + | ||
| 237 | + | if !shared { | |
| 238 | + | return f.lead(key, e, build) | |
| 239 | + | } | |
| 240 | + | select { | |
| 241 | + | case <-e.done: | |
| 242 | + | case <-ctx.Done(): | |
| 243 | + | return nil, ctx.Err() | |
| 244 | + | } | |
| 245 | + | if e.err == nil { | |
| 246 | + | return e.frags, nil | |
| 247 | + | } | |
| 153 | 248 | } | |
| 154 | - | kv, err := renderFragment(ctx, "refs-list", RefsKV(p)) | |
| 155 | - | return []fragment{body, kv}, err | |
| 249 | + | return build() | |
| 250 | + | } | |
| 251 | + | ||
| 252 | + | // lead builds the entry that the caller registered. | |
| 253 | + | func (f *fanout) lead(key fanKey, e *fanEntry, build func() ([]fragment, error)) (frags []fragment, err error) { | |
| 254 | + | err = errors.New("web: fragments were not built") // a panic of build | |
| 255 | + | defer func() { | |
| 256 | + | f.mu.Lock() | |
| 257 | + | e.frags, e.err, e.at = frags, err, time.Now() | |
| 258 | + | if err != nil && f.m[key] == e { | |
| 259 | + | delete(f.m, key) | |
| 260 | + | } | |
| 261 | + | f.mu.Unlock() | |
| 262 | + | close(e.done) | |
| 263 | + | }() | |
| 264 | + | return build() | |
| 156 | 265 | } | |
| 157 | 266 | ||
| 158 | 267 | // banner tells the commits page how many commits its branch got since the | |
| @@ -5,7 +5,10 @@ package web | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "bytes" | 6 | "bytes" | ||
| 7 | "context" | 7 | "context" | ||
| 8 | + | "errors" | |||
| 8 | "strconv" | 9 | "strconv" | ||
| 10 | + | "sync" | |||
| 11 | + | "time" | |||
| 9 | 12 | ||||
| 10 | "connectrpc.com/connect" | 13 | "connectrpc.com/connect" | ||
| 11 | "github.com/a-h/templ" | 14 | "github.com/a-h/templ" | ||
| @@ -121,7 +124,7 @@ func (l *liveStream) repoPage(ctx context.Context, ev *pb.Event) ([]fragment, er | |||||
| 121 | } | 124 | } | ||
| 122 | switch l.view { | 125 | switch l.view { | ||
| 123 | case "refs": | 126 | case "refs": | ||
| 124 | - | return l.refs(ctx) | 127 | + | return l.refs(ctx, ev.GetId()) |
| 125 | case "commits": | 128 | case "commits": | ||
| 126 | return l.banner(ctx, k.Pushed) | 129 | return l.banner(ctx, k.Pushed) | ||
| 127 | } | 130 | } | ||
| @@ -140,19 +143,125 @@ func (l *liveStream) lastPush(ctx context.Context) ([]fragment, error) { | |||||
| 140 | return []fragment{f}, err | 143 | return []fragment{f}, err | ||
| 141 | } | 144 | } | ||
| 142 | 145 | ||||
| 143 | - | // refs is the table of the refs page and the facts of its rail. | 146 | + | // refs is the table of the refs page and the facts of its rail. The fragments |
| 144 | - | func (l *liveStream) refs(ctx context.Context) ([]fragment, error) { | 147 | + | // of one event are built once for all the watchers of the repository that |
| 145 | - | d, err := l.s.loadRepo(ctx, l.repo) | 148 | + | // see the same things (see fanout); eventID names the event. |
| 146 | - | if err != nil { | 149 | + | func (l *liveStream) refs(ctx context.Context, eventID string) ([]fragment, error) { |
| 147 | - | return nil, err | 150 | + | key := fanKey{srv: l.s, repo: l.repo, event: eventID, owner: SignedIn(ctx)} |
| 151 | + | return refsFan.do(ctx, key, func() ([]fragment, error) { | |||
| 152 | + | d, err := l.s.loadRepo(ctx, l.repo) | |||
| 153 | + | if err != nil { | |||
| 154 | + | return nil, err | |||
| 155 | + | } | |||
| 156 | + | p := l.s.refsPage(ctx, d) | |||
| 157 | + | body, err := renderFragment(ctx, "refs-list", RefsBody(p)) | |||
| 158 | + | if err != nil { | |||
| 159 | + | return nil, err | |||
| 160 | + | } | |||
| 161 | + | kv, err := renderFragment(ctx, "refs-list", RefsKV(p)) | |||
| 162 | + | return []fragment{body, kv}, err | |||
| 163 | + | }) | |||
| 164 | + | } | |||
| 165 | + | ||||
| 166 | + | const ( | |||
| 167 | + | // fanTTL is how long the fragments of an event are kept for the watchers | |||
| 168 | + | // that are told of it a little later than the first. | |||
| 169 | + | fanTTL = 2 * time.Second | |||
| 170 | + | // fanMax bounds the events kept; past it an event is built for each | |||
| 171 | + | // watcher, as without sharing. | |||
| 172 | + | fanMax = 64 | |||
| 173 | + | ) | |||
| 174 | + | ||||
| 175 | + | // fanKey says which watchers may share the fragments of an event: those of | |||
| 176 | + | // the same repository, for the same event, and of the same class. The class | |||
| 177 | + | // is anonymous or owner (a session is the owner's alone, ADR 0018): what is | |||
| 178 | + | // built as the owner can show what an anonymous visitor may not see, so the | |||
| 179 | + | // two never share an entry. | |||
| 180 | + | type fanKey struct { | |||
| 181 | + | srv *Server // events of two servers never share | |||
| 182 | + | repo, event string | |||
| 183 | + | owner bool | |||
| 184 | + | } | |||
| 185 | + | ||||
| 186 | + | type fanEntry struct { | |||
| 187 | + | done chan struct{} // closed once frags or err is set | |||
| 188 | + | frags []fragment | |||
| 189 | + | err error | |||
| 190 | + | at time.Time // when it was built; zero while it is | |||
| 191 | + | } | |||
| 192 | + | ||||
| 193 | + | // refsFan shares the fragments of the refs page between the watchers of a | |||
| 194 | + | // push. It is a variable of the package, not a field of Server, only because | |||
| 195 | + | // keys carry the server; entries are few and short-lived. | |||
| 196 | + | var refsFan fanout | |||
| 197 | + | ||||
| 198 | + | // fanout builds the fragments of an event once for the many watchers that | |||
| 199 | + | // receive it: a push to a repository that 256 pages watch would otherwise | |||
| 200 | + | // load the refs and the commits they designate 256 times. The zero value is | |||
| 201 | + | // ready. A failure is never kept: the next watcher builds it again. | |||
| 202 | + | type fanout struct { | |||
| 203 | + | mu sync.Mutex | |||
| 204 | + | m map[fanKey]*fanEntry | |||
| 205 | + | } | |||
| 206 | + | ||||
| 207 | + | // do returns the fragments of key, built by build when nobody has or is | |||
| 208 | + | // building them. build runs with the context of the watcher that calls it, | |||
| 209 | + | // which is of the class of the key; a watcher whose builder failed (its | |||
| 210 | + | // request was cancelled, say) does not fail the others, they build again. | |||
| 211 | + | func (f *fanout) do(ctx context.Context, key fanKey, build func() ([]fragment, error)) ([]fragment, error) { | |||
| 212 | + | if key.event == "" { | |||
| 213 | + | return build() // no event to share by | |||
| 148 | } | 214 | } | ||
| 149 | - | p := l.s.refsPage(ctx, d) | 215 | + | for range 3 { |
| 150 | - | body, err := renderFragment(ctx, "refs-list", RefsBody(p)) | 216 | + | f.mu.Lock() |
| 151 | - | if err != nil { | 217 | + | now := time.Now() |
| 152 | - | return nil, err | 218 | + | for k, e := range f.m { |
| 219 | + | if !e.at.IsZero() && now.Sub(e.at) > fanTTL { | |||
| 220 | + | delete(f.m, k) | |||
| 221 | + | } | |||
| 222 | + | } | |||
| 223 | + | e, shared := f.m[key] | |||
| 224 | + | if !shared { | |||
| 225 | + | if len(f.m) >= fanMax { | |||
| 226 | + | f.mu.Unlock() | |||
| 227 | + | return build() | |||
| 228 | + | } | |||
| 229 | + | if f.m == nil { | |||
| 230 | + | f.m = make(map[fanKey]*fanEntry) | |||
| 231 | + | } | |||
| 232 | + | e = &fanEntry{done: make(chan struct{})} | |||
| 233 | + | f.m[key] = e | |||
| 234 | + | } | |||
| 235 | + | f.mu.Unlock() | |||
| 236 | + | ||||
| 237 | + | if !shared { | |||
| 238 | + | return f.lead(key, e, build) | |||
| 239 | + | } | |||
| 240 | + | select { | |||
| 241 | + | case <-e.done: | |||
| 242 | + | case <-ctx.Done(): | |||
| 243 | + | return nil, ctx.Err() | |||
| 244 | + | } | |||
| 245 | + | if e.err == nil { | |||
| 246 | + | return e.frags, nil | |||
| 247 | + | } | |||
| 153 | } | 248 | } | ||
| 154 | - | kv, err := renderFragment(ctx, "refs-list", RefsKV(p)) | 249 | + | return build() |
| 155 | - | return []fragment{body, kv}, err | 250 | + | } |
| 251 | + | ||||
| 252 | + | // lead builds the entry that the caller registered. | |||
| 253 | + | func (f *fanout) lead(key fanKey, e *fanEntry, build func() ([]fragment, error)) (frags []fragment, err error) { | |||
| 254 | + | err = errors.New("web: fragments were not built") // a panic of build | |||
| 255 | + | defer func() { | |||
| 256 | + | f.mu.Lock() | |||
| 257 | + | e.frags, e.err, e.at = frags, err, time.Now() | |||
| 258 | + | if err != nil && f.m[key] == e { | |||
| 259 | + | delete(f.m, key) | |||
| 260 | + | } | |||
| 261 | + | f.mu.Unlock() | |||
| 262 | + | close(e.done) | |||
| 263 | + | }() | |||
| 264 | + | return build() | |||
| 156 | } | 265 | } | ||
| 157 | 266 | ||||
| 158 | // banner tells the commits page how many commits its branch got since the | 267 | // banner tells the commits page how many commits its branch got since the | ||
M internal/adapters/web/highlight.go +24 -6
| @@ -3,6 +3,7 @@ | |||
| 3 | 3 | package web | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "bytes" | |
| 6 | 7 | "context" | |
| 7 | 8 | "fmt" | |
| 8 | 9 | "html" | |
| @@ -109,7 +110,7 @@ const maxViewLines = 20000 | |||
| 109 | 110 | ||
| 110 | 111 | // countLines is the number of lines of src, as the code view numbers them. | |
| 111 | 112 | func countLines(src []byte) int { | |
| 112 | - | n := strings.Count(string(src), "\n") | |
| 113 | + | n := bytes.Count(src, []byte{'\n'}) | |
| 113 | 114 | if len(src) > 0 && src[len(src)-1] != '\n' { | |
| 114 | 115 | n++ | |
| 115 | 116 | } | |
| @@ -158,18 +159,35 @@ func tokenise(ctx context.Context, lx chroma.Lexer, src string) (toks []chroma.T | |||
| 158 | 159 | } | |
| 159 | 160 | toks = append(toks, t) | |
| 160 | 161 | } | |
| 161 | - | if strings.TrimRight(joinTokens(toks), "\n") != strings.TrimRight(src, "\n") { | |
| 162 | + | if !tokensMatch(toks, src) { | |
| 162 | 163 | return nil, false | |
| 163 | 164 | } | |
| 164 | 165 | return toks, true | |
| 165 | 166 | } | |
| 166 | 167 | ||
| 167 | - | func joinTokens(toks []chroma.Token) string { | |
| 168 | - | var b strings.Builder | |
| 168 | + | // tokensMatch reports whether the values of the tokens, joined, are src but | |
| 169 | + | // for the line feeds that end either: it compares piece by piece, where | |
| 170 | + | // joining the tokens first would copy the whole file. | |
| 171 | + | func tokensMatch(toks []chroma.Token, src string) bool { | |
| 172 | + | want := strings.TrimRight(src, "\n") | |
| 173 | + | pos := 0 | |
| 169 | 174 | for _, t := range toks { | |
| 170 | - | b.WriteString(t.Value) | |
| 175 | + | v := t.Value | |
| 176 | + | rest := want[pos:] | |
| 177 | + | if len(v) <= len(rest) { | |
| 178 | + | if rest[:len(v)] != v { | |
| 179 | + | return false | |
| 180 | + | } | |
| 181 | + | pos += len(v) | |
| 182 | + | continue | |
| 183 | + | } | |
| 184 | + | // The token reaches past the end of the source: only line feeds may. | |
| 185 | + | if v[:len(rest)] != rest || strings.Trim(v[len(rest):], "\n") != "" { | |
| 186 | + | return false | |
| 187 | + | } | |
| 188 | + | pos = len(want) | |
| 171 | 189 | } | |
| 172 | - | return b.String() | |
| 190 | + | return pos == len(want) | |
| 173 | 191 | } | |
| 174 | 192 | ||
| 175 | 193 | // splitSource splits plain text into lines, without the empty element after | |
| @@ -3,6 +3,7 @@ | |||||
| 3 | package web | 3 | package web | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "bytes" | |||
| 6 | "context" | 7 | "context" | ||
| 7 | "fmt" | 8 | "fmt" | ||
| 8 | "html" | 9 | "html" | ||
| @@ -109,7 +110,7 @@ const maxViewLines = 20000 | |||||
| 109 | 110 | ||||
| 110 | // countLines is the number of lines of src, as the code view numbers them. | 111 | // countLines is the number of lines of src, as the code view numbers them. | ||
| 111 | func countLines(src []byte) int { | 112 | func countLines(src []byte) int { | ||
| 112 | - | n := strings.Count(string(src), "\n") | 113 | + | n := bytes.Count(src, []byte{'\n'}) |
| 113 | if len(src) > 0 && src[len(src)-1] != '\n' { | 114 | if len(src) > 0 && src[len(src)-1] != '\n' { | ||
| 114 | n++ | 115 | n++ | ||
| 115 | } | 116 | } | ||
| @@ -158,18 +159,35 @@ func tokenise(ctx context.Context, lx chroma.Lexer, src string) (toks []chroma.T | |||||
| 158 | } | 159 | } | ||
| 159 | toks = append(toks, t) | 160 | toks = append(toks, t) | ||
| 160 | } | 161 | } | ||
| 161 | - | if strings.TrimRight(joinTokens(toks), "\n") != strings.TrimRight(src, "\n") { | 162 | + | if !tokensMatch(toks, src) { |
| 162 | return nil, false | 163 | return nil, false | ||
| 163 | } | 164 | } | ||
| 164 | return toks, true | 165 | return toks, true | ||
| 165 | } | 166 | } | ||
| 166 | 167 | ||||
| 167 | - | func joinTokens(toks []chroma.Token) string { | 168 | + | // tokensMatch reports whether the values of the tokens, joined, are src but |
| 168 | - | var b strings.Builder | 169 | + | // for the line feeds that end either: it compares piece by piece, where |
| 170 | + | // joining the tokens first would copy the whole file. | |||
| 171 | + | func tokensMatch(toks []chroma.Token, src string) bool { | |||
| 172 | + | want := strings.TrimRight(src, "\n") | |||
| 173 | + | pos := 0 | |||
| 169 | for _, t := range toks { | 174 | for _, t := range toks { | ||
| 170 | - | b.WriteString(t.Value) | 175 | + | v := t.Value |
| 176 | + | rest := want[pos:] | |||
| 177 | + | if len(v) <= len(rest) { | |||
| 178 | + | if rest[:len(v)] != v { | |||
| 179 | + | return false | |||
| 180 | + | } | |||
| 181 | + | pos += len(v) | |||
| 182 | + | continue | |||
| 183 | + | } | |||
| 184 | + | // The token reaches past the end of the source: only line feeds may. | |||
| 185 | + | if v[:len(rest)] != rest || strings.Trim(v[len(rest):], "\n") != "" { | |||
| 186 | + | return false | |||
| 187 | + | } | |||
| 188 | + | pos = len(want) | |||
| 171 | } | 189 | } | ||
| 172 | - | return b.String() | 190 | + | return pos == len(want) |
| 173 | } | 191 | } | ||
| 174 | 192 | ||||
| 175 | // splitSource splits plain text into lines, without the empty element after | 193 | // splitSource splits plain text into lines, without the empty element after | ||
M internal/adapters/web/highlight_test.go +33 -0
| @@ -218,3 +218,36 @@ func normSpace(s string) string { | |||
| 218 | 218 | } | |
| 219 | 219 | return strings.Join(out, "\n") | |
| 220 | 220 | } | |
| 221 | + | ||
| 222 | + | func TestTokensMatch(t *testing.T) { | |
| 223 | + | toks := func(vs ...string) []chroma.Token { | |
| 224 | + | var out []chroma.Token | |
| 225 | + | for _, v := range vs { | |
| 226 | + | out = append(out, chroma.Token{Type: chroma.Text, Value: v}) | |
| 227 | + | } | |
| 228 | + | return out | |
| 229 | + | } | |
| 230 | + | for _, tt := range []struct { | |
| 231 | + | name string | |
| 232 | + | toks []chroma.Token | |
| 233 | + | src string | |
| 234 | + | want bool | |
| 235 | + | }{ | |
| 236 | + | {"same pieces", toks("ab", "c\n", "d"), "abc\nd", true}, | |
| 237 | + | {"lexer added a final line feed", toks("a\n", "b", "\n"), "a\nb", true}, | |
| 238 | + | {"source ends with line feeds", toks("a", "\n"), "a\n\n", true}, | |
| 239 | + | {"empty", nil, "", true}, | |
| 240 | + | {"empty tokens", toks("", ""), "\n", true}, | |
| 241 | + | {"only line feeds", toks("\n\n"), "", true}, | |
| 242 | + | {"token differs", toks("a", "x"), "ab", false}, | |
| 243 | + | {"tokens stop short", toks("a"), "ab", false}, | |
| 244 | + | {"tokens go past the source", toks("abc"), "ab", false}, | |
| 245 | + | {"extra text after the source", toks("ab", "\nx"), "ab", false}, | |
| 246 | + | {"lexer dropped a line", toks("a\n", "c"), "a\nb\nc", false}, | |
| 247 | + | {"unicode", toks("é", "日本"), "é日本", true}, | |
| 248 | + | } { | |
| 249 | + | t.Run(tt.name, func(t *testing.T) { | |
| 250 | + | assert.Equal(t, tt.want, tokensMatch(tt.toks, tt.src)) | |
| 251 | + | }) | |
| 252 | + | } | |
| 253 | + | } | |
| @@ -218,3 +218,36 @@ func normSpace(s string) string { | |||||
| 218 | } | 218 | } | ||
| 219 | return strings.Join(out, "\n") | 219 | return strings.Join(out, "\n") | ||
| 220 | } | 220 | } | ||
| 221 | + | ||||
| 222 | + | func TestTokensMatch(t *testing.T) { | |||
| 223 | + | toks := func(vs ...string) []chroma.Token { | |||
| 224 | + | var out []chroma.Token | |||
| 225 | + | for _, v := range vs { | |||
| 226 | + | out = append(out, chroma.Token{Type: chroma.Text, Value: v}) | |||
| 227 | + | } | |||
| 228 | + | return out | |||
| 229 | + | } | |||
| 230 | + | for _, tt := range []struct { | |||
| 231 | + | name string | |||
| 232 | + | toks []chroma.Token | |||
| 233 | + | src string | |||
| 234 | + | want bool | |||
| 235 | + | }{ | |||
| 236 | + | {"same pieces", toks("ab", "c\n", "d"), "abc\nd", true}, | |||
| 237 | + | {"lexer added a final line feed", toks("a\n", "b", "\n"), "a\nb", true}, | |||
| 238 | + | {"source ends with line feeds", toks("a", "\n"), "a\n\n", true}, | |||
| 239 | + | {"empty", nil, "", true}, | |||
| 240 | + | {"empty tokens", toks("", ""), "\n", true}, | |||
| 241 | + | {"only line feeds", toks("\n\n"), "", true}, | |||
| 242 | + | {"token differs", toks("a", "x"), "ab", false}, | |||
| 243 | + | {"tokens stop short", toks("a"), "ab", false}, | |||
| 244 | + | {"tokens go past the source", toks("abc"), "ab", false}, | |||
| 245 | + | {"extra text after the source", toks("ab", "\nx"), "ab", false}, | |||
| 246 | + | {"lexer dropped a line", toks("a\n", "c"), "a\nb\nc", false}, | |||
| 247 | + | {"unicode", toks("é", "日本"), "é日本", true}, | |||
| 248 | + | } { | |||
| 249 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 250 | + | assert.Equal(t, tt.want, tokensMatch(tt.toks, tt.src)) | |||
| 251 | + | }) | |||
| 252 | + | } | |||
| 253 | + | } | |||
M internal/adapters/web/home.go +7 -3
| @@ -18,16 +18,20 @@ import ( | |||
| 18 | 18 | // maxRepos bounds the repositories one page lists. | |
| 19 | 19 | const maxRepos = 1000 | |
| 20 | 20 | ||
| 21 | - | // pageSize is the page size asked from the list RPCs: the largest the API | |
| 22 | - | // accepts. | |
| 21 | + | // pageSize is the page size asked from the commit lists: the largest the API | |
| 22 | + | // accepts for them. | |
| 23 | 23 | const pageSize = 100 | |
| 24 | 24 | ||
| 25 | 25 | // listRepos reads the repositories the visitor may see, ordered by name. | |
| 26 | + | // Every page of ListRepos scans the whole repository list and reads the HEAD | |
| 27 | + | // of each repository of the page, so the list is asked in one page of up to | |
| 28 | + | // maxRepos: one scan in the common case, whatever the API cap is (a lower | |
| 29 | + | // one only means more pages, followed by their token). | |
| 26 | 30 | func (s *Server) listRepos(ctx context.Context) ([]*pb.Repo, error) { | |
| 27 | 31 | var repos []*pb.Repo | |
| 28 | 32 | token := "" | |
| 29 | 33 | for len(repos) < maxRepos { | |
| 30 | - | res, err := s.cfg.Repos.ListRepos(ctx, connect.NewRequest(&pb.ListReposRequest{PageSize: pageSize, PageToken: token})) | |
| 34 | + | res, err := s.cfg.Repos.ListRepos(ctx, connect.NewRequest(&pb.ListReposRequest{PageSize: maxRepos, PageToken: token})) | |
| 31 | 35 | if err != nil { | |
| 32 | 36 | return nil, err | |
| 33 | 37 | } | |
| @@ -18,16 +18,20 @@ import ( | |||||
| 18 | // maxRepos bounds the repositories one page lists. | 18 | // maxRepos bounds the repositories one page lists. | ||
| 19 | const maxRepos = 1000 | 19 | const maxRepos = 1000 | ||
| 20 | 20 | ||||
| 21 | - | // pageSize is the page size asked from the list RPCs: the largest the API | 21 | + | // pageSize is the page size asked from the commit lists: the largest the API |
| 22 | - | // accepts. | 22 | + | // accepts for them. |
| 23 | const pageSize = 100 | 23 | const pageSize = 100 | ||
| 24 | 24 | ||||
| 25 | // listRepos reads the repositories the visitor may see, ordered by name. | 25 | // listRepos reads the repositories the visitor may see, ordered by name. | ||
| 26 | + | // Every page of ListRepos scans the whole repository list and reads the HEAD | |||
| 27 | + | // of each repository of the page, so the list is asked in one page of up to | |||
| 28 | + | // maxRepos: one scan in the common case, whatever the API cap is (a lower | |||
| 29 | + | // one only means more pages, followed by their token). | |||
| 26 | func (s *Server) listRepos(ctx context.Context) ([]*pb.Repo, error) { | 30 | func (s *Server) listRepos(ctx context.Context) ([]*pb.Repo, error) { | ||
| 27 | var repos []*pb.Repo | 31 | var repos []*pb.Repo | ||
| 28 | token := "" | 32 | token := "" | ||
| 29 | for len(repos) < maxRepos { | 33 | for len(repos) < maxRepos { | ||
| 30 | - | res, err := s.cfg.Repos.ListRepos(ctx, connect.NewRequest(&pb.ListReposRequest{PageSize: pageSize, PageToken: token})) | 34 | + | res, err := s.cfg.Repos.ListRepos(ctx, connect.NewRequest(&pb.ListReposRequest{PageSize: maxRepos, PageToken: token})) |
| 31 | if err != nil { | 35 | if err != nil { | ||
| 32 | return nil, err | 36 | return nil, err | ||
| 33 | } | 37 | } | ||
M internal/adapters/web/repo.go +4 -1
| @@ -42,10 +42,13 @@ func (s *Server) loadRepo(ctx context.Context, name string) (*repoData, error) { | |||
| 42 | 42 | return nil, err | |
| 43 | 43 | } | |
| 44 | 44 | d := &repoData{repo: res.Msg.GetRepo()} | |
| 45 | + | // Each page of ListRefs runs the whole for-each-ref again: the refs are | |
| 46 | + | // asked in one page of up to maxRefs (the API lowers it to its cap, and | |
| 47 | + | // the token then leads to the rest). | |
| 45 | 48 | token, n := "", 0 | |
| 46 | 49 | for { | |
| 47 | 50 | page, err := s.cfg.Git.ListRefs(ctx, connect.NewRequest(&pb.ListRefsRequest{ | |
| 48 | - | Repo: name, PageSize: pageSize, PageToken: token, | |
| 51 | + | Repo: name, PageSize: maxRefs, PageToken: token, | |
| 49 | 52 | Kinds: []pb.RefKind{pb.RefKind_REF_KIND_BRANCH, pb.RefKind_REF_KIND_TAG}, | |
| 50 | 53 | })) | |
| 51 | 54 | if err != nil { | |
| @@ -42,10 +42,13 @@ func (s *Server) loadRepo(ctx context.Context, name string) (*repoData, error) { | |||||
| 42 | return nil, err | 42 | return nil, err | ||
| 43 | } | 43 | } | ||
| 44 | d := &repoData{repo: res.Msg.GetRepo()} | 44 | d := &repoData{repo: res.Msg.GetRepo()} | ||
| 45 | + | // Each page of ListRefs runs the whole for-each-ref again: the refs are | |||
| 46 | + | // asked in one page of up to maxRefs (the API lowers it to its cap, and | |||
| 47 | + | // the token then leads to the rest). | |||
| 45 | token, n := "", 0 | 48 | token, n := "", 0 | ||
| 46 | for { | 49 | for { | ||
| 47 | page, err := s.cfg.Git.ListRefs(ctx, connect.NewRequest(&pb.ListRefsRequest{ | 50 | page, err := s.cfg.Git.ListRefs(ctx, connect.NewRequest(&pb.ListRefsRequest{ | ||
| 48 | - | Repo: name, PageSize: pageSize, PageToken: token, | 51 | + | Repo: name, PageSize: maxRefs, PageToken: token, |
| 49 | Kinds: []pb.RefKind{pb.RefKind_REF_KIND_BRANCH, pb.RefKind_REF_KIND_TAG}, | 52 | Kinds: []pb.RefKind{pb.RefKind_REF_KIND_BRANCH, pb.RefKind_REF_KIND_TAG}, | ||
| 50 | })) | 53 | })) | ||
| 51 | if err != nil { | 54 | if err != nil { | ||
A internal/adapters/web/rules_test.go +55 -0
| @@ -0,0 +1,55 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package web | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "encoding/json" | |
| 7 | + | "testing" | |
| 8 | + | ||
| 9 | + | "github.com/stretchr/testify/assert" | |
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // The speculation rules prefetch the light pages and leave the heavy ones | |
| 14 | + | // (several Git reads each) to a click. | |
| 15 | + | func TestSpeculationRulesSkipHeavyPages(t *testing.T) { | |
| 16 | + | raw, err := staticFS.ReadFile("static/rules.json") | |
| 17 | + | require.NoError(t, err) | |
| 18 | + | var rules struct { | |
| 19 | + | Prefetch []struct { | |
| 20 | + | Where struct { | |
| 21 | + | And []struct { | |
| 22 | + | Href string `json:"href_matches"` | |
| 23 | + | Not *struct { | |
| 24 | + | Href string `json:"href_matches"` | |
| 25 | + | } `json:"not"` | |
| 26 | + | } `json:"and"` | |
| 27 | + | } `json:"where"` | |
| 28 | + | Eagerness string `json:"eagerness"` | |
| 29 | + | } `json:"prefetch"` | |
| 30 | + | } | |
| 31 | + | require.NoError(t, json.Unmarshal(raw, &rules)) | |
| 32 | + | require.Len(t, rules.Prefetch, 1) | |
| 33 | + | assert.Equal(t, "moderate", rules.Prefetch[0].Eagerness) | |
| 34 | + | ||
| 35 | + | var allowed string | |
| 36 | + | excluded := map[string]bool{} | |
| 37 | + | for _, c := range rules.Prefetch[0].Where.And { | |
| 38 | + | if c.Href != "" { | |
| 39 | + | allowed = c.Href | |
| 40 | + | } | |
| 41 | + | if c.Not != nil && c.Not.Href != "" { | |
| 42 | + | excluded[c.Not.Href] = true | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | assert.Equal(t, "/*", allowed) | |
| 46 | + | for _, p := range []string{ | |
| 47 | + | "/-/*", "/*/raw/*", "/*/tree", "/*/tree/*", "/*/blob/*", "/*/commit/*", | |
| 48 | + | "/*/commits", "/*/commits/*", "/*/compare/*", "/*/refs", "/*/merge-requests/*/since", | |
| 49 | + | } { | |
| 50 | + | assert.True(t, excluded[p], "%s is not prefetched", p) | |
| 51 | + | } | |
| 52 | + | for _, p := range []string{"/", "/*", "/*/merge-requests", "/*/merge-requests/*"} { | |
| 53 | + | assert.False(t, excluded[p], "%s stays prefetched", p) | |
| 54 | + | } | |
| 55 | + | } | |
| @@ -0,0 +1,55 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package web | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "encoding/json" | |||
| 7 | + | "testing" | |||
| 8 | + | ||||
| 9 | + | "github.com/stretchr/testify/assert" | |||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // The speculation rules prefetch the light pages and leave the heavy ones | |||
| 14 | + | // (several Git reads each) to a click. | |||
| 15 | + | func TestSpeculationRulesSkipHeavyPages(t *testing.T) { | |||
| 16 | + | raw, err := staticFS.ReadFile("static/rules.json") | |||
| 17 | + | require.NoError(t, err) | |||
| 18 | + | var rules struct { | |||
| 19 | + | Prefetch []struct { | |||
| 20 | + | Where struct { | |||
| 21 | + | And []struct { | |||
| 22 | + | Href string `json:"href_matches"` | |||
| 23 | + | Not *struct { | |||
| 24 | + | Href string `json:"href_matches"` | |||
| 25 | + | } `json:"not"` | |||
| 26 | + | } `json:"and"` | |||
| 27 | + | } `json:"where"` | |||
| 28 | + | Eagerness string `json:"eagerness"` | |||
| 29 | + | } `json:"prefetch"` | |||
| 30 | + | } | |||
| 31 | + | require.NoError(t, json.Unmarshal(raw, &rules)) | |||
| 32 | + | require.Len(t, rules.Prefetch, 1) | |||
| 33 | + | assert.Equal(t, "moderate", rules.Prefetch[0].Eagerness) | |||
| 34 | + | ||||
| 35 | + | var allowed string | |||
| 36 | + | excluded := map[string]bool{} | |||
| 37 | + | for _, c := range rules.Prefetch[0].Where.And { | |||
| 38 | + | if c.Href != "" { | |||
| 39 | + | allowed = c.Href | |||
| 40 | + | } | |||
| 41 | + | if c.Not != nil && c.Not.Href != "" { | |||
| 42 | + | excluded[c.Not.Href] = true | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | assert.Equal(t, "/*", allowed) | |||
| 46 | + | for _, p := range []string{ | |||
| 47 | + | "/-/*", "/*/raw/*", "/*/tree", "/*/tree/*", "/*/blob/*", "/*/commit/*", | |||
| 48 | + | "/*/commits", "/*/commits/*", "/*/compare/*", "/*/refs", "/*/merge-requests/*/since", | |||
| 49 | + | } { | |||
| 50 | + | assert.True(t, excluded[p], "%s is not prefetched", p) | |||
| 51 | + | } | |||
| 52 | + | for _, p := range []string{"/", "/*", "/*/merge-requests", "/*/merge-requests/*"} { | |||
| 53 | + | assert.False(t, excluded[p], "%s stays prefetched", p) | |||
| 54 | + | } | |||
| 55 | + | } | |||
M internal/adapters/web/static/rules.json +9 -0
| @@ -7,6 +7,15 @@ | |||
| 7 | 7 | { "href_matches": "/*" }, | |
| 8 | 8 | { "not": { "href_matches": "/-/*" } }, | |
| 9 | 9 | { "not": { "href_matches": "/*/raw/*" } }, | |
| 10 | + | { "not": { "href_matches": "/*/tree" } }, | |
| 11 | + | { "not": { "href_matches": "/*/tree/*" } }, | |
| 12 | + | { "not": { "href_matches": "/*/blob/*" } }, | |
| 13 | + | { "not": { "href_matches": "/*/commit/*" } }, | |
| 14 | + | { "not": { "href_matches": "/*/commits" } }, | |
| 15 | + | { "not": { "href_matches": "/*/commits/*" } }, | |
| 16 | + | { "not": { "href_matches": "/*/compare/*" } }, | |
| 17 | + | { "not": { "href_matches": "/*/refs" } }, | |
| 18 | + | { "not": { "href_matches": "/*/merge-requests/*/since" } }, | |
| 10 | 19 | { "not": { "selector_matches": "[rel~=nofollow]" } } | |
| 11 | 20 | ] | |
| 12 | 21 | }, | |
| @@ -7,6 +7,15 @@ | |||||
| 7 | { "href_matches": "/*" }, | 7 | { "href_matches": "/*" }, | ||
| 8 | { "not": { "href_matches": "/-/*" } }, | 8 | { "not": { "href_matches": "/-/*" } }, | ||
| 9 | { "not": { "href_matches": "/*/raw/*" } }, | 9 | { "not": { "href_matches": "/*/raw/*" } }, | ||
| 10 | + | { "not": { "href_matches": "/*/tree" } }, | |||
| 11 | + | { "not": { "href_matches": "/*/tree/*" } }, | |||
| 12 | + | { "not": { "href_matches": "/*/blob/*" } }, | |||
| 13 | + | { "not": { "href_matches": "/*/commit/*" } }, | |||
| 14 | + | { "not": { "href_matches": "/*/commits" } }, | |||
| 15 | + | { "not": { "href_matches": "/*/commits/*" } }, | |||
| 16 | + | { "not": { "href_matches": "/*/compare/*" } }, | |||
| 17 | + | { "not": { "href_matches": "/*/refs" } }, | |||
| 18 | + | { "not": { "href_matches": "/*/merge-requests/*/since" } }, | |||
| 10 | { "not": { "selector_matches": "[rel~=nofollow]" } } | 19 | { "not": { "selector_matches": "[rel~=nofollow]" } } | ||
| 11 | ] | 20 | ] | ||
| 12 | }, | 21 | }, | ||
M internal/app/browse.go +2 -2
| @@ -65,7 +65,7 @@ func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain. | |||
| 65 | 65 | type ListRefsParams struct { | |
| 66 | 66 | // Kinds restricts the result; empty means every kind. | |
| 67 | 67 | Kinds []domain.RefKind | |
| 68 | - | // PageSize is 0 for the default, and lowered to MaxPageSize. | |
| 68 | + | // PageSize is 0 for the default, and lowered to MaxRefPageSize. | |
| 69 | 69 | PageSize int | |
| 70 | 70 | // PageToken is the NextPageToken of the previous page, or empty. | |
| 71 | 71 | PageToken string | |
| @@ -92,7 +92,7 @@ func kindRank(k domain.RefKind) int { | |||
| 92 | 92 | // name. The page token names the last ref of the previous page, so refs that | |
| 93 | 93 | // appear or disappear between two pages never shift the others. | |
| 94 | 94 | func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListRefsParams) (RefPage, error) { | |
| 95 | - | size, err := pageSize(params.PageSize) | |
| 95 | + | size, err := pageSizeUpTo(params.PageSize, MaxRefPageSize) | |
| 96 | 96 | if err != nil { | |
| 97 | 97 | return RefPage{}, err | |
| 98 | 98 | } | |
| @@ -65,7 +65,7 @@ func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain. | |||||
| 65 | type ListRefsParams struct { | 65 | type ListRefsParams struct { | ||
| 66 | // Kinds restricts the result; empty means every kind. | 66 | // Kinds restricts the result; empty means every kind. | ||
| 67 | Kinds []domain.RefKind | 67 | Kinds []domain.RefKind | ||
| 68 | - | // PageSize is 0 for the default, and lowered to MaxPageSize. | 68 | + | // PageSize is 0 for the default, and lowered to MaxRefPageSize. |
| 69 | PageSize int | 69 | PageSize int | ||
| 70 | // PageToken is the NextPageToken of the previous page, or empty. | 70 | // PageToken is the NextPageToken of the previous page, or empty. | ||
| 71 | PageToken string | 71 | PageToken string | ||
| @@ -92,7 +92,7 @@ func kindRank(k domain.RefKind) int { | |||||
| 92 | // name. The page token names the last ref of the previous page, so refs that | 92 | // name. The page token names the last ref of the previous page, so refs that | ||
| 93 | // appear or disappear between two pages never shift the others. | 93 | // appear or disappear between two pages never shift the others. | ||
| 94 | func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListRefsParams) (RefPage, error) { | 94 | func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.RepoName, params ListRefsParams) (RefPage, error) { | ||
| 95 | - | size, err := pageSize(params.PageSize) | 95 | + | size, err := pageSizeUpTo(params.PageSize, MaxRefPageSize) |
| 96 | if err != nil { | 96 | if err != nil { | ||
| 97 | return RefPage{}, err | 97 | return RefPage{}, err | ||
| 98 | } | 98 | } | ||
M internal/app/browse_test.go +4 -3
| @@ -241,8 +241,8 @@ func TestListRefsPagingAndKinds(t *testing.T) { | |||
| 241 | 241 | t.Run("default and maximum page size", func(t *testing.T) { | |
| 242 | 242 | var many []domain.Ref | |
| 243 | 243 | many = append(many, ref(domain.RefKindHead, "HEAD")) | |
| 244 | - | for i := range 250 { | |
| 245 | - | many = append(many, ref(domain.RefKindTag, fmt.Sprintf("refs/tags/t%03d", i))) | |
| 244 | + | for i := range app.MaxRefPageSize + 100 { | |
| 245 | + | many = append(many, ref(domain.RefKindTag, fmt.Sprintf("refs/tags/t%04d", i))) | |
| 246 | 246 | } | |
| 247 | 247 | b := setup(t, many) | |
| 248 | 248 | page, err := b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{}) | |
| @@ -253,7 +253,8 @@ func TestListRefsPagingAndKinds(t *testing.T) { | |||
| 253 | 253 | b = setup(t, many) | |
| 254 | 254 | page, err = b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{PageSize: 100000}) | |
| 255 | 255 | require.NoError(t, err) | |
| 256 | - | assert.Len(t, page.Refs, app.MaxPageSize) | |
| 256 | + | assert.Len(t, page.Refs, app.MaxRefPageSize, "a page holds up to MaxRefPageSize refs") | |
| 257 | + | assert.NotEmpty(t, page.NextPageToken) | |
| 257 | 258 | }) | |
| 258 | 259 | ||
| 259 | 260 | t.Run("bad input never reaches the reader", func(t *testing.T) { | |
| @@ -241,8 +241,8 @@ func TestListRefsPagingAndKinds(t *testing.T) { | |||||
| 241 | t.Run("default and maximum page size", func(t *testing.T) { | 241 | t.Run("default and maximum page size", func(t *testing.T) { | ||
| 242 | var many []domain.Ref | 242 | var many []domain.Ref | ||
| 243 | many = append(many, ref(domain.RefKindHead, "HEAD")) | 243 | many = append(many, ref(domain.RefKindHead, "HEAD")) | ||
| 244 | - | for i := range 250 { | 244 | + | for i := range app.MaxRefPageSize + 100 { |
| 245 | - | many = append(many, ref(domain.RefKindTag, fmt.Sprintf("refs/tags/t%03d", i))) | 245 | + | many = append(many, ref(domain.RefKindTag, fmt.Sprintf("refs/tags/t%04d", i))) |
| 246 | } | 246 | } | ||
| 247 | b := setup(t, many) | 247 | b := setup(t, many) | ||
| 248 | page, err := b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{}) | 248 | page, err := b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{}) | ||
| @@ -253,7 +253,8 @@ func TestListRefsPagingAndKinds(t *testing.T) { | |||||
| 253 | b = setup(t, many) | 253 | b = setup(t, many) | ||
| 254 | page, err = b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{PageSize: 100000}) | 254 | page, err = b.ListRefs(ctx, domain.Anonymous(), pub.Name, app.ListRefsParams{PageSize: 100000}) | ||
| 255 | require.NoError(t, err) | 255 | require.NoError(t, err) | ||
| 256 | - | assert.Len(t, page.Refs, app.MaxPageSize) | 256 | + | assert.Len(t, page.Refs, app.MaxRefPageSize, "a page holds up to MaxRefPageSize refs") |
| 257 | + | assert.NotEmpty(t, page.NextPageToken) | |||
| 257 | }) | 258 | }) | ||
| 258 | 259 | ||||
| 259 | t.Run("bad input never reaches the reader", func(t *testing.T) { | 260 | t.Run("bad input never reaches the reader", func(t *testing.T) { | ||
M internal/app/paging.go +15 -4
| @@ -27,6 +27,14 @@ const ( | |||
| 27 | 27 | DefaultPageSize = 50 | |
| 28 | 28 | // MaxPageSize is the largest page; a larger request is lowered to it. | |
| 29 | 29 | MaxPageSize = 100 | |
| 30 | + | // MaxRepoPageSize is the largest page of ListRepos. A repository list is | |
| 31 | + | // one scan of the metadata whatever the page, so a caller that wants the | |
| 32 | + | // whole list (the web pages) asks for it in one page rather than in | |
| 33 | + | // several scans. | |
| 34 | + | MaxRepoPageSize = 1000 | |
| 35 | + | // MaxRefPageSize is the largest page of ListRefs, for the same reason: | |
| 36 | + | // each page of refs runs the whole for-each-ref again. | |
| 37 | + | MaxRefPageSize = 1000 | |
| 30 | 38 | ) | |
| 31 | 39 | ||
| 32 | 40 | // MaxCommitSkip bounds how deep into a history a page token may point: each | |
| @@ -34,14 +42,17 @@ const ( | |||
| 34 | 42 | const MaxCommitSkip = 100_000 | |
| 35 | 43 | ||
| 36 | 44 | // pageSize applies the default and the maximum. | |
| 37 | - | func pageSize(n int) (int, error) { | |
| 45 | + | func pageSize(n int) (int, error) { return pageSizeUpTo(n, MaxPageSize) } | |
| 46 | + | ||
| 47 | + | // pageSizeUpTo applies the default and the given maximum. | |
| 48 | + | func pageSizeUpTo(n, most int) (int, error) { | |
| 38 | 49 | switch { | |
| 39 | 50 | case n < 0: | |
| 40 | 51 | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | |
| 41 | 52 | case n == 0: | |
| 42 | 53 | return DefaultPageSize, nil | |
| 43 | 54 | } | |
| 44 | - | return min(n, MaxPageSize), nil | |
| 55 | + | return min(n, most), nil | |
| 45 | 56 | } | |
| 46 | 57 | ||
| 47 | 58 | func decodeToken(tok, prefix string) (string, bool) { | |
| @@ -80,11 +91,11 @@ type RepoPage struct { | |||
| 80 | 91 | } | |
| 81 | 92 | ||
| 82 | 93 | // ListPage returns a page of the repositories the principal may read, ordered | |
| 83 | - | // by name. size is 0 for DefaultPageSize and lowered to MaxPageSize; the page | |
| 94 | + | // by name. size is 0 for DefaultPageSize and lowered to MaxRepoPageSize; the page | |
| 84 | 95 | // token names the last repository of the previous page, so repositories that | |
| 85 | 96 | // appear or disappear between two pages never shift the others. | |
| 86 | 97 | func (r *Repos) ListPage(ctx context.Context, p domain.Principal, size int, token string) (RepoPage, error) { | |
| 87 | - | n, err := pageSize(size) | |
| 98 | + | n, err := pageSizeUpTo(size, MaxRepoPageSize) | |
| 88 | 99 | if err != nil { | |
| 89 | 100 | return RepoPage{}, err | |
| 90 | 101 | } | |
| @@ -27,6 +27,14 @@ const ( | |||||
| 27 | DefaultPageSize = 50 | 27 | DefaultPageSize = 50 | ||
| 28 | // MaxPageSize is the largest page; a larger request is lowered to it. | 28 | // MaxPageSize is the largest page; a larger request is lowered to it. | ||
| 29 | MaxPageSize = 100 | 29 | MaxPageSize = 100 | ||
| 30 | + | // MaxRepoPageSize is the largest page of ListRepos. A repository list is | |||
| 31 | + | // one scan of the metadata whatever the page, so a caller that wants the | |||
| 32 | + | // whole list (the web pages) asks for it in one page rather than in | |||
| 33 | + | // several scans. | |||
| 34 | + | MaxRepoPageSize = 1000 | |||
| 35 | + | // MaxRefPageSize is the largest page of ListRefs, for the same reason: | |||
| 36 | + | // each page of refs runs the whole for-each-ref again. | |||
| 37 | + | MaxRefPageSize = 1000 | |||
| 30 | ) | 38 | ) | ||
| 31 | 39 | ||||
| 32 | // MaxCommitSkip bounds how deep into a history a page token may point: each | 40 | // MaxCommitSkip bounds how deep into a history a page token may point: each | ||
| @@ -34,14 +42,17 @@ const ( | |||||
| 34 | const MaxCommitSkip = 100_000 | 42 | const MaxCommitSkip = 100_000 | ||
| 35 | 43 | ||||
| 36 | // pageSize applies the default and the maximum. | 44 | // pageSize applies the default and the maximum. | ||
| 37 | - | func pageSize(n int) (int, error) { | 45 | + | func pageSize(n int) (int, error) { return pageSizeUpTo(n, MaxPageSize) } |
| 46 | + | ||||
| 47 | + | // pageSizeUpTo applies the default and the given maximum. | |||
| 48 | + | func pageSizeUpTo(n, most int) (int, error) { | |||
| 38 | switch { | 49 | switch { | ||
| 39 | case n < 0: | 50 | case n < 0: | ||
| 40 | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | 51 | return 0, fmt.Errorf("%w: %d", domain.ErrInvalidPageSize, n) | ||
| 41 | case n == 0: | 52 | case n == 0: | ||
| 42 | return DefaultPageSize, nil | 53 | return DefaultPageSize, nil | ||
| 43 | } | 54 | } | ||
| 44 | - | return min(n, MaxPageSize), nil | 55 | + | return min(n, most), nil |
| 45 | } | 56 | } | ||
| 46 | 57 | ||||
| 47 | func decodeToken(tok, prefix string) (string, bool) { | 58 | func decodeToken(tok, prefix string) (string, bool) { | ||
| @@ -80,11 +91,11 @@ type RepoPage struct { | |||||
| 80 | } | 91 | } | ||
| 81 | 92 | ||||
| 82 | // ListPage returns a page of the repositories the principal may read, ordered | 93 | // ListPage returns a page of the repositories the principal may read, ordered | ||
| 83 | - | // by name. size is 0 for DefaultPageSize and lowered to MaxPageSize; the page | 94 | + | // by name. size is 0 for DefaultPageSize and lowered to MaxRepoPageSize; the page |
| 84 | // token names the last repository of the previous page, so repositories that | 95 | // token names the last repository of the previous page, so repositories that | ||
| 85 | // appear or disappear between two pages never shift the others. | 96 | // appear or disappear between two pages never shift the others. | ||
| 86 | func (r *Repos) ListPage(ctx context.Context, p domain.Principal, size int, token string) (RepoPage, error) { | 97 | func (r *Repos) ListPage(ctx context.Context, p domain.Principal, size int, token string) (RepoPage, error) { | ||
| 87 | - | n, err := pageSize(size) | 98 | + | n, err := pageSizeUpTo(size, MaxRepoPageSize) |
| 88 | if err != nil { | 99 | if err != nil { | ||
| 89 | return RepoPage{}, err | 100 | return RepoPage{}, err | ||
| 90 | } | 101 | } | ||
M internal/app/repos_write_test.go +29 -0
| @@ -5,6 +5,7 @@ package app_test | |||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | 7 | "errors" | |
| 8 | + | "fmt" | |
| 8 | 9 | "strings" | |
| 9 | 10 | "testing" | |
| 10 | 11 | ||
| @@ -211,6 +212,34 @@ func TestView(t *testing.T) { | |||
| 211 | 212 | assert.Empty(t, v.DefaultBranch) | |
| 212 | 213 | } | |
| 213 | 214 | ||
| 215 | + | func TestListPageSizes(t *testing.T) { | |
| 216 | + | ctx := context.Background() | |
| 217 | + | var all []domain.Repo | |
| 218 | + | for i := range app.MaxRepoPageSize + 10 { | |
| 219 | + | all = append(all, domain.Repo{Name: name(t, fmt.Sprintf("r%04d", i)), Visibility: domain.VisibilityPublic}) | |
| 220 | + | } | |
| 221 | + | for _, tt := range []struct { | |
| 222 | + | name string | |
| 223 | + | size int | |
| 224 | + | want int | |
| 225 | + | more bool | |
| 226 | + | }{ | |
| 227 | + | {"default", 0, app.DefaultPageSize, true}, | |
| 228 | + | {"within the maximum", 500, 500, true}, | |
| 229 | + | {"the maximum", app.MaxRepoPageSize, app.MaxRepoPageSize, true}, | |
| 230 | + | {"lowered to the maximum", app.MaxRepoPageSize * 10, app.MaxRepoPageSize, true}, | |
| 231 | + | } { | |
| 232 | + | t.Run(tt.name, func(t *testing.T) { | |
| 233 | + | f := newFixture(t) | |
| 234 | + | f.meta.EXPECT().List(ctx).Return(all, nil) | |
| 235 | + | page, err := f.repos.ListPage(ctx, domain.Anonymous(), tt.size, "") | |
| 236 | + | require.NoError(t, err) | |
| 237 | + | assert.Len(t, page.Repos, tt.want) | |
| 238 | + | assert.Equal(t, tt.more, page.NextPageToken != "") | |
| 239 | + | }) | |
| 240 | + | } | |
| 241 | + | } | |
| 242 | + | ||
| 214 | 243 | func TestListPage(t *testing.T) { | |
| 215 | 244 | ctx := context.Background() | |
| 216 | 245 | var all []domain.Repo | |
| @@ -5,6 +5,7 @@ package app_test | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "errors" | 7 | "errors" | ||
| 8 | + | "fmt" | |||
| 8 | "strings" | 9 | "strings" | ||
| 9 | "testing" | 10 | "testing" | ||
| 10 | 11 | ||||
| @@ -211,6 +212,34 @@ func TestView(t *testing.T) { | |||||
| 211 | assert.Empty(t, v.DefaultBranch) | 212 | assert.Empty(t, v.DefaultBranch) | ||
| 212 | } | 213 | } | ||
| 213 | 214 | ||||
| 215 | + | func TestListPageSizes(t *testing.T) { | |||
| 216 | + | ctx := context.Background() | |||
| 217 | + | var all []domain.Repo | |||
| 218 | + | for i := range app.MaxRepoPageSize + 10 { | |||
| 219 | + | all = append(all, domain.Repo{Name: name(t, fmt.Sprintf("r%04d", i)), Visibility: domain.VisibilityPublic}) | |||
| 220 | + | } | |||
| 221 | + | for _, tt := range []struct { | |||
| 222 | + | name string | |||
| 223 | + | size int | |||
| 224 | + | want int | |||
| 225 | + | more bool | |||
| 226 | + | }{ | |||
| 227 | + | {"default", 0, app.DefaultPageSize, true}, | |||
| 228 | + | {"within the maximum", 500, 500, true}, | |||
| 229 | + | {"the maximum", app.MaxRepoPageSize, app.MaxRepoPageSize, true}, | |||
| 230 | + | {"lowered to the maximum", app.MaxRepoPageSize * 10, app.MaxRepoPageSize, true}, | |||
| 231 | + | } { | |||
| 232 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 233 | + | f := newFixture(t) | |||
| 234 | + | f.meta.EXPECT().List(ctx).Return(all, nil) | |||
| 235 | + | page, err := f.repos.ListPage(ctx, domain.Anonymous(), tt.size, "") | |||
| 236 | + | require.NoError(t, err) | |||
| 237 | + | assert.Len(t, page.Repos, tt.want) | |||
| 238 | + | assert.Equal(t, tt.more, page.NextPageToken != "") | |||
| 239 | + | }) | |||
| 240 | + | } | |||
| 241 | + | } | |||
| 242 | + | ||||
| 214 | func TestListPage(t *testing.T) { | 243 | func TestListPage(t *testing.T) { | ||
| 215 | ctx := context.Background() | 244 | ctx := context.Background() | ||
| 216 | var all []domain.Repo | 245 | var all []domain.Repo | ||