2026-03-13 17:05:14 +00:00
package handler
import (
"bytes"
"context"
2026-09-04 10:58:22 +01:00
"crypto/sha256"
2026-03-13 17:05:14 +00:00
"database/sql"
"errors"
"io"
"log/slog"
"net/http"
"net/http/httptest"
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
"slices"
2026-03-13 17:05:14 +00:00
"strings"
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
"sync"
2026-03-13 17:05:14 +00:00
"testing"
"time"
2026-09-04 10:58:22 +01:00
"github.com/git-pkgs/artifacts"
2026-07-17 18:11:47 -07:00
"github.com/git-pkgs/proxy/internal/config"
2026-03-13 17:05:14 +00:00
"github.com/git-pkgs/proxy/internal/database"
2026-08-16 18:01:59 +01:00
"github.com/git-pkgs/proxy/internal/metrics"
2026-03-13 17:05:14 +00:00
"github.com/git-pkgs/proxy/internal/storage"
2026-08-13 07:35:07 +01:00
"github.com/git-pkgs/purl"
2026-03-13 17:05:14 +00:00
"github.com/git-pkgs/registries/fetch"
2026-09-04 10:58:22 +01:00
"github.com/opencontainers/go-digest"
2026-08-29 11:19:54 +01:00
"github.com/prometheus/client_golang/prometheus"
2026-08-16 18:01:59 +01:00
"github.com/prometheus/client_golang/prometheus/testutil"
2026-08-29 11:19:54 +01:00
dto "github.com/prometheus/client_model/go"
2026-03-13 17:05:14 +00:00
)
// mockStorage implements storage.Storage for testing.
type mockStorage struct {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
mu sync . Mutex
2026-04-27 12:04:38 +01:00
files map [ string ] [ ] byte
storeErr error
openErr error
signedURL string
signErr error
Support HTTP byte-range requests for local file cached artifact downloads (#404)
* feat(storage): expose efficient seek for local files
Open file-backed artifacts with os.Open so local readers support
efficient seeking. Leave cloud storage readers unchanged.
Keep the legacy sidecar mapping test focused on fileblob's reader:
Blob.Open now reads local files directly and no longer consults the
sidecar. Continue checking sidecar cleanup and post-store reads.
* feat(handler): preserve efficient seek through integrity checks
Keep the cache result's single Reader field and preserve io.Seeker
when the underlying storage reader supports efficient seeking.
Sequential reads continue through whole-object integrity verification.
A successful seek switches subsequent reads to the source for range
responses, where full-object verification is not possible.
Test both modes and verify the source is closed once.
* feat(handler): add request-aware byte range responses
Add a request-aware artifact serving helper for bounded, open-ended,
and suffix byte ranges, including If-Range handling, 206 responses,
and 416 responses for valid unsatisfiable ranges.
Ignore malformed and multi-range requests by serving the full artifact.
Advertise byte-range support only for seekable readers, preserving HEAD,
redirect, and non-seekable behavior. Clarify that net/http recovers
ErrAbortHandler per request and keeps the server running.
* feat(handler): enable byte ranges across artifact downloads
Forward each artifact request to the shared range-aware response helper,
including cached generic release assets and OCI blobs. This lets the
handler honor ranges only when its reader supports efficient seeking,
without changing direct-storage redirect behavior.
Add warm-cache endpoint tests that assert partial response headers and
bytes without an upstream refetch. Document supported range behavior,
unsupported reader fallback, and the integrity limitation of partial reads.
2026-10-04 09:48:41 -07:00
seekable bool
2026-03-13 17:05:14 +00:00
}
func newMockStorage ( ) * mockStorage {
return & mockStorage { files : make ( map [ string ] [ ] byte ) }
}
func ( s * mockStorage ) Store ( _ context . Context , path string , r io . Reader ) ( int64 , string , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
if s . storeErr != nil {
return 0 , "" , s . storeErr
}
data , err := io . ReadAll ( r )
if err != nil {
return 0 , "" , err
}
s . files [ path ] = data
2026-08-17 09:20:11 +01:00
return int64 ( len ( data ) ) , sha256Hex ( string ( data ) ) , nil
2026-03-13 17:05:14 +00:00
}
func ( s * mockStorage ) Open ( _ context . Context , path string ) ( io . ReadCloser , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
if s . openErr != nil {
return nil , s . openErr
}
data , ok := s . files [ path ]
if ! ok {
return nil , storage . ErrNotFound
}
Support HTTP byte-range requests for local file cached artifact downloads (#404)
* feat(storage): expose efficient seek for local files
Open file-backed artifacts with os.Open so local readers support
efficient seeking. Leave cloud storage readers unchanged.
Keep the legacy sidecar mapping test focused on fileblob's reader:
Blob.Open now reads local files directly and no longer consults the
sidecar. Continue checking sidecar cleanup and post-store reads.
* feat(handler): preserve efficient seek through integrity checks
Keep the cache result's single Reader field and preserve io.Seeker
when the underlying storage reader supports efficient seeking.
Sequential reads continue through whole-object integrity verification.
A successful seek switches subsequent reads to the source for range
responses, where full-object verification is not possible.
Test both modes and verify the source is closed once.
* feat(handler): add request-aware byte range responses
Add a request-aware artifact serving helper for bounded, open-ended,
and suffix byte ranges, including If-Range handling, 206 responses,
and 416 responses for valid unsatisfiable ranges.
Ignore malformed and multi-range requests by serving the full artifact.
Advertise byte-range support only for seekable readers, preserving HEAD,
redirect, and non-seekable behavior. Clarify that net/http recovers
ErrAbortHandler per request and keeps the server running.
* feat(handler): enable byte ranges across artifact downloads
Forward each artifact request to the shared range-aware response helper,
including cached generic release assets and OCI blobs. This lets the
handler honor ranges only when its reader supports efficient seeking,
without changing direct-storage redirect behavior.
Add warm-cache endpoint tests that assert partial response headers and
bytes without an upstream refetch. Document supported range behavior,
unsupported reader fallback, and the integrity limitation of partial reads.
2026-10-04 09:48:41 -07:00
if s . seekable {
return & mockSeekableReadCloser { Reader : bytes . NewReader ( data ) } , nil
}
2026-03-13 17:05:14 +00:00
return io . NopCloser ( bytes . NewReader ( data ) ) , nil
}
Support HTTP byte-range requests for local file cached artifact downloads (#404)
* feat(storage): expose efficient seek for local files
Open file-backed artifacts with os.Open so local readers support
efficient seeking. Leave cloud storage readers unchanged.
Keep the legacy sidecar mapping test focused on fileblob's reader:
Blob.Open now reads local files directly and no longer consults the
sidecar. Continue checking sidecar cleanup and post-store reads.
* feat(handler): preserve efficient seek through integrity checks
Keep the cache result's single Reader field and preserve io.Seeker
when the underlying storage reader supports efficient seeking.
Sequential reads continue through whole-object integrity verification.
A successful seek switches subsequent reads to the source for range
responses, where full-object verification is not possible.
Test both modes and verify the source is closed once.
* feat(handler): add request-aware byte range responses
Add a request-aware artifact serving helper for bounded, open-ended,
and suffix byte ranges, including If-Range handling, 206 responses,
and 416 responses for valid unsatisfiable ranges.
Ignore malformed and multi-range requests by serving the full artifact.
Advertise byte-range support only for seekable readers, preserving HEAD,
redirect, and non-seekable behavior. Clarify that net/http recovers
ErrAbortHandler per request and keeps the server running.
* feat(handler): enable byte ranges across artifact downloads
Forward each artifact request to the shared range-aware response helper,
including cached generic release assets and OCI blobs. This lets the
handler honor ranges only when its reader supports efficient seeking,
without changing direct-storage redirect behavior.
Add warm-cache endpoint tests that assert partial response headers and
bytes without an upstream refetch. Document supported range behavior,
unsupported reader fallback, and the integrity limitation of partial reads.
2026-10-04 09:48:41 -07:00
type mockSeekableReadCloser struct {
* bytes . Reader
}
func ( r * mockSeekableReadCloser ) Close ( ) error { return nil }
2026-03-13 17:05:14 +00:00
func ( s * mockStorage ) Exists ( _ context . Context , path string ) ( bool , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
_ , ok := s . files [ path ]
return ok , nil
}
2026-09-03 05:06:44 -04:00
func ( s * mockStorage ) Delete ( ctx context . Context , path string ) error {
// Real backends (S3/GCS SDKs) fail fast on an already-cancelled
// context; mirror that here so tests can catch cleanup calls that
// forgot to detach from a cancelled client context.
if err := ctx . Err ( ) ; err != nil {
return err
}
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
delete ( s . files , path )
return nil
}
func ( s * mockStorage ) Size ( _ context . Context , path string ) ( int64 , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
data , ok := s . files [ path ]
if ! ok {
return 0 , storage . ErrNotFound
}
return int64 ( len ( data ) ) , nil
}
func ( s * mockStorage ) UsedSpace ( _ context . Context ) ( int64 , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
s . mu . Lock ( )
defer s . mu . Unlock ( )
2026-03-13 17:05:14 +00:00
var total int64
for _ , data := range s . files {
total += int64 ( len ( data ) )
}
return total , nil
}
2026-04-27 12:04:38 +01:00
func ( s * mockStorage ) SignedURL ( _ context . Context , _ string , _ time . Duration ) ( string , error ) {
if s . signErr != nil {
return "" , s . signErr
}
if s . signedURL == "" {
return "" , storage . ErrSignedURLUnsupported
}
return s . signedURL , nil
}
2026-04-03 14:06:51 +01:00
func ( s * mockStorage ) URL ( ) string { return "mem://" }
func ( s * mockStorage ) Close ( ) error { return nil }
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
// mockFetcher implements fetch.FetcherInterface for testing. Recording is
// locked because coalescing tests call the handler from many goroutines; tests
// read the recorded fields only after those calls have returned.
2026-03-13 17:05:14 +00:00
type mockFetcher struct {
2026-05-22 18:05:20 +02:00
artifact * fetch . Artifact
fetchErr error
fetchErrByURL map [ string ] error
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
mu sync . Mutex
2026-05-22 18:05:20 +02:00
fetchCalled bool
fetchedURL string
2026-09-02 13:06:34 +01:00
fetchedHeader http . Header
2026-03-13 17:05:14 +00:00
}
2026-04-01 15:22:39 +01:00
func ( f * mockFetcher ) Fetch ( ctx context . Context , url string ) ( * fetch . Artifact , error ) {
return f . FetchWithHeaders ( ctx , url , nil )
}
2026-09-02 13:06:34 +01:00
func ( f * mockFetcher ) FetchWithHeaders ( _ context . Context , url string , headers http . Header ) ( * fetch . Artifact , error ) {
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
f . mu . Lock ( )
2026-03-13 17:05:14 +00:00
f . fetchCalled = true
f . fetchedURL = url
2026-09-02 13:06:34 +01:00
f . fetchedHeader = headers . Clone ( )
Fix duplicate fetches and 502s on concurrent cache misses (#329)
* Return the stored artifact from storeArtifact, not a reader
storeArtifact returned a CacheResult holding an open file handle. A
handle has one read position, so it can only ever serve a single caller,
which is what blocks sharing one fetch between concurrent requests.
Return the artifact and its storage path instead, and let each caller
open its own reader through openStoredArtifact. Threading that type
through fetchAndCache, fetchAndCacheFromURL and their error paths is
mechanical; behaviour is unchanged.
* Coalesce concurrent cache misses
A cache miss went from checkCache straight to an upstream fetch with
nothing tracking in-flight work, so N concurrent requests for one
uncached artifact produced N upstream fetches and N stores to the same
key. That is the CI shape: parallel jobs installing overlapping
dependencies against a cold cache. The duplicate stores also fail
requests, racing fileblob's per-key ".attrs" sidecar into a partial read
served as a 502. Over 12 runs of 8 simultaneous requests for one
uncached tarball, against bb2205a: before, 8 fetches per run and 12 of
96 responses were 502; after, 1 fetch per run and none failed.
Route both miss paths through a shared in-flight map keyed on the
artifact, including the download URL and upstream-declared hash so
callers expecting different bytes never share a fetch.
singleflight does not fit: Do gives waiters no way to leave, while
DoChan lets the caller running the fetch abandon it, breaking
storeArtifact's scan-on-disconnect contract. Deciding roles under a
mutex gives both behaviours. The fetch runs on the first caller's
context and is seen through; waiters leave when their own clients do.
This removes the sidecar trigger on this path. The race is in fileblob
and three writers bypass this path entirely, so it is fixed separately.
Fewer failures now reach the circuit breaker, so it trips later.
Sixteen concurrent callers against real file:// storage fail 10 of 10
runs on main and pass 10 of 10 here. Other tests pin key discrimination,
failure propagation, resolver-path coalescing, per-caller readers,
waiter cancellation, key release and panic safety. allocs/op is
unchanged. mockStorage gains a mutex so concurrent tests can use it.
* Normalize digest case in the coalescing key
artifactHashMatches compares digests with strings.EqualFold, but the
coalescing key used the hash verbatim. The same digest in two casings
produced two keys, so two callers for one artifact each ran their own
upstream fetch and store, which is what the coalescing is meant to
prevent.
* Make the panic coalescing test deterministic
The test timed the second caller's arrival with a sleep, so which caller
became the leader was left to the scheduler. When it lost that race the
second caller ran the fetch itself, and its panic was not recovered, so
the test binary died instead of the test failing.
Whether a caller has reached the wait is not observable from outside:
it runs a cache lookup against the database first, so releasing the
leader on a timer races that query. Drive coalesceFetch directly and
hold the shared entry instead, which removes the timing entirely. The
panicking fetcher is no longer needed.
* Recheck the cache before running a shared fetch
A caller checks the cache before it reaches coalesceFetch, so a fetch
that commits in that gap is invisible to it. Arriving after the sharing
entry is gone, it became a new leader and fetched, stored and scanned an
artifact the cache already held.
The leader now rechecks the committed record first. It serves that
record only if its bytes still open, because a record can outlive them,
and refetching is the recovery the cache lookup already makes for that
case. Waiters are unaffected: the record fills the same shared value a
fetch would, and each caller opens its own reader from it.
The recheck is the leader's alone. A waiter has a fetch in flight to
wait on, and rechecking would race it for no gain.
* Lock the mock fetcher's bookkeeping
Coalescing tests call the handler from many goroutines. The key keeps
the fetch itself serialized, but the mock should not lean on that: it
now locks the fields it records, so any concurrency the handler applies
is safe under the race detector.
* Wait for the leader's fetch instead of sleeping
The canceled-waiter test slept 200ms and assumed the leader had taken
the key by then. On a slow scheduler the canceled call could become the
leader and the test would no longer cover waiter cancellation. The
fetcher now signals when its first fetch begins, which happens only once
the key is held.
2026-09-15 11:59:03 -07:00
f . mu . Unlock ( )
2026-05-22 18:05:20 +02:00
if f . fetchErrByURL != nil {
if err , ok := f . fetchErrByURL [ url ] ; ok {
return nil , err
}
}
2026-03-13 17:05:14 +00:00
if f . fetchErr != nil {
return nil , f . fetchErr
}
return f . artifact , nil
}
func ( f * mockFetcher ) Head ( _ context . Context , _ string ) ( int64 , string , error ) {
return 0 , "" , nil
}
// setupTestProxy creates a Proxy with a real DB (SQLite in temp dir) and mock storage/fetcher.
2026-08-13 08:06:41 +01:00
func setupTestProxy ( t testing . TB ) ( * Proxy , * database . DB , * mockStorage , * mockFetcher ) {
2026-03-13 17:05:14 +00:00
t . Helper ( )
dir := t . TempDir ( )
db , err := database . Create ( dir + "/test.db" )
if err != nil {
t . Fatalf ( "failed to create test database: %v" , err )
}
t . Cleanup ( func ( ) { _ = db . Close ( ) } )
store := newMockStorage ( )
fetcher := & mockFetcher { }
resolver := fetch . NewResolver ( )
logger := slog . New ( slog . NewTextHandler ( io . Discard , nil ) )
proxy := NewProxy ( db , store , fetcher , resolver , logger )
return proxy , db , store , fetcher
}
2026-08-29 11:19:54 +01:00
func histogramSampleCount ( t testing . TB , observer prometheus . Observer ) uint64 {
t . Helper ( )
metric , ok := observer . ( prometheus . Metric )
if ! ok {
t . Fatal ( "observer does not implement prometheus.Metric" )
}
value := & dto . Metric { }
if err := metric . Write ( value ) ; err != nil {
t . Fatalf ( "writing Prometheus metric: %v" , err )
}
return value . GetHistogram ( ) . GetSampleCount ( )
}
2026-09-04 10:58:22 +01:00
func testArtifact ( content , packageURL , filename , mediaType string ) artifacts . Artifact {
return artifacts . Artifact {
PURL : packageURL ,
Digest : digest . Digest ( "sha256:" + sha256Hex ( content ) ) ,
Size : int64 ( len ( content ) ) ,
Filename : filename ,
MediaType : mediaType ,
}
}
2026-03-13 17:05:14 +00:00
// seedPackage creates a package, version, and cached artifact in the test DB and storage.
2026-08-13 08:06:41 +01:00
func seedPackage ( t testing . TB , db * database . DB , store * mockStorage , ecosystem , name , version , filename , content string ) {
2026-03-13 17:05:14 +00:00
t . Helper ( )
pkg := & database . Package {
2026-08-13 07:35:07 +01:00
PURL : purl . MakePURLString ( ecosystem , name , "" ) ,
2026-03-13 17:05:14 +00:00
Ecosystem : ecosystem ,
Name : name ,
}
if err := db . UpsertPackage ( pkg ) ; err != nil {
t . Fatalf ( "failed to upsert package: %v" , err )
}
2026-08-13 07:35:07 +01:00
versionPURL := purl . MakePURLString ( ecosystem , name , version )
2026-03-13 17:05:14 +00:00
ver := & database . Version {
PURL : versionPURL ,
PackagePURL : pkg . PURL ,
}
if err := db . UpsertVersion ( ver ) ; err != nil {
t . Fatalf ( "failed to upsert version: %v" , err )
}
storagePath := storage . ArtifactPath ( ecosystem , "" , name , version , filename )
store . files [ storagePath ] = [ ] byte ( content )
2026-09-04 10:58:22 +01:00
sharedArtifact := testArtifact ( content , versionPURL , filename , "application/octet-stream" )
2026-03-13 17:05:14 +00:00
art := & database . Artifact {
VersionPURL : versionPURL ,
Filename : filename ,
UpstreamURL : "https://example.com/" + filename ,
StoragePath : sql . NullString { String : storagePath , Valid : true } ,
2026-09-04 10:58:22 +01:00
ContentHash : sql . NullString { String : sharedArtifact . Digest . Encoded ( ) , Valid : true } ,
2026-03-13 17:05:14 +00:00
Size : sql . NullInt64 { Int64 : int64 ( len ( content ) ) , Valid : true } ,
ContentType : sql . NullString { String : "application/octet-stream" , Valid : true } ,
FetchedAt : sql . NullTime { Time : time . Now ( ) , Valid : true } ,
}
if err := db . UpsertArtifact ( art ) ; err != nil {
t . Fatalf ( "failed to upsert artifact: %v" , err )
}
}
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
// recordedStoragePath returns the storage path the artifact's record points at.
func recordedStoragePath ( t testing . TB , db * database . DB , versionPURL , filename string ) string {
t . Helper ( )
art , err := db . GetArtifact ( versionPURL , filename )
if err != nil || art == nil || ! art . StoragePath . Valid {
t . Fatalf ( "no storage path recorded for %s %s (err %v)" , versionPURL , filename , err )
}
return art . StoragePath . String
}
2026-03-18 10:59:29 +00:00
// pathParseCase holds a single test case for path parsing functions that return
// (name, version, arch).
type pathParseCase struct {
path string
wantName string
wantVersion string
wantArch string
}
// assertPathParser runs table-driven tests for a path parser function that returns
// three strings (name, version, arch).
func assertPathParser ( t * testing . T , funcName string , parse func ( string ) ( string , string , string ) , cases [ ] pathParseCase ) {
t . Helper ( )
for _ , tt := range cases {
t . Run ( tt . path , func ( t * testing . T ) {
name , version , arch := parse ( tt . path )
if name != tt . wantName {
t . Errorf ( "%s() name = %q, want %q" , funcName , name , tt . wantName )
}
if version != tt . wantVersion {
t . Errorf ( "%s() version = %q, want %q" , funcName , version , tt . wantVersion )
}
if arch != tt . wantArch {
t . Errorf ( "%s() arch = %q, want %q" , funcName , arch , tt . wantArch )
}
} )
}
}
// assertRoutesBasics checks that a handler's Routes() returns a non-nil handler,
// rejects POST requests with 405, and rejects path traversal with 400.
func assertRoutesBasics ( t * testing . T , handler http . Handler , postPath , traversalPath string ) {
t . Helper ( )
if handler == nil {
t . Fatal ( "Routes() returned nil" )
}
req := httptest . NewRequest ( http . MethodPost , postPath , nil )
w := httptest . NewRecorder ( )
handler . ServeHTTP ( w , req )
if w . Code != http . StatusMethodNotAllowed {
t . Errorf ( "POST request: got status %d, want %d" , w . Code , http . StatusMethodNotAllowed )
}
req = httptest . NewRequest ( http . MethodGet , traversalPath , nil )
w = httptest . NewRecorder ( )
handler . ServeHTTP ( w , req )
if w . Code != http . StatusBadRequest {
t . Errorf ( "path traversal: got status %d, want %d" , w . Code , http . StatusBadRequest )
}
}
2026-03-13 17:05:14 +00:00
func TestGetOrFetchArtifact_CacheHit ( t * testing . T ) {
proxy , db , store , fetcher := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if ! result . Cached {
t . Error ( "expected result to be cached" )
}
if fetcher . fetchCalled {
t . Error ( "fetcher should not be called on cache hit" )
}
body , _ := io . ReadAll ( result . Reader )
if string ( body ) != "cached content" {
t . Errorf ( "got body %q, want %q" , body , "cached content" )
}
2026-09-04 10:58:22 +01:00
if result . Artifact . MediaType != "application/octet-stream" {
t . Errorf ( "got content type %q, want %q" , result . Artifact . MediaType , "application/octet-stream" )
2026-03-13 17:05:14 +00:00
}
2026-09-04 10:58:22 +01:00
if result . Artifact . Digest . Encoded ( ) != sha256Hex ( "cached content" ) {
t . Errorf ( "got digest %q, want %q" , result . Artifact . Digest . Encoded ( ) , sha256Hex ( "cached content" ) )
2026-08-17 09:20:11 +01:00
}
}
func TestGetCachedArtifactRejectsMalformedIntegrityMetadata ( t * testing . T ) {
tests := [ ] struct {
name string
malformedHash string
malformedIntegrity string
} {
{ name : "content hash" , malformedHash : "abc123" } ,
{ name : "native integrity" , malformedIntegrity : "sha512-abc123" } ,
}
for _ , test := range tests {
t . Run ( test . name , func ( t * testing . T ) {
assertMalformedCacheRejected ( t , test . malformedHash , test . malformedIntegrity )
} )
}
}
func assertMalformedCacheRejected ( t * testing . T , malformedHash , malformedIntegrity string ) {
t . Helper ( )
proxy , db , store , _ := setupTestProxy ( t )
const (
packageName = "broken"
version = "1.0.0"
filename = "broken-1.0.0.tgz"
)
seedPackage ( t , db , store , "npm" , packageName , version , filename , "cached content" )
versionPURL := purl . MakePURLString ( "npm" , packageName , version )
if malformedHash != "" {
artifact , err := db . GetArtifact ( versionPURL , filename )
if err != nil {
t . Fatal ( err )
}
artifact . ContentHash = sql . NullString { String : malformedHash , Valid : true }
if err := db . UpsertArtifact ( artifact ) ; err != nil {
t . Fatal ( err )
}
}
if malformedIntegrity != "" {
versionRecord := & database . Version {
PURL : versionPURL ,
PackagePURL : purl . MakePURLString ( "npm" , packageName , "" ) ,
Integrity : sql . NullString { String : malformedIntegrity , Valid : true } ,
}
if err := db . UpsertVersion ( versionRecord ) ; err != nil {
t . Fatal ( err )
}
}
proxy . DirectServe = true
store . signedURL = "https://cache.example/broken"
result , err := proxy . GetCachedArtifact ( context . Background ( ) , "npm" , packageName , version , filename )
if err != nil {
t . Fatalf ( "GetCachedArtifact: %v" , err )
}
if result != nil {
t . Errorf ( "GetCachedArtifact = %+v, want nil" , result )
}
artifact , err := db . GetArtifact ( versionPURL , filename )
if err != nil {
t . Fatal ( err )
}
if artifact . StoragePath . Valid {
t . Error ( "unusable cache record retained its storage path" )
2026-03-13 17:05:14 +00:00
}
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
assertQueuedForDeletion ( t , db , storage . ArtifactPath ( "npm" , "" , packageName , version , filename ) )
}
// TestCorruptCachedArtifactIsQueuedForDeletion streams cached bytes that no
// longer match their recorded digest, which clears the record once the stream
// ends. With each fetch writing its own path, no later fetch overwrites them,
// so they must be queued for deletion.
func TestCorruptCachedArtifactIsQueuedForDeletion ( t * testing . T ) {
proxy , db , store , _ := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "corrupt" , "1.0.0" , "corrupt-1.0.0.tgz" , "cached content" )
storagePath := storage . ArtifactPath ( "npm" , "" , "corrupt" , "1.0.0" , "corrupt-1.0.0.tgz" )
store . files [ storagePath ] = [ ] byte ( "tampered bytes" )
result , err := proxy . GetCachedArtifact ( context . Background ( ) , "npm" , "corrupt" , "1.0.0" , "corrupt-1.0.0.tgz" )
if err != nil || result == nil {
t . Fatalf ( "GetCachedArtifact = %v, %v" , result , err )
}
drain ( result )
artifact , err := db . GetArtifact ( "pkg:npm/corrupt@1.0.0" , "corrupt-1.0.0.tgz" )
if err != nil || artifact == nil || artifact . StoragePath . Valid {
t . Errorf ( "record = %+v (err %v), want it cleared" , artifact , err )
}
assertQueuedForDeletion ( t , db , storagePath )
}
// assertQueuedForDeletion fails unless path waits in the pending delete queue.
func assertQueuedForDeletion ( t * testing . T , db * database . DB , path string ) {
t . Helper ( )
queued , err := db . GetDuePendingDeletes ( time . Now ( ) . Add ( time . Hour ) , 100 )
if err != nil || ! slices . Contains ( queued , path ) {
t . Errorf ( "queued %v (err %v), want %q queued for deletion" , queued , err , path )
}
2026-03-13 17:05:14 +00:00
}
func TestGetOrFetchArtifact_CacheMiss_NoPackage ( t * testing . T ) {
proxy , _ , _ , fetcher := setupTestProxy ( t )
2026-08-16 18:01:59 +01:00
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "npm" ) )
2026-03-13 17:05:14 +00:00
// The resolver will fail because "nonexistent" isn't a real package,
// but we're testing that it tries to fetch (doesn't return from cache).
fetcher . fetchErr = errors . New ( "upstream unavailable" )
_ , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "nonexistent" , "1.0.0" , "nonexistent-1.0.0.tgz" )
if err == nil {
t . Fatal ( "expected error for uncached package" )
}
2026-08-16 18:01:59 +01:00
missesAfter := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "npm" ) )
if diff := missesAfter - missesBefore ; diff != 1 {
t . Errorf ( "cache misses delta = %.0f, want 1" , diff )
}
2026-03-13 17:05:14 +00:00
}
func TestGetOrFetchArtifactFromURL_CacheMiss_StorageMissing ( t * testing . T ) {
proxy , db , store , fetcher := setupTestProxy ( t )
// Seed DB but don't put the file in storage
pkg := & database . Package { PURL : "pkg:npm/missing" , Ecosystem : "npm" , Name : "missing" }
_ = db . UpsertPackage ( pkg )
ver := & database . Version { PURL : "pkg:npm/missing@1.0.0" , PackagePURL : pkg . PURL }
_ = db . UpsertVersion ( ver )
art := & database . Artifact {
VersionPURL : ver . PURL ,
Filename : "missing-1.0.0.tgz" ,
UpstreamURL : "https://example.com/missing.tgz" ,
StoragePath : sql . NullString { String : "nonexistent/path.tgz" , Valid : true } ,
2026-08-17 09:20:11 +01:00
ContentHash : sql . NullString { String : sha256Hex ( "missing content" ) , Valid : true } ,
2026-03-13 17:05:14 +00:00
Size : sql . NullInt64 { Int64 : 100 , Valid : true } ,
ContentType : sql . NullString { String : "application/octet-stream" , Valid : true } ,
FetchedAt : sql . NullTime { Time : time . Now ( ) , Valid : true } ,
}
_ = db . UpsertArtifact ( art )
// Storage doesn't have the file, so checkCache should return nil and trigger a refetch.
fetcher . artifact = & fetch . Artifact {
Body : io . NopCloser ( strings . NewReader ( "refetched content" ) ) ,
ContentType : "application/gzip" ,
}
result , err := proxy . GetOrFetchArtifactFromURL ( context . Background ( ) , "npm" , "missing" , "1.0.0" , "missing-1.0.0.tgz" , "https://example.com/missing.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if result . Cached {
t . Error ( "expected cache miss (storage file was missing)" )
}
if ! fetcher . fetchCalled {
t . Error ( "fetcher should be called when storage file is missing" )
}
// Verify the new content was stored
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
storagePath := recordedStoragePath ( t , db , "pkg:npm/missing@1.0.0" , "missing-1.0.0.tgz" )
2026-03-13 17:05:14 +00:00
if _ , ok := store . files [ storagePath ] ; ! ok {
t . Error ( "refetched artifact should be stored" )
}
}
2026-09-02 13:06:34 +01:00
func TestArtifactCacheRejectsUnsupportedPackageIdentity ( t * testing . T ) {
proxy , _ , _ , fetcher := setupTestProxy ( t )
_ , err := proxy . GetCachedArtifact (
context . Background ( ) , "swift" , "apple/example" , "1.2.3" , "example-1.2.3.zip" ,
)
if ! errors . Is ( err , errUnsupportedPackageIdentity ) {
t . Fatalf ( "GetCachedArtifact() error = %v, want unsupported package identity" , err )
}
_ , err = proxy . GetOrFetchArtifactFromURL (
context . Background ( ) , "swift" , "apple/example" , "1.2.3" , "example-1.2.3.zip" ,
"https://registry.example/apple/example/1.2.3.zip" ,
)
if ! errors . Is ( err , errUnsupportedPackageIdentity ) {
t . Fatalf ( "GetOrFetchArtifactFromURL() error = %v, want unsupported package identity" , err )
}
if fetcher . fetchCalled {
t . Error ( "unsupported package identity reached the artifact fetcher" )
}
}
2026-04-27 12:04:38 +01:00
func TestGetOrFetchArtifact_DirectServe_Redirect ( t * testing . T ) {
proxy , db , store , fetcher := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
proxy . DirectServe = true
proxy . DirectServeTTL = 15 * time . Minute
store . signedURL = "https://bucket.s3.amazonaws.com/npm/lodash?X-Amz-Signature=abc"
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
if ! result . Cached {
t . Error ( "expected result to be cached" )
}
if result . RedirectURL != store . signedURL {
t . Errorf ( "RedirectURL = %q, want %q" , result . RedirectURL , store . signedURL )
}
if result . Reader != nil {
t . Error ( "Reader should be nil when redirecting" )
}
if fetcher . fetchCalled {
t . Error ( "fetcher should not be called on cache hit" )
}
// Hit count should still be recorded on the redirect path.
art , _ := db . GetArtifact ( "pkg:npm/lodash@4.17.21" , "lodash-4.17.21.tgz" )
if art == nil || art . HitCount != 1 {
t . Errorf ( "artifact hit count not recorded: %+v" , art )
}
}
2026-04-27 12:14:37 +01:00
func TestGetOrFetchArtifact_DirectServe_BaseURLRewrite ( t * testing . T ) {
proxy , db , store , _ := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
proxy . DirectServe = true
proxy . DirectServeBaseURL = "https://cdn.example.com"
store . signedURL = "http://127.0.0.1:9000/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900"
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
want := "https://cdn.example.com/bucket/npm/lodash?X-Amz-Signature=abc&X-Amz-Expires=900"
if result . RedirectURL != want {
t . Errorf ( "RedirectURL = %q, want %q" , result . RedirectURL , want )
}
}
func TestRewriteSignedURLHost ( t * testing . T ) {
tests := [ ] struct {
name string
signed string
baseURL string
want string
} {
{
"empty base url is no-op" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
"" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
} ,
{
"replaces scheme and host" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
"https://cdn.example.com" ,
"https://cdn.example.com/bucket/key?sig=abc" ,
} ,
{
"preserves path and query" ,
"http://minio:9000/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101" ,
"https://files.example.com" ,
"https://files.example.com/bucket/npm/lodash/4.17.21/lodash.tgz?X-Amz-Signature=abc&X-Amz-Date=20260101" ,
} ,
{
"ignores base url path" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
"https://cdn.example.com/ignored" ,
"https://cdn.example.com/bucket/key?sig=abc" ,
} ,
{
"invalid base url is no-op" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
"://bad" ,
"http://127.0.0.1:9000/bucket/key?sig=abc" ,
} ,
}
for _ , tt := range tests {
t . Run ( tt . name , func ( t * testing . T ) {
got := rewriteSignedURLHost ( tt . signed , tt . baseURL )
if got != tt . want {
t . Errorf ( "rewriteSignedURLHost(%q, %q) = %q, want %q" , tt . signed , tt . baseURL , got , tt . want )
}
} )
}
}
2026-04-27 12:04:38 +01:00
func TestGetOrFetchArtifact_DirectServe_FallbackOnUnsupported ( t * testing . T ) {
proxy , db , store , _ := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
proxy . DirectServe = true
// store.signedURL is empty so SignedURL returns ErrSignedURLUnsupported.
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if result . RedirectURL != "" {
t . Errorf ( "RedirectURL should be empty, got %q" , result . RedirectURL )
}
if result . Reader == nil {
t . Fatal ( "Reader should be set when signing is unsupported" )
}
body , _ := io . ReadAll ( result . Reader )
if string ( body ) != "cached content" {
t . Errorf ( "got body %q, want %q" , body , "cached content" )
}
}
func TestGetOrFetchArtifact_DirectServe_FallbackOnError ( t * testing . T ) {
proxy , db , store , _ := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
proxy . DirectServe = true
store . signErr = errors . New ( "signing failed" )
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if result . RedirectURL != "" {
t . Errorf ( "RedirectURL should be empty on signing error, got %q" , result . RedirectURL )
}
if result . Reader == nil {
t . Fatal ( "Reader should be set when signing fails" )
}
}
func TestGetOrFetchArtifact_DirectServe_DisabledIgnoresSigning ( t * testing . T ) {
proxy , db , store , _ := setupTestProxy ( t )
seedPackage ( t , db , store , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" , "cached content" )
proxy . DirectServe = false
store . signedURL = "https://bucket.example/should-not-be-used"
result , err := proxy . GetOrFetchArtifact ( context . Background ( ) , "npm" , "lodash" , "4.17.21" , "lodash-4.17.21.tgz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if result . RedirectURL != "" {
t . Errorf ( "RedirectURL should be empty when DirectServe is off, got %q" , result . RedirectURL )
}
}
func TestServeArtifact_Redirect ( t * testing . T ) {
w := httptest . NewRecorder ( )
ServeArtifact ( w , & CacheResult {
RedirectURL : "https://bucket.s3.amazonaws.com/file?sig=abc" ,
2026-09-04 10:58:22 +01:00
Artifact : artifacts . Artifact {
Digest : digest . Digest ( "sha256:" + strings . Repeat ( "a" , sha256 . Size * 2 ) ) ,
} ,
Cached : true ,
2026-04-27 12:04:38 +01:00
} )
if w . Code != http . StatusFound {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusFound )
}
if loc := w . Header ( ) . Get ( "Location" ) ; loc != "https://bucket.s3.amazonaws.com/file?sig=abc" {
t . Errorf ( "Location = %q" , loc )
}
2026-09-04 10:58:22 +01:00
if etag := w . Header ( ) . Get ( "ETag" ) ; etag != ` " ` + strings . Repeat ( "a" , sha256 . Size * 2 ) + ` " ` {
t . Errorf ( "ETag = %q" , etag )
2026-04-27 12:04:38 +01:00
}
if cl := w . Header ( ) . Get ( "Content-Length" ) ; cl != "" {
t . Errorf ( "Content-Length should not be set on redirect, got %q" , cl )
}
}
func TestServeArtifact_Stream ( t * testing . T ) {
w := httptest . NewRecorder ( )
ServeArtifact ( w , & CacheResult {
2026-09-04 10:58:22 +01:00
Reader : io . NopCloser ( strings . NewReader ( "payload" ) ) ,
Artifact : testArtifact (
"payload" ,
"pkg:npm/example@1.0.0" ,
"example.tgz" ,
"application/octet-stream" ,
) ,
2026-04-27 12:04:38 +01:00
} )
if w . Code != http . StatusOK {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusOK )
}
if w . Body . String ( ) != "payload" {
t . Errorf ( "body = %q, want %q" , w . Body . String ( ) , "payload" )
}
if ct := w . Header ( ) . Get ( "Content-Type" ) ; ct != "application/octet-stream" {
t . Errorf ( "Content-Type = %q" , ct )
}
}
Support HTTP byte-range requests for local file cached artifact downloads (#404)
* feat(storage): expose efficient seek for local files
Open file-backed artifacts with os.Open so local readers support
efficient seeking. Leave cloud storage readers unchanged.
Keep the legacy sidecar mapping test focused on fileblob's reader:
Blob.Open now reads local files directly and no longer consults the
sidecar. Continue checking sidecar cleanup and post-store reads.
* feat(handler): preserve efficient seek through integrity checks
Keep the cache result's single Reader field and preserve io.Seeker
when the underlying storage reader supports efficient seeking.
Sequential reads continue through whole-object integrity verification.
A successful seek switches subsequent reads to the source for range
responses, where full-object verification is not possible.
Test both modes and verify the source is closed once.
* feat(handler): add request-aware byte range responses
Add a request-aware artifact serving helper for bounded, open-ended,
and suffix byte ranges, including If-Range handling, 206 responses,
and 416 responses for valid unsatisfiable ranges.
Ignore malformed and multi-range requests by serving the full artifact.
Advertise byte-range support only for seekable readers, preserving HEAD,
redirect, and non-seekable behavior. Clarify that net/http recovers
ErrAbortHandler per request and keeps the server running.
* feat(handler): enable byte ranges across artifact downloads
Forward each artifact request to the shared range-aware response helper,
including cached generic release assets and OCI blobs. This lets the
handler honor ranges only when its reader supports efficient seeking,
without changing direct-storage redirect behavior.
Add warm-cache endpoint tests that assert partial response headers and
bytes without an upstream refetch. Document supported range behavior,
unsupported reader fallback, and the integrity limitation of partial reads.
2026-10-04 09:48:41 -07:00
func TestServeArtifactRequestRanges ( t * testing . T ) {
const payload = "hello world"
tests := [ ] struct {
name string
rangeHeader string
secondRange string
wantStatus int
wantRange string
wantLength string
wantBody string
} {
{ name : "bounded" , rangeHeader : "bytes=1-3" , wantStatus : http . StatusPartialContent , wantRange : "bytes 1-3/11" , wantLength : "3" , wantBody : "ell" } ,
{ name : "open ended" , rangeHeader : "bytes=6-" , wantStatus : http . StatusPartialContent , wantRange : "bytes 6-10/11" , wantLength : "5" , wantBody : "world" } ,
{ name : "suffix" , rangeHeader : "bytes=-4" , wantStatus : http . StatusPartialContent , wantRange : "bytes 7-10/11" , wantLength : "4" , wantBody : "orld" } ,
{ name : "suffix larger than artifact" , rangeHeader : "bytes=-99" , wantStatus : http . StatusPartialContent , wantRange : "bytes 0-10/11" , wantLength : "11" , wantBody : payload } ,
{ name : "unsatisfiable" , rangeHeader : "bytes=11-" , wantStatus : http . StatusRequestedRangeNotSatisfiable , wantRange : "bytes */11" , wantLength : "0" } ,
{ name : "malformed" , rangeHeader : "bytes=invalid" , wantStatus : http . StatusOK , wantLength : "11" , wantBody : payload } ,
{ name : "reversed" , rangeHeader : "bytes=4-2" , wantStatus : http . StatusOK , wantLength : "11" , wantBody : payload } ,
{ name : "multiple ranges" , rangeHeader : "bytes=0-1,4-5" , wantStatus : http . StatusOK , wantLength : "11" , wantBody : payload } ,
{ name : "multiple range fields" , rangeHeader : "bytes=0-1" , secondRange : "bytes=4-5" , wantStatus : http . StatusOK , wantLength : "11" , wantBody : payload } ,
{ name : "no range" , wantStatus : http . StatusOK , wantLength : "11" , wantBody : payload } ,
}
for _ , test := range tests {
t . Run ( test . name , func ( t * testing . T ) {
request := httptest . NewRequest ( http . MethodGet , "/artifact" , nil )
if test . rangeHeader != "" {
request . Header . Set ( "Range" , test . rangeHeader )
}
if test . secondRange != "" {
request . Header . Add ( "Range" , test . secondRange )
}
w := httptest . NewRecorder ( )
ServeArtifactRequest ( w , request , newRangeTestResult ( payload ) )
if w . Code != test . wantStatus {
t . Errorf ( "status = %d, want %d" , w . Code , test . wantStatus )
}
if got := w . Header ( ) . Get ( "Content-Range" ) ; got != test . wantRange {
t . Errorf ( "Content-Range = %q, want %q" , got , test . wantRange )
}
if got := w . Header ( ) . Get ( headerContentLength ) ; got != test . wantLength {
t . Errorf ( "Content-Length = %q, want %q" , got , test . wantLength )
}
if got := w . Body . String ( ) ; got != test . wantBody {
t . Errorf ( "body = %q, want %q" , got , test . wantBody )
}
if got := w . Header ( ) . Get ( "Accept-Ranges" ) ; got != "bytes" {
t . Errorf ( "Accept-Ranges = %q, want bytes" , got )
}
} )
}
}
func TestServeArtifactRequestIfRange ( t * testing . T ) {
const payload = "hello world"
tests := [ ] struct {
name string
ifRange string
wantStatus int
wantRange string
wantBody string
} {
{ name : "matching etag" , ifRange : ` "sha256-matching" ` , wantStatus : http . StatusPartialContent , wantRange : "bytes 0-4/11" , wantBody : "hello" } ,
{ name : "mismatching etag" , ifRange : ` "sha256-stale" ` , wantStatus : http . StatusOK , wantBody : payload } ,
{ name : "weak etag" , ifRange : ` W/"sha256-matching" ` , wantStatus : http . StatusOK , wantBody : payload } ,
}
for _ , test := range tests {
t . Run ( test . name , func ( t * testing . T ) {
result := newRangeTestResult ( payload )
request := httptest . NewRequest ( http . MethodGet , "/artifact" , nil )
request . Header . Set ( "Range" , "bytes=0-4" )
if test . name == "matching etag" {
test . ifRange = ` " ` + result . Artifact . Digest . Encoded ( ) + ` " `
}
request . Header . Set ( "If-Range" , test . ifRange )
w := httptest . NewRecorder ( )
ServeArtifactRequest ( w , request , result )
if w . Code != test . wantStatus {
t . Errorf ( "status = %d, want %d" , w . Code , test . wantStatus )
}
if got := w . Header ( ) . Get ( "Content-Range" ) ; got != test . wantRange {
t . Errorf ( "Content-Range = %q, want %q" , got , test . wantRange )
}
if got := w . Body . String ( ) ; got != test . wantBody {
t . Errorf ( "body = %q, want %q" , got , test . wantBody )
}
} )
}
}
func TestServeArtifactRequestHeadIgnoresRange ( t * testing . T ) {
request := httptest . NewRequest ( http . MethodHead , "/artifact" , nil )
request . Header . Set ( "Range" , "bytes=0-1" )
w := httptest . NewRecorder ( )
ServeArtifactRequest ( w , request , newRangeTestResult ( "hello world" ) )
if w . Code != http . StatusOK {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusOK )
}
if got := w . Header ( ) . Get ( headerContentLength ) ; got != "11" {
t . Errorf ( "Content-Length = %q, want 11" , got )
}
if got := w . Header ( ) . Get ( "Content-Range" ) ; got != "" {
t . Errorf ( "Content-Range = %q, want empty" , got )
}
if w . Body . Len ( ) != 0 {
t . Errorf ( "HEAD body = %q, want empty" , w . Body . String ( ) )
}
}
func TestServeArtifactRequestWithoutSeekCapability ( t * testing . T ) {
request := httptest . NewRequest ( http . MethodGet , "/artifact" , nil )
request . Header . Set ( "Range" , "bytes=1-2" )
w := httptest . NewRecorder ( )
ServeArtifactRequest ( w , request , & CacheResult {
Reader : io . NopCloser ( strings . NewReader ( "payload" ) ) ,
Artifact : testArtifact ( "payload" , "pkg:npm/example@1.0.0" , "example.tgz" , "application/gzip" ) ,
} )
if w . Code != http . StatusOK || w . Body . String ( ) != "payload" {
t . Errorf ( "response = %d %q, want 200 with full payload" , w . Code , w . Body . String ( ) )
}
if got := w . Header ( ) . Get ( "Accept-Ranges" ) ; got != "" {
t . Errorf ( "Accept-Ranges = %q, want empty" , got )
}
}
func TestServeArtifactRequestRedirectDoesNotAdvertiseRanges ( t * testing . T ) {
request := httptest . NewRequest ( http . MethodGet , "/artifact" , nil )
request . Header . Set ( "Range" , "bytes=1-2" )
w := httptest . NewRecorder ( )
ServeArtifactRequest ( w , request , & CacheResult { RedirectURL : "https://storage.example/artifact" } )
if w . Code != http . StatusFound {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusFound )
}
if got := w . Header ( ) . Get ( "Accept-Ranges" ) ; got != "" {
t . Errorf ( "Accept-Ranges = %q, want empty" , got )
}
}
type rangeTestReadSeeker struct {
* bytes . Reader
}
func ( r * rangeTestReadSeeker ) Close ( ) error { return nil }
func newRangeTestResult ( payload string ) * CacheResult {
return & CacheResult {
Reader : & rangeTestReadSeeker { Reader : bytes . NewReader ( [ ] byte ( payload ) ) } ,
Artifact : testArtifact ( payload , "pkg:npm/example@1.0.0" , "example.tgz" , "application/gzip" ) ,
}
}
2026-03-13 17:05:14 +00:00
func TestGetOrFetchArtifactFromURL_CacheHit ( t * testing . T ) {
proxy , db , store , fetcher := setupTestProxy ( t )
seedPackage ( t , db , store , "pypi" , "requests" , "2.28.0" , "requests-2.28.0.tar.gz" , "pypi content" )
2026-08-16 18:01:59 +01:00
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "pypi" ) )
2026-03-13 17:05:14 +00:00
result , err := proxy . GetOrFetchArtifactFromURL ( context . Background ( ) , "pypi" , "requests" , "2.28.0" , "requests-2.28.0.tar.gz" , "https://pypi.org/files/requests-2.28.0.tar.gz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if ! result . Cached {
t . Error ( "expected cache hit" )
}
if fetcher . fetchCalled {
t . Error ( "fetcher should not be called on cache hit" )
}
2026-08-16 18:01:59 +01:00
missesAfter := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "pypi" ) )
if diff := missesAfter - missesBefore ; diff != 0 {
t . Errorf ( "cache misses delta = %.0f, want 0" , diff )
}
2026-03-13 17:05:14 +00:00
}
func TestGetOrFetchArtifactFromURL_CacheMiss ( t * testing . T ) {
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
proxy , db , store , fetcher := setupTestProxy ( t )
2026-08-16 18:01:59 +01:00
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "pypi" ) )
2026-08-29 11:19:54 +01:00
fetchesBefore := histogramSampleCount ( t , metrics . UpstreamFetchDuration . WithLabelValues ( "pypi" ) )
writesBefore := histogramSampleCount ( t , metrics . StorageOperationDuration . WithLabelValues ( "write" ) )
readsBefore := histogramSampleCount ( t , metrics . StorageOperationDuration . WithLabelValues ( "read" ) )
2026-03-13 17:05:14 +00:00
fetcher . artifact = & fetch . Artifact {
Body : io . NopCloser ( strings . NewReader ( "fetched content" ) ) ,
ContentType : "application/gzip" ,
}
result , err := proxy . GetOrFetchArtifactFromURL ( context . Background ( ) , "pypi" , "newpkg" , "1.0.0" , "newpkg-1.0.0.tar.gz" , "https://pypi.org/files/newpkg-1.0.0.tar.gz" )
if err != nil {
t . Fatalf ( "unexpected error: %v" , err )
}
defer func ( ) { _ = result . Reader . Close ( ) } ( )
if result . Cached {
t . Error ( "expected cache miss" )
}
if ! fetcher . fetchCalled {
t . Error ( "fetcher should be called on cache miss" )
}
if fetcher . fetchedURL != "https://pypi.org/files/newpkg-1.0.0.tar.gz" {
t . Errorf ( "fetcher called with wrong URL: %s" , fetcher . fetchedURL )
}
body , _ := io . ReadAll ( result . Reader )
if string ( body ) != "fetched content" {
t . Errorf ( "got body %q, want %q" , body , "fetched content" )
}
2026-09-04 10:58:22 +01:00
if err := result . Artifact . Validate ( ) ; err != nil {
t . Errorf ( "Artifact.Validate() error = %v" , err )
}
if result . Artifact . PURL != "pkg:pypi/newpkg@1.0.0" {
t . Errorf ( "PURL = %q" , result . Artifact . PURL )
}
if result . Artifact . Size != int64 ( len ( "fetched content" ) ) {
t . Errorf ( "Size = %d" , result . Artifact . Size )
}
if result . Artifact . MediaType != "application/gzip" {
t . Errorf ( "MediaType = %q" , result . Artifact . MediaType )
}
2026-03-13 17:05:14 +00:00
// Verify it was stored
Give each fetch its own storage path
Every fetch of an artifact wrote one path, so fetches with different URLs
or digests, which do not share a coalescing key, could overwrite each
other's bytes, serve the wrong ones, or delete them on a digest mismatch
while the other was about to open them.
storeArtifact now writes {ecosystem}/{name}/{version}/{fetch id}/{filename},
with a random id per fetch. Most ecosystems learn the digest only after
the fetch, and storage has no rename, so the id is the one rule that fits
all of them. Existing records keep their paths and stay readable. On a
file:// bucket, Delete now removes an emptied fetch directory, which
fileblob leaves behind, and no other directory, since another fetch may
be creating its own inside it.
A path a record stops pointing at is queued in pending_deletes rather
than deleted, since a request that read the record may still open it. A
loop deletes queued paths after max(1h, direct_serve_ttl), whether or not
max_size is set. UpsertArtifact and clears apply only if the record still
holds the path the caller read, so racing commits each queue the path
they replaced and a clear never orphans a newer commit. Eviction still
deletes inline, and counts space as freed only when the record still
pointed at what it deleted.
2026-09-23 14:40:13 -07:00
storagePath := recordedStoragePath ( t , db , "pkg:pypi/newpkg@1.0.0" , "newpkg-1.0.0.tar.gz" )
2026-03-13 17:05:14 +00:00
if _ , ok := store . files [ storagePath ] ; ! ok {
t . Error ( "artifact was not stored in storage" )
}
2026-08-16 18:01:59 +01:00
missesAfter := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "pypi" ) )
if diff := missesAfter - missesBefore ; diff != 1 {
t . Errorf ( "cache misses delta = %.0f, want 1" , diff )
}
2026-08-29 11:19:54 +01:00
if diff := histogramSampleCount ( t , metrics . UpstreamFetchDuration . WithLabelValues ( "pypi" ) ) - fetchesBefore ; diff != 1 {
t . Errorf ( "upstream fetch observations delta = %d, want 1" , diff )
}
if diff := histogramSampleCount ( t , metrics . StorageOperationDuration . WithLabelValues ( "write" ) ) - writesBefore ; diff != 1 {
t . Errorf ( "storage write observations delta = %d, want 1" , diff )
}
if diff := histogramSampleCount ( t , metrics . StorageOperationDuration . WithLabelValues ( "read" ) ) - readsBefore ; diff != 1 {
t . Errorf ( "storage read observations delta = %d, want 1" , diff )
}
2026-03-13 17:05:14 +00:00
}
2026-10-12 18:44:12 +02:00
// isolateRecentFailures empties the process-wide failure history for the
// test and again when it ends, so neither earlier tests nor repeated runs
// reach its assertions.
func isolateRecentFailures ( t * testing . T ) {
t . Helper ( )
metrics . ResetRecentFailures ( )
t . Cleanup ( metrics . ResetRecentFailures )
}
2026-03-13 17:05:14 +00:00
func TestGetOrFetchArtifactFromURL_FetchError ( t * testing . T ) {
2026-10-12 18:44:12 +02:00
isolateRecentFailures ( t )
2026-03-13 17:05:14 +00:00
proxy , _ , _ , fetcher := setupTestProxy ( t )
fetcher . fetchErr = errors . New ( "connection refused" )
2026-08-29 11:19:54 +01:00
errorsBefore := testutil . ToFloat64 ( metrics . UpstreamErrors . WithLabelValues ( "pypi" , "fetch_failed" ) )
2026-03-13 17:05:14 +00:00
_ , err := proxy . GetOrFetchArtifactFromURL ( context . Background ( ) , "pypi" , "fail" , "1.0.0" , "fail-1.0.0.tar.gz" , "https://pypi.org/files/fail-1.0.0.tar.gz" )
if err == nil {
t . Fatal ( "expected error on fetch failure" )
}
if ! strings . Contains ( err . Error ( ) , "fetching from upstream" ) {
t . Errorf ( "expected upstream error, got: %v" , err )
}
2026-08-29 11:19:54 +01:00
if diff := testutil . ToFloat64 ( metrics . UpstreamErrors . WithLabelValues ( "pypi" , "fetch_failed" ) ) - errorsBefore ; diff != 1 {
t . Errorf ( "upstream errors delta = %.0f, want 1" , diff )
}
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
recent := metrics . RecentFailures ( "proxy_upstream_errors_total" , map [ string ] string { "ecosystem" : "pypi" , "error_type" : "fetch_failed" } )
2026-10-12 18:44:12 +02:00
if len ( recent ) != 1 || recent [ 0 ] . PURL != "pkg:pypi/fail@1.0.0" ||
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
recent [ 0 ] . URL != "https://pypi.org/files/fail-1.0.0.tar.gz" || recent [ 0 ] . Error != "connection refused" {
2026-10-12 18:44:12 +02:00
t . Errorf ( "upstream failures = %+v, want one: the failed package, its URL and the error" , recent )
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
}
2026-03-13 17:05:14 +00:00
}
func TestGetOrFetchArtifactFromURL_StoreError ( t * testing . T ) {
2026-10-12 18:44:12 +02:00
isolateRecentFailures ( t )
2026-03-13 17:05:14 +00:00
proxy , _ , store , fetcher := setupTestProxy ( t )
store . storeErr = errors . New ( "disk full" )
2026-08-29 11:19:54 +01:00
errorsBefore := testutil . ToFloat64 ( metrics . StorageErrors . WithLabelValues ( "write" ) )
2026-03-13 17:05:14 +00:00
fetcher . artifact = & fetch . Artifact {
Body : io . NopCloser ( strings . NewReader ( "data" ) ) ,
ContentType : "application/gzip" ,
}
_ , err := proxy . GetOrFetchArtifactFromURL ( context . Background ( ) , "pypi" , "fail" , "1.0.0" , "fail-1.0.0.tar.gz" , "https://pypi.org/files/fail.tar.gz" )
if err == nil {
t . Fatal ( "expected error on store failure" )
}
if ! strings . Contains ( err . Error ( ) , "storing artifact" ) {
t . Errorf ( "expected storage error, got: %v" , err )
}
2026-08-29 11:19:54 +01:00
if diff := testutil . ToFloat64 ( metrics . StorageErrors . WithLabelValues ( "write" ) ) - errorsBefore ; diff != 1 {
t . Errorf ( "storage errors delta = %.0f, want 1" , diff )
}
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
recent := metrics . RecentFailures ( "proxy_storage_errors_total" , map [ string ] string { "operation" : "write" } )
2026-10-12 18:44:12 +02:00
if len ( recent ) != 1 || recent [ 0 ] . PURL != "pkg:pypi/fail@1.0.0" || recent [ 0 ] . Filename != "fail-1.0.0.tar.gz" ||
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
recent [ 0 ] . Path == "" || recent [ 0 ] . Error != "disk full" {
2026-10-12 18:44:12 +02:00
t . Errorf ( "storage failures = %+v, want one: the failed package, its storage path and the error" , recent )
Record the detail behind each failure counter
- Failure recorders take a FailureDetail: PURL, filename, URL, storage path, error
- Keep the last 20 failures per counter series in memory, reset on restart
- Strip userinfo, query and fragment from URLs, including those inside error text
- Pass the detail from upstream, storage, integrity, health probe and scan call sites
- Package names and URLs stay out of Prometheus labels
2026-10-12 09:41:42 +02:00
}
2026-03-13 17:05:14 +00:00
}
func TestServeArtifact ( t * testing . T ) {
result := & CacheResult {
2026-09-04 10:58:22 +01:00
Reader : io . NopCloser ( strings . NewReader ( "file contents" ) ) ,
Artifact : testArtifact ( "file contents" , "pkg:npm/example@1.0.0" , "example.tgz" , "application/gzip" ) ,
Cached : true ,
2026-03-13 17:05:14 +00:00
}
w := httptest . NewRecorder ( )
ServeArtifact ( w , result )
if w . Code != http . StatusOK {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusOK )
}
if w . Header ( ) . Get ( "Content-Type" ) != "application/gzip" {
t . Errorf ( "Content-Type = %q, want %q" , w . Header ( ) . Get ( "Content-Type" ) , "application/gzip" )
}
if w . Header ( ) . Get ( "Content-Length" ) != "13" {
t . Errorf ( "Content-Length = %q, want %q" , w . Header ( ) . Get ( "Content-Length" ) , "13" )
}
2026-09-04 10:58:22 +01:00
wantETag := ` " ` + result . Artifact . Digest . Encoded ( ) + ` " `
if w . Header ( ) . Get ( "ETag" ) != wantETag {
t . Errorf ( "ETag = %q, want %q" , w . Header ( ) . Get ( "ETag" ) , wantETag )
2026-03-13 17:05:14 +00:00
}
if w . Body . String ( ) != "file contents" {
t . Errorf ( "body = %q, want %q" , w . Body . String ( ) , "file contents" )
}
}
func TestServeArtifact_EmptyFields ( t * testing . T ) {
result := & CacheResult {
Reader : io . NopCloser ( strings . NewReader ( "data" ) ) ,
}
w := httptest . NewRecorder ( )
ServeArtifact ( w , result )
if w . Code != http . StatusOK {
t . Errorf ( "status = %d, want %d" , w . Code , http . StatusOK )
}
if w . Header ( ) . Get ( "Content-Type" ) != "" {
t . Errorf ( "Content-Type should be empty, got %q" , w . Header ( ) . Get ( "Content-Type" ) )
}
if w . Header ( ) . Get ( "Content-Length" ) != "" {
t . Errorf ( "Content-Length should be empty, got %q" , w . Header ( ) . Get ( "Content-Length" ) )
}
if w . Header ( ) . Get ( "ETag" ) != "" {
t . Errorf ( "ETag should be empty, got %q" , w . Header ( ) . Get ( "ETag" ) )
}
}
func TestJSONError ( t * testing . T ) {
tests := [ ] struct {
status int
message string
} {
{ http . StatusBadRequest , "bad request" } ,
{ http . StatusNotFound , "not found" } ,
{ http . StatusInternalServerError , "internal error" } ,
}
for _ , tt := range tests {
w := httptest . NewRecorder ( )
JSONError ( w , tt . status , tt . message )
if w . Code != tt . status {
t . Errorf ( "status = %d, want %d" , w . Code , tt . status )
}
if w . Header ( ) . Get ( "Content-Type" ) != "application/json" {
t . Errorf ( "Content-Type = %q, want %q" , w . Header ( ) . Get ( "Content-Type" ) , "application/json" )
}
body := w . Body . String ( )
if ! strings . Contains ( body , tt . message ) {
t . Errorf ( "body %q should contain %q" , body , tt . message )
}
}
}
func TestNewProxy_NilLogger ( t * testing . T ) {
dir := t . TempDir ( )
db , err := database . Create ( dir + "/test.db" )
if err != nil {
t . Fatalf ( "failed to create test database: %v" , err )
}
defer func ( ) { _ = db . Close ( ) } ( )
proxy := NewProxy ( db , newMockStorage ( ) , & mockFetcher { } , fetch . NewResolver ( ) , nil )
if proxy . Logger == nil {
t . Error ( "Logger should be set to default when nil is passed" )
}
}
Fix metadata caching, 404 propagation, mirror progress, and registry stubs
- ProxyCached now stores upstream Last-Modified in the cache and uses it
(along with ETag) for conditional request handling, returning 304 when
client validators match. Adds Content-Length to cached responses.
- Handlers calling FetchOrCacheMetadata (pypi, composer, pub, nuget) now
check for ErrUpstreamNotFound and return 404 instead of 502, matching
the existing npm and cargo behavior.
- Mirror jobs report live progress via a periodic callback while running,
so API polls return real counts instead of zeroed progress.
- Registry mirroring removed from CLI flags, API acceptance, README, and
docs since every enumerator was a stub returning "not yet implemented".
- Added tests for the conditional metadata path (ETag/If-None-Match,
Last-Modified/If-Modified-Since, 304 responses, header omission).
2026-04-01 20:14:11 +01:00
const testLastModified = "Wed, 01 Jan 2025 12:00:00 GMT"
// setupCachedProxy creates a Proxy with CacheMetadata enabled and an upstream
// test server that returns JSON with ETag and Last-Modified headers.
func setupCachedProxy ( t * testing . T , upstreamETag , upstreamLastModified string ) ( * Proxy , * httptest . Server ) {
t . Helper ( )
upstream := httptest . NewServer ( http . HandlerFunc ( func ( w http . ResponseWriter , r * http . Request ) {
if upstreamETag != "" {
w . Header ( ) . Set ( "ETag" , upstreamETag )
}
if upstreamLastModified != "" {
w . Header ( ) . Set ( "Last-Modified" , upstreamLastModified )
}
w . Header ( ) . Set ( "Content-Type" , "application/json" )
w . WriteHeader ( http . StatusOK )
_ , _ = w . Write ( [ ] byte ( ` { "ok":true} ` ) )
} ) )
t . Cleanup ( upstream . Close )
proxy , _ , _ , _ := setupTestProxy ( t )
proxy . CacheMetadata = true
proxy . HTTPClient = upstream . Client ( )
return proxy , upstream
}
func TestProxyCached_SetsETagAndLastModified ( t * testing . T ) {
lm := testLastModified
proxy , upstream := setupCachedProxy ( t , ` "abc123" ` , lm )
// First request populates the cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "test-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "status = %d, want 200" , w . Code )
}
if got := w . Header ( ) . Get ( "ETag" ) ; got != ` "abc123" ` {
t . Errorf ( "ETag = %q, want %q" , got , ` "abc123" ` )
}
if got := w . Header ( ) . Get ( "Last-Modified" ) ; got != lm {
t . Errorf ( "Last-Modified = %q, want %q" , got , lm )
}
if got := w . Header ( ) . Get ( "Content-Length" ) ; got != "11" {
t . Errorf ( "Content-Length = %q, want %q" , got , "11" )
}
if w . Body . String ( ) != ` { "ok":true} ` {
t . Errorf ( "body = %q, want %q" , w . Body . String ( ) , ` { "ok":true} ` )
}
}
func TestProxyCached_IfNoneMatch_Returns304 ( t * testing . T ) {
proxy , upstream := setupCachedProxy ( t , ` "abc123" ` , "" )
// Populate cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "etag-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "initial request: status = %d, want 200" , w . Code )
}
// Conditional request with matching ETag
req = httptest . NewRequest ( http . MethodGet , "/test" , nil )
req . Header . Set ( "If-None-Match" , ` "abc123" ` )
w = httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "etag-key" )
if w . Code != http . StatusNotModified {
t . Errorf ( "conditional request: status = %d, want 304" , w . Code )
}
if w . Body . Len ( ) != 0 {
t . Errorf ( "304 response should have empty body, got %d bytes" , w . Body . Len ( ) )
}
}
func TestProxyCached_IfNoneMatch_NonMatching_Returns200 ( t * testing . T ) {
proxy , upstream := setupCachedProxy ( t , ` "abc123" ` , "" )
// Populate cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "etag-nm-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "initial request: status = %d, want 200" , w . Code )
}
// Conditional request with non-matching ETag
req = httptest . NewRequest ( http . MethodGet , "/test" , nil )
req . Header . Set ( "If-None-Match" , ` "different" ` )
w = httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "etag-nm-key" )
if w . Code != http . StatusOK {
t . Errorf ( "non-matching ETag: status = %d, want 200" , w . Code )
}
}
func TestProxyCached_IfModifiedSince_Returns304 ( t * testing . T ) {
lm := testLastModified
proxy , upstream := setupCachedProxy ( t , "" , lm )
// Populate cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "lm-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "initial request: status = %d, want 200" , w . Code )
}
// Conditional request with If-Modified-Since equal to Last-Modified
req = httptest . NewRequest ( http . MethodGet , "/test" , nil )
req . Header . Set ( "If-Modified-Since" , lm )
w = httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "lm-key" )
if w . Code != http . StatusNotModified {
t . Errorf ( "conditional request: status = %d, want 304" , w . Code )
}
}
func TestProxyCached_IfModifiedSince_OlderDate_Returns200 ( t * testing . T ) {
lm := testLastModified
proxy , upstream := setupCachedProxy ( t , "" , lm )
// Populate cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "lm-old-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "initial request: status = %d, want 200" , w . Code )
}
// Conditional request with If-Modified-Since older than Last-Modified
req = httptest . NewRequest ( http . MethodGet , "/test" , nil )
req . Header . Set ( "If-Modified-Since" , "Mon, 01 Dec 2024 12:00:00 GMT" )
w = httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "lm-old-key" )
if w . Code != http . StatusOK {
t . Errorf ( "older If-Modified-Since: status = %d, want 200" , w . Code )
}
}
func TestProxyCached_NoValidators_OmitsHeaders ( t * testing . T ) {
proxy , upstream := setupCachedProxy ( t , "" , "" )
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "no-val-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "status = %d, want 200" , w . Code )
}
if got := w . Header ( ) . Get ( "ETag" ) ; got != "" {
t . Errorf ( "ETag should be empty when upstream has none, got %q" , got )
}
if got := w . Header ( ) . Get ( "Last-Modified" ) ; got != "" {
t . Errorf ( "Last-Modified should be empty when upstream has none, got %q" , got )
}
}
2026-04-06 19:30:59 +01:00
func TestFetchOrCacheMetadata_TTL_ServesFreshFromCache ( t * testing . T ) {
2026-08-16 18:01:59 +01:00
hitsBefore := testutil . ToFloat64 ( metrics . CacheHits . WithLabelValues ( "test" ) )
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "test" ) )
2026-04-06 19:30:59 +01:00
upstreamHits := 0
upstream := httptest . NewServer ( http . HandlerFunc ( func ( w http . ResponseWriter , r * http . Request ) {
upstreamHits ++
w . Header ( ) . Set ( "Content-Type" , "application/json" )
_ , _ = w . Write ( [ ] byte ( ` { "v":1} ` ) )
} ) )
t . Cleanup ( upstream . Close )
proxy , _ , _ , _ := setupTestProxy ( t )
proxy . CacheMetadata = true
proxy . MetadataTTL = 1 * time . Hour
proxy . HTTPClient = upstream . Client ( )
ctx := context . Background ( )
// First request populates cache
body , _ , err := proxy . FetchOrCacheMetadata ( ctx , "test" , "ttl-pkg" , upstream . URL + "/pkg" )
if err != nil {
t . Fatalf ( "first fetch: %v" , err )
}
if string ( body ) != ` { "v":1} ` {
t . Errorf ( "body = %q, want %q" , body , ` { "v":1} ` )
}
if upstreamHits != 1 {
t . Fatalf ( "expected 1 upstream hit, got %d" , upstreamHits )
}
2026-08-16 18:01:59 +01:00
if diff := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "test" ) ) - missesBefore ; diff != 1 {
t . Errorf ( "cache misses delta after first request = %.0f, want 1" , diff )
}
if diff := testutil . ToFloat64 ( metrics . CacheHits . WithLabelValues ( "test" ) ) - hitsBefore ; diff != 0 {
t . Errorf ( "cache hits delta after first request = %.0f, want 0" , diff )
}
2026-04-06 19:30:59 +01:00
// Second request within TTL should serve from cache without hitting upstream
body , _ , err = proxy . FetchOrCacheMetadata ( ctx , "test" , "ttl-pkg" , upstream . URL + "/pkg" )
if err != nil {
t . Fatalf ( "second fetch: %v" , err )
}
if string ( body ) != ` { "v":1} ` {
t . Errorf ( "body = %q, want %q" , body , ` { "v":1} ` )
}
if upstreamHits != 1 {
t . Errorf ( "expected upstream to still be hit only once, got %d" , upstreamHits )
}
2026-08-16 18:01:59 +01:00
if diff := testutil . ToFloat64 ( metrics . CacheHits . WithLabelValues ( "test" ) ) - hitsBefore ; diff != 1 {
t . Errorf ( "cache hits delta after second request = %.0f, want 1" , diff )
}
if diff := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "test" ) ) - missesBefore ; diff != 1 {
t . Errorf ( "cache misses delta after second request = %.0f, want 1" , diff )
}
2026-04-06 19:30:59 +01:00
}
func TestFetchOrCacheMetadata_TTL_Zero_AlwaysRevalidates ( t * testing . T ) {
2026-08-16 18:01:59 +01:00
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "test" ) )
2026-04-06 19:30:59 +01:00
upstreamHits := 0
upstream := httptest . NewServer ( http . HandlerFunc ( func ( w http . ResponseWriter , r * http . Request ) {
upstreamHits ++
w . Header ( ) . Set ( "Content-Type" , "application/json" )
_ , _ = w . Write ( [ ] byte ( ` { "v":1} ` ) )
} ) )
t . Cleanup ( upstream . Close )
proxy , _ , _ , _ := setupTestProxy ( t )
proxy . CacheMetadata = true
proxy . MetadataTTL = 0 // always revalidate
proxy . HTTPClient = upstream . Client ( )
ctx := context . Background ( )
_ , _ , err := proxy . FetchOrCacheMetadata ( ctx , "test" , "ttl0-pkg" , upstream . URL + "/pkg" )
if err != nil {
t . Fatalf ( "first fetch: %v" , err )
}
_ , _ , err = proxy . FetchOrCacheMetadata ( ctx , "test" , "ttl0-pkg" , upstream . URL + "/pkg" )
if err != nil {
t . Fatalf ( "second fetch: %v" , err )
}
if upstreamHits != 2 {
t . Errorf ( "expected 2 upstream hits with TTL=0, got %d" , upstreamHits )
}
2026-08-16 18:01:59 +01:00
missesAfter := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( "test" ) )
if diff := missesAfter - missesBefore ; diff != 2 {
t . Errorf ( "cache misses delta = %.0f, want 2" , diff )
}
}
func TestFetchOrCacheMetadata_CacheDisabledDoesNotRecordMetrics ( t * testing . T ) {
const ecosystem = "metadata-disabled"
upstream := httptest . NewServer ( http . HandlerFunc ( func ( w http . ResponseWriter , r * http . Request ) {
w . Header ( ) . Set ( "Content-Type" , "application/json" )
_ , _ = w . Write ( [ ] byte ( ` { "v":1} ` ) )
} ) )
t . Cleanup ( upstream . Close )
proxy , _ , _ , _ := setupTestProxy ( t )
proxy . HTTPClient = upstream . Client ( )
hitsBefore := testutil . ToFloat64 ( metrics . CacheHits . WithLabelValues ( ecosystem ) )
missesBefore := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( ecosystem ) )
_ , _ , err := proxy . FetchOrCacheMetadata ( context . Background ( ) , ecosystem , "pkg" , upstream . URL + "/pkg" )
if err != nil {
t . Fatalf ( "fetch metadata: %v" , err )
}
hitsAfter := testutil . ToFloat64 ( metrics . CacheHits . WithLabelValues ( ecosystem ) )
missesAfter := testutil . ToFloat64 ( metrics . CacheMisses . WithLabelValues ( ecosystem ) )
if diff := hitsAfter - hitsBefore ; diff != 0 {
t . Errorf ( "cache hits delta = %.0f, want 0" , diff )
}
if diff := missesAfter - missesBefore ; diff != 0 {
t . Errorf ( "cache misses delta = %.0f, want 0" , diff )
}
2026-04-06 19:30:59 +01:00
}
func TestProxyCached_StaleWarningHeader ( t * testing . T ) {
requestCount := 0
upstream := httptest . NewServer ( http . HandlerFunc ( func ( w http . ResponseWriter , r * http . Request ) {
requestCount ++
if requestCount == 1 {
// First request succeeds to populate cache
w . Header ( ) . Set ( "Content-Type" , "application/json" )
_ , _ = w . Write ( [ ] byte ( ` { "cached":true} ` ) )
return
}
// Subsequent requests fail to simulate upstream outage
w . WriteHeader ( http . StatusBadGateway )
} ) )
t . Cleanup ( upstream . Close )
proxy , _ , _ , _ := setupTestProxy ( t )
proxy . CacheMetadata = true
proxy . MetadataTTL = 1 * time . Millisecond // very short TTL so it expires immediately
proxy . HTTPClient = upstream . Client ( )
// First request populates cache
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "stale-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "initial request: status = %d, want 200" , w . Code )
}
// Wait for TTL to expire
time . Sleep ( 5 * time . Millisecond )
// Second request: upstream fails, should serve stale cache with Warning header
req = httptest . NewRequest ( http . MethodGet , "/test" , nil )
w = httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "stale-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "stale request: status = %d, want 200" , w . Code )
}
if w . Body . String ( ) != ` { "cached":true} ` {
t . Errorf ( "body = %q, want %q" , w . Body . String ( ) , ` { "cached":true} ` )
}
if got := w . Header ( ) . Get ( "Warning" ) ; got != ` 110 - "Response is Stale" ` {
t . Errorf ( "Warning = %q, want %q" , got , ` 110 - "Response is Stale" ` )
}
}
func TestProxyCached_FreshResponse_NoWarningHeader ( t * testing . T ) {
proxy , upstream := setupCachedProxy ( t , "" , "" )
proxy . MetadataTTL = 1 * time . Hour
req := httptest . NewRequest ( http . MethodGet , "/test" , nil )
w := httptest . NewRecorder ( )
proxy . ProxyCached ( w , req , upstream . URL + "/test" , "test-eco" , "fresh-key" )
if w . Code != http . StatusOK {
t . Fatalf ( "status = %d, want 200" , w . Code )
}
if got := w . Header ( ) . Get ( "Warning" ) ; got != "" {
t . Errorf ( "Warning should be empty for fresh response, got %q" , got )
}
}
2026-07-17 18:11:47 -07:00
// TestCanonicalPackagePURLMatchesConfig ensures the runtime cooldown lookup key
// agrees with config.CooldownConfig.NormalizedPackages for the same package,
// so a configured override is actually found regardless of how the user wrote it.
func TestCanonicalPackagePURLMatchesConfig ( t * testing . T ) {
tests := [ ] struct {
ecosystem string
requestName string
configKey string
} {
{ "npm" , "@babel/core" , "pkg:npm/@babel/core" } ,
{ "npm" , "@babel/core" , "pkg:npm/%40babel/core" } ,
{ "npm" , "@typescript/typescript-darwin-arm64" , "pkg:npm/@typescript/typescript-darwin-arm64" } ,
{ "pypi" , "Django" , "pkg:pypi/Django" } ,
{ "pypi" , "django" , "pkg:pypi/Django" } ,
{ "composer" , "symfony/console" , "pkg:composer/Symfony/Console" } ,
{ "cargo" , "serde" , "pkg:cargo/serde" } ,
}
for _ , tt := range tests {
t . Run ( tt . ecosystem + "/" + tt . requestName + "<=" + tt . configKey , func ( t * testing . T ) {
cfg := config . CooldownConfig { Packages : map [ string ] string { tt . configKey : "1d" } }
normalized := cfg . NormalizedPackages ( )
lookup := canonicalPackagePURL ( tt . ecosystem , tt . requestName )
if _ , ok := normalized [ lookup ] ; ! ok {
t . Errorf ( "lookup key %q not found in normalized config %v" , lookup , normalized )
}
} )
}
}
