fork of whitequark.org/git-pages with mods for tangled

Refactor `S3Backend.GetManifest`. NFCI

This is both to reduce the amount of loose variables in the code, as
well as to make it closer to `S3Backend.GetBlob`.

Changed files
+27 -18
src
+27 -18
src/backend_s3.go
··· 399 399 } 400 400 401 401 func (l s3ManifestLoader) load(ctx context.Context, name string, oldManifest *CachedManifest) (*CachedManifest, error) { 402 - log.Printf("s3: get manifest %s\n", name) 402 + loader := func() (*CachedManifest, error) { 403 + log.Printf("s3: get manifest %s\n", name) 403 404 404 - startTime := time.Now() 405 + startTime := time.Now() 405 406 406 - manifest, size, etag, err := func() (*Manifest, uint32, string, error) { 407 407 opts := minio.GetObjectOptions{} 408 408 if oldManifest != nil && oldManifest.etag != "" { 409 409 opts.SetMatchETagExcept(oldManifest.etag) ··· 411 411 object, err := l.s3.client.GetObject(ctx, l.s3.bucket, manifestObjectName(name), opts) 412 412 // Note that many errors (e.g. NoSuchKey) will be reported only after this point. 413 413 if err != nil { 414 - return nil, 0, "", err 414 + return nil, err 415 415 } 416 416 defer object.Close() 417 417 418 418 data, err := io.ReadAll(object) 419 419 if err != nil { 420 - return nil, 0, "", err 420 + return nil, err 421 421 } 422 422 423 423 stat, err := object.Stat() 424 424 if err != nil { 425 - return nil, 0, "", err 425 + return nil, err 426 426 } 427 427 428 428 manifest, err := DecodeManifest(data) 429 429 if err != nil { 430 - return nil, 0, "", err 430 + return nil, err 431 431 } 432 432 433 - return manifest, uint32(len(data)), stat.ETag, nil 434 - }() 433 + s3GetObjectDurationSeconds. 434 + With(prometheus.Labels{"kind": "manifest"}). 435 + Observe(time.Since(startTime).Seconds()) 435 436 436 - s3GetObjectDurationSeconds. 437 - With(prometheus.Labels{"kind": "manifest"}). 438 - Observe(time.Since(startTime).Seconds()) 437 + return &CachedManifest{manifest, uint32(len(data)), stat.ETag, nil}, nil 438 + } 439 439 440 + var cached *CachedManifest 441 + cached, err := loader() 440 442 if err != nil { 441 443 if errResp := minio.ToErrorResponse(err); errResp.Code == "NoSuchKey" { 442 444 s3GetObjectErrorsCount.With(prometheus.Labels{"object_kind": "manifest"}).Inc() 443 445 err = fmt.Errorf("%w: %s", ErrObjectNotFound, errResp.Key) 444 - return &CachedManifest{nil, 1, etag, err}, nil 446 + return &CachedManifest{nil, 1, "", err}, nil 445 447 } else if errResp.StatusCode == http.StatusNotModified && oldManifest != nil { 446 448 return oldManifest, nil 447 449 } else { 448 450 return nil, err 449 451 } 450 452 } else { 451 - return &CachedManifest{manifest, size, etag, err}, nil 453 + return cached, nil 452 454 } 453 455 } 454 456 455 - func (s3 *S3Backend) GetManifest(ctx context.Context, name string, opts GetManifestOptions) (*Manifest, error) { 457 + func (s3 *S3Backend) GetManifest( 458 + ctx context.Context, name string, opts GetManifestOptions, 459 + ) ( 460 + manifest *Manifest, err error, 461 + ) { 456 462 if opts.BypassCache { 457 463 entry, found := s3.siteCache.Cache.GetEntry(name) 458 464 if found && entry.RefreshableAt().Before(time.Now()) { ··· 460 466 } 461 467 } 462 468 463 - cached, err := s3.siteCache.Get(ctx, name, s3ManifestLoader{s3}) 469 + var cached *CachedManifest 470 + cached, err = s3.siteCache.Get(ctx, name, s3ManifestLoader{s3}) 464 471 if err != nil { 465 - return nil, err 472 + return 466 473 } else { 467 - return cached.manifest, cached.err 474 + // This could be `manifest, nil` or `nil, ErrObjectNotFound`. 475 + manifest, err = cached.manifest, cached.err 476 + return 468 477 } 469 478 } 470 479