From a71d126239b37760b84b8de66e8b65cdb7496794 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 4 Oct 2026 22:07:21 +1100 Subject: [PATCH] Filter remote object listings by prefix (#129) `GET /api/v2/remotes/{name}/objects` dropped the `prefix` query parameter, so a filtered listing returned the first page of every artifact in the remote. - add a `prefix` argument to `ListArtifacts` that matches path prefixes - pass the `prefix` query parameter through from the objects list handler Reviewed-on: https://git.unkin.net/unkin/artifactapi/pulls/129 Co-authored-by: unkin-agent Co-committed-by: unkin-agent --- internal/api/v2/objects.go | 2 +- internal/api/v2/objects_test.go | 68 ++++++++++++++++++++++++++++++ internal/database/artifacts.go | 9 ++-- internal/database/database_test.go | 52 +++++++++++++++++++++-- 4 files changed, 123 insertions(+), 8 deletions(-) create mode 100644 internal/api/v2/objects_test.go diff --git a/internal/api/v2/objects.go b/internal/api/v2/objects.go index 7d66451..1db69be 100644 --- a/internal/api/v2/objects.go +++ b/internal/api/v2/objects.go @@ -62,7 +62,7 @@ func (h *ObjectsHandler) list(w http.ResponseWriter, r *http.Request) { remoteName := chi.URLParam(r, "name") limit, offset := pageBounds(r) - artifacts, err := h.db.ListArtifacts(r.Context(), remoteName, limit, offset) + artifacts, err := h.db.ListArtifacts(r.Context(), remoteName, r.URL.Query().Get("prefix"), limit, offset) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/internal/api/v2/objects_test.go b/internal/api/v2/objects_test.go new file mode 100644 index 0000000..7208055 --- /dev/null +++ b/internal/api/v2/objects_test.go @@ -0,0 +1,68 @@ +package v2 + +import ( + "context" + "encoding/json" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + + "git.unkin.net/unkin/artifactapi/internal/database" + "git.unkin.net/unkin/artifactapi/pkg/models" +) + +// TestObjectsListPrefix verifies the remote objects listing passes ?prefix= +// through to the database filter. +func TestObjectsListPrefix(t *testing.T) { + if testDSN == "" { + t.Skip("Docker unavailable") + } + ctx := context.Background() + db, err := database.New(testDSN) + if err != nil { + t.Fatal(err) + } + defer db.Close() + + const remote = "generic-objs-prefix" + if err := db.CreateRemote(ctx, &models.Remote{ + Name: remote, PackageType: models.PackageGeneric, RepoType: models.RepoTypeRemote, + BaseURL: "https://example.com", MutableTTL: 3600, + }); err != nil { + t.Fatal(err) + } + const hash = "sha256:bb22" + if err := db.UpsertBlob(ctx, hash, "blobs/bb/22", 10, "text/plain"); err != nil { + t.Fatal(err) + } + for _, p := range []string{"a/one.txt", "b/two.txt"} { + if err := db.UpsertArtifact(ctx, remote, p, hash, ""); err != nil { + t.Fatal(err) + } + } + + router := chi.NewRouter() + router.Mount("/remotes/{name}/objects", NewObjectsHandler(db).Routes()) + + list := func(query string) []models.Artifact { + t.Helper() + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest("GET", "/remotes/"+remote+"/objects"+query, nil)) + if w.Code != 200 { + t.Fatalf("list%s = %d, want 200", query, w.Code) + } + var got []models.Artifact + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + return got + } + + if got := list(""); len(got) != 2 { + t.Fatalf("unfiltered listing returned %d objects, want 2", len(got)) + } + if got := list("?prefix=b/"); len(got) != 1 || got[0].Path != "b/two.txt" { + t.Fatalf("prefix=b/ listing = %+v, want only b/two.txt", got) + } +} diff --git a/internal/database/artifacts.go b/internal/database/artifacts.go index da24e46..c16269f 100644 --- a/internal/database/artifacts.go +++ b/internal/database/artifacts.go @@ -65,7 +65,8 @@ func (db *DB) TouchArtifactAccess(ctx context.Context, remoteName, path string) return err } -func (db *DB) ListArtifacts(ctx context.Context, remoteName string, limit, offset int) ([]models.Artifact, error) { +// ListArtifacts pages a remote's artifacts whose path starts with prefix ("" lists all). +func (db *DB) ListArtifacts(ctx context.Context, remoteName, prefix string, limit, offset int) ([]models.Artifact, error) { rows, err := db.Pool.Query(ctx, ` SELECT a.id, a.remote_name, a.path, a.content_hash, a.upstream_etag, a.upstream_last_modified, a.first_seen_at, a.last_fetched_at, @@ -73,10 +74,10 @@ func (db *DB) ListArtifacts(ctx context.Context, remoteName string, limit, offse b.size_bytes, b.content_type FROM artifacts a JOIN blobs b ON a.content_hash = b.content_hash - WHERE a.remote_name = $1 + WHERE a.remote_name = $1 AND left(a.path, length($2)) = $2 ORDER BY a.path - LIMIT $2 OFFSET $3 - `, remoteName, limit, offset) + LIMIT $3 OFFSET $4 + `, remoteName, prefix, limit, offset) if err != nil { return nil, err } diff --git a/internal/database/database_test.go b/internal/database/database_test.go index ad1fcf7..2f11049 100644 --- a/internal/database/database_test.go +++ b/internal/database/database_test.go @@ -168,10 +168,17 @@ func TestArtifactsAndBlobs(t *testing.T) { if err := testDB.TouchArtifactAccess(ctx(), "r-art", "path/a.txt"); err != nil { t.Fatal(err) } - arts, err := testDB.ListArtifacts(ctx(), "r-art", 10, 0) - if err != nil || len(arts) != 1 { + if err := testDB.UpsertArtifact(ctx(), "r-art", "other/b.txt", hash, ""); err != nil { + t.Fatal(err) + } + arts, err := testDB.ListArtifacts(ctx(), "r-art", "", 10, 0) + if err != nil || len(arts) != 2 { t.Fatalf("list artifacts: %v %v", len(arts), err) } + arts, err = testDB.ListArtifacts(ctx(), "r-art", "path/", 10, 0) + if err != nil || len(arts) != 1 || arts[0].Path != "path/a.txt" { + t.Fatalf("list artifacts with prefix: %+v %v", arts, err) + } if err := testDB.InsertAccessLog(ctx(), "r-art", "path/a.txt", true, 10, 5, "1.2.3.4"); err != nil { t.Fatal(err) } @@ -188,6 +195,45 @@ func TestArtifactsAndBlobs(t *testing.T) { } } +func TestListArtifactsPrefix(t *testing.T) { + requireDB(t) + seedRemote(t, "r-prefix") + seedBlob(t, "prefixhash") + for _, p := range []string{"a%b/y", "a1b/y", "a_b/x", "aXb/x", "pkg/1", "pkg/2", "pkg/3", "pkgx/4"} { + if err := testDB.UpsertArtifact(ctx(), "r-prefix", p, "sha256:prefixhash", ""); err != nil { + t.Fatal(err) + } + } + paths := func(prefix string, limit, offset int) []string { + t.Helper() + arts, err := testDB.ListArtifacts(ctx(), "r-prefix", prefix, limit, offset) + if err != nil { + t.Fatalf("list %q: %v", prefix, err) + } + out := make([]string, len(arts)) + for i, a := range arts { + out[i] = a.Path + } + return out + } + + // LIKE wildcards in the prefix must match literally. + if got := paths("a_b/", 10, 0); len(got) != 1 || got[0] != "a_b/x" { + t.Fatalf("prefix a_b/ = %v, want [a_b/x]", got) + } + if got := paths("a%", 10, 0); len(got) != 1 || got[0] != "a%b/y" { + t.Fatalf("prefix a%% = %v, want [a%%b/y]", got) + } + + // limit/offset page the filtered set, not the whole remote. + if got := paths("pkg/", 2, 0); len(got) != 2 || got[0] != "pkg/1" || got[1] != "pkg/2" { + t.Fatalf("prefix pkg/ page 1 = %v, want [pkg/1 pkg/2]", got) + } + if got := paths("pkg/", 2, 2); len(got) != 1 || got[0] != "pkg/3" { + t.Fatalf("prefix pkg/ page 2 = %v, want [pkg/3]", got) + } +} + func TestOrphanAndColdCleanup(t *testing.T) { requireDB(t) seedBlob(t, "orphanhash") @@ -320,7 +366,7 @@ func TestDatabaseErrorPaths(t *testing.T) { if _, err := bad.ListVirtuals(ctx); err == nil { t.Error("ListVirtuals should error") } - if _, err := bad.ListArtifacts(ctx, "r", 10, 0); err == nil { + if _, err := bad.ListArtifacts(ctx, "r", "", 10, 0); err == nil { t.Error("ListArtifacts should error") } if _, err := bad.ListLocalFiles(ctx, "r", 10, 0); err == nil {