diff --git a/datasets.go b/datasets.go index 8a88371..108e384 100644 --- a/datasets.go +++ b/datasets.go @@ -208,34 +208,37 @@ func getDatasetDownloadURL(server, datasetID, version string, token *StoredToken return &downloadURL, nil } -func getDatasetVersions(server, datasetID string, token *StoredToken) ([]Version, error) { - // Get all datasets +// getDatasetByID fetches the dataset record for the given UUID from the full +// listing (the datasets API has no single-dataset GET). +func getDatasetByID(server, datasetID string, token *StoredToken) (*Dataset, error) { datasets, err := getDatasets(server, token) if err != nil { return nil, fmt.Errorf("failed to get datasets: %w", err) } - // Find the dataset with the matching ID - var targetDataset *Dataset for i := range datasets { if datasets[i].ID == datasetID { - targetDataset = &datasets[i] - break + return &datasets[i], nil } } - if targetDataset == nil { - return nil, fmt.Errorf("dataset with ID %s not found", datasetID) + return nil, fmt.Errorf("dataset with ID %s not found", datasetID) +} + +func getDatasetVersions(server, datasetID string, token *StoredToken) ([]Version, error) { + dataset, err := getDatasetByID(server, datasetID, token) + if err != nil { + return nil, err } - fmt.Printf("DEBUG: Found dataset: %s\n", targetDataset.Name) + fmt.Printf("DEBUG: Found dataset: %s\n", dataset.Name) fmt.Printf("DEBUG: Dataset versions:\n") - for i, version := range targetDataset.Versions { + for i, version := range dataset.Versions { fmt.Printf(" [%d] Version %d, Size: %d, Date: %s, BlobstorePath: %s\n", i, version.Version, version.Size, version.Date.Time.Format(time.RFC3339), version.BlobstorePath) } - return targetDataset.Versions, nil + return dataset.Versions, nil } func getDatasets(server string, token *StoredToken) ([]Dataset, error) { @@ -342,6 +345,16 @@ func downloadDataset(server, datasetIdentifier, version, localPath string) error return err } + // Download URLs are per-file for BlobTree datasets (the server rejects a + // whole-tree URL request with "File path missing for BlobTree"). + dataset, err := getDatasetByID(server, datasetID, token) + if err != nil { + return err + } + if dataset.Type == "BlobTree" { + return fmt.Errorf("dataset '%s' is a BlobTree (a file tree): whole-tree download is not supported yet, and download URLs are per-file", dataset.Name) + } + var versionNumber string var datasetName string @@ -446,6 +459,11 @@ func statusDataset(server, datasetIdentifier, version string) error { return err } + dataset, err := getDatasetByID(server, datasetID, token) + if err != nil { + return err + } + var versionNumber string if version != "" { @@ -485,6 +503,17 @@ func statusDataset(server, datasetIdentifier, version string) error { versionNumber = fmt.Sprintf("%d", latestVersion.Version) } + // Download URLs are per-file for BlobTree datasets (the server rejects a + // whole-tree URL request with "File path missing for BlobTree"), so status + // for those is reported without one. + if dataset.Type == "BlobTree" { + fmt.Printf("Dataset: %s\n", dataset.Name) + fmt.Printf("Version: v%s\n", versionNumber) + fmt.Printf("Type: BlobTree (a file tree; download URLs are per-file)\n") + fmt.Printf("Status: Ready\n") + return nil + } + // Get download URL (but don't download) downloadInfo, err := getDatasetDownloadURL(server, datasetID, versionNumber, token) if err != nil { diff --git a/e2e/dataset_test.go b/e2e/dataset_test.go index ddb2ba4..f598480 100644 --- a/e2e/dataset_test.go +++ b/e2e/dataset_test.go @@ -10,7 +10,9 @@ import ( ) // `jh dataset` — list, and the entity-specific status/download driven off the -// first dataset returned by list. +// first dataset of the relevant type returned by list. Blob and BlobTree +// datasets behave differently (whole-blob download URL vs per-file), so each +// gets its own tests instead of taking whichever type happens to sort first. // TestDatasetList verifies the listing renders (whether empty or not). func TestDatasetList(t *testing.T) { @@ -25,14 +27,14 @@ func TestDatasetList(t *testing.T) { } } -// TestDatasetStatusFirst lists datasets, takes the first, and checks its status -// resolves a version and a download URL. +// TestDatasetStatusFirst lists datasets, takes the first Blob-type one, and +// checks its status resolves a version and a download URL. func TestDatasetStatusFirst(t *testing.T) { requireCreds(t) list := runOK(t, "dataset", "list").combined() - id := firstID(list) + id := firstIDOfType(list, "Blob") if id == "" { - t.Skip("no datasets on this instance to inspect") + t.Skip("no Blob-type datasets on this instance to inspect") } res := runJH(t, "dataset", "status", id) @@ -48,14 +50,15 @@ func TestDatasetStatusFirst(t *testing.T) { } } -// TestDatasetDownloadFirst lists datasets, takes the first, downloads it to a -// temp path, and asserts the file was written and is non-empty. +// TestDatasetDownloadFirst lists datasets, takes the first Blob-type one, +// downloads it to a temp path, and asserts the file was written and is +// non-empty. func TestDatasetDownloadFirst(t *testing.T) { requireCreds(t) list := runOK(t, "dataset", "list").combined() - id := firstID(list) + id := firstIDOfType(list, "Blob") if id == "" { - t.Skip("no datasets on this instance to download") + t.Skip("no Blob-type datasets on this instance to download") } dest := filepath.Join(t.TempDir(), "dataset.bin") @@ -73,3 +76,50 @@ func TestDatasetDownloadFirst(t *testing.T) { t.Errorf("downloaded dataset file is empty: %s", dest) } } + +// TestDatasetStatusFirstBlobTree checks status of a BlobTree dataset succeeds +// without a download URL (BlobTree download URLs are per-file; the server has +// no whole-tree URL to report). +func TestDatasetStatusFirstBlobTree(t *testing.T) { + requireCreds(t) + list := runOK(t, "dataset", "list").combined() + id := firstIDOfType(list, "BlobTree") + if id == "" { + t.Skip("no BlobTree-type datasets on this instance to inspect") + } + + res := runJH(t, "dataset", "status", id) + if res.exitCode != 0 { + skipIfUnsupported(t, res) + t.Fatalf("dataset status %s exited %d\nstderr: %s", id, res.exitCode, res.stderr) + } + out := res.combined() + assertContains(t, out, "Dataset:") + assertContains(t, out, "Version:") + assertContains(t, out, "BlobTree") +} + +// TestDatasetDownloadFirstBlobTree checks a whole-tree download attempt fails +// fast with a clear explanation, not a raw server 400 ("File path missing for +// BlobTree"). +func TestDatasetDownloadFirstBlobTree(t *testing.T) { + requireCreds(t) + list := runOK(t, "dataset", "list").combined() + id := firstIDOfType(list, "BlobTree") + if id == "" { + t.Skip("no BlobTree-type datasets on this instance to download") + } + + dest := filepath.Join(t.TempDir(), "dataset.bin") + res := runJH(t, "dataset", "download", id, dest) + if res.exitCode == 0 { + t.Fatalf("dataset download %s unexpectedly succeeded for a BlobTree dataset:\n%s", + id, truncate(res.combined())) + } + out := res.combined() + assertContains(t, out, "BlobTree") + assertContains(t, out, "not supported") + if _, err := os.Stat(dest); err == nil { + t.Errorf("no file should have been written for a refused BlobTree download: %s", dest) + } +} diff --git a/e2e/helpers_test.go b/e2e/helpers_test.go index 5769fc2..cd732e6 100644 --- a/e2e/helpers_test.go +++ b/e2e/helpers_test.go @@ -75,6 +75,62 @@ func firstID(out string) string { return "" } +// firstIDOfType returns the ID of the first `dataset list` entry whose "Type:" +// field equals dtype exactly (e.g. "Blob" does not match "BlobTree"). Blob and +// BlobTree datasets have different download semantics (whole-blob URL vs +// per-file), so type-specific tests must not depend on which type happens to +// sort first on the instance. +func firstIDOfType(out, dtype string) string { + typeRe := regexp.MustCompile(`(?m)^Type:\s*` + regexp.QuoteMeta(dtype) + `\s*$`) + ids := reIDLine.FindAllStringSubmatchIndex(out, -1) + for i, loc := range ids { + end := len(out) + if i+1 < len(ids) { + end = ids[i+1][0] + } + if typeRe.MatchString(out[loc[0]:end]) { + return out[loc[2]:loc[3]] + } + } + return "" +} + +// TestFirstIDOfType pins the listing-parse behaviour firstIDOfType relies on, +// in particular that "Blob" does not match a "BlobTree" entry. Needs no +// credentials — pure output parsing. +func TestFirstIDOfType(t *testing.T) { + listing := "Found 3 dataset(s):\n" + + "\n" + + "ID: 0ba40730-c30d-460a-b5a4-bc1c2d3b6cbc\n" + + "Name: tree_first\n" + + "Owner: admin (User)\n" + + "Size: 51 bytes\n" + + "Visibility: private\n" + + "Type: BlobTree\n" + + "Version: v1\n" + + "\n" + + "ID: 1a678f8f-a352-4554-a37c-1eee605e8aeb\n" + + "Name: blob_second\n" + + "Type: Blob\n" + + "\n" + + "ID: 2ae31596-ef0c-4998-9362-7eb6b28688d4\n" + + "Name: blob_third\n" + + "Type: Blob\n" + + if got := firstIDOfType(listing, "Blob"); got != "1a678f8f-a352-4554-a37c-1eee605e8aeb" { + t.Errorf("firstIDOfType(Blob) = %q, want the second entry (must not match BlobTree)", got) + } + if got := firstIDOfType(listing, "BlobTree"); got != "0ba40730-c30d-460a-b5a4-bc1c2d3b6cbc" { + t.Errorf("firstIDOfType(BlobTree) = %q, want the first entry", got) + } + if got := firstIDOfType(listing, "Nope"); got != "" { + t.Errorf("firstIDOfType(Nope) = %q, want empty", got) + } + if got := firstIDOfType("No datasets found", "Blob"); got != "" { + t.Errorf("firstIDOfType on empty listing = %q, want empty", got) + } +} + // firstField returns the value of the first ": " line (case-insensitive). func firstField(out, field string) string { re := regexp.MustCompile(`(?mi)^` + regexp.QuoteMeta(field) + `:\s*(.+)$`)