fix(releases): hide draft events and cursors, honour update targets, share range handling
- commit
- 1428b20175e3ed5b7fc47047011f0d9090241e5f
- author
- brice · · 2026-10-21 16:33 UTC
- committer
- brice · · 2026-10-21 17:53 UTC
- parent
- 97e2b33
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
20 files · +572 -115
M .hexcheck.yaml +7 -2
| @@ -21,6 +21,11 @@ components: | |||
| 21 | 21 | role: adapter | |
| 22 | 22 | paths: | |
| 23 | 23 | - internal/clientlimit/** | |
| 24 | + | httprange: | |
| 25 | + | # Byte-range selection: infrastructure shared by the HTTP adapters. | |
| 26 | + | role: adapter | |
| 27 | + | paths: | |
| 28 | + | - internal/httprange/** | |
| 24 | 29 | entrypoints: | |
| 25 | 30 | # cmd/ermite only calls app.Run. | |
| 26 | 31 | role: entrypoint | |
| @@ -80,7 +85,7 @@ allow: | |||
| 80 | 85 | reason: rate-limits Git clients with internal/clientlimit | |
| 81 | 86 | - rule: no-adapter-to-adapter-imports | |
| 82 | 87 | path: internal/adapters/githubapi/** | |
| 83 | - | reason: rate-limits the clients of the REST subset with internal/clientlimit | |
| 88 | + | reason: rate-limits the clients of the REST subset with internal/clientlimit and selects byte ranges with internal/httprange | |
| 84 | 89 | - rule: no-adapter-to-adapter-imports | |
| 85 | 90 | path: internal/adapters/oidchttp/** | |
| 86 | 91 | reason: rate-limits the clients of the OIDC token endpoint with internal/clientlimit | |
| @@ -89,4 +94,4 @@ allow: | |||
| 89 | 94 | reason: rate-limits API clients with internal/clientlimit | |
| 90 | 95 | - rule: no-adapter-to-adapter-imports | |
| 91 | 96 | path: internal/adapters/web/** | |
| 92 | - | reason: rate-limits login attempts with internal/clientlimit | |
| 97 | + | reason: rate-limits login attempts with internal/clientlimit and selects byte ranges with internal/httprange | |
| @@ -21,6 +21,11 @@ components: | |||||
| 21 | role: adapter | 21 | role: adapter | ||
| 22 | paths: | 22 | paths: | ||
| 23 | - internal/clientlimit/** | 23 | - internal/clientlimit/** | ||
| 24 | + | httprange: | |||
| 25 | + | # Byte-range selection: infrastructure shared by the HTTP adapters. | |||
| 26 | + | role: adapter | |||
| 27 | + | paths: | |||
| 28 | + | - internal/httprange/** | |||
| 24 | entrypoints: | 29 | entrypoints: | ||
| 25 | # cmd/ermite only calls app.Run. | 30 | # cmd/ermite only calls app.Run. | ||
| 26 | role: entrypoint | 31 | role: entrypoint | ||
| @@ -80,7 +85,7 @@ allow: | |||||
| 80 | reason: rate-limits Git clients with internal/clientlimit | 85 | reason: rate-limits Git clients with internal/clientlimit | ||
| 81 | - rule: no-adapter-to-adapter-imports | 86 | - rule: no-adapter-to-adapter-imports | ||
| 82 | path: internal/adapters/githubapi/** | 87 | path: internal/adapters/githubapi/** | ||
| 83 | - | reason: rate-limits the clients of the REST subset with internal/clientlimit | 88 | + | reason: rate-limits the clients of the REST subset with internal/clientlimit and selects byte ranges with internal/httprange |
| 84 | - rule: no-adapter-to-adapter-imports | 89 | - rule: no-adapter-to-adapter-imports | ||
| 85 | path: internal/adapters/oidchttp/** | 90 | path: internal/adapters/oidchttp/** | ||
| 86 | reason: rate-limits the clients of the OIDC token endpoint with internal/clientlimit | 91 | reason: rate-limits the clients of the OIDC token endpoint with internal/clientlimit | ||
| @@ -89,4 +94,4 @@ allow: | |||||
| 89 | reason: rate-limits API clients with internal/clientlimit | 94 | reason: rate-limits API clients with internal/clientlimit | ||
| 90 | - rule: no-adapter-to-adapter-imports | 95 | - rule: no-adapter-to-adapter-imports | ||
| 91 | path: internal/adapters/web/** | 96 | path: internal/adapters/web/** | ||
| 92 | - | reason: rate-limits login attempts with internal/clientlimit | 97 | + | reason: rate-limits login attempts with internal/clientlimit and selects byte ranges with internal/httprange |
M internal/adapters/githubapi/releases.go +11 -42
| @@ -14,6 +14,7 @@ import ( | |||
| 14 | 14 | "unicode" | |
| 15 | 15 | ||
| 16 | 16 | "bnema.dev/ermite/internal/domain" | |
| 17 | + | "bnema.dev/ermite/internal/httprange" | |
| 17 | 18 | "bnema.dev/ermite/internal/ports" | |
| 18 | 19 | "github.com/bnema/zerowrap" | |
| 19 | 20 | ) | |
| @@ -235,11 +236,14 @@ type releaseValidationError struct { | |||
| 235 | 236 | ||
| 236 | 237 | func writeReleaseValidation(w http.ResponseWriter, err error, kind, field string) { | |
| 237 | 238 | resource, code, doc := "Release", "invalid", "" | |
| 239 | + | asset := kind == "release-upload" || kind == "release-asset" | |
| 240 | + | if asset { | |
| 241 | + | resource = "ReleaseAsset" | |
| 242 | + | } | |
| 238 | 243 | if errors.Is(err, domain.ErrAlreadyExists) { | |
| 239 | 244 | code = "already_exists" | |
| 240 | 245 | field = "tag_name" | |
| 241 | - | if kind == "release-upload" || kind == "release-asset" { | |
| 242 | - | resource = "ReleaseAsset" | |
| 246 | + | if asset { | |
| 243 | 247 | field = "name" | |
| 244 | 248 | } | |
| 245 | 249 | if kind == "release-upload" { | |
| @@ -280,39 +284,6 @@ func releaseErrorField(text string) string { | |||
| 280 | 284 | return "" | |
| 281 | 285 | } | |
| 282 | 286 | ||
| 283 | - | // assetRange returns a single byte range. Multiple ranges and unknown units are | |
| 284 | - | // ignored. Unsatisfiable or malformed byte ranges return false. | |
| 285 | - | func assetRange(header string, size int64) (offset, length int64, partial, ok bool) { | |
| 286 | - | if header == "" || !strings.HasPrefix(header, "bytes=") || strings.Contains(header, ",") { | |
| 287 | - | return 0, size, false, true | |
| 288 | - | } | |
| 289 | - | spec := strings.TrimSpace(strings.TrimPrefix(header, "bytes=")) | |
| 290 | - | a, b, found := strings.Cut(spec, "-") | |
| 291 | - | if !found || size <= 0 { | |
| 292 | - | return 0, 0, false, false | |
| 293 | - | } | |
| 294 | - | if a == "" { | |
| 295 | - | n, err := strconv.ParseInt(b, 10, 64) | |
| 296 | - | if err != nil || n <= 0 { | |
| 297 | - | return 0, 0, false, false | |
| 298 | - | } | |
| 299 | - | n = min(n, size) | |
| 300 | - | return size - n, n, true, true | |
| 301 | - | } | |
| 302 | - | start, err := strconv.ParseInt(a, 10, 64) | |
| 303 | - | if err != nil || start < 0 || start >= size { | |
| 304 | - | return 0, 0, false, false | |
| 305 | - | } | |
| 306 | - | end := size - 1 | |
| 307 | - | if b != "" { | |
| 308 | - | end, err = strconv.ParseInt(b, 10, 64) | |
| 309 | - | if err != nil || end < start { | |
| 310 | - | return 0, 0, false, false | |
| 311 | - | } | |
| 312 | - | end = min(end, size-1) | |
| 313 | - | } | |
| 314 | - | return start, end - start + 1, true, true | |
| 315 | - | } | |
| 316 | 287 | func releaseContentDisposition(name string) string { | |
| 317 | 288 | name = strings.Map(func(r rune) rune { | |
| 318 | 289 | if unicode.IsControl(r) || r == '"' || r == '\\' || r == '/' || r == unicode.ReplacementChar { | |
| @@ -386,16 +357,16 @@ func (h *Handler) downloadRelease(w http.ResponseWriter, r *http.Request, p doma | |||
| 386 | 357 | w.WriteHeader(304) | |
| 387 | 358 | return | |
| 388 | 359 | } | |
| 389 | - | offset, length, partial, ok := assetRange(r.Header.Get("Range"), a.Size) | |
| 390 | - | if !ok { | |
| 391 | - | w.Header().Set("Content-Range", "bytes */"+strconv.FormatInt(a.Size, 10)) | |
| 360 | + | offset, length, partial, err := httprange.Select(r, a.Size, etag) | |
| 361 | + | if err != nil { | |
| 362 | + | w.Header().Set("Content-Range", httprange.Unsatisfied(a.Size)) | |
| 392 | 363 | w.WriteHeader(416) | |
| 393 | 364 | return | |
| 394 | 365 | } | |
| 395 | 366 | status := 200 | |
| 396 | 367 | if partial { | |
| 397 | 368 | status = 206 | |
| 398 | - | w.Header().Set("Content-Range", "bytes "+strconv.FormatInt(offset, 10)+"-"+strconv.FormatInt(offset+length-1, 10)+"/"+strconv.FormatInt(a.Size, 10)) | |
| 369 | + | w.Header().Set("Content-Range", httprange.ContentRange(offset, length, a.Size)) | |
| 399 | 370 | } | |
| 400 | 371 | w.Header().Set("Content-Length", strconv.FormatInt(length, 10)) | |
| 401 | 372 | if r.Method == http.MethodHead { | |
| @@ -421,9 +392,7 @@ func (h *Handler) downloadRelease(w http.ResponseWriter, r *http.Request, p doma | |||
| 421 | 392 | _, e := writer.Write(chunk.Data) | |
| 422 | 393 | return e | |
| 423 | 394 | }) | |
| 424 | - | // BlobStore.Open reports offset >= size as out of range, including the | |
| 425 | - | // valid empty object at offset zero. It has still checked existence. | |
| 426 | - | if err == nil || (a.Size == 0 && errors.Is(err, domain.ErrOutOfRange)) { | |
| 395 | + | if err == nil { | |
| 427 | 396 | if !started { | |
| 428 | 397 | w.WriteHeader(status) | |
| 429 | 398 | } | |
| @@ -14,6 +14,7 @@ import ( | |||||
| 14 | "unicode" | 14 | "unicode" | ||
| 15 | 15 | ||||
| 16 | "bnema.dev/ermite/internal/domain" | 16 | "bnema.dev/ermite/internal/domain" | ||
| 17 | + | "bnema.dev/ermite/internal/httprange" | |||
| 17 | "bnema.dev/ermite/internal/ports" | 18 | "bnema.dev/ermite/internal/ports" | ||
| 18 | "github.com/bnema/zerowrap" | 19 | "github.com/bnema/zerowrap" | ||
| 19 | ) | 20 | ) | ||
| @@ -235,11 +236,14 @@ type releaseValidationError struct { | |||||
| 235 | 236 | ||||
| 236 | func writeReleaseValidation(w http.ResponseWriter, err error, kind, field string) { | 237 | func writeReleaseValidation(w http.ResponseWriter, err error, kind, field string) { | ||
| 237 | resource, code, doc := "Release", "invalid", "" | 238 | resource, code, doc := "Release", "invalid", "" | ||
| 239 | + | asset := kind == "release-upload" || kind == "release-asset" | |||
| 240 | + | if asset { | |||
| 241 | + | resource = "ReleaseAsset" | |||
| 242 | + | } | |||
| 238 | if errors.Is(err, domain.ErrAlreadyExists) { | 243 | if errors.Is(err, domain.ErrAlreadyExists) { | ||
| 239 | code = "already_exists" | 244 | code = "already_exists" | ||
| 240 | field = "tag_name" | 245 | field = "tag_name" | ||
| 241 | - | if kind == "release-upload" || kind == "release-asset" { | 246 | + | if asset { |
| 242 | - | resource = "ReleaseAsset" | |||
| 243 | field = "name" | 247 | field = "name" | ||
| 244 | } | 248 | } | ||
| 245 | if kind == "release-upload" { | 249 | if kind == "release-upload" { | ||
| @@ -280,39 +284,6 @@ func releaseErrorField(text string) string { | |||||
| 280 | return "" | 284 | return "" | ||
| 281 | } | 285 | } | ||
| 282 | 286 | ||||
| 283 | - | // assetRange returns a single byte range. Multiple ranges and unknown units are | |||
| 284 | - | // ignored. Unsatisfiable or malformed byte ranges return false. | |||
| 285 | - | func assetRange(header string, size int64) (offset, length int64, partial, ok bool) { | |||
| 286 | - | if header == "" || !strings.HasPrefix(header, "bytes=") || strings.Contains(header, ",") { | |||
| 287 | - | return 0, size, false, true | |||
| 288 | - | } | |||
| 289 | - | spec := strings.TrimSpace(strings.TrimPrefix(header, "bytes=")) | |||
| 290 | - | a, b, found := strings.Cut(spec, "-") | |||
| 291 | - | if !found || size <= 0 { | |||
| 292 | - | return 0, 0, false, false | |||
| 293 | - | } | |||
| 294 | - | if a == "" { | |||
| 295 | - | n, err := strconv.ParseInt(b, 10, 64) | |||
| 296 | - | if err != nil || n <= 0 { | |||
| 297 | - | return 0, 0, false, false | |||
| 298 | - | } | |||
| 299 | - | n = min(n, size) | |||
| 300 | - | return size - n, n, true, true | |||
| 301 | - | } | |||
| 302 | - | start, err := strconv.ParseInt(a, 10, 64) | |||
| 303 | - | if err != nil || start < 0 || start >= size { | |||
| 304 | - | return 0, 0, false, false | |||
| 305 | - | } | |||
| 306 | - | end := size - 1 | |||
| 307 | - | if b != "" { | |||
| 308 | - | end, err = strconv.ParseInt(b, 10, 64) | |||
| 309 | - | if err != nil || end < start { | |||
| 310 | - | return 0, 0, false, false | |||
| 311 | - | } | |||
| 312 | - | end = min(end, size-1) | |||
| 313 | - | } | |||
| 314 | - | return start, end - start + 1, true, true | |||
| 315 | - | } | |||
| 316 | func releaseContentDisposition(name string) string { | 287 | func releaseContentDisposition(name string) string { | ||
| 317 | name = strings.Map(func(r rune) rune { | 288 | name = strings.Map(func(r rune) rune { | ||
| 318 | if unicode.IsControl(r) || r == '"' || r == '\\' || r == '/' || r == unicode.ReplacementChar { | 289 | if unicode.IsControl(r) || r == '"' || r == '\\' || r == '/' || r == unicode.ReplacementChar { | ||
| @@ -386,16 +357,16 @@ func (h *Handler) downloadRelease(w http.ResponseWriter, r *http.Request, p doma | |||||
| 386 | w.WriteHeader(304) | 357 | w.WriteHeader(304) | ||
| 387 | return | 358 | return | ||
| 388 | } | 359 | } | ||
| 389 | - | offset, length, partial, ok := assetRange(r.Header.Get("Range"), a.Size) | 360 | + | offset, length, partial, err := httprange.Select(r, a.Size, etag) |
| 390 | - | if !ok { | 361 | + | if err != nil { |
| 391 | - | w.Header().Set("Content-Range", "bytes */"+strconv.FormatInt(a.Size, 10)) | 362 | + | w.Header().Set("Content-Range", httprange.Unsatisfied(a.Size)) |
| 392 | w.WriteHeader(416) | 363 | w.WriteHeader(416) | ||
| 393 | return | 364 | return | ||
| 394 | } | 365 | } | ||
| 395 | status := 200 | 366 | status := 200 | ||
| 396 | if partial { | 367 | if partial { | ||
| 397 | status = 206 | 368 | status = 206 | ||
| 398 | - | w.Header().Set("Content-Range", "bytes "+strconv.FormatInt(offset, 10)+"-"+strconv.FormatInt(offset+length-1, 10)+"/"+strconv.FormatInt(a.Size, 10)) | 369 | + | w.Header().Set("Content-Range", httprange.ContentRange(offset, length, a.Size)) |
| 399 | } | 370 | } | ||
| 400 | w.Header().Set("Content-Length", strconv.FormatInt(length, 10)) | 371 | w.Header().Set("Content-Length", strconv.FormatInt(length, 10)) | ||
| 401 | if r.Method == http.MethodHead { | 372 | if r.Method == http.MethodHead { | ||
| @@ -421,9 +392,7 @@ func (h *Handler) downloadRelease(w http.ResponseWriter, r *http.Request, p doma | |||||
| 421 | _, e := writer.Write(chunk.Data) | 392 | _, e := writer.Write(chunk.Data) | ||
| 422 | return e | 393 | return e | ||
| 423 | }) | 394 | }) | ||
| 424 | - | // BlobStore.Open reports offset >= size as out of range, including the | 395 | + | if err == nil { |
| 425 | - | // valid empty object at offset zero. It has still checked existence. | |||
| 426 | - | if err == nil || (a.Size == 0 && errors.Is(err, domain.ErrOutOfRange)) { | |||
| 427 | if !started { | 396 | if !started { | ||
| 428 | w.WriteHeader(status) | 397 | w.WriteHeader(status) | ||
| 429 | } | 398 | } | ||
M internal/adapters/githubapi/releases_test.go +40 -2
| @@ -166,6 +166,7 @@ func TestReleaseUpload(t *testing.T) { | |||
| 166 | 166 | require.Equal(t, 201, w.Code) | |
| 167 | 167 | w = releaseRequest(t, h, "POST", releaseUploadPath, "abc", nil) | |
| 168 | 168 | require.Equal(t, 422, w.Code) | |
| 169 | + | require.Equal(t, `{"message":"Validation Failed","errors":[{"resource":"ReleaseAsset","code":"invalid","field":"name"}]}`, w.Body.String()) | |
| 169 | 170 | for _, ct := range []string{"", "text/plain"} { | |
| 170 | 171 | expectedType := ct | |
| 171 | 172 | if ct == "" { | |
| @@ -281,7 +282,7 @@ func TestReleaseDownload(t *testing.T) { | |||
| 281 | 282 | {"zero suffix", "GET", "bytes=-0", "", 416, 0, 0, "", "bytes */10"}, | |
| 282 | 283 | {"conditional", "GET", "", `"sha256-abcdef"`, 304, 0, 0, "", ""}, | |
| 283 | 284 | {"head", "HEAD", "", "", 200, 0, 10, "", ""}, | |
| 284 | - | {"head range", "HEAD", "bytes=2-5", "", 206, 2, 4, "", "bytes 2-5/10"}, | |
| 285 | + | {"head ignores range", "HEAD", "bytes=2-5", "", 200, 0, 10, "", ""}, | |
| 285 | 286 | } | |
| 286 | 287 | for _, tc := range cases { | |
| 287 | 288 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -380,7 +381,7 @@ func TestReleaseEmptyAndMissingDownload(t *testing.T) { | |||
| 380 | 381 | err error | |
| 381 | 382 | want int | |
| 382 | 383 | }{ | |
| 383 | - | {"empty", 0, domain.ErrOutOfRange, 200}, | |
| 384 | + | {"empty", 0, nil, 200}, | |
| 384 | 385 | {"missing empty", 0, domain.ErrNotFound, 404}, | |
| 385 | 386 | {"store range", 10, domain.ErrOutOfRange, 416}, | |
| 386 | 387 | } { | |
| @@ -426,3 +427,40 @@ func TestReleaseDownloadGlobalSlot(t *testing.T) { | |||
| 426 | 427 | require.Contains(t, w.Body.String(), "Server busy") | |
| 427 | 428 | require.NotContains(t, h.clients, "192.0.2.1") | |
| 428 | 429 | } | |
| 430 | + | ||
| 431 | + | func TestReleaseDownloadIfRange(t *testing.T) { | |
| 432 | + | for _, tc := range []struct { | |
| 433 | + | name, ifRange string | |
| 434 | + | status int | |
| 435 | + | offset int64 | |
| 436 | + | length int64 | |
| 437 | + | body, cr string | |
| 438 | + | }{ | |
| 439 | + | {"matching", `"sha256-abcdef"`, 206, 2, 4, "2345", "bytes 2-5/10"}, | |
| 440 | + | {"stale", `"sha256-other"`, 200, 0, 10, "0123456789", ""}, | |
| 441 | + | {"date", "Wed, 21 Oct 2026 12:00:00 GMT", 200, 0, 10, "0123456789", ""}, | |
| 442 | + | } { | |
| 443 | + | t.Run(tc.name, func(t *testing.T) { | |
| 444 | + | api := mocks.NewGitHubReleaseAPI(t) | |
| 445 | + | dl := mocks.NewReleaseService(t) | |
| 446 | + | a := downloadAsset(t) | |
| 447 | + | api.EXPECT().AssetByID(mock.Anything, domain.Anonymous(), "acme", "proj", int64(7)).Return(a, nil).Once() | |
| 448 | + | dl.EXPECT().DownloadAsset(mock.Anything, domain.Anonymous(), a.Repo, ports.AssetRef{ID: 7}, tc.offset, tc.length, mock.Anything).RunAndReturn(func(_ context.Context, _ domain.Principal, _ domain.RepoName, _ ports.AssetRef, offset, length int64, send func(ports.AssetChunk) error) error { | |
| 449 | + | require.NoError(t, send(ports.AssetChunk{Asset: &a})) | |
| 450 | + | return send(ports.AssetChunk{Data: []byte("0123456789")[offset : offset+length]}) | |
| 451 | + | }).Once() | |
| 452 | + | w := releaseRequest(t, New(Config{Releases: api, ReleaseDownload: dl}), "GET", releaseAssetPath, "", map[string]string{"Accept": "application/octet-stream", "Range": "bytes=2-5", "If-Range": tc.ifRange}) | |
| 453 | + | require.Equal(t, tc.status, w.Code) | |
| 454 | + | require.Equal(t, tc.body, w.Body.String()) | |
| 455 | + | require.Equal(t, tc.cr, w.Header().Get("Content-Range")) | |
| 456 | + | }) | |
| 457 | + | } | |
| 458 | + | } | |
| 459 | + | ||
| 460 | + | func TestReleaseAssetValidationResource(t *testing.T) { | |
| 461 | + | api := mocks.NewGitHubReleaseAPI(t) | |
| 462 | + | api.EXPECT().UpdateAsset(mock.Anything, domain.Anonymous(), "acme", "proj", int64(7), mock.Anything).Return(nil, fmt.Errorf("%w: asset name is empty", domain.ErrInvalid)).Once() | |
| 463 | + | w := releaseRequest(t, New(Config{Releases: api}), "PATCH", releaseAssetPath, `{"name":""}`, nil) | |
| 464 | + | require.Equal(t, 422, w.Code) | |
| 465 | + | require.Equal(t, `{"message":"Validation Failed","errors":[{"resource":"ReleaseAsset","code":"invalid","field":"name"}]}`, w.Body.String()) | |
| 466 | + | } | |
| @@ -166,6 +166,7 @@ func TestReleaseUpload(t *testing.T) { | |||||
| 166 | require.Equal(t, 201, w.Code) | 166 | require.Equal(t, 201, w.Code) | ||
| 167 | w = releaseRequest(t, h, "POST", releaseUploadPath, "abc", nil) | 167 | w = releaseRequest(t, h, "POST", releaseUploadPath, "abc", nil) | ||
| 168 | require.Equal(t, 422, w.Code) | 168 | require.Equal(t, 422, w.Code) | ||
| 169 | + | require.Equal(t, `{"message":"Validation Failed","errors":[{"resource":"ReleaseAsset","code":"invalid","field":"name"}]}`, w.Body.String()) | |||
| 169 | for _, ct := range []string{"", "text/plain"} { | 170 | for _, ct := range []string{"", "text/plain"} { | ||
| 170 | expectedType := ct | 171 | expectedType := ct | ||
| 171 | if ct == "" { | 172 | if ct == "" { | ||
| @@ -281,7 +282,7 @@ func TestReleaseDownload(t *testing.T) { | |||||
| 281 | {"zero suffix", "GET", "bytes=-0", "", 416, 0, 0, "", "bytes */10"}, | 282 | {"zero suffix", "GET", "bytes=-0", "", 416, 0, 0, "", "bytes */10"}, | ||
| 282 | {"conditional", "GET", "", `"sha256-abcdef"`, 304, 0, 0, "", ""}, | 283 | {"conditional", "GET", "", `"sha256-abcdef"`, 304, 0, 0, "", ""}, | ||
| 283 | {"head", "HEAD", "", "", 200, 0, 10, "", ""}, | 284 | {"head", "HEAD", "", "", 200, 0, 10, "", ""}, | ||
| 284 | - | {"head range", "HEAD", "bytes=2-5", "", 206, 2, 4, "", "bytes 2-5/10"}, | 285 | + | {"head ignores range", "HEAD", "bytes=2-5", "", 200, 0, 10, "", ""}, |
| 285 | } | 286 | } | ||
| 286 | for _, tc := range cases { | 287 | for _, tc := range cases { | ||
| 287 | t.Run(tc.name, func(t *testing.T) { | 288 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -380,7 +381,7 @@ func TestReleaseEmptyAndMissingDownload(t *testing.T) { | |||||
| 380 | err error | 381 | err error | ||
| 381 | want int | 382 | want int | ||
| 382 | }{ | 383 | }{ | ||
| 383 | - | {"empty", 0, domain.ErrOutOfRange, 200}, | 384 | + | {"empty", 0, nil, 200}, |
| 384 | {"missing empty", 0, domain.ErrNotFound, 404}, | 385 | {"missing empty", 0, domain.ErrNotFound, 404}, | ||
| 385 | {"store range", 10, domain.ErrOutOfRange, 416}, | 386 | {"store range", 10, domain.ErrOutOfRange, 416}, | ||
| 386 | } { | 387 | } { | ||
| @@ -426,3 +427,40 @@ func TestReleaseDownloadGlobalSlot(t *testing.T) { | |||||
| 426 | require.Contains(t, w.Body.String(), "Server busy") | 427 | require.Contains(t, w.Body.String(), "Server busy") | ||
| 427 | require.NotContains(t, h.clients, "192.0.2.1") | 428 | require.NotContains(t, h.clients, "192.0.2.1") | ||
| 428 | } | 429 | } | ||
| 430 | + | ||||
| 431 | + | func TestReleaseDownloadIfRange(t *testing.T) { | |||
| 432 | + | for _, tc := range []struct { | |||
| 433 | + | name, ifRange string | |||
| 434 | + | status int | |||
| 435 | + | offset int64 | |||
| 436 | + | length int64 | |||
| 437 | + | body, cr string | |||
| 438 | + | }{ | |||
| 439 | + | {"matching", `"sha256-abcdef"`, 206, 2, 4, "2345", "bytes 2-5/10"}, | |||
| 440 | + | {"stale", `"sha256-other"`, 200, 0, 10, "0123456789", ""}, | |||
| 441 | + | {"date", "Wed, 21 Oct 2026 12:00:00 GMT", 200, 0, 10, "0123456789", ""}, | |||
| 442 | + | } { | |||
| 443 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 444 | + | api := mocks.NewGitHubReleaseAPI(t) | |||
| 445 | + | dl := mocks.NewReleaseService(t) | |||
| 446 | + | a := downloadAsset(t) | |||
| 447 | + | api.EXPECT().AssetByID(mock.Anything, domain.Anonymous(), "acme", "proj", int64(7)).Return(a, nil).Once() | |||
| 448 | + | dl.EXPECT().DownloadAsset(mock.Anything, domain.Anonymous(), a.Repo, ports.AssetRef{ID: 7}, tc.offset, tc.length, mock.Anything).RunAndReturn(func(_ context.Context, _ domain.Principal, _ domain.RepoName, _ ports.AssetRef, offset, length int64, send func(ports.AssetChunk) error) error { | |||
| 449 | + | require.NoError(t, send(ports.AssetChunk{Asset: &a})) | |||
| 450 | + | return send(ports.AssetChunk{Data: []byte("0123456789")[offset : offset+length]}) | |||
| 451 | + | }).Once() | |||
| 452 | + | w := releaseRequest(t, New(Config{Releases: api, ReleaseDownload: dl}), "GET", releaseAssetPath, "", map[string]string{"Accept": "application/octet-stream", "Range": "bytes=2-5", "If-Range": tc.ifRange}) | |||
| 453 | + | require.Equal(t, tc.status, w.Code) | |||
| 454 | + | require.Equal(t, tc.body, w.Body.String()) | |||
| 455 | + | require.Equal(t, tc.cr, w.Header().Get("Content-Range")) | |||
| 456 | + | }) | |||
| 457 | + | } | |||
| 458 | + | } | |||
| 459 | + | ||||
| 460 | + | func TestReleaseAssetValidationResource(t *testing.T) { | |||
| 461 | + | api := mocks.NewGitHubReleaseAPI(t) | |||
| 462 | + | api.EXPECT().UpdateAsset(mock.Anything, domain.Anonymous(), "acme", "proj", int64(7), mock.Anything).Return(nil, fmt.Errorf("%w: asset name is empty", domain.ErrInvalid)).Once() | |||
| 463 | + | w := releaseRequest(t, New(Config{Releases: api}), "PATCH", releaseAssetPath, `{"name":""}`, nil) | |||
| 464 | + | require.Equal(t, 422, w.Code) | |||
| 465 | + | require.Equal(t, `{"message":"Validation Failed","errors":[{"resource":"ReleaseAsset","code":"invalid","field":"name"}]}`, w.Body.String()) | |||
| 466 | + | } | |||
M internal/adapters/rpc/release.go +22 -3
| @@ -27,16 +27,31 @@ var _ ermiteconnect.ReleaseServiceHandler = (*releaseService)(nil) | |||
| 27 | 27 | // releaseMessage fills the computed latest flag and uploaded assets using the | |
| 28 | 28 | // same caller's read permissions. No latest release is a normal repository state. | |
| 29 | 29 | func (s *releaseService) releaseMessage(ctx context.Context, repo domain.RepoName, r domain.Release) (*pb.Release, error) { | |
| 30 | + | latest, err := s.latestID(ctx, repo) | |
| 31 | + | if err != nil { | |
| 32 | + | return nil, err | |
| 33 | + | } | |
| 34 | + | return s.releaseRow(ctx, repo, r, latest) | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | // latestID is the id of the latest release, or 0 when the repository has none. | |
| 38 | + | func (s *releaseService) latestID(ctx context.Context, repo domain.RepoName) (int64, error) { | |
| 30 | 39 | latest, err := s.rel.GetRelease(ctx, caller(ctx), repo, ports.ReleaseRef{Latest: true}) | |
| 31 | 40 | if err != nil && !errors.Is(err, domain.ErrNotFound) { | |
| 32 | - | return nil, err | |
| 41 | + | return 0, err | |
| 33 | 42 | } | |
| 43 | + | return latest.ID, nil | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | // releaseRow converts one release given the id of the latest one, so a list | |
| 47 | + | // resolves the latest release once. | |
| 48 | + | func (s *releaseService) releaseRow(ctx context.Context, repo domain.RepoName, r domain.Release, latest int64) (*pb.Release, error) { | |
| 34 | 49 | assets, err := s.rel.ListAssets(ctx, caller(ctx), repo, r.ID) | |
| 35 | 50 | if err != nil { | |
| 36 | 51 | return nil, err | |
| 37 | 52 | } | |
| 38 | 53 | out := releaseToPB(r) | |
| 39 | - | out.Latest = latest.ID != 0 && latest.ID == r.ID | |
| 54 | + | out.Latest = latest != 0 && latest == r.ID | |
| 40 | 55 | out.Assets = assetsToPB(assets) | |
| 41 | 56 | return out, nil | |
| 42 | 57 | } | |
| @@ -106,8 +121,12 @@ func (s *releaseService) ListReleases(ctx context.Context, req *connect.Request[ | |||
| 106 | 121 | return nil, connectError(ctx, err) | |
| 107 | 122 | } | |
| 108 | 123 | out := &pb.ListReleasesResponse{Releases: make([]*pb.Release, 0, len(page.Releases)), NextPageToken: page.NextPageToken} | |
| 124 | + | latest, err := s.latestID(ctx, repo) | |
| 125 | + | if err != nil { | |
| 126 | + | return nil, connectError(ctx, err) | |
| 127 | + | } | |
| 109 | 128 | for _, r := range page.Releases { | |
| 110 | - | m, err := s.releaseMessage(ctx, repo, r) | |
| 129 | + | m, err := s.releaseRow(ctx, repo, r, latest) | |
| 111 | 130 | if err != nil { | |
| 112 | 131 | return nil, connectError(ctx, err) | |
| 113 | 132 | } | |
| @@ -27,16 +27,31 @@ var _ ermiteconnect.ReleaseServiceHandler = (*releaseService)(nil) | |||||
| 27 | // releaseMessage fills the computed latest flag and uploaded assets using the | 27 | // releaseMessage fills the computed latest flag and uploaded assets using the | ||
| 28 | // same caller's read permissions. No latest release is a normal repository state. | 28 | // same caller's read permissions. No latest release is a normal repository state. | ||
| 29 | func (s *releaseService) releaseMessage(ctx context.Context, repo domain.RepoName, r domain.Release) (*pb.Release, error) { | 29 | func (s *releaseService) releaseMessage(ctx context.Context, repo domain.RepoName, r domain.Release) (*pb.Release, error) { | ||
| 30 | + | latest, err := s.latestID(ctx, repo) | |||
| 31 | + | if err != nil { | |||
| 32 | + | return nil, err | |||
| 33 | + | } | |||
| 34 | + | return s.releaseRow(ctx, repo, r, latest) | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | // latestID is the id of the latest release, or 0 when the repository has none. | |||
| 38 | + | func (s *releaseService) latestID(ctx context.Context, repo domain.RepoName) (int64, error) { | |||
| 30 | latest, err := s.rel.GetRelease(ctx, caller(ctx), repo, ports.ReleaseRef{Latest: true}) | 39 | latest, err := s.rel.GetRelease(ctx, caller(ctx), repo, ports.ReleaseRef{Latest: true}) | ||
| 31 | if err != nil && !errors.Is(err, domain.ErrNotFound) { | 40 | if err != nil && !errors.Is(err, domain.ErrNotFound) { | ||
| 32 | - | return nil, err | 41 | + | return 0, err |
| 33 | } | 42 | } | ||
| 43 | + | return latest.ID, nil | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | // releaseRow converts one release given the id of the latest one, so a list | |||
| 47 | + | // resolves the latest release once. | |||
| 48 | + | func (s *releaseService) releaseRow(ctx context.Context, repo domain.RepoName, r domain.Release, latest int64) (*pb.Release, error) { | |||
| 34 | assets, err := s.rel.ListAssets(ctx, caller(ctx), repo, r.ID) | 49 | assets, err := s.rel.ListAssets(ctx, caller(ctx), repo, r.ID) | ||
| 35 | if err != nil { | 50 | if err != nil { | ||
| 36 | return nil, err | 51 | return nil, err | ||
| 37 | } | 52 | } | ||
| 38 | out := releaseToPB(r) | 53 | out := releaseToPB(r) | ||
| 39 | - | out.Latest = latest.ID != 0 && latest.ID == r.ID | 54 | + | out.Latest = latest != 0 && latest == r.ID |
| 40 | out.Assets = assetsToPB(assets) | 55 | out.Assets = assetsToPB(assets) | ||
| 41 | return out, nil | 56 | return out, nil | ||
| 42 | } | 57 | } | ||
| @@ -106,8 +121,12 @@ func (s *releaseService) ListReleases(ctx context.Context, req *connect.Request[ | |||||
| 106 | return nil, connectError(ctx, err) | 121 | return nil, connectError(ctx, err) | ||
| 107 | } | 122 | } | ||
| 108 | out := &pb.ListReleasesResponse{Releases: make([]*pb.Release, 0, len(page.Releases)), NextPageToken: page.NextPageToken} | 123 | out := &pb.ListReleasesResponse{Releases: make([]*pb.Release, 0, len(page.Releases)), NextPageToken: page.NextPageToken} | ||
| 124 | + | latest, err := s.latestID(ctx, repo) | |||
| 125 | + | if err != nil { | |||
| 126 | + | return nil, connectError(ctx, err) | |||
| 127 | + | } | |||
| 109 | for _, r := range page.Releases { | 128 | for _, r := range page.Releases { | ||
| 110 | - | m, err := s.releaseMessage(ctx, repo, r) | 129 | + | m, err := s.releaseRow(ctx, repo, r, latest) |
| 111 | if err != nil { | 130 | if err != nil { | ||
| 112 | return nil, connectError(ctx, err) | 131 | return nil, connectError(ctx, err) | ||
| 113 | } | 132 | } | ||
M internal/adapters/rpc/release_test.go +50 -0
| @@ -420,3 +420,53 @@ func TestReleaseRPCRejectsInvalidMasksAndSelectors(t *testing.T) { | |||
| 420 | 420 | } | |
| 421 | 421 | requireCode(t, connect.CodeInvalidArgument, err) | |
| 422 | 422 | } | |
| 423 | + | ||
| 424 | + | func TestReleaseRPCListResolvesLatestOnce(t *testing.T) { | |
| 425 | + | repo := mustRepoName(t, "demo") | |
| 426 | + | rel := mocks.NewReleaseService(t) | |
| 427 | + | c, _ := releaseClient(t, rel) | |
| 428 | + | p := domain.Anonymous() | |
| 429 | + | rows := []domain.Release{{ID: 3, Repo: repo, Tag: "c"}, {ID: 2, Repo: repo, Tag: "b"}, {ID: 1, Repo: repo, Tag: "a"}} | |
| 430 | + | rel.EXPECT().ListReleases(mock.Anything, p, repo, 0, "").Return(ports.ReleasePage{Releases: rows}, nil).Once() | |
| 431 | + | rel.EXPECT().GetRelease(mock.Anything, p, repo, ports.ReleaseRef{Latest: true}).Return(rows[1], nil).Once() | |
| 432 | + | for _, r := range rows { | |
| 433 | + | rel.EXPECT().ListAssets(mock.Anything, p, repo, r.ID).Return(nil, nil).Once() | |
| 434 | + | } | |
| 435 | + | res, err := c.ListReleases(context.Background(), connect.NewRequest(&pb.ListReleasesRequest{Repo: "demo"})) | |
| 436 | + | require.NoError(t, err) | |
| 437 | + | require.Len(t, res.Msg.Releases, 3) | |
| 438 | + | for _, m := range res.Msg.Releases { | |
| 439 | + | assert.Equal(t, m.Id == 2, m.Latest, m.Tag) | |
| 440 | + | } | |
| 441 | + | } | |
| 442 | + | ||
| 443 | + | func TestReleaseRPCListWithoutLatest(t *testing.T) { | |
| 444 | + | repo := mustRepoName(t, "demo") | |
| 445 | + | rel := mocks.NewReleaseService(t) | |
| 446 | + | c, _ := releaseClient(t, rel) | |
| 447 | + | p := domain.Anonymous() | |
| 448 | + | rel.EXPECT().ListReleases(mock.Anything, p, repo, 0, "").Return(ports.ReleasePage{Releases: []domain.Release{{ID: 1, Repo: repo}}}, nil).Once() | |
| 449 | + | rel.EXPECT().GetRelease(mock.Anything, p, repo, ports.ReleaseRef{Latest: true}).Return(domain.Release{}, domain.ErrNotFound).Once() | |
| 450 | + | rel.EXPECT().ListAssets(mock.Anything, p, repo, int64(1)).Return(nil, nil).Once() | |
| 451 | + | res, err := c.ListReleases(context.Background(), connect.NewRequest(&pb.ListReleasesRequest{Repo: "demo"})) | |
| 452 | + | require.NoError(t, err) | |
| 453 | + | require.Len(t, res.Msg.Releases, 1) | |
| 454 | + | assert.False(t, res.Msg.Releases[0].Latest) | |
| 455 | + | } | |
| 456 | + | ||
| 457 | + | func TestReleaseRPCDownloadZeroByteAsset(t *testing.T) { | |
| 458 | + | rel := mocks.NewReleaseService(t) | |
| 459 | + | c, _ := releaseClient(t, rel) | |
| 460 | + | a := domain.ReleaseAsset{ID: 9, ReleaseID: 7, Name: "empty", Size: 0} | |
| 461 | + | // The use case sends the metadata and no data for an empty asset. | |
| 462 | + | rel.EXPECT().DownloadAsset(mock.Anything, domain.Anonymous(), mustRepoName(t, "demo"), ports.AssetRef{ID: 9}, int64(0), int64(-1), mock.Anything).RunAndReturn(func(_ context.Context, _ domain.Principal, _ domain.RepoName, _ ports.AssetRef, _, _ int64, send func(ports.AssetChunk) error) error { | |
| 463 | + | return send(ports.AssetChunk{Asset: &a}) | |
| 464 | + | }).Once() | |
| 465 | + | stream, err := c.DownloadReleaseAsset(context.Background(), connect.NewRequest(&pb.DownloadReleaseAssetRequest{Repo: "demo", Selector: &pb.DownloadReleaseAssetRequest_AssetId{AssetId: 9}})) | |
| 466 | + | require.NoError(t, err) | |
| 467 | + | defer stream.Close() | |
| 468 | + | require.True(t, stream.Receive()) | |
| 469 | + | assert.True(t, proto.Equal(assetToPB(a), stream.Msg().Asset)) | |
| 470 | + | assert.False(t, stream.Receive()) | |
| 471 | + | require.NoError(t, stream.Err()) | |
| 472 | + | } | |
| @@ -420,3 +420,53 @@ func TestReleaseRPCRejectsInvalidMasksAndSelectors(t *testing.T) { | |||||
| 420 | } | 420 | } | ||
| 421 | requireCode(t, connect.CodeInvalidArgument, err) | 421 | requireCode(t, connect.CodeInvalidArgument, err) | ||
| 422 | } | 422 | } | ||
| 423 | + | ||||
| 424 | + | func TestReleaseRPCListResolvesLatestOnce(t *testing.T) { | |||
| 425 | + | repo := mustRepoName(t, "demo") | |||
| 426 | + | rel := mocks.NewReleaseService(t) | |||
| 427 | + | c, _ := releaseClient(t, rel) | |||
| 428 | + | p := domain.Anonymous() | |||
| 429 | + | rows := []domain.Release{{ID: 3, Repo: repo, Tag: "c"}, {ID: 2, Repo: repo, Tag: "b"}, {ID: 1, Repo: repo, Tag: "a"}} | |||
| 430 | + | rel.EXPECT().ListReleases(mock.Anything, p, repo, 0, "").Return(ports.ReleasePage{Releases: rows}, nil).Once() | |||
| 431 | + | rel.EXPECT().GetRelease(mock.Anything, p, repo, ports.ReleaseRef{Latest: true}).Return(rows[1], nil).Once() | |||
| 432 | + | for _, r := range rows { | |||
| 433 | + | rel.EXPECT().ListAssets(mock.Anything, p, repo, r.ID).Return(nil, nil).Once() | |||
| 434 | + | } | |||
| 435 | + | res, err := c.ListReleases(context.Background(), connect.NewRequest(&pb.ListReleasesRequest{Repo: "demo"})) | |||
| 436 | + | require.NoError(t, err) | |||
| 437 | + | require.Len(t, res.Msg.Releases, 3) | |||
| 438 | + | for _, m := range res.Msg.Releases { | |||
| 439 | + | assert.Equal(t, m.Id == 2, m.Latest, m.Tag) | |||
| 440 | + | } | |||
| 441 | + | } | |||
| 442 | + | ||||
| 443 | + | func TestReleaseRPCListWithoutLatest(t *testing.T) { | |||
| 444 | + | repo := mustRepoName(t, "demo") | |||
| 445 | + | rel := mocks.NewReleaseService(t) | |||
| 446 | + | c, _ := releaseClient(t, rel) | |||
| 447 | + | p := domain.Anonymous() | |||
| 448 | + | rel.EXPECT().ListReleases(mock.Anything, p, repo, 0, "").Return(ports.ReleasePage{Releases: []domain.Release{{ID: 1, Repo: repo}}}, nil).Once() | |||
| 449 | + | rel.EXPECT().GetRelease(mock.Anything, p, repo, ports.ReleaseRef{Latest: true}).Return(domain.Release{}, domain.ErrNotFound).Once() | |||
| 450 | + | rel.EXPECT().ListAssets(mock.Anything, p, repo, int64(1)).Return(nil, nil).Once() | |||
| 451 | + | res, err := c.ListReleases(context.Background(), connect.NewRequest(&pb.ListReleasesRequest{Repo: "demo"})) | |||
| 452 | + | require.NoError(t, err) | |||
| 453 | + | require.Len(t, res.Msg.Releases, 1) | |||
| 454 | + | assert.False(t, res.Msg.Releases[0].Latest) | |||
| 455 | + | } | |||
| 456 | + | ||||
| 457 | + | func TestReleaseRPCDownloadZeroByteAsset(t *testing.T) { | |||
| 458 | + | rel := mocks.NewReleaseService(t) | |||
| 459 | + | c, _ := releaseClient(t, rel) | |||
| 460 | + | a := domain.ReleaseAsset{ID: 9, ReleaseID: 7, Name: "empty", Size: 0} | |||
| 461 | + | // The use case sends the metadata and no data for an empty asset. | |||
| 462 | + | rel.EXPECT().DownloadAsset(mock.Anything, domain.Anonymous(), mustRepoName(t, "demo"), ports.AssetRef{ID: 9}, int64(0), int64(-1), mock.Anything).RunAndReturn(func(_ context.Context, _ domain.Principal, _ domain.RepoName, _ ports.AssetRef, _, _ int64, send func(ports.AssetChunk) error) error { | |||
| 463 | + | return send(ports.AssetChunk{Asset: &a}) | |||
| 464 | + | }).Once() | |||
| 465 | + | stream, err := c.DownloadReleaseAsset(context.Background(), connect.NewRequest(&pb.DownloadReleaseAssetRequest{Repo: "demo", Selector: &pb.DownloadReleaseAssetRequest_AssetId{AssetId: 9}})) | |||
| 466 | + | require.NoError(t, err) | |||
| 467 | + | defer stream.Close() | |||
| 468 | + | require.True(t, stream.Receive()) | |||
| 469 | + | assert.True(t, proto.Equal(assetToPB(a), stream.Msg().Asset)) | |||
| 470 | + | assert.False(t, stream.Receive()) | |||
| 471 | + | require.NoError(t, stream.Err()) | |||
| 472 | + | } | |||
M internal/adapters/web/releases.go +5 -44
| @@ -16,6 +16,7 @@ import ( | |||
| 16 | 16 | "github.com/bnema/zerowrap" | |
| 17 | 17 | ||
| 18 | 18 | "bnema.dev/ermite/internal/domain" | |
| 19 | + | "bnema.dev/ermite/internal/httprange" | |
| 19 | 20 | "bnema.dev/ermite/internal/ports" | |
| 20 | 21 | ) | |
| 21 | 22 | ||
| @@ -146,40 +147,6 @@ func (s *Server) releaseLatest(w http.ResponseWriter, r *http.Request) { | |||
| 146 | 147 | http.Redirect(w, r, releaseHref(repo.String(), rel.Tag), http.StatusSeeOther) | |
| 147 | 148 | } | |
| 148 | 149 | ||
| 149 | - | var errReleaseRange = errors.New("web: unsatisfiable byte range") | |
| 150 | - | ||
| 151 | - | // releaseRange accepts a single byte range, including open-ended and suffix | |
| 152 | - | // ranges. Multiple ranges are intentionally ignored (ADR 0034). | |
| 153 | - | func releaseRange(value string, size int64) (offset, length int64, partial bool, err error) { | |
| 154 | - | if value == "" || strings.Contains(value, ",") || !strings.HasPrefix(value, "bytes=") { | |
| 155 | - | return 0, size, false, nil | |
| 156 | - | } | |
| 157 | - | a, b, ok := strings.Cut(strings.TrimPrefix(value, "bytes="), "-") | |
| 158 | - | if !ok || size == 0 { | |
| 159 | - | return 0, 0, false, errReleaseRange | |
| 160 | - | } | |
| 161 | - | if a == "" { | |
| 162 | - | n, e := strconv.ParseInt(b, 10, 64) | |
| 163 | - | if e != nil || n <= 0 { | |
| 164 | - | return 0, 0, false, errReleaseRange | |
| 165 | - | } | |
| 166 | - | n = min(n, size) | |
| 167 | - | return size - n, n, true, nil | |
| 168 | - | } | |
| 169 | - | start, e := strconv.ParseInt(a, 10, 64) | |
| 170 | - | if e != nil || start < 0 || start >= size { | |
| 171 | - | return 0, 0, false, errReleaseRange | |
| 172 | - | } | |
| 173 | - | end := size - 1 | |
| 174 | - | if b != "" { | |
| 175 | - | end, e = strconv.ParseInt(b, 10, 64) | |
| 176 | - | if e != nil || end < start { | |
| 177 | - | return 0, 0, false, errReleaseRange | |
| 178 | - | } | |
| 179 | - | end = min(end, size-1) | |
| 180 | - | } | |
| 181 | - | return start, end - start + 1, true, nil | |
| 182 | - | } | |
| 183 | 150 | func releaseHeaders(h http.Header, a domain.ReleaseAsset) { | |
| 184 | 151 | // Reuse the artifact sandbox and attachment policy; never trust an | |
| 185 | 152 | // uploader's media type to execute content in the forge's origin. | |
| @@ -224,15 +191,9 @@ func (s *Server) releaseDownload(w http.ResponseWriter, r *http.Request) { | |||
| 224 | 191 | h := w.Header() | |
| 225 | 192 | releaseHeaders(h, a) | |
| 226 | 193 | cacheHeaders(ctx, h, "no-cache", sessionCache) | |
| 227 | - | offset, length, partial, err := releaseRange(strings.Join(r.Header.Values("Range"), ","), a.Size) | |
| 228 | - | if r.Method == http.MethodHead { | |
| 229 | - | offset, length, partial, err = 0, a.Size, false, nil | |
| 230 | - | } | |
| 231 | - | if r.Header.Get("If-Range") != "" && r.Header.Get("If-Range") != h.Get("ETag") { | |
| 232 | - | offset, length, partial, err = 0, a.Size, false, nil | |
| 233 | - | } | |
| 194 | + | offset, length, partial, err := httprange.Select(r, a.Size, h.Get("ETag")) | |
| 234 | 195 | if err != nil { | |
| 235 | - | h.Set("Content-Range", "bytes */"+strconv.FormatInt(a.Size, 10)) | |
| 196 | + | h.Set("Content-Range", httprange.Unsatisfied(a.Size)) | |
| 236 | 197 | h.Set("Content-Length", "0") | |
| 237 | 198 | w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) | |
| 238 | 199 | return | |
| @@ -240,10 +201,10 @@ func (s *Server) releaseDownload(w http.ResponseWriter, r *http.Request) { | |||
| 240 | 201 | status := http.StatusOK | |
| 241 | 202 | if partial { | |
| 242 | 203 | status = http.StatusPartialContent | |
| 243 | - | h.Set("Content-Range", "bytes "+strconv.FormatInt(offset, 10)+"-"+strconv.FormatInt(offset+length-1, 10)+"/"+strconv.FormatInt(a.Size, 10)) | |
| 204 | + | h.Set("Content-Range", httprange.ContentRange(offset, length, a.Size)) | |
| 244 | 205 | } | |
| 245 | 206 | h.Set("Content-Length", strconv.FormatInt(length, 10)) | |
| 246 | - | if r.Method == http.MethodHead || a.Size == 0 { | |
| 207 | + | if r.Method == http.MethodHead { | |
| 247 | 208 | w.WriteHeader(status) | |
| 248 | 209 | return | |
| 249 | 210 | } | |
| @@ -16,6 +16,7 @@ import ( | |||||
| 16 | "github.com/bnema/zerowrap" | 16 | "github.com/bnema/zerowrap" | ||
| 17 | 17 | ||||
| 18 | "bnema.dev/ermite/internal/domain" | 18 | "bnema.dev/ermite/internal/domain" | ||
| 19 | + | "bnema.dev/ermite/internal/httprange" | |||
| 19 | "bnema.dev/ermite/internal/ports" | 20 | "bnema.dev/ermite/internal/ports" | ||
| 20 | ) | 21 | ) | ||
| 21 | 22 | ||||
| @@ -146,40 +147,6 @@ func (s *Server) releaseLatest(w http.ResponseWriter, r *http.Request) { | |||||
| 146 | http.Redirect(w, r, releaseHref(repo.String(), rel.Tag), http.StatusSeeOther) | 147 | http.Redirect(w, r, releaseHref(repo.String(), rel.Tag), http.StatusSeeOther) | ||
| 147 | } | 148 | } | ||
| 148 | 149 | ||||
| 149 | - | var errReleaseRange = errors.New("web: unsatisfiable byte range") | |||
| 150 | - | ||||
| 151 | - | // releaseRange accepts a single byte range, including open-ended and suffix | |||
| 152 | - | // ranges. Multiple ranges are intentionally ignored (ADR 0034). | |||
| 153 | - | func releaseRange(value string, size int64) (offset, length int64, partial bool, err error) { | |||
| 154 | - | if value == "" || strings.Contains(value, ",") || !strings.HasPrefix(value, "bytes=") { | |||
| 155 | - | return 0, size, false, nil | |||
| 156 | - | } | |||
| 157 | - | a, b, ok := strings.Cut(strings.TrimPrefix(value, "bytes="), "-") | |||
| 158 | - | if !ok || size == 0 { | |||
| 159 | - | return 0, 0, false, errReleaseRange | |||
| 160 | - | } | |||
| 161 | - | if a == "" { | |||
| 162 | - | n, e := strconv.ParseInt(b, 10, 64) | |||
| 163 | - | if e != nil || n <= 0 { | |||
| 164 | - | return 0, 0, false, errReleaseRange | |||
| 165 | - | } | |||
| 166 | - | n = min(n, size) | |||
| 167 | - | return size - n, n, true, nil | |||
| 168 | - | } | |||
| 169 | - | start, e := strconv.ParseInt(a, 10, 64) | |||
| 170 | - | if e != nil || start < 0 || start >= size { | |||
| 171 | - | return 0, 0, false, errReleaseRange | |||
| 172 | - | } | |||
| 173 | - | end := size - 1 | |||
| 174 | - | if b != "" { | |||
| 175 | - | end, e = strconv.ParseInt(b, 10, 64) | |||
| 176 | - | if e != nil || end < start { | |||
| 177 | - | return 0, 0, false, errReleaseRange | |||
| 178 | - | } | |||
| 179 | - | end = min(end, size-1) | |||
| 180 | - | } | |||
| 181 | - | return start, end - start + 1, true, nil | |||
| 182 | - | } | |||
| 183 | func releaseHeaders(h http.Header, a domain.ReleaseAsset) { | 150 | func releaseHeaders(h http.Header, a domain.ReleaseAsset) { | ||
| 184 | // Reuse the artifact sandbox and attachment policy; never trust an | 151 | // Reuse the artifact sandbox and attachment policy; never trust an | ||
| 185 | // uploader's media type to execute content in the forge's origin. | 152 | // uploader's media type to execute content in the forge's origin. | ||
| @@ -224,15 +191,9 @@ func (s *Server) releaseDownload(w http.ResponseWriter, r *http.Request) { | |||||
| 224 | h := w.Header() | 191 | h := w.Header() | ||
| 225 | releaseHeaders(h, a) | 192 | releaseHeaders(h, a) | ||
| 226 | cacheHeaders(ctx, h, "no-cache", sessionCache) | 193 | cacheHeaders(ctx, h, "no-cache", sessionCache) | ||
| 227 | - | offset, length, partial, err := releaseRange(strings.Join(r.Header.Values("Range"), ","), a.Size) | 194 | + | offset, length, partial, err := httprange.Select(r, a.Size, h.Get("ETag")) |
| 228 | - | if r.Method == http.MethodHead { | |||
| 229 | - | offset, length, partial, err = 0, a.Size, false, nil | |||
| 230 | - | } | |||
| 231 | - | if r.Header.Get("If-Range") != "" && r.Header.Get("If-Range") != h.Get("ETag") { | |||
| 232 | - | offset, length, partial, err = 0, a.Size, false, nil | |||
| 233 | - | } | |||
| 234 | if err != nil { | 195 | if err != nil { | ||
| 235 | - | h.Set("Content-Range", "bytes */"+strconv.FormatInt(a.Size, 10)) | 196 | + | h.Set("Content-Range", httprange.Unsatisfied(a.Size)) |
| 236 | h.Set("Content-Length", "0") | 197 | h.Set("Content-Length", "0") | ||
| 237 | w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) | 198 | w.WriteHeader(http.StatusRequestedRangeNotSatisfiable) | ||
| 238 | return | 199 | return | ||
| @@ -240,10 +201,10 @@ func (s *Server) releaseDownload(w http.ResponseWriter, r *http.Request) { | |||||
| 240 | status := http.StatusOK | 201 | status := http.StatusOK | ||
| 241 | if partial { | 202 | if partial { | ||
| 242 | status = http.StatusPartialContent | 203 | status = http.StatusPartialContent | ||
| 243 | - | h.Set("Content-Range", "bytes "+strconv.FormatInt(offset, 10)+"-"+strconv.FormatInt(offset+length-1, 10)+"/"+strconv.FormatInt(a.Size, 10)) | 204 | + | h.Set("Content-Range", httprange.ContentRange(offset, length, a.Size)) |
| 244 | } | 205 | } | ||
| 245 | h.Set("Content-Length", strconv.FormatInt(length, 10)) | 206 | h.Set("Content-Length", strconv.FormatInt(length, 10)) | ||
| 246 | - | if r.Method == http.MethodHead || a.Size == 0 { | 207 | + | if r.Method == http.MethodHead { |
| 247 | w.WriteHeader(status) | 208 | w.WriteHeader(status) | ||
| 248 | return | 209 | return | ||
| 249 | } | 210 | } | ||
M internal/adapters/web/releases_test.go +33 -7
| @@ -8,6 +8,7 @@ import ( | |||
| 8 | 8 | "mime" | |
| 9 | 9 | "net/http" | |
| 10 | 10 | "net/http/httptest" | |
| 11 | + | "net/url" | |
| 11 | 12 | "strings" | |
| 12 | 13 | "testing" | |
| 13 | 14 | "time" | |
| @@ -175,6 +176,7 @@ func TestReleaseDownloads(t *testing.T) { | |||
| 175 | 176 | before := f.downloads | |
| 176 | 177 | w := f.request("HEAD", path, "", "bytes=1-2") | |
| 177 | 178 | assert.Equal(t, 200, w.Code) | |
| 179 | + | assert.Empty(t, w.Header().Get("Content-Range")) | |
| 178 | 180 | assert.Empty(t, w.Body.String()) | |
| 179 | 181 | assert.Equal(t, "10", w.Header().Get("Content-Length")) | |
| 180 | 182 | assert.Equal(t, before, f.downloads) | |
| @@ -190,6 +192,37 @@ func TestReleaseDownloads(t *testing.T) { | |||
| 190 | 192 | assert.Equal(t, 200, w.Code) | |
| 191 | 193 | assert.Equal(t, f.data, w.Body.String()) | |
| 192 | 194 | } | |
| 195 | + | func TestReleaseDownloadIfRange(t *testing.T) { | |
| 196 | + | f := newReleaseFixture(t) | |
| 197 | + | etag := `"sha256-` + strings.Repeat("b", 64) + `"` | |
| 198 | + | for _, tc := range []struct { | |
| 199 | + | name, ifRange, body, cr string | |
| 200 | + | status int | |
| 201 | + | }{ | |
| 202 | + | {"matching", etag, "2345", "bytes 2-5/10", 206}, | |
| 203 | + | {"stale", `"old"`, "0123456789", "", 200}, | |
| 204 | + | {"date", "Wed, 21 Oct 2026 12:00:00 GMT", "0123456789", "", 200}, | |
| 205 | + | } { | |
| 206 | + | r := httptest.NewRequest("GET", "/proj/releases/assets/7", nil) | |
| 207 | + | r.Header.Set("Range", "bytes=2-5") | |
| 208 | + | r.Header.Set("If-Range", tc.ifRange) | |
| 209 | + | w := httptest.NewRecorder() | |
| 210 | + | f.srv.ServeHTTP(w, r) | |
| 211 | + | assert.Equal(t, tc.status, w.Code, tc.name) | |
| 212 | + | assert.Equal(t, tc.body, w.Body.String(), tc.name) | |
| 213 | + | assert.Equal(t, tc.cr, w.Header().Get("Content-Range"), tc.name) | |
| 214 | + | } | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | // A tag with a slash is one path segment once escaped: the download route must | |
| 218 | + | // match it and resolve the tag with the slash. | |
| 219 | + | func TestReleaseDownloadSlashTag(t *testing.T) { | |
| 220 | + | f := newReleaseFixture(t) // the published tag is "stable/v1" | |
| 221 | + | w := f.request("GET", "/proj/releases/download/"+url.PathEscape(f.published.Tag)+"/build.zip", "", "") | |
| 222 | + | require.Equal(t, 200, w.Code) | |
| 223 | + | assert.Equal(t, f.data, w.Body.String()) | |
| 224 | + | } | |
| 225 | + | ||
| 193 | 226 | func TestReleaseDownloadFailures(t *testing.T) { | |
| 194 | 227 | f := newReleaseFixture(t) | |
| 195 | 228 | f.downloadErr = errors.New("blob unavailable") | |
| @@ -215,12 +248,6 @@ func TestReleaseAttachmentHeaders(t *testing.T) { | |||
| 215 | 248 | assert.Equal(t, attachmentName(name), params["filename"]) | |
| 216 | 249 | } | |
| 217 | 250 | } | |
| 218 | - | func TestReleaseRangeInvalid(t *testing.T) { | |
| 219 | - | for _, value := range []string{"bytes=", "bytes=-0", "bytes=-1-2", "bytes=5-2", "bytes=999999999999999999999999-", "bytes=0-nope"} { | |
| 220 | - | _, _, _, err := releaseRange(value, 10) | |
| 221 | - | assert.ErrorIs(t, err, errReleaseRange, value) | |
| 222 | - | } | |
| 223 | - | } | |
| 224 | 251 | func TestReleasesDisabled(t *testing.T) { | |
| 225 | 252 | s := newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "proj")), Browse: mocks.NewBrowseService(t)}) | |
| 226 | 253 | for _, path := range []string{"/proj/releases", "/proj/releases/latest", "/proj/releases/assets/7"} { | |
| @@ -238,7 +265,6 @@ func TestReleaseEmptyDownloads(t *testing.T) { | |||
| 238 | 265 | assert.Equal(t, 200, w.Code) | |
| 239 | 266 | assert.Equal(t, "0", w.Header().Get("Content-Length")) | |
| 240 | 267 | assert.Empty(t, w.Body.String()) | |
| 241 | - | assert.Zero(t, f.downloads) | |
| 242 | 268 | w = f.request("GET", "/proj/releases/assets/7", "", "bytes=0-") | |
| 243 | 269 | assert.Equal(t, 416, w.Code) | |
| 244 | 270 | assert.Equal(t, "bytes */0", w.Header().Get("Content-Range")) | |
| @@ -8,6 +8,7 @@ import ( | |||||
| 8 | "mime" | 8 | "mime" | ||
| 9 | "net/http" | 9 | "net/http" | ||
| 10 | "net/http/httptest" | 10 | "net/http/httptest" | ||
| 11 | + | "net/url" | |||
| 11 | "strings" | 12 | "strings" | ||
| 12 | "testing" | 13 | "testing" | ||
| 13 | "time" | 14 | "time" | ||
| @@ -175,6 +176,7 @@ func TestReleaseDownloads(t *testing.T) { | |||||
| 175 | before := f.downloads | 176 | before := f.downloads | ||
| 176 | w := f.request("HEAD", path, "", "bytes=1-2") | 177 | w := f.request("HEAD", path, "", "bytes=1-2") | ||
| 177 | assert.Equal(t, 200, w.Code) | 178 | assert.Equal(t, 200, w.Code) | ||
| 179 | + | assert.Empty(t, w.Header().Get("Content-Range")) | |||
| 178 | assert.Empty(t, w.Body.String()) | 180 | assert.Empty(t, w.Body.String()) | ||
| 179 | assert.Equal(t, "10", w.Header().Get("Content-Length")) | 181 | assert.Equal(t, "10", w.Header().Get("Content-Length")) | ||
| 180 | assert.Equal(t, before, f.downloads) | 182 | assert.Equal(t, before, f.downloads) | ||
| @@ -190,6 +192,37 @@ func TestReleaseDownloads(t *testing.T) { | |||||
| 190 | assert.Equal(t, 200, w.Code) | 192 | assert.Equal(t, 200, w.Code) | ||
| 191 | assert.Equal(t, f.data, w.Body.String()) | 193 | assert.Equal(t, f.data, w.Body.String()) | ||
| 192 | } | 194 | } | ||
| 195 | + | func TestReleaseDownloadIfRange(t *testing.T) { | |||
| 196 | + | f := newReleaseFixture(t) | |||
| 197 | + | etag := `"sha256-` + strings.Repeat("b", 64) + `"` | |||
| 198 | + | for _, tc := range []struct { | |||
| 199 | + | name, ifRange, body, cr string | |||
| 200 | + | status int | |||
| 201 | + | }{ | |||
| 202 | + | {"matching", etag, "2345", "bytes 2-5/10", 206}, | |||
| 203 | + | {"stale", `"old"`, "0123456789", "", 200}, | |||
| 204 | + | {"date", "Wed, 21 Oct 2026 12:00:00 GMT", "0123456789", "", 200}, | |||
| 205 | + | } { | |||
| 206 | + | r := httptest.NewRequest("GET", "/proj/releases/assets/7", nil) | |||
| 207 | + | r.Header.Set("Range", "bytes=2-5") | |||
| 208 | + | r.Header.Set("If-Range", tc.ifRange) | |||
| 209 | + | w := httptest.NewRecorder() | |||
| 210 | + | f.srv.ServeHTTP(w, r) | |||
| 211 | + | assert.Equal(t, tc.status, w.Code, tc.name) | |||
| 212 | + | assert.Equal(t, tc.body, w.Body.String(), tc.name) | |||
| 213 | + | assert.Equal(t, tc.cr, w.Header().Get("Content-Range"), tc.name) | |||
| 214 | + | } | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | // A tag with a slash is one path segment once escaped: the download route must | |||
| 218 | + | // match it and resolve the tag with the slash. | |||
| 219 | + | func TestReleaseDownloadSlashTag(t *testing.T) { | |||
| 220 | + | f := newReleaseFixture(t) // the published tag is "stable/v1" | |||
| 221 | + | w := f.request("GET", "/proj/releases/download/"+url.PathEscape(f.published.Tag)+"/build.zip", "", "") | |||
| 222 | + | require.Equal(t, 200, w.Code) | |||
| 223 | + | assert.Equal(t, f.data, w.Body.String()) | |||
| 224 | + | } | |||
| 225 | + | ||||
| 193 | func TestReleaseDownloadFailures(t *testing.T) { | 226 | func TestReleaseDownloadFailures(t *testing.T) { | ||
| 194 | f := newReleaseFixture(t) | 227 | f := newReleaseFixture(t) | ||
| 195 | f.downloadErr = errors.New("blob unavailable") | 228 | f.downloadErr = errors.New("blob unavailable") | ||
| @@ -215,12 +248,6 @@ func TestReleaseAttachmentHeaders(t *testing.T) { | |||||
| 215 | assert.Equal(t, attachmentName(name), params["filename"]) | 248 | assert.Equal(t, attachmentName(name), params["filename"]) | ||
| 216 | } | 249 | } | ||
| 217 | } | 250 | } | ||
| 218 | - | func TestReleaseRangeInvalid(t *testing.T) { | |||
| 219 | - | for _, value := range []string{"bytes=", "bytes=-0", "bytes=-1-2", "bytes=5-2", "bytes=999999999999999999999999-", "bytes=0-nope"} { | |||
| 220 | - | _, _, _, err := releaseRange(value, 10) | |||
| 221 | - | assert.ErrorIs(t, err, errReleaseRange, value) | |||
| 222 | - | } | |||
| 223 | - | } | |||
| 224 | func TestReleasesDisabled(t *testing.T) { | 251 | func TestReleasesDisabled(t *testing.T) { | ||
| 225 | s := newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "proj")), Browse: mocks.NewBrowseService(t)}) | 252 | s := newTestServer(t, Config{Repos: reposServing(t, publicRepo(t, "proj")), Browse: mocks.NewBrowseService(t)}) | ||
| 226 | for _, path := range []string{"/proj/releases", "/proj/releases/latest", "/proj/releases/assets/7"} { | 253 | for _, path := range []string{"/proj/releases", "/proj/releases/latest", "/proj/releases/assets/7"} { | ||
| @@ -238,7 +265,6 @@ func TestReleaseEmptyDownloads(t *testing.T) { | |||||
| 238 | assert.Equal(t, 200, w.Code) | 265 | assert.Equal(t, 200, w.Code) | ||
| 239 | assert.Equal(t, "0", w.Header().Get("Content-Length")) | 266 | assert.Equal(t, "0", w.Header().Get("Content-Length")) | ||
| 240 | assert.Empty(t, w.Body.String()) | 267 | assert.Empty(t, w.Body.String()) | ||
| 241 | - | assert.Zero(t, f.downloads) | |||
| 242 | w = f.request("GET", "/proj/releases/assets/7", "", "bytes=0-") | 268 | w = f.request("GET", "/proj/releases/assets/7", "", "bytes=0-") | ||
| 243 | assert.Equal(t, 416, w.Code) | 269 | assert.Equal(t, 416, w.Code) | ||
| 244 | assert.Equal(t, "bytes */0", w.Header().Get("Content-Range")) | 270 | assert.Equal(t, "bytes */0", w.Header().Get("Content-Range")) | ||
M internal/app/web_release_test.go +17 -0
| @@ -4,6 +4,7 @@ package app | |||
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | 6 | "context" | |
| 7 | + | "encoding/json" | |
| 7 | 8 | "io" | |
| 8 | 9 | "net/http" | |
| 9 | 10 | "strconv" | |
| @@ -15,6 +16,7 @@ import ( | |||
| 15 | 16 | ||
| 16 | 17 | "bnema.dev/ermite/internal/domain" | |
| 17 | 18 | "bnema.dev/ermite/internal/ports" | |
| 19 | + | "bnema.dev/ermite/internal/usecase" | |
| 18 | 20 | ) | |
| 19 | 21 | ||
| 20 | 22 | // Releases and web use the production forge wiring. | |
| @@ -84,6 +86,21 @@ func TestWebRelease(t *testing.T) { | |||
| 84 | 86 | status, _, body = get("GET", "/demo/releases/tag/series/v1", "") | |
| 85 | 87 | assert.Equal(t, 200, status) | |
| 86 | 88 | assert.Contains(t, body, slash.Title) | |
| 89 | + | // The browser_download_url of the REST API for a slash tag is served by | |
| 90 | + | // the web route. | |
| 91 | + | slashAsset, err := releases.UploadAsset(h.ctx, domain.Root(), repo, ports.UploadAssetParams{ReleaseID: slash.ID, Name: "s.bin", Size: 5}, strings.NewReader("slash")) | |
| 92 | + | require.NoError(t, err) | |
| 93 | + | gh := usecase.NewReleasesGitHub(releases, "owner", ts.URL) | |
| 94 | + | raw, err := gh.GetAsset(h.ctx, domain.Root(), "owner", "demo", slashAsset.ID) | |
| 95 | + | require.NoError(t, err) | |
| 96 | + | var obj struct { | |
| 97 | + | URL string `json:"browser_download_url"` | |
| 98 | + | } | |
| 99 | + | require.NoError(t, json.Unmarshal(raw, &obj)) | |
| 100 | + | require.Equal(t, ts.URL+"/demo/releases/download/series%2Fv1/s.bin", obj.URL) | |
| 101 | + | status, _, body = get("GET", strings.TrimPrefix(obj.URL, ts.URL), "") | |
| 102 | + | assert.Equal(t, 200, status) | |
| 103 | + | assert.Equal(t, "slash", body) | |
| 87 | 104 | // A missing blob is a 404 page before any binary headers are written. | |
| 88 | 105 | require.NoError(t, blobs.Delete(context.Background(), asset.BlobKey())) | |
| 89 | 106 | status, headers, body := get("GET", "/demo/releases/assets/"+strconv.FormatInt(asset.ID, 10), "") | |
| @@ -4,6 +4,7 @@ package app | |||||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | "context" | 6 | "context" | ||
| 7 | + | "encoding/json" | |||
| 7 | "io" | 8 | "io" | ||
| 8 | "net/http" | 9 | "net/http" | ||
| 9 | "strconv" | 10 | "strconv" | ||
| @@ -15,6 +16,7 @@ import ( | |||||
| 15 | 16 | ||||
| 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/usecase" | |||
| 18 | ) | 20 | ) | ||
| 19 | 21 | ||||
| 20 | // Releases and web use the production forge wiring. | 22 | // Releases and web use the production forge wiring. | ||
| @@ -84,6 +86,21 @@ func TestWebRelease(t *testing.T) { | |||||
| 84 | status, _, body = get("GET", "/demo/releases/tag/series/v1", "") | 86 | status, _, body = get("GET", "/demo/releases/tag/series/v1", "") | ||
| 85 | assert.Equal(t, 200, status) | 87 | assert.Equal(t, 200, status) | ||
| 86 | assert.Contains(t, body, slash.Title) | 88 | assert.Contains(t, body, slash.Title) | ||
| 89 | + | // The browser_download_url of the REST API for a slash tag is served by | |||
| 90 | + | // the web route. | |||
| 91 | + | slashAsset, err := releases.UploadAsset(h.ctx, domain.Root(), repo, ports.UploadAssetParams{ReleaseID: slash.ID, Name: "s.bin", Size: 5}, strings.NewReader("slash")) | |||
| 92 | + | require.NoError(t, err) | |||
| 93 | + | gh := usecase.NewReleasesGitHub(releases, "owner", ts.URL) | |||
| 94 | + | raw, err := gh.GetAsset(h.ctx, domain.Root(), "owner", "demo", slashAsset.ID) | |||
| 95 | + | require.NoError(t, err) | |||
| 96 | + | var obj struct { | |||
| 97 | + | URL string `json:"browser_download_url"` | |||
| 98 | + | } | |||
| 99 | + | require.NoError(t, json.Unmarshal(raw, &obj)) | |||
| 100 | + | require.Equal(t, ts.URL+"/demo/releases/download/series%2Fv1/s.bin", obj.URL) | |||
| 101 | + | status, _, body = get("GET", strings.TrimPrefix(obj.URL, ts.URL), "") | |||
| 102 | + | assert.Equal(t, 200, status) | |||
| 103 | + | assert.Equal(t, "slash", body) | |||
| 87 | // A missing blob is a 404 page before any binary headers are written. | 104 | // A missing blob is a 404 page before any binary headers are written. | ||
| 88 | require.NoError(t, blobs.Delete(context.Background(), asset.BlobKey())) | 105 | require.NoError(t, blobs.Delete(context.Background(), asset.BlobKey())) | ||
| 89 | status, headers, body := get("GET", "/demo/releases/assets/"+strconv.FormatInt(asset.ID, 10), "") | 106 | status, headers, body := get("GET", "/demo/releases/assets/"+strconv.FormatInt(asset.ID, 10), "") | ||
M internal/boundary/boundary_test.go +14 -2
| @@ -35,6 +35,7 @@ const ( | |||
| 35 | 35 | layerUsecase packageLayer = "usecase" | |
| 36 | 36 | layerAdapter packageLayer = "adapter" | |
| 37 | 37 | layerClientLimit packageLayer = "clientlimit" | |
| 38 | + | layerHTTPRange packageLayer = "httprange" | |
| 38 | 39 | layerConfig packageLayer = "config" | |
| 39 | 40 | layerPorts packageLayer = "ports" | |
| 40 | 41 | layerDomain packageLayer = "domain" | |
| @@ -55,13 +56,16 @@ const ( | |||
| 55 | 56 | // each other, and keeps the web adapter off the generated API code: like | |
| 56 | 57 | // every other adapter it reads and writes through the ports (ADR 0020). | |
| 57 | 58 | // - internal/clientlimit is infrastructure shared by the HTTP adapters. | |
| 59 | + | // - internal/httprange is infrastructure shared by the HTTP adapters that | |
| 60 | + | // serve byte ranges; it imports nothing of the module. | |
| 58 | 61 | // - internal/config parses the environment into plain values. | |
| 59 | 62 | var productionDependencies = map[packageLayer]map[packageLayer]bool{ | |
| 60 | 63 | layerCommand: {layerApp: true}, | |
| 61 | - | layerApp: {layerUsecase: true, layerAdapter: true, layerClientLimit: true, layerConfig: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | |
| 64 | + | layerApp: {layerUsecase: true, layerAdapter: true, layerClientLimit: true, layerHTTPRange: true, layerConfig: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | |
| 62 | 65 | layerUsecase: {layerUsecase: true, layerPorts: true, layerDomain: true}, | |
| 63 | - | layerAdapter: {layerAdapter: true, layerClientLimit: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | |
| 66 | + | layerAdapter: {layerAdapter: true, layerClientLimit: true, layerHTTPRange: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | |
| 64 | 67 | layerClientLimit: {layerPorts: true}, | |
| 68 | + | layerHTTPRange: {}, | |
| 65 | 69 | layerConfig: {}, | |
| 66 | 70 | layerPorts: {layerDomain: true}, | |
| 67 | 71 | layerDomain: {layerDomain: true}, | |
| @@ -149,6 +153,12 @@ func TestImportBoundaryFixtures(t *testing.T) { | |||
| 149 | 153 | {"adapter rejects config", p("internal/adapters/rpc"), p("internal/config"), false, false}, | |
| 150 | 154 | {"adapter rejects sibling adapter", p("internal/adapters/web"), p("internal/adapters/gitcli"), false, false}, | |
| 151 | 155 | {"adapter test may import sibling adapter", p("internal/adapters/ermitegit"), p("internal/adapters/gitcli"), true, true}, | |
| 156 | + | {"app accepts httprange", p("internal/app"), p("internal/httprange"), false, true}, | |
| 157 | + | {"usecase rejects httprange", p("internal/usecase"), p("internal/httprange"), false, false}, | |
| 158 | + | {"domain rejects httprange", p("internal/domain"), p("internal/httprange"), false, false}, | |
| 159 | + | {"ports reject httprange", p("internal/ports"), p("internal/httprange"), false, false}, | |
| 160 | + | {"adapter accepts httprange", p("internal/adapters/web"), p("internal/httprange"), false, true}, | |
| 161 | + | {"httprange rejects domain", p("internal/httprange"), p("internal/domain"), false, false}, | |
| 152 | 162 | {"clientlimit accepts ports", p("internal/clientlimit"), p("internal/ports"), false, true}, | |
| 153 | 163 | {"clientlimit rejects usecase", p("internal/clientlimit"), p("internal/usecase"), false, false}, | |
| 154 | 164 | {"config rejects domain", p("internal/config"), p("internal/domain"), false, false}, | |
| @@ -319,6 +329,8 @@ func classifyPackage(path string) (packageLayer, error) { | |||
| 319 | 329 | return layerAdapter, nil | |
| 320 | 330 | case under(path, p("internal/clientlimit")): | |
| 321 | 331 | return layerClientLimit, nil | |
| 332 | + | case under(path, p("internal/httprange")): | |
| 333 | + | return layerHTTPRange, nil | |
| 322 | 334 | case under(path, p("internal/config")): | |
| 323 | 335 | return layerConfig, nil | |
| 324 | 336 | case path == p("internal/ports"): | |
| @@ -35,6 +35,7 @@ const ( | |||||
| 35 | layerUsecase packageLayer = "usecase" | 35 | layerUsecase packageLayer = "usecase" | ||
| 36 | layerAdapter packageLayer = "adapter" | 36 | layerAdapter packageLayer = "adapter" | ||
| 37 | layerClientLimit packageLayer = "clientlimit" | 37 | layerClientLimit packageLayer = "clientlimit" | ||
| 38 | + | layerHTTPRange packageLayer = "httprange" | |||
| 38 | layerConfig packageLayer = "config" | 39 | layerConfig packageLayer = "config" | ||
| 39 | layerPorts packageLayer = "ports" | 40 | layerPorts packageLayer = "ports" | ||
| 40 | layerDomain packageLayer = "domain" | 41 | layerDomain packageLayer = "domain" | ||
| @@ -55,13 +56,16 @@ const ( | |||||
| 55 | // each other, and keeps the web adapter off the generated API code: like | 56 | // each other, and keeps the web adapter off the generated API code: like | ||
| 56 | // every other adapter it reads and writes through the ports (ADR 0020). | 57 | // every other adapter it reads and writes through the ports (ADR 0020). | ||
| 57 | // - internal/clientlimit is infrastructure shared by the HTTP adapters. | 58 | // - internal/clientlimit is infrastructure shared by the HTTP adapters. | ||
| 59 | + | // - internal/httprange is infrastructure shared by the HTTP adapters that | |||
| 60 | + | // serve byte ranges; it imports nothing of the module. | |||
| 58 | // - internal/config parses the environment into plain values. | 61 | // - internal/config parses the environment into plain values. | ||
| 59 | var productionDependencies = map[packageLayer]map[packageLayer]bool{ | 62 | var productionDependencies = map[packageLayer]map[packageLayer]bool{ | ||
| 60 | layerCommand: {layerApp: true}, | 63 | layerCommand: {layerApp: true}, | ||
| 61 | - | layerApp: {layerUsecase: true, layerAdapter: true, layerClientLimit: true, layerConfig: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | 64 | + | layerApp: {layerUsecase: true, layerAdapter: true, layerClientLimit: true, layerHTTPRange: true, layerConfig: true, layerPorts: true, layerDomain: true, layerGenerated: true}, |
| 62 | layerUsecase: {layerUsecase: true, layerPorts: true, layerDomain: true}, | 65 | layerUsecase: {layerUsecase: true, layerPorts: true, layerDomain: true}, | ||
| 63 | - | layerAdapter: {layerAdapter: true, layerClientLimit: true, layerPorts: true, layerDomain: true, layerGenerated: true}, | 66 | + | layerAdapter: {layerAdapter: true, layerClientLimit: true, layerHTTPRange: true, layerPorts: true, layerDomain: true, layerGenerated: true}, |
| 64 | layerClientLimit: {layerPorts: true}, | 67 | layerClientLimit: {layerPorts: true}, | ||
| 68 | + | layerHTTPRange: {}, | |||
| 65 | layerConfig: {}, | 69 | layerConfig: {}, | ||
| 66 | layerPorts: {layerDomain: true}, | 70 | layerPorts: {layerDomain: true}, | ||
| 67 | layerDomain: {layerDomain: true}, | 71 | layerDomain: {layerDomain: true}, | ||
| @@ -149,6 +153,12 @@ func TestImportBoundaryFixtures(t *testing.T) { | |||||
| 149 | {"adapter rejects config", p("internal/adapters/rpc"), p("internal/config"), false, false}, | 153 | {"adapter rejects config", p("internal/adapters/rpc"), p("internal/config"), false, false}, | ||
| 150 | {"adapter rejects sibling adapter", p("internal/adapters/web"), p("internal/adapters/gitcli"), false, false}, | 154 | {"adapter rejects sibling adapter", p("internal/adapters/web"), p("internal/adapters/gitcli"), false, false}, | ||
| 151 | {"adapter test may import sibling adapter", p("internal/adapters/ermitegit"), p("internal/adapters/gitcli"), true, true}, | 155 | {"adapter test may import sibling adapter", p("internal/adapters/ermitegit"), p("internal/adapters/gitcli"), true, true}, | ||
| 156 | + | {"app accepts httprange", p("internal/app"), p("internal/httprange"), false, true}, | |||
| 157 | + | {"usecase rejects httprange", p("internal/usecase"), p("internal/httprange"), false, false}, | |||
| 158 | + | {"domain rejects httprange", p("internal/domain"), p("internal/httprange"), false, false}, | |||
| 159 | + | {"ports reject httprange", p("internal/ports"), p("internal/httprange"), false, false}, | |||
| 160 | + | {"adapter accepts httprange", p("internal/adapters/web"), p("internal/httprange"), false, true}, | |||
| 161 | + | {"httprange rejects domain", p("internal/httprange"), p("internal/domain"), false, false}, | |||
| 152 | {"clientlimit accepts ports", p("internal/clientlimit"), p("internal/ports"), false, true}, | 162 | {"clientlimit accepts ports", p("internal/clientlimit"), p("internal/ports"), false, true}, | ||
| 153 | {"clientlimit rejects usecase", p("internal/clientlimit"), p("internal/usecase"), false, false}, | 163 | {"clientlimit rejects usecase", p("internal/clientlimit"), p("internal/usecase"), false, false}, | ||
| 154 | {"config rejects domain", p("internal/config"), p("internal/domain"), false, false}, | 164 | {"config rejects domain", p("internal/config"), p("internal/domain"), false, false}, | ||
| @@ -319,6 +329,8 @@ func classifyPackage(path string) (packageLayer, error) { | |||||
| 319 | return layerAdapter, nil | 329 | return layerAdapter, nil | ||
| 320 | case under(path, p("internal/clientlimit")): | 330 | case under(path, p("internal/clientlimit")): | ||
| 321 | return layerClientLimit, nil | 331 | return layerClientLimit, nil | ||
| 332 | + | case under(path, p("internal/httprange")): | |||
| 333 | + | return layerHTTPRange, nil | |||
| 322 | case under(path, p("internal/config")): | 334 | case under(path, p("internal/config")): | ||
| 323 | return layerConfig, nil | 335 | return layerConfig, nil | ||
| 324 | case path == p("internal/ports"): | 336 | case path == p("internal/ports"): | ||
A internal/httprange/httprange.go +77 -0
| @@ -0,0 +1,77 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | // Package httprange selects the byte range of a download from the Range and | |
| 4 | + | // If-Range headers of a request. The HTTP adapters that serve release assets | |
| 5 | + | // share it, so they answer the same way. | |
| 6 | + | package httprange | |
| 7 | + | ||
| 8 | + | import ( | |
| 9 | + | "errors" | |
| 10 | + | "net/http" | |
| 11 | + | "strconv" | |
| 12 | + | "strings" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | // ErrUnsatisfiable means the request asked for a range the content cannot give. | |
| 16 | + | // The caller answers 416 with "Content-Range: bytes */<size>". | |
| 17 | + | var ErrUnsatisfiable = errors.New("httprange: unsatisfiable byte range") | |
| 18 | + | ||
| 19 | + | // Select returns the part of a size-byte body to send. | |
| 20 | + | // | |
| 21 | + | // Without a usable Range it returns the whole body (0, size, false, nil): | |
| 22 | + | // - the method is not GET (HEAD answers with the headers of the full body); | |
| 23 | + | // - there is no Range header, its unit is not "bytes", or it lists several | |
| 24 | + | // ranges (ADR 0034: multipart ranges are not served); | |
| 25 | + | // - If-Range is present and is not exactly etag. The comparison is strong: | |
| 26 | + | // a weak validator or an HTTP date never matches. | |
| 27 | + | // | |
| 28 | + | // A single "bytes=a-b", "bytes=a-" or "bytes=-n" gives (offset, length, true, | |
| 29 | + | // nil); an end past the content is cut to it. A malformed range, a start at or | |
| 30 | + | // past size, a suffix of zero, or any range of an empty body returns | |
| 31 | + | // ErrUnsatisfiable. | |
| 32 | + | func Select(r *http.Request, size int64, etag string) (offset, length int64, partial bool, err error) { | |
| 33 | + | whole := func() (int64, int64, bool, error) { return 0, size, false, nil } | |
| 34 | + | if r.Method != http.MethodGet { | |
| 35 | + | return whole() | |
| 36 | + | } | |
| 37 | + | value := strings.Join(r.Header.Values("Range"), ",") | |
| 38 | + | if value == "" || strings.Contains(value, ",") || !strings.HasPrefix(value, "bytes=") { | |
| 39 | + | return whole() | |
| 40 | + | } | |
| 41 | + | if ifRange := r.Header.Get("If-Range"); ifRange != "" && (etag == "" || ifRange != etag || strings.HasPrefix(ifRange, "W/")) { | |
| 42 | + | return whole() | |
| 43 | + | } | |
| 44 | + | a, b, ok := strings.Cut(strings.TrimSpace(strings.TrimPrefix(value, "bytes=")), "-") | |
| 45 | + | if !ok || size <= 0 { | |
| 46 | + | return 0, 0, false, ErrUnsatisfiable | |
| 47 | + | } | |
| 48 | + | if a == "" { | |
| 49 | + | n, e := strconv.ParseInt(b, 10, 64) | |
| 50 | + | if e != nil || n <= 0 { | |
| 51 | + | return 0, 0, false, ErrUnsatisfiable | |
| 52 | + | } | |
| 53 | + | n = min(n, size) | |
| 54 | + | return size - n, n, true, nil | |
| 55 | + | } | |
| 56 | + | start, e := strconv.ParseInt(a, 10, 64) | |
| 57 | + | if e != nil || start < 0 || start >= size { | |
| 58 | + | return 0, 0, false, ErrUnsatisfiable | |
| 59 | + | } | |
| 60 | + | end := size - 1 | |
| 61 | + | if b != "" { | |
| 62 | + | end, e = strconv.ParseInt(b, 10, 64) | |
| 63 | + | if e != nil || end < start { | |
| 64 | + | return 0, 0, false, ErrUnsatisfiable | |
| 65 | + | } | |
| 66 | + | end = min(end, size-1) | |
| 67 | + | } | |
| 68 | + | return start, end - start + 1, true, nil | |
| 69 | + | } | |
| 70 | + | ||
| 71 | + | // ContentRange is the Content-Range value of a partial answer. | |
| 72 | + | func ContentRange(offset, length, size int64) string { | |
| 73 | + | return "bytes " + strconv.FormatInt(offset, 10) + "-" + strconv.FormatInt(offset+length-1, 10) + "/" + strconv.FormatInt(size, 10) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | // Unsatisfied is the Content-Range value of a 416 answer. | |
| 77 | + | func Unsatisfied(size int64) string { return "bytes */" + strconv.FormatInt(size, 10) } | |
| @@ -0,0 +1,77 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | // Package httprange selects the byte range of a download from the Range and | |||
| 4 | + | // If-Range headers of a request. The HTTP adapters that serve release assets | |||
| 5 | + | // share it, so they answer the same way. | |||
| 6 | + | package httprange | |||
| 7 | + | ||||
| 8 | + | import ( | |||
| 9 | + | "errors" | |||
| 10 | + | "net/http" | |||
| 11 | + | "strconv" | |||
| 12 | + | "strings" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | // ErrUnsatisfiable means the request asked for a range the content cannot give. | |||
| 16 | + | // The caller answers 416 with "Content-Range: bytes */<size>". | |||
| 17 | + | var ErrUnsatisfiable = errors.New("httprange: unsatisfiable byte range") | |||
| 18 | + | ||||
| 19 | + | // Select returns the part of a size-byte body to send. | |||
| 20 | + | // | |||
| 21 | + | // Without a usable Range it returns the whole body (0, size, false, nil): | |||
| 22 | + | // - the method is not GET (HEAD answers with the headers of the full body); | |||
| 23 | + | // - there is no Range header, its unit is not "bytes", or it lists several | |||
| 24 | + | // ranges (ADR 0034: multipart ranges are not served); | |||
| 25 | + | // - If-Range is present and is not exactly etag. The comparison is strong: | |||
| 26 | + | // a weak validator or an HTTP date never matches. | |||
| 27 | + | // | |||
| 28 | + | // A single "bytes=a-b", "bytes=a-" or "bytes=-n" gives (offset, length, true, | |||
| 29 | + | // nil); an end past the content is cut to it. A malformed range, a start at or | |||
| 30 | + | // past size, a suffix of zero, or any range of an empty body returns | |||
| 31 | + | // ErrUnsatisfiable. | |||
| 32 | + | func Select(r *http.Request, size int64, etag string) (offset, length int64, partial bool, err error) { | |||
| 33 | + | whole := func() (int64, int64, bool, error) { return 0, size, false, nil } | |||
| 34 | + | if r.Method != http.MethodGet { | |||
| 35 | + | return whole() | |||
| 36 | + | } | |||
| 37 | + | value := strings.Join(r.Header.Values("Range"), ",") | |||
| 38 | + | if value == "" || strings.Contains(value, ",") || !strings.HasPrefix(value, "bytes=") { | |||
| 39 | + | return whole() | |||
| 40 | + | } | |||
| 41 | + | if ifRange := r.Header.Get("If-Range"); ifRange != "" && (etag == "" || ifRange != etag || strings.HasPrefix(ifRange, "W/")) { | |||
| 42 | + | return whole() | |||
| 43 | + | } | |||
| 44 | + | a, b, ok := strings.Cut(strings.TrimSpace(strings.TrimPrefix(value, "bytes=")), "-") | |||
| 45 | + | if !ok || size <= 0 { | |||
| 46 | + | return 0, 0, false, ErrUnsatisfiable | |||
| 47 | + | } | |||
| 48 | + | if a == "" { | |||
| 49 | + | n, e := strconv.ParseInt(b, 10, 64) | |||
| 50 | + | if e != nil || n <= 0 { | |||
| 51 | + | return 0, 0, false, ErrUnsatisfiable | |||
| 52 | + | } | |||
| 53 | + | n = min(n, size) | |||
| 54 | + | return size - n, n, true, nil | |||
| 55 | + | } | |||
| 56 | + | start, e := strconv.ParseInt(a, 10, 64) | |||
| 57 | + | if e != nil || start < 0 || start >= size { | |||
| 58 | + | return 0, 0, false, ErrUnsatisfiable | |||
| 59 | + | } | |||
| 60 | + | end := size - 1 | |||
| 61 | + | if b != "" { | |||
| 62 | + | end, e = strconv.ParseInt(b, 10, 64) | |||
| 63 | + | if e != nil || end < start { | |||
| 64 | + | return 0, 0, false, ErrUnsatisfiable | |||
| 65 | + | } | |||
| 66 | + | end = min(end, size-1) | |||
| 67 | + | } | |||
| 68 | + | return start, end - start + 1, true, nil | |||
| 69 | + | } | |||
| 70 | + | ||||
| 71 | + | // ContentRange is the Content-Range value of a partial answer. | |||
| 72 | + | func ContentRange(offset, length, size int64) string { | |||
| 73 | + | return "bytes " + strconv.FormatInt(offset, 10) + "-" + strconv.FormatInt(offset+length-1, 10) + "/" + strconv.FormatInt(size, 10) | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | // Unsatisfied is the Content-Range value of a 416 answer. | |||
| 77 | + | func Unsatisfied(size int64) string { return "bytes */" + strconv.FormatInt(size, 10) } | |||
A internal/httprange/httprange_test.go +72 -0
| @@ -0,0 +1,72 @@ | |||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | ||
| 3 | + | package httprange_test | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "net/http" | |
| 7 | + | "net/http/httptest" | |
| 8 | + | "testing" | |
| 9 | + | ||
| 10 | + | "github.com/stretchr/testify/require" | |
| 11 | + | ||
| 12 | + | "bnema.dev/ermite/internal/httprange" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestSelect(t *testing.T) { | |
| 16 | + | const etag = `"sha256-abc"` | |
| 17 | + | for _, tc := range []struct { | |
| 18 | + | name string | |
| 19 | + | method, rng, ifRange string | |
| 20 | + | size int64 | |
| 21 | + | offset, length int64 | |
| 22 | + | partial bool | |
| 23 | + | err error | |
| 24 | + | }{ | |
| 25 | + | {name: "no range", size: 10, length: 10}, | |
| 26 | + | {name: "closed", rng: "bytes=2-4", size: 10, offset: 2, length: 3, partial: true}, | |
| 27 | + | {name: "open", rng: "bytes=7-", size: 10, offset: 7, length: 3, partial: true}, | |
| 28 | + | {name: "suffix", rng: "bytes=-4", size: 10, offset: 6, length: 4, partial: true}, | |
| 29 | + | {name: "suffix longer than body", rng: "bytes=-40", size: 10, offset: 0, length: 10, partial: true}, | |
| 30 | + | {name: "end cut to size", rng: "bytes=8-99", size: 10, offset: 8, length: 2, partial: true}, | |
| 31 | + | {name: "spaces", rng: "bytes= 1-2 ", size: 10, offset: 1, length: 2, partial: true}, | |
| 32 | + | {name: "start past end", rng: "bytes=10-", size: 10, err: httprange.ErrUnsatisfiable}, | |
| 33 | + | {name: "end before start", rng: "bytes=5-4", size: 10, err: httprange.ErrUnsatisfiable}, | |
| 34 | + | {name: "zero suffix", rng: "bytes=-0", size: 10, err: httprange.ErrUnsatisfiable}, | |
| 35 | + | {name: "garbage", rng: "bytes=a-b", size: 10, err: httprange.ErrUnsatisfiable}, | |
| 36 | + | {name: "no dash", rng: "bytes=5", size: 10, err: httprange.ErrUnsatisfiable}, | |
| 37 | + | {name: "empty body", rng: "bytes=0-", size: 0, err: httprange.ErrUnsatisfiable}, | |
| 38 | + | {name: "multiple ignored", rng: "bytes=0-1,3-4", size: 10, length: 10}, | |
| 39 | + | {name: "other unit ignored", rng: "items=0-1", size: 10, length: 10}, | |
| 40 | + | {name: "head ignores range", method: http.MethodHead, rng: "bytes=2-4", size: 10, length: 10}, | |
| 41 | + | {name: "head ignores bad range", method: http.MethodHead, rng: "bytes=99-", size: 10, length: 10}, | |
| 42 | + | {name: "matching if-range", rng: "bytes=2-4", ifRange: etag, size: 10, offset: 2, length: 3, partial: true}, | |
| 43 | + | {name: "stale if-range", rng: "bytes=2-4", ifRange: `"other"`, size: 10, length: 10}, | |
| 44 | + | {name: "date if-range", rng: "bytes=2-4", ifRange: "Wed, 21 Oct 2015 07:28:00 GMT", size: 10, length: 10}, | |
| 45 | + | {name: "weak if-range", rng: "bytes=2-4", ifRange: "W/" + etag, size: 10, length: 10}, | |
| 46 | + | {name: "stale if-range with bad range", rng: "bytes=99-", ifRange: `"other"`, size: 10, length: 10}, | |
| 47 | + | } { | |
| 48 | + | t.Run(tc.name, func(t *testing.T) { | |
| 49 | + | method := tc.method | |
| 50 | + | if method == "" { | |
| 51 | + | method = http.MethodGet | |
| 52 | + | } | |
| 53 | + | r := httptest.NewRequest(method, "/x", nil) | |
| 54 | + | if tc.rng != "" { | |
| 55 | + | r.Header.Set("Range", tc.rng) | |
| 56 | + | } | |
| 57 | + | if tc.ifRange != "" { | |
| 58 | + | r.Header.Set("If-Range", tc.ifRange) | |
| 59 | + | } | |
| 60 | + | offset, length, partial, err := httprange.Select(r, tc.size, etag) | |
| 61 | + | require.ErrorIs(t, err, tc.err) | |
| 62 | + | require.Equal(t, tc.offset, offset) | |
| 63 | + | require.Equal(t, tc.length, length) | |
| 64 | + | require.Equal(t, tc.partial, partial) | |
| 65 | + | }) | |
| 66 | + | } | |
| 67 | + | } | |
| 68 | + | ||
| 69 | + | func TestContentRange(t *testing.T) { | |
| 70 | + | require.Equal(t, "bytes 2-4/10", httprange.ContentRange(2, 3, 10)) | |
| 71 | + | require.Equal(t, "bytes */10", httprange.Unsatisfied(10)) | |
| 72 | + | } | |
| @@ -0,0 +1,72 @@ | |||||
| 1 | + | // SPDX-License-Identifier: AGPL-3.0-or-later | |||
| 2 | + | ||||
| 3 | + | package httprange_test | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "net/http" | |||
| 7 | + | "net/http/httptest" | |||
| 8 | + | "testing" | |||
| 9 | + | ||||
| 10 | + | "github.com/stretchr/testify/require" | |||
| 11 | + | ||||
| 12 | + | "bnema.dev/ermite/internal/httprange" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestSelect(t *testing.T) { | |||
| 16 | + | const etag = `"sha256-abc"` | |||
| 17 | + | for _, tc := range []struct { | |||
| 18 | + | name string | |||
| 19 | + | method, rng, ifRange string | |||
| 20 | + | size int64 | |||
| 21 | + | offset, length int64 | |||
| 22 | + | partial bool | |||
| 23 | + | err error | |||
| 24 | + | }{ | |||
| 25 | + | {name: "no range", size: 10, length: 10}, | |||
| 26 | + | {name: "closed", rng: "bytes=2-4", size: 10, offset: 2, length: 3, partial: true}, | |||
| 27 | + | {name: "open", rng: "bytes=7-", size: 10, offset: 7, length: 3, partial: true}, | |||
| 28 | + | {name: "suffix", rng: "bytes=-4", size: 10, offset: 6, length: 4, partial: true}, | |||
| 29 | + | {name: "suffix longer than body", rng: "bytes=-40", size: 10, offset: 0, length: 10, partial: true}, | |||
| 30 | + | {name: "end cut to size", rng: "bytes=8-99", size: 10, offset: 8, length: 2, partial: true}, | |||
| 31 | + | {name: "spaces", rng: "bytes= 1-2 ", size: 10, offset: 1, length: 2, partial: true}, | |||
| 32 | + | {name: "start past end", rng: "bytes=10-", size: 10, err: httprange.ErrUnsatisfiable}, | |||
| 33 | + | {name: "end before start", rng: "bytes=5-4", size: 10, err: httprange.ErrUnsatisfiable}, | |||
| 34 | + | {name: "zero suffix", rng: "bytes=-0", size: 10, err: httprange.ErrUnsatisfiable}, | |||
| 35 | + | {name: "garbage", rng: "bytes=a-b", size: 10, err: httprange.ErrUnsatisfiable}, | |||
| 36 | + | {name: "no dash", rng: "bytes=5", size: 10, err: httprange.ErrUnsatisfiable}, | |||
| 37 | + | {name: "empty body", rng: "bytes=0-", size: 0, err: httprange.ErrUnsatisfiable}, | |||
| 38 | + | {name: "multiple ignored", rng: "bytes=0-1,3-4", size: 10, length: 10}, | |||
| 39 | + | {name: "other unit ignored", rng: "items=0-1", size: 10, length: 10}, | |||
| 40 | + | {name: "head ignores range", method: http.MethodHead, rng: "bytes=2-4", size: 10, length: 10}, | |||
| 41 | + | {name: "head ignores bad range", method: http.MethodHead, rng: "bytes=99-", size: 10, length: 10}, | |||
| 42 | + | {name: "matching if-range", rng: "bytes=2-4", ifRange: etag, size: 10, offset: 2, length: 3, partial: true}, | |||
| 43 | + | {name: "stale if-range", rng: "bytes=2-4", ifRange: `"other"`, size: 10, length: 10}, | |||
| 44 | + | {name: "date if-range", rng: "bytes=2-4", ifRange: "Wed, 21 Oct 2015 07:28:00 GMT", size: 10, length: 10}, | |||
| 45 | + | {name: "weak if-range", rng: "bytes=2-4", ifRange: "W/" + etag, size: 10, length: 10}, | |||
| 46 | + | {name: "stale if-range with bad range", rng: "bytes=99-", ifRange: `"other"`, size: 10, length: 10}, | |||
| 47 | + | } { | |||
| 48 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 49 | + | method := tc.method | |||
| 50 | + | if method == "" { | |||
| 51 | + | method = http.MethodGet | |||
| 52 | + | } | |||
| 53 | + | r := httptest.NewRequest(method, "/x", nil) | |||
| 54 | + | if tc.rng != "" { | |||
| 55 | + | r.Header.Set("Range", tc.rng) | |||
| 56 | + | } | |||
| 57 | + | if tc.ifRange != "" { | |||
| 58 | + | r.Header.Set("If-Range", tc.ifRange) | |||
| 59 | + | } | |||
| 60 | + | offset, length, partial, err := httprange.Select(r, tc.size, etag) | |||
| 61 | + | require.ErrorIs(t, err, tc.err) | |||
| 62 | + | require.Equal(t, tc.offset, offset) | |||
| 63 | + | require.Equal(t, tc.length, length) | |||
| 64 | + | require.Equal(t, tc.partial, partial) | |||
| 65 | + | }) | |||
| 66 | + | } | |||
| 67 | + | } | |||
| 68 | + | ||||
| 69 | + | func TestContentRange(t *testing.T) { | |||
| 70 | + | require.Equal(t, "bytes 2-4/10", httprange.ContentRange(2, 3, 10)) | |||
| 71 | + | require.Equal(t, "bytes */10", httprange.Unsatisfied(10)) | |||
| 72 | + | } | |||
M internal/usecase/events.go +13 -4
| @@ -540,13 +540,13 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f | |||
| 540 | 540 | case domain.MirrorRecovered: | |
| 541 | 541 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | |
| 542 | 542 | case domain.ReleasePublished: | |
| 543 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | |
| 543 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) | |
| 544 | 544 | case domain.ReleaseUpdated: | |
| 545 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | |
| 545 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) | |
| 546 | 546 | case domain.ReleaseDeleted: | |
| 547 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | |
| 547 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) | |
| 548 | 548 | case domain.ReleaseUnpublished: | |
| 549 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | |
| 549 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) | |
| 550 | 550 | case domain.PushEvent: | |
| 551 | 551 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | |
| 552 | 552 | case domain.MergeRequestOpened: | |
| @@ -662,6 +662,15 @@ func (e *Events) deliverRepo(ctx context.Context, w *watch, name domain.RepoName | |||
| 662 | 662 | return send(out) | |
| 663 | 663 | } | |
| 664 | 664 | ||
| 665 | + | // deliverRelease sends a release event. The event of a draft goes only to | |
| 666 | + | // release writers: a draft is invisible to every other reader. | |
| 667 | + | func (e *Events) deliverRelease(ctx context.Context, w *watch, rel domain.Release, be bufferedEvent, out ports.WatchEvent, send func(ports.WatchEvent) error) error { | |
| 668 | + | if rel.Draft && !releaseWriter(w.p, rel.Repo) { | |
| 669 | + | return nil | |
| 670 | + | } | |
| 671 | + | return e.deliverRepo(ctx, w, rel.Repo, false, be, out, send) | |
| 672 | + | } | |
| 673 | + | ||
| 665 | 674 | // isIssueEvent reports whether ev is about an issue. | |
| 666 | 675 | func isIssueEvent(ev domain.Event) bool { | |
| 667 | 676 | switch ev.(type) { | |
| @@ -540,13 +540,13 @@ func (e *Events) deliver(ctx context.Context, w *watch, be bufferedEvent, send f | |||||
| 540 | case domain.MirrorRecovered: | 540 | case domain.MirrorRecovered: | ||
| 541 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | 541 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | ||
| 542 | case domain.ReleasePublished: | 542 | case domain.ReleasePublished: | ||
| 543 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | 543 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) |
| 544 | case domain.ReleaseUpdated: | 544 | case domain.ReleaseUpdated: | ||
| 545 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | 545 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) |
| 546 | case domain.ReleaseDeleted: | 546 | case domain.ReleaseDeleted: | ||
| 547 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | 547 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) |
| 548 | case domain.ReleaseUnpublished: | 548 | case domain.ReleaseUnpublished: | ||
| 549 | - | return e.deliverRepo(ctx, w, ev.Release.Repo, false, be, out, send) | 549 | + | return e.deliverRelease(ctx, w, ev.Release, be, out, send) |
| 550 | case domain.PushEvent: | 550 | case domain.PushEvent: | ||
| 551 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | 551 | return e.deliverRepo(ctx, w, ev.Repo, false, be, out, send) | ||
| 552 | case domain.MergeRequestOpened: | 552 | case domain.MergeRequestOpened: | ||
| @@ -662,6 +662,15 @@ func (e *Events) deliverRepo(ctx context.Context, w *watch, name domain.RepoName | |||||
| 662 | return send(out) | 662 | return send(out) | ||
| 663 | } | 663 | } | ||
| 664 | 664 | ||||
| 665 | + | // deliverRelease sends a release event. The event of a draft goes only to | |||
| 666 | + | // release writers: a draft is invisible to every other reader. | |||
| 667 | + | func (e *Events) deliverRelease(ctx context.Context, w *watch, rel domain.Release, be bufferedEvent, out ports.WatchEvent, send func(ports.WatchEvent) error) error { | |||
| 668 | + | if rel.Draft && !releaseWriter(w.p, rel.Repo) { | |||
| 669 | + | return nil | |||
| 670 | + | } | |||
| 671 | + | return e.deliverRepo(ctx, w, rel.Repo, false, be, out, send) | |||
| 672 | + | } | |||
| 673 | + | ||||
| 665 | // isIssueEvent reports whether ev is about an issue. | 674 | // isIssueEvent reports whether ev is about an issue. | ||
| 666 | func isIssueEvent(ev domain.Event) bool { | 675 | func isIssueEvent(ev domain.Event) bool { | ||
| 667 | switch ev.(type) { | 676 | switch ev.(type) { | ||
M internal/usecase/events_test.go +95 -0
| @@ -1034,3 +1034,98 @@ func TestIssueReferencedRepoOnly(t *testing.T) { | |||
| 1034 | 1034 | } | |
| 1035 | 1035 | }) | |
| 1036 | 1036 | } | |
| 1037 | + | ||
| 1038 | + | func TestDraftReleaseEventsReachOnlyReleaseWriters(t *testing.T) { | |
| 1039 | + | t.Parallel() | |
| 1040 | + | synctest.Test(t, func(t *testing.T) { | |
| 1041 | + | r := newEventRig(t, 64) | |
| 1042 | + | pub := r.repo("pub", domain.VisibilityPublic) | |
| 1043 | + | other := r.repo("other", domain.VisibilityPublic) | |
| 1044 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 1045 | + | defer cancel() | |
| 1046 | + | ||
| 1047 | + | job := func(repo domain.RepoName, level string) domain.Principal { | |
| 1048 | + | return domain.RepoTokenPrincipal(domain.CIRepoToken{ID: "j", Repo: repo, Permissions: map[string]string{"contents": level}}) | |
| 1049 | + | } | |
| 1050 | + | writerBranch := scopedToken(domain.ScopeReleaseWrite, []string{"agent/"}, "pub") | |
| 1051 | + | cases := []struct { | |
| 1052 | + | name string | |
| 1053 | + | p domain.Principal | |
| 1054 | + | want bool | |
| 1055 | + | }{ | |
| 1056 | + | {"anonymous", domain.Anonymous(), false}, | |
| 1057 | + | {"reader", tokenPrincipal(domain.ScopeRepoRead), false}, | |
| 1058 | + | {"branch-limited writer", writerBranch, false}, | |
| 1059 | + | {"job token of another repo", job(other, "write"), false}, | |
| 1060 | + | {"job token contents:read", job(pub, "read"), false}, | |
| 1061 | + | {"release:write token", tokenPrincipal(domain.ScopeReleaseWrite, "pub"), true}, | |
| 1062 | + | {"job token contents:write", job(pub, "write"), true}, | |
| 1063 | + | } | |
| 1064 | + | ||
| 1065 | + | anchor := r.watch(ctx, domain.Root(), ports.WatchParams{}) | |
| 1066 | + | synctest.Wait() | |
| 1067 | + | r.push(pub, 1) | |
| 1068 | + | anchorID := anchor.next(t).ID | |
| 1069 | + | ||
| 1070 | + | draft := domain.Release{ID: 1, Repo: pub, Tag: "d", Draft: true} | |
| 1071 | + | published := domain.Release{ID: 2, Repo: pub, Tag: "v1"} | |
| 1072 | + | events := func(rel domain.Release) []domain.Event { | |
| 1073 | + | return []domain.Event{ | |
| 1074 | + | domain.ReleasePublished{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |
| 1075 | + | domain.ReleaseUpdated{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |
| 1076 | + | domain.ReleaseDeleted{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |
| 1077 | + | domain.ReleaseUnpublished{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |
| 1078 | + | } | |
| 1079 | + | } | |
| 1080 | + | check := func(t *testing.T, w *watcher, want bool) { | |
| 1081 | + | t.Helper() | |
| 1082 | + | if want { | |
| 1083 | + | for _, ev := range events(draft) { | |
| 1084 | + | require.Equal(t, ev.EventKind(), w.next(t).Event.EventKind()) | |
| 1085 | + | } | |
| 1086 | + | } | |
| 1087 | + | for _, ev := range events(published) { | |
| 1088 | + | e := w.next(t).Event | |
| 1089 | + | require.Equal(t, ev.EventKind(), e.EventKind()) | |
| 1090 | + | require.EqualValues(t, 2, releaseOf(e)) | |
| 1091 | + | } | |
| 1092 | + | w.quiet(t, r, pub) | |
| 1093 | + | } | |
| 1094 | + | ||
| 1095 | + | var live []*watcher | |
| 1096 | + | for _, tc := range cases { | |
| 1097 | + | live = append(live, r.watch(ctx, tc.p, ports.WatchParams{})) | |
| 1098 | + | } | |
| 1099 | + | synctest.Wait() | |
| 1100 | + | for _, ev := range events(draft) { | |
| 1101 | + | r.feed <- ev | |
| 1102 | + | } | |
| 1103 | + | for _, ev := range events(published) { | |
| 1104 | + | r.feed <- ev | |
| 1105 | + | } | |
| 1106 | + | for i, tc := range cases { | |
| 1107 | + | t.Log("live", tc.name) | |
| 1108 | + | check(t, live[i], tc.want) | |
| 1109 | + | } | |
| 1110 | + | for _, tc := range cases { | |
| 1111 | + | t.Log("replay", tc.name) | |
| 1112 | + | w := r.watch(ctx, tc.p, ports.WatchParams{ResumeAfter: anchorID}) | |
| 1113 | + | check(t, w, tc.want) | |
| 1114 | + | } | |
| 1115 | + | }) | |
| 1116 | + | } | |
| 1117 | + | ||
| 1118 | + | // releaseOf returns the id of the release an event is about. | |
| 1119 | + | func releaseOf(ev domain.Event) int64 { | |
| 1120 | + | switch ev := ev.(type) { | |
| 1121 | + | case domain.ReleasePublished: | |
| 1122 | + | return ev.Release.ID | |
| 1123 | + | case domain.ReleaseUpdated: | |
| 1124 | + | return ev.Release.ID | |
| 1125 | + | case domain.ReleaseDeleted: | |
| 1126 | + | return ev.Release.ID | |
| 1127 | + | case domain.ReleaseUnpublished: | |
| 1128 | + | return ev.Release.ID | |
| 1129 | + | } | |
| 1130 | + | return 0 | |
| 1131 | + | } | |
| @@ -1034,3 +1034,98 @@ func TestIssueReferencedRepoOnly(t *testing.T) { | |||||
| 1034 | } | 1034 | } | ||
| 1035 | }) | 1035 | }) | ||
| 1036 | } | 1036 | } | ||
| 1037 | + | ||||
| 1038 | + | func TestDraftReleaseEventsReachOnlyReleaseWriters(t *testing.T) { | |||
| 1039 | + | t.Parallel() | |||
| 1040 | + | synctest.Test(t, func(t *testing.T) { | |||
| 1041 | + | r := newEventRig(t, 64) | |||
| 1042 | + | pub := r.repo("pub", domain.VisibilityPublic) | |||
| 1043 | + | other := r.repo("other", domain.VisibilityPublic) | |||
| 1044 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 1045 | + | defer cancel() | |||
| 1046 | + | ||||
| 1047 | + | job := func(repo domain.RepoName, level string) domain.Principal { | |||
| 1048 | + | return domain.RepoTokenPrincipal(domain.CIRepoToken{ID: "j", Repo: repo, Permissions: map[string]string{"contents": level}}) | |||
| 1049 | + | } | |||
| 1050 | + | writerBranch := scopedToken(domain.ScopeReleaseWrite, []string{"agent/"}, "pub") | |||
| 1051 | + | cases := []struct { | |||
| 1052 | + | name string | |||
| 1053 | + | p domain.Principal | |||
| 1054 | + | want bool | |||
| 1055 | + | }{ | |||
| 1056 | + | {"anonymous", domain.Anonymous(), false}, | |||
| 1057 | + | {"reader", tokenPrincipal(domain.ScopeRepoRead), false}, | |||
| 1058 | + | {"branch-limited writer", writerBranch, false}, | |||
| 1059 | + | {"job token of another repo", job(other, "write"), false}, | |||
| 1060 | + | {"job token contents:read", job(pub, "read"), false}, | |||
| 1061 | + | {"release:write token", tokenPrincipal(domain.ScopeReleaseWrite, "pub"), true}, | |||
| 1062 | + | {"job token contents:write", job(pub, "write"), true}, | |||
| 1063 | + | } | |||
| 1064 | + | ||||
| 1065 | + | anchor := r.watch(ctx, domain.Root(), ports.WatchParams{}) | |||
| 1066 | + | synctest.Wait() | |||
| 1067 | + | r.push(pub, 1) | |||
| 1068 | + | anchorID := anchor.next(t).ID | |||
| 1069 | + | ||||
| 1070 | + | draft := domain.Release{ID: 1, Repo: pub, Tag: "d", Draft: true} | |||
| 1071 | + | published := domain.Release{ID: 2, Repo: pub, Tag: "v1"} | |||
| 1072 | + | events := func(rel domain.Release) []domain.Event { | |||
| 1073 | + | return []domain.Event{ | |||
| 1074 | + | domain.ReleasePublished{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |||
| 1075 | + | domain.ReleaseUpdated{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |||
| 1076 | + | domain.ReleaseDeleted{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |||
| 1077 | + | domain.ReleaseUnpublished{Release: rel, At: now, CreatedAt: now, Visibility: domain.VisibilityPublic}, | |||
| 1078 | + | } | |||
| 1079 | + | } | |||
| 1080 | + | check := func(t *testing.T, w *watcher, want bool) { | |||
| 1081 | + | t.Helper() | |||
| 1082 | + | if want { | |||
| 1083 | + | for _, ev := range events(draft) { | |||
| 1084 | + | require.Equal(t, ev.EventKind(), w.next(t).Event.EventKind()) | |||
| 1085 | + | } | |||
| 1086 | + | } | |||
| 1087 | + | for _, ev := range events(published) { | |||
| 1088 | + | e := w.next(t).Event | |||
| 1089 | + | require.Equal(t, ev.EventKind(), e.EventKind()) | |||
| 1090 | + | require.EqualValues(t, 2, releaseOf(e)) | |||
| 1091 | + | } | |||
| 1092 | + | w.quiet(t, r, pub) | |||
| 1093 | + | } | |||
| 1094 | + | ||||
| 1095 | + | var live []*watcher | |||
| 1096 | + | for _, tc := range cases { | |||
| 1097 | + | live = append(live, r.watch(ctx, tc.p, ports.WatchParams{})) | |||
| 1098 | + | } | |||
| 1099 | + | synctest.Wait() | |||
| 1100 | + | for _, ev := range events(draft) { | |||
| 1101 | + | r.feed <- ev | |||
| 1102 | + | } | |||
| 1103 | + | for _, ev := range events(published) { | |||
| 1104 | + | r.feed <- ev | |||
| 1105 | + | } | |||
| 1106 | + | for i, tc := range cases { | |||
| 1107 | + | t.Log("live", tc.name) | |||
| 1108 | + | check(t, live[i], tc.want) | |||
| 1109 | + | } | |||
| 1110 | + | for _, tc := range cases { | |||
| 1111 | + | t.Log("replay", tc.name) | |||
| 1112 | + | w := r.watch(ctx, tc.p, ports.WatchParams{ResumeAfter: anchorID}) | |||
| 1113 | + | check(t, w, tc.want) | |||
| 1114 | + | } | |||
| 1115 | + | }) | |||
| 1116 | + | } | |||
| 1117 | + | ||||
| 1118 | + | // releaseOf returns the id of the release an event is about. | |||
| 1119 | + | func releaseOf(ev domain.Event) int64 { | |||
| 1120 | + | switch ev := ev.(type) { | |||
| 1121 | + | case domain.ReleasePublished: | |||
| 1122 | + | return ev.Release.ID | |||
| 1123 | + | case domain.ReleaseUpdated: | |||
| 1124 | + | return ev.Release.ID | |||
| 1125 | + | case domain.ReleaseDeleted: | |||
| 1126 | + | return ev.Release.ID | |||
| 1127 | + | case domain.ReleaseUnpublished: | |||
| 1128 | + | return ev.Release.ID | |||
| 1129 | + | } | |||
| 1130 | + | return 0 | |||
| 1131 | + | } | |||
M internal/usecase/releases.go +9 -4
| @@ -298,16 +298,17 @@ func (r *Releases) ListReleases(ctx context.Context, p domain.Principal, name do | |||
| 298 | 298 | return 0 | |
| 299 | 299 | }) | |
| 300 | 300 | start := token == "" | |
| 301 | + | writer := releaseWriter(p, name) | |
| 301 | 302 | for _, rel := range all { | |
| 303 | + | if rel.Draft && !writer { | |
| 304 | + | continue // a hidden draft is not even a valid cursor | |
| 305 | + | } | |
| 302 | 306 | if !start { | |
| 303 | 307 | if rel.ID == cursor { | |
| 304 | 308 | start = true | |
| 305 | 309 | } | |
| 306 | 310 | continue | |
| 307 | 311 | } | |
| 308 | - | if rel.Draft && !releaseWriter(p, name) { | |
| 309 | - | continue | |
| 310 | - | } | |
| 311 | 312 | if len(out.Releases) == n { | |
| 312 | 313 | out.NextPageToken = strconv.FormatInt(out.Releases[n-1].ID, 10) | |
| 313 | 314 | break | |
| @@ -362,7 +363,11 @@ func (r *Releases) UpdateRelease(ctx context.Context, p domain.Principal, name d | |||
| 362 | 363 | if params.MakeLatest != nil { | |
| 363 | 364 | rel.MakeLatest = *params.MakeLatest | |
| 364 | 365 | } | |
| 365 | - | if err = r.bind(ctx, p, &rel, "", false); err != nil { | |
| 366 | + | target := "" | |
| 367 | + | if params.TargetCommitish != nil { | |
| 368 | + | target = *params.TargetCommitish | |
| 369 | + | } | |
| 370 | + | if err = r.bind(ctx, p, &rel, target, target != ""); err != nil { | |
| 366 | 371 | return err | |
| 367 | 372 | } | |
| 368 | 373 | kind = "updated" | |
| @@ -298,16 +298,17 @@ func (r *Releases) ListReleases(ctx context.Context, p domain.Principal, name do | |||||
| 298 | return 0 | 298 | return 0 | ||
| 299 | }) | 299 | }) | ||
| 300 | start := token == "" | 300 | start := token == "" | ||
| 301 | + | writer := releaseWriter(p, name) | |||
| 301 | for _, rel := range all { | 302 | for _, rel := range all { | ||
| 303 | + | if rel.Draft && !writer { | |||
| 304 | + | continue // a hidden draft is not even a valid cursor | |||
| 305 | + | } | |||
| 302 | if !start { | 306 | if !start { | ||
| 303 | if rel.ID == cursor { | 307 | if rel.ID == cursor { | ||
| 304 | start = true | 308 | start = true | ||
| 305 | } | 309 | } | ||
| 306 | continue | 310 | continue | ||
| 307 | } | 311 | } | ||
| 308 | - | if rel.Draft && !releaseWriter(p, name) { | |||
| 309 | - | continue | |||
| 310 | - | } | |||
| 311 | if len(out.Releases) == n { | 312 | if len(out.Releases) == n { | ||
| 312 | out.NextPageToken = strconv.FormatInt(out.Releases[n-1].ID, 10) | 313 | out.NextPageToken = strconv.FormatInt(out.Releases[n-1].ID, 10) | ||
| 313 | break | 314 | break | ||
| @@ -362,7 +363,11 @@ func (r *Releases) UpdateRelease(ctx context.Context, p domain.Principal, name d | |||||
| 362 | if params.MakeLatest != nil { | 363 | if params.MakeLatest != nil { | ||
| 363 | rel.MakeLatest = *params.MakeLatest | 364 | rel.MakeLatest = *params.MakeLatest | ||
| 364 | } | 365 | } | ||
| 365 | - | if err = r.bind(ctx, p, &rel, "", false); err != nil { | 366 | + | target := "" |
| 367 | + | if params.TargetCommitish != nil { | |||
| 368 | + | target = *params.TargetCommitish | |||
| 369 | + | } | |||
| 370 | + | if err = r.bind(ctx, p, &rel, target, target != ""); err != nil { | |||
| 366 | return err | 371 | return err | ||
| 367 | } | 372 | } | ||
| 368 | kind = "updated" | 373 | kind = "updated" | ||
M internal/usecase/releases_assets.go +10 -1
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | "fmt" | |
| 11 | 11 | "io" | |
| 12 | 12 | "slices" | |
| 13 | + | "strings" | |
| 13 | 14 | "time" | |
| 14 | 15 | ||
| 15 | 16 | "bnema.dev/ermite/internal/domain" | |
| @@ -391,7 +392,15 @@ func (r *Releases) DownloadAsset(ctx context.Context, p domain.Principal, repo d | |||
| 391 | 392 | if err != nil { | |
| 392 | 393 | return err | |
| 393 | 394 | } | |
| 394 | - | body, _, err := r.blobs.Open(ctx, a.BlobKey(), offset, length) | |
| 395 | + | var body io.ReadCloser | |
| 396 | + | if a.Size == 0 && offset == 0 { | |
| 397 | + | // Blob stores reject Open on an empty object: stat it instead. | |
| 398 | + | if _, err = r.blobs.Stat(ctx, a.BlobKey()); err == nil { | |
| 399 | + | body = io.NopCloser(strings.NewReader("")) | |
| 400 | + | } | |
| 401 | + | } else { | |
| 402 | + | body, _, err = r.blobs.Open(ctx, a.BlobKey(), offset, length) | |
| 403 | + | } | |
| 395 | 404 | if err != nil { | |
| 396 | 405 | if errors.Is(err, domain.ErrNotFound) { | |
| 397 | 406 | ctxLog(ctx).Error().Int64("asset", a.ID).Msg("asset blob missing") | |
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | "fmt" | 10 | "fmt" | ||
| 11 | "io" | 11 | "io" | ||
| 12 | "slices" | 12 | "slices" | ||
| 13 | + | "strings" | |||
| 13 | "time" | 14 | "time" | ||
| 14 | 15 | ||||
| 15 | "bnema.dev/ermite/internal/domain" | 16 | "bnema.dev/ermite/internal/domain" | ||
| @@ -391,7 +392,15 @@ func (r *Releases) DownloadAsset(ctx context.Context, p domain.Principal, repo d | |||||
| 391 | if err != nil { | 392 | if err != nil { | ||
| 392 | return err | 393 | return err | ||
| 393 | } | 394 | } | ||
| 394 | - | body, _, err := r.blobs.Open(ctx, a.BlobKey(), offset, length) | 395 | + | var body io.ReadCloser |
| 396 | + | if a.Size == 0 && offset == 0 { | |||
| 397 | + | // Blob stores reject Open on an empty object: stat it instead. | |||
| 398 | + | if _, err = r.blobs.Stat(ctx, a.BlobKey()); err == nil { | |||
| 399 | + | body = io.NopCloser(strings.NewReader("")) | |||
| 400 | + | } | |||
| 401 | + | } else { | |||
| 402 | + | body, _, err = r.blobs.Open(ctx, a.BlobKey(), offset, length) | |||
| 403 | + | } | |||
| 395 | if err != nil { | 404 | if err != nil { | ||
| 396 | if errors.Is(err, domain.ErrNotFound) { | 405 | if errors.Is(err, domain.ErrNotFound) { | ||
| 397 | ctxLog(ctx).Error().Int64("asset", a.ID).Msg("asset blob missing") | 406 | ctxLog(ctx).Error().Int64("asset", a.ID).Msg("asset blob missing") | ||
M internal/usecase/releases_assets_test.go +19 -0
| @@ -287,3 +287,22 @@ func TestReleaseAssetQuotaAcrossReleasesAndWrongRepository(t *testing.T) { | |||
| 287 | 287 | _, err = r.svc.UploadAsset(r.ctx, domain.Root(), other, ports.UploadAssetParams{ReleaseID: b.ID, Name: "wrong"}, bytes.NewReader(nil)) | |
| 288 | 288 | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |
| 289 | 289 | } | |
| 290 | + | ||
| 291 | + | func TestReleaseAssetDownloadZeroBytes(t *testing.T) { | |
| 292 | + | r := newReleaseRig(t, ports.ReleaseLimits{AssetMax: 10, Quota: 10}) | |
| 293 | + | rel := r.create(t, "v1", false) | |
| 294 | + | a := r.upload(t, rel, "empty", nil) | |
| 295 | + | var chunks []ports.AssetChunk | |
| 296 | + | err := r.svc.DownloadAsset(r.ctx, domain.Anonymous(), r.repo, ports.AssetRef{ID: a.ID}, 0, -1, func(c ports.AssetChunk) error { | |
| 297 | + | chunks = append(chunks, c) | |
| 298 | + | return nil | |
| 299 | + | }) | |
| 300 | + | require.NoError(t, err) | |
| 301 | + | require.Len(t, chunks, 1) | |
| 302 | + | require.Equal(t, a.ID, chunks[0].Asset.ID) | |
| 303 | + | ||
| 304 | + | // A missing blob stays an error. | |
| 305 | + | require.NoError(t, r.blobs.Delete(r.ctx, a.BlobKey())) | |
| 306 | + | err = r.svc.DownloadAsset(r.ctx, domain.Anonymous(), r.repo, ports.AssetRef{ID: a.ID}, 0, -1, func(ports.AssetChunk) error { return nil }) | |
| 307 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |
| 308 | + | } | |
| @@ -287,3 +287,22 @@ func TestReleaseAssetQuotaAcrossReleasesAndWrongRepository(t *testing.T) { | |||||
| 287 | _, err = r.svc.UploadAsset(r.ctx, domain.Root(), other, ports.UploadAssetParams{ReleaseID: b.ID, Name: "wrong"}, bytes.NewReader(nil)) | 287 | _, err = r.svc.UploadAsset(r.ctx, domain.Root(), other, ports.UploadAssetParams{ReleaseID: b.ID, Name: "wrong"}, bytes.NewReader(nil)) | ||
| 288 | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | 288 | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | ||
| 289 | } | 289 | } | ||
| 290 | + | ||||
| 291 | + | func TestReleaseAssetDownloadZeroBytes(t *testing.T) { | |||
| 292 | + | r := newReleaseRig(t, ports.ReleaseLimits{AssetMax: 10, Quota: 10}) | |||
| 293 | + | rel := r.create(t, "v1", false) | |||
| 294 | + | a := r.upload(t, rel, "empty", nil) | |||
| 295 | + | var chunks []ports.AssetChunk | |||
| 296 | + | err := r.svc.DownloadAsset(r.ctx, domain.Anonymous(), r.repo, ports.AssetRef{ID: a.ID}, 0, -1, func(c ports.AssetChunk) error { | |||
| 297 | + | chunks = append(chunks, c) | |||
| 298 | + | return nil | |||
| 299 | + | }) | |||
| 300 | + | require.NoError(t, err) | |||
| 301 | + | require.Len(t, chunks, 1) | |||
| 302 | + | require.Equal(t, a.ID, chunks[0].Asset.ID) | |||
| 303 | + | ||||
| 304 | + | // A missing blob stays an error. | |||
| 305 | + | require.NoError(t, r.blobs.Delete(r.ctx, a.BlobKey())) | |||
| 306 | + | err = r.svc.DownloadAsset(r.ctx, domain.Anonymous(), r.repo, ports.AssetRef{ID: a.ID}, 0, -1, func(ports.AssetChunk) error { return nil }) | |||
| 307 | + | require.ErrorIs(t, err, domain.ErrNotFound) | |||
| 308 | + | } | |||
M internal/usecase/releases_github.go +1 -1
| @@ -67,7 +67,7 @@ func (g *ReleasesGitHub) assetObj(a domain.ReleaseAsset, tag string) ordered { | |||
| 67 | 67 | {"uploader", g.user(a.Uploader)}, {"content_type", a.ContentType}, {"state", "uploaded"}, | |
| 68 | 68 | {"size", a.Size}, {"digest", a.Digest}, {"download_count", 0}, | |
| 69 | 69 | {"created_at", a.CreatedAt.UTC().Format(timeLayout)}, {"updated_at", a.UpdatedAt.UTC().Format(timeLayout)}, | |
| 70 | - | {"browser_download_url", g.publicURL + "/" + a.Repo.String() + "/releases/download/" + escapeReleaseTag(tag) + "/" + url.PathEscape(a.Name)}, | |
| 70 | + | {"browser_download_url", g.publicURL + "/" + a.Repo.String() + "/releases/download/" + url.PathEscape(tag) + "/" + url.PathEscape(a.Name)}, | |
| 71 | 71 | } | |
| 72 | 72 | } | |
| 73 | 73 | func (g *ReleasesGitHub) releaseObj(ctx context.Context, p domain.Principal, r domain.Release) (ordered, error) { | |
| @@ -67,7 +67,7 @@ func (g *ReleasesGitHub) assetObj(a domain.ReleaseAsset, tag string) ordered { | |||||
| 67 | {"uploader", g.user(a.Uploader)}, {"content_type", a.ContentType}, {"state", "uploaded"}, | 67 | {"uploader", g.user(a.Uploader)}, {"content_type", a.ContentType}, {"state", "uploaded"}, | ||
| 68 | {"size", a.Size}, {"digest", a.Digest}, {"download_count", 0}, | 68 | {"size", a.Size}, {"digest", a.Digest}, {"download_count", 0}, | ||
| 69 | {"created_at", a.CreatedAt.UTC().Format(timeLayout)}, {"updated_at", a.UpdatedAt.UTC().Format(timeLayout)}, | 69 | {"created_at", a.CreatedAt.UTC().Format(timeLayout)}, {"updated_at", a.UpdatedAt.UTC().Format(timeLayout)}, | ||
| 70 | - | {"browser_download_url", g.publicURL + "/" + a.Repo.String() + "/releases/download/" + escapeReleaseTag(tag) + "/" + url.PathEscape(a.Name)}, | 70 | + | {"browser_download_url", g.publicURL + "/" + a.Repo.String() + "/releases/download/" + url.PathEscape(tag) + "/" + url.PathEscape(a.Name)}, |
| 71 | } | 71 | } | ||
| 72 | } | 72 | } | ||
| 73 | func (g *ReleasesGitHub) releaseObj(ctx context.Context, p domain.Principal, r domain.Release) (ordered, error) { | 73 | func (g *ReleasesGitHub) releaseObj(ctx context.Context, p domain.Principal, r domain.Release) (ordered, error) { | ||
M internal/usecase/releases_github_test.go +14 -1
| @@ -162,7 +162,7 @@ func TestGitHubReleaseAssetsAndRetry(t *testing.T) { | |||
| 162 | 162 | URL string `json:"browser_download_url"` | |
| 163 | 163 | } | |
| 164 | 164 | require.NoError(t, json.Unmarshal(b, &asset)) | |
| 165 | - | require.Equal(t, "http://localhost:7341/releases/releases/download/v1/slash/%C3%A9%20%23%3F.txt", asset.URL) | |
| 165 | + | require.Equal(t, "http://localhost:7341/releases/releases/download/v1%2Fslash/%C3%A9%20%23%3F.txt", asset.URL) | |
| 166 | 166 | _, err = upload("é #?.txt") | |
| 167 | 167 | require.ErrorIs(t, err, domain.ErrAlreadyExists) | |
| 168 | 168 | b, err = g.UpdateAsset(r.ctx, p, "acme", repo, asset.ID, []byte(`{"label":""}`)) | |
| @@ -273,6 +273,19 @@ func TestGitHubReleaseTargetAndMissingTag(t *testing.T) { | |||
| 273 | 273 | require.NoError(t, err) | |
| 274 | 274 | require.NoError(t, json.Unmarshal(b, &got)) | |
| 275 | 275 | require.Equal(t, oidA, got.Target) | |
| 276 | + | ||
| 277 | + | // PATCH with target_commitish publishes a draft whose tag is missing. | |
| 278 | + | r.resolve(t, "patched", domain.ErrNotFound) | |
| 279 | + | b, err = g.CreateRelease(r.ctx, p, "acme", repo, []byte(`{"tag_name":"patched","draft":true}`)) | |
| 280 | + | require.NoError(t, err) | |
| 281 | + | require.NoError(t, json.Unmarshal(b, &got)) | |
| 282 | + | r.reader.EXPECT().ResolveRevision(r.ctx, r.repo, v).Return(domain.ResolvedRevision{CommitID: oid}, nil).Once() | |
| 283 | + | r.tags.EXPECT().CreateTag(r.ctx, r.repo, "patched", oid).Return(nil).Once() | |
| 284 | + | r.pushes.EXPECT().Record(r.ctx, p, r.repo, []domain.RefUpdate{{Ref: "refs/tags/patched", Old: domain.ZeroOID, New: oidA}}).Return(nil).Once() | |
| 285 | + | b, err = g.UpdateRelease(r.ctx, p, "acme", repo, got.ID, []byte(`{"draft":false,"target_commitish":"main"}`)) | |
| 286 | + | require.NoError(t, err) | |
| 287 | + | require.Contains(t, string(b), `"draft":false`) | |
| 288 | + | require.Contains(t, string(b), `"target_commitish":"`+oidA+`"`) | |
| 276 | 289 | } | |
| 277 | 290 | ||
| 278 | 291 | // A real HTTP round trip through the facade, metadata store and disk blob | |
| @@ -162,7 +162,7 @@ func TestGitHubReleaseAssetsAndRetry(t *testing.T) { | |||||
| 162 | URL string `json:"browser_download_url"` | 162 | URL string `json:"browser_download_url"` | ||
| 163 | } | 163 | } | ||
| 164 | require.NoError(t, json.Unmarshal(b, &asset)) | 164 | require.NoError(t, json.Unmarshal(b, &asset)) | ||
| 165 | - | require.Equal(t, "http://localhost:7341/releases/releases/download/v1/slash/%C3%A9%20%23%3F.txt", asset.URL) | 165 | + | require.Equal(t, "http://localhost:7341/releases/releases/download/v1%2Fslash/%C3%A9%20%23%3F.txt", asset.URL) |
| 166 | _, err = upload("é #?.txt") | 166 | _, err = upload("é #?.txt") | ||
| 167 | require.ErrorIs(t, err, domain.ErrAlreadyExists) | 167 | require.ErrorIs(t, err, domain.ErrAlreadyExists) | ||
| 168 | b, err = g.UpdateAsset(r.ctx, p, "acme", repo, asset.ID, []byte(`{"label":""}`)) | 168 | b, err = g.UpdateAsset(r.ctx, p, "acme", repo, asset.ID, []byte(`{"label":""}`)) | ||
| @@ -273,6 +273,19 @@ func TestGitHubReleaseTargetAndMissingTag(t *testing.T) { | |||||
| 273 | require.NoError(t, err) | 273 | require.NoError(t, err) | ||
| 274 | require.NoError(t, json.Unmarshal(b, &got)) | 274 | require.NoError(t, json.Unmarshal(b, &got)) | ||
| 275 | require.Equal(t, oidA, got.Target) | 275 | require.Equal(t, oidA, got.Target) | ||
| 276 | + | ||||
| 277 | + | // PATCH with target_commitish publishes a draft whose tag is missing. | |||
| 278 | + | r.resolve(t, "patched", domain.ErrNotFound) | |||
| 279 | + | b, err = g.CreateRelease(r.ctx, p, "acme", repo, []byte(`{"tag_name":"patched","draft":true}`)) | |||
| 280 | + | require.NoError(t, err) | |||
| 281 | + | require.NoError(t, json.Unmarshal(b, &got)) | |||
| 282 | + | r.reader.EXPECT().ResolveRevision(r.ctx, r.repo, v).Return(domain.ResolvedRevision{CommitID: oid}, nil).Once() | |||
| 283 | + | r.tags.EXPECT().CreateTag(r.ctx, r.repo, "patched", oid).Return(nil).Once() | |||
| 284 | + | r.pushes.EXPECT().Record(r.ctx, p, r.repo, []domain.RefUpdate{{Ref: "refs/tags/patched", Old: domain.ZeroOID, New: oidA}}).Return(nil).Once() | |||
| 285 | + | b, err = g.UpdateRelease(r.ctx, p, "acme", repo, got.ID, []byte(`{"draft":false,"target_commitish":"main"}`)) | |||
| 286 | + | require.NoError(t, err) | |||
| 287 | + | require.Contains(t, string(b), `"draft":false`) | |||
| 288 | + | require.Contains(t, string(b), `"target_commitish":"`+oidA+`"`) | |||
| 276 | } | 289 | } | ||
| 277 | 290 | ||||
| 278 | // A real HTTP round trip through the facade, metadata store and disk blob | 291 | // A real HTTP round trip through the facade, metadata store and disk blob | ||
M internal/usecase/releases_integration_test.go +61 -0
| @@ -273,3 +273,64 @@ func TestReleaseWriteRetriesUsageConflict(t *testing.T) { | |||
| 273 | 273 | require.NoError(t, err) | |
| 274 | 274 | require.EqualValues(t, 1, rel.ID) | |
| 275 | 275 | } | |
| 276 | + | ||
| 277 | + | func TestReleaseListHidesDraftCursors(t *testing.T) { | |
| 278 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |
| 279 | + | pub := r.create(t, "v1", false) | |
| 280 | + | d1 := r.create(t, "d1", true) | |
| 281 | + | d2 := r.create(t, "d2", true) | |
| 282 | + | anon := domain.Anonymous() | |
| 283 | + | ||
| 284 | + | _, errDraft := r.svc.ListReleases(r.ctx, anon, r.repo, 1, fmt.Sprint(d1.ID)) | |
| 285 | + | _, errUnknown := r.svc.ListReleases(r.ctx, anon, r.repo, 1, "999999") | |
| 286 | + | require.ErrorIs(t, errUnknown, domain.ErrInvalidPageToken) | |
| 287 | + | require.Equal(t, errUnknown, errDraft) | |
| 288 | + | ||
| 289 | + | // A writer still pages through drafts, newest first. | |
| 290 | + | var got []int64 | |
| 291 | + | token := "" | |
| 292 | + | for { | |
| 293 | + | page, err := r.svc.ListReleases(r.ctx, domain.Root(), r.repo, 1, token) | |
| 294 | + | require.NoError(t, err) | |
| 295 | + | for _, rel := range page.Releases { | |
| 296 | + | got = append(got, rel.ID) | |
| 297 | + | } | |
| 298 | + | if page.NextPageToken == "" { | |
| 299 | + | break | |
| 300 | + | } | |
| 301 | + | token = page.NextPageToken | |
| 302 | + | } | |
| 303 | + | require.ElementsMatch(t, []int64{pub.ID, d1.ID, d2.ID}, got) | |
| 304 | + | } | |
| 305 | + | ||
| 306 | + | func TestReleaseUpdateTargetCreatesMissingTag(t *testing.T) { | |
| 307 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |
| 308 | + | r.resolve(t, "missing", domain.ErrNotFound) | |
| 309 | + | draft, err := r.svc.CreateRelease(r.ctx, domain.Root(), r.repo, ports.CreateReleaseParams{Tag: "missing", Draft: true}) | |
| 310 | + | require.NoError(t, err) | |
| 311 | + | no := false | |
| 312 | + | _, err = r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, draft.ID, ports.UpdateReleaseParams{Draft: &no}) | |
| 313 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |
| 314 | + | ||
| 315 | + | v, err := domain.ParseRevision("main") | |
| 316 | + | require.NoError(t, err) | |
| 317 | + | oid, err := domain.ParseObjectID(oidA) | |
| 318 | + | require.NoError(t, err) | |
| 319 | + | r.reader.EXPECT().ResolveRevision(r.ctx, r.repo, v).Return(domain.ResolvedRevision{CommitID: oid}, nil).Once() | |
| 320 | + | r.tags.EXPECT().CreateTag(r.ctx, r.repo, "missing", oid).Return(nil).Once() | |
| 321 | + | r.pushes.EXPECT().Record(r.ctx, domain.Root(), r.repo, []domain.RefUpdate{{Ref: "refs/tags/missing", Old: domain.ZeroOID, New: oidA}}).Return(nil).Once() | |
| 322 | + | main := "main" | |
| 323 | + | rel, err := r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, draft.ID, ports.UpdateReleaseParams{Draft: &no, TargetCommitish: &main}) | |
| 324 | + | require.NoError(t, err) | |
| 325 | + | require.False(t, rel.Draft) | |
| 326 | + | require.Equal(t, oidA, rel.TargetCommitish) | |
| 327 | + | } | |
| 328 | + | ||
| 329 | + | func TestReleaseUpdateTargetNeverMovesExistingTag(t *testing.T) { | |
| 330 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |
| 331 | + | rel := r.create(t, "v1", false) | |
| 332 | + | other := "other" // never resolved: the mock would fail the test | |
| 333 | + | got, err := r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, rel.ID, ports.UpdateReleaseParams{TargetCommitish: &other}) | |
| 334 | + | require.NoError(t, err) | |
| 335 | + | require.Equal(t, oidA, got.TargetCommitish) | |
| 336 | + | } | |
| @@ -273,3 +273,64 @@ func TestReleaseWriteRetriesUsageConflict(t *testing.T) { | |||||
| 273 | require.NoError(t, err) | 273 | require.NoError(t, err) | ||
| 274 | require.EqualValues(t, 1, rel.ID) | 274 | require.EqualValues(t, 1, rel.ID) | ||
| 275 | } | 275 | } | ||
| 276 | + | ||||
| 277 | + | func TestReleaseListHidesDraftCursors(t *testing.T) { | |||
| 278 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |||
| 279 | + | pub := r.create(t, "v1", false) | |||
| 280 | + | d1 := r.create(t, "d1", true) | |||
| 281 | + | d2 := r.create(t, "d2", true) | |||
| 282 | + | anon := domain.Anonymous() | |||
| 283 | + | ||||
| 284 | + | _, errDraft := r.svc.ListReleases(r.ctx, anon, r.repo, 1, fmt.Sprint(d1.ID)) | |||
| 285 | + | _, errUnknown := r.svc.ListReleases(r.ctx, anon, r.repo, 1, "999999") | |||
| 286 | + | require.ErrorIs(t, errUnknown, domain.ErrInvalidPageToken) | |||
| 287 | + | require.Equal(t, errUnknown, errDraft) | |||
| 288 | + | ||||
| 289 | + | // A writer still pages through drafts, newest first. | |||
| 290 | + | var got []int64 | |||
| 291 | + | token := "" | |||
| 292 | + | for { | |||
| 293 | + | page, err := r.svc.ListReleases(r.ctx, domain.Root(), r.repo, 1, token) | |||
| 294 | + | require.NoError(t, err) | |||
| 295 | + | for _, rel := range page.Releases { | |||
| 296 | + | got = append(got, rel.ID) | |||
| 297 | + | } | |||
| 298 | + | if page.NextPageToken == "" { | |||
| 299 | + | break | |||
| 300 | + | } | |||
| 301 | + | token = page.NextPageToken | |||
| 302 | + | } | |||
| 303 | + | require.ElementsMatch(t, []int64{pub.ID, d1.ID, d2.ID}, got) | |||
| 304 | + | } | |||
| 305 | + | ||||
| 306 | + | func TestReleaseUpdateTargetCreatesMissingTag(t *testing.T) { | |||
| 307 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |||
| 308 | + | r.resolve(t, "missing", domain.ErrNotFound) | |||
| 309 | + | draft, err := r.svc.CreateRelease(r.ctx, domain.Root(), r.repo, ports.CreateReleaseParams{Tag: "missing", Draft: true}) | |||
| 310 | + | require.NoError(t, err) | |||
| 311 | + | no := false | |||
| 312 | + | _, err = r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, draft.ID, ports.UpdateReleaseParams{Draft: &no}) | |||
| 313 | + | require.ErrorIs(t, err, domain.ErrPreconditionFailed) | |||
| 314 | + | ||||
| 315 | + | v, err := domain.ParseRevision("main") | |||
| 316 | + | require.NoError(t, err) | |||
| 317 | + | oid, err := domain.ParseObjectID(oidA) | |||
| 318 | + | require.NoError(t, err) | |||
| 319 | + | r.reader.EXPECT().ResolveRevision(r.ctx, r.repo, v).Return(domain.ResolvedRevision{CommitID: oid}, nil).Once() | |||
| 320 | + | r.tags.EXPECT().CreateTag(r.ctx, r.repo, "missing", oid).Return(nil).Once() | |||
| 321 | + | r.pushes.EXPECT().Record(r.ctx, domain.Root(), r.repo, []domain.RefUpdate{{Ref: "refs/tags/missing", Old: domain.ZeroOID, New: oidA}}).Return(nil).Once() | |||
| 322 | + | main := "main" | |||
| 323 | + | rel, err := r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, draft.ID, ports.UpdateReleaseParams{Draft: &no, TargetCommitish: &main}) | |||
| 324 | + | require.NoError(t, err) | |||
| 325 | + | require.False(t, rel.Draft) | |||
| 326 | + | require.Equal(t, oidA, rel.TargetCommitish) | |||
| 327 | + | } | |||
| 328 | + | ||||
| 329 | + | func TestReleaseUpdateTargetNeverMovesExistingTag(t *testing.T) { | |||
| 330 | + | r := newReleaseRig(t, ports.ReleaseLimits{}) | |||
| 331 | + | rel := r.create(t, "v1", false) | |||
| 332 | + | other := "other" // never resolved: the mock would fail the test | |||
| 333 | + | got, err := r.svc.UpdateRelease(r.ctx, domain.Root(), r.repo, rel.ID, ports.UpdateReleaseParams{TargetCommitish: &other}) | |||
| 334 | + | require.NoError(t, err) | |||
| 335 | + | require.Equal(t, oidA, got.TargetCommitish) | |||
| 336 | + | } | |||
M skills/ermite/CLI.md +2 -2
| @@ -106,9 +106,9 @@ token, or a CI job's `contents: write` permission. Select a release by tag or id | |||
| 106 | 106 | | `release create <repo> <tag> [--title T] [--notes TEXT\|--notes-file F\|-] [--target REF] [--draft] [--prerelease] [--latest] [--asset FILE[#LABEL]] [--json]` | existing tags never move; target creates a missing tag | | |
| 107 | 107 | | `release list <repo> [--limit N] [--json]` | newest first | | |
| 108 | 108 | | `release view <repo> <tag\|id\|latest> [--json\|--notes]` | metadata and assets | | |
| 109 | - | | `release edit <repo> <tag\|id> [--title T] [--notes TEXT\|--notes-file F\|-] [--tag T] [--target REF] [--draft\|--publish] [--prerelease\|--no-prerelease] [--latest] [--json]` | only supplied fields change | | |
| 109 | + | | `release edit <repo> <tag\|id> [--title T] [--notes TEXT\|--notes-file F\|-] [--tag T] [--target REF] [--draft\|--publish] [--prerelease\|--no-prerelease] [--latest]` | only supplied fields change | | |
| 110 | 110 | | `release delete <repo> <tag\|id> [--yes]` | leaves the Git tag | | |
| 111 | - | | `release upload <repo> <tag\|id> <file[#label]>… [--clobber] [--content-type TYPE] [--json]` | clobber replaces same-named assets | | |
| 111 | + | | `release upload <repo> <tag\|id> <file[#label]>… [--clobber] [--content-type TYPE]` | clobber replaces same-named assets | | |
| 112 | 112 | | `release download <repo> <tag\|id\|latest> [--name N]… [--pattern GLOB]… [--dir D]` | refuses existing local files | | |
| 113 | 113 | ||
| 114 | 114 | See [publishing and storage](../../docs/releases.md). | |
| @@ -106,9 +106,9 @@ token, or a CI job's `contents: write` permission. Select a release by tag or id | |||||
| 106 | | `release create <repo> <tag> [--title T] [--notes TEXT\|--notes-file F\|-] [--target REF] [--draft] [--prerelease] [--latest] [--asset FILE[#LABEL]] [--json]` | existing tags never move; target creates a missing tag | | 106 | | `release create <repo> <tag> [--title T] [--notes TEXT\|--notes-file F\|-] [--target REF] [--draft] [--prerelease] [--latest] [--asset FILE[#LABEL]] [--json]` | existing tags never move; target creates a missing tag | | ||
| 107 | | `release list <repo> [--limit N] [--json]` | newest first | | 107 | | `release list <repo> [--limit N] [--json]` | newest first | | ||
| 108 | | `release view <repo> <tag\|id\|latest> [--json\|--notes]` | metadata and assets | | 108 | | `release view <repo> <tag\|id\|latest> [--json\|--notes]` | metadata and assets | | ||
| 109 | - | | `release edit <repo> <tag\|id> [--title T] [--notes TEXT\|--notes-file F\|-] [--tag T] [--target REF] [--draft\|--publish] [--prerelease\|--no-prerelease] [--latest] [--json]` | only supplied fields change | | 109 | + | | `release edit <repo> <tag\|id> [--title T] [--notes TEXT\|--notes-file F\|-] [--tag T] [--target REF] [--draft\|--publish] [--prerelease\|--no-prerelease] [--latest]` | only supplied fields change | |
| 110 | | `release delete <repo> <tag\|id> [--yes]` | leaves the Git tag | | 110 | | `release delete <repo> <tag\|id> [--yes]` | leaves the Git tag | | ||
| 111 | - | | `release upload <repo> <tag\|id> <file[#label]>… [--clobber] [--content-type TYPE] [--json]` | clobber replaces same-named assets | | 111 | + | | `release upload <repo> <tag\|id> <file[#label]>… [--clobber] [--content-type TYPE]` | clobber replaces same-named assets | |
| 112 | | `release download <repo> <tag\|id\|latest> [--name N]… [--pattern GLOB]… [--dir D]` | refuses existing local files | | 112 | | `release download <repo> <tag\|id\|latest> [--name N]… [--pattern GLOB]… [--dir D]` | refuses existing local files | | ||
| 113 | 113 | ||||
| 114 | See [publishing and storage](../../docs/releases.md). | 114 | See [publishing and storage](../../docs/releases.md). | ||