From ac6e1bf79804f0871412a79dd6a898023a57b348 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 18 Oct 2026 21:16:04 +1100 Subject: [PATCH] Key goproxy merge cache on owners and cache module misses --- internal/virtual/goproxy_merger.go | 34 ++++++++++++---- internal/virtual/goproxy_merger_test.go | 54 +++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) diff --git a/internal/virtual/goproxy_merger.go b/internal/virtual/goproxy_merger.go index edb83b9..f502631 100644 --- a/internal/virtual/goproxy_merger.go +++ b/internal/virtual/goproxy_merger.go @@ -4,7 +4,9 @@ import ( "context" "errors" "fmt" + "maps" "net/http" + "slices" "strings" "golang.org/x/mod/module" @@ -25,14 +27,25 @@ type GoMember struct { } // GoDoc is a goproxy virtual's merged @v/list for one module. Owners maps each -// version to the first member listing it. +// version to the first member listing it; missing caches that no member has it. type GoDoc struct { - Body []byte - Owners map[string]string - Latest string + Body []byte + Owners map[string]string + Latest string + missing bool +} + +func (d *GoDoc) index() []byte { + if d.missing { + return nil + } + b := append(slices.Clone(d.Body), 0) + for _, v := range slices.Sorted(maps.Keys(d.Owners)) { + b = append(b, v+"="+d.Owners[v]+"\n"...) + } + return b } -func (d *GoDoc) index() []byte { return d.Body } func (d *GoDoc) files() map[string][]byte { return nil } // MergeGoList unions member @v/list documents, sorted by semver; the first @@ -163,7 +176,11 @@ func (e *Engine) goList(ctx context.Context, virt models.Virtual, escMod string) if err != nil { return nil, err } - return MergeGoList(members) + d, err := MergeGoList(members) + if errors.Is(err, ErrNotFound) { + return &GoDoc{missing: true}, nil + } + return d, err }) if err != nil { return nil, err @@ -171,7 +188,10 @@ func (e *Engine) goList(ctx context.Context, virt models.Virtual, escMod string) if inserted { e.evictOldest(prefix, maxGoModules) } - return repo.(*GoDoc), nil + if d := repo.(*GoDoc); !d.missing { + return d, nil + } + return nil, ErrNotFound } // goProbe returns the first member, in member order, that serves path. diff --git a/internal/virtual/goproxy_merger_test.go b/internal/virtual/goproxy_merger_test.go index 2796aff..0d99d19 100644 --- a/internal/virtual/goproxy_merger_test.go +++ b/internal/virtual/goproxy_merger_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "git.unkin.net/unkin/artifactapi/internal/database" + "git.unkin.net/unkin/artifactapi/internal/testsupport" "git.unkin.net/unkin/artifactapi/pkg/models" ) @@ -117,6 +119,31 @@ func TestGoProxyRedirectsToFirstOwner(t *testing.T) { if n := asked["internal example.unkin.net/!hello/@v/list"]; n != 1 { t.Fatalf("merged list must be cached, internal asked %d times", n) } + if n := asked["goproxy missing.example/mod/@v/list"]; n != 1 { + t.Fatalf("a module no member has must be cached, goproxy asked %d times", n) + } +} + +func TestGoProxyFollowsOwnerChange(t *testing.T) { + docs := map[string]string{"internal example.com/mod/@v/list": "v0.1.0\n"} + e, _ := goEngine(docs, "") + e.mergeTTL = 0 + zip := func() string { + t.Helper() + _, loc, err := e.GoProxy(context.Background(), goVirt(), "example.com/mod/@v/v0.1.0.zip", "") + if err != nil { + t.Fatal(err) + } + return loc + } + if loc := zip(); loc != "/api/v1/local/internal/example.com/mod/@v/v0.1.0.zip" { + t.Fatalf("before move: %q", loc) + } + delete(docs, "internal example.com/mod/@v/list") + docs["goproxy example.com/mod/@v/list"] = "v0.1.0\n" + if loc := zip(); loc != "/api/v1/remote/goproxy/example.com/mod/@v/v0.1.0.zip" { + t.Fatalf("same list, new owner: redirect still %q", loc) + } } func TestGoProxyFailsClosed(t *testing.T) { @@ -127,3 +154,30 @@ func TestGoProxyFailsClosed(t *testing.T) { } } } + +func TestGoProxyLocalMemberNotFound(t *testing.T) { + ctx := context.Background() + dsn, terminate, err := testsupport.StartPostgres(ctx) + if err != nil { + t.Skip("Docker unavailable") + } + defer terminate() + db, err := database.New(dsn) + if err != nil { + t.Fatal(err) + } + defer db.Close() + local := models.Remote{Name: "internal", PackageType: models.PackageGoProxy, RepoType: models.RepoTypeLocal} + if err := db.CreateRemote(ctx, &local); err != nil { + t.Fatal(err) + } + + e := NewEngine(db, nil) + if _, err := e.fetchMemberPath(ctx, local, "example.com/mod/@latest"); !errors.Is(err, ErrNotFound) { + t.Fatalf("local 404 must be ErrNotFound, got %v", err) + } + virt := models.Virtual{Name: "go", PackageType: models.PackageGoProxy, Members: []string{"internal"}} + if _, _, err := e.GoProxy(ctx, virt, "example.com/mod/@latest", ""); !errors.Is(err, ErrNotFound) { + t.Fatalf("module only absent from a local member must be not-found, got %v", err) + } +}