package handler import ( "context" "errors" "io" "strings" "sync" "testing" "time" "github.com/git-pkgs/artifacts" "github.com/git-pkgs/registries/fetch" ) // runConcurrent runs fn in n goroutines released together and returns their errors. func runConcurrent(n int, fn func(i int) error) []error { errs := make([]error, n) start := make(chan struct{}) var wg sync.WaitGroup for i := 0; i < n; i++ { wg.Add(1) go func(i int) { defer wg.Done() <-start errs[i] = fn(i) }(i) } close(start) wg.Wait() return errs } // artifactBody builds a one-shot upstream artifact carrying the given bytes. func artifactBody(content string) *fetch.Artifact { return &fetch.Artifact{ Body: io.NopCloser(strings.NewReader(content)), ContentType: "application/gzip", } } // drain consumes and closes a CacheResult reader, if there is one. func drain(res *CacheResult) { if res != nil && res.Reader != nil { _, _ = io.Copy(io.Discard, res.Reader) _ = res.Reader.Close() } } // TestCoalesceKey_DifferentUpstreamHashDoesNotShare is the safety property that // makes coalescing sound: callers expecting different bytes must never share a // fetch, so a re-published version cannot serve stale bytes to a caller that // asked for the new digest. func TestCoalesceKey_DifferentUpstreamHashDoesNotShare(t *testing.T) { const content = "artifact bytes" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: content, delay: fetchHoldTime} proxy.Fetcher = fetcher // The digest must carry the "sha256:" prefix; without it the API treats the // value as unverifiable and clears the hash, which would legitimately let // the two callers share one fetch. hashes := []string{ "sha256:" + sha256Hex(content), "sha256:" + sha256Hex("something else entirely"), } _ = runConcurrent(2, func(i int) error { res, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", hashes[i]) drain(res) return err }) if got := fetcher.calls.Load(); got != 2 { t.Errorf("upstream fetches = %d, want 2: callers expecting different digests must not share a fetch", got) } } // TestCoalesceKey_HashCasingSharesOneFetch is the other half of that property. // artifactHashMatches compares digests case-insensitively, so one digest in two // casings describes one artifact and must not split into two fetches. func TestCoalesceKey_HashCasingSharesOneFetch(t *testing.T) { const content = "artifact bytes" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: content, delay: fetchHoldTime} proxy.Fetcher = fetcher hex := sha256Hex(content) digests := []string{"sha256:" + hex, "sha256:" + strings.ToUpper(hex)} for i, err := range runConcurrent(2, func(i int) error { res, err := proxy.GetOrFetchArtifactFromURLWithDigest(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", digests[i]) drain(res) return err }) { if err != nil { t.Fatalf("caller %d failed: %v", i, err) } } if got := fetcher.calls.Load(); got != 1 { t.Errorf("upstream fetches = %d, want 1: one digest in two casings is one artifact", got) } } // TestCoalesceKey_DifferentDownloadURLDoesNotShare covers the other half of the // key: same package, different upstream URL, must not collapse into one fetch. func TestCoalesceKey_DifferentDownloadURLDoesNotShare(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher urls := []string{ "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", "https://mirror.example.com/pkg/-/pkg-1.0.0.tgz", } _ = runConcurrent(2, func(i int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", urls[i]) drain(res) return err }) if got := fetcher.calls.Load(); got != 2 { t.Errorf("upstream fetches = %d, want 2: different upstream URLs must not share a fetch", got) } } // TestCoalesceKey_DistinctArtifactsDoNotSerialize guards against an over-broad // key: four packages fetched at once must still produce four fetches. func TestCoalesceKey_DistinctArtifactsDoNotSerialize(t *testing.T) { const n = 4 proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher names := []string{"alpha", "beta", "gamma", "delta"} errs := runConcurrent(n, func(i int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", names[i], "1.0.0", names[i]+"-1.0.0.tgz", "https://registry.npmjs.org/"+names[i]+"/-/"+names[i]+"-1.0.0.tgz") drain(res) return err }) for i, err := range errs { if err != nil { t.Errorf("caller %d (%s): %v", i, names[i], err) } } if got := fetcher.calls.Load(); got != n { t.Errorf("upstream fetches = %d, want %d: distinct artifacts must not share a fetch", got, n) } } // TestCoalesce_FailedFetchReachesEveryCallerAndIsRetriable verifies both claims // in coalesceFetch's doc comment: a failed fetch reaches every caller sharing // it, and the key is released so a later request retries. func TestCoalesce_FailedFetchReachesEveryCallerAndIsRetriable(t *testing.T) { const callers = 8 proxy, _, _, fetcher := setupTestProxy(t) boom := errors.New("upstream unavailable") fetcher.fetchErr = boom errs := runConcurrent(callers, func(int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") drain(res) return err }) for i, err := range errs { if err == nil { t.Errorf("caller %d: got nil error, want the shared fetch's failure", i) } else if !errors.Is(err, boom) { t.Errorf("caller %d: got %v, want it to wrap %v", i, err, boom) } } // The key must be released: a later request retries rather than inheriting // the failure. fetcher.fetchErr = nil fetcher.artifact = artifactBody("recovered bytes") res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") if err != nil { t.Fatalf("retry after failed coalesced fetch: %v", err) } body, _ := io.ReadAll(res.Reader) _ = res.Reader.Close() if string(body) != "recovered bytes" { t.Errorf("retry body = %q, want %q", body, "recovered bytes") } } // TestCoalesce_ResolverPath covers the other entry point: GetOrFetchArtifact // resolves the URL itself, so it is keyed without one. func TestCoalesce_ResolverPath(t *testing.T) { const callers = 8 proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "resolved artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher errs := runConcurrent(callers, func(int) error { res, err := proxy.GetOrFetchArtifact(context.Background(), "npm", "left-pad", "1.3.0", "left-pad-1.3.0.tgz") drain(res) return err }) for i, err := range errs { if err != nil { t.Errorf("caller %d: %v", i, err) } } if got := fetcher.calls.Load(); got != 1 { t.Errorf("upstream fetches = %d, want 1", got) } } // TestCoalesce_ResolverPathEmptyFilename exercises that path when the filename // is left to be resolved, which the key cannot know up front. func TestCoalesce_ResolverPathEmptyFilename(t *testing.T) { const callers = 8 proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "resolved artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher errs := runConcurrent(callers, func(int) error { res, err := proxy.GetOrFetchArtifact(context.Background(), "npm", "left-pad", "1.3.0", "") drain(res) return err }) for i, err := range errs { if err != nil { t.Errorf("caller %d: %v", i, err) } } if got := fetcher.calls.Load(); got != 1 { t.Errorf("upstream fetches = %d, want 1", got) } } // TestCoalesce_SubsequentRequestIsACacheHit confirms the coalesced fetch was // committed and is visible later, not just streamed to the waiting callers. func TestCoalesce_SubsequentRequestIsACacheHit(t *testing.T) { const callers = 8 const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher _ = runConcurrent(callers, func(int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) drain(res) return err }) res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) if err != nil { t.Fatalf("follow-up request: %v", err) } defer func() { _ = res.Reader.Close() }() if !res.Cached { t.Error("follow-up request should be served from cache") } if got := fetcher.calls.Load(); got != 1 { t.Errorf("upstream fetches = %d, want 1 after a follow-up cache hit", got) } } // TestCoalesce_ReadersAreIndependent guards openStoredArtifact: callers sharing // a fetch each need their own reader, or one closing early breaks the rest. func TestCoalesce_ReadersAreIndependent(t *testing.T) { const callers = 8 const content = "artifact bytes that every caller must receive intact" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: content, delay: fetchHoldTime} proxy.Fetcher = fetcher results := make([]*CacheResult, callers) errs := runConcurrent(callers, func(i int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz") results[i] = res return err }) for i, err := range errs { if err != nil { t.Fatalf("caller %d: %v", i, err) } } // Close the first caller's reader before anyone else has read a byte. _ = results[0].Reader.Close() for i := 1; i < callers; i++ { body, err := io.ReadAll(results[i].Reader) _ = results[i].Reader.Close() if err != nil { t.Errorf("caller %d read after another caller closed: %v", i, err) continue } if string(body) != content { t.Errorf("caller %d got %q, want %q", i, body, content) } } } // TestCoalesce_CanceledWaiterDoesNotWaitForTheSharedFetch checks that joining a // coalesced fetch does not cost a caller its own cancellation. Without the // leader/waiter split a waiter is pinned until the shared fetch resolves, // bounded only by the artifact client timeout, so clients that have already // gone away keep handler goroutines alive for minutes. func TestCoalesce_CanceledWaiterDoesNotWaitForTheSharedFetch(t *testing.T) { const leaderFetch = 2 * time.Second const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "artifact bytes", delay: leaderFetch, entered: make(chan struct{})} proxy.Fetcher = fetcher leaderDone := make(chan error, 1) go func() { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) drain(res) leaderDone <- err }() select { case <-fetcher.entered: // the leader holds the key and is inside its fetch case <-time.After(5 * time.Second): t.Fatal("leader never started its fetch") } ctx, cancel := context.WithCancel(context.Background()) cancel() start := time.Now() _, err := proxy.GetOrFetchArtifactFromURL(ctx, "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) blocked := time.Since(start) if !errors.Is(err, context.Canceled) { t.Errorf("waiter error = %v, want context.Canceled", err) } if blocked > leaderFetch/4 { t.Errorf("canceled waiter blocked %v, want well under %v: it is pinned to the shared fetch", blocked, leaderFetch/4) } // A waiter leaving must not disturb the fetch the others share. if err := <-leaderDone; err != nil { t.Fatalf("leader failed after a waiter canceled: %v", err) } res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) if err != nil { t.Fatalf("follow-up after leader completed: %v", err) } defer func() { _ = res.Reader.Close() }() if !res.Cached { t.Error("leader's fetch should have been committed to the cache") } if got := fetcher.calls.Load(); got != 1 { t.Errorf("upstream fetches = %d, want 1", got) } } // inFlightLen reports how many coalesced fetches are currently registered. func inFlightLen(p *Proxy) int { p.fetchMu.Lock() defer p.fetchMu.Unlock() return len(p.inFlight) } // TestCoalesce_KeyIsReleasedAfterFetch guards the bug this hand-rolled map can // have that singleflight could not: a key left behind means later callers join // a finished entry, see its closed done channel, and are served that stale // result forever, while the map grows without bound. func TestCoalesce_KeyIsReleasedAfterFetch(t *testing.T) { const url = "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz" proxy, _, _, _ := setupTestProxy(t) fetcher := &countingFetcher{content: "artifact bytes", delay: fetchHoldTime} proxy.Fetcher = fetcher _ = runConcurrent(8, func(int) error { res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) drain(res) return err }) if n := inFlightLen(proxy); n != 0 { t.Errorf("in-flight entries after a successful fetch = %d, want 0", n) } // A fresh miss for the same key must start a new fetch, not rejoin the old // entry. Clearing the cache record forces the miss path again. if err := proxy.ClearCachedArtifact(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz"); err != nil { t.Fatalf("clear cached artifact: %v", err) } res, err := proxy.GetOrFetchArtifactFromURL(context.Background(), "npm", "pkg", "1.0.0", "pkg-1.0.0.tgz", url) if err != nil { t.Fatalf("second miss for the same key: %v", err) } drain(res) if got := fetcher.calls.Load(); got != 2 { t.Errorf("upstream fetches = %d, want 2: the second miss must not reuse the finished entry", got) } if n := inFlightLen(proxy); n != 0 { t.Errorf("in-flight entries at end = %d, want 0", n) } } // missingFromCache is a recheck that always reports a miss, so the shared fetch // runs. func missingFromCache() (artifacts.Artifact, string, bool) { return artifacts.Artifact{}, "", false } // TestCoalesce_LeaderRechecksCacheBeforeFetching covers the window between a // caller's own cache lookup and it becoming the leader: a concurrent fetch can // commit the artifact in that gap, and the leader must serve that rather than // fetch it a second time. func TestCoalesce_LeaderRechecksCacheBeforeFetching(t *testing.T) { const content = "artifact bytes" proxy, _, store, _ := setupTestProxy(t) const storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" if _, _, err := store.Store(context.Background(), storagePath, strings.NewReader(content)); err != nil { t.Fatalf("seeding storage: %v", err) } committed := artifacts.Artifact{ PURL: "pkg:npm/pkg@1.0.0", Filename: "pkg-1.0.0.tgz", Size: int64(len(content)), } res, err := proxy.coalesceFetch(context.Background(), "any-key", func() (artifacts.Artifact, string, bool) { return committed, storagePath, true }, func(context.Context) (artifacts.Artifact, string, error) { t.Error("fetched an artifact that was already in the cache") return artifacts.Artifact{}, "", errors.New("commit must not run") }) if err != nil { t.Fatalf("coalesceFetch failed: %v", err) } defer drain(res) got, err := io.ReadAll(res.Reader) if err != nil { t.Fatalf("reading result: %v", err) } if string(got) != content { t.Errorf("got %q, want %q", got, content) } if n := inFlightLen(proxy); n != 0 { t.Errorf("in-flight entries = %d, want 0", n) } } // TestCachedArtifactRecord covers the recheck itself: it must report the row a // concurrent fetch committed, match its digest the way artifactHashMatches // does, and report a miss for anything else. func TestCachedArtifactRecord(t *testing.T) { const ( content = "artifact bytes" pkgPURL = "pkg:npm/pkg" versionPURL = "pkg:npm/pkg@1.0.0" filename = "pkg-1.0.0.tgz" storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" ) proxy, _, _, _ := setupTestProxy(t) hex := sha256Hex(content) committed := testArtifact(content, versionPURL, filename, "application/gzip") if err := proxy.updateCacheDB("npm", "pkg", pkgPURL, "https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz", storagePath, committed); err != nil { t.Fatalf("seeding cache record: %v", err) } for _, tc := range []struct { name, filename, hash string want bool }{ {"no upstream hash", filename, "", true}, {"matching hash", filename, hex, true}, {"matching hash in upper case", filename, strings.ToUpper(hex), true}, {"different hash", filename, sha256Hex("something else entirely"), false}, {"unknown filename", "pkg-1.0.0.zip", hex, false}, } { t.Run(tc.name, func(t *testing.T) { got, path, ok := proxy.cachedArtifactRecord(pkgPURL, versionPURL, tc.filename, tc.hash) if ok != tc.want { t.Fatalf("ok = %v, want %v", ok, tc.want) } if !ok { return } if path != storagePath { t.Errorf("storage path = %q, want %q", path, storagePath) } if got.Digest.Encoded() != hex { t.Errorf("digest = %q, want %q", got.Digest.Encoded(), hex) } }) } } // TestCoalesce_LeaderFetchesWhenRecheckedBytesAreGone covers the other branch // of the recheck: a record whose bytes no longer open is not served, and the // shared fetch runs instead, the same recovery the cache lookup makes. func TestCoalesce_LeaderFetchesWhenRecheckedBytesAreGone(t *testing.T) { const content = "fetched bytes" const storagePath = "npm/pkg/1.0.0/pkg-1.0.0.tgz" proxy, _, store, _ := setupTestProxy(t) stale := artifacts.Artifact{PURL: "pkg:npm/pkg@1.0.0", Filename: "pkg-1.0.0.tgz"} if _, err := store.Open(context.Background(), storagePath); err == nil { t.Fatal("stale bytes were present, so the test proves nothing") } // The leader runs commit on its own goroutine, so a plain counter is safe. fetches := 0 res, err := proxy.coalesceFetch(context.Background(), "any-key", func() (artifacts.Artifact, string, bool) { return stale, storagePath, true }, func(ctx context.Context) (artifacts.Artifact, string, error) { fetches++ if _, _, err := store.Store(ctx, storagePath, strings.NewReader(content)); err != nil { return artifacts.Artifact{}, "", err } return testArtifact(content, stale.PURL, stale.Filename, "application/gzip"), storagePath, nil }) if err != nil { t.Fatalf("coalesceFetch failed: %v", err) } defer drain(res) if fetches != 1 { t.Errorf("shared fetches = %d, want 1: a record without bytes must be refetched", fetches) } got, err := io.ReadAll(res.Reader) if err != nil { t.Fatalf("reading result: %v", err) } if string(got) != content { t.Errorf("got %q, want %q", got, content) } if n := inFlightLen(proxy); n != 0 { t.Errorf("in-flight entries = %d, want 0", n) } } // TestCoalesce_PanicInSharedFetchDoesNotStrandWaiters checks the failure mode // that matters most: a caller parked on a shared fetch must never be left // blocked forever when that fetch dies. // // This drives coalesceFetch directly and holds the shared entry itself, because // whether a second 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. Losing the race made a second caller the leader // instead of a waiter, and its panic was unrecovered, killing the test binary // rather than failing the test. func TestCoalesce_PanicInSharedFetchDoesNotStrandWaiters(t *testing.T) { proxy, _, _, _ := setupTestProxy(t) const key = "pkg:npm/pkg@1.0.0\x00pkg-1.0.0.tgz" inCommit := make(chan struct{}) release := make(chan struct{}) leaderPanicked := make(chan struct{}) go func() { defer func() { _ = recover() // the panic surfaces in the leader, as it would in a handler close(leaderPanicked) }() _, _ = proxy.coalesceFetch(context.Background(), key, missingFromCache, func(context.Context) (artifacts.Artifact, string, error) { close(inCommit) <-release panic("upstream fetch exploded") }) }() <-inCommit // the leader holds the key and is inside the fetch // Take the entry a waiter would park on, while the leader is still held. proxy.fetchMu.Lock() shared := proxy.inFlight[key] proxy.fetchMu.Unlock() if shared == nil { t.Fatal("no in-flight entry registered for a running fetch") } close(release) select { case <-shared.done: case <-time.After(5 * time.Second): t.Fatal("waiter stranded: a panicking shared fetch never released its waiters") } if !errors.Is(shared.err, errSharedFetchAbandoned) { t.Errorf("waiter error = %v, want errSharedFetchAbandoned", shared.err) } <-leaderPanicked if n := inFlightLen(proxy); n != 0 { t.Errorf("in-flight entries after a panic = %d, want 0", n) } }