fix: key the history cache on repo identity, revert raw tree lookup, scope refs fanout to the server
- history cache keys include the repo creation time so a recreated repo never serves the deleted one's history - raw revalidation reads the blob id from the first GetBlob message again - gzip writers drop their buffer before returning to the pool - the refs fanout is a Server field; its test proves waiters block on one build - web tests use generated SessionAuth and Connect client mocks - startup warns when the per-client git cap runs without trusted proxies
- commit
- bde7c01895ada2b79d8e5777b0f2043588c16c95
- author
- brice · · 2026-10-11 20:47 UTC
- parent
- 8954329
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
18 files · +769 -286
M .mockery.yml +2 -0
| @@ -17,6 +17,8 @@ packages: | |||
| 17 | 17 | config: *mocks | |
| 18 | 18 | bnema.dev/ermite/internal/app: | |
| 19 | 19 | config: *mocks | |
| 20 | + | bnema.dev/ermite/internal/adapters/web: | |
| 21 | + | config: *mocks | |
| 20 | 22 | bnema.dev/ermite-api/gen/ermite/ermiteconnect: | |
| 21 | 23 | config: | |
| 22 | 24 | include-auto-generated: true | |
| @@ -17,6 +17,8 @@ packages: | |||||
| 17 | config: *mocks | 17 | config: *mocks | ||
| 18 | bnema.dev/ermite/internal/app: | 18 | bnema.dev/ermite/internal/app: | ||
| 19 | config: *mocks | 19 | config: *mocks | ||
| 20 | + | bnema.dev/ermite/internal/adapters/web: | |||
| 21 | + | config: *mocks | |||
| 20 | bnema.dev/ermite-api/gen/ermite/ermiteconnect: | 22 | bnema.dev/ermite-api/gen/ermite/ermiteconnect: | ||
| 21 | config: | 23 | config: | ||
| 22 | include-auto-generated: true | 24 | include-auto-generated: true | ||
M cmd/ermite/forge.go +1 -0
| @@ -240,6 +240,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e | |||
| 240 | 240 | routes = append(routes, httpserver.Route{Pattern: p, Handler: gh}) | |
| 241 | 241 | } | |
| 242 | 242 | warnSessionsNeedHTTPS(ctx, cfg.PublicURL) | |
| 243 | + | warnSharedClientKey(ctx, cfg.TrustedProxies, githttp.DefaultMaxPerClient) | |
| 243 | 244 | site, err := newWeb(cfg, f.API, f.Sessions, limiter) | |
| 244 | 245 | if err != nil { | |
| 245 | 246 | return nil, fmt.Errorf("web: %w", err) | |
| @@ -240,6 +240,7 @@ func newForge(ctx context.Context, cfg config.Config, opts options) (_ *forge, e | |||||
| 240 | routes = append(routes, httpserver.Route{Pattern: p, Handler: gh}) | 240 | routes = append(routes, httpserver.Route{Pattern: p, Handler: gh}) | ||
| 241 | } | 241 | } | ||
| 242 | warnSessionsNeedHTTPS(ctx, cfg.PublicURL) | 242 | warnSessionsNeedHTTPS(ctx, cfg.PublicURL) | ||
| 243 | + | warnSharedClientKey(ctx, cfg.TrustedProxies, githttp.DefaultMaxPerClient) | |||
| 243 | site, err := newWeb(cfg, f.API, f.Sessions, limiter) | 244 | site, err := newWeb(cfg, f.API, f.Sessions, limiter) | ||
| 244 | if err != nil { | 245 | if err != nil { | ||
| 245 | return nil, fmt.Errorf("web: %w", err) | 246 | return nil, fmt.Errorf("web: %w", err) | ||
M cmd/ermite/web.go +17 -0
| @@ -79,6 +79,23 @@ func warnSessionsNeedHTTPS(ctx context.Context, publicURL string) { | |||
| 79 | 79 | "and the session cookie is Secure (__Host-), so browsers drop it; serve the site over https") | |
| 80 | 80 | } | |
| 81 | 81 | ||
| 82 | + | // warnSharedClientKey tells the operator, at startup, that without trusted | |
| 83 | + | // proxies every request is attributed to the address of its TCP peer. Behind a | |
| 84 | + | // reverse proxy that is the proxy: all the clients then share one key, and so | |
| 85 | + | // one cap on the Git backends running at once (maxPerClient) and one budget of | |
| 86 | + | // failed authentications. The keying is unchanged; the fix is to list the | |
| 87 | + | // proxy networks. | |
| 88 | + | func warnSharedClientKey(ctx context.Context, trusted []netip.Prefix, maxPerClient int) { | |
| 89 | + | if len(trusted) > 0 || maxPerClient <= 0 { | |
| 90 | + | return | |
| 91 | + | } | |
| 92 | + | log := zerowrap.FromCtx(ctx) | |
| 93 | + | log.Warn().Int("max_git_backends_per_client", maxPerClient).Msg( | |
| 94 | + | config.EnvTrustedProxies + " is not set: clients are told apart by their TCP peer address only. " + | |
| 95 | + | "Behind a reverse proxy, all clients share the proxy's address, so one cap of concurrent Git " + | |
| 96 | + | "requests and one budget of failed authentications; set it to the proxy networks") | |
| 97 | + | } | |
| 98 | + | ||
| 82 | 99 | // webSessions adapts the session use cases to web.SessionAuth: a valid token | |
| 83 | 100 | // gives the context of the owner, whose principal the API calls of the page | |
| 84 | 101 | // carry (rpc.API.InProcessClient). | |
| @@ -79,6 +79,23 @@ func warnSessionsNeedHTTPS(ctx context.Context, publicURL string) { | |||||
| 79 | "and the session cookie is Secure (__Host-), so browsers drop it; serve the site over https") | 79 | "and the session cookie is Secure (__Host-), so browsers drop it; serve the site over https") | ||
| 80 | } | 80 | } | ||
| 81 | 81 | ||||
| 82 | + | // warnSharedClientKey tells the operator, at startup, that without trusted | |||
| 83 | + | // proxies every request is attributed to the address of its TCP peer. Behind a | |||
| 84 | + | // reverse proxy that is the proxy: all the clients then share one key, and so | |||
| 85 | + | // one cap on the Git backends running at once (maxPerClient) and one budget of | |||
| 86 | + | // failed authentications. The keying is unchanged; the fix is to list the | |||
| 87 | + | // proxy networks. | |||
| 88 | + | func warnSharedClientKey(ctx context.Context, trusted []netip.Prefix, maxPerClient int) { | |||
| 89 | + | if len(trusted) > 0 || maxPerClient <= 0 { | |||
| 90 | + | return | |||
| 91 | + | } | |||
| 92 | + | log := zerowrap.FromCtx(ctx) | |||
| 93 | + | log.Warn().Int("max_git_backends_per_client", maxPerClient).Msg( | |||
| 94 | + | config.EnvTrustedProxies + " is not set: clients are told apart by their TCP peer address only. " + | |||
| 95 | + | "Behind a reverse proxy, all clients share the proxy's address, so one cap of concurrent Git " + | |||
| 96 | + | "requests and one budget of failed authentications; set it to the proxy networks") | |||
| 97 | + | } | |||
| 98 | + | ||||
| 82 | // webSessions adapts the session use cases to web.SessionAuth: a valid token | 99 | // webSessions adapts the session use cases to web.SessionAuth: a valid token | ||
| 83 | // gives the context of the owner, whose principal the API calls of the page | 100 | // gives the context of the owner, whose principal the API calls of the page | ||
| 84 | // carry (rpc.API.InProcessClient). | 101 | // carry (rpc.API.InProcessClient). | ||
M cmd/ermite/web_session_test.go +30 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | "context" | |
| 7 | 7 | "io" | |
| 8 | 8 | "net/http" | |
| 9 | + | "net/netip" | |
| 9 | 10 | "net/url" | |
| 10 | 11 | "strings" | |
| 11 | 12 | "testing" | |
| @@ -558,3 +559,32 @@ func TestStartupWarnsWhenSignInCannotWork(t *testing.T) { | |||
| 558 | 559 | warnSessionsNeedHTTPS(ctx, "http://forge.example") | |
| 559 | 560 | assert.Contains(t, buf.String(), "ERMITE_PUBLIC_URL") | |
| 560 | 561 | } | |
| 562 | + | ||
| 563 | + | func TestWarnSharedClientKey(t *testing.T) { | |
| 564 | + | t.Parallel() | |
| 565 | + | proxies := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} | |
| 566 | + | tests := []struct { | |
| 567 | + | name string | |
| 568 | + | trusted []netip.Prefix | |
| 569 | + | cap int | |
| 570 | + | warns bool | |
| 571 | + | }{ | |
| 572 | + | {"no proxy, cap active", nil, 4, true}, | |
| 573 | + | {"proxies trusted", proxies, 4, false}, | |
| 574 | + | {"no proxy, no cap", nil, 0, false}, | |
| 575 | + | } | |
| 576 | + | for _, tt := range tests { | |
| 577 | + | t.Run(tt.name, func(t *testing.T) { | |
| 578 | + | t.Parallel() | |
| 579 | + | var buf syncBuffer | |
| 580 | + | ctx := zerowrap.WithCtx(context.Background(), zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: &buf})) | |
| 581 | + | warnSharedClientKey(ctx, tt.trusted, tt.cap) | |
| 582 | + | if !tt.warns { | |
| 583 | + | assert.Empty(t, buf.String()) | |
| 584 | + | return | |
| 585 | + | } | |
| 586 | + | assert.Contains(t, buf.String(), `"level":"warn"`) | |
| 587 | + | assert.Contains(t, buf.String(), "ERMITE_TRUSTED_PROXIES") | |
| 588 | + | }) | |
| 589 | + | } | |
| 590 | + | } | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | "context" | 6 | "context" | ||
| 7 | "io" | 7 | "io" | ||
| 8 | "net/http" | 8 | "net/http" | ||
| 9 | + | "net/netip" | |||
| 9 | "net/url" | 10 | "net/url" | ||
| 10 | "strings" | 11 | "strings" | ||
| 11 | "testing" | 12 | "testing" | ||
| @@ -558,3 +559,32 @@ func TestStartupWarnsWhenSignInCannotWork(t *testing.T) { | |||||
| 558 | warnSessionsNeedHTTPS(ctx, "http://forge.example") | 559 | warnSessionsNeedHTTPS(ctx, "http://forge.example") | ||
| 559 | assert.Contains(t, buf.String(), "ERMITE_PUBLIC_URL") | 560 | assert.Contains(t, buf.String(), "ERMITE_PUBLIC_URL") | ||
| 560 | } | 561 | } | ||
| 562 | + | ||||
| 563 | + | func TestWarnSharedClientKey(t *testing.T) { | |||
| 564 | + | t.Parallel() | |||
| 565 | + | proxies := []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")} | |||
| 566 | + | tests := []struct { | |||
| 567 | + | name string | |||
| 568 | + | trusted []netip.Prefix | |||
| 569 | + | cap int | |||
| 570 | + | warns bool | |||
| 571 | + | }{ | |||
| 572 | + | {"no proxy, cap active", nil, 4, true}, | |||
| 573 | + | {"proxies trusted", proxies, 4, false}, | |||
| 574 | + | {"no proxy, no cap", nil, 0, false}, | |||
| 575 | + | } | |||
| 576 | + | for _, tt := range tests { | |||
| 577 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 578 | + | t.Parallel() | |||
| 579 | + | var buf syncBuffer | |||
| 580 | + | ctx := zerowrap.WithCtx(context.Background(), zerowrap.New(zerowrap.Config{Level: "info", Format: "json", Output: &buf})) | |||
| 581 | + | warnSharedClientKey(ctx, tt.trusted, tt.cap) | |||
| 582 | + | if !tt.warns { | |||
| 583 | + | assert.Empty(t, buf.String()) | |||
| 584 | + | return | |||
| 585 | + | } | |||
| 586 | + | assert.Contains(t, buf.String(), `"level":"warn"`) | |||
| 587 | + | assert.Contains(t, buf.String(), "ERMITE_TRUSTED_PROXIES") | |||
| 588 | + | }) | |||
| 589 | + | } | |||
| 590 | + | } | |||
M internal/adapters/rpc/api.go +4 -1
| @@ -49,9 +49,12 @@ type Config struct { | |||
| 49 | 49 | TrustedProxies []netip.Prefix | |
| 50 | 50 | // Limiter counts failed authentications; nil builds a default one. Share | |
| 51 | 51 | // it with the other adapters that authenticate, so that a client has one | |
| 52 | - | // budget of failures. | |
| 52 | + | // budget of failures. Leave it unset rather than assigning a typed nil | |
| 53 | + | // pointer (a nil *T in the interface is not nil, and would be called). | |
| 53 | 54 | Limiter FailureLimiter | |
| 54 | 55 | // Sessions are the owner's web sessions; nil serves no SessionService. | |
| 56 | + | // Leave it unset rather than assigning a typed nil pointer (a nil *T in | |
| 57 | + | // the interface is not nil, and the service would be served and called). | |
| 55 | 58 | Sessions Sessions | |
| 56 | 59 | } | |
| 57 | 60 | ||
| @@ -49,9 +49,12 @@ type Config struct { | |||||
| 49 | TrustedProxies []netip.Prefix | 49 | TrustedProxies []netip.Prefix | ||
| 50 | // Limiter counts failed authentications; nil builds a default one. Share | 50 | // Limiter counts failed authentications; nil builds a default one. Share | ||
| 51 | // it with the other adapters that authenticate, so that a client has one | 51 | // it with the other adapters that authenticate, so that a client has one | ||
| 52 | - | // budget of failures. | 52 | + | // budget of failures. Leave it unset rather than assigning a typed nil |
| 53 | + | // pointer (a nil *T in the interface is not nil, and would be called). | |||
| 53 | Limiter FailureLimiter | 54 | Limiter FailureLimiter | ||
| 54 | // Sessions are the owner's web sessions; nil serves no SessionService. | 55 | // Sessions are the owner's web sessions; nil serves no SessionService. | ||
| 56 | + | // Leave it unset rather than assigning a typed nil pointer (a nil *T in | |||
| 57 | + | // the interface is not nil, and the service would be served and called). | |||
| 55 | Sessions Sessions | 58 | Sessions Sessions | ||
| 56 | } | 59 | } | ||
| 57 | 60 | ||||
M internal/adapters/web/compress.go +7 -1
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "compress/gzip" | |
| 8 | 8 | "crypto/sha256" | |
| 9 | 9 | "encoding/hex" | |
| 10 | + | "io" | |
| 10 | 11 | "net/http" | |
| 11 | 12 | "strconv" | |
| 12 | 13 | "strings" | |
| @@ -33,7 +34,12 @@ var gzipWriters = sync.Pool{New: func() any { | |||
| 33 | 34 | func gzipPage(b []byte) ([]byte, error) { | |
| 34 | 35 | var out bytes.Buffer | |
| 35 | 36 | zw := gzipWriters.Get().(*gzip.Writer) | |
| 36 | - | defer gzipWriters.Put(zw) | |
| 37 | + | defer func() { | |
| 38 | + | // Drop the reference to out, so a pooled writer does not keep the | |
| 39 | + | // last page alive. | |
| 40 | + | zw.Reset(io.Discard) | |
| 41 | + | gzipWriters.Put(zw) | |
| 42 | + | }() | |
| 37 | 43 | zw.Reset(&out) | |
| 38 | 44 | if _, err := zw.Write(b); err != nil { | |
| 39 | 45 | return nil, err | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "compress/gzip" | 7 | "compress/gzip" | ||
| 8 | "crypto/sha256" | 8 | "crypto/sha256" | ||
| 9 | "encoding/hex" | 9 | "encoding/hex" | ||
| 10 | + | "io" | |||
| 10 | "net/http" | 11 | "net/http" | ||
| 11 | "strconv" | 12 | "strconv" | ||
| 12 | "strings" | 13 | "strings" | ||
| @@ -33,7 +34,12 @@ var gzipWriters = sync.Pool{New: func() any { | |||||
| 33 | func gzipPage(b []byte) ([]byte, error) { | 34 | func gzipPage(b []byte) ([]byte, error) { | ||
| 34 | var out bytes.Buffer | 35 | var out bytes.Buffer | ||
| 35 | zw := gzipWriters.Get().(*gzip.Writer) | 36 | zw := gzipWriters.Get().(*gzip.Writer) | ||
| 36 | - | defer gzipWriters.Put(zw) | 37 | + | defer func() { |
| 38 | + | // Drop the reference to out, so a pooled writer does not keep the | |||
| 39 | + | // last page alive. | |||
| 40 | + | zw.Reset(io.Discard) | |||
| 41 | + | gzipWriters.Put(zw) | |||
| 42 | + | }() | |||
| 37 | zw.Reset(&out) | 43 | zw.Reset(&out) | ||
| 38 | if _, err := zw.Write(b); err != nil { | 44 | if _, err := zw.Write(b); err != nil { | ||
| 39 | return nil, err | 45 | return nil, err | ||
M internal/adapters/web/events_test.go +12 -14
| @@ -20,6 +20,7 @@ import ( | |||
| 20 | 20 | ||
| 21 | 21 | pb "bnema.dev/ermite-api/gen/ermite" | |
| 22 | 22 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 23 | + | "bnema.dev/ermite/internal/adapters/web/mocks" | |
| 23 | 24 | ) | |
| 24 | 25 | ||
| 25 | 26 | func TestFormatEvent(t *testing.T) { | |
| @@ -293,15 +294,12 @@ func limiterFor(sess SessionAuth) FailureLimiter { | |||
| 293 | 294 | return &countingLimiter{} | |
| 294 | 295 | } | |
| 295 | 296 | ||
| 296 | - | // revocable is a SessionAuth whose one session ends when the test says so, like | |
| 297 | - | // SignOutEverywhere does. | |
| 298 | - | type revocable struct { | |
| 299 | - | fakeSessions | |
| 300 | - | ended atomic.Bool | |
| 301 | - | } | |
| 302 | - | ||
| 303 | - | func (r *revocable) Verify(ctx context.Context, tok string) (context.Context, bool, error) { | |
| 304 | - | return ctx, tok == r.valid && !r.ended.Load(), nil | |
| 297 | + | // newRevocable is a SessionAuth whose one session ends when the test sets | |
| 298 | + | // ended, like SignOutEverywhere does. | |
| 299 | + | func newRevocable(t *testing.T, valid string) (*mocks.SessionAuth, *atomic.Bool) { | |
| 300 | + | t.Helper() | |
| 301 | + | var ended atomic.Bool | |
| 302 | + | return newVerifier(t, valid, func(tok string) (bool, error) { return tok == valid && !ended.Load(), nil }), &ended | |
| 305 | 303 | } | |
| 306 | 304 | ||
| 307 | 305 | // GO-002: the stream of the owner is checked before each event is sent, not | |
| @@ -310,7 +308,7 @@ func (r *revocable) Verify(ctx context.Context, tok string) (context.Context, bo | |||
| 310 | 308 | func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | |
| 311 | 309 | t.Parallel() | |
| 312 | 310 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | |
| 313 | - | sess := &revocable{fakeSessions: fakeSessions{valid: "tok"}} | |
| 311 | + | sess, ended := newRevocable(t, "tok") | |
| 314 | 312 | ts := eventsServerWith(t, f, time.Hour, 0, sess) // no heartbeat in the test | |
| 315 | 313 | resp, cancel := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=tok"}) | |
| 316 | 314 | require.Equal(t, http.StatusOK, resp.StatusCode) | |
| @@ -338,7 +336,7 @@ func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | |||
| 338 | 336 | assert.Contains(t, seen, "id: b-1") | |
| 339 | 337 | ||
| 340 | 338 | // Signed out everywhere: the next event is not sent, the stream ends. | |
| 341 | - | sess.ended.Store(true) | |
| 339 | + | ended.Store(true) | |
| 342 | 340 | select { | |
| 343 | 341 | case f.push <- &pb.Event{Id: "b-2", Kind: &pb.Event_Gap{Gap: &pb.Gap{}}}: | |
| 344 | 342 | case <-time.After(5 * time.Second): | |
| @@ -353,8 +351,8 @@ func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | |||
| 353 | 351 | func TestEventsVisitorStreamIgnoresSessions(t *testing.T) { | |
| 354 | 352 | t.Parallel() | |
| 355 | 353 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | |
| 356 | - | sess := &revocable{fakeSessions: fakeSessions{valid: "tok"}} | |
| 357 | - | sess.ended.Store(true) | |
| 354 | + | sess, ended := newRevocable(t, "tok") | |
| 355 | + | ended.Store(true) | |
| 358 | 356 | ts := eventsServerWith(t, f, time.Hour, 0, sess) | |
| 359 | 357 | resp, _ := open(t, ts, "topic=home", nil) | |
| 360 | 358 | require.Equal(t, http.StatusOK, resp.StatusCode) | |
| @@ -375,7 +373,7 @@ func TestEventsVisitorStreamIgnoresSessions(t *testing.T) { | |||
| 375 | 373 | func TestEventsInvalidCookieStream(t *testing.T) { | |
| 376 | 374 | t.Parallel() | |
| 377 | 375 | f := &fakeEvents{release: make(chan struct{})} | |
| 378 | - | ts := eventsServerWith(t, f, time.Hour, 0, &fakeSessions{valid: "tok"}) | |
| 376 | + | ts := eventsServerWith(t, f, time.Hour, 0, newSessions(t, "tok")) | |
| 379 | 377 | resp, _ := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=bad"}) | |
| 380 | 378 | require.Equal(t, http.StatusOK, resp.StatusCode) | |
| 381 | 379 | assert.Equal(t, "no-store, no-transform", resp.Header.Get("Cache-Control")) | |
| @@ -20,6 +20,7 @@ import ( | |||||
| 20 | 20 | ||||
| 21 | pb "bnema.dev/ermite-api/gen/ermite" | 21 | pb "bnema.dev/ermite-api/gen/ermite" | ||
| 22 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | 22 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | ||
| 23 | + | "bnema.dev/ermite/internal/adapters/web/mocks" | |||
| 23 | ) | 24 | ) | ||
| 24 | 25 | ||||
| 25 | func TestFormatEvent(t *testing.T) { | 26 | func TestFormatEvent(t *testing.T) { | ||
| @@ -293,15 +294,12 @@ func limiterFor(sess SessionAuth) FailureLimiter { | |||||
| 293 | return &countingLimiter{} | 294 | return &countingLimiter{} | ||
| 294 | } | 295 | } | ||
| 295 | 296 | ||||
| 296 | - | // revocable is a SessionAuth whose one session ends when the test says so, like | 297 | + | // newRevocable is a SessionAuth whose one session ends when the test sets |
| 297 | - | // SignOutEverywhere does. | 298 | + | // ended, like SignOutEverywhere does. |
| 298 | - | type revocable struct { | 299 | + | func newRevocable(t *testing.T, valid string) (*mocks.SessionAuth, *atomic.Bool) { |
| 299 | - | fakeSessions | 300 | + | t.Helper() |
| 300 | - | ended atomic.Bool | 301 | + | var ended atomic.Bool |
| 301 | - | } | 302 | + | return newVerifier(t, valid, func(tok string) (bool, error) { return tok == valid && !ended.Load(), nil }), &ended |
| 302 | - | ||||
| 303 | - | func (r *revocable) Verify(ctx context.Context, tok string) (context.Context, bool, error) { | |||
| 304 | - | return ctx, tok == r.valid && !r.ended.Load(), nil | |||
| 305 | } | 303 | } | ||
| 306 | 304 | ||||
| 307 | // GO-002: the stream of the owner is checked before each event is sent, not | 305 | // GO-002: the stream of the owner is checked before each event is sent, not | ||
| @@ -310,7 +308,7 @@ func (r *revocable) Verify(ctx context.Context, tok string) (context.Context, bo | |||||
| 310 | func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | 308 | func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | ||
| 311 | t.Parallel() | 309 | t.Parallel() | ||
| 312 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | 310 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | ||
| 313 | - | sess := &revocable{fakeSessions: fakeSessions{valid: "tok"}} | 311 | + | sess, ended := newRevocable(t, "tok") |
| 314 | ts := eventsServerWith(t, f, time.Hour, 0, sess) // no heartbeat in the test | 312 | ts := eventsServerWith(t, f, time.Hour, 0, sess) // no heartbeat in the test | ||
| 315 | resp, cancel := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=tok"}) | 313 | resp, cancel := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=tok"}) | ||
| 316 | require.Equal(t, http.StatusOK, resp.StatusCode) | 314 | require.Equal(t, http.StatusOK, resp.StatusCode) | ||
| @@ -338,7 +336,7 @@ func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | |||||
| 338 | assert.Contains(t, seen, "id: b-1") | 336 | assert.Contains(t, seen, "id: b-1") | ||
| 339 | 337 | ||||
| 340 | // Signed out everywhere: the next event is not sent, the stream ends. | 338 | // Signed out everywhere: the next event is not sent, the stream ends. | ||
| 341 | - | sess.ended.Store(true) | 339 | + | ended.Store(true) |
| 342 | select { | 340 | select { | ||
| 343 | case f.push <- &pb.Event{Id: "b-2", Kind: &pb.Event_Gap{Gap: &pb.Gap{}}}: | 341 | case f.push <- &pb.Event{Id: "b-2", Kind: &pb.Event_Gap{Gap: &pb.Gap{}}}: | ||
| 344 | case <-time.After(5 * time.Second): | 342 | case <-time.After(5 * time.Second): | ||
| @@ -353,8 +351,8 @@ func TestEventsOwnerStreamEndsBeforeNextEvent(t *testing.T) { | |||||
| 353 | func TestEventsVisitorStreamIgnoresSessions(t *testing.T) { | 351 | func TestEventsVisitorStreamIgnoresSessions(t *testing.T) { | ||
| 354 | t.Parallel() | 352 | t.Parallel() | ||
| 355 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | 353 | f := &fakeEvents{release: make(chan struct{}), push: make(chan *pb.Event)} | ||
| 356 | - | sess := &revocable{fakeSessions: fakeSessions{valid: "tok"}} | 354 | + | sess, ended := newRevocable(t, "tok") |
| 357 | - | sess.ended.Store(true) | 355 | + | ended.Store(true) |
| 358 | ts := eventsServerWith(t, f, time.Hour, 0, sess) | 356 | ts := eventsServerWith(t, f, time.Hour, 0, sess) | ||
| 359 | resp, _ := open(t, ts, "topic=home", nil) | 357 | resp, _ := open(t, ts, "topic=home", nil) | ||
| 360 | require.Equal(t, http.StatusOK, resp.StatusCode) | 358 | require.Equal(t, http.StatusOK, resp.StatusCode) | ||
| @@ -375,7 +373,7 @@ func TestEventsVisitorStreamIgnoresSessions(t *testing.T) { | |||||
| 375 | func TestEventsInvalidCookieStream(t *testing.T) { | 373 | func TestEventsInvalidCookieStream(t *testing.T) { | ||
| 376 | t.Parallel() | 374 | t.Parallel() | ||
| 377 | f := &fakeEvents{release: make(chan struct{})} | 375 | f := &fakeEvents{release: make(chan struct{})} | ||
| 378 | - | ts := eventsServerWith(t, f, time.Hour, 0, &fakeSessions{valid: "tok"}) | 376 | + | ts := eventsServerWith(t, f, time.Hour, 0, newSessions(t, "tok")) |
| 379 | resp, _ := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=bad"}) | 377 | resp, _ := open(t, ts, "topic=home", map[string]string{"Cookie": CookieName + "=bad"}) | ||
| 380 | require.Equal(t, http.StatusOK, resp.StatusCode) | 378 | require.Equal(t, http.StatusOK, resp.StatusCode) | ||
| 381 | assert.Equal(t, "no-store, no-transform", resp.Header.Get("Cache-Control")) | 379 | assert.Equal(t, "no-store, no-transform", resp.Header.Get("Cache-Control")) | ||
M internal/adapters/web/fanout_test.go +85 -93
| @@ -5,37 +5,35 @@ package web | |||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | 7 | "errors" | |
| 8 | - | "net/http" | |
| 9 | - | "net/http/httptest" | |
| 10 | 8 | "sync" | |
| 11 | 9 | "sync/atomic" | |
| 12 | 10 | "testing" | |
| 11 | + | "testing/synctest" | |
| 13 | 12 | ||
| 14 | 13 | "connectrpc.com/connect" | |
| 15 | 14 | "github.com/stretchr/testify/assert" | |
| 15 | + | "github.com/stretchr/testify/mock" | |
| 16 | 16 | "github.com/stretchr/testify/require" | |
| 17 | 17 | ||
| 18 | 18 | pb "bnema.dev/ermite-api/gen/ermite" | |
| 19 | - | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 19 | + | "bnema.dev/ermite/internal/adapters/web/mocks" | |
| 20 | 20 | ) | |
| 21 | 21 | ||
| 22 | 22 | func frag(s string) []fragment { return []fragment{{event: "refs-list", html: s}} } | |
| 23 | 23 | ||
| 24 | 24 | func TestFanoutBuildsOncePerKey(t *testing.T) { | |
| 25 | 25 | t.Parallel() | |
| 26 | - | srvA, srvB := &Server{}, &Server{} | |
| 27 | - | base := fanKey{srv: srvA, repo: "x", event: "b-1"} | |
| 26 | + | base := fanKey{repo: "x", event: "b-1"} | |
| 28 | 27 | for _, tt := range []struct { | |
| 29 | 28 | name string | |
| 30 | 29 | keys []fanKey | |
| 31 | 30 | built int32 | |
| 32 | 31 | }{ | |
| 33 | 32 | {"same key", []fanKey{base, base, base}, 1}, | |
| 34 | - | {"owner and anonymous never share", []fanKey{base, {srv: srvA, repo: "x", event: "b-1", owner: true}}, 2}, | |
| 35 | - | {"another event", []fanKey{base, {srv: srvA, repo: "x", event: "b-2"}}, 2}, | |
| 36 | - | {"another repository", []fanKey{base, {srv: srvA, repo: "y", event: "b-1"}}, 2}, | |
| 37 | - | {"another server", []fanKey{base, {srv: srvB, repo: "x", event: "b-1"}}, 2}, | |
| 38 | - | {"no event id: nothing to share by", []fanKey{{srv: srvA, repo: "x"}, {srv: srvA, repo: "x"}}, 2}, | |
| 33 | + | {"owner and anonymous never share", []fanKey{base, {repo: "x", event: "b-1", owner: true}}, 2}, | |
| 34 | + | {"another event", []fanKey{base, {repo: "x", event: "b-2"}}, 2}, | |
| 35 | + | {"another repository", []fanKey{base, {repo: "y", event: "b-1"}}, 2}, | |
| 36 | + | {"no event id: nothing to share by", []fanKey{{repo: "x"}, {repo: "x"}}, 2}, | |
| 39 | 37 | } { | |
| 40 | 38 | t.Run(tt.name, func(t *testing.T) { | |
| 41 | 39 | t.Parallel() | |
| @@ -55,37 +53,51 @@ func TestFanoutBuildsOncePerKey(t *testing.T) { | |||
| 55 | 53 | } | |
| 56 | 54 | } | |
| 57 | 55 | ||
| 56 | + | // While the first watcher builds, every other one waits for it: in a synctest | |
| 57 | + | // bubble Wait returns once all goroutines are blocked, so the count of the | |
| 58 | + | // watchers that entered and none that returned proves the others wait on the | |
| 59 | + | // build instead of making their own. | |
| 58 | 60 | func TestFanoutConcurrentWatchersShareOneBuild(t *testing.T) { | |
| 59 | 61 | t.Parallel() | |
| 60 | - | var f fanout | |
| 61 | - | var built atomic.Int32 | |
| 62 | - | release := make(chan struct{}) | |
| 63 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 64 | - | var wg sync.WaitGroup | |
| 65 | - | results := make([][]fragment, 50) | |
| 66 | - | for i := range results { | |
| 67 | - | wg.Add(1) | |
| 68 | - | go func() { | |
| 69 | - | defer wg.Done() | |
| 70 | - | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { | |
| 71 | - | built.Add(1) | |
| 72 | - | <-release | |
| 73 | - | return frag("once"), nil | |
| 74 | - | }) | |
| 75 | - | }() | |
| 76 | - | } | |
| 77 | - | close(release) | |
| 78 | - | wg.Wait() | |
| 79 | - | assert.Equal(t, int32(1), built.Load()) | |
| 80 | - | for _, r := range results { | |
| 81 | - | assert.Equal(t, frag("once"), r) | |
| 82 | - | } | |
| 62 | + | const watchers = 50 | |
| 63 | + | synctest.Test(t, func(t *testing.T) { | |
| 64 | + | var f fanout | |
| 65 | + | var built, entered, returned atomic.Int32 | |
| 66 | + | release := make(chan struct{}) | |
| 67 | + | key := fanKey{repo: "x", event: "b-1"} | |
| 68 | + | var wg sync.WaitGroup | |
| 69 | + | results := make([][]fragment, watchers) | |
| 70 | + | for i := range results { | |
| 71 | + | wg.Add(1) | |
| 72 | + | go func() { | |
| 73 | + | defer wg.Done() | |
| 74 | + | entered.Add(1) | |
| 75 | + | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { | |
| 76 | + | built.Add(1) | |
| 77 | + | <-release | |
| 78 | + | return frag("once"), nil | |
| 79 | + | }) | |
| 80 | + | returned.Add(1) | |
| 81 | + | }() | |
| 82 | + | } | |
| 83 | + | synctest.Wait() // the builder is on release, the others on its entry | |
| 84 | + | require.Equal(t, int32(watchers), entered.Load()) | |
| 85 | + | require.Equal(t, int32(1), built.Load(), "one builder") | |
| 86 | + | require.Zero(t, returned.Load(), "the other %d wait for it", watchers-1) | |
| 87 | + | ||
| 88 | + | close(release) | |
| 89 | + | wg.Wait() | |
| 90 | + | assert.Equal(t, int32(1), built.Load()) | |
| 91 | + | for _, r := range results { | |
| 92 | + | assert.Equal(t, frag("once"), r) | |
| 93 | + | } | |
| 94 | + | }) | |
| 83 | 95 | } | |
| 84 | 96 | ||
| 85 | 97 | func TestFanoutFailureIsNotKept(t *testing.T) { | |
| 86 | 98 | t.Parallel() | |
| 87 | 99 | var f fanout | |
| 88 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 100 | + | key := fanKey{repo: "x", event: "b-1"} | |
| 89 | 101 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return nil, errors.New("boom") }) | |
| 90 | 102 | require.Error(t, err) | |
| 91 | 103 | got, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("ok"), nil }) | |
| @@ -96,7 +108,7 @@ func TestFanoutFailureIsNotKept(t *testing.T) { | |||
| 96 | 108 | func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | |
| 97 | 109 | t.Parallel() | |
| 98 | 110 | var f fanout | |
| 99 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | |
| 111 | + | key := fanKey{repo: "x", event: "b-1"} | |
| 100 | 112 | started, release := make(chan struct{}), make(chan struct{}) | |
| 101 | 113 | go func() { | |
| 102 | 114 | _, _ = f.do(context.Background(), key, func() ([]fragment, error) { | |
| @@ -116,56 +128,42 @@ func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | |||
| 116 | 128 | func TestFanoutIsBounded(t *testing.T) { | |
| 117 | 129 | t.Parallel() | |
| 118 | 130 | var f fanout | |
| 119 | - | srv := &Server{} | |
| 120 | 131 | for i := range fanMax + 10 { | |
| 121 | - | key := fanKey{srv: srv, repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} | |
| 132 | + | key := fanKey{repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} | |
| 122 | 133 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("x"), nil }) | |
| 123 | 134 | require.NoError(t, err) | |
| 124 | 135 | } | |
| 125 | 136 | assert.LessOrEqual(t, len(f.m), fanMax) | |
| 126 | 137 | } | |
| 127 | 138 | ||
| 128 | - | // refsAPI counts the calls the refs fragment makes to the API. | |
| 129 | - | type refsAPI struct { | |
| 130 | - | ermiteconnect.UnimplementedRepoServiceHandler | |
| 131 | - | ermiteconnect.UnimplementedGitServiceHandler | |
| 132 | - | repo, refs, commits atomic.Int32 | |
| 133 | - | pageSizes []int32 | |
| 134 | - | mu sync.Mutex | |
| 135 | - | } | |
| 136 | - | ||
| 137 | - | func (a *refsAPI) GetRepo(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { | |
| 138 | - | a.repo.Add(1) | |
| 139 | - | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil | |
| 140 | - | } | |
| 141 | - | ||
| 142 | - | func (a *refsAPI) ListRefs(_ context.Context, req *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |
| 143 | - | a.refs.Add(1) | |
| 144 | - | a.mu.Lock() | |
| 145 | - | a.pageSizes = append(a.pageSizes, req.Msg.GetPageSize()) | |
| 146 | - | a.mu.Unlock() | |
| 147 | - | id := "0123456789abcdef0123456789abcdef01234567" | |
| 148 | - | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |
| 149 | - | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |
| 150 | - | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |
| 151 | - | }}), nil | |
| 152 | - | } | |
| 153 | - | ||
| 154 | - | func (a *refsAPI) ListCommits(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |
| 155 | - | a.commits.Add(1) | |
| 156 | - | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |
| 157 | - | } | |
| 158 | - | ||
| 159 | - | func refsServer(t *testing.T, api *refsAPI) *Server { | |
| 139 | + | // refsMocks is a web server over mocks that expect the refs fragment to load | |
| 140 | + | // the repository and its refs repos times, and to ask for a commit commits | |
| 141 | + | // times. | |
| 142 | + | func refsMocks(t *testing.T, repos, commits int) *Server { | |
| 160 | 143 | t.Helper() | |
| 161 | - | mux := http.NewServeMux() | |
| 162 | - | mux.Handle(ermiteconnect.NewRepoServiceHandler(api)) | |
| 163 | - | mux.Handle(ermiteconnect.NewGitServiceHandler(api)) | |
| 164 | - | backend := httptest.NewServer(mux) | |
| 165 | - | t.Cleanup(backend.Close) | |
| 144 | + | api := mocks.NewRepoServiceClient(t) | |
| 145 | + | git := mocks.NewGitServiceClient(t) | |
| 146 | + | api.EXPECT().GetRepo(mock.Anything, mock.Anything).RunAndReturn( | |
| 147 | + | func(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { | |
| 148 | + | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil | |
| 149 | + | }).Times(repos) | |
| 150 | + | id := "0123456789abcdef0123456789abcdef01234567" | |
| 151 | + | git.EXPECT().ListRefs(mock.Anything, mock.MatchedBy(func(req *connect.Request[pb.ListRefsRequest]) bool { | |
| 152 | + | return req.Msg.GetPageSize() == maxRefs // the refs are asked in one page | |
| 153 | + | })).RunAndReturn(func(context.Context, *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |
| 154 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |
| 155 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |
| 156 | + | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |
| 157 | + | }}), nil | |
| 158 | + | }).Times(repos) | |
| 159 | + | if commits > 0 { | |
| 160 | + | git.EXPECT().ListCommits(mock.Anything, mock.Anything).RunAndReturn( | |
| 161 | + | func(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |
| 162 | + | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |
| 163 | + | }).Times(commits) | |
| 164 | + | } | |
| 166 | 165 | s, err := New(Config{ | |
| 167 | - | Repos: ermiteconnect.NewRepoServiceClient(backend.Client(), backend.URL), | |
| 168 | - | Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), | |
| 166 | + | Repos: api, Git: git, | |
| 169 | 167 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | |
| 170 | 168 | }) | |
| 171 | 169 | require.NoError(t, err) | |
| @@ -175,23 +173,22 @@ func refsServer(t *testing.T, api *refsAPI) *Server { | |||
| 175 | 173 | // A push is rendered once for the watchers of the same class, and the owner | |
| 176 | 174 | // and the anonymous visitors never share what was built. | |
| 177 | 175 | func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | |
| 178 | - | api := &refsAPI{} | |
| 179 | - | s := refsServer(t, api) | |
| 176 | + | t.Parallel() | |
| 180 | 177 | owner := context.WithValue(context.Background(), signedInKey{}, struct{}{}) | |
| 181 | 178 | anon := context.Background() | |
| 182 | 179 | ||
| 183 | 180 | for _, tt := range []struct { | |
| 184 | - | name string | |
| 185 | - | ctxs []context.Context | |
| 186 | - | wantRepo, wantCommits int32 | |
| 181 | + | name string | |
| 182 | + | ctxs []context.Context | |
| 183 | + | builds int // loads of the repository and refs, and ListCommits calls: one per class | |
| 187 | 184 | }{ | |
| 188 | - | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1, 1}, | |
| 189 | - | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2, 2}, | |
| 185 | + | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1}, | |
| 186 | + | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2}, | |
| 190 | 187 | } { | |
| 191 | 188 | t.Run(tt.name, func(t *testing.T) { | |
| 192 | - | api.repo.Store(0) | |
| 193 | - | api.commits.Store(0) | |
| 194 | - | ev := &pb.Event{Id: "b-" + tt.name, Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} | |
| 189 | + | t.Parallel() | |
| 190 | + | s := refsMocks(t, tt.builds, tt.builds) | |
| 191 | + | ev := &pb.Event{Id: "b-1", Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} | |
| 195 | 192 | for _, ctx := range tt.ctxs { | |
| 196 | 193 | st := &liveStream{s: s, repo: "x", view: "refs"} | |
| 197 | 194 | frags, err := st.render(ctx, ev) | |
| @@ -200,21 +197,16 @@ func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | |||
| 200 | 197 | assert.Equal(t, "refs-list", frags[0].event) | |
| 201 | 198 | assert.Contains(t, frags[0].html, `id="ref-rows"`) | |
| 202 | 199 | } | |
| 203 | - | assert.Equal(t, tt.wantRepo, api.repo.Load(), "loadRepo") | |
| 204 | - | assert.Equal(t, tt.wantCommits, api.commits.Load(), "one ListCommits per distinct commit and class") | |
| 205 | 200 | }) | |
| 206 | 201 | } | |
| 207 | 202 | } | |
| 208 | 203 | ||
| 209 | - | // The refs are asked in one page of up to maxRefs. | |
| 204 | + | // The refs are asked in one page of up to maxRefs (the mock only answers that). | |
| 210 | 205 | func TestLoadRepoAsksTheRefsInOnePage(t *testing.T) { | |
| 211 | 206 | t.Parallel() | |
| 212 | - | api := &refsAPI{} | |
| 213 | - | s := refsServer(t, api) | |
| 207 | + | s := refsMocks(t, 1, 0) | |
| 214 | 208 | d, err := s.loadRepo(context.Background(), "x") | |
| 215 | 209 | require.NoError(t, err) | |
| 216 | 210 | assert.Len(t, d.branches, 1) | |
| 217 | 211 | assert.Len(t, d.tags, 1) | |
| 218 | - | assert.Equal(t, int32(1), api.refs.Load()) | |
| 219 | - | assert.Equal(t, []int32{maxRefs}, api.pageSizes) | |
| 220 | 212 | } | |
| @@ -5,37 +5,35 @@ package web | |||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | "errors" | 7 | "errors" | ||
| 8 | - | "net/http" | |||
| 9 | - | "net/http/httptest" | |||
| 10 | "sync" | 8 | "sync" | ||
| 11 | "sync/atomic" | 9 | "sync/atomic" | ||
| 12 | "testing" | 10 | "testing" | ||
| 11 | + | "testing/synctest" | |||
| 13 | 12 | ||||
| 14 | "connectrpc.com/connect" | 13 | "connectrpc.com/connect" | ||
| 15 | "github.com/stretchr/testify/assert" | 14 | "github.com/stretchr/testify/assert" | ||
| 15 | + | "github.com/stretchr/testify/mock" | |||
| 16 | "github.com/stretchr/testify/require" | 16 | "github.com/stretchr/testify/require" | ||
| 17 | 17 | ||||
| 18 | pb "bnema.dev/ermite-api/gen/ermite" | 18 | pb "bnema.dev/ermite-api/gen/ermite" | ||
| 19 | - | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | 19 | + | "bnema.dev/ermite/internal/adapters/web/mocks" |
| 20 | ) | 20 | ) | ||
| 21 | 21 | ||||
| 22 | func frag(s string) []fragment { return []fragment{{event: "refs-list", html: s}} } | 22 | func frag(s string) []fragment { return []fragment{{event: "refs-list", html: s}} } | ||
| 23 | 23 | ||||
| 24 | func TestFanoutBuildsOncePerKey(t *testing.T) { | 24 | func TestFanoutBuildsOncePerKey(t *testing.T) { | ||
| 25 | t.Parallel() | 25 | t.Parallel() | ||
| 26 | - | srvA, srvB := &Server{}, &Server{} | 26 | + | base := fanKey{repo: "x", event: "b-1"} |
| 27 | - | base := fanKey{srv: srvA, repo: "x", event: "b-1"} | |||
| 28 | for _, tt := range []struct { | 27 | for _, tt := range []struct { | ||
| 29 | name string | 28 | name string | ||
| 30 | keys []fanKey | 29 | keys []fanKey | ||
| 31 | built int32 | 30 | built int32 | ||
| 32 | }{ | 31 | }{ | ||
| 33 | {"same key", []fanKey{base, base, base}, 1}, | 32 | {"same key", []fanKey{base, base, base}, 1}, | ||
| 34 | - | {"owner and anonymous never share", []fanKey{base, {srv: srvA, repo: "x", event: "b-1", owner: true}}, 2}, | 33 | + | {"owner and anonymous never share", []fanKey{base, {repo: "x", event: "b-1", owner: true}}, 2}, |
| 35 | - | {"another event", []fanKey{base, {srv: srvA, repo: "x", event: "b-2"}}, 2}, | 34 | + | {"another event", []fanKey{base, {repo: "x", event: "b-2"}}, 2}, |
| 36 | - | {"another repository", []fanKey{base, {srv: srvA, repo: "y", event: "b-1"}}, 2}, | 35 | + | {"another repository", []fanKey{base, {repo: "y", event: "b-1"}}, 2}, |
| 37 | - | {"another server", []fanKey{base, {srv: srvB, repo: "x", event: "b-1"}}, 2}, | 36 | + | {"no event id: nothing to share by", []fanKey{{repo: "x"}, {repo: "x"}}, 2}, |
| 38 | - | {"no event id: nothing to share by", []fanKey{{srv: srvA, repo: "x"}, {srv: srvA, repo: "x"}}, 2}, | |||
| 39 | } { | 37 | } { | ||
| 40 | t.Run(tt.name, func(t *testing.T) { | 38 | t.Run(tt.name, func(t *testing.T) { | ||
| 41 | t.Parallel() | 39 | t.Parallel() | ||
| @@ -55,37 +53,51 @@ func TestFanoutBuildsOncePerKey(t *testing.T) { | |||||
| 55 | } | 53 | } | ||
| 56 | } | 54 | } | ||
| 57 | 55 | ||||
| 56 | + | // While the first watcher builds, every other one waits for it: in a synctest | |||
| 57 | + | // bubble Wait returns once all goroutines are blocked, so the count of the | |||
| 58 | + | // watchers that entered and none that returned proves the others wait on the | |||
| 59 | + | // build instead of making their own. | |||
| 58 | func TestFanoutConcurrentWatchersShareOneBuild(t *testing.T) { | 60 | func TestFanoutConcurrentWatchersShareOneBuild(t *testing.T) { | ||
| 59 | t.Parallel() | 61 | t.Parallel() | ||
| 60 | - | var f fanout | 62 | + | const watchers = 50 |
| 61 | - | var built atomic.Int32 | 63 | + | synctest.Test(t, func(t *testing.T) { |
| 62 | - | release := make(chan struct{}) | 64 | + | var f fanout |
| 63 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | 65 | + | var built, entered, returned atomic.Int32 |
| 64 | - | var wg sync.WaitGroup | 66 | + | release := make(chan struct{}) |
| 65 | - | results := make([][]fragment, 50) | 67 | + | key := fanKey{repo: "x", event: "b-1"} |
| 66 | - | for i := range results { | 68 | + | var wg sync.WaitGroup |
| 67 | - | wg.Add(1) | 69 | + | results := make([][]fragment, watchers) |
| 68 | - | go func() { | 70 | + | for i := range results { |
| 69 | - | defer wg.Done() | 71 | + | wg.Add(1) |
| 70 | - | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { | 72 | + | go func() { |
| 71 | - | built.Add(1) | 73 | + | defer wg.Done() |
| 72 | - | <-release | 74 | + | entered.Add(1) |
| 73 | - | return frag("once"), nil | 75 | + | results[i], _ = f.do(context.Background(), key, func() ([]fragment, error) { |
| 74 | - | }) | 76 | + | built.Add(1) |
| 75 | - | }() | 77 | + | <-release |
| 76 | - | } | 78 | + | return frag("once"), nil |
| 77 | - | close(release) | 79 | + | }) |
| 78 | - | wg.Wait() | 80 | + | returned.Add(1) |
| 79 | - | assert.Equal(t, int32(1), built.Load()) | 81 | + | }() |
| 80 | - | for _, r := range results { | 82 | + | } |
| 81 | - | assert.Equal(t, frag("once"), r) | 83 | + | synctest.Wait() // the builder is on release, the others on its entry |
| 82 | - | } | 84 | + | require.Equal(t, int32(watchers), entered.Load()) |
| 85 | + | require.Equal(t, int32(1), built.Load(), "one builder") | |||
| 86 | + | require.Zero(t, returned.Load(), "the other %d wait for it", watchers-1) | |||
| 87 | + | ||||
| 88 | + | close(release) | |||
| 89 | + | wg.Wait() | |||
| 90 | + | assert.Equal(t, int32(1), built.Load()) | |||
| 91 | + | for _, r := range results { | |||
| 92 | + | assert.Equal(t, frag("once"), r) | |||
| 93 | + | } | |||
| 94 | + | }) | |||
| 83 | } | 95 | } | ||
| 84 | 96 | ||||
| 85 | func TestFanoutFailureIsNotKept(t *testing.T) { | 97 | func TestFanoutFailureIsNotKept(t *testing.T) { | ||
| 86 | t.Parallel() | 98 | t.Parallel() | ||
| 87 | var f fanout | 99 | var f fanout | ||
| 88 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | 100 | + | key := fanKey{repo: "x", event: "b-1"} |
| 89 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return nil, errors.New("boom") }) | 101 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return nil, errors.New("boom") }) | ||
| 90 | require.Error(t, err) | 102 | require.Error(t, err) | ||
| 91 | got, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("ok"), nil }) | 103 | got, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("ok"), nil }) | ||
| @@ -96,7 +108,7 @@ func TestFanoutFailureIsNotKept(t *testing.T) { | |||||
| 96 | func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | 108 | func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | ||
| 97 | t.Parallel() | 109 | t.Parallel() | ||
| 98 | var f fanout | 110 | var f fanout | ||
| 99 | - | key := fanKey{srv: &Server{}, repo: "x", event: "b-1"} | 111 | + | key := fanKey{repo: "x", event: "b-1"} |
| 100 | started, release := make(chan struct{}), make(chan struct{}) | 112 | started, release := make(chan struct{}), make(chan struct{}) | ||
| 101 | go func() { | 113 | go func() { | ||
| 102 | _, _ = f.do(context.Background(), key, func() ([]fragment, error) { | 114 | _, _ = f.do(context.Background(), key, func() ([]fragment, error) { | ||
| @@ -116,56 +128,42 @@ func TestFanoutWaiterStopsWithItsContext(t *testing.T) { | |||||
| 116 | func TestFanoutIsBounded(t *testing.T) { | 128 | func TestFanoutIsBounded(t *testing.T) { | ||
| 117 | t.Parallel() | 129 | t.Parallel() | ||
| 118 | var f fanout | 130 | var f fanout | ||
| 119 | - | srv := &Server{} | |||
| 120 | for i := range fanMax + 10 { | 131 | for i := range fanMax + 10 { | ||
| 121 | - | key := fanKey{srv: srv, repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} | 132 | + | key := fanKey{repo: "x", event: "b-" + string(rune('A'+i%26)) + string(rune('a'+i/26))} |
| 122 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("x"), nil }) | 133 | _, err := f.do(context.Background(), key, func() ([]fragment, error) { return frag("x"), nil }) | ||
| 123 | require.NoError(t, err) | 134 | require.NoError(t, err) | ||
| 124 | } | 135 | } | ||
| 125 | assert.LessOrEqual(t, len(f.m), fanMax) | 136 | assert.LessOrEqual(t, len(f.m), fanMax) | ||
| 126 | } | 137 | } | ||
| 127 | 138 | ||||
| 128 | - | // refsAPI counts the calls the refs fragment makes to the API. | 139 | + | // refsMocks is a web server over mocks that expect the refs fragment to load |
| 129 | - | type refsAPI struct { | 140 | + | // the repository and its refs repos times, and to ask for a commit commits |
| 130 | - | ermiteconnect.UnimplementedRepoServiceHandler | 141 | + | // times. |
| 131 | - | ermiteconnect.UnimplementedGitServiceHandler | 142 | + | func refsMocks(t *testing.T, repos, commits int) *Server { |
| 132 | - | repo, refs, commits atomic.Int32 | |||
| 133 | - | pageSizes []int32 | |||
| 134 | - | mu sync.Mutex | |||
| 135 | - | } | |||
| 136 | - | ||||
| 137 | - | func (a *refsAPI) GetRepo(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { | |||
| 138 | - | a.repo.Add(1) | |||
| 139 | - | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil | |||
| 140 | - | } | |||
| 141 | - | ||||
| 142 | - | func (a *refsAPI) ListRefs(_ context.Context, req *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |||
| 143 | - | a.refs.Add(1) | |||
| 144 | - | a.mu.Lock() | |||
| 145 | - | a.pageSizes = append(a.pageSizes, req.Msg.GetPageSize()) | |||
| 146 | - | a.mu.Unlock() | |||
| 147 | - | id := "0123456789abcdef0123456789abcdef01234567" | |||
| 148 | - | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |||
| 149 | - | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |||
| 150 | - | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |||
| 151 | - | }}), nil | |||
| 152 | - | } | |||
| 153 | - | ||||
| 154 | - | func (a *refsAPI) ListCommits(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |||
| 155 | - | a.commits.Add(1) | |||
| 156 | - | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |||
| 157 | - | } | |||
| 158 | - | ||||
| 159 | - | func refsServer(t *testing.T, api *refsAPI) *Server { | |||
| 160 | t.Helper() | 143 | t.Helper() | ||
| 161 | - | mux := http.NewServeMux() | 144 | + | api := mocks.NewRepoServiceClient(t) |
| 162 | - | mux.Handle(ermiteconnect.NewRepoServiceHandler(api)) | 145 | + | git := mocks.NewGitServiceClient(t) |
| 163 | - | mux.Handle(ermiteconnect.NewGitServiceHandler(api)) | 146 | + | api.EXPECT().GetRepo(mock.Anything, mock.Anything).RunAndReturn( |
| 164 | - | backend := httptest.NewServer(mux) | 147 | + | func(_ context.Context, req *connect.Request[pb.GetRepoRequest]) (*connect.Response[pb.GetRepoResponse], error) { |
| 165 | - | t.Cleanup(backend.Close) | 148 | + | return connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: req.Msg.GetRepo(), DefaultBranch: "main"}}), nil |
| 149 | + | }).Times(repos) | |||
| 150 | + | id := "0123456789abcdef0123456789abcdef01234567" | |||
| 151 | + | git.EXPECT().ListRefs(mock.Anything, mock.MatchedBy(func(req *connect.Request[pb.ListRefsRequest]) bool { | |||
| 152 | + | return req.Msg.GetPageSize() == maxRefs // the refs are asked in one page | |||
| 153 | + | })).RunAndReturn(func(context.Context, *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |||
| 154 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |||
| 155 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: id}, | |||
| 156 | + | {Name: "refs/tags/v1", ShortName: "v1", Kind: pb.RefKind_REF_KIND_TAG, CommitId: id}, | |||
| 157 | + | }}), nil | |||
| 158 | + | }).Times(repos) | |||
| 159 | + | if commits > 0 { | |||
| 160 | + | git.EXPECT().ListCommits(mock.Anything, mock.Anything).RunAndReturn( | |||
| 161 | + | func(_ context.Context, req *connect.Request[pb.ListCommitsRequest]) (*connect.Response[pb.ListCommitsResponse], error) { | |||
| 162 | + | return connect.NewResponse(&pb.ListCommitsResponse{Commits: []*pb.Commit{{Id: req.Msg.GetRevision(), Summary: "subject"}}}), nil | |||
| 163 | + | }).Times(commits) | |||
| 164 | + | } | |||
| 166 | s, err := New(Config{ | 165 | s, err := New(Config{ | ||
| 167 | - | Repos: ermiteconnect.NewRepoServiceClient(backend.Client(), backend.URL), | 166 | + | Repos: api, Git: git, |
| 168 | - | Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), | |||
| 169 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | 167 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | ||
| 170 | }) | 168 | }) | ||
| 171 | require.NoError(t, err) | 169 | require.NoError(t, err) | ||
| @@ -175,23 +173,22 @@ func refsServer(t *testing.T, api *refsAPI) *Server { | |||||
| 175 | // A push is rendered once for the watchers of the same class, and the owner | 173 | // A push is rendered once for the watchers of the same class, and the owner | ||
| 176 | // and the anonymous visitors never share what was built. | 174 | // and the anonymous visitors never share what was built. | ||
| 177 | func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | 175 | func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | ||
| 178 | - | api := &refsAPI{} | 176 | + | t.Parallel() |
| 179 | - | s := refsServer(t, api) | |||
| 180 | owner := context.WithValue(context.Background(), signedInKey{}, struct{}{}) | 177 | owner := context.WithValue(context.Background(), signedInKey{}, struct{}{}) | ||
| 181 | anon := context.Background() | 178 | anon := context.Background() | ||
| 182 | 179 | ||||
| 183 | for _, tt := range []struct { | 180 | for _, tt := range []struct { | ||
| 184 | - | name string | 181 | + | name string |
| 185 | - | ctxs []context.Context | 182 | + | ctxs []context.Context |
| 186 | - | wantRepo, wantCommits int32 | 183 | + | builds int // loads of the repository and refs, and ListCommits calls: one per class |
| 187 | }{ | 184 | }{ | ||
| 188 | - | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1, 1}, | 185 | + | {"five anonymous watchers", []context.Context{anon, anon, anon, anon, anon}, 1}, |
| 189 | - | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2, 2}, | 186 | + | {"owner and anonymous", []context.Context{anon, owner, anon, owner}, 2}, |
| 190 | } { | 187 | } { | ||
| 191 | t.Run(tt.name, func(t *testing.T) { | 188 | t.Run(tt.name, func(t *testing.T) { | ||
| 192 | - | api.repo.Store(0) | 189 | + | t.Parallel() |
| 193 | - | api.commits.Store(0) | 190 | + | s := refsMocks(t, tt.builds, tt.builds) |
| 194 | - | ev := &pb.Event{Id: "b-" + tt.name, Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} | 191 | + | ev := &pb.Event{Id: "b-1", Kind: &pb.Event_Pushed{Pushed: &pb.Pushed{Repo: "x"}}} |
| 195 | for _, ctx := range tt.ctxs { | 192 | for _, ctx := range tt.ctxs { | ||
| 196 | st := &liveStream{s: s, repo: "x", view: "refs"} | 193 | st := &liveStream{s: s, repo: "x", view: "refs"} | ||
| 197 | frags, err := st.render(ctx, ev) | 194 | frags, err := st.render(ctx, ev) | ||
| @@ -200,21 +197,16 @@ func TestLiveRefsSharesAPushBetweenWatchers(t *testing.T) { | |||||
| 200 | assert.Equal(t, "refs-list", frags[0].event) | 197 | assert.Equal(t, "refs-list", frags[0].event) | ||
| 201 | assert.Contains(t, frags[0].html, `id="ref-rows"`) | 198 | assert.Contains(t, frags[0].html, `id="ref-rows"`) | ||
| 202 | } | 199 | } | ||
| 203 | - | assert.Equal(t, tt.wantRepo, api.repo.Load(), "loadRepo") | |||
| 204 | - | assert.Equal(t, tt.wantCommits, api.commits.Load(), "one ListCommits per distinct commit and class") | |||
| 205 | }) | 200 | }) | ||
| 206 | } | 201 | } | ||
| 207 | } | 202 | } | ||
| 208 | 203 | ||||
| 209 | - | // The refs are asked in one page of up to maxRefs. | 204 | + | // The refs are asked in one page of up to maxRefs (the mock only answers that). |
| 210 | func TestLoadRepoAsksTheRefsInOnePage(t *testing.T) { | 205 | func TestLoadRepoAsksTheRefsInOnePage(t *testing.T) { | ||
| 211 | t.Parallel() | 206 | t.Parallel() | ||
| 212 | - | api := &refsAPI{} | 207 | + | s := refsMocks(t, 1, 0) |
| 213 | - | s := refsServer(t, api) | |||
| 214 | d, err := s.loadRepo(context.Background(), "x") | 208 | d, err := s.loadRepo(context.Background(), "x") | ||
| 215 | require.NoError(t, err) | 209 | require.NoError(t, err) | ||
| 216 | assert.Len(t, d.branches, 1) | 210 | assert.Len(t, d.branches, 1) | ||
| 217 | assert.Len(t, d.tags, 1) | 211 | assert.Len(t, d.tags, 1) | ||
| 218 | - | assert.Equal(t, int32(1), api.refs.Load()) | |||
| 219 | - | assert.Equal(t, []int32{maxRefs}, api.pageSizes) | |||
| 220 | } | 212 | } | ||
M internal/adapters/web/fragments.go +2 -8
| @@ -147,8 +147,8 @@ func (l *liveStream) lastPush(ctx context.Context) ([]fragment, error) { | |||
| 147 | 147 | // of one event are built once for all the watchers of the repository that | |
| 148 | 148 | // see the same things (see fanout); eventID names the event. | |
| 149 | 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) { | |
| 150 | + | key := fanKey{repo: l.repo, event: eventID, owner: SignedIn(ctx)} | |
| 151 | + | return l.s.refsFan.do(ctx, key, func() ([]fragment, error) { | |
| 152 | 152 | d, err := l.s.loadRepo(ctx, l.repo) | |
| 153 | 153 | if err != nil { | |
| 154 | 154 | return nil, err | |
| @@ -178,7 +178,6 @@ const ( | |||
| 178 | 178 | // built as the owner can show what an anonymous visitor may not see, so the | |
| 179 | 179 | // two never share an entry. | |
| 180 | 180 | type fanKey struct { | |
| 181 | - | srv *Server // events of two servers never share | |
| 182 | 181 | repo, event string | |
| 183 | 182 | owner bool | |
| 184 | 183 | } | |
| @@ -190,11 +189,6 @@ type fanEntry struct { | |||
| 190 | 189 | at time.Time // when it was built; zero while it is | |
| 191 | 190 | } | |
| 192 | 191 | ||
| 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 | 192 | // fanout builds the fragments of an event once for the many watchers that | |
| 199 | 193 | // receive it: a push to a repository that 256 pages watch would otherwise | |
| 200 | 194 | // load the refs and the commits they designate 256 times. The zero value is | |
| @@ -147,8 +147,8 @@ func (l *liveStream) lastPush(ctx context.Context) ([]fragment, error) { | |||||
| 147 | // of one event are built once for all the watchers of the repository that | 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. | 148 | // see the same things (see fanout); eventID names the event. | ||
| 149 | func (l *liveStream) refs(ctx context.Context, eventID string) ([]fragment, error) { | 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)} | 150 | + | key := fanKey{repo: l.repo, event: eventID, owner: SignedIn(ctx)} |
| 151 | - | return refsFan.do(ctx, key, func() ([]fragment, error) { | 151 | + | return l.s.refsFan.do(ctx, key, func() ([]fragment, error) { |
| 152 | d, err := l.s.loadRepo(ctx, l.repo) | 152 | d, err := l.s.loadRepo(ctx, l.repo) | ||
| 153 | if err != nil { | 153 | if err != nil { | ||
| 154 | return nil, err | 154 | return nil, err | ||
| @@ -178,7 +178,6 @@ const ( | |||||
| 178 | // built as the owner can show what an anonymous visitor may not see, so the | 178 | // built as the owner can show what an anonymous visitor may not see, so the | ||
| 179 | // two never share an entry. | 179 | // two never share an entry. | ||
| 180 | type fanKey struct { | 180 | type fanKey struct { | ||
| 181 | - | srv *Server // events of two servers never share | |||
| 182 | repo, event string | 181 | repo, event string | ||
| 183 | owner bool | 182 | owner bool | ||
| 184 | } | 183 | } | ||
| @@ -190,11 +189,6 @@ type fanEntry struct { | |||||
| 190 | at time.Time // when it was built; zero while it is | 189 | at time.Time // when it was built; zero while it is | ||
| 191 | } | 190 | } | ||
| 192 | 191 | ||||
| 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 | 192 | // 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 | 193 | // 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 | 194 | // load the refs and the commits they designate 256 times. The zero value is | ||
A internal/adapters/web/mocks/FailureLimiter.go +160 -0
| @@ -0,0 +1,160 @@ | |||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |
| 2 | + | // github.com/vektra/mockery | |
| 3 | + | // template: testify | |
| 4 | + | ||
| 5 | + | package mocks | |
| 6 | + | ||
| 7 | + | import ( | |
| 8 | + | "context" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | mock "github.com/stretchr/testify/mock" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // NewFailureLimiter creates a new instance of FailureLimiter. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 15 | + | // The first argument is typically a *testing.T value. | |
| 16 | + | func NewFailureLimiter(t interface { | |
| 17 | + | mock.TestingT | |
| 18 | + | Cleanup(func()) | |
| 19 | + | }) *FailureLimiter { | |
| 20 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 21 | + | helper.Helper() | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | mock := &FailureLimiter{} | |
| 25 | + | mock.Mock.Test(t) | |
| 26 | + | ||
| 27 | + | t.Cleanup(func() { | |
| 28 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 29 | + | helper.Helper() | |
| 30 | + | } | |
| 31 | + | mock.AssertExpectations(t) | |
| 32 | + | }) | |
| 33 | + | ||
| 34 | + | return mock | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | // FailureLimiter is an autogenerated mock type for the FailureLimiter type | |
| 38 | + | type FailureLimiter struct { | |
| 39 | + | mock.Mock | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | type FailureLimiter_Expecter struct { | |
| 43 | + | mock *mock.Mock | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func (_m *FailureLimiter) EXPECT() *FailureLimiter_Expecter { | |
| 47 | + | return &FailureLimiter_Expecter{mock: &_m.Mock} | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | // Blocked provides a mock function for the type FailureLimiter | |
| 51 | + | func (_mock *FailureLimiter) Blocked(ctx context.Context, addr string) (bool, time.Duration) { | |
| 52 | + | ret := _mock.Called(ctx, addr) | |
| 53 | + | ||
| 54 | + | if len(ret) == 0 { | |
| 55 | + | panic("no return value specified for Blocked") | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | var r0 bool | |
| 59 | + | var r1 time.Duration | |
| 60 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (bool, time.Duration)); ok { | |
| 61 | + | return returnFunc(ctx, addr) | |
| 62 | + | } | |
| 63 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) bool); ok { | |
| 64 | + | r0 = returnFunc(ctx, addr) | |
| 65 | + | } else { | |
| 66 | + | r0 = ret.Get(0).(bool) | |
| 67 | + | } | |
| 68 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) time.Duration); ok { | |
| 69 | + | r1 = returnFunc(ctx, addr) | |
| 70 | + | } else { | |
| 71 | + | r1 = ret.Get(1).(time.Duration) | |
| 72 | + | } | |
| 73 | + | return r0, r1 | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | // FailureLimiter_Blocked_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Blocked' | |
| 77 | + | type FailureLimiter_Blocked_Call struct { | |
| 78 | + | *mock.Call | |
| 79 | + | } | |
| 80 | + | ||
| 81 | + | // Blocked is a helper method to define mock.On call | |
| 82 | + | // - ctx context.Context | |
| 83 | + | // - addr string | |
| 84 | + | func (_e *FailureLimiter_Expecter) Blocked(ctx any, addr any) *FailureLimiter_Blocked_Call { | |
| 85 | + | return &FailureLimiter_Blocked_Call{Call: _e.mock.On("Blocked", ctx, addr)} | |
| 86 | + | } | |
| 87 | + | ||
| 88 | + | func (_c *FailureLimiter_Blocked_Call) Run(run func(ctx context.Context, addr string)) *FailureLimiter_Blocked_Call { | |
| 89 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 90 | + | var arg0 context.Context | |
| 91 | + | if args[0] != nil { | |
| 92 | + | arg0 = args[0].(context.Context) | |
| 93 | + | } | |
| 94 | + | var arg1 string | |
| 95 | + | if args[1] != nil { | |
| 96 | + | arg1 = args[1].(string) | |
| 97 | + | } | |
| 98 | + | run( | |
| 99 | + | arg0, | |
| 100 | + | arg1, | |
| 101 | + | ) | |
| 102 | + | }) | |
| 103 | + | return _c | |
| 104 | + | } | |
| 105 | + | ||
| 106 | + | func (_c *FailureLimiter_Blocked_Call) Return(b bool, duration time.Duration) *FailureLimiter_Blocked_Call { | |
| 107 | + | _c.Call.Return(b, duration) | |
| 108 | + | return _c | |
| 109 | + | } | |
| 110 | + | ||
| 111 | + | func (_c *FailureLimiter_Blocked_Call) RunAndReturn(run func(ctx context.Context, addr string) (bool, time.Duration)) *FailureLimiter_Blocked_Call { | |
| 112 | + | _c.Call.Return(run) | |
| 113 | + | return _c | |
| 114 | + | } | |
| 115 | + | ||
| 116 | + | // Fail provides a mock function for the type FailureLimiter | |
| 117 | + | func (_mock *FailureLimiter) Fail(ctx context.Context, addr string) { | |
| 118 | + | _mock.Called(ctx, addr) | |
| 119 | + | return | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | // FailureLimiter_Fail_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Fail' | |
| 123 | + | type FailureLimiter_Fail_Call struct { | |
| 124 | + | *mock.Call | |
| 125 | + | } | |
| 126 | + | ||
| 127 | + | // Fail is a helper method to define mock.On call | |
| 128 | + | // - ctx context.Context | |
| 129 | + | // - addr string | |
| 130 | + | func (_e *FailureLimiter_Expecter) Fail(ctx any, addr any) *FailureLimiter_Fail_Call { | |
| 131 | + | return &FailureLimiter_Fail_Call{Call: _e.mock.On("Fail", ctx, addr)} | |
| 132 | + | } | |
| 133 | + | ||
| 134 | + | func (_c *FailureLimiter_Fail_Call) Run(run func(ctx context.Context, addr string)) *FailureLimiter_Fail_Call { | |
| 135 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 136 | + | var arg0 context.Context | |
| 137 | + | if args[0] != nil { | |
| 138 | + | arg0 = args[0].(context.Context) | |
| 139 | + | } | |
| 140 | + | var arg1 string | |
| 141 | + | if args[1] != nil { | |
| 142 | + | arg1 = args[1].(string) | |
| 143 | + | } | |
| 144 | + | run( | |
| 145 | + | arg0, | |
| 146 | + | arg1, | |
| 147 | + | ) | |
| 148 | + | }) | |
| 149 | + | return _c | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | func (_c *FailureLimiter_Fail_Call) Return() *FailureLimiter_Fail_Call { | |
| 153 | + | _c.Call.Return() | |
| 154 | + | return _c | |
| 155 | + | } | |
| 156 | + | ||
| 157 | + | func (_c *FailureLimiter_Fail_Call) RunAndReturn(run func(ctx context.Context, addr string)) *FailureLimiter_Fail_Call { | |
| 158 | + | _c.Run(run) | |
| 159 | + | return _c | |
| 160 | + | } | |
| @@ -0,0 +1,160 @@ | |||||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |||
| 2 | + | // github.com/vektra/mockery | |||
| 3 | + | // template: testify | |||
| 4 | + | ||||
| 5 | + | package mocks | |||
| 6 | + | ||||
| 7 | + | import ( | |||
| 8 | + | "context" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | mock "github.com/stretchr/testify/mock" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // NewFailureLimiter creates a new instance of FailureLimiter. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |||
| 15 | + | // The first argument is typically a *testing.T value. | |||
| 16 | + | func NewFailureLimiter(t interface { | |||
| 17 | + | mock.TestingT | |||
| 18 | + | Cleanup(func()) | |||
| 19 | + | }) *FailureLimiter { | |||
| 20 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 21 | + | helper.Helper() | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | mock := &FailureLimiter{} | |||
| 25 | + | mock.Mock.Test(t) | |||
| 26 | + | ||||
| 27 | + | t.Cleanup(func() { | |||
| 28 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 29 | + | helper.Helper() | |||
| 30 | + | } | |||
| 31 | + | mock.AssertExpectations(t) | |||
| 32 | + | }) | |||
| 33 | + | ||||
| 34 | + | return mock | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | // FailureLimiter is an autogenerated mock type for the FailureLimiter type | |||
| 38 | + | type FailureLimiter struct { | |||
| 39 | + | mock.Mock | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | type FailureLimiter_Expecter struct { | |||
| 43 | + | mock *mock.Mock | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func (_m *FailureLimiter) EXPECT() *FailureLimiter_Expecter { | |||
| 47 | + | return &FailureLimiter_Expecter{mock: &_m.Mock} | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | // Blocked provides a mock function for the type FailureLimiter | |||
| 51 | + | func (_mock *FailureLimiter) Blocked(ctx context.Context, addr string) (bool, time.Duration) { | |||
| 52 | + | ret := _mock.Called(ctx, addr) | |||
| 53 | + | ||||
| 54 | + | if len(ret) == 0 { | |||
| 55 | + | panic("no return value specified for Blocked") | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | var r0 bool | |||
| 59 | + | var r1 time.Duration | |||
| 60 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (bool, time.Duration)); ok { | |||
| 61 | + | return returnFunc(ctx, addr) | |||
| 62 | + | } | |||
| 63 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) bool); ok { | |||
| 64 | + | r0 = returnFunc(ctx, addr) | |||
| 65 | + | } else { | |||
| 66 | + | r0 = ret.Get(0).(bool) | |||
| 67 | + | } | |||
| 68 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) time.Duration); ok { | |||
| 69 | + | r1 = returnFunc(ctx, addr) | |||
| 70 | + | } else { | |||
| 71 | + | r1 = ret.Get(1).(time.Duration) | |||
| 72 | + | } | |||
| 73 | + | return r0, r1 | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | // FailureLimiter_Blocked_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Blocked' | |||
| 77 | + | type FailureLimiter_Blocked_Call struct { | |||
| 78 | + | *mock.Call | |||
| 79 | + | } | |||
| 80 | + | ||||
| 81 | + | // Blocked is a helper method to define mock.On call | |||
| 82 | + | // - ctx context.Context | |||
| 83 | + | // - addr string | |||
| 84 | + | func (_e *FailureLimiter_Expecter) Blocked(ctx any, addr any) *FailureLimiter_Blocked_Call { | |||
| 85 | + | return &FailureLimiter_Blocked_Call{Call: _e.mock.On("Blocked", ctx, addr)} | |||
| 86 | + | } | |||
| 87 | + | ||||
| 88 | + | func (_c *FailureLimiter_Blocked_Call) Run(run func(ctx context.Context, addr string)) *FailureLimiter_Blocked_Call { | |||
| 89 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 90 | + | var arg0 context.Context | |||
| 91 | + | if args[0] != nil { | |||
| 92 | + | arg0 = args[0].(context.Context) | |||
| 93 | + | } | |||
| 94 | + | var arg1 string | |||
| 95 | + | if args[1] != nil { | |||
| 96 | + | arg1 = args[1].(string) | |||
| 97 | + | } | |||
| 98 | + | run( | |||
| 99 | + | arg0, | |||
| 100 | + | arg1, | |||
| 101 | + | ) | |||
| 102 | + | }) | |||
| 103 | + | return _c | |||
| 104 | + | } | |||
| 105 | + | ||||
| 106 | + | func (_c *FailureLimiter_Blocked_Call) Return(b bool, duration time.Duration) *FailureLimiter_Blocked_Call { | |||
| 107 | + | _c.Call.Return(b, duration) | |||
| 108 | + | return _c | |||
| 109 | + | } | |||
| 110 | + | ||||
| 111 | + | func (_c *FailureLimiter_Blocked_Call) RunAndReturn(run func(ctx context.Context, addr string) (bool, time.Duration)) *FailureLimiter_Blocked_Call { | |||
| 112 | + | _c.Call.Return(run) | |||
| 113 | + | return _c | |||
| 114 | + | } | |||
| 115 | + | ||||
| 116 | + | // Fail provides a mock function for the type FailureLimiter | |||
| 117 | + | func (_mock *FailureLimiter) Fail(ctx context.Context, addr string) { | |||
| 118 | + | _mock.Called(ctx, addr) | |||
| 119 | + | return | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | // FailureLimiter_Fail_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Fail' | |||
| 123 | + | type FailureLimiter_Fail_Call struct { | |||
| 124 | + | *mock.Call | |||
| 125 | + | } | |||
| 126 | + | ||||
| 127 | + | // Fail is a helper method to define mock.On call | |||
| 128 | + | // - ctx context.Context | |||
| 129 | + | // - addr string | |||
| 130 | + | func (_e *FailureLimiter_Expecter) Fail(ctx any, addr any) *FailureLimiter_Fail_Call { | |||
| 131 | + | return &FailureLimiter_Fail_Call{Call: _e.mock.On("Fail", ctx, addr)} | |||
| 132 | + | } | |||
| 133 | + | ||||
| 134 | + | func (_c *FailureLimiter_Fail_Call) Run(run func(ctx context.Context, addr string)) *FailureLimiter_Fail_Call { | |||
| 135 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 136 | + | var arg0 context.Context | |||
| 137 | + | if args[0] != nil { | |||
| 138 | + | arg0 = args[0].(context.Context) | |||
| 139 | + | } | |||
| 140 | + | var arg1 string | |||
| 141 | + | if args[1] != nil { | |||
| 142 | + | arg1 = args[1].(string) | |||
| 143 | + | } | |||
| 144 | + | run( | |||
| 145 | + | arg0, | |||
| 146 | + | arg1, | |||
| 147 | + | ) | |||
| 148 | + | }) | |||
| 149 | + | return _c | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | func (_c *FailureLimiter_Fail_Call) Return() *FailureLimiter_Fail_Call { | |||
| 153 | + | _c.Call.Return() | |||
| 154 | + | return _c | |||
| 155 | + | } | |||
| 156 | + | ||||
| 157 | + | func (_c *FailureLimiter_Fail_Call) RunAndReturn(run func(ctx context.Context, addr string)) *FailureLimiter_Fail_Call { | |||
| 158 | + | _c.Run(run) | |||
| 159 | + | return _c | |||
| 160 | + | } | |||
A internal/adapters/web/mocks/SessionAuth.go +245 -0
| @@ -0,0 +1,245 @@ | |||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |
| 2 | + | // github.com/vektra/mockery | |
| 3 | + | // template: testify | |
| 4 | + | ||
| 5 | + | package mocks | |
| 6 | + | ||
| 7 | + | import ( | |
| 8 | + | "context" | |
| 9 | + | "time" | |
| 10 | + | ||
| 11 | + | mock "github.com/stretchr/testify/mock" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // NewSessionAuth creates a new instance of SessionAuth. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |
| 15 | + | // The first argument is typically a *testing.T value. | |
| 16 | + | func NewSessionAuth(t interface { | |
| 17 | + | mock.TestingT | |
| 18 | + | Cleanup(func()) | |
| 19 | + | }) *SessionAuth { | |
| 20 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 21 | + | helper.Helper() | |
| 22 | + | } | |
| 23 | + | ||
| 24 | + | mock := &SessionAuth{} | |
| 25 | + | mock.Mock.Test(t) | |
| 26 | + | ||
| 27 | + | t.Cleanup(func() { | |
| 28 | + | if helper, ok := t.(interface{ Helper() }); ok { | |
| 29 | + | helper.Helper() | |
| 30 | + | } | |
| 31 | + | mock.AssertExpectations(t) | |
| 32 | + | }) | |
| 33 | + | ||
| 34 | + | return mock | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | // SessionAuth is an autogenerated mock type for the SessionAuth type | |
| 38 | + | type SessionAuth struct { | |
| 39 | + | mock.Mock | |
| 40 | + | } | |
| 41 | + | ||
| 42 | + | type SessionAuth_Expecter struct { | |
| 43 | + | mock *mock.Mock | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func (_m *SessionAuth) EXPECT() *SessionAuth_Expecter { | |
| 47 | + | return &SessionAuth_Expecter{mock: &_m.Mock} | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | // Login provides a mock function for the type SessionAuth | |
| 51 | + | func (_mock *SessionAuth) Login(ctx context.Context, nonce string) (string, time.Duration, error) { | |
| 52 | + | ret := _mock.Called(ctx, nonce) | |
| 53 | + | ||
| 54 | + | if len(ret) == 0 { | |
| 55 | + | panic("no return value specified for Login") | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | var r0 string | |
| 59 | + | var r1 time.Duration | |
| 60 | + | var r2 error | |
| 61 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (string, time.Duration, error)); ok { | |
| 62 | + | return returnFunc(ctx, nonce) | |
| 63 | + | } | |
| 64 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) string); ok { | |
| 65 | + | r0 = returnFunc(ctx, nonce) | |
| 66 | + | } else { | |
| 67 | + | r0 = ret.Get(0).(string) | |
| 68 | + | } | |
| 69 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) time.Duration); ok { | |
| 70 | + | r1 = returnFunc(ctx, nonce) | |
| 71 | + | } else { | |
| 72 | + | r1 = ret.Get(1).(time.Duration) | |
| 73 | + | } | |
| 74 | + | if returnFunc, ok := ret.Get(2).(func(context.Context, string) error); ok { | |
| 75 | + | r2 = returnFunc(ctx, nonce) | |
| 76 | + | } else { | |
| 77 | + | r2 = ret.Error(2) | |
| 78 | + | } | |
| 79 | + | return r0, r1, r2 | |
| 80 | + | } | |
| 81 | + | ||
| 82 | + | // SessionAuth_Login_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Login' | |
| 83 | + | type SessionAuth_Login_Call struct { | |
| 84 | + | *mock.Call | |
| 85 | + | } | |
| 86 | + | ||
| 87 | + | // Login is a helper method to define mock.On call | |
| 88 | + | // - ctx context.Context | |
| 89 | + | // - nonce string | |
| 90 | + | func (_e *SessionAuth_Expecter) Login(ctx any, nonce any) *SessionAuth_Login_Call { | |
| 91 | + | return &SessionAuth_Login_Call{Call: _e.mock.On("Login", ctx, nonce)} | |
| 92 | + | } | |
| 93 | + | ||
| 94 | + | func (_c *SessionAuth_Login_Call) Run(run func(ctx context.Context, nonce string)) *SessionAuth_Login_Call { | |
| 95 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 96 | + | var arg0 context.Context | |
| 97 | + | if args[0] != nil { | |
| 98 | + | arg0 = args[0].(context.Context) | |
| 99 | + | } | |
| 100 | + | var arg1 string | |
| 101 | + | if args[1] != nil { | |
| 102 | + | arg1 = args[1].(string) | |
| 103 | + | } | |
| 104 | + | run( | |
| 105 | + | arg0, | |
| 106 | + | arg1, | |
| 107 | + | ) | |
| 108 | + | }) | |
| 109 | + | return _c | |
| 110 | + | } | |
| 111 | + | ||
| 112 | + | func (_c *SessionAuth_Login_Call) Return(token string, lifetime time.Duration, err error) *SessionAuth_Login_Call { | |
| 113 | + | _c.Call.Return(token, lifetime, err) | |
| 114 | + | return _c | |
| 115 | + | } | |
| 116 | + | ||
| 117 | + | func (_c *SessionAuth_Login_Call) RunAndReturn(run func(ctx context.Context, nonce string) (string, time.Duration, error)) *SessionAuth_Login_Call { | |
| 118 | + | _c.Call.Return(run) | |
| 119 | + | return _c | |
| 120 | + | } | |
| 121 | + | ||
| 122 | + | // LoginLinkValid provides a mock function for the type SessionAuth | |
| 123 | + | func (_mock *SessionAuth) LoginLinkValid(nonce string) bool { | |
| 124 | + | ret := _mock.Called(nonce) | |
| 125 | + | ||
| 126 | + | if len(ret) == 0 { | |
| 127 | + | panic("no return value specified for LoginLinkValid") | |
| 128 | + | } | |
| 129 | + | ||
| 130 | + | var r0 bool | |
| 131 | + | if returnFunc, ok := ret.Get(0).(func(string) bool); ok { | |
| 132 | + | r0 = returnFunc(nonce) | |
| 133 | + | } else { | |
| 134 | + | r0 = ret.Get(0).(bool) | |
| 135 | + | } | |
| 136 | + | return r0 | |
| 137 | + | } | |
| 138 | + | ||
| 139 | + | // SessionAuth_LoginLinkValid_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'LoginLinkValid' | |
| 140 | + | type SessionAuth_LoginLinkValid_Call struct { | |
| 141 | + | *mock.Call | |
| 142 | + | } | |
| 143 | + | ||
| 144 | + | // LoginLinkValid is a helper method to define mock.On call | |
| 145 | + | // - nonce string | |
| 146 | + | func (_e *SessionAuth_Expecter) LoginLinkValid(nonce any) *SessionAuth_LoginLinkValid_Call { | |
| 147 | + | return &SessionAuth_LoginLinkValid_Call{Call: _e.mock.On("LoginLinkValid", nonce)} | |
| 148 | + | } | |
| 149 | + | ||
| 150 | + | func (_c *SessionAuth_LoginLinkValid_Call) Run(run func(nonce string)) *SessionAuth_LoginLinkValid_Call { | |
| 151 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 152 | + | var arg0 string | |
| 153 | + | if args[0] != nil { | |
| 154 | + | arg0 = args[0].(string) | |
| 155 | + | } | |
| 156 | + | run( | |
| 157 | + | arg0, | |
| 158 | + | ) | |
| 159 | + | }) | |
| 160 | + | return _c | |
| 161 | + | } | |
| 162 | + | ||
| 163 | + | func (_c *SessionAuth_LoginLinkValid_Call) Return(b bool) *SessionAuth_LoginLinkValid_Call { | |
| 164 | + | _c.Call.Return(b) | |
| 165 | + | return _c | |
| 166 | + | } | |
| 167 | + | ||
| 168 | + | func (_c *SessionAuth_LoginLinkValid_Call) RunAndReturn(run func(nonce string) bool) *SessionAuth_LoginLinkValid_Call { | |
| 169 | + | _c.Call.Return(run) | |
| 170 | + | return _c | |
| 171 | + | } | |
| 172 | + | ||
| 173 | + | // Verify provides a mock function for the type SessionAuth | |
| 174 | + | func (_mock *SessionAuth) Verify(ctx context.Context, token string) (context.Context, bool, error) { | |
| 175 | + | ret := _mock.Called(ctx, token) | |
| 176 | + | ||
| 177 | + | if len(ret) == 0 { | |
| 178 | + | panic("no return value specified for Verify") | |
| 179 | + | } | |
| 180 | + | ||
| 181 | + | var r0 context.Context | |
| 182 | + | var r1 bool | |
| 183 | + | var r2 error | |
| 184 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (context.Context, bool, error)); ok { | |
| 185 | + | return returnFunc(ctx, token) | |
| 186 | + | } | |
| 187 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) context.Context); ok { | |
| 188 | + | r0 = returnFunc(ctx, token) | |
| 189 | + | } else { | |
| 190 | + | if ret.Get(0) != nil { | |
| 191 | + | r0 = ret.Get(0).(context.Context) | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) bool); ok { | |
| 195 | + | r1 = returnFunc(ctx, token) | |
| 196 | + | } else { | |
| 197 | + | r1 = ret.Get(1).(bool) | |
| 198 | + | } | |
| 199 | + | if returnFunc, ok := ret.Get(2).(func(context.Context, string) error); ok { | |
| 200 | + | r2 = returnFunc(ctx, token) | |
| 201 | + | } else { | |
| 202 | + | r2 = ret.Error(2) | |
| 203 | + | } | |
| 204 | + | return r0, r1, r2 | |
| 205 | + | } | |
| 206 | + | ||
| 207 | + | // SessionAuth_Verify_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Verify' | |
| 208 | + | type SessionAuth_Verify_Call struct { | |
| 209 | + | *mock.Call | |
| 210 | + | } | |
| 211 | + | ||
| 212 | + | // Verify is a helper method to define mock.On call | |
| 213 | + | // - ctx context.Context | |
| 214 | + | // - token string | |
| 215 | + | func (_e *SessionAuth_Expecter) Verify(ctx any, token any) *SessionAuth_Verify_Call { | |
| 216 | + | return &SessionAuth_Verify_Call{Call: _e.mock.On("Verify", ctx, token)} | |
| 217 | + | } | |
| 218 | + | ||
| 219 | + | func (_c *SessionAuth_Verify_Call) Run(run func(ctx context.Context, token string)) *SessionAuth_Verify_Call { | |
| 220 | + | _c.Call.Run(func(args mock.Arguments) { | |
| 221 | + | var arg0 context.Context | |
| 222 | + | if args[0] != nil { | |
| 223 | + | arg0 = args[0].(context.Context) | |
| 224 | + | } | |
| 225 | + | var arg1 string | |
| 226 | + | if args[1] != nil { | |
| 227 | + | arg1 = args[1].(string) | |
| 228 | + | } | |
| 229 | + | run( | |
| 230 | + | arg0, | |
| 231 | + | arg1, | |
| 232 | + | ) | |
| 233 | + | }) | |
| 234 | + | return _c | |
| 235 | + | } | |
| 236 | + | ||
| 237 | + | func (_c *SessionAuth_Verify_Call) Return(owner context.Context, ok bool, err error) *SessionAuth_Verify_Call { | |
| 238 | + | _c.Call.Return(owner, ok, err) | |
| 239 | + | return _c | |
| 240 | + | } | |
| 241 | + | ||
| 242 | + | func (_c *SessionAuth_Verify_Call) RunAndReturn(run func(ctx context.Context, token string) (context.Context, bool, error)) *SessionAuth_Verify_Call { | |
| 243 | + | _c.Call.Return(run) | |
| 244 | + | return _c | |
| 245 | + | } | |
| @@ -0,0 +1,245 @@ | |||||
| 1 | + | // Code generated by mockery; DO NOT EDIT. | |||
| 2 | + | // github.com/vektra/mockery | |||
| 3 | + | // template: testify | |||
| 4 | + | ||||
| 5 | + | package mocks | |||
| 6 | + | ||||
| 7 | + | import ( | |||
| 8 | + | "context" | |||
| 9 | + | "time" | |||
| 10 | + | ||||
| 11 | + | mock "github.com/stretchr/testify/mock" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // NewSessionAuth creates a new instance of SessionAuth. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. | |||
| 15 | + | // The first argument is typically a *testing.T value. | |||
| 16 | + | func NewSessionAuth(t interface { | |||
| 17 | + | mock.TestingT | |||
| 18 | + | Cleanup(func()) | |||
| 19 | + | }) *SessionAuth { | |||
| 20 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 21 | + | helper.Helper() | |||
| 22 | + | } | |||
| 23 | + | ||||
| 24 | + | mock := &SessionAuth{} | |||
| 25 | + | mock.Mock.Test(t) | |||
| 26 | + | ||||
| 27 | + | t.Cleanup(func() { | |||
| 28 | + | if helper, ok := t.(interface{ Helper() }); ok { | |||
| 29 | + | helper.Helper() | |||
| 30 | + | } | |||
| 31 | + | mock.AssertExpectations(t) | |||
| 32 | + | }) | |||
| 33 | + | ||||
| 34 | + | return mock | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | // SessionAuth is an autogenerated mock type for the SessionAuth type | |||
| 38 | + | type SessionAuth struct { | |||
| 39 | + | mock.Mock | |||
| 40 | + | } | |||
| 41 | + | ||||
| 42 | + | type SessionAuth_Expecter struct { | |||
| 43 | + | mock *mock.Mock | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func (_m *SessionAuth) EXPECT() *SessionAuth_Expecter { | |||
| 47 | + | return &SessionAuth_Expecter{mock: &_m.Mock} | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | // Login provides a mock function for the type SessionAuth | |||
| 51 | + | func (_mock *SessionAuth) Login(ctx context.Context, nonce string) (string, time.Duration, error) { | |||
| 52 | + | ret := _mock.Called(ctx, nonce) | |||
| 53 | + | ||||
| 54 | + | if len(ret) == 0 { | |||
| 55 | + | panic("no return value specified for Login") | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | var r0 string | |||
| 59 | + | var r1 time.Duration | |||
| 60 | + | var r2 error | |||
| 61 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (string, time.Duration, error)); ok { | |||
| 62 | + | return returnFunc(ctx, nonce) | |||
| 63 | + | } | |||
| 64 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) string); ok { | |||
| 65 | + | r0 = returnFunc(ctx, nonce) | |||
| 66 | + | } else { | |||
| 67 | + | r0 = ret.Get(0).(string) | |||
| 68 | + | } | |||
| 69 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) time.Duration); ok { | |||
| 70 | + | r1 = returnFunc(ctx, nonce) | |||
| 71 | + | } else { | |||
| 72 | + | r1 = ret.Get(1).(time.Duration) | |||
| 73 | + | } | |||
| 74 | + | if returnFunc, ok := ret.Get(2).(func(context.Context, string) error); ok { | |||
| 75 | + | r2 = returnFunc(ctx, nonce) | |||
| 76 | + | } else { | |||
| 77 | + | r2 = ret.Error(2) | |||
| 78 | + | } | |||
| 79 | + | return r0, r1, r2 | |||
| 80 | + | } | |||
| 81 | + | ||||
| 82 | + | // SessionAuth_Login_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Login' | |||
| 83 | + | type SessionAuth_Login_Call struct { | |||
| 84 | + | *mock.Call | |||
| 85 | + | } | |||
| 86 | + | ||||
| 87 | + | // Login is a helper method to define mock.On call | |||
| 88 | + | // - ctx context.Context | |||
| 89 | + | // - nonce string | |||
| 90 | + | func (_e *SessionAuth_Expecter) Login(ctx any, nonce any) *SessionAuth_Login_Call { | |||
| 91 | + | return &SessionAuth_Login_Call{Call: _e.mock.On("Login", ctx, nonce)} | |||
| 92 | + | } | |||
| 93 | + | ||||
| 94 | + | func (_c *SessionAuth_Login_Call) Run(run func(ctx context.Context, nonce string)) *SessionAuth_Login_Call { | |||
| 95 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 96 | + | var arg0 context.Context | |||
| 97 | + | if args[0] != nil { | |||
| 98 | + | arg0 = args[0].(context.Context) | |||
| 99 | + | } | |||
| 100 | + | var arg1 string | |||
| 101 | + | if args[1] != nil { | |||
| 102 | + | arg1 = args[1].(string) | |||
| 103 | + | } | |||
| 104 | + | run( | |||
| 105 | + | arg0, | |||
| 106 | + | arg1, | |||
| 107 | + | ) | |||
| 108 | + | }) | |||
| 109 | + | return _c | |||
| 110 | + | } | |||
| 111 | + | ||||
| 112 | + | func (_c *SessionAuth_Login_Call) Return(token string, lifetime time.Duration, err error) *SessionAuth_Login_Call { | |||
| 113 | + | _c.Call.Return(token, lifetime, err) | |||
| 114 | + | return _c | |||
| 115 | + | } | |||
| 116 | + | ||||
| 117 | + | func (_c *SessionAuth_Login_Call) RunAndReturn(run func(ctx context.Context, nonce string) (string, time.Duration, error)) *SessionAuth_Login_Call { | |||
| 118 | + | _c.Call.Return(run) | |||
| 119 | + | return _c | |||
| 120 | + | } | |||
| 121 | + | ||||
| 122 | + | // LoginLinkValid provides a mock function for the type SessionAuth | |||
| 123 | + | func (_mock *SessionAuth) LoginLinkValid(nonce string) bool { | |||
| 124 | + | ret := _mock.Called(nonce) | |||
| 125 | + | ||||
| 126 | + | if len(ret) == 0 { | |||
| 127 | + | panic("no return value specified for LoginLinkValid") | |||
| 128 | + | } | |||
| 129 | + | ||||
| 130 | + | var r0 bool | |||
| 131 | + | if returnFunc, ok := ret.Get(0).(func(string) bool); ok { | |||
| 132 | + | r0 = returnFunc(nonce) | |||
| 133 | + | } else { | |||
| 134 | + | r0 = ret.Get(0).(bool) | |||
| 135 | + | } | |||
| 136 | + | return r0 | |||
| 137 | + | } | |||
| 138 | + | ||||
| 139 | + | // SessionAuth_LoginLinkValid_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'LoginLinkValid' | |||
| 140 | + | type SessionAuth_LoginLinkValid_Call struct { | |||
| 141 | + | *mock.Call | |||
| 142 | + | } | |||
| 143 | + | ||||
| 144 | + | // LoginLinkValid is a helper method to define mock.On call | |||
| 145 | + | // - nonce string | |||
| 146 | + | func (_e *SessionAuth_Expecter) LoginLinkValid(nonce any) *SessionAuth_LoginLinkValid_Call { | |||
| 147 | + | return &SessionAuth_LoginLinkValid_Call{Call: _e.mock.On("LoginLinkValid", nonce)} | |||
| 148 | + | } | |||
| 149 | + | ||||
| 150 | + | func (_c *SessionAuth_LoginLinkValid_Call) Run(run func(nonce string)) *SessionAuth_LoginLinkValid_Call { | |||
| 151 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 152 | + | var arg0 string | |||
| 153 | + | if args[0] != nil { | |||
| 154 | + | arg0 = args[0].(string) | |||
| 155 | + | } | |||
| 156 | + | run( | |||
| 157 | + | arg0, | |||
| 158 | + | ) | |||
| 159 | + | }) | |||
| 160 | + | return _c | |||
| 161 | + | } | |||
| 162 | + | ||||
| 163 | + | func (_c *SessionAuth_LoginLinkValid_Call) Return(b bool) *SessionAuth_LoginLinkValid_Call { | |||
| 164 | + | _c.Call.Return(b) | |||
| 165 | + | return _c | |||
| 166 | + | } | |||
| 167 | + | ||||
| 168 | + | func (_c *SessionAuth_LoginLinkValid_Call) RunAndReturn(run func(nonce string) bool) *SessionAuth_LoginLinkValid_Call { | |||
| 169 | + | _c.Call.Return(run) | |||
| 170 | + | return _c | |||
| 171 | + | } | |||
| 172 | + | ||||
| 173 | + | // Verify provides a mock function for the type SessionAuth | |||
| 174 | + | func (_mock *SessionAuth) Verify(ctx context.Context, token string) (context.Context, bool, error) { | |||
| 175 | + | ret := _mock.Called(ctx, token) | |||
| 176 | + | ||||
| 177 | + | if len(ret) == 0 { | |||
| 178 | + | panic("no return value specified for Verify") | |||
| 179 | + | } | |||
| 180 | + | ||||
| 181 | + | var r0 context.Context | |||
| 182 | + | var r1 bool | |||
| 183 | + | var r2 error | |||
| 184 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) (context.Context, bool, error)); ok { | |||
| 185 | + | return returnFunc(ctx, token) | |||
| 186 | + | } | |||
| 187 | + | if returnFunc, ok := ret.Get(0).(func(context.Context, string) context.Context); ok { | |||
| 188 | + | r0 = returnFunc(ctx, token) | |||
| 189 | + | } else { | |||
| 190 | + | if ret.Get(0) != nil { | |||
| 191 | + | r0 = ret.Get(0).(context.Context) | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | if returnFunc, ok := ret.Get(1).(func(context.Context, string) bool); ok { | |||
| 195 | + | r1 = returnFunc(ctx, token) | |||
| 196 | + | } else { | |||
| 197 | + | r1 = ret.Get(1).(bool) | |||
| 198 | + | } | |||
| 199 | + | if returnFunc, ok := ret.Get(2).(func(context.Context, string) error); ok { | |||
| 200 | + | r2 = returnFunc(ctx, token) | |||
| 201 | + | } else { | |||
| 202 | + | r2 = ret.Error(2) | |||
| 203 | + | } | |||
| 204 | + | return r0, r1, r2 | |||
| 205 | + | } | |||
| 206 | + | ||||
| 207 | + | // SessionAuth_Verify_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Verify' | |||
| 208 | + | type SessionAuth_Verify_Call struct { | |||
| 209 | + | *mock.Call | |||
| 210 | + | } | |||
| 211 | + | ||||
| 212 | + | // Verify is a helper method to define mock.On call | |||
| 213 | + | // - ctx context.Context | |||
| 214 | + | // - token string | |||
| 215 | + | func (_e *SessionAuth_Expecter) Verify(ctx any, token any) *SessionAuth_Verify_Call { | |||
| 216 | + | return &SessionAuth_Verify_Call{Call: _e.mock.On("Verify", ctx, token)} | |||
| 217 | + | } | |||
| 218 | + | ||||
| 219 | + | func (_c *SessionAuth_Verify_Call) Run(run func(ctx context.Context, token string)) *SessionAuth_Verify_Call { | |||
| 220 | + | _c.Call.Run(func(args mock.Arguments) { | |||
| 221 | + | var arg0 context.Context | |||
| 222 | + | if args[0] != nil { | |||
| 223 | + | arg0 = args[0].(context.Context) | |||
| 224 | + | } | |||
| 225 | + | var arg1 string | |||
| 226 | + | if args[1] != nil { | |||
| 227 | + | arg1 = args[1].(string) | |||
| 228 | + | } | |||
| 229 | + | run( | |||
| 230 | + | arg0, | |||
| 231 | + | arg1, | |||
| 232 | + | ) | |||
| 233 | + | }) | |||
| 234 | + | return _c | |||
| 235 | + | } | |||
| 236 | + | ||||
| 237 | + | func (_c *SessionAuth_Verify_Call) Return(owner context.Context, ok bool, err error) *SessionAuth_Verify_Call { | |||
| 238 | + | _c.Call.Return(owner, ok, err) | |||
| 239 | + | return _c | |||
| 240 | + | } | |||
| 241 | + | ||||
| 242 | + | func (_c *SessionAuth_Verify_Call) RunAndReturn(run func(ctx context.Context, token string) (context.Context, bool, error)) *SessionAuth_Verify_Call { | |||
| 243 | + | _c.Call.Return(run) | |||
| 244 | + | return _c | |||
| 245 | + | } | |||
M internal/adapters/web/raw.go +4 -42
| @@ -8,13 +8,9 @@ import ( | |||
| 8 | 8 | "net/http" | |
| 9 | 9 | "path" | |
| 10 | 10 | "strconv" | |
| 11 | - | "strings" | |
| 12 | 11 | "time" | |
| 13 | 12 | ||
| 14 | - | "connectrpc.com/connect" | |
| 15 | 13 | "github.com/bnema/zerowrap" | |
| 16 | - | ||
| 17 | - | pb "bnema.dev/ermite-api/gen/ermite" | |
| 18 | 14 | ) | |
| 19 | 15 | ||
| 20 | 16 | // rawTimeout bounds the stream of one file to the visitor. | |
| @@ -36,17 +32,6 @@ func (s *Server) raw(w http.ResponseWriter, r *http.Request) { | |||
| 36 | 32 | s.fail(ctx, w, r, err) | |
| 37 | 33 | return | |
| 38 | 34 | } | |
| 39 | - | // A revalidation of an unchanged file ends here, before the content is | |
| 40 | - | // opened: for a large blob, opening starts a Git process. | |
| 41 | - | if match := r.Header.Get("If-None-Match"); match != "" && s.rawUnchanged(ctx, repo, t, match) { | |
| 42 | - | h := w.Header() | |
| 43 | - | h.Set("ETag", match) | |
| 44 | - | h.Set("X-Content-Type-Options", "nosniff") | |
| 45 | - | h.Set("Content-Security-Policy", rawCSP) | |
| 46 | - | cacheHeaders(ctx, h, "no-cache", sessionCache) | |
| 47 | - | w.WriteHeader(http.StatusNotModified) | |
| 48 | - | return | |
| 49 | - | } | |
| 50 | 35 | b, err := s.openBlob(ctx, repo, t) | |
| 51 | 36 | if err != nil { | |
| 52 | 37 | s.fail(ctx, w, r, err) | |
| @@ -78,7 +63,10 @@ func (s *Server) raw(w http.ResponseWriter, r *http.Request) { | |||
| 78 | 63 | } | |
| 79 | 64 | // Revalidated every time, even at a full commit id: a repository can turn | |
| 80 | 65 | // private, and a shared cache must not keep serving its files (AC2). The | |
| 81 | - | // ETag keeps the revalidation cheap. | |
| 66 | + | // ETag spares the body: the blob id comes with the first message of | |
| 67 | + | // the stream, and a match ends the answer there. No cheaper call gives the | |
| 68 | + | // id of the blob at a path: a tree listing of its directory costs more | |
| 69 | + | // than opening a small file. | |
| 82 | 70 | cacheHeaders(ctx, h, "no-cache", sessionCache) | |
| 83 | 71 | if match := r.Header.Get("If-None-Match"); match != "" && h.Get("ETag") != "" && match == h.Get("ETag") { | |
| 84 | 72 | w.WriteHeader(http.StatusNotModified) | |
| @@ -119,32 +107,6 @@ func (s *Server) rawTarget(ctx context.Context, r *http.Request) (target, string | |||
| 119 | 107 | return t, d.name(), nil | |
| 120 | 108 | } | |
| 121 | 109 | ||
| 122 | - | // rawUnchanged reports whether match is the ETag of the file, which is the id | |
| 123 | - | // of its blob. The id comes from the entry of the file in its directory, so | |
| 124 | - | // nothing of the content is read. Any doubt (the entry is not found, the | |
| 125 | - | // listing is cut, the call fails) answers false: the file is then opened and | |
| 126 | - | // the real outcome, error included, follows. | |
| 127 | - | func (s *Server) rawUnchanged(ctx context.Context, repo string, t target, match string) bool { | |
| 128 | - | dir, _ := path.Split(t.path) | |
| 129 | - | res, err := s.cfg.Git.GetTree(ctx, connect.NewRequest(&pb.GetTreeRequest{ | |
| 130 | - | Repo: repo, Revision: t.rev, Path: strings.TrimSuffix(dir, "/"), | |
| 131 | - | })) | |
| 132 | - | if err != nil { | |
| 133 | - | return false | |
| 134 | - | } | |
| 135 | - | for _, e := range res.Msg.GetEntries() { | |
| 136 | - | if e.GetPath() != t.path { | |
| 137 | - | continue | |
| 138 | - | } | |
| 139 | - | switch e.GetKind() { | |
| 140 | - | case pb.EntryKind_ENTRY_KIND_FILE, pb.EntryKind_ENTRY_KIND_EXECUTABLE, pb.EntryKind_ENTRY_KIND_SYMLINK: | |
| 141 | - | return e.GetId() != "" && match == `"`+e.GetId()+`"` | |
| 142 | - | } | |
| 143 | - | return false | |
| 144 | - | } | |
| 145 | - | return false | |
| 146 | - | } | |
| 147 | - | ||
| 148 | 110 | // rawType is the Content-Type of a file and whether it is an attachment. | |
| 149 | 111 | // b.buf holds the first bytes (up to 512 when the file has that many). | |
| 150 | 112 | func rawType(b *blobStream) (typ string, attachment bool) { | |
| @@ -8,13 +8,9 @@ import ( | |||||
| 8 | "net/http" | 8 | "net/http" | ||
| 9 | "path" | 9 | "path" | ||
| 10 | "strconv" | 10 | "strconv" | ||
| 11 | - | "strings" | |||
| 12 | "time" | 11 | "time" | ||
| 13 | 12 | ||||
| 14 | - | "connectrpc.com/connect" | |||
| 15 | "github.com/bnema/zerowrap" | 13 | "github.com/bnema/zerowrap" | ||
| 16 | - | ||||
| 17 | - | pb "bnema.dev/ermite-api/gen/ermite" | |||
| 18 | ) | 14 | ) | ||
| 19 | 15 | ||||
| 20 | // rawTimeout bounds the stream of one file to the visitor. | 16 | // rawTimeout bounds the stream of one file to the visitor. | ||
| @@ -36,17 +32,6 @@ func (s *Server) raw(w http.ResponseWriter, r *http.Request) { | |||||
| 36 | s.fail(ctx, w, r, err) | 32 | s.fail(ctx, w, r, err) | ||
| 37 | return | 33 | return | ||
| 38 | } | 34 | } | ||
| 39 | - | // A revalidation of an unchanged file ends here, before the content is | |||
| 40 | - | // opened: for a large blob, opening starts a Git process. | |||
| 41 | - | if match := r.Header.Get("If-None-Match"); match != "" && s.rawUnchanged(ctx, repo, t, match) { | |||
| 42 | - | h := w.Header() | |||
| 43 | - | h.Set("ETag", match) | |||
| 44 | - | h.Set("X-Content-Type-Options", "nosniff") | |||
| 45 | - | h.Set("Content-Security-Policy", rawCSP) | |||
| 46 | - | cacheHeaders(ctx, h, "no-cache", sessionCache) | |||
| 47 | - | w.WriteHeader(http.StatusNotModified) | |||
| 48 | - | return | |||
| 49 | - | } | |||
| 50 | b, err := s.openBlob(ctx, repo, t) | 35 | b, err := s.openBlob(ctx, repo, t) | ||
| 51 | if err != nil { | 36 | if err != nil { | ||
| 52 | s.fail(ctx, w, r, err) | 37 | s.fail(ctx, w, r, err) | ||
| @@ -78,7 +63,10 @@ func (s *Server) raw(w http.ResponseWriter, r *http.Request) { | |||||
| 78 | } | 63 | } | ||
| 79 | // Revalidated every time, even at a full commit id: a repository can turn | 64 | // Revalidated every time, even at a full commit id: a repository can turn | ||
| 80 | // private, and a shared cache must not keep serving its files (AC2). The | 65 | // private, and a shared cache must not keep serving its files (AC2). The | ||
| 81 | - | // ETag keeps the revalidation cheap. | 66 | + | // ETag spares the body: the blob id comes with the first message of |
| 67 | + | // the stream, and a match ends the answer there. No cheaper call gives the | |||
| 68 | + | // id of the blob at a path: a tree listing of its directory costs more | |||
| 69 | + | // than opening a small file. | |||
| 82 | cacheHeaders(ctx, h, "no-cache", sessionCache) | 70 | cacheHeaders(ctx, h, "no-cache", sessionCache) | ||
| 83 | if match := r.Header.Get("If-None-Match"); match != "" && h.Get("ETag") != "" && match == h.Get("ETag") { | 71 | if match := r.Header.Get("If-None-Match"); match != "" && h.Get("ETag") != "" && match == h.Get("ETag") { | ||
| 84 | w.WriteHeader(http.StatusNotModified) | 72 | w.WriteHeader(http.StatusNotModified) | ||
| @@ -119,32 +107,6 @@ func (s *Server) rawTarget(ctx context.Context, r *http.Request) (target, string | |||||
| 119 | return t, d.name(), nil | 107 | return t, d.name(), nil | ||
| 120 | } | 108 | } | ||
| 121 | 109 | ||||
| 122 | - | // rawUnchanged reports whether match is the ETag of the file, which is the id | |||
| 123 | - | // of its blob. The id comes from the entry of the file in its directory, so | |||
| 124 | - | // nothing of the content is read. Any doubt (the entry is not found, the | |||
| 125 | - | // listing is cut, the call fails) answers false: the file is then opened and | |||
| 126 | - | // the real outcome, error included, follows. | |||
| 127 | - | func (s *Server) rawUnchanged(ctx context.Context, repo string, t target, match string) bool { | |||
| 128 | - | dir, _ := path.Split(t.path) | |||
| 129 | - | res, err := s.cfg.Git.GetTree(ctx, connect.NewRequest(&pb.GetTreeRequest{ | |||
| 130 | - | Repo: repo, Revision: t.rev, Path: strings.TrimSuffix(dir, "/"), | |||
| 131 | - | })) | |||
| 132 | - | if err != nil { | |||
| 133 | - | return false | |||
| 134 | - | } | |||
| 135 | - | for _, e := range res.Msg.GetEntries() { | |||
| 136 | - | if e.GetPath() != t.path { | |||
| 137 | - | continue | |||
| 138 | - | } | |||
| 139 | - | switch e.GetKind() { | |||
| 140 | - | case pb.EntryKind_ENTRY_KIND_FILE, pb.EntryKind_ENTRY_KIND_EXECUTABLE, pb.EntryKind_ENTRY_KIND_SYMLINK: | |||
| 141 | - | return e.GetId() != "" && match == `"`+e.GetId()+`"` | |||
| 142 | - | } | |||
| 143 | - | return false | |||
| 144 | - | } | |||
| 145 | - | return false | |||
| 146 | - | } | |||
| 147 | - | ||||
| 148 | // rawType is the Content-Type of a file and whether it is an attachment. | 110 | // rawType is the Content-Type of a file and whether it is an attachment. | ||
| 149 | // b.buf holds the first bytes (up to 512 when the file has that many). | 111 | // b.buf holds the first bytes (up to 512 when the file has that many). | ||
| 150 | func rawType(b *blobStream) (typ string, attachment bool) { | 112 | func rawType(b *blobStream) (typ string, attachment bool) { | ||
M internal/adapters/web/raw_test.go +57 -81
| @@ -14,103 +14,64 @@ import ( | |||
| 14 | 14 | "github.com/stretchr/testify/require" | |
| 15 | 15 | ||
| 16 | 16 | pb "bnema.dev/ermite-api/gen/ermite" | |
| 17 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 17 | 18 | "bnema.dev/ermite/internal/adapters/web/mocks" | |
| 18 | 19 | ) | |
| 19 | 20 | ||
| 20 | 21 | const blobID = "0123456789abcdef0123456789abcdef01234567" | |
| 21 | 22 | ||
| 22 | - | // A revalidation of an unchanged file is answered from the tree entry, before | |
| 23 | - | // the content is opened; anything else opens the file. | |
| 24 | - | func TestRawChecksETagBeforeOpeningTheBlob(t *testing.T) { | |
| 25 | - | t.Parallel() | |
| 26 | - | entry := func(kind pb.EntryKind, id string) *pb.TreeEntry { | |
| 27 | - | return &pb.TreeEntry{Name: "big.bin", Path: "docs/big.bin", Kind: kind, Id: id} | |
| 23 | + | // blobAPI serves one text file, the only call raw makes besides the refs. | |
| 24 | + | type blobAPI struct { | |
| 25 | + | ermiteconnect.UnimplementedGitServiceHandler | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | func (*blobAPI) GetBlob(_ context.Context, req *connect.Request[pb.GetBlobRequest], st *connect.ServerStream[pb.GetBlobResponse]) error { | |
| 29 | + | if req.Msg.GetPath() != "docs/a.txt" { | |
| 30 | + | return connect.NewError(connect.CodeNotFound, assert.AnError) | |
| 28 | 31 | } | |
| 32 | + | return st.Send(&pb.GetBlobResponse{ | |
| 33 | + | Info: &pb.BlobInfo{Id: blobID, Path: "docs/a.txt", Size: 5, MediaType: "text/plain"}, | |
| 34 | + | Chunk: []byte("hello"), | |
| 35 | + | }) | |
| 36 | + | } | |
| 37 | + | ||
| 38 | + | // A revalidation of an unchanged file ends with a 304 and no body. The blob is | |
| 39 | + | // opened in every case: its id is only known from the stream. | |
| 40 | + | func TestRawRevalidation(t *testing.T) { | |
| 41 | + | t.Parallel() | |
| 29 | 42 | tests := []struct { | |
| 30 | - | name string | |
| 31 | - | url string | |
| 32 | - | ifNone string | |
| 33 | - | wantDir string | |
| 34 | - | tree *pb.GetTreeResponse | |
| 35 | - | treeErr error | |
| 36 | - | want int | |
| 37 | - | wantOpens int // GetBlob calls | |
| 43 | + | name string | |
| 44 | + | url string | |
| 45 | + | method string | |
| 46 | + | ifNone string | |
| 47 | + | want int | |
| 48 | + | body string | |
| 38 | 49 | }{ | |
| 39 | - | { | |
| 40 | - | name: "unchanged file", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 41 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, blobID)}}, | |
| 42 | - | want: http.StatusNotModified, | |
| 43 | - | }, | |
| 44 | - | { | |
| 45 | - | name: "unchanged executable", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 46 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_EXECUTABLE, blobID)}}, | |
| 47 | - | want: http.StatusNotModified, | |
| 48 | - | }, | |
| 49 | - | { | |
| 50 | - | name: "file at the root", url: "/r/raw/main/big.bin", ifNone: `"` + blobID + `"`, wantDir: "", | |
| 51 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{{Name: "big.bin", Path: "big.bin", Kind: pb.EntryKind_ENTRY_KIND_FILE, Id: blobID}}}, | |
| 52 | - | want: http.StatusNotModified, | |
| 53 | - | }, | |
| 54 | - | { | |
| 55 | - | name: "changed file", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 56 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, "ffffffffffffffffffffffffffffffffffffffff")}}, | |
| 57 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 58 | - | }, | |
| 59 | - | { | |
| 60 | - | name: "directory with that id", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 61 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_DIRECTORY, blobID)}}, | |
| 62 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 63 | - | }, | |
| 64 | - | { | |
| 65 | - | name: "entry not listed", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 66 | - | tree: &pb.GetTreeResponse{Truncated: true}, | |
| 67 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 68 | - | }, | |
| 69 | - | { | |
| 70 | - | name: "tree lookup fails", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |
| 71 | - | treeErr: connect.NewError(connect.CodeNotFound, assert.AnError), | |
| 72 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 73 | - | }, | |
| 74 | - | { | |
| 75 | - | name: "weak validator is not a match", url: "/r/raw/main/docs/big.bin", ifNone: `W/"` + blobID + `"`, wantDir: "docs", | |
| 76 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, blobID)}}, | |
| 77 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 78 | - | }, | |
| 79 | - | { | |
| 80 | - | name: "no validator", url: "/r/raw/main/docs/big.bin", ifNone: "", | |
| 81 | - | want: http.StatusNotFound, wantOpens: 1, | |
| 82 | - | }, | |
| 50 | + | {"no validator", "/r/raw/main/docs/a.txt", http.MethodGet, "", http.StatusOK, "hello"}, | |
| 51 | + | {"matching validator", "/r/raw/main/docs/a.txt", http.MethodGet, `"` + blobID + `"`, http.StatusNotModified, ""}, | |
| 52 | + | {"other validator", "/r/raw/main/docs/a.txt", http.MethodGet, `"ffffffffffffffffffffffffffffffffffffffff"`, http.StatusOK, "hello"}, | |
| 53 | + | {"weak validator is not a match", "/r/raw/main/docs/a.txt", http.MethodGet, `W/"` + blobID + `"`, http.StatusOK, "hello"}, | |
| 54 | + | {"head", "/r/raw/main/docs/a.txt", http.MethodHead, "", http.StatusOK, ""}, | |
| 55 | + | {"missing file", "/r/raw/main/docs/none.txt", http.MethodGet, `"` + blobID + `"`, http.StatusNotFound, ""}, | |
| 56 | + | {"directory", "/r/raw/main", http.MethodGet, `"` + blobID + `"`, http.StatusNotFound, ""}, | |
| 83 | 57 | } | |
| 84 | 58 | for _, tt := range tests { | |
| 85 | 59 | t.Run(tt.name, func(t *testing.T) { | |
| 86 | 60 | t.Parallel() | |
| 87 | 61 | repos := mocks.NewRepoServiceClient(t) | |
| 88 | - | git := mocks.NewGitServiceClient(t) | |
| 89 | 62 | repos.EXPECT().GetRepo(mock.Anything, mock.Anything). | |
| 90 | 63 | Return(connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: "r", DefaultBranch: "main"}}), nil) | |
| 91 | - | git.EXPECT().ListRefs(mock.Anything, mock.Anything). | |
| 92 | - | Return(connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |
| 93 | - | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: "c0ffee0"}, | |
| 94 | - | }}), nil) | |
| 95 | - | if tt.ifNone != "" { | |
| 96 | - | git.EXPECT().GetTree(mock.Anything, mock.MatchedBy(func(req *connect.Request[pb.GetTreeRequest]) bool { | |
| 97 | - | m := req.Msg | |
| 98 | - | return m.GetRepo() == "r" && m.GetRevision() == "c0ffee0" && m.GetPath() == tt.wantDir && !m.GetIncludeLastCommit() | |
| 99 | - | })).Return(connect.NewResponse(tt.tree), tt.treeErr).Once() | |
| 100 | - | } | |
| 101 | - | opens := 0 | |
| 102 | - | git.EXPECT().GetBlob(mock.Anything, mock.Anything).RunAndReturn( | |
| 103 | - | func(_ context.Context, _ *connect.Request[pb.GetBlobRequest]) (*connect.ServerStreamForClient[pb.GetBlobResponse], error) { | |
| 104 | - | opens++ | |
| 105 | - | return nil, connect.NewError(connect.CodeNotFound, assert.AnError) | |
| 106 | - | }).Maybe() | |
| 64 | + | mux := http.NewServeMux() | |
| 65 | + | mux.Handle(ermiteconnect.NewGitServiceHandler(&refsGit{})) | |
| 66 | + | backend := httptest.NewServer(mux) | |
| 67 | + | t.Cleanup(backend.Close) | |
| 107 | 68 | ||
| 108 | 69 | srv, err := New(Config{ | |
| 109 | - | Repos: repos, Git: git, | |
| 70 | + | Repos: repos, Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), | |
| 110 | 71 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | |
| 111 | 72 | }) | |
| 112 | 73 | require.NoError(t, err) | |
| 113 | - | req := httptest.NewRequest(http.MethodGet, tt.url, nil) | |
| 74 | + | req := httptest.NewRequest(tt.method, tt.url, nil) | |
| 114 | 75 | if tt.ifNone != "" { | |
| 115 | 76 | req.Header.Set("If-None-Match", tt.ifNone) | |
| 116 | 77 | } | |
| @@ -118,13 +79,28 @@ func TestRawChecksETagBeforeOpeningTheBlob(t *testing.T) { | |||
| 118 | 79 | srv.ServeHTTP(rec, req) | |
| 119 | 80 | ||
| 120 | 81 | assert.Equal(t, tt.want, rec.Code) | |
| 121 | - | assert.Equal(t, tt.wantOpens, opens, "GetBlob calls") | |
| 82 | + | if tt.want != http.StatusNotFound { | |
| 83 | + | assert.Equal(t, tt.body, rec.Body.String()) | |
| 84 | + | } | |
| 122 | 85 | if tt.want == http.StatusNotModified { | |
| 123 | - | assert.Equal(t, tt.ifNone, rec.Header().Get("ETag")) | |
| 124 | - | assert.Equal(t, rawCSP, rec.Header().Get("Content-Security-Policy")) | |
| 86 | + | assert.Equal(t, `"`+blobID+`"`, rec.Header().Get("ETag")) | |
| 125 | 87 | assert.Equal(t, "no-cache", rec.Header().Get("Cache-Control")) | |
| 126 | - | assert.Empty(t, rec.Body.Bytes()) | |
| 88 | + | } | |
| 89 | + | if tt.want == http.StatusOK { | |
| 90 | + | assert.Equal(t, `"`+blobID+`"`, rec.Header().Get("ETag")) | |
| 91 | + | assert.Equal(t, rawCSP, rec.Header().Get("Content-Security-Policy")) | |
| 127 | 92 | } | |
| 128 | 93 | }) | |
| 129 | 94 | } | |
| 130 | 95 | } | |
| 96 | + | ||
| 97 | + | // refsGit adds the refs of the repository to blobAPI. | |
| 98 | + | type refsGit struct { | |
| 99 | + | blobAPI | |
| 100 | + | } | |
| 101 | + | ||
| 102 | + | func (g *refsGit) ListRefs(context.Context, *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |
| 103 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |
| 104 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: "c0ffee0"}, | |
| 105 | + | }}), nil | |
| 106 | + | } | |
| @@ -14,103 +14,64 @@ import ( | |||||
| 14 | "github.com/stretchr/testify/require" | 14 | "github.com/stretchr/testify/require" | ||
| 15 | 15 | ||||
| 16 | pb "bnema.dev/ermite-api/gen/ermite" | 16 | pb "bnema.dev/ermite-api/gen/ermite" | ||
| 17 | + | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |||
| 17 | "bnema.dev/ermite/internal/adapters/web/mocks" | 18 | "bnema.dev/ermite/internal/adapters/web/mocks" | ||
| 18 | ) | 19 | ) | ||
| 19 | 20 | ||||
| 20 | const blobID = "0123456789abcdef0123456789abcdef01234567" | 21 | const blobID = "0123456789abcdef0123456789abcdef01234567" | ||
| 21 | 22 | ||||
| 22 | - | // A revalidation of an unchanged file is answered from the tree entry, before | 23 | + | // blobAPI serves one text file, the only call raw makes besides the refs. |
| 23 | - | // the content is opened; anything else opens the file. | 24 | + | type blobAPI struct { |
| 24 | - | func TestRawChecksETagBeforeOpeningTheBlob(t *testing.T) { | 25 | + | ermiteconnect.UnimplementedGitServiceHandler |
| 25 | - | t.Parallel() | 26 | + | } |
| 26 | - | entry := func(kind pb.EntryKind, id string) *pb.TreeEntry { | 27 | + | |
| 27 | - | return &pb.TreeEntry{Name: "big.bin", Path: "docs/big.bin", Kind: kind, Id: id} | 28 | + | func (*blobAPI) GetBlob(_ context.Context, req *connect.Request[pb.GetBlobRequest], st *connect.ServerStream[pb.GetBlobResponse]) error { |
| 29 | + | if req.Msg.GetPath() != "docs/a.txt" { | |||
| 30 | + | return connect.NewError(connect.CodeNotFound, assert.AnError) | |||
| 28 | } | 31 | } | ||
| 32 | + | return st.Send(&pb.GetBlobResponse{ | |||
| 33 | + | Info: &pb.BlobInfo{Id: blobID, Path: "docs/a.txt", Size: 5, MediaType: "text/plain"}, | |||
| 34 | + | Chunk: []byte("hello"), | |||
| 35 | + | }) | |||
| 36 | + | } | |||
| 37 | + | ||||
| 38 | + | // A revalidation of an unchanged file ends with a 304 and no body. The blob is | |||
| 39 | + | // opened in every case: its id is only known from the stream. | |||
| 40 | + | func TestRawRevalidation(t *testing.T) { | |||
| 41 | + | t.Parallel() | |||
| 29 | tests := []struct { | 42 | tests := []struct { | ||
| 30 | - | name string | 43 | + | name string |
| 31 | - | url string | 44 | + | url string |
| 32 | - | ifNone string | 45 | + | method string |
| 33 | - | wantDir string | 46 | + | ifNone string |
| 34 | - | tree *pb.GetTreeResponse | 47 | + | want int |
| 35 | - | treeErr error | 48 | + | body string |
| 36 | - | want int | |||
| 37 | - | wantOpens int // GetBlob calls | |||
| 38 | }{ | 49 | }{ | ||
| 39 | - | { | 50 | + | {"no validator", "/r/raw/main/docs/a.txt", http.MethodGet, "", http.StatusOK, "hello"}, |
| 40 | - | name: "unchanged file", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | 51 | + | {"matching validator", "/r/raw/main/docs/a.txt", http.MethodGet, `"` + blobID + `"`, http.StatusNotModified, ""}, |
| 41 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, blobID)}}, | 52 | + | {"other validator", "/r/raw/main/docs/a.txt", http.MethodGet, `"ffffffffffffffffffffffffffffffffffffffff"`, http.StatusOK, "hello"}, |
| 42 | - | want: http.StatusNotModified, | 53 | + | {"weak validator is not a match", "/r/raw/main/docs/a.txt", http.MethodGet, `W/"` + blobID + `"`, http.StatusOK, "hello"}, |
| 43 | - | }, | 54 | + | {"head", "/r/raw/main/docs/a.txt", http.MethodHead, "", http.StatusOK, ""}, |
| 44 | - | { | 55 | + | {"missing file", "/r/raw/main/docs/none.txt", http.MethodGet, `"` + blobID + `"`, http.StatusNotFound, ""}, |
| 45 | - | name: "unchanged executable", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | 56 | + | {"directory", "/r/raw/main", http.MethodGet, `"` + blobID + `"`, http.StatusNotFound, ""}, |
| 46 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_EXECUTABLE, blobID)}}, | |||
| 47 | - | want: http.StatusNotModified, | |||
| 48 | - | }, | |||
| 49 | - | { | |||
| 50 | - | name: "file at the root", url: "/r/raw/main/big.bin", ifNone: `"` + blobID + `"`, wantDir: "", | |||
| 51 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{{Name: "big.bin", Path: "big.bin", Kind: pb.EntryKind_ENTRY_KIND_FILE, Id: blobID}}}, | |||
| 52 | - | want: http.StatusNotModified, | |||
| 53 | - | }, | |||
| 54 | - | { | |||
| 55 | - | name: "changed file", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |||
| 56 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, "ffffffffffffffffffffffffffffffffffffffff")}}, | |||
| 57 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 58 | - | }, | |||
| 59 | - | { | |||
| 60 | - | name: "directory with that id", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |||
| 61 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_DIRECTORY, blobID)}}, | |||
| 62 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 63 | - | }, | |||
| 64 | - | { | |||
| 65 | - | name: "entry not listed", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |||
| 66 | - | tree: &pb.GetTreeResponse{Truncated: true}, | |||
| 67 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 68 | - | }, | |||
| 69 | - | { | |||
| 70 | - | name: "tree lookup fails", url: "/r/raw/main/docs/big.bin", ifNone: `"` + blobID + `"`, wantDir: "docs", | |||
| 71 | - | treeErr: connect.NewError(connect.CodeNotFound, assert.AnError), | |||
| 72 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 73 | - | }, | |||
| 74 | - | { | |||
| 75 | - | name: "weak validator is not a match", url: "/r/raw/main/docs/big.bin", ifNone: `W/"` + blobID + `"`, wantDir: "docs", | |||
| 76 | - | tree: &pb.GetTreeResponse{Entries: []*pb.TreeEntry{entry(pb.EntryKind_ENTRY_KIND_FILE, blobID)}}, | |||
| 77 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 78 | - | }, | |||
| 79 | - | { | |||
| 80 | - | name: "no validator", url: "/r/raw/main/docs/big.bin", ifNone: "", | |||
| 81 | - | want: http.StatusNotFound, wantOpens: 1, | |||
| 82 | - | }, | |||
| 83 | } | 57 | } | ||
| 84 | for _, tt := range tests { | 58 | for _, tt := range tests { | ||
| 85 | t.Run(tt.name, func(t *testing.T) { | 59 | t.Run(tt.name, func(t *testing.T) { | ||
| 86 | t.Parallel() | 60 | t.Parallel() | ||
| 87 | repos := mocks.NewRepoServiceClient(t) | 61 | repos := mocks.NewRepoServiceClient(t) | ||
| 88 | - | git := mocks.NewGitServiceClient(t) | |||
| 89 | repos.EXPECT().GetRepo(mock.Anything, mock.Anything). | 62 | repos.EXPECT().GetRepo(mock.Anything, mock.Anything). | ||
| 90 | Return(connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: "r", DefaultBranch: "main"}}), nil) | 63 | Return(connect.NewResponse(&pb.GetRepoResponse{Repo: &pb.Repo{Name: "r", DefaultBranch: "main"}}), nil) | ||
| 91 | - | git.EXPECT().ListRefs(mock.Anything, mock.Anything). | 64 | + | mux := http.NewServeMux() |
| 92 | - | Return(connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | 65 | + | mux.Handle(ermiteconnect.NewGitServiceHandler(&refsGit{})) |
| 93 | - | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: "c0ffee0"}, | 66 | + | backend := httptest.NewServer(mux) |
| 94 | - | }}), nil) | 67 | + | t.Cleanup(backend.Close) |
| 95 | - | if tt.ifNone != "" { | |||
| 96 | - | git.EXPECT().GetTree(mock.Anything, mock.MatchedBy(func(req *connect.Request[pb.GetTreeRequest]) bool { | |||
| 97 | - | m := req.Msg | |||
| 98 | - | return m.GetRepo() == "r" && m.GetRevision() == "c0ffee0" && m.GetPath() == tt.wantDir && !m.GetIncludeLastCommit() | |||
| 99 | - | })).Return(connect.NewResponse(tt.tree), tt.treeErr).Once() | |||
| 100 | - | } | |||
| 101 | - | opens := 0 | |||
| 102 | - | git.EXPECT().GetBlob(mock.Anything, mock.Anything).RunAndReturn( | |||
| 103 | - | func(_ context.Context, _ *connect.Request[pb.GetBlobRequest]) (*connect.ServerStreamForClient[pb.GetBlobResponse], error) { | |||
| 104 | - | opens++ | |||
| 105 | - | return nil, connect.NewError(connect.CodeNotFound, assert.AnError) | |||
| 106 | - | }).Maybe() | |||
| 107 | 68 | ||||
| 108 | srv, err := New(Config{ | 69 | srv, err := New(Config{ | ||
| 109 | - | Repos: repos, Git: git, | 70 | + | Repos: repos, Git: ermiteconnect.NewGitServiceClient(backend.Client(), backend.URL), |
| 110 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | 71 | PublicURL: "https://forge.example", SourceURL: "https://forge.example/ermite", Version: "v1", | ||
| 111 | }) | 72 | }) | ||
| 112 | require.NoError(t, err) | 73 | require.NoError(t, err) | ||
| 113 | - | req := httptest.NewRequest(http.MethodGet, tt.url, nil) | 74 | + | req := httptest.NewRequest(tt.method, tt.url, nil) |
| 114 | if tt.ifNone != "" { | 75 | if tt.ifNone != "" { | ||
| 115 | req.Header.Set("If-None-Match", tt.ifNone) | 76 | req.Header.Set("If-None-Match", tt.ifNone) | ||
| 116 | } | 77 | } | ||
| @@ -118,13 +79,28 @@ func TestRawChecksETagBeforeOpeningTheBlob(t *testing.T) { | |||||
| 118 | srv.ServeHTTP(rec, req) | 79 | srv.ServeHTTP(rec, req) | ||
| 119 | 80 | ||||
| 120 | assert.Equal(t, tt.want, rec.Code) | 81 | assert.Equal(t, tt.want, rec.Code) | ||
| 121 | - | assert.Equal(t, tt.wantOpens, opens, "GetBlob calls") | 82 | + | if tt.want != http.StatusNotFound { |
| 83 | + | assert.Equal(t, tt.body, rec.Body.String()) | |||
| 84 | + | } | |||
| 122 | if tt.want == http.StatusNotModified { | 85 | if tt.want == http.StatusNotModified { | ||
| 123 | - | assert.Equal(t, tt.ifNone, rec.Header().Get("ETag")) | 86 | + | assert.Equal(t, `"`+blobID+`"`, rec.Header().Get("ETag")) |
| 124 | - | assert.Equal(t, rawCSP, rec.Header().Get("Content-Security-Policy")) | |||
| 125 | assert.Equal(t, "no-cache", rec.Header().Get("Cache-Control")) | 87 | assert.Equal(t, "no-cache", rec.Header().Get("Cache-Control")) | ||
| 126 | - | assert.Empty(t, rec.Body.Bytes()) | 88 | + | } |
| 89 | + | if tt.want == http.StatusOK { | |||
| 90 | + | assert.Equal(t, `"`+blobID+`"`, rec.Header().Get("ETag")) | |||
| 91 | + | assert.Equal(t, rawCSP, rec.Header().Get("Content-Security-Policy")) | |||
| 127 | } | 92 | } | ||
| 128 | }) | 93 | }) | ||
| 129 | } | 94 | } | ||
| 130 | } | 95 | } | ||
| 96 | + | ||||
| 97 | + | // refsGit adds the refs of the repository to blobAPI. | |||
| 98 | + | type refsGit struct { | |||
| 99 | + | blobAPI | |||
| 100 | + | } | |||
| 101 | + | ||||
| 102 | + | func (g *refsGit) ListRefs(context.Context, *connect.Request[pb.ListRefsRequest]) (*connect.Response[pb.ListRefsResponse], error) { | |||
| 103 | + | return connect.NewResponse(&pb.ListRefsResponse{Refs: []*pb.Ref{ | |||
| 104 | + | {Name: "refs/heads/main", ShortName: "main", Kind: pb.RefKind_REF_KIND_BRANCH, CommitId: "c0ffee0"}, | |||
| 105 | + | }}), nil | |||
| 106 | + | } | |||
M internal/adapters/web/session_test.go +49 -24
| @@ -13,30 +13,54 @@ import ( | |||
| 13 | 13 | "time" | |
| 14 | 14 | ||
| 15 | 15 | "github.com/stretchr/testify/assert" | |
| 16 | + | "github.com/stretchr/testify/mock" | |
| 16 | 17 | "github.com/stretchr/testify/require" | |
| 17 | 18 | ||
| 18 | 19 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | |
| 20 | + | "bnema.dev/ermite/internal/adapters/web/mocks" | |
| 19 | 21 | ) | |
| 20 | 22 | ||
| 21 | - | // fakeSessions accepts one token. | |
| 22 | - | type fakeSessions struct { | |
| 23 | - | valid string | |
| 24 | - | err error | |
| 25 | - | used []string | |
| 23 | + | // nonceA is the one valid login link of the mocks. | |
| 24 | + | var nonceA = strings.Repeat("a", 43) | |
| 25 | + | ||
| 26 | + | // newVerifier is a SessionAuth that accepts the session token valid, checked | |
| 27 | + | // by verify when it is not nil, and that does not know any login link. A | |
| 28 | + | // call that is not expected fails the test. | |
| 29 | + | func newVerifier(t *testing.T, valid string, verify func(token string) (bool, error)) *mocks.SessionAuth { | |
| 30 | + | t.Helper() | |
| 31 | + | if verify == nil { | |
| 32 | + | verify = func(token string) (bool, error) { return token == valid, nil } | |
| 33 | + | } | |
| 34 | + | m := mocks.NewSessionAuth(t) | |
| 35 | + | m.EXPECT().Verify(mock.Anything, mock.Anything).RunAndReturn( | |
| 36 | + | func(ctx context.Context, token string) (context.Context, bool, error) { | |
| 37 | + | ok, err := verify(token) | |
| 38 | + | return ctx, ok, err | |
| 39 | + | }).Maybe() | |
| 40 | + | return m | |
| 26 | 41 | } | |
| 27 | 42 | ||
| 28 | - | func (f *fakeSessions) LoginLinkValid(n string) bool { return n == strings.Repeat("a", 43) } | |
| 43 | + | // expectLogin makes nonceA the one link of m, answering with the token valid. | |
| 44 | + | func expectLogin(m *mocks.SessionAuth, valid string) { | |
| 45 | + | m.EXPECT().LoginLinkValid(mock.Anything).RunAndReturn(func(n string) bool { return n == nonceA }).Maybe() | |
| 46 | + | m.EXPECT().Login(mock.Anything, nonceA).Return(valid, time.Hour, nil).Maybe() | |
| 47 | + | m.EXPECT().Login(mock.Anything, mock.Anything).Return("", time.Duration(0), ErrLinkRejected).Maybe() | |
| 48 | + | } | |
| 29 | 49 | ||
| 30 | - | func (f *fakeSessions) Login(_ context.Context, n string) (string, time.Duration, error) { | |
| 31 | - | f.used = append(f.used, n) | |
| 32 | - | if n != strings.Repeat("a", 43) { | |
| 33 | - | return "", 0, ErrLinkRejected | |
| 34 | - | } | |
| 35 | - | return f.valid, time.Hour, nil | |
| 50 | + | // newSessions is a SessionAuth with one session token and one login link. | |
| 51 | + | func newSessions(t *testing.T, valid string) *mocks.SessionAuth { | |
| 52 | + | t.Helper() | |
| 53 | + | m := newVerifier(t, valid, nil) | |
| 54 | + | expectLogin(m, valid) | |
| 55 | + | return m | |
| 36 | 56 | } | |
| 37 | 57 | ||
| 38 | - | func (f *fakeSessions) Verify(ctx context.Context, tok string) (context.Context, bool, error) { | |
| 39 | - | return ctx, f.err == nil && tok == f.valid, f.err | |
| 58 | + | // newBrokenSessions is a SessionAuth whose check cannot be made. | |
| 59 | + | func newBrokenSessions(t *testing.T, valid string) *mocks.SessionAuth { | |
| 60 | + | t.Helper() | |
| 61 | + | m := newVerifier(t, valid, func(string) (bool, error) { return false, errors.New("kv down") }) | |
| 62 | + | expectLogin(m, valid) | |
| 63 | + | return m | |
| 40 | 64 | } | |
| 41 | 65 | ||
| 42 | 66 | type countingLimiter struct{ fails int } | |
| @@ -87,7 +111,7 @@ func TestValidNonce(t *testing.T) { | |||
| 87 | 111 | // a session unless it is the login. | |
| 88 | 112 | func TestGate(t *testing.T) { | |
| 89 | 113 | t.Parallel() | |
| 90 | - | srv, lim := newSessionServer(t, &fakeSessions{valid: "tok"}, "https://forge.example") | |
| 114 | + | srv, lim := newSessionServer(t, newSessions(t, "tok"), "https://forge.example") | |
| 91 | 115 | do := func(path, origin, cookie string) int { | |
| 92 | 116 | req := httptest.NewRequest(http.MethodPost, path, nil) | |
| 93 | 117 | req.Host = "forge.example" | |
| @@ -126,7 +150,7 @@ func TestGate(t *testing.T) { | |||
| 126 | 150 | // must not turn another route into a "login". | |
| 127 | 151 | func TestGateLoginExemptionIsExact(t *testing.T) { | |
| 128 | 152 | t.Parallel() | |
| 129 | - | sess := &fakeSessions{valid: "tok"} | |
| 153 | + | sess := newVerifier(t, "tok", nil) // no Login is expected until the last request | |
| 130 | 154 | lim := &countingLimiter{} | |
| 131 | 155 | srv, err := New(Config{ | |
| 132 | 156 | Repos: failing{err: errors.New("x")}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | |
| @@ -144,7 +168,7 @@ func TestGateLoginExemptionIsExact(t *testing.T) { | |||
| 144 | 168 | srv.ServeHTTP(rec, req) | |
| 145 | 169 | return rec.Code | |
| 146 | 170 | } | |
| 147 | - | nonce := strings.Repeat("a", 43) | |
| 171 | + | nonce := nonceA | |
| 148 | 172 | for _, target := range []string{ | |
| 149 | 173 | "/-%2Flogin%2Fx/merge-requests/1/close", | |
| 150 | 174 | "/-%2flogin%2fx/merge-requests/1/close", | |
| @@ -163,17 +187,18 @@ func TestGateLoginExemptionIsExact(t *testing.T) { | |||
| 163 | 187 | } { | |
| 164 | 188 | assert.Equal(t, http.StatusForbidden, post(target, ""), target) | |
| 165 | 189 | } | |
| 166 | - | assert.Empty(t, sess.used, "no link was used") | |
| 190 | + | // No Login was expected so far: the mock fails the test on any call. | |
| 191 | + | sess.AssertNotCalled(t, "Login", mock.Anything, mock.Anything) | |
| 167 | 192 | ||
| 193 | + | sess.EXPECT().Login(mock.Anything, nonce).Return("tok", time.Hour, nil).Once() | |
| 168 | 194 | assert.Equal(t, http.StatusSeeOther, post("/-/login/"+nonce, ""), "a valid link still logs in") | |
| 169 | - | assert.Equal(t, []string{nonce}, sess.used) | |
| 170 | 195 | } | |
| 171 | 196 | ||
| 172 | 197 | // A session check that cannot be made fails closed (anonymous) and keeps the | |
| 173 | 198 | // cookie; a check that says no clears it. | |
| 174 | 199 | func TestWithSessionFailsClosed(t *testing.T) { | |
| 175 | 200 | t.Parallel() | |
| 176 | - | broken := &fakeSessions{valid: "tok", err: errors.New("kv down")} | |
| 201 | + | broken := newBrokenSessions(t, "tok") | |
| 177 | 202 | srv, _ := newSessionServer(t, broken, "https://forge.example") | |
| 178 | 203 | req := httptest.NewRequest(http.MethodGet, "/", nil) | |
| 179 | 204 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "tok"}) | |
| @@ -182,7 +207,7 @@ func TestWithSessionFailsClosed(t *testing.T) { | |||
| 182 | 207 | assert.False(t, SignedIn(r.Context())) | |
| 183 | 208 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | |
| 184 | 209 | ||
| 185 | - | srv, _ = newSessionServer(t, &fakeSessions{valid: "tok"}, "https://forge.example") | |
| 210 | + | srv, _ = newSessionServer(t, newSessions(t, "tok"), "https://forge.example") | |
| 186 | 211 | req = httptest.NewRequest(http.MethodGet, "/", nil) | |
| 187 | 212 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"}) | |
| 188 | 213 | rec = httptest.NewRecorder() | |
| @@ -200,7 +225,7 @@ func TestWithSessionFailsClosed(t *testing.T) { | |||
| 200 | 225 | // writer, the raw file handler and the event stream keep it. | |
| 201 | 226 | func TestInvalidCookieAnswerIsNotCached(t *testing.T) { | |
| 202 | 227 | t.Parallel() | |
| 203 | - | sess := &fakeSessions{valid: "tok"} | |
| 228 | + | sess := newSessions(t, "tok") | |
| 204 | 229 | srv, _ := newSessionServer(t, sess, "https://forge.example") | |
| 205 | 230 | ||
| 206 | 231 | // withSession alone. | |
| @@ -213,7 +238,7 @@ func TestInvalidCookieAnswerIsNotCached(t *testing.T) { | |||
| 213 | 238 | assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0") | |
| 214 | 239 | ||
| 215 | 240 | // A check that cannot be made keeps the cookie and the headers as they are. | |
| 216 | - | broken, _ := newSessionServer(t, &fakeSessions{valid: "tok", err: errors.New("kv down")}, "https://forge.example") | |
| 241 | + | broken, _ := newSessionServer(t, newBrokenSessions(t, "tok"), "https://forge.example") | |
| 217 | 242 | rec = httptest.NewRecorder() | |
| 218 | 243 | broken.withSession(rec, req) | |
| 219 | 244 | assert.Empty(t, rec.Header().Values("Cache-Control")) | |
| @@ -269,7 +294,7 @@ func TestNoSessionsNoRoutes(t *testing.T) { | |||
| 269 | 294 | ||
| 270 | 295 | _, err = New(Config{ | |
| 271 | 296 | Repos: failing{}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | |
| 272 | - | PublicURL: "https://forge.example", Sessions: &fakeSessions{}, | |
| 297 | + | PublicURL: "https://forge.example", Sessions: mocks.NewSessionAuth(t), | |
| 273 | 298 | }) | |
| 274 | 299 | assert.Error(t, err, "sessions need a limiter") | |
| 275 | 300 | } | |
| @@ -13,30 +13,54 @@ import ( | |||||
| 13 | "time" | 13 | "time" | ||
| 14 | 14 | ||||
| 15 | "github.com/stretchr/testify/assert" | 15 | "github.com/stretchr/testify/assert" | ||
| 16 | + | "github.com/stretchr/testify/mock" | |||
| 16 | "github.com/stretchr/testify/require" | 17 | "github.com/stretchr/testify/require" | ||
| 17 | 18 | ||||
| 18 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | 19 | "bnema.dev/ermite-api/gen/ermite/ermiteconnect" | ||
| 20 | + | "bnema.dev/ermite/internal/adapters/web/mocks" | |||
| 19 | ) | 21 | ) | ||
| 20 | 22 | ||||
| 21 | - | // fakeSessions accepts one token. | 23 | + | // nonceA is the one valid login link of the mocks. |
| 22 | - | type fakeSessions struct { | 24 | + | var nonceA = strings.Repeat("a", 43) |
| 23 | - | valid string | 25 | + | |
| 24 | - | err error | 26 | + | // newVerifier is a SessionAuth that accepts the session token valid, checked |
| 25 | - | used []string | 27 | + | // by verify when it is not nil, and that does not know any login link. A |
| 28 | + | // call that is not expected fails the test. | |||
| 29 | + | func newVerifier(t *testing.T, valid string, verify func(token string) (bool, error)) *mocks.SessionAuth { | |||
| 30 | + | t.Helper() | |||
| 31 | + | if verify == nil { | |||
| 32 | + | verify = func(token string) (bool, error) { return token == valid, nil } | |||
| 33 | + | } | |||
| 34 | + | m := mocks.NewSessionAuth(t) | |||
| 35 | + | m.EXPECT().Verify(mock.Anything, mock.Anything).RunAndReturn( | |||
| 36 | + | func(ctx context.Context, token string) (context.Context, bool, error) { | |||
| 37 | + | ok, err := verify(token) | |||
| 38 | + | return ctx, ok, err | |||
| 39 | + | }).Maybe() | |||
| 40 | + | return m | |||
| 26 | } | 41 | } | ||
| 27 | 42 | ||||
| 28 | - | func (f *fakeSessions) LoginLinkValid(n string) bool { return n == strings.Repeat("a", 43) } | 43 | + | // expectLogin makes nonceA the one link of m, answering with the token valid. |
| 44 | + | func expectLogin(m *mocks.SessionAuth, valid string) { | |||
| 45 | + | m.EXPECT().LoginLinkValid(mock.Anything).RunAndReturn(func(n string) bool { return n == nonceA }).Maybe() | |||
| 46 | + | m.EXPECT().Login(mock.Anything, nonceA).Return(valid, time.Hour, nil).Maybe() | |||
| 47 | + | m.EXPECT().Login(mock.Anything, mock.Anything).Return("", time.Duration(0), ErrLinkRejected).Maybe() | |||
| 48 | + | } | |||
| 29 | 49 | ||||
| 30 | - | func (f *fakeSessions) Login(_ context.Context, n string) (string, time.Duration, error) { | 50 | + | // newSessions is a SessionAuth with one session token and one login link. |
| 31 | - | f.used = append(f.used, n) | 51 | + | func newSessions(t *testing.T, valid string) *mocks.SessionAuth { |
| 32 | - | if n != strings.Repeat("a", 43) { | 52 | + | t.Helper() |
| 33 | - | return "", 0, ErrLinkRejected | 53 | + | m := newVerifier(t, valid, nil) |
| 34 | - | } | 54 | + | expectLogin(m, valid) |
| 35 | - | return f.valid, time.Hour, nil | 55 | + | return m |
| 36 | } | 56 | } | ||
| 37 | 57 | ||||
| 38 | - | func (f *fakeSessions) Verify(ctx context.Context, tok string) (context.Context, bool, error) { | 58 | + | // newBrokenSessions is a SessionAuth whose check cannot be made. |
| 39 | - | return ctx, f.err == nil && tok == f.valid, f.err | 59 | + | func newBrokenSessions(t *testing.T, valid string) *mocks.SessionAuth { |
| 60 | + | t.Helper() | |||
| 61 | + | m := newVerifier(t, valid, func(string) (bool, error) { return false, errors.New("kv down") }) | |||
| 62 | + | expectLogin(m, valid) | |||
| 63 | + | return m | |||
| 40 | } | 64 | } | ||
| 41 | 65 | ||||
| 42 | type countingLimiter struct{ fails int } | 66 | type countingLimiter struct{ fails int } | ||
| @@ -87,7 +111,7 @@ func TestValidNonce(t *testing.T) { | |||||
| 87 | // a session unless it is the login. | 111 | // a session unless it is the login. | ||
| 88 | func TestGate(t *testing.T) { | 112 | func TestGate(t *testing.T) { | ||
| 89 | t.Parallel() | 113 | t.Parallel() | ||
| 90 | - | srv, lim := newSessionServer(t, &fakeSessions{valid: "tok"}, "https://forge.example") | 114 | + | srv, lim := newSessionServer(t, newSessions(t, "tok"), "https://forge.example") |
| 91 | do := func(path, origin, cookie string) int { | 115 | do := func(path, origin, cookie string) int { | ||
| 92 | req := httptest.NewRequest(http.MethodPost, path, nil) | 116 | req := httptest.NewRequest(http.MethodPost, path, nil) | ||
| 93 | req.Host = "forge.example" | 117 | req.Host = "forge.example" | ||
| @@ -126,7 +150,7 @@ func TestGate(t *testing.T) { | |||||
| 126 | // must not turn another route into a "login". | 150 | // must not turn another route into a "login". | ||
| 127 | func TestGateLoginExemptionIsExact(t *testing.T) { | 151 | func TestGateLoginExemptionIsExact(t *testing.T) { | ||
| 128 | t.Parallel() | 152 | t.Parallel() | ||
| 129 | - | sess := &fakeSessions{valid: "tok"} | 153 | + | sess := newVerifier(t, "tok", nil) // no Login is expected until the last request |
| 130 | lim := &countingLimiter{} | 154 | lim := &countingLimiter{} | ||
| 131 | srv, err := New(Config{ | 155 | srv, err := New(Config{ | ||
| 132 | Repos: failing{err: errors.New("x")}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | 156 | Repos: failing{err: errors.New("x")}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | ||
| @@ -144,7 +168,7 @@ func TestGateLoginExemptionIsExact(t *testing.T) { | |||||
| 144 | srv.ServeHTTP(rec, req) | 168 | srv.ServeHTTP(rec, req) | ||
| 145 | return rec.Code | 169 | return rec.Code | ||
| 146 | } | 170 | } | ||
| 147 | - | nonce := strings.Repeat("a", 43) | 171 | + | nonce := nonceA |
| 148 | for _, target := range []string{ | 172 | for _, target := range []string{ | ||
| 149 | "/-%2Flogin%2Fx/merge-requests/1/close", | 173 | "/-%2Flogin%2Fx/merge-requests/1/close", | ||
| 150 | "/-%2flogin%2fx/merge-requests/1/close", | 174 | "/-%2flogin%2fx/merge-requests/1/close", | ||
| @@ -163,17 +187,18 @@ func TestGateLoginExemptionIsExact(t *testing.T) { | |||||
| 163 | } { | 187 | } { | ||
| 164 | assert.Equal(t, http.StatusForbidden, post(target, ""), target) | 188 | assert.Equal(t, http.StatusForbidden, post(target, ""), target) | ||
| 165 | } | 189 | } | ||
| 166 | - | assert.Empty(t, sess.used, "no link was used") | 190 | + | // No Login was expected so far: the mock fails the test on any call. |
| 191 | + | sess.AssertNotCalled(t, "Login", mock.Anything, mock.Anything) | |||
| 167 | 192 | ||||
| 193 | + | sess.EXPECT().Login(mock.Anything, nonce).Return("tok", time.Hour, nil).Once() | |||
| 168 | assert.Equal(t, http.StatusSeeOther, post("/-/login/"+nonce, ""), "a valid link still logs in") | 194 | assert.Equal(t, http.StatusSeeOther, post("/-/login/"+nonce, ""), "a valid link still logs in") | ||
| 169 | - | assert.Equal(t, []string{nonce}, sess.used) | |||
| 170 | } | 195 | } | ||
| 171 | 196 | ||||
| 172 | // A session check that cannot be made fails closed (anonymous) and keeps the | 197 | // A session check that cannot be made fails closed (anonymous) and keeps the | ||
| 173 | // cookie; a check that says no clears it. | 198 | // cookie; a check that says no clears it. | ||
| 174 | func TestWithSessionFailsClosed(t *testing.T) { | 199 | func TestWithSessionFailsClosed(t *testing.T) { | ||
| 175 | t.Parallel() | 200 | t.Parallel() | ||
| 176 | - | broken := &fakeSessions{valid: "tok", err: errors.New("kv down")} | 201 | + | broken := newBrokenSessions(t, "tok") |
| 177 | srv, _ := newSessionServer(t, broken, "https://forge.example") | 202 | srv, _ := newSessionServer(t, broken, "https://forge.example") | ||
| 178 | req := httptest.NewRequest(http.MethodGet, "/", nil) | 203 | req := httptest.NewRequest(http.MethodGet, "/", nil) | ||
| 179 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "tok"}) | 204 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "tok"}) | ||
| @@ -182,7 +207,7 @@ func TestWithSessionFailsClosed(t *testing.T) { | |||||
| 182 | assert.False(t, SignedIn(r.Context())) | 207 | assert.False(t, SignedIn(r.Context())) | ||
| 183 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | 208 | assert.Empty(t, rec.Header().Values("Set-Cookie")) | ||
| 184 | 209 | ||||
| 185 | - | srv, _ = newSessionServer(t, &fakeSessions{valid: "tok"}, "https://forge.example") | 210 | + | srv, _ = newSessionServer(t, newSessions(t, "tok"), "https://forge.example") |
| 186 | req = httptest.NewRequest(http.MethodGet, "/", nil) | 211 | req = httptest.NewRequest(http.MethodGet, "/", nil) | ||
| 187 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"}) | 212 | req.AddCookie(&http.Cookie{Name: CookieName, Value: "bad"}) | ||
| 188 | rec = httptest.NewRecorder() | 213 | rec = httptest.NewRecorder() | ||
| @@ -200,7 +225,7 @@ func TestWithSessionFailsClosed(t *testing.T) { | |||||
| 200 | // writer, the raw file handler and the event stream keep it. | 225 | // writer, the raw file handler and the event stream keep it. | ||
| 201 | func TestInvalidCookieAnswerIsNotCached(t *testing.T) { | 226 | func TestInvalidCookieAnswerIsNotCached(t *testing.T) { | ||
| 202 | t.Parallel() | 227 | t.Parallel() | ||
| 203 | - | sess := &fakeSessions{valid: "tok"} | 228 | + | sess := newSessions(t, "tok") |
| 204 | srv, _ := newSessionServer(t, sess, "https://forge.example") | 229 | srv, _ := newSessionServer(t, sess, "https://forge.example") | ||
| 205 | 230 | ||||
| 206 | // withSession alone. | 231 | // withSession alone. | ||
| @@ -213,7 +238,7 @@ func TestInvalidCookieAnswerIsNotCached(t *testing.T) { | |||||
| 213 | assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0") | 238 | assert.Contains(t, rec.Header().Get("Set-Cookie"), "Max-Age=0") | ||
| 214 | 239 | ||||
| 215 | // A check that cannot be made keeps the cookie and the headers as they are. | 240 | // A check that cannot be made keeps the cookie and the headers as they are. | ||
| 216 | - | broken, _ := newSessionServer(t, &fakeSessions{valid: "tok", err: errors.New("kv down")}, "https://forge.example") | 241 | + | broken, _ := newSessionServer(t, newBrokenSessions(t, "tok"), "https://forge.example") |
| 217 | rec = httptest.NewRecorder() | 242 | rec = httptest.NewRecorder() | ||
| 218 | broken.withSession(rec, req) | 243 | broken.withSession(rec, req) | ||
| 219 | assert.Empty(t, rec.Header().Values("Cache-Control")) | 244 | assert.Empty(t, rec.Header().Values("Cache-Control")) | ||
| @@ -269,7 +294,7 @@ func TestNoSessionsNoRoutes(t *testing.T) { | |||||
| 269 | 294 | ||||
| 270 | _, err = New(Config{ | 295 | _, err = New(Config{ | ||
| 271 | Repos: failing{}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | 296 | Repos: failing{}, Git: ermiteconnect.NewGitServiceClient(http.DefaultClient, "http://unused.invalid"), | ||
| 272 | - | PublicURL: "https://forge.example", Sessions: &fakeSessions{}, | 297 | + | PublicURL: "https://forge.example", Sessions: mocks.NewSessionAuth(t), |
| 273 | }) | 298 | }) | ||
| 274 | assert.Error(t, err, "sessions need a limiter") | 299 | assert.Error(t, err, "sessions need a limiter") | ||
| 275 | } | 300 | } | ||
M internal/adapters/web/web.go +3 -0
| @@ -86,6 +86,9 @@ type Server struct { | |||
| 86 | 86 | assets *assetSet | |
| 87 | 87 | md *markdown | |
| 88 | 88 | mux *http.ServeMux | |
| 89 | + | // refsFan shares the fragments of the refs page between the watchers of | |
| 90 | + | // a push; entries are few and short-lived. | |
| 91 | + | refsFan fanout | |
| 89 | 92 | } | |
| 90 | 93 | ||
| 91 | 94 | // New builds the web handler. | |
| @@ -86,6 +86,9 @@ type Server struct { | |||||
| 86 | assets *assetSet | 86 | assets *assetSet | ||
| 87 | md *markdown | 87 | md *markdown | ||
| 88 | mux *http.ServeMux | 88 | mux *http.ServeMux | ||
| 89 | + | // refsFan shares the fragments of the refs page between the watchers of | |||
| 90 | + | // a push; entries are few and short-lived. | |||
| 91 | + | refsFan fanout | |||
| 89 | } | 92 | } | ||
| 90 | 93 | ||||
| 91 | // New builds the web handler. | 94 | // New builds the web handler. | ||
M internal/app/browse.go +29 -16
| @@ -49,16 +49,26 @@ func (b *Browse) WithHistoryCache(maxEntries int, maxBytes int64) *Browse { | |||
| 49 | 49 | return b | |
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | - | // authorize returns nil if p may read the repository. | |
| 53 | - | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) error { | |
| 52 | + | // authorize returns the repository if p may read it. The caller uses it to | |
| 53 | + | // tell a repository from a later one created under the same name. | |
| 54 | + | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) (domain.Repo, error) { | |
| 54 | 55 | repo, err := lookupRepo(ctx, b.meta, name) | |
| 55 | 56 | if err != nil { | |
| 56 | - | return err | |
| 57 | + | return domain.Repo{}, err | |
| 57 | 58 | } | |
| 58 | 59 | if !canRead(p, repo) { | |
| 59 | - | return domain.ErrNotFound | |
| 60 | + | return domain.Repo{}, domain.ErrNotFound | |
| 60 | 61 | } | |
| 61 | - | return nil | |
| 62 | + | return repo, nil | |
| 63 | + | } | |
| 64 | + | ||
| 65 | + | // repoEpoch identifies one life of a repository name: its creation time in | |
| 66 | + | // nanoseconds, 0 if unknown. | |
| 67 | + | func repoEpoch(r domain.Repo) int64 { | |
| 68 | + | if r.CreatedAt.IsZero() { | |
| 69 | + | return 0 | |
| 70 | + | } | |
| 71 | + | return r.CreatedAt.UnixNano() | |
| 62 | 72 | } | |
| 63 | 73 | ||
| 64 | 74 | // ListRefsParams selects the refs to list. | |
| @@ -105,7 +115,7 @@ func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.R | |||
| 105 | 115 | if err != nil { | |
| 106 | 116 | return RefPage{}, err | |
| 107 | 117 | } | |
| 108 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 118 | + | if _, err := b.authorize(ctx, p, repo); err != nil { | |
| 109 | 119 | return RefPage{}, err | |
| 110 | 120 | } | |
| 111 | 121 | all, err := b.reader.ListRefs(ctx, repo) | |
| @@ -168,7 +178,7 @@ func decodeRefToken(tok string) (*refCursor, error) { | |||
| 168 | 178 | ||
| 169 | 179 | // ResolveRevision turns a revision into the commit it designates. | |
| 170 | 180 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | |
| 171 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 181 | + | if _, err := b.authorize(ctx, p, repo); err != nil { | |
| 172 | 182 | return domain.ResolvedRevision{}, err | |
| 173 | 183 | } | |
| 174 | 184 | return b.reader.ResolveRevision(ctx, repo, rev) | |
| @@ -177,7 +187,7 @@ func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo d | |||
| 177 | 187 | // ResolveRefPath splits a path such as "feature/x/dir/file" into the longest | |
| 178 | 188 | // leading ref and the path inside it. | |
| 179 | 189 | func (b *Browse) ResolveRefPath(ctx context.Context, p domain.Principal, repo domain.RepoName, path string) (domain.RefPath, error) { | |
| 180 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 190 | + | if _, err := b.authorize(ctx, p, repo); err != nil { | |
| 181 | 191 | return domain.RefPath{}, err | |
| 182 | 192 | } | |
| 183 | 193 | return b.reader.ResolveRefPath(ctx, repo, path) | |
| @@ -186,7 +196,8 @@ func (b *Browse) ResolveRefPath(ctx context.Context, p domain.Principal, repo do | |||
| 186 | 196 | // GetTree lists a directory at a revision. With includeLastCommit, entries get | |
| 187 | 197 | // their last commit, for at most domain.MaxLastCommitEntries of them. | |
| 188 | 198 | func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error) { | |
| 189 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 199 | + | r, err := b.authorize(ctx, p, repo) | |
| 200 | + | if err != nil { | |
| 190 | 201 | return domain.Tree{}, err | |
| 191 | 202 | } | |
| 192 | 203 | // The last commits are the costly part (a history walk per entry) and | |
| @@ -195,7 +206,7 @@ func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.Re | |||
| 195 | 206 | if !includeLastCommit || !rev.IsObjectID() { | |
| 196 | 207 | return b.reader.GetTree(ctx, repo, rev, path, includeLastCommit) | |
| 197 | 208 | } | |
| 198 | - | key := historyKey{repo: repo, rev: rev.String(), path: path.String(), kind: kindTreeLast} | |
| 209 | + | key := historyKey{repo: repo, created: repoEpoch(r), rev: rev.String(), path: path.String(), kind: kindTreeLast} | |
| 199 | 210 | if e, ok := b.history.get(key); ok { | |
| 200 | 211 | return cloneTree(e.tree), nil | |
| 201 | 212 | } | |
| @@ -210,7 +221,7 @@ func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.Re | |||
| 210 | 221 | // OpenBlob returns the description of a file and a reader of its content. The | |
| 211 | 222 | // caller closes the reader. | |
| 212 | 223 | func (b *Browse) OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error) { | |
| 213 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 224 | + | if _, err := b.authorize(ctx, p, repo); err != nil { | |
| 214 | 225 | return domain.BlobInfo{}, nil, err | |
| 215 | 226 | } | |
| 216 | 227 | return b.reader.OpenBlob(ctx, repo, rev, path) | |
| @@ -254,10 +265,11 @@ func (b *Browse) ListCommits(ctx context.Context, p domain.Principal, repo domai | |||
| 254 | 265 | } | |
| 255 | 266 | q.Revision, q.Skip = domain.RevisionOf(head), skip | |
| 256 | 267 | } | |
| 257 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 268 | + | r, err := b.authorize(ctx, p, repo) | |
| 269 | + | if err != nil { | |
| 258 | 270 | return CommitPage{}, err | |
| 259 | 271 | } | |
| 260 | - | res, err := b.listCommits(ctx, repo, q) | |
| 272 | + | res, err := b.listCommits(ctx, r, q) | |
| 261 | 273 | if err != nil { | |
| 262 | 274 | return CommitPage{}, err | |
| 263 | 275 | } | |
| @@ -271,11 +283,12 @@ func (b *Browse) ListCommits(ctx context.Context, p domain.Principal, repo domai | |||
| 271 | 283 | // listCommits asks the reader, through the history cache for the first | |
| 272 | 284 | // commit of a path at a full commit id (the "last change" of a file): that | |
| 273 | 285 | // answer is immutable. | |
| 274 | - | func (b *Browse) listCommits(ctx context.Context, repo domain.RepoName, q ports.CommitQuery) (ports.CommitPage, error) { | |
| 286 | + | func (b *Browse) listCommits(ctx context.Context, r domain.Repo, q ports.CommitQuery) (ports.CommitPage, error) { | |
| 287 | + | repo := r.Name | |
| 275 | 288 | if q.Limit != 1 || q.Skip != 0 || !q.Revision.IsObjectID() { | |
| 276 | 289 | return b.reader.ListCommits(ctx, repo, q) | |
| 277 | 290 | } | |
| 278 | - | key := historyKey{repo: repo, rev: q.Revision.String(), path: q.Path.String(), kind: kindPage, limit: q.Limit} | |
| 291 | + | key := historyKey{repo: r.Name, created: repoEpoch(r), rev: q.Revision.String(), path: q.Path.String(), kind: kindPage, limit: q.Limit} | |
| 279 | 292 | if e, ok := b.history.get(key); ok { | |
| 280 | 293 | return clonePage(e.page), nil | |
| 281 | 294 | } | |
| @@ -318,7 +331,7 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int, | |||
| 318 | 331 | // GetCommit returns a commit with its statistics and its diff against the | |
| 319 | 332 | // first parent, within the default diff limits. | |
| 320 | 333 | func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error) { | |
| 321 | - | if err := b.authorize(ctx, p, repo); err != nil { | |
| 334 | + | if _, err := b.authorize(ctx, p, repo); err != nil { | |
| 322 | 335 | return domain.CommitDetail{}, err | |
| 323 | 336 | } | |
| 324 | 337 | return b.reader.GetCommit(ctx, repo, rev, domain.DefaultDiffLimits()) | |
| @@ -49,16 +49,26 @@ func (b *Browse) WithHistoryCache(maxEntries int, maxBytes int64) *Browse { | |||||
| 49 | return b | 49 | return b | ||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | - | // authorize returns nil if p may read the repository. | 52 | + | // authorize returns the repository if p may read it. The caller uses it to |
| 53 | - | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) error { | 53 | + | // tell a repository from a later one created under the same name. |
| 54 | + | func (b *Browse) authorize(ctx context.Context, p domain.Principal, name domain.RepoName) (domain.Repo, error) { | |||
| 54 | repo, err := lookupRepo(ctx, b.meta, name) | 55 | repo, err := lookupRepo(ctx, b.meta, name) | ||
| 55 | if err != nil { | 56 | if err != nil { | ||
| 56 | - | return err | 57 | + | return domain.Repo{}, err |
| 57 | } | 58 | } | ||
| 58 | if !canRead(p, repo) { | 59 | if !canRead(p, repo) { | ||
| 59 | - | return domain.ErrNotFound | 60 | + | return domain.Repo{}, domain.ErrNotFound |
| 60 | } | 61 | } | ||
| 61 | - | return nil | 62 | + | return repo, nil |
| 63 | + | } | |||
| 64 | + | ||||
| 65 | + | // repoEpoch identifies one life of a repository name: its creation time in | |||
| 66 | + | // nanoseconds, 0 if unknown. | |||
| 67 | + | func repoEpoch(r domain.Repo) int64 { | |||
| 68 | + | if r.CreatedAt.IsZero() { | |||
| 69 | + | return 0 | |||
| 70 | + | } | |||
| 71 | + | return r.CreatedAt.UnixNano() | |||
| 62 | } | 72 | } | ||
| 63 | 73 | ||||
| 64 | // ListRefsParams selects the refs to list. | 74 | // ListRefsParams selects the refs to list. | ||
| @@ -105,7 +115,7 @@ func (b *Browse) ListRefs(ctx context.Context, p domain.Principal, repo domain.R | |||||
| 105 | if err != nil { | 115 | if err != nil { | ||
| 106 | return RefPage{}, err | 116 | return RefPage{}, err | ||
| 107 | } | 117 | } | ||
| 108 | - | if err := b.authorize(ctx, p, repo); err != nil { | 118 | + | if _, err := b.authorize(ctx, p, repo); err != nil { |
| 109 | return RefPage{}, err | 119 | return RefPage{}, err | ||
| 110 | } | 120 | } | ||
| 111 | all, err := b.reader.ListRefs(ctx, repo) | 121 | all, err := b.reader.ListRefs(ctx, repo) | ||
| @@ -168,7 +178,7 @@ func decodeRefToken(tok string) (*refCursor, error) { | |||||
| 168 | 178 | ||||
| 169 | // ResolveRevision turns a revision into the commit it designates. | 179 | // ResolveRevision turns a revision into the commit it designates. | ||
| 170 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | 180 | func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.ResolvedRevision, error) { | ||
| 171 | - | if err := b.authorize(ctx, p, repo); err != nil { | 181 | + | if _, err := b.authorize(ctx, p, repo); err != nil { |
| 172 | return domain.ResolvedRevision{}, err | 182 | return domain.ResolvedRevision{}, err | ||
| 173 | } | 183 | } | ||
| 174 | return b.reader.ResolveRevision(ctx, repo, rev) | 184 | return b.reader.ResolveRevision(ctx, repo, rev) | ||
| @@ -177,7 +187,7 @@ func (b *Browse) ResolveRevision(ctx context.Context, p domain.Principal, repo d | |||||
| 177 | // ResolveRefPath splits a path such as "feature/x/dir/file" into the longest | 187 | // ResolveRefPath splits a path such as "feature/x/dir/file" into the longest | ||
| 178 | // leading ref and the path inside it. | 188 | // leading ref and the path inside it. | ||
| 179 | func (b *Browse) ResolveRefPath(ctx context.Context, p domain.Principal, repo domain.RepoName, path string) (domain.RefPath, error) { | 189 | func (b *Browse) ResolveRefPath(ctx context.Context, p domain.Principal, repo domain.RepoName, path string) (domain.RefPath, error) { | ||
| 180 | - | if err := b.authorize(ctx, p, repo); err != nil { | 190 | + | if _, err := b.authorize(ctx, p, repo); err != nil { |
| 181 | return domain.RefPath{}, err | 191 | return domain.RefPath{}, err | ||
| 182 | } | 192 | } | ||
| 183 | return b.reader.ResolveRefPath(ctx, repo, path) | 193 | return b.reader.ResolveRefPath(ctx, repo, path) | ||
| @@ -186,7 +196,8 @@ func (b *Browse) ResolveRefPath(ctx context.Context, p domain.Principal, repo do | |||||
| 186 | // GetTree lists a directory at a revision. With includeLastCommit, entries get | 196 | // GetTree lists a directory at a revision. With includeLastCommit, entries get | ||
| 187 | // their last commit, for at most domain.MaxLastCommitEntries of them. | 197 | // their last commit, for at most domain.MaxLastCommitEntries of them. | ||
| 188 | func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error) { | 198 | func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath, includeLastCommit bool) (domain.Tree, error) { | ||
| 189 | - | if err := b.authorize(ctx, p, repo); err != nil { | 199 | + | r, err := b.authorize(ctx, p, repo) |
| 200 | + | if err != nil { | |||
| 190 | return domain.Tree{}, err | 201 | return domain.Tree{}, err | ||
| 191 | } | 202 | } | ||
| 192 | // The last commits are the costly part (a history walk per entry) and | 203 | // The last commits are the costly part (a history walk per entry) and | ||
| @@ -195,7 +206,7 @@ func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.Re | |||||
| 195 | if !includeLastCommit || !rev.IsObjectID() { | 206 | if !includeLastCommit || !rev.IsObjectID() { | ||
| 196 | return b.reader.GetTree(ctx, repo, rev, path, includeLastCommit) | 207 | return b.reader.GetTree(ctx, repo, rev, path, includeLastCommit) | ||
| 197 | } | 208 | } | ||
| 198 | - | key := historyKey{repo: repo, rev: rev.String(), path: path.String(), kind: kindTreeLast} | 209 | + | key := historyKey{repo: repo, created: repoEpoch(r), rev: rev.String(), path: path.String(), kind: kindTreeLast} |
| 199 | if e, ok := b.history.get(key); ok { | 210 | if e, ok := b.history.get(key); ok { | ||
| 200 | return cloneTree(e.tree), nil | 211 | return cloneTree(e.tree), nil | ||
| 201 | } | 212 | } | ||
| @@ -210,7 +221,7 @@ func (b *Browse) GetTree(ctx context.Context, p domain.Principal, repo domain.Re | |||||
| 210 | // OpenBlob returns the description of a file and a reader of its content. The | 221 | // OpenBlob returns the description of a file and a reader of its content. The | ||
| 211 | // caller closes the reader. | 222 | // caller closes the reader. | ||
| 212 | func (b *Browse) OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error) { | 223 | func (b *Browse) OpenBlob(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision, path domain.TreePath) (domain.BlobInfo, io.ReadCloser, error) { | ||
| 213 | - | if err := b.authorize(ctx, p, repo); err != nil { | 224 | + | if _, err := b.authorize(ctx, p, repo); err != nil { |
| 214 | return domain.BlobInfo{}, nil, err | 225 | return domain.BlobInfo{}, nil, err | ||
| 215 | } | 226 | } | ||
| 216 | return b.reader.OpenBlob(ctx, repo, rev, path) | 227 | return b.reader.OpenBlob(ctx, repo, rev, path) | ||
| @@ -254,10 +265,11 @@ func (b *Browse) ListCommits(ctx context.Context, p domain.Principal, repo domai | |||||
| 254 | } | 265 | } | ||
| 255 | q.Revision, q.Skip = domain.RevisionOf(head), skip | 266 | q.Revision, q.Skip = domain.RevisionOf(head), skip | ||
| 256 | } | 267 | } | ||
| 257 | - | if err := b.authorize(ctx, p, repo); err != nil { | 268 | + | r, err := b.authorize(ctx, p, repo) |
| 269 | + | if err != nil { | |||
| 258 | return CommitPage{}, err | 270 | return CommitPage{}, err | ||
| 259 | } | 271 | } | ||
| 260 | - | res, err := b.listCommits(ctx, repo, q) | 272 | + | res, err := b.listCommits(ctx, r, q) |
| 261 | if err != nil { | 273 | if err != nil { | ||
| 262 | return CommitPage{}, err | 274 | return CommitPage{}, err | ||
| 263 | } | 275 | } | ||
| @@ -271,11 +283,12 @@ func (b *Browse) ListCommits(ctx context.Context, p domain.Principal, repo domai | |||||
| 271 | // listCommits asks the reader, through the history cache for the first | 283 | // listCommits asks the reader, through the history cache for the first | ||
| 272 | // commit of a path at a full commit id (the "last change" of a file): that | 284 | // commit of a path at a full commit id (the "last change" of a file): that | ||
| 273 | // answer is immutable. | 285 | // answer is immutable. | ||
| 274 | - | func (b *Browse) listCommits(ctx context.Context, repo domain.RepoName, q ports.CommitQuery) (ports.CommitPage, error) { | 286 | + | func (b *Browse) listCommits(ctx context.Context, r domain.Repo, q ports.CommitQuery) (ports.CommitPage, error) { |
| 287 | + | repo := r.Name | |||
| 275 | if q.Limit != 1 || q.Skip != 0 || !q.Revision.IsObjectID() { | 288 | if q.Limit != 1 || q.Skip != 0 || !q.Revision.IsObjectID() { | ||
| 276 | return b.reader.ListCommits(ctx, repo, q) | 289 | return b.reader.ListCommits(ctx, repo, q) | ||
| 277 | } | 290 | } | ||
| 278 | - | key := historyKey{repo: repo, rev: q.Revision.String(), path: q.Path.String(), kind: kindPage, limit: q.Limit} | 291 | + | key := historyKey{repo: r.Name, created: repoEpoch(r), rev: q.Revision.String(), path: q.Path.String(), kind: kindPage, limit: q.Limit} |
| 279 | if e, ok := b.history.get(key); ok { | 292 | if e, ok := b.history.get(key); ok { | ||
| 280 | return clonePage(e.page), nil | 293 | return clonePage(e.page), nil | ||
| 281 | } | 294 | } | ||
| @@ -318,7 +331,7 @@ func decodeCommitToken(tok string, path domain.TreePath) (domain.ObjectID, int, | |||||
| 318 | // GetCommit returns a commit with its statistics and its diff against the | 331 | // GetCommit returns a commit with its statistics and its diff against the | ||
| 319 | // first parent, within the default diff limits. | 332 | // first parent, within the default diff limits. | ||
| 320 | func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error) { | 333 | func (b *Browse) GetCommit(ctx context.Context, p domain.Principal, repo domain.RepoName, rev domain.Revision) (domain.CommitDetail, error) { | ||
| 321 | - | if err := b.authorize(ctx, p, repo); err != nil { | 334 | + | if _, err := b.authorize(ctx, p, repo); err != nil { |
| 322 | return domain.CommitDetail{}, err | 335 | return domain.CommitDetail{}, err | ||
| 323 | } | 336 | } | ||
| 324 | return b.reader.GetCommit(ctx, repo, rev, domain.DefaultDiffLimits()) | 337 | return b.reader.GetCommit(ctx, repo, rev, domain.DefaultDiffLimits()) | ||
M internal/app/browsecache.go +9 -6
| @@ -22,13 +22,16 @@ const ( | |||
| 22 | 22 | // historyKey names an answer that depends only on immutable data: a commit | |
| 23 | 23 | // designated by its full id, the path inside it, and for a page of history | |
| 24 | 24 | // where it starts and how long it is. The repository is part of the key so | |
| 25 | - | // that an answer is never served across a name. | |
| 25 | + | // that an answer is never served across a name, and so is its creation time | |
| 26 | + | // (in nanoseconds): a repository deleted and created again under the same | |
| 27 | + | // name is another repository, whose commit ids must not hit the old answers. | |
| 26 | 28 | type historyKey struct { | |
| 27 | - | repo domain.RepoName | |
| 28 | - | rev string // full commit id | |
| 29 | - | path string | |
| 30 | - | kind historyKind | |
| 31 | - | limit int | |
| 29 | + | repo domain.RepoName | |
| 30 | + | created int64 | |
| 31 | + | rev string // full commit id | |
| 32 | + | path string | |
| 33 | + | kind historyKind | |
| 34 | + | limit int | |
| 32 | 35 | } | |
| 33 | 36 | ||
| 34 | 37 | type historyKind uint8 | |
| @@ -22,13 +22,16 @@ const ( | |||||
| 22 | // historyKey names an answer that depends only on immutable data: a commit | 22 | // historyKey names an answer that depends only on immutable data: a commit | ||
| 23 | // designated by its full id, the path inside it, and for a page of history | 23 | // designated by its full id, the path inside it, and for a page of history | ||
| 24 | // where it starts and how long it is. The repository is part of the key so | 24 | // where it starts and how long it is. The repository is part of the key so | ||
| 25 | - | // that an answer is never served across a name. | 25 | + | // that an answer is never served across a name, and so is its creation time |
| 26 | + | // (in nanoseconds): a repository deleted and created again under the same | |||
| 27 | + | // name is another repository, whose commit ids must not hit the old answers. | |||
| 26 | type historyKey struct { | 28 | type historyKey struct { | ||
| 27 | - | repo domain.RepoName | 29 | + | repo domain.RepoName |
| 28 | - | rev string // full commit id | 30 | + | created int64 |
| 29 | - | path string | 31 | + | rev string // full commit id |
| 30 | - | kind historyKind | 32 | + | path string |
| 31 | - | limit int | 33 | + | kind historyKind |
| 34 | + | limit int | |||
| 32 | } | 35 | } | ||
| 33 | 36 | ||||
| 34 | type historyKind uint8 | 37 | type historyKind uint8 | ||
M internal/app/browsecache_test.go +53 -0
| @@ -7,6 +7,7 @@ import ( | |||
| 7 | 7 | "strings" | |
| 8 | 8 | "sync" | |
| 9 | 9 | "testing" | |
| 10 | + | "time" | |
| 10 | 11 | ||
| 11 | 12 | "github.com/stretchr/testify/assert" | |
| 12 | 13 | "github.com/stretchr/testify/mock" | |
| @@ -15,6 +16,7 @@ import ( | |||
| 15 | 16 | "bnema.dev/ermite/internal/app" | |
| 16 | 17 | "bnema.dev/ermite/internal/domain" | |
| 17 | 18 | "bnema.dev/ermite/internal/ports" | |
| 19 | + | "bnema.dev/ermite/internal/ports/mocks" | |
| 18 | 20 | ) | |
| 19 | 21 | ||
| 20 | 22 | func treePath(t *testing.T, s string) domain.TreePath { | |
| @@ -102,6 +104,57 @@ func TestGetTreeLastCommitCacheKey(t *testing.T) { | |||
| 102 | 104 | } | |
| 103 | 105 | } | |
| 104 | 106 | ||
| 107 | + | // TestHistoryCacheNotServedAcrossRecreate: a repository deleted and created | |
| 108 | + | // again under the same name does not get the answers of the old one for a | |
| 109 | + | // commit id they both could hold. | |
| 110 | + | func TestHistoryCacheNotServedAcrossRecreate(t *testing.T) { | |
| 111 | + | t.Parallel() | |
| 112 | + | ctx := context.Background() | |
| 113 | + | created := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) | |
| 114 | + | rev, dir := domain.RevisionOf(oid("a")), treePath(t, "d") | |
| 115 | + | n := name(t, "pub") | |
| 116 | + | old := domain.Repo{Name: n, Visibility: domain.VisibilityPublic, CreatedAt: created} | |
| 117 | + | again := domain.Repo{Name: n, Visibility: domain.VisibilityPublic, CreatedAt: created.Add(time.Hour)} | |
| 118 | + | ||
| 119 | + | tests := []struct { | |
| 120 | + | name string | |
| 121 | + | call func(b *app.Browse) error | |
| 122 | + | // expect registers the reader call of the old repository (answers) | |
| 123 | + | // and of the new one (not found). | |
| 124 | + | expect func(r *mocks.RepoReader) | |
| 125 | + | }{ | |
| 126 | + | {"tree with last commits", func(b *app.Browse) error { | |
| 127 | + | _, err := b.GetTree(ctx, domain.Anonymous(), n, rev, dir, true) | |
| 128 | + | return err | |
| 129 | + | }, func(r *mocks.RepoReader) { | |
| 130 | + | r.EXPECT().GetTree(mock.Anything, n, rev, dir, true).Return(treeWithLast("b"), nil).Once() | |
| 131 | + | r.EXPECT().GetTree(mock.Anything, n, rev, dir, true).Return(domain.Tree{}, domain.ErrNotFound).Once() | |
| 132 | + | }}, | |
| 133 | + | {"last change of a path", func(b *app.Browse) error { | |
| 134 | + | _, err := b.ListCommits(ctx, domain.Anonymous(), n, app.ListCommitsParams{Revision: rev, Path: dir, PageSize: 1}) | |
| 135 | + | return err | |
| 136 | + | }, func(r *mocks.RepoReader) { | |
| 137 | + | q := ports.CommitQuery{Revision: rev, Path: dir, Limit: 1} | |
| 138 | + | r.EXPECT().ListCommits(mock.Anything, n, q).Return(ports.CommitPage{ | |
| 139 | + | Head: oid("a"), Commits: []domain.Commit{{ID: oid("c")}}, | |
| 140 | + | }, nil).Once() | |
| 141 | + | r.EXPECT().ListCommits(mock.Anything, n, q).Return(ports.CommitPage{}, domain.ErrNotFound).Once() | |
| 142 | + | }}, | |
| 143 | + | } | |
| 144 | + | for _, tc := range tests { | |
| 145 | + | t.Run(tc.name, func(t *testing.T) { | |
| 146 | + | t.Parallel() | |
| 147 | + | f := newBrowseFixture(t) | |
| 148 | + | f.meta.EXPECT().Get(mock.Anything, n).Return(old, nil).Twice() // warm, hit | |
| 149 | + | f.meta.EXPECT().Get(mock.Anything, n).Return(again, nil).Once() | |
| 150 | + | tc.expect(f.reader) | |
| 151 | + | require.NoError(t, tc.call(f.browse)) | |
| 152 | + | require.NoError(t, tc.call(f.browse)) // served from the cache | |
| 153 | + | assert.ErrorIs(t, tc.call(f.browse), domain.ErrNotFound) | |
| 154 | + | }) | |
| 155 | + | } | |
| 156 | + | } | |
| 157 | + | ||
| 105 | 158 | // TestGetTreeCacheKeepsAccessRule: a hit never skips the access check, and an | |
| 106 | 159 | // error is not cached. | |
| 107 | 160 | func TestGetTreeCacheKeepsAccessRule(t *testing.T) { | |
| @@ -7,6 +7,7 @@ import ( | |||||
| 7 | "strings" | 7 | "strings" | ||
| 8 | "sync" | 8 | "sync" | ||
| 9 | "testing" | 9 | "testing" | ||
| 10 | + | "time" | |||
| 10 | 11 | ||||
| 11 | "github.com/stretchr/testify/assert" | 12 | "github.com/stretchr/testify/assert" | ||
| 12 | "github.com/stretchr/testify/mock" | 13 | "github.com/stretchr/testify/mock" | ||
| @@ -15,6 +16,7 @@ import ( | |||||
| 15 | "bnema.dev/ermite/internal/app" | 16 | "bnema.dev/ermite/internal/app" | ||
| 16 | "bnema.dev/ermite/internal/domain" | 17 | "bnema.dev/ermite/internal/domain" | ||
| 17 | "bnema.dev/ermite/internal/ports" | 18 | "bnema.dev/ermite/internal/ports" | ||
| 19 | + | "bnema.dev/ermite/internal/ports/mocks" | |||
| 18 | ) | 20 | ) | ||
| 19 | 21 | ||||
| 20 | func treePath(t *testing.T, s string) domain.TreePath { | 22 | func treePath(t *testing.T, s string) domain.TreePath { | ||
| @@ -102,6 +104,57 @@ func TestGetTreeLastCommitCacheKey(t *testing.T) { | |||||
| 102 | } | 104 | } | ||
| 103 | } | 105 | } | ||
| 104 | 106 | ||||
| 107 | + | // TestHistoryCacheNotServedAcrossRecreate: a repository deleted and created | |||
| 108 | + | // again under the same name does not get the answers of the old one for a | |||
| 109 | + | // commit id they both could hold. | |||
| 110 | + | func TestHistoryCacheNotServedAcrossRecreate(t *testing.T) { | |||
| 111 | + | t.Parallel() | |||
| 112 | + | ctx := context.Background() | |||
| 113 | + | created := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) | |||
| 114 | + | rev, dir := domain.RevisionOf(oid("a")), treePath(t, "d") | |||
| 115 | + | n := name(t, "pub") | |||
| 116 | + | old := domain.Repo{Name: n, Visibility: domain.VisibilityPublic, CreatedAt: created} | |||
| 117 | + | again := domain.Repo{Name: n, Visibility: domain.VisibilityPublic, CreatedAt: created.Add(time.Hour)} | |||
| 118 | + | ||||
| 119 | + | tests := []struct { | |||
| 120 | + | name string | |||
| 121 | + | call func(b *app.Browse) error | |||
| 122 | + | // expect registers the reader call of the old repository (answers) | |||
| 123 | + | // and of the new one (not found). | |||
| 124 | + | expect func(r *mocks.RepoReader) | |||
| 125 | + | }{ | |||
| 126 | + | {"tree with last commits", func(b *app.Browse) error { | |||
| 127 | + | _, err := b.GetTree(ctx, domain.Anonymous(), n, rev, dir, true) | |||
| 128 | + | return err | |||
| 129 | + | }, func(r *mocks.RepoReader) { | |||
| 130 | + | r.EXPECT().GetTree(mock.Anything, n, rev, dir, true).Return(treeWithLast("b"), nil).Once() | |||
| 131 | + | r.EXPECT().GetTree(mock.Anything, n, rev, dir, true).Return(domain.Tree{}, domain.ErrNotFound).Once() | |||
| 132 | + | }}, | |||
| 133 | + | {"last change of a path", func(b *app.Browse) error { | |||
| 134 | + | _, err := b.ListCommits(ctx, domain.Anonymous(), n, app.ListCommitsParams{Revision: rev, Path: dir, PageSize: 1}) | |||
| 135 | + | return err | |||
| 136 | + | }, func(r *mocks.RepoReader) { | |||
| 137 | + | q := ports.CommitQuery{Revision: rev, Path: dir, Limit: 1} | |||
| 138 | + | r.EXPECT().ListCommits(mock.Anything, n, q).Return(ports.CommitPage{ | |||
| 139 | + | Head: oid("a"), Commits: []domain.Commit{{ID: oid("c")}}, | |||
| 140 | + | }, nil).Once() | |||
| 141 | + | r.EXPECT().ListCommits(mock.Anything, n, q).Return(ports.CommitPage{}, domain.ErrNotFound).Once() | |||
| 142 | + | }}, | |||
| 143 | + | } | |||
| 144 | + | for _, tc := range tests { | |||
| 145 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 146 | + | t.Parallel() | |||
| 147 | + | f := newBrowseFixture(t) | |||
| 148 | + | f.meta.EXPECT().Get(mock.Anything, n).Return(old, nil).Twice() // warm, hit | |||
| 149 | + | f.meta.EXPECT().Get(mock.Anything, n).Return(again, nil).Once() | |||
| 150 | + | tc.expect(f.reader) | |||
| 151 | + | require.NoError(t, tc.call(f.browse)) | |||
| 152 | + | require.NoError(t, tc.call(f.browse)) // served from the cache | |||
| 153 | + | assert.ErrorIs(t, tc.call(f.browse), domain.ErrNotFound) | |||
| 154 | + | }) | |||
| 155 | + | } | |||
| 156 | + | } | |||
| 157 | + | ||||
| 105 | // TestGetTreeCacheKeepsAccessRule: a hit never skips the access check, and an | 158 | // TestGetTreeCacheKeepsAccessRule: a hit never skips the access check, and an | ||
| 106 | // error is not cached. | 159 | // error is not cached. | ||
| 107 | func TestGetTreeCacheKeepsAccessRule(t *testing.T) { | 160 | func TestGetTreeCacheKeepsAccessRule(t *testing.T) { | ||