From 20a0b0c1b650b9b10adb84aee1e4fcfa947a5d5d Mon Sep 17 00:00:00 2001 From: Jonathan Yoder Date: Mon, 3 Aug 2026 19:47:14 -0400 Subject: [PATCH] feat(index): CachedJSONIndex with a bounded, singleflighted cache Implements RFD 0001 Phase 3.2's go-pyresolver half: Files served from the per-snapshot index JSON, cached by (package, snapshot). RSFIndex is deliberately NOT here. It needs PPM's deps-blob decoder and store types -- depsblob.DecodePackage returns map[string]storetypes.PyPIVersionMetadata, carrying Blocked, BlockingRule, Vulns and MetadataMD5Sum -- and src/rsf has no go.mod of its own, so it is part of a private module that imports this one. Implementing it here would invert the dependency. Moved to rstudio/package-manager#19437. RFD Section 6 already implies this split: DBIndex is "backed by the pypi_projects table", which a public module cannot reach either, so Section 6's list is an inventory of implementations rather than a claim about which repo each lives in. Metadata always returns ErrMetadataUnavailable, on purpose. Rev 15 moved the dependency fieldset into the resident RSF specifically so resolution works air-gapped; serving requires_dist out of the CDN document this type already holds would quietly reintroduce a per-package network fetch on the resolution path. Refusing keeps that regression from being one convenient edit away, and a test asserts it makes no HTTP request at all. Versions IS served, since the same document lists them and it costs nothing extra. Wire format was taken from the real producer, not approximated: the fixture is an excerpt of PPM's own index_json_v2 document for click. That is what surfaced the details a hand-written fixture gets wrong -- digests arrive as a MAP (not a flat sha256 field), the timestamp field is upload_time_iso_8601 in RFC 3339 with fractional seconds, wheel-vs-sdist comes from an explicit packagetype rather than the filename, and yanked_reason is JSON null when absent. Cache: a small LRU with an explicit byte budget and singleflight coalescing, rather than ristretto. Not NIH -- PPM's own cachehelpers.BoundedCache exists for exactly this reason, documented at bounded_cache.go: ristretto "cannot size a non-[]byte value, so it admits Go objects with cost 0 and they silently escape the byte budget entirely" (#19374). Explicit accounting is the established pattern, and it spares a public library a cache dependency. Copy-on-return is load-bearing and tested. Singleflight hands the SAME value to every coalesced waiter, so copying only on a cache hit leaves the entry shared; Files copies on the way out. Verified by removing the copy and watching -race report a genuine DATA RACE plus a deterministic non-concurrent failure -- the exact bug class that bit PPM's snapshot cache (#19291). Memory budget is configurable with a conservative default rather than a constant, as the issue asked. Section 5.1 targets ~300-500MB, but Server.MemoryCacheSize has a 100MB floor on-prem against 4GB on P3M and this cache is ADDITIONAL to it, so defaulting to the RFD figure would roughly quadruple an on-prem memory floor as a side effect of enabling resolution. Default is 512 entries / 64 MiB; degradation is LRU eviction, so exceeding the budget costs a refetch of the coldest package, not a failure. Two judgment calls, both tested and commented: * Malformed per-file metadata is lenient -- an unparseable requires_python leaves the constraint unset rather than dropping the file, because dropping a package's only wheel makes it unresolvable for a reason nobody can see. Strict policy belongs to candidate selection, which can see all files at once. * An unparseable version key is skipped rather than failing the package, so one bad key cannot hide every other version. Also: a 5xx is never reported as ErrPackageNotFound (one is retryable, the other tells a resolver to give up), and Files falls back to PEP 440-equal comparison when the document's key spelling differs from the requested version ("1.0" vs "1.0.0" are the same version). Config is named CachedJSONConfig rather than Config, since this package will gain FilteredIndex and MultiIndex. Verified: 93.5% statement coverage; -race clean; gofmt silent; golangci-lint v2.11.2 reports 0 issues from the module root. Refs rstudio/package-manager#18647 Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 22 ++ go.mod | 5 +- go.sum | 2 + index/cache.go | 174 ++++++++++ index/cachedjson.go | 422 +++++++++++++++++++++++ index/cachedjson_test.go | 700 +++++++++++++++++++++++++++++++++++++++ index/doc.go | 42 +-- 7 files changed, 1346 insertions(+), 21 deletions(-) create mode 100644 index/cache.go create mode 100644 index/cachedjson.go create mode 100644 index/cachedjson_test.go diff --git a/CLAUDE.md b/CLAUDE.md index bfcb5f9..b5890c7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -55,6 +55,28 @@ keeps the generic solver free of Python knowledge, and it is required from day one: without it, any user of `[extras]` syntax silently gets an incomplete closure. +**PPM-backed index implementations do NOT live here.** `RSFIndex`, +`OfflineIndex`, and `DBIndex` need PPM's deps-blob decoder, store types, or +database — all in a private repo that imports this module, so putting them here +would invert the dependency. They implement `index.MetadataIndex` from the PPM +side. `DBIndex` is the clearest case: a public module cannot reach PPM's +`pypi_projects` table. What belongs here is anything generic: the interface, +`MockIndex`, `CachedJSONIndex`, and eventually `FilteredIndex`/`MultiIndex`. + +**A cached value handed to more than one caller must be copied on every path.** +`boundedCache.get` coalesces concurrent misses through singleflight, which hands +the *same* value to every waiter — so copying only on a cache hit is not enough, +and the leftover sharing shows up as a data race under load rather than as a +test failure. `CachedJSONIndex.Files` copies on the way out for this reason, and +there is a `-race` test that fails if the copy is removed. PPM hit this exact +bug in its own snapshot cache (#19291). + +**Cache keys must name immutable content.** A key that can describe two +payloads over time serves a stale one until eviction, and no TTL fixes that — it +only shortens the window. `(package, snapshot)` qualifies, with one documented +exception: `yanked` is mutable within a published snapshot (RFD §5.1), tracked +as #18650. + ## Build & test ```bash diff --git a/go.mod b/go.mod index 1c3da77..a540e24 100644 --- a/go.mod +++ b/go.mod @@ -2,6 +2,9 @@ module github.com/posit-dev/go-pyresolver go 1.25.0 -require github.com/posit-dev/go-python-packaging v0.2.0 +require ( + github.com/posit-dev/go-python-packaging v0.2.0 + golang.org/x/sync v0.22.0 +) require github.com/rstudio/go-version v0.0.2 // indirect diff --git a/go.sum b/go.sum index 239aea6..baa6d7f 100644 --- a/go.sum +++ b/go.sum @@ -8,5 +8,7 @@ github.com/rstudio/go-version v0.0.2 h1:ihU0xaF+Yuya0p2J6C8dfyB4gu/YehM1d6cTpabf github.com/rstudio/go-version v0.0.2/go.mod h1:Xfuma+m4R9L0P+Hof8iDDiL4/TJ8VjpdDXWq8qGXHL4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= +golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek= +golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/index/cache.go b/index/cache.go new file mode 100644 index 0000000..6dfbabb --- /dev/null +++ b/index/cache.go @@ -0,0 +1,174 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT + +package index + +import ( + "container/list" + "sync" + + "golang.org/x/sync/singleflight" +) + +// boundedCache is an LRU cache with an explicit byte budget and singleflight +// coalescing of concurrent misses. +// +// # Why byte accounting is caller-supplied rather than delegated to ristretto +// +// A general-purpose cache cannot size an arbitrary Go value. PPM learned this +// the hard way: its shared ristretto cache cannot size a non-[]byte value, so +// it admits Go objects at cost 0 and they escape the byte budget entirely +// (rstudio/package-manager#19374). PPM's own cachehelpers.BoundedCache exists +// for exactly that reason and takes a caller-supplied sizer. This does the +// same, which also spares a public library a cache dependency. +// +// # Safety contract +// +// An entry is only sound to cache if its key names IMMUTABLE content. A key +// that can describe two different payloads over time will serve a stale one +// until eviction, and no TTL makes that correct -- it only makes the window +// shorter. See CachedJSONIndex for how its key satisfies this, and for the one +// field that does not. +// +// # Copy contract +// +// This cache shares values by reference with every reader, and Get hands the +// SAME value to every goroutine coalesced into one singleflight flight. +// Callers that hand values onward to code which may mutate them must copy on +// BOTH paths -- see CachedJSONIndex.Files. +type boundedCache[V any] struct { + maxEntries int + maxBytes int64 + sizeOf func(V) int64 + + sf singleflight.Group + + mu sync.Mutex + entries map[string]*list.Element + lru *list.List // front = most recently used + totalBytes int64 +} + +type cacheEntry[V any] struct { + key string + value V + bytes int64 +} + +func newBoundedCache[V any](maxEntries int, maxBytes int64, sizeOf func(V) int64) *boundedCache[V] { + return &boundedCache[V]{ + maxEntries: maxEntries, + maxBytes: maxBytes, + sizeOf: sizeOf, + entries: make(map[string]*list.Element), + lru: list.New(), + } +} + +// lookup returns the cached value for key, promoting it to most-recently-used. +func (c *boundedCache[V]) lookup(key string) (V, bool) { + c.mu.Lock() + defer c.mu.Unlock() + + el, ok := c.entries[key] + if !ok { + var zero V + return zero, false + } + c.lru.MoveToFront(el) + + return el.Value.(*cacheEntry[V]).value, true +} + +// put stores value under key, evicting least-recently-used entries until both +// budgets are satisfied. +func (c *boundedCache[V]) put(key string, value V) { + size := c.sizeOf(value) + + c.mu.Lock() + defer c.mu.Unlock() + + if el, ok := c.entries[key]; ok { + existing := el.Value.(*cacheEntry[V]) + c.totalBytes -= existing.bytes + existing.value = value + existing.bytes = size + c.totalBytes += size + c.lru.MoveToFront(el) + c.evictLocked() + return + } + + // An entry larger than the whole budget is not cached at all. Storing it + // would evict everything else and then still not fit. + if c.maxBytes > 0 && size > c.maxBytes { + return + } + + c.entries[key] = c.lru.PushFront(&cacheEntry[V]{key: key, value: value, bytes: size}) + c.totalBytes += size + c.evictLocked() +} + +// evictLocked drops least-recently-used entries until both budgets hold. +// Callers must hold c.mu. +func (c *boundedCache[V]) evictLocked() { + for c.lru.Len() > 0 { + overEntries := c.maxEntries > 0 && c.lru.Len() > c.maxEntries + overBytes := c.maxBytes > 0 && c.totalBytes > c.maxBytes + if !overEntries && !overBytes { + return + } + + oldest := c.lru.Back() + if oldest == nil { + return + } + entry := oldest.Value.(*cacheEntry[V]) + c.lru.Remove(oldest) + delete(c.entries, entry.key) + c.totalBytes -= entry.bytes + } +} + +// get returns the cached value for key, building it with build on a miss. +// Concurrent misses for one key are coalesced into a single build. +// +// The returned value is NOT a copy -- see the copy contract on boundedCache. +func (c *boundedCache[V]) get(key string, build func() (V, error)) (V, error) { + if v, ok := c.lookup(key); ok { + return v, nil + } + + res, err, _ := c.sf.Do(key, func() (any, error) { + // Re-check under the flight: a concurrent flight for this key may have + // completed and populated the cache between our miss and here. + if v, ok := c.lookup(key); ok { + return v, nil + } + + built, err := build() + if err != nil { + return nil, err + } + c.put(key, built) + return built, nil + }) + if err != nil { + var zero V + return zero, err + } + + v, ok := res.(V) + if !ok { + var zero V + return zero, nil + } + return v, nil +} + +// stats reports current occupancy, for tests and for a future metrics surface. +func (c *boundedCache[V]) stats() (entries int, bytes int64) { + c.mu.Lock() + defer c.mu.Unlock() + return c.lru.Len(), c.totalBytes +} diff --git a/index/cachedjson.go b/index/cachedjson.go new file mode 100644 index 0000000..eea4327 --- /dev/null +++ b/index/cachedjson.go @@ -0,0 +1,422 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT + +package index + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strings" + "time" + + "github.com/posit-dev/go-python-packaging/version" +) + +// IndexJSONFolder is the path segment carrying the current per-snapshot index +// documents. The name is versioned by the producer; a v1 layout ("index_json") +// predates this one and is not read here. +const IndexJSONFolder = "index_json_v2" + +// Cache budget defaults. +// +// These are conservative on purpose. RFD 0001 Section 5.1 targets ~300-500MB +// for this cache, but that figure assumes headroom a small on-premises server +// does not have: PPM's Server.MemoryCacheSize has a 100MB floor on-prem +// against 4GB on P3M, and THIS cache is additional to it. Defaulting to the +// RFD's target would therefore roughly quadruple the memory floor of an +// on-prem install as a side effect of enabling resolution. +// +// So the default is small enough to be safe unconfigured, and the budget is +// explicit in CachedJSONConfig so an operator with headroom can raise it. Degradation at +// the default is graceful and quantifiable: entries are evicted +// least-recently-used, so exceeding the budget costs a repeat fetch of the +// coldest package, not a failure. +const ( + DefaultMaxCacheEntries = 512 + DefaultMaxCacheBytes = 64 << 20 // 64 MiB +) + +// CachedJSONConfig configures a CachedJSONIndex. +type CachedJSONConfig struct { + // BaseURL is the root the index documents hang from; the request path is + // BaseURL + "/" + IndexJSONFolder + "/" + pkg + "/" + Snapshot + ".json". + // + // Required. A file:// URL is NOT handled here -- see Client for how to + // serve an air-gapped deployment. + BaseURL string + + // Snapshot identifies which snapshot to read, e.g. "20260803T120000". + // + // It is fixed per index rather than per call because MetadataIndex's + // methods take no snapshot: a resolution runs against one snapshot, and + // letting it drift mid-resolution would let the resolver observe two + // different worlds. + Snapshot string + + // Client performs the requests. Defaults to a client with a 30s timeout. + // + // This is the injection point for anything deployment-specific: a + // transport that reads from local disk for an air-gapped install, one that + // adds credentials, or one that applies retries. Keeping it an + // http.Client rather than growing options for each concern is what keeps + // this type usable outside PPM. + Client *http.Client + + // UserAgent is sent with each request. Empty means Go's default. + UserAgent string + + // Origin labels which index answered, surfaced as PackageMetadata.Origin. + // Defaults to BaseURL. + Origin string + + // MaxCacheEntries and MaxCacheBytes bound the cache. Zero means the + // corresponding default; negative means unbounded on that axis. + MaxCacheEntries int + MaxCacheBytes int64 +} + +// CachedJSONIndex serves DistFile lists from the per-snapshot index JSON, with +// a bounded in-memory cache keyed by (package, snapshot). +// +// # What it does and does not serve +// +// Files is the point of this type. Versions is also served, because the same +// document lists them and answering costs nothing extra. +// +// Metadata deliberately ALWAYS returns ErrMetadataUnavailable. Dependency +// metadata must come from the resident RSF, not from here: RFD 0001 Rev 15 +// reversed the carrier for the dependency fieldset precisely so resolution +// works air-gapped, and serving requires_dist from a CDN document would +// quietly reintroduce a per-package network fetch on the resolution hot path. +// Refusing is what keeps that regression from being one convenient edit away. +// Compose this with an RSF-backed index rather than using it alone. +// +// # Cache soundness +// +// An entry is only sound to cache if its key names immutable content. A +// snapshot identifier does name immutable content -- that is what a snapshot +// is -- with ONE exception: per RFD Section 5.1, a file's yanked status is +// mutable within a published snapshot. That is not a flaw in this key, it is +// an accepted property of the data, and invalidating on a yank event is +// tracked separately as rstudio/package-manager#18650. Until then a yank can +// take up to one eviction to become visible. +type CachedJSONIndex struct { + baseURL string + snapshot string + client *http.Client + userAgent string + origin string + + cache *boundedCache[map[string][]DistFile] +} + +// NewCachedJSONIndex validates cfg and returns a CachedJSONIndex. +func NewCachedJSONIndex(cfg CachedJSONConfig) (*CachedJSONIndex, error) { + if cfg.BaseURL == "" { + return nil, errors.New("index: CachedJSONConfig.BaseURL is required") + } + if _, err := url.Parse(cfg.BaseURL); err != nil { + return nil, fmt.Errorf("index: CachedJSONConfig.BaseURL %q: %w", cfg.BaseURL, err) + } + if cfg.Snapshot == "" { + return nil, errors.New("index: CachedJSONConfig.Snapshot is required") + } + + client := cfg.Client + if client == nil { + client = &http.Client{Timeout: 30 * time.Second} + } + + origin := cfg.Origin + if origin == "" { + origin = cfg.BaseURL + } + + maxEntries := cfg.MaxCacheEntries + if maxEntries == 0 { + maxEntries = DefaultMaxCacheEntries + } else if maxEntries < 0 { + maxEntries = 0 // unbounded on this axis + } + + maxBytes := cfg.MaxCacheBytes + if maxBytes == 0 { + maxBytes = DefaultMaxCacheBytes + } else if maxBytes < 0 { + maxBytes = 0 + } + + return &CachedJSONIndex{ + baseURL: strings.TrimSuffix(cfg.BaseURL, "/"), + snapshot: cfg.Snapshot, + client: client, + userAgent: cfg.UserAgent, + origin: origin, + cache: newBoundedCache(maxEntries, maxBytes, sizeOfReleases), + }, nil +} + +// documentURL builds the request URL for one package. +func (c *CachedJSONIndex) documentURL(pkg PackageName) string { + return c.baseURL + "/" + IndexJSONFolder + "/" + url.PathEscape(pkg.String()) + "/" + url.PathEscape(c.snapshot) + ".json" +} + +// cacheKey names the (package, snapshot) pair. +func (c *CachedJSONIndex) cacheKey(pkg PackageName) string { + return pkg.String() + "@" + c.snapshot +} + +// indexDocument is the subset of the per-snapshot document this type reads. +// +// Deliberately narrow. The producer's record carries store-side concerns -- +// blocking rules, vulnerability lists, download counts -- that a resolver has +// no use for, and unmarshalling them would cost allocation per file on the +// resolution path for fields nothing reads. +type indexDocument struct { + Releases map[string][]distributionRecord `json:"releases"` +} + +type distributionRecord struct { + Filename string `json:"filename"` + URL string `json:"url"` + PackageType string `json:"packagetype"` + Size int64 `json:"size"` + Digests map[string]string `json:"digests"` + UploadTimeISO8601 string `json:"upload_time_iso_8601"` + RequiresPython string `json:"requires_python"` + Yanked bool `json:"yanked"` + YankedReason string `json:"yanked_reason"` +} + +// Versions implements MetadataIndex, reading the version list from the same +// document Files uses. +func (c *CachedJSONIndex) Versions(ctx context.Context, pkg PackageName) ([]version.Version, error) { + byVersion, err := c.releases(ctx, pkg) + if err != nil { + return nil, err + } + + out := make([]version.Version, 0, len(byVersion)) + for raw := range byVersion { + v, err := version.Parse(raw) + if err != nil { + // A version key the PEP 440 parser rejects is skipped rather than + // failing the package. Third-party indexes do publish + // non-conforming keys, and one bad key must not make every other + // version of the package unreachable. + continue + } + out = append(out, v) + } + + return out, nil +} + +// Metadata implements MetadataIndex by always reporting +// ErrMetadataUnavailable. See the type documentation: dependency metadata +// belongs to the resident RSF, and serving it from a CDN document would +// reintroduce a per-package network fetch on the resolution path and break +// air-gapped resolution. +func (c *CachedJSONIndex) Metadata(_ context.Context, pkg PackageName, ver version.Version) (PackageMetadata, error) { + return PackageMetadata{}, fmt.Errorf( + "index %q: %q %s: dependency metadata is served from the resident RSF, not the index JSON: %w", + c.origin, pkg, ver, ErrMetadataUnavailable) +} + +// Files implements MetadataIndex. +func (c *CachedJSONIndex) Files(ctx context.Context, pkg PackageName, ver version.Version) ([]DistFile, error) { + byVersion, err := c.releases(ctx, pkg) + if err != nil { + return nil, err + } + + files, ok := byVersion[ver.String()] + if !ok { + // Fall back to a normalized comparison: the document's keys are + // whatever the publisher wrote, so "1.0" and "1.0.0" can both appear + // and neither is wrong. + for raw, candidate := range byVersion { + parsed, parseErr := version.Parse(raw) + if parseErr == nil && parsed.Equal(ver) { + files = candidate + ok = true + break + } + } + } + if !ok { + return nil, fmt.Errorf("index %q: %q %s: %w", c.origin, pkg, ver, ErrPackageNotFound) + } + + // Copy before returning. The cache shares one slice with every reader, and + // a consumer is entitled to sort what it is handed -- candidate selection + // does exactly that. + return append([]DistFile(nil), files...), nil +} + +// releases returns the cached per-version file lists for pkg. +// +// The returned map is shared with the cache and MUST NOT be mutated or handed +// to a caller; the exported methods copy what they return out of it. +func (c *CachedJSONIndex) releases(ctx context.Context, pkg PackageName) (map[string][]DistFile, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + return c.cache.get(c.cacheKey(pkg), func() (map[string][]DistFile, error) { + return c.fetch(ctx, pkg) + }) +} + +// fetch retrieves and decodes the per-snapshot document for pkg. +func (c *CachedJSONIndex) fetch(ctx context.Context, pkg PackageName) (map[string][]DistFile, error) { + target := c.documentURL(pkg) + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + if err != nil { + return nil, fmt.Errorf("index %q: building request for %q: %w", c.origin, pkg, err) + } + if c.userAgent != "" { + req.Header.Set("User-Agent", c.userAgent) + } + // Accept-Encoding is deliberately not set, so net/http negotiates gzip and + // transparently decompresses. Setting it by hand would make this code + // responsible for decompression. + + res, err := c.client.Do(req) + if err != nil { + return nil, fmt.Errorf("index %q: fetching %q: %w", c.origin, target, err) + } + defer func() { _ = res.Body.Close() }() + + switch { + case res.StatusCode == http.StatusNotFound || res.StatusCode == http.StatusGone: + return nil, fmt.Errorf("index %q: %q: %w", c.origin, pkg, ErrPackageNotFound) + case res.StatusCode < 200 || res.StatusCode > 299: + return nil, fmt.Errorf("index %q: fetching %q: unexpected status %s", c.origin, target, res.Status) + } + + body, err := io.ReadAll(res.Body) + if err != nil { + return nil, fmt.Errorf("index %q: reading %q: %w", c.origin, target, err) + } + + var doc indexDocument + if err := json.Unmarshal(body, &doc); err != nil { + return nil, fmt.Errorf("index %q: decoding %q: %w", c.origin, target, err) + } + + out := make(map[string][]DistFile, len(doc.Releases)) + for raw, records := range doc.Releases { + files := make([]DistFile, 0, len(records)) + for _, rec := range records { + files = append(files, rec.toDistFile()) + } + out[raw] = files + } + return out, nil +} + +// toDistFile converts one producer record. +func (r distributionRecord) toDistFile() DistFile { + f := DistFile{ + Filename: r.Filename, + Location: r.URL, + Kind: distKindFromPackageType(r.PackageType, r.Filename), + Size: r.Size, + Yanked: r.Yanked, + YankedReason: r.YankedReason, + } + + if len(r.Digests) > 0 { + f.Hashes = make(map[string]string, len(r.Digests)) + for algo, digest := range r.Digests { + f.Hashes[strings.ToLower(algo)] = strings.ToLower(digest) + } + } + + if r.UploadTimeISO8601 != "" { + // The producer writes RFC 3339 with fractional seconds. Parsing with + // RFC3339 accepts that; a failure leaves the zero time rather than + // rejecting the file, since an unusable timestamp is not a reason to + // hide a distribution that exists. + if t, err := time.Parse(time.RFC3339, r.UploadTimeISO8601); err == nil { + f.UploadTime = t + } + } + + if r.RequiresPython != "" { + // A malformed constraint leaves RequiresPython unset, i.e. + // unconstrained, rather than dropping the file. + // + // This is the lenient direction, chosen because this type is a + // data-fidelity layer: dropping the only wheel of a package because + // its metadata is malformed makes the package unresolvable, which is a + // worse and much harder-to-diagnose outcome than surfacing a file + // whose interpreter constraint could not be read. Enforcing a stricter + // policy is candidate selection's job, which can see all the files at + // once and choose among them. + if specs, err := version.NewSpecifiers(r.RequiresPython); err == nil { + f.RequiresPython = specs + } + } + + return f +} + +// distKindFromPackageType maps the producer's packagetype to a DistKind. +// +// packagetype is authoritative when present -- it comes straight from the +// upstream index -- and the filename is only a fallback for a record that +// omits it. +func distKindFromPackageType(packageType, filename string) DistKind { + switch strings.ToLower(packageType) { + case "bdist_wheel": + return DistKindWheel + case "sdist": + return DistKindSDist + } + + switch { + case strings.HasSuffix(strings.ToLower(filename), ".whl"): + return DistKindWheel + case strings.HasSuffix(strings.ToLower(filename), ".tar.gz"), + strings.HasSuffix(strings.ToLower(filename), ".zip"), + strings.HasSuffix(strings.ToLower(filename), ".tar.bz2"): + return DistKindSDist + } + + return DistKindUnknown +} + +// sizeOfReleases approximates the retained size of one cache entry. +// +// Approximate is sufficient and honest: the budget exists to bound growth, and +// spending real time measuring an exact figure would cost more than the +// precision is worth. The constant covers the fixed part of the struct. +func sizeOfReleases(byVersion map[string][]DistFile) int64 { + const ( + perVersionOverhead = 64 + perFileOverhead = 256 + ) + + var total int64 + for rawVersion, files := range byVersion { + total += perVersionOverhead + int64(len(rawVersion)) + for _, f := range files { + total += perFileOverhead + total += int64(len(f.Filename) + len(f.Location) + len(f.YankedReason)) + for algo, digest := range f.Hashes { + total += int64(len(algo) + len(digest)) + } + } + } + return total +} + +// Compile-time assertion that CachedJSONIndex satisfies the interface. +var _ MetadataIndex = (*CachedJSONIndex)(nil) diff --git a/index/cachedjson_test.go b/index/cachedjson_test.go new file mode 100644 index 0000000..db83176 --- /dev/null +++ b/index/cachedjson_test.go @@ -0,0 +1,700 @@ +// SPDX-License-Identifier: Apache-2.0 OR MIT + +package index + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "sort" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/posit-dev/go-python-packaging/version" +) + +// clickDocument is a trimmed excerpt of a real per-snapshot document, taken +// from PPM's own fixture at +// test/bats/localpython_e2e/pypi/index_json_v2/click/20210710T082015.json. +// +// Using the real producer's bytes rather than a hand-written approximation is +// deliberate: the field spellings (upload_time_iso_8601, packagetype, digests +// as a map) and the JSON null in yanked_reason are exactly the details a +// hand-rolled fixture gets subtly wrong. +const clickDocument = `{ + "info": {"name": "click", "version": "7.1.2"}, + "last_serial": 7175136, + "releases": { + "7.1.2": [ + { + "comment_text": "", + "digests": { + "md5": "B4233221CACC473ACD422A1D54FF4C41", + "sha256": "DACCA89F4BFADD5DE3D7489B7C8A566EEE0D3676333FBB50030263894C38C0DC" + }, + "downloads": -1, + "filename": "click-7.1.2-py2.py3-none-any.whl", + "has_sig": true, + "md5_digest": "b4233221cacc473acd422a1d54ff4c41", + "packagetype": "bdist_wheel", + "python_version": "py2.py3", + "requires_python": ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*", + "size": 82780, + "upload_time": "2020-04-27T20:22:42", + "upload_time_iso_8601": "2020-04-27T20:22:42.629571Z", + "url": "https://files.pythonhosted.org/packages/d2/3d/click-7.1.2-py2.py3-none-any.whl", + "yanked": false, + "yanked_reason": null + }, + { + "comment_text": "", + "digests": {"md5": "53692f62cb99a1a10c59248f1776d9c0", "sha256": "d2b5255c7c6349bc1bd1e59e08cd12acbbd63ce649f2588755783aa94dfb6b1a"}, + "downloads": -1, + "filename": "click-7.1.2.tar.gz", + "has_sig": true, + "packagetype": "sdist", + "python_version": "source", + "requires_python": ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*", + "size": 297279, + "upload_time_iso_8601": "2020-04-27T20:22:45.014623Z", + "url": "https://files.pythonhosted.org/packages/27/6f/click-7.1.2.tar.gz", + "yanked": false, + "yanked_reason": null + } + ], + "0.1": [ + { + "digests": {"sha256": "aaa"}, + "filename": "click-0.1-py2.py3-none-any.whl", + "packagetype": "bdist_wheel", + "size": 100, + "upload_time_iso_8601": "2014-01-01T00:00:00Z", + "url": "https://files.pythonhosted.org/packages/aa/click-0.1-py2.py3-none-any.whl", + "yanked": true, + "yanked_reason": "friends with Mitch McConnell - yanked file" + } + ] + } +}` + +// newTestIndex serves the given body for every request and returns the index +// plus a counter of requests actually made. +func newTestIndex(t *testing.T, body string, cfg CachedJSONConfig) (*CachedJSONIndex, *atomic.Int64, *httptest.Server) { + t.Helper() + + var requests atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + + cfg.BaseURL = srv.URL + if cfg.Snapshot == "" { + cfg.Snapshot = "20210710T082015" + } + cfg.Client = srv.Client() + + idx, err := NewCachedJSONIndex(cfg) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + return idx, &requests, srv +} + +func TestCachedJSONIndexParsesRealProducerDocument(t *testing.T) { + idx, _, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + + files, err := idx.Files(context.Background(), NewPackageName("click"), mustVersion(t, "7.1.2")) + if err != nil { + t.Fatalf("Files: %v", err) + } + if len(files) != 2 { + t.Fatalf("got %d files, want 2 (one wheel, one sdist)", len(files)) + } + + sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename }) + wheel, sdist := files[0], files[1] + + if wheel.Filename != "click-7.1.2-py2.py3-none-any.whl" || !wheel.IsWheel() { + t.Errorf("wheel = %q kind=%v, want the .whl classified as a wheel", wheel.Filename, wheel.Kind) + } + if sdist.Kind != DistKindSDist { + t.Errorf("sdist kind = %v, want DistKindSDist", sdist.Kind) + } + if wheel.Location != "https://files.pythonhosted.org/packages/d2/3d/click-7.1.2-py2.py3-none-any.whl" { + t.Errorf("Location = %q", wheel.Location) + } + if wheel.Size != 82780 { + t.Errorf("Size = %d, want 82780", wheel.Size) + } + + // Digests arrive as a map and are lowercased on both sides, so a + // case-varying producer cannot produce two spellings of one digest. + if got := wheel.Hashes["sha256"]; got != "dacca89f4bfadd5de3d7489b7c8a566eee0d3676333fbb50030263894c38c0dc" { + t.Errorf("sha256 = %q, want it lowercased", got) + } + if _, ok := wheel.Hashes["md5"]; !ok { + t.Error("md5 digest missing; all published digests should be carried") + } + + // upload_time_iso_8601 carries fractional seconds. + want := time.Date(2020, 4, 27, 20, 22, 42, 629571000, time.UTC) + if !wheel.UploadTime.Equal(want) { + t.Errorf("UploadTime = %v, want %v", wheel.UploadTime, want) + } + + // A comma-separated requires_python must parse as a specifier set. + if wheel.RequiresPython.String() == "" { + t.Error("RequiresPython did not parse; the producer publishes a comma-separated set here") + } + if wheel.RequiresPython.Check(mustVersion(t, "3.0.1")) { + t.Error("requires_python excludes 3.0.*, so 3.0.1 must not satisfy it") + } + if !wheel.RequiresPython.Check(mustVersion(t, "3.9")) { + t.Error("requires_python should admit 3.9") + } +} + +// TestCachedJSONIndexYankedFields covers the one mutable field in an otherwise +// immutable snapshot, and the JSON null the producer writes for absent reasons. +func TestCachedJSONIndexYankedFields(t *testing.T) { + idx, _, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + ctx := context.Background() + + yanked, err := idx.Files(ctx, NewPackageName("click"), mustVersion(t, "0.1")) + if err != nil { + t.Fatalf("Files: %v", err) + } + if len(yanked) != 1 { + t.Fatalf("got %d files, want 1", len(yanked)) + } + if !yanked[0].Yanked { + t.Error("Yanked = false, want true") + } + if yanked[0].YankedReason != "friends with Mitch McConnell - yanked file" { + t.Errorf("YankedReason = %q", yanked[0].YankedReason) + } + + // "yanked_reason": null must decode to "" rather than failing the document. + live, err := idx.Files(ctx, NewPackageName("click"), mustVersion(t, "7.1.2")) + if err != nil { + t.Fatalf("Files: %v", err) + } + for _, f := range live { + if f.Yanked || f.YankedReason != "" { + t.Errorf("%q: Yanked=%v reason=%q, want false/empty", f.Filename, f.Yanked, f.YankedReason) + } + } +} + +func TestCachedJSONIndexVersions(t *testing.T) { + idx, _, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + + versions, err := idx.Versions(context.Background(), NewPackageName("click")) + if err != nil { + t.Fatalf("Versions: %v", err) + } + + got := make([]string, 0, len(versions)) + for _, v := range versions { + got = append(got, v.String()) + } + sort.Strings(got) + + if len(got) != 2 || got[0] != "0.1" || got[1] != "7.1.2" { + t.Errorf("versions = %v, want [0.1 7.1.2]", got) + } +} + +// TestCachedJSONIndexMetadataAlwaysRefuses pins the deliberate refusal. If this +// ever starts answering, dependency resolution has quietly acquired a +// per-package network fetch and air-gapped resolution is broken. +func TestCachedJSONIndexMetadataAlwaysRefuses(t *testing.T) { + idx, requests, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + + _, err := idx.Metadata(context.Background(), NewPackageName("click"), mustVersion(t, "7.1.2")) + if !errors.Is(err, ErrMetadataUnavailable) { + t.Fatalf("got %v, want ErrMetadataUnavailable", err) + } + if n := requests.Load(); n != 0 { + t.Errorf("Metadata made %d HTTP requests, want 0 -- it must not even try", n) + } +} + +func TestCachedJSONIndexCachesByPackageAndSnapshot(t *testing.T) { + idx, requests, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + ctx := context.Background() + pkg := NewPackageName("click") + + for range 5 { + if _, err := idx.Files(ctx, pkg, mustVersion(t, "7.1.2")); err != nil { + t.Fatalf("Files: %v", err) + } + } + if _, err := idx.Versions(ctx, pkg); err != nil { + t.Fatalf("Versions: %v", err) + } + + if n := requests.Load(); n != 1 { + t.Errorf("made %d requests for one (package, snapshot), want 1", n) + } +} + +// TestCachedJSONIndexSingleflightCoalesces is the -race test the issue calls +// for. Singleflight hands the SAME value to every coalesced waiter, so a +// consumer mutating what it received would corrupt the shared entry. Every +// goroutine here sorts its own result, which is what a real candidate-selection +// layer does. +func TestCachedJSONIndexSingleflightCoalescesAndIsolates(t *testing.T) { + var requests atomic.Int64 + release := make(chan struct{}) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + <-release // hold every request open so the flights overlap + _, _ = w.Write([]byte(clickDocument)) + })) + defer srv.Close() + + idx, err := NewCachedJSONIndex(CachedJSONConfig{ + BaseURL: srv.URL, + Snapshot: "20210710T082015", + Client: srv.Client(), + }) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + + ctx := context.Background() + pkg := NewPackageName("click") + v := mustVersion(t, "7.1.2") + + const goroutines = 24 + var wg sync.WaitGroup + results := make([][]DistFile, goroutines) + + for i := range goroutines { + wg.Add(1) + go func(i int) { + defer wg.Done() + files, err := idx.Files(ctx, pkg, v) + if err != nil { + t.Errorf("Files: %v", err) + return + } + // Mutate: reverse-sort in place. If the cache leaked one shared + // slice, this races with every sibling goroutine and corrupts the + // entry -- exactly the bug -race exists to catch. + sort.Slice(files, func(a, b int) bool { return files[a].Filename > files[b].Filename }) + results[i] = files + }(i) + } + + // Let the goroutines pile into the flight before answering. + time.Sleep(50 * time.Millisecond) + close(release) + wg.Wait() + + if n := requests.Load(); n != 1 { + t.Errorf("made %d requests, want 1 (singleflight should coalesce)", n) + } + + for i, files := range results { + if len(files) != 2 { + t.Fatalf("goroutine %d got %d files, want 2", i, len(files)) + } + } + + // The cached entry must still be intact after all that mutation. + after, err := idx.Files(ctx, pkg, v) + if err != nil { + t.Fatalf("Files after concurrent mutation: %v", err) + } + if len(after) != 2 { + t.Errorf("cached entry now has %d files, want 2", len(after)) + } +} + +// TestCachedJSONIndexReturnsCopies is the non-concurrent statement of the same +// property, so a failure points at the copy rather than at a data race. +func TestCachedJSONIndexReturnsCopies(t *testing.T) { + idx, _, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + ctx := context.Background() + pkg := NewPackageName("click") + v := mustVersion(t, "7.1.2") + + first, err := idx.Files(ctx, pkg, v) + if err != nil { + t.Fatalf("Files: %v", err) + } + first[0].Filename = "mutated.whl" + + second, err := idx.Files(ctx, pkg, v) + if err != nil { + t.Fatalf("Files: %v", err) + } + for _, f := range second { + if f.Filename == "mutated.whl" { + t.Error("mutating a returned DistFile changed the cached entry") + } + } +} + +func TestCachedJSONIndexNotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.NotFound(w, r) + })) + defer srv.Close() + + idx, err := NewCachedJSONIndex(CachedJSONConfig{BaseURL: srv.URL, Snapshot: "s", Client: srv.Client()}) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + + if _, err := idx.Versions(context.Background(), NewPackageName("nope")); !errors.Is(err, ErrPackageNotFound) { + t.Errorf("got %v, want ErrPackageNotFound", err) + } +} + +func TestCachedJSONIndexUnknownVersionIsNotFound(t *testing.T) { + idx, _, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + + _, err := idx.Files(context.Background(), NewPackageName("click"), mustVersion(t, "99.0")) + if !errors.Is(err, ErrPackageNotFound) { + t.Errorf("got %v, want ErrPackageNotFound", err) + } +} + +// TestCachedJSONIndexMatchesEquivalentVersionSpelling covers a document key +// that is PEP 440-equal to the requested version but spelled differently. The +// producer writes whatever the publisher used, and "1.0" vs "1.0.0" are the +// same version. +func TestCachedJSONIndexMatchesEquivalentVersionSpelling(t *testing.T) { + doc := `{"releases": {"1.0": [{"filename": "p-1.0.tar.gz", "packagetype": "sdist", "url": "https://e/p-1.0.tar.gz"}]}}` + idx, _, _ := newTestIndex(t, doc, CachedJSONConfig{}) + + files, err := idx.Files(context.Background(), NewPackageName("p"), mustVersion(t, "1.0.0")) + if err != nil { + t.Fatalf("Files for the equivalent spelling 1.0.0: %v", err) + } + if len(files) != 1 { + t.Fatalf("got %d files, want 1", len(files)) + } +} + +func TestCachedJSONIndexServerErrorIsNotSwallowed(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + idx, err := NewCachedJSONIndex(CachedJSONConfig{BaseURL: srv.URL, Snapshot: "s", Client: srv.Client()}) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + + _, err = idx.Files(context.Background(), NewPackageName("p"), mustVersion(t, "1.0")) + if err == nil { + t.Fatal("expected an error for a 500") + } + // A 5xx must NOT look like "no such package": one is retryable, the other + // tells a resolver to give up on the package. + if errors.Is(err, ErrPackageNotFound) { + t.Error("a 500 must not be reported as ErrPackageNotFound") + } + if !strings.Contains(err.Error(), "500") { + t.Errorf("error should name the status: %v", err) + } +} + +func TestCachedJSONIndexMalformedJSON(t *testing.T) { + idx, _, _ := newTestIndex(t, `{"releases": [`, CachedJSONConfig{}) + + if _, err := idx.Versions(context.Background(), NewPackageName("p")); err == nil { + t.Error("expected an error for malformed JSON") + } +} + +// TestCachedJSONIndexToleratesBadPerFileMetadata pins the lenient direction, +// which is a judgment call worth making explicit: a file whose requires_python +// cannot be parsed is surfaced unconstrained rather than dropped, because +// dropping a package's only wheel makes it unresolvable for a reason nobody can +// see. Stricter policy belongs to candidate selection. +func TestCachedJSONIndexToleratesBadPerFileMetadata(t *testing.T) { + doc := `{"releases": {"1.0": [ + {"filename": "p-1.0-py3-none-any.whl", "packagetype": "bdist_wheel", "url": "https://e/p.whl", + "requires_python": "not a specifier", "upload_time_iso_8601": "never"} + ]}}` + idx, _, _ := newTestIndex(t, doc, CachedJSONConfig{}) + + files, err := idx.Files(context.Background(), NewPackageName("p"), mustVersion(t, "1.0")) + if err != nil { + t.Fatalf("Files: %v", err) + } + if len(files) != 1 { + t.Fatalf("got %d files, want 1 -- a bad field must not drop the file", len(files)) + } + if files[0].RequiresPython.String() != "" { + t.Errorf("RequiresPython = %q, want unset", files[0].RequiresPython) + } + if !files[0].UploadTime.IsZero() { + t.Errorf("UploadTime = %v, want the zero time", files[0].UploadTime) + } +} + +// TestCachedJSONIndexSkipsUnparseableVersionKeys checks one bad version key +// does not hide the rest of the package. +func TestCachedJSONIndexSkipsUnparseableVersionKeys(t *testing.T) { + doc := `{"releases": {"1.0": [], "not-a-version": [], "2.0": []}}` + idx, _, _ := newTestIndex(t, doc, CachedJSONConfig{}) + + versions, err := idx.Versions(context.Background(), NewPackageName("p")) + if err != nil { + t.Fatalf("Versions: %v", err) + } + if len(versions) != 2 { + t.Errorf("got %d versions, want 2 (the unparseable key skipped)", len(versions)) + } +} + +func TestCachedJSONIndexHonorsContextCancellation(t *testing.T) { + idx, requests, _ := newTestIndex(t, clickDocument, CachedJSONConfig{}) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if _, err := idx.Files(ctx, NewPackageName("click"), mustVersion(t, "1.0")); !errors.Is(err, context.Canceled) { + t.Errorf("Files: got %v, want context.Canceled", err) + } + if n := requests.Load(); n != 0 { + t.Errorf("made %d requests on a cancelled context, want 0", n) + } +} + +func TestCachedJSONIndexRequestShape(t *testing.T) { + var gotPath, gotUA string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotUA = r.Header.Get("User-Agent") + _, _ = w.Write([]byte(clickDocument)) + })) + defer srv.Close() + + idx, err := NewCachedJSONIndex(CachedJSONConfig{ + BaseURL: srv.URL + "/", // trailing slash must not double up + Snapshot: "20210710T082015", + Client: srv.Client(), + UserAgent: "test-agent/1.0", + }) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + + if _, err := idx.Versions(context.Background(), NewPackageName("Click")); err != nil { + t.Fatalf("Versions: %v", err) + } + + // The name is normalized before it reaches the URL, and the folder is the + // v2 layout. + want := "/" + IndexJSONFolder + "/click/20210710T082015.json" + if gotPath != want { + t.Errorf("request path = %q, want %q", gotPath, want) + } + if gotUA != "test-agent/1.0" { + t.Errorf("User-Agent = %q, want %q", gotUA, "test-agent/1.0") + } +} + +func TestNewCachedJSONIndexValidation(t *testing.T) { + if _, err := NewCachedJSONIndex(CachedJSONConfig{Snapshot: "s"}); err == nil { + t.Error("expected an error when BaseURL is empty") + } + if _, err := NewCachedJSONIndex(CachedJSONConfig{BaseURL: "https://e"}); err == nil { + t.Error("expected an error when Snapshot is empty") + } +} + +func TestCachedJSONIndexOriginDefaultsToBaseURL(t *testing.T) { + idx, err := NewCachedJSONIndex(CachedJSONConfig{BaseURL: "https://example.com/pypi", Snapshot: "s"}) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + + // Origin surfaces in the refusal message, which is the one place it is + // observable on this type. + _, err = idx.Metadata(context.Background(), NewPackageName("p"), mustVersion(t, "1.0")) + if !strings.Contains(err.Error(), "https://example.com/pypi") { + t.Errorf("error should carry the default Origin: %v", err) + } +} + +func TestDistKindFromPackageType(t *testing.T) { + for _, tc := range []struct { + packageType, filename string + want DistKind + }{ + {"bdist_wheel", "p-1.0-py3-none-any.whl", DistKindWheel}, + {"sdist", "p-1.0.tar.gz", DistKindSDist}, + {"BDIST_WHEEL", "p.whl", DistKindWheel}, + // packagetype is authoritative when present, even against the filename. + {"sdist", "p-1.0-py3-none-any.whl", DistKindSDist}, + // Fall back to the filename only when packagetype is absent. + {"", "p-1.0-py3-none-any.whl", DistKindWheel}, + {"", "p-1.0.tar.gz", DistKindSDist}, + {"", "p-1.0.zip", DistKindSDist}, + {"", "p-1.0.tar.bz2", DistKindSDist}, + {"", "p-1.0.exe", DistKindUnknown}, + {"bdist_egg", "p-1.0.egg", DistKindUnknown}, + } { + if got := distKindFromPackageType(tc.packageType, tc.filename); got != tc.want { + t.Errorf("distKindFromPackageType(%q, %q) = %v, want %v", + tc.packageType, tc.filename, got, tc.want) + } + } +} + +// --- boundedCache --- + +func TestBoundedCacheEvictsByEntryCount(t *testing.T) { + c := newBoundedCache(2, 0, func(v string) int64 { return int64(len(v)) }) + + for _, k := range []string{"a", "b", "c"} { + if _, err := c.get(k, func() (string, error) { return k, nil }); err != nil { + t.Fatalf("get(%q): %v", k, err) + } + } + + if entries, _ := c.stats(); entries != 2 { + t.Errorf("cache holds %d entries, want 2", entries) + } + if _, ok := c.lookup("a"); ok { + t.Error("oldest entry should have been evicted") + } + if _, ok := c.lookup("c"); !ok { + t.Error("newest entry should be present") + } +} + +func TestBoundedCacheEvictsByBytes(t *testing.T) { + c := newBoundedCache(0, 10, func(v string) int64 { return int64(len(v)) }) + + for _, k := range []string{"aaaa", "bbbb", "cccc"} { + if _, err := c.get(k, func() (string, error) { return k, nil }); err != nil { + t.Fatalf("get: %v", err) + } + } + + _, bytes := c.stats() + if bytes > 10 { + t.Errorf("cache holds %d bytes, want <= 10", bytes) + } +} + +// TestBoundedCacheRejectsOversizedEntry pins that an entry larger than the +// whole budget is not cached, rather than evicting everything and still not +// fitting. +func TestBoundedCacheRejectsOversizedEntry(t *testing.T) { + c := newBoundedCache(0, 4, func(v string) int64 { return int64(len(v)) }) + + if _, err := c.get("big", func() (string, error) { return "waytoolong", nil }); err != nil { + t.Fatalf("get: %v", err) + } + + if entries, bytes := c.stats(); entries != 0 || bytes != 0 { + t.Errorf("cache holds %d entries / %d bytes, want 0/0", entries, bytes) + } +} + +func TestBoundedCacheLRUPromotesOnRead(t *testing.T) { + c := newBoundedCache(2, 0, func(v string) int64 { return 1 }) + + for _, k := range []string{"a", "b"} { + if _, err := c.get(k, func() (string, error) { return k, nil }); err != nil { + t.Fatalf("get: %v", err) + } + } + // Touch "a" so "b" becomes least-recently-used. + if _, ok := c.lookup("a"); !ok { + t.Fatal("a should be present") + } + if _, err := c.get("c", func() (string, error) { return "c", nil }); err != nil { + t.Fatalf("get: %v", err) + } + + if _, ok := c.lookup("a"); !ok { + t.Error("a was touched and should have survived") + } + if _, ok := c.lookup("b"); ok { + t.Error("b was least-recently-used and should have been evicted") + } +} + +func TestBoundedCacheBuildErrorIsNotCached(t *testing.T) { + c := newBoundedCache(4, 0, func(v string) int64 { return 1 }) + + var calls atomic.Int64 + build := func() (string, error) { + calls.Add(1) + return "", fmt.Errorf("boom") + } + + for range 3 { + if _, err := c.get("k", build); err == nil { + t.Fatal("expected the build error") + } + } + + if n := calls.Load(); n != 3 { + t.Errorf("build called %d times, want 3 -- a failure must not be cached", n) + } + if entries, _ := c.stats(); entries != 0 { + t.Errorf("cache holds %d entries after failures, want 0", entries) + } +} + +func TestBoundedCacheUnboundedWhenNegative(t *testing.T) { + idx, err := NewCachedJSONIndex(CachedJSONConfig{ + BaseURL: "https://example.com", + Snapshot: "s", + MaxCacheEntries: -1, + MaxCacheBytes: -1, + }) + if err != nil { + t.Fatalf("NewCachedJSONIndex: %v", err) + } + if idx.cache.maxEntries != 0 || idx.cache.maxBytes != 0 { + t.Errorf("negative budgets should map to unbounded (0), got %d/%d", + idx.cache.maxEntries, idx.cache.maxBytes) + } +} + +func TestSizeOfReleasesGrowsWithContent(t *testing.T) { + small := map[string][]DistFile{"1.0": {{Filename: "a.whl"}}} + large := map[string][]DistFile{ + "1.0": {{Filename: "a.whl", Location: strings.Repeat("x", 500), Hashes: map[string]string{"sha256": strings.Repeat("y", 64)}}}, + } + + if sizeOfReleases(large) <= sizeOfReleases(small) { + t.Errorf("sizer must grow with content: small=%d large=%d", + sizeOfReleases(small), sizeOfReleases(large)) + } + if sizeOfReleases(nil) != 0 { + t.Errorf("sizeOfReleases(nil) = %d, want 0", sizeOfReleases(nil)) + } +} + +// TestProducerRequiresPythonFormParses guards that the version library still +// parses the comma-separated form the producer publishes, since the +// requires_python assertions above depend on it. +func TestProducerRequiresPythonFormParses(t *testing.T) { + if _, err := version.NewSpecifiers(">=2.7, !=3.0.*, !=3.1.*"); err != nil { + t.Fatalf("the producer's comma-separated requires_python form must parse: %v", err) + } +} diff --git a/index/doc.go b/index/doc.go index 66c21cf..82229d0 100644 --- a/index/doc.go +++ b/index/doc.go @@ -10,29 +10,31 @@ // That separation is what makes the same resolver usable for connected PPM, // air-gapped PPM, local Python sources, and tests. // -// The interface, its types, and MockIndex are implemented. RSFIndex and -// CachedJSONIndex follow in rstudio/package-manager#18647. -// // # Implementation status // // Per RFD 0001 Section 6: // -// - RSFIndex + CachedJSONIndex: connected PPM. RSFIndex reads both the -// version list and the per-version dependencies from the resident PyPI -// RSF with no CDN call; only file lists go to the CDN, via -// CachedJSONIndex with Ristretto caching. Note the direction here — RFD -// Rev 15 reversed the carrier for the dependency fieldset, so -// requires_dist, requires_python, and provides_extra live IN the RSF. -// Only Files() is CDN-backed. -// - OfflineIndex: air-gapped PPM. Dependencies come from the same resident -// RSF, so Metadata needs no network; file lists come from local files -// pre-warmed by the offline downloader. -// - DBIndex: local Python sources, backed by the pypi_projects table. -// - MockIndex: in-memory, for tests. IMPLEMENTED. -// - FilteredIndex: composable wrapper applying snapshot-date, prerelease, -// and yanked policy. -// - MultiIndex: combines ordered sources. +// - MetadataIndex, its types, MockIndex — IMPLEMENTED here. +// - CachedJSONIndex — IMPLEMENTED here. Serves Files (and Versions) from the +// per-snapshot index JSON, with a bounded cache keyed by +// (package, snapshot). +// - RSFIndex — implemented in PPM, not here. It needs PPM's deps-blob +// decoder and store types, which live in a private repo that imports this +// module; putting it here would invert the dependency. Tracked as +// rstudio/package-manager#19437. +// - OfflineIndex (air-gapped, same resident RSF) and DBIndex (PPM's +// pypi_projects table) belong in PPM for the same reason. DBIndex is the +// clearest case: a public module cannot reach PPM's database. +// - FilteredIndex (snapshot-date, prerelease, and yanked policy) and +// MultiIndex (ordered sources) are generic and belong here, but are out of +// scope for the initial release. +// +// # Where dependency metadata comes from // -// Only RSFIndex, CachedJSONIndex, and MockIndex are in scope for the initial -// release. +// Not from the CDN. RFD Rev 15 reversed the carrier for the dependency +// fieldset, so requires_dist, requires_python, and provides_extra are resident +// IN the PyPI RSF and read in-process. Only Files() is CDN-backed. That +// reversal is what makes air-gapped resolution possible, which is why +// CachedJSONIndex refuses Metadata outright rather than serving it from the +// document it already has in hand. package index