From f6ecb3ba45967403e4950fd0828d2af7be091bc7 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:02:25 +0200 Subject: [PATCH 01/33] implement general usage and command sytax --- sda-admin/go.mod | 15 ++ sda-admin/go.sum | 14 ++ sda-admin/main.go | 460 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 489 insertions(+) create mode 100644 sda-admin/go.mod create mode 100644 sda-admin/go.sum create mode 100644 sda-admin/main.go diff --git a/sda-admin/go.mod b/sda-admin/go.mod new file mode 100644 index 000000000..e8c9d098e --- /dev/null +++ b/sda-admin/go.mod @@ -0,0 +1,15 @@ +module github.com/neicnordic/sensitive-data-archive/sda-admin + +go 1.22.3 + +require ( + github.com/golang-jwt/jwt v3.2.2+incompatible + github.com/stretchr/testify v1.9.0 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/stretchr/objx v0.5.2 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/sda-admin/go.sum b/sda-admin/go.sum new file mode 100644 index 000000000..708180fd4 --- /dev/null +++ b/sda-admin/go.sum @@ -0,0 +1,14 @@ +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY= +github.com/golang-jwt/jwt v3.2.2+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= +github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= +github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= +github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +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/sda-admin/main.go b/sda-admin/main.go new file mode 100644 index 000000000..8ba823b84 --- /dev/null +++ b/sda-admin/main.go @@ -0,0 +1,460 @@ +package main + +import ( + "flag" + "fmt" + "os" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/dataset" + "github.com/neicnordic/sensitive-data-archive/sda-admin/file" + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" + "github.com/neicnordic/sensitive-data-archive/sda-admin/list" +) + +var version = "1.0.0" + +var ( + api_uri string + token string +) + +// Command-line usage +var usage = `USAGE: + sda-admin [-uri URI] [-token TOKEN] [options] + +Commands: + list users List all users + list files [-user USERNAME] List all files associated to the token, optionally for a specified user + file ingest -filepath FILEPATH -user USERNAME + Trigger ingestion of the given file + file accession -filepath FILEPATH -user USERNAME -accession-id accessionID + Assign accession ID to a file + dataset create -dataset-id DATASET_ID accessionID [accessionID ...] + Create a dataset from a list of accession IDs and the dataset ID + dataset release -dataset-id DATASET_ID + Release a dataset for downloading + +Global Options: + -uri URI Set the URI for the API server (optional if API_HOST is set) + -token TOKEN Set the authentication token (optional if ACCESS_TOKEN is set) + +Additional Commands: + help Show this help message + -h, -help Show this help message +` + +func listUsage() { + usageText := ` +List Users: + Usage: sda-admin list users + List all users in the system. + +List Files: + Usage: sda-admin list files [-user USERNAME] + List all files, optionally for a specific user. + +Options: + -user USERNAME (For list files) List files owned by the specified username. + +Use 'sda-admin help list ' for information on a specific list command. +` + fmt.Println(usageText) +} + +func listUsersUsage() { + usageText := `Usage: sda-admin list users + List all users in the system. +` + fmt.Println(usageText) +} + +func listFilesUsage() { + usageText := `Usage: sda-admin list files [-user USERNAME] + List all files, optionally for a specific user + +Options: + -user USERNAME List files owned by the specified username. +` + fmt.Println(usageText) +} + +func fileUsage() { + usageText := ` +Ingest a File: + Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME + Trigger the ingestion of a given file for a specific user. + +Accession a File: + Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID + Assign an accession ID to a file and associate it with a user. + +Common Options for 'file' Commands: + -filepath FILEPATH Specify the path of the file. + -user USERNAME Specify the username associated with the file. + + For 'file accession' additionally: + -accession-id ID Specify the accession ID to assign to the file. + +Use 'sda-admin help file ' for information on a specific command. +` + fmt.Println(usageText) +} + +func fileIngestUsage() { + usageText := `Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME + Trigger ingestion of the given file for a specific user. + +Options: + -filepath FILEPATH Specify the path of the file to ingest. + -user USERNAME Specify the username associated with the file. +` + fmt.Println(usageText) +} + +func fileAccessionUsage() { + usageText := `Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID + Assign accession ID to a file and associate it with a user. + +Options: + -filepath FILEPATH Specify the path of the file to assign the accession ID. + -user USERNAME Specify the username associated with the file. + -accession-id ID Specify the accession ID to assign to the file. +` + fmt.Println(usageText) +} + +func datasetUsage() { + usageText := ` +Create a Dataset: + Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] + Create a dataset from a list of accession IDs and the dataset ID. + +Release a Dataset: + Usage: sda-admin dataset release -dataset-id DATASET_ID + Release a dataset for downloading based on its dataset ID. + +Options: + -dataset-id DATASET_ID Specify the unique identifier for the dataset. + [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. + +Use 'sda-admin help dataset ' for information on a specific dataset command. +` + fmt.Println(usageText) +} + +func datasetCreateUsage() { + usageText := `Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] + Create a dataset from a list of accession IDs and the dataset ID. + +Options: + -dataset-id DATASET_ID Specify the unique identifier for the dataset. + [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. +` + fmt.Println(usageText) +} + +func datasetReleaseUsage() { + usageText := `Usage: sda-admin dataset release -dataset-id DATASET_ID + Release a dataset for downloading based on its dataset ID. + +Options: + -dataset-id DATASET_ID Specify the unique identifier for the dataset to release. +` + fmt.Println(usageText) +} + +func versionUsage() { + usageText := `Usage: sda-admin version + Show the version information for sda-admin. +` + fmt.Println(usageText) +} + +func printVersion() { + fmt.Printf("sda-admin version %s\n", version) +} + +func checkToken(token string) { + if err := helpers.CheckTokenExpiration(token); err != nil { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + os.Exit(1) + } +} + +func parseFlagsAndEnv() { + // Set up flags + flag.StringVar(&api_uri, "uri", "", "Set the URI for the SDA server (optional if API_HOST is set)") + flag.StringVar(&token, "token", "", "Set the authentication token (optional if ACCESS_TOKEN is set)") + + // Custom usage message + flag.Usage = func() { + fmt.Println(usage) + } + + // Parse global flags first + flag.Parse() + + // Check environment variables if flags are not provided + if api_uri == "" { + api_uri = os.Getenv("API_HOST") + if api_uri == "" { + fmt.Println("Error: either -uri must be provided or API_HOST environment variable must be set.") + os.Exit(1) + } + } + + if token == "" { + token = os.Getenv("ACCESS_TOKEN") + if token == "" { + fmt.Println("Error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") + os.Exit(1) + } + } +} + +func handleHelpCommand() { + if flag.NArg() > 1 { + switch flag.Arg(1) { + case "list": + handleHelpList() + case "file": + handleHelpFile() + case "dataset": + handleHelpDataset() + case "version": + versionUsage() + os.Exit(0) + default: + fmt.Printf("Unknown command '%s'.\n", flag.Arg(1)) + flag.Usage() + } + } else { + flag.Usage() + } + os.Exit(0) +} + +func handleHelpList() { + if flag.NArg() == 2 { + listUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "users" { + listUsersUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "files" { + listFilesUsage() + } else { + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + listUsage() + } +} + +func handleHelpFile() { + if flag.NArg() == 2 { + fileUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "ingest" { + fileIngestUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "accession" { + fileAccessionUsage() + } else { + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + fileUsage() + } +} + +func handleHelpDataset() { + if flag.NArg() == 2 { + datasetUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "create" { + datasetCreateUsage() + } else if flag.NArg() > 2 && flag.Arg(2) == "release" { + datasetReleaseUsage() + } else { + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + datasetUsage() + } +} + +func handleListCommand() { + if flag.NArg() < 2 { + fmt.Println("Error: 'list' requires a subcommand (users, files).") + listUsage() + os.Exit(1) + } + switch flag.Arg(1) { + case "users": + checkToken(token) + err := list.ListUsers(api_uri, token) + if err != nil { + fmt.Printf("Error: failed to get users, reason: %v\n", err) + } + case "files": + handleListFilesCommand() + default: + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + listUsage() + os.Exit(1) + } +} + +func handleListFilesCommand() { + listFilesCmd := flag.NewFlagSet("files", flag.ExitOnError) + var username string + listFilesCmd.StringVar(&username, "user", "", "Filter files by username") + listFilesCmd.Parse(flag.Args()[2:]) + checkToken(token) + err := list.ListFiles(api_uri, token, username) + if err != nil { + fmt.Printf("Error: failed to get files, reason: %v\n", err) + } +} + +func handleFileCommand() { + if flag.NArg() < 2 { + fmt.Println("Error: 'file' requires a subcommand (ingest, accession).") + fileUsage() + os.Exit(1) + } + switch flag.Arg(1) { + case "ingest": + handleFileIngestCommand() + case "accession": + handleFileAccessionCommand() + default: + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + fileUsage() + os.Exit(1) + } +} + +func handleFileIngestCommand() { + fileIngestCmd := flag.NewFlagSet("ingest", flag.ExitOnError) + var filepath, username string + fileIngestCmd.StringVar(&filepath, "filepath", "", "Filepath to ingest") + fileIngestCmd.StringVar(&username, "user", "", "Username to associate with the file") + fileIngestCmd.Parse(flag.Args()[2:]) + + if filepath == "" || username == "" { + fmt.Println("Error: both -filepath and -user are required.") + fileIngestUsage() + os.Exit(1) + } + + checkToken(token) + err := file.FileIngest(api_uri, token, username, filepath) + if err != nil { + fmt.Printf("Error: failed to ingest file, reason: %v\n", err) + } else { + fmt.Println("File ingestion triggered successfully.") + } +} + +func handleFileAccessionCommand() { + fileAccessionCmd := flag.NewFlagSet("accession", flag.ExitOnError) + var filepath, username, accessionID string + fileAccessionCmd.StringVar(&filepath, "filepath", "", "Filepath to assign accession ID") + fileAccessionCmd.StringVar(&username, "user", "", "Username to associate with the file") + fileAccessionCmd.StringVar(&accessionID, "accession-id", "", "Accession ID to assign") + fileAccessionCmd.Parse(flag.Args()[2:]) + + if filepath == "" || username == "" || accessionID == "" { + fmt.Println("Error: -filepath, -user, and -accession-id are required.") + fileAccessionUsage() + os.Exit(1) + } + + checkToken(token) + err := file.FileAccession(api_uri, token, username, filepath, accessionID) + if err != nil { + fmt.Printf("Error: failed to assign accession ID to file, reason: %v\n", err) + } else { + fmt.Println("Accession ID assigned to file successfully.") + } +} + +func handleDatasetCommand() { + if flag.NArg() < 2 { + fmt.Println("Error: 'dataset' requires a subcommand (create, release).") + datasetUsage() + os.Exit(1) + } + switch flag.Arg(1) { + case "create": + handleDatasetCreateCommand() + case "release": + handleDatasetReleaseCommand() + default: + fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + datasetUsage() + os.Exit(1) + } +} + +func handleDatasetCreateCommand() { + datasetCreateCmd := flag.NewFlagSet("create", flag.ExitOnError) + var datasetID string + datasetCreateCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to create") + datasetCreateCmd.Parse(flag.Args()[2:]) + accessionIDs := datasetCreateCmd.Args() // Args() returns the non-flag arguments after parsing + + if datasetID == "" || len(accessionIDs) == 0 { + fmt.Println("Error: -dataset-id and at least one accession ID are required.") + datasetCreateUsage() + os.Exit(1) + } + + checkToken(token) + err := dataset.DatasetCreate(api_uri, token, datasetID, accessionIDs) + if err != nil { + fmt.Printf("Error: failed to create dataset, reason: %v\n", err) + } else { + fmt.Println("Dataset created successfully.") + } +} + +func handleDatasetReleaseCommand() { + datasetReleaseCmd := flag.NewFlagSet("release", flag.ExitOnError) + var datasetID string + datasetReleaseCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to release") + datasetReleaseCmd.Parse(flag.Args()[2:]) + + if datasetID == "" { + fmt.Println("Error: -dataset-id is required.") + datasetReleaseUsage() + os.Exit(1) + } + + checkToken(token) + err := dataset.DatasetRelease(api_uri, token, datasetID) + if err != nil { + fmt.Printf("Error: failed to release dataset, reason: %v\n", err) + } else { + fmt.Println("Dataset released successfully.") + } +} + +func main() { + parseFlagsAndEnv() + + if flag.NArg() < 1 { + fmt.Println("Error: a command is required.") + flag.Usage() + os.Exit(1) + } + + switch flag.Arg(0) { + case "help", "-h", "-help": + handleHelpCommand() + case "list": + handleListCommand() + case "file": + handleFileCommand() + case "dataset": + handleDatasetCommand() + case "version": + printVersion() + os.Exit(0) + default: + fmt.Printf("Unknown command '%s'.\n", flag.Arg(0)) + flag.Usage() + os.Exit(1) + } +} From 4d5908c5e52138d3fe1895d2fcdbca2d50e0cbd3 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:02:56 +0200 Subject: [PATCH 02/33] implement list command --- sda-admin/list/list.go | 41 ++++++++++++++++ sda-admin/list/list_test.go | 93 +++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+) create mode 100644 sda-admin/list/list.go create mode 100644 sda-admin/list/list_test.go diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go new file mode 100644 index 000000000..7769f89c7 --- /dev/null +++ b/sda-admin/list/list.go @@ -0,0 +1,41 @@ +package list + +import ( + "fmt" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" +) + +// ListUsers returns all users +func ListUsers(api_uri, token string) error { + + url := api_uri + "/users" + response, err := helpers.GetResponseBody(url, token) + if err != nil { + return err + } + + fmt.Println(string(response)) + + return nil +} + +// ListFiles returns all files +func ListFiles(api_uri, token, username string) error { + var url string + + if username == "" { + url = api_uri + "/files" + } else { + url = fmt.Sprintf("%s/users/%s/files", api_uri, username) + } + + response, err := helpers.GetResponseBody(url, token) + if err != nil { + return err + } + + fmt.Println(string(response)) + + return nil +} diff --git a/sda-admin/list/list_test.go b/sda-admin/list/list_test.go new file mode 100644 index 000000000..1276a408a --- /dev/null +++ b/sda-admin/list/list_test.go @@ -0,0 +1,93 @@ +package list + +import ( + "errors" + "testing" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +// MockHelpers is a mock of the helpers package +type MockHelpers struct { + mock.Mock +} + +// Mock the GetResponseBody function +func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { + args := m.Called(url, token) + return args.Get(0).([]byte), args.Error(1) +} + +func TestListUsers_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(`["user1", "user2"]`), nil) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := ListUsers("http://example.com", "test-token") + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestListUsers_Failure(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(nil), errors.New("failed to get users")) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := ListUsers("http://example.com", "test-token") + assert.Error(t, err) + assert.EqualError(t, err, "failed to get users") + mockHelpers.AssertExpectations(t) +} + +func TestListFiles_NoUsername_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := ListFiles("http://example.com", "test-token", "") + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestListFiles_WithUsername_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/users/testuser/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := ListFiles("http://example.com", "test-token", "testuser") + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestListFiles_Failure(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/files", "test-token").Return([]byte(nil), errors.New("failed to get files")) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := ListFiles("http://example.com", "test-token", "") + assert.Error(t, err) + assert.EqualError(t, err, "failed to get files") + mockHelpers.AssertExpectations(t) +} From f3f24cfb56b2ce3d3cf1d3ba4ffd63fc31da4a50 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:03:30 +0200 Subject: [PATCH 03/33] add helping functions --- sda-admin/helpers/helpers.go | 148 ++++++++++++++++++++++++++++++ sda-admin/helpers/helpers_test.go | 109 ++++++++++++++++++++++ 2 files changed, 257 insertions(+) create mode 100644 sda-admin/helpers/helpers.go create mode 100644 sda-admin/helpers/helpers_test.go diff --git a/sda-admin/helpers/helpers.go b/sda-admin/helpers/helpers.go new file mode 100644 index 000000000..c98f07930 --- /dev/null +++ b/sda-admin/helpers/helpers.go @@ -0,0 +1,148 @@ +package helpers + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "strconv" + "time" + + "github.com/golang-jwt/jwt" +) + +// Helper functions used by more than one module + +// CheckTokenExpiration is used to determine whether the token is expiring in less than a day +func CheckTokenExpiration(accessToken string) error { + // Parse jwt token with unverifies, since we don't need to check the signatures here + token, _, err := new(jwt.Parser).ParseUnverified(accessToken, jwt.MapClaims{}) + if err != nil { + return fmt.Errorf("could not parse token, reason: %s", err) + } + + claims, ok := token.Claims.(jwt.MapClaims) + if !ok { + return fmt.Errorf("broken token (claims are empty): %v\nerror: %s", claims, err) + } + + // Check if the token has exp claim + if claims["exp"] == nil { + return fmt.Errorf("could not parse token, reason: no expiration date") + } + + // Parse the expiration date from token, handle cases where + // the date format is nonstandard, e.g. test tokens are used + var expiration time.Time + switch iat := claims["exp"].(type) { + case float64: + expiration = time.Unix(int64(iat), 0) + case json.Number: + tmp, _ := iat.Int64() + expiration = time.Unix(tmp, 0) + case string: + i, err := strconv.ParseInt(iat, 10, 64) + if err != nil { + return fmt.Errorf("could not parse token, reason: %s", err) + } + expiration = time.Unix(int64(i), 0) + default: + return fmt.Errorf("could not parse token, reason: unknown expiration date format") + } + + switch untilExp := time.Until(expiration); { + case untilExp < 0: + return fmt.Errorf("the provided access token has expired, please renew it") + case untilExp > 0 && untilExp < 24*time.Hour: + fmt.Fprintln( + os.Stderr, + "The provided access token expires in", + time.Until(expiration).Truncate(time.Second), + ) + fmt.Fprintln(os.Stderr, "Consider renewing the token.") + default: + fmt.Fprintln( + os.Stderr, + "The provided access token expires in", + time.Until(expiration).Truncate(time.Second), + ) + } + + return nil +} + +// necessary for mocking in unit tests +var GetResponseBody = GetBody + +// GetBody gets the body of the response from the URL +func GetBody(url, token string) ([]byte, error) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create the request, reason: %v", err) + } + + // Add headers + req.Header.Add("Authorization", "Bearer "+token) + req.Header.Add("Content-Type", "application/json") + + // Send the request + client := &http.Client{} + res, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to get response, reason: %v", err) + } + + // Check the status code + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned status %d", res.StatusCode) + } + + // Read the response body + resBody, err := io.ReadAll(res.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body, reason: %v", err) + } + + defer res.Body.Close() + + return resBody, nil +} + +// necessary for mocking in unit tests +var PostRequest = PostReq + +// PostReq sends a POST request to the server with a JSON body and returns the response body or an error. +func PostReq(url, token string, jsonBody []byte) ([]byte, error) { + // Create a new POST request with the provided JSON body + req, err := http.NewRequest("POST", url, bytes.NewBuffer(jsonBody)) + if err != nil { + return nil, fmt.Errorf("failed to create the request, reason: %v", err) + } + + // Add headers + req.Header.Add("Authorization", "Bearer "+token) + req.Header.Add("Content-Type", "application/json") + + // Send the request + client := &http.Client{} + res, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to send request, reason: %v", err) + } + defer res.Body.Close() + + // Check the status code + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned status %d", res.StatusCode) + } + + // Read the response body + resBody, err := io.ReadAll(res.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body, reason: %v", err) + } + + return resBody, nil +} diff --git a/sda-admin/helpers/helpers_test.go b/sda-admin/helpers/helpers_test.go new file mode 100644 index 000000000..fa059aa74 --- /dev/null +++ b/sda-admin/helpers/helpers_test.go @@ -0,0 +1,109 @@ +package helpers + +import ( + "crypto/rand" + "crypto/rsa" + "log" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/golang-jwt/jwt" + "github.com/stretchr/testify/assert" +) + +// generate jwts for testing the expDate +func generateDummyToken(expDate int64) string { + // Generate a new private key + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + log.Fatalf("Failed to generate private key: %s", err) + } + + // Create the Claims + claims := &jwt.StandardClaims{ + Issuer: "test", + } + if expDate != 0 { + claims = &jwt.StandardClaims{ + ExpiresAt: expDate, + Issuer: "test", + } + } + + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + ss, err := token.SignedString(privateKey) + if err != nil { + log.Fatalf("Failed to sign token: %s", err) + } + + return ss +} + +func TestTokenExpiration(t *testing.T) { + // Token without exp claim + token := generateDummyToken(0) + err := CheckTokenExpiration(token) + assert.EqualError(t, err, "could not parse token, reason: no expiration date") + + // Token with expired date + token = generateDummyToken(time.Now().Unix()) + err = CheckTokenExpiration(token) + assert.EqualError(t, err, "the provided access token has expired, please renew it") + + // Token with valid expiration + token = generateDummyToken(time.Now().Add(time.Hour * 72).Unix()) + err = CheckTokenExpiration(token) + assert.NoError(t, err) +} + +func TestGetBody(t *testing.T) { + // Mock server + mockResponse := `{"key": "value"}` + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Write([]byte(mockResponse)) + })) + defer server.Close() + + // Test successful case + body, err := GetBody(server.URL, "mock_token") + assert.NoError(t, err) + assert.JSONEq(t, mockResponse, string(body)) + + // Test non-200 status code + serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusInternalServerError) + })) + defer serverError.Close() + + body, err = GetBody(serverError.URL, "mock_token") + assert.Error(t, err) + assert.Nil(t, body) +} + +func TestPostReq(t *testing.T) { + // Mock server + mockResponse := `{"key": "value"}` + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + assert.Equal(t, "application/json", req.Header.Get("Content-Type")) + assert.Equal(t, "Bearer mock_token", req.Header.Get("Authorization")) + rw.Write([]byte(mockResponse)) + })) + defer server.Close() + + // Test successful case + body, err := PostReq(server.URL, "mock_token", []byte(`{"name":"test"}`)) + assert.NoError(t, err) + assert.JSONEq(t, mockResponse, string(body)) + + // Test non-200 status code + serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.WriteHeader(http.StatusInternalServerError) + })) + defer serverError.Close() + + body, err = PostReq(serverError.URL, "mock_token", []byte(`{"name":"test"}`)) + assert.Error(t, err) + assert.Nil(t, body) +} From 8d285c414d863b519044c08e922dc332f430d15a Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:03:44 +0200 Subject: [PATCH 04/33] implement file commands --- sda-admin/file/file.go | 66 ++++++++++++++++++++++++ sda-admin/file/file_test.go | 100 ++++++++++++++++++++++++++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 sda-admin/file/file.go create mode 100644 sda-admin/file/file_test.go diff --git a/sda-admin/file/file.go b/sda-admin/file/file.go new file mode 100644 index 000000000..b1733acb6 --- /dev/null +++ b/sda-admin/file/file.go @@ -0,0 +1,66 @@ +package file + +import ( + "encoding/json" + "fmt" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" +) + +type RequestBodyFileIngest struct { + Filepath string `json:"filepath"` + User string `json:"user"` +} + +type RequestBodyFileAccession struct { + AccessionID string `json:"accession_id"` + Filepath string `json:"filepath"` + User string `json:"user"` +} + +// FileIngest triggers the ingestion of a given file +func FileIngest(api_uri, token, username, filepath string) error { + + url := api_uri + "/file/ingest" + + requestBody := RequestBodyFileIngest{ + Filepath: filepath, + User: username, + } + + jsonBody, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) + } + + _, err = helpers.PostRequest(url, token, jsonBody) + + if err != nil { + return err + } + return nil +} + +// FileAccession assigns a given file to a given accession ID +func FileAccession(api_uri, token, username, filepath, accessionID string) error { + + url := api_uri + "/file/accession" + + requestBody := RequestBodyFileAccession{ + AccessionID: accessionID, + Filepath: filepath, + User: username, + } + + jsonBody, err := json.Marshal(requestBody) + if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) + } + + _, err = helpers.PostRequest(url, token, jsonBody) + + if err != nil { + return err + } + return nil +} \ No newline at end of file diff --git a/sda-admin/file/file_test.go b/sda-admin/file/file_test.go new file mode 100644 index 000000000..6e97f4e63 --- /dev/null +++ b/sda-admin/file/file_test.go @@ -0,0 +1,100 @@ +package file + +import ( + "errors" + "testing" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +// MockHelpers is a mock implementation of the helpers package functions +type MockHelpers struct { + mock.Mock +} + +func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, error) { + args := m.Called(url, token, jsonBody) + return args.Get(0).([]byte), args.Error(1) +} + +func TestFileIngest_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/file/ingest" + token := "test-token" + username := "test-user" + filepath := "/path/to/file" + jsonBody := []byte(`{"filepath":"/path/to/file","user":"test-user"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) + + err := FileIngest("http://example.com", token, username, filepath) + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestFileIngest_PostRequestFailure(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/file/ingest" + token := "test-token" + username := "test-user" + filepath := "/path/to/file" + jsonBody := []byte(`{"filepath":"/path/to/file","user":"test-user"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) + + err := FileIngest("http://example.com", token, username, filepath) + assert.Error(t, err) + assert.EqualError(t, err, "failed to send request") + mockHelpers.AssertExpectations(t) +} + +func TestFileAccession_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/file/accession" + token := "test-token" + username := "test-user" + filepath := "/path/to/file" + accessionID := "accession-123" + jsonBody := []byte(`{"accession_id":"accession-123","filepath":"/path/to/file","user":"test-user"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) + + err := FileAccession("http://example.com", token, username, filepath, accessionID) + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestFileAccession_PostRequestFailure(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/file/accession" + token := "test-token" + username := "test-user" + filepath := "/path/to/file" + accessionID := "accession-123" + jsonBody := []byte(`{"accession_id":"accession-123","filepath":"/path/to/file","user":"test-user"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) + + err := FileAccession("http://example.com", token, username, filepath, accessionID) + assert.Error(t, err) + assert.EqualError(t, err, "failed to send request") + mockHelpers.AssertExpectations(t) +} From 7bed1673f5ecba7b3cbb163fa3e6ec5a9af68fc9 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:04:00 +0200 Subject: [PATCH 05/33] implement dataset commands --- sda-admin/dataset/dataset.go | 52 +++++++++++++++++ sda-admin/dataset/dataset_test.go | 94 +++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 sda-admin/dataset/dataset.go create mode 100644 sda-admin/dataset/dataset_test.go diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go new file mode 100644 index 000000000..24cf40e38 --- /dev/null +++ b/sda-admin/dataset/dataset.go @@ -0,0 +1,52 @@ +package dataset + +import ( + "encoding/json" + "fmt" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" +) + +type RequestBodyDataset struct { + AccessionIDs []string `json:"accession_ids"` + DatasetID string `json:"dataset_id"` +} + +// DatasetCreate creates a dataset from a list of accession IDs and the dataset ID. +func DatasetCreate(api_uri, token, datasetID string, accessionIDs []string) error { + + url := api_uri + "/dataset/create" + + requestBody := RequestBodyDataset{ + AccessionIDs: accessionIDs, + DatasetID: datasetID, + } + + jsonBody, err := json.Marshal(requestBody) + + if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) + } + + _, err = helpers.PostRequest(url, token, jsonBody) + + if err != nil { + return err + } + return nil +} + +// DatasetRelease releases a dataset for downloading +func DatasetRelease(api_uri, token, datasetID string) error { + + url := api_uri + "/dataset/release/" + datasetID + + jsonBody := []byte("{}") + _, err := helpers.PostRequest(url, token, jsonBody) + + if err != nil { + return err + } + return nil +} + diff --git a/sda-admin/dataset/dataset_test.go b/sda-admin/dataset/dataset_test.go new file mode 100644 index 000000000..a85c4ed46 --- /dev/null +++ b/sda-admin/dataset/dataset_test.go @@ -0,0 +1,94 @@ +package dataset + +import ( + "errors" + "testing" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +// MockHelpers is a mock implementation of the helpers package functions +type MockHelpers struct { + mock.Mock +} + +func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, error) { + args := m.Called(url, token, jsonBody) + return args.Get(0).([]byte), args.Error(1) +} + +func TestDatasetCreate_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/dataset/create" + token := "test-token" + datasetID := "dataset-123" + accessionIDs := []string{"accession-1", "accession-2"} + jsonBody := []byte(`{"accession_ids":["accession-1","accession-2"],"dataset_id":"dataset-123"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) + + err := DatasetCreate("http://example.com", token, datasetID, accessionIDs) + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestDatasetCreate_PostRequestFailure(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/dataset/create" + token := "test-token" + datasetID := "dataset-123" + accessionIDs := []string{"accession-1", "accession-2"} + jsonBody := []byte(`{"accession_ids":["accession-1","accession-2"],"dataset_id":"dataset-123"}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) + + err := DatasetCreate("http://example.com", token, datasetID, accessionIDs) + assert.Error(t, err) + assert.EqualError(t, err, "failed to send request") + mockHelpers.AssertExpectations(t) +} + +func TestDatasetRelease_Success(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/dataset/release/dataset-123" + token := "test-token" + jsonBody := []byte(`{}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) + + err := DatasetRelease("http://example.com", token, "dataset-123") + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + +func TestDatasetRelease_PostRequestFailure(t *testing.T) { + mockHelpers := new(MockHelpers) + originalFunc := helpers.PostRequest + helpers.PostRequest = mockHelpers.PostRequest + defer func() { helpers.PostRequest = originalFunc }() // Restore original after test + + expectedURL := "http://example.com/dataset/release/dataset-123" + token := "test-token" + jsonBody := []byte(`{}`) + + mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) + + err := DatasetRelease("http://example.com", token, "dataset-123") + assert.Error(t, err) + assert.EqualError(t, err, "failed to send request") + mockHelpers.AssertExpectations(t) +} From 4af1f088def9eb118e068de090fa17024c337299 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 16:04:29 +0200 Subject: [PATCH 06/33] add usage instructions --- sda-admin/README.md | 103 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 sda-admin/README.md diff --git a/sda-admin/README.md b/sda-admin/README.md new file mode 100644 index 000000000..8301a10aa --- /dev/null +++ b/sda-admin/README.md @@ -0,0 +1,103 @@ +# sda-admin + +`sda-admin` is a command-line tool for managing sensitive data archives. It provides functionalities to list users and files, ingest and set accession IDs for files, and create or release datasets. + +## General Usage + +```sh +sda-admin [-uri URI] [-token TOKEN] [options] +``` + +## Global Options +- `-uri URI` +Set the URI for the API server (optional if the environmental variable `API_HOST` is set). +- `-token TOKEN` +Set the authentication token (optional if the environmental variable `ACCESS_TOKEN` is set). + +## List all users + +Use the following command to return all users with active uploads as a JSON array +```sh +sda-admin list users +``` + +## List all files, optionally filtered by a specific user. + +Use the following command to return all files belonging to the user associated with the token + +```sh +sda-admin list files +``` + +Use the following command to return all files belonging to the specified user `test@dummy.org` +```sh +sda-admin list files -user test@dummy.org +``` + +## Ingest a file + +Use the following command to trigger the ingesting of a given file `/path/to/file.c4gh` that belongs to the user `test@dummy.org` + +```sh +sda-admin file ingest -filepath /path/to/file.c4gh -user test@dummy.org +``` + +## Assign an accession ID to a file + +Use the following command to assign an accession ID `my-accession-id-1` to a given file `/path/to/file.c4gh` that belongs to the user `test@dummy.org` + +```sh +sda-admin file accession -filepath /path/to/file.c4gh -user test@dummy.org -accession-id my-accession-id-1 +``` + +## Create a dataset from a list of accession IDs and the dataset ID + +Use the following command to create a dataset `dataset001` from accession IDs `my-accession-id-1` and `my-accession-id-2` + +```sh +sda-admin dataset create -dataset-id dataset001 my-accession-id-1 my-accession-id-2 +``` + + +## Release a dataset for downloading + +Use the following command to release the dataset `dataset001` for downloading + +```sh +sda-admin dataset release -dataset-id dataset001 +``` + +## Show version information + +Use the following command to show the version information for sda-admin. + +```sh +sda-admin version +``` + +## Help + +For detailed usage information about specific commands or options, use: + +```sh +sda-admin help +``` + +### Examples + +To get help on the list command: +```sh +sda-admin help list +``` + +To get help on the file ingest command: + +```sh +sda-admin help file ingest +``` + +To get help on the dataset create command: + +```sh +sda-admin help dataset create +``` \ No newline at end of file From e1ae956bf510dd9b4b4cffaf5bcc5ae841a2ebc5 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 3 Sep 2024 19:37:33 +0200 Subject: [PATCH 07/33] improve help message handling --- sda-admin/main.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index 8ba823b84..8ad12efe5 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -194,20 +194,28 @@ func parseFlagsAndEnv() { // Parse global flags first flag.Parse() + // If no command or help command is provided, show usage + if flag.NArg() == 0 || (flag.NArg() == 1 && flag.Arg(0) == "help") { + flag.Usage() + os.Exit(0) + } + // Check environment variables if flags are not provided - if api_uri == "" { - api_uri = os.Getenv("API_HOST") + if flag.Arg(0) != "help" { if api_uri == "" { - fmt.Println("Error: either -uri must be provided or API_HOST environment variable must be set.") - os.Exit(1) + api_uri = os.Getenv("API_HOST") + if api_uri == "" { + fmt.Println("Error: either -uri must be provided or API_HOST environment variable must be set.") + os.Exit(1) + } } - } - if token == "" { - token = os.Getenv("ACCESS_TOKEN") if token == "" { - fmt.Println("Error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") - os.Exit(1) + token = os.Getenv("ACCESS_TOKEN") + if token == "" { + fmt.Println("Error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") + os.Exit(1) + } } } } @@ -434,12 +442,6 @@ func handleDatasetReleaseCommand() { func main() { parseFlagsAndEnv() - if flag.NArg() < 1 { - fmt.Println("Error: a command is required.") - flag.Usage() - os.Exit(1) - } - switch flag.Arg(0) { case "help", "-h", "-help": handleHelpCommand() From ec71cb475404a5b99e500f6fe7fb5c5c24f680ab Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Wed, 4 Sep 2024 15:37:21 +0200 Subject: [PATCH 08/33] user flag is mandatory for the list files command --- sda-admin/list/list.go | 9 +-------- sda-admin/main.go | 22 +++++++++++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go index 7769f89c7..7fa57c7f3 100644 --- a/sda-admin/list/list.go +++ b/sda-admin/list/list.go @@ -22,15 +22,8 @@ func ListUsers(api_uri, token string) error { // ListFiles returns all files func ListFiles(api_uri, token, username string) error { - var url string + response, err := helpers.GetResponseBody(fmt.Sprintf("%s/users/%s/files", api_uri, username), token) - if username == "" { - url = api_uri + "/files" - } else { - url = fmt.Sprintf("%s/users/%s/files", api_uri, username) - } - - response, err := helpers.GetResponseBody(url, token) if err != nil { return err } diff --git a/sda-admin/main.go b/sda-admin/main.go index 8ad12efe5..465e8ea4b 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -24,7 +24,7 @@ var usage = `USAGE: Commands: list users List all users - list files [-user USERNAME] List all files associated to the token, optionally for a specified user + list files -user USERNAME List all files for a specified user file ingest -filepath FILEPATH -user USERNAME Trigger ingestion of the given file file accession -filepath FILEPATH -user USERNAME -accession-id accessionID @@ -50,11 +50,11 @@ List Users: List all users in the system. List Files: - Usage: sda-admin list files [-user USERNAME] - List all files, optionally for a specific user. + Usage: sda-admin list files -user USERNAME + List all files for a specified user. Options: - -user USERNAME (For list files) List files owned by the specified username. + -user USERNAME (For list files) List files owned by the specified user Use 'sda-admin help list ' for information on a specific list command. ` @@ -69,11 +69,11 @@ func listUsersUsage() { } func listFilesUsage() { - usageText := `Usage: sda-admin list files [-user USERNAME] - List all files, optionally for a specific user + usageText := `Usage: sda-admin list files -user USERNAME + List all files for a specified user. Options: - -user USERNAME List files owned by the specified username. + -user USERNAME List files owned by the specified user ` fmt.Println(usageText) } @@ -308,6 +308,14 @@ func handleListFilesCommand() { var username string listFilesCmd.StringVar(&username, "user", "", "Filter files by username") listFilesCmd.Parse(flag.Args()[2:]) + + // Check if the -user flag was provided + if username == "" { + fmt.Println("Error: the -user flag is required.") + listFilesUsage() + os.Exit(1) + } + checkToken(token) err := list.ListFiles(api_uri, token, username) if err != nil { From ccfd3b9b949265acb75108d32941c6f854750168 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 14:15:31 +0000 Subject: [PATCH 09/33] do not use underscore in variable name --- sda-admin/dataset/dataset.go | 8 ++++---- sda-admin/file/file.go | 8 ++++---- sda-admin/list/list.go | 8 ++++---- sda-admin/main.go | 22 +++++++++++----------- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index 24cf40e38..31c5a9f4c 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -13,9 +13,9 @@ type RequestBodyDataset struct { } // DatasetCreate creates a dataset from a list of accession IDs and the dataset ID. -func DatasetCreate(api_uri, token, datasetID string, accessionIDs []string) error { +func DatasetCreate(apiURI, token, datasetID string, accessionIDs []string) error { - url := api_uri + "/dataset/create" + url := apiURI + "/dataset/create" requestBody := RequestBodyDataset{ AccessionIDs: accessionIDs, @@ -37,9 +37,9 @@ func DatasetCreate(api_uri, token, datasetID string, accessionIDs []string) erro } // DatasetRelease releases a dataset for downloading -func DatasetRelease(api_uri, token, datasetID string) error { +func DatasetRelease(apiURI, token, datasetID string) error { - url := api_uri + "/dataset/release/" + datasetID + url := apiURI + "/dataset/release/" + datasetID jsonBody := []byte("{}") _, err := helpers.PostRequest(url, token, jsonBody) diff --git a/sda-admin/file/file.go b/sda-admin/file/file.go index b1733acb6..8ca88b76b 100644 --- a/sda-admin/file/file.go +++ b/sda-admin/file/file.go @@ -19,9 +19,9 @@ type RequestBodyFileAccession struct { } // FileIngest triggers the ingestion of a given file -func FileIngest(api_uri, token, username, filepath string) error { +func FileIngest(apiURI, token, username, filepath string) error { - url := api_uri + "/file/ingest" + url := apiURI + "/file/ingest" requestBody := RequestBodyFileIngest{ Filepath: filepath, @@ -42,9 +42,9 @@ func FileIngest(api_uri, token, username, filepath string) error { } // FileAccession assigns a given file to a given accession ID -func FileAccession(api_uri, token, username, filepath, accessionID string) error { +func FileAccession(apiURI, token, username, filepath, accessionID string) error { - url := api_uri + "/file/accession" + url := apiURI + "/file/accession" requestBody := RequestBodyFileAccession{ AccessionID: accessionID, diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go index 7fa57c7f3..8d01f651a 100644 --- a/sda-admin/list/list.go +++ b/sda-admin/list/list.go @@ -7,9 +7,9 @@ import ( ) // ListUsers returns all users -func ListUsers(api_uri, token string) error { +func ListUsers(apiURI, token string) error { - url := api_uri + "/users" + url := apiURI + "/users" response, err := helpers.GetResponseBody(url, token) if err != nil { return err @@ -21,8 +21,8 @@ func ListUsers(api_uri, token string) error { } // ListFiles returns all files -func ListFiles(api_uri, token, username string) error { - response, err := helpers.GetResponseBody(fmt.Sprintf("%s/users/%s/files", api_uri, username), token) +func ListFiles(apiURI, token, username string) error { + response, err := helpers.GetResponseBody(fmt.Sprintf("%s/users/%s/files", apiURI, username), token) if err != nil { return err diff --git a/sda-admin/main.go b/sda-admin/main.go index 465e8ea4b..b4668ad05 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -14,7 +14,7 @@ import ( var version = "1.0.0" var ( - api_uri string + apiURI string token string ) @@ -183,7 +183,7 @@ func checkToken(token string) { func parseFlagsAndEnv() { // Set up flags - flag.StringVar(&api_uri, "uri", "", "Set the URI for the SDA server (optional if API_HOST is set)") + flag.StringVar(&apiURI, "uri", "", "Set the URI for the SDA server (optional if API_HOST is set)") flag.StringVar(&token, "token", "", "Set the authentication token (optional if ACCESS_TOKEN is set)") // Custom usage message @@ -202,9 +202,9 @@ func parseFlagsAndEnv() { // Check environment variables if flags are not provided if flag.Arg(0) != "help" { - if api_uri == "" { - api_uri = os.Getenv("API_HOST") - if api_uri == "" { + if apiURI == "" { + apiURI = os.Getenv("API_HOST") + if apiURI == "" { fmt.Println("Error: either -uri must be provided or API_HOST environment variable must be set.") os.Exit(1) } @@ -290,7 +290,7 @@ func handleListCommand() { switch flag.Arg(1) { case "users": checkToken(token) - err := list.ListUsers(api_uri, token) + err := list.ListUsers(apiURI, token) if err != nil { fmt.Printf("Error: failed to get users, reason: %v\n", err) } @@ -317,7 +317,7 @@ func handleListFilesCommand() { } checkToken(token) - err := list.ListFiles(api_uri, token, username) + err := list.ListFiles(apiURI, token, username) if err != nil { fmt.Printf("Error: failed to get files, reason: %v\n", err) } @@ -355,7 +355,7 @@ func handleFileIngestCommand() { } checkToken(token) - err := file.FileIngest(api_uri, token, username, filepath) + err := file.FileIngest(apiURI, token, username, filepath) if err != nil { fmt.Printf("Error: failed to ingest file, reason: %v\n", err) } else { @@ -378,7 +378,7 @@ func handleFileAccessionCommand() { } checkToken(token) - err := file.FileAccession(api_uri, token, username, filepath, accessionID) + err := file.FileAccession(apiURI, token, username, filepath, accessionID) if err != nil { fmt.Printf("Error: failed to assign accession ID to file, reason: %v\n", err) } else { @@ -418,7 +418,7 @@ func handleDatasetCreateCommand() { } checkToken(token) - err := dataset.DatasetCreate(api_uri, token, datasetID, accessionIDs) + err := dataset.DatasetCreate(apiURI, token, datasetID, accessionIDs) if err != nil { fmt.Printf("Error: failed to create dataset, reason: %v\n", err) } else { @@ -439,7 +439,7 @@ func handleDatasetReleaseCommand() { } checkToken(token) - err := dataset.DatasetRelease(api_uri, token, datasetID) + err := dataset.DatasetRelease(apiURI, token, datasetID) if err != nil { fmt.Printf("Error: failed to release dataset, reason: %v\n", err) } else { From 9003edc2e110d5df179aa7849aa6ca78ef82496f Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 14:25:55 +0000 Subject: [PATCH 10/33] remove irrelevant unit tests suggested by jocke --- sda-admin/list/list_test.go | 49 ++----------------------------------- 1 file changed, 2 insertions(+), 47 deletions(-) diff --git a/sda-admin/list/list_test.go b/sda-admin/list/list_test.go index 1276a408a..a529c870a 100644 --- a/sda-admin/list/list_test.go +++ b/sda-admin/list/list_test.go @@ -1,7 +1,6 @@ package list import ( - "errors" "testing" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" @@ -20,7 +19,7 @@ func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { return args.Get(0).([]byte), args.Error(1) } -func TestListUsers_Success(t *testing.T) { +func TestListUsers(t *testing.T) { mockHelpers := new(MockHelpers) mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(`["user1", "user2"]`), nil) @@ -34,36 +33,7 @@ func TestListUsers_Success(t *testing.T) { mockHelpers.AssertExpectations(t) } -func TestListUsers_Failure(t *testing.T) { - mockHelpers := new(MockHelpers) - mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(nil), errors.New("failed to get users")) - - // Replace the original GetResponseBody with the mock - originalFunc := helpers.GetResponseBody - defer func() { helpers.GetResponseBody = originalFunc }() - helpers.GetResponseBody = mockHelpers.GetResponseBody - - err := ListUsers("http://example.com", "test-token") - assert.Error(t, err) - assert.EqualError(t, err, "failed to get users") - mockHelpers.AssertExpectations(t) -} - -func TestListFiles_NoUsername_Success(t *testing.T) { - mockHelpers := new(MockHelpers) - mockHelpers.On("GetResponseBody", "http://example.com/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) - - // Replace the original GetResponseBody with the mock - originalFunc := helpers.GetResponseBody - defer func() { helpers.GetResponseBody = originalFunc }() - helpers.GetResponseBody = mockHelpers.GetResponseBody - - err := ListFiles("http://example.com", "test-token", "") - assert.NoError(t, err) - mockHelpers.AssertExpectations(t) -} - -func TestListFiles_WithUsername_Success(t *testing.T) { +func TestListFiles(t *testing.T) { mockHelpers := new(MockHelpers) mockHelpers.On("GetResponseBody", "http://example.com/users/testuser/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) @@ -76,18 +46,3 @@ func TestListFiles_WithUsername_Success(t *testing.T) { assert.NoError(t, err) mockHelpers.AssertExpectations(t) } - -func TestListFiles_Failure(t *testing.T) { - mockHelpers := new(MockHelpers) - mockHelpers.On("GetResponseBody", "http://example.com/files", "test-token").Return([]byte(nil), errors.New("failed to get files")) - - // Replace the original GetResponseBody with the mock - originalFunc := helpers.GetResponseBody - defer func() { helpers.GetResponseBody = originalFunc }() - helpers.GetResponseBody = mockHelpers.GetResponseBody - - err := ListFiles("http://example.com", "test-token", "") - assert.Error(t, err) - assert.EqualError(t, err, "failed to get files") - mockHelpers.AssertExpectations(t) -} From 8c181cb7d215ccfd2b0bf178737c38afee546c81 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 17:36:43 +0000 Subject: [PATCH 11/33] parse url properly and remove prefix in function names --- sda-admin/dataset/dataset.go | 28 +++++++++++++++++++------- sda-admin/file/file.go | 39 ++++++++++++++++++++++++------------ sda-admin/list/list.go | 29 ++++++++++++++++++++------- sda-admin/main.go | 14 ++++++------- 4 files changed, 76 insertions(+), 34 deletions(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index 31c5a9f4c..af70c97ec 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -3,6 +3,7 @@ package dataset import ( "encoding/json" "fmt" + "net/url" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) @@ -12,10 +13,14 @@ type RequestBodyDataset struct { DatasetID string `json:"dataset_id"` } -// DatasetCreate creates a dataset from a list of accession IDs and the dataset ID. -func DatasetCreate(apiURI, token, datasetID string, accessionIDs []string) error { +// Create creates a dataset from a list of accession IDs and the dataset ID. +func Create(apiURI, token, datasetID string, accessionIDs []string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { - url := apiURI + "/dataset/create" + return err + } + parsedURL.Path = fmt.Sprintf("%s/dataset/create", parsedURL.Path) requestBody := RequestBodyDataset{ AccessionIDs: accessionIDs, @@ -25,28 +30,37 @@ func DatasetCreate(apiURI, token, datasetID string, accessionIDs []string) error jsonBody, err := json.Marshal(requestBody) if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) } - _, err = helpers.PostRequest(url, token, jsonBody) + _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { + return err } + return nil } // DatasetRelease releases a dataset for downloading -func DatasetRelease(apiURI, token, datasetID string) error { +func Release(apiURI, token, datasetID string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { - url := apiURI + "/dataset/release/" + datasetID + return err + } + parsedURL.Path = fmt.Sprintf("%s/dataset/release/%s", parsedURL.Path, datasetID) jsonBody := []byte("{}") - _, err := helpers.PostRequest(url, token, jsonBody) + _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { + return err } + return nil } diff --git a/sda-admin/file/file.go b/sda-admin/file/file.go index 8ca88b76b..367dd5830 100644 --- a/sda-admin/file/file.go +++ b/sda-admin/file/file.go @@ -3,6 +3,7 @@ package file import ( "encoding/json" "fmt" + "net/url" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) @@ -18,10 +19,14 @@ type RequestBodyFileAccession struct { User string `json:"user"` } -// FileIngest triggers the ingestion of a given file -func FileIngest(apiURI, token, username, filepath string) error { +// Ingest triggers the ingestion of a given file +func Ingest(apiURI, token, username, filepath string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { - url := apiURI + "/file/ingest" + return err + } + parsedURL.Path = fmt.Sprintf("%s/file/ingest", parsedURL.Path) requestBody := RequestBodyFileIngest{ Filepath: filepath, @@ -30,37 +35,45 @@ func FileIngest(apiURI, token, username, filepath string) error { jsonBody, err := json.Marshal(requestBody) if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) } - - _, err = helpers.PostRequest(url, token, jsonBody) - + _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { + return err } + return nil } -// FileAccession assigns a given file to a given accession ID -func FileAccession(apiURI, token, username, filepath, accessionID string) error { +// Accession assigns a given file to a given accession ID +func Accession(apiURI, token, username, filepath, accessionID string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { - url := apiURI + "/file/accession" + return err + } + parsedURL.Path = fmt.Sprintf("%s/file/accession", parsedURL.Path) requestBody := RequestBodyFileAccession{ AccessionID: accessionID, - Filepath: filepath, - User: username, + Filepath: filepath, + User: username, } jsonBody, err := json.Marshal(requestBody) if err != nil { + return fmt.Errorf("failed to marshal JSON, reason: %v", err) } - _, err = helpers.PostRequest(url, token, jsonBody) + _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { + return err } + return nil -} \ No newline at end of file +} diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go index 8d01f651a..35487da47 100644 --- a/sda-admin/list/list.go +++ b/sda-admin/list/list.go @@ -2,16 +2,24 @@ package list import ( "fmt" + "net/url" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) -// ListUsers returns all users -func ListUsers(apiURI, token string) error { +// Users returns all users +func Users(apiURI, token string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { + + return err + } + parsedURL.Path = fmt.Sprintf("%s/users", parsedURL.Path) + - url := apiURI + "/users" - response, err := helpers.GetResponseBody(url, token) + response, err := helpers.GetResponseBody(parsedURL.String(), token) if err != nil { + return err } @@ -20,11 +28,18 @@ func ListUsers(apiURI, token string) error { return nil } -// ListFiles returns all files -func ListFiles(apiURI, token, username string) error { - response, err := helpers.GetResponseBody(fmt.Sprintf("%s/users/%s/files", apiURI, username), token) +// Files returns all files +func Files(apiURI, token, username string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { + return err + } + parsedURL.Path = fmt.Sprintf("%s/users/%s/files", parsedURL.Path, username) + + response, err := helpers.GetResponseBody(parsedURL.String(), token) if err != nil { + return err } diff --git a/sda-admin/main.go b/sda-admin/main.go index b4668ad05..7aaabe3d2 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -15,7 +15,7 @@ var version = "1.0.0" var ( apiURI string - token string + token string ) // Command-line usage @@ -290,7 +290,7 @@ func handleListCommand() { switch flag.Arg(1) { case "users": checkToken(token) - err := list.ListUsers(apiURI, token) + err := list.Users(apiURI, token) if err != nil { fmt.Printf("Error: failed to get users, reason: %v\n", err) } @@ -317,7 +317,7 @@ func handleListFilesCommand() { } checkToken(token) - err := list.ListFiles(apiURI, token, username) + err := list.Files(apiURI, token, username) if err != nil { fmt.Printf("Error: failed to get files, reason: %v\n", err) } @@ -355,7 +355,7 @@ func handleFileIngestCommand() { } checkToken(token) - err := file.FileIngest(apiURI, token, username, filepath) + err := file.Ingest(apiURI, token, username, filepath) if err != nil { fmt.Printf("Error: failed to ingest file, reason: %v\n", err) } else { @@ -378,7 +378,7 @@ func handleFileAccessionCommand() { } checkToken(token) - err := file.FileAccession(apiURI, token, username, filepath, accessionID) + err := file.Accession(apiURI, token, username, filepath, accessionID) if err != nil { fmt.Printf("Error: failed to assign accession ID to file, reason: %v\n", err) } else { @@ -418,7 +418,7 @@ func handleDatasetCreateCommand() { } checkToken(token) - err := dataset.DatasetCreate(apiURI, token, datasetID, accessionIDs) + err := dataset.Create(apiURI, token, datasetID, accessionIDs) if err != nil { fmt.Printf("Error: failed to create dataset, reason: %v\n", err) } else { @@ -439,7 +439,7 @@ func handleDatasetReleaseCommand() { } checkToken(token) - err := dataset.DatasetRelease(apiURI, token, datasetID) + err := dataset.Release(apiURI, token, datasetID) if err != nil { fmt.Printf("Error: failed to release dataset, reason: %v\n", err) } else { From 42c85f9dfdcda3970dbf408757fa0f815811368b Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 17:45:20 +0000 Subject: [PATCH 12/33] make sure all return statements have a blank line before --- sda-admin/dataset/dataset_test.go | 1 + sda-admin/file/file_test.go | 1 + sda-admin/list/list_test.go | 1 + 3 files changed, 3 insertions(+) diff --git a/sda-admin/dataset/dataset_test.go b/sda-admin/dataset/dataset_test.go index a85c4ed46..5c3fbd78c 100644 --- a/sda-admin/dataset/dataset_test.go +++ b/sda-admin/dataset/dataset_test.go @@ -16,6 +16,7 @@ type MockHelpers struct { func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, error) { args := m.Called(url, token, jsonBody) + return args.Get(0).([]byte), args.Error(1) } diff --git a/sda-admin/file/file_test.go b/sda-admin/file/file_test.go index 6e97f4e63..851b5375b 100644 --- a/sda-admin/file/file_test.go +++ b/sda-admin/file/file_test.go @@ -16,6 +16,7 @@ type MockHelpers struct { func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, error) { args := m.Called(url, token, jsonBody) + return args.Get(0).([]byte), args.Error(1) } diff --git a/sda-admin/list/list_test.go b/sda-admin/list/list_test.go index a529c870a..edeebae6a 100644 --- a/sda-admin/list/list_test.go +++ b/sda-admin/list/list_test.go @@ -16,6 +16,7 @@ type MockHelpers struct { // Mock the GetResponseBody function func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { args := m.Called(url, token) + return args.Get(0).([]byte), args.Error(1) } From f1242e12dd98f92a31e2f433bd9650cd945c10d0 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 20:02:55 +0000 Subject: [PATCH 13/33] sanitize filepath --- sda-admin/helpers/helpers.go | 18 ++++++++++++++++++ sda-admin/helpers/helpers_test.go | 13 +++++++++++++ sda-admin/main.go | 16 ++++++++++++++-- 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/sda-admin/helpers/helpers.go b/sda-admin/helpers/helpers.go index c98f07930..15efe0aaa 100644 --- a/sda-admin/helpers/helpers.go +++ b/sda-admin/helpers/helpers.go @@ -7,6 +7,8 @@ import ( "io" "net/http" "os" + "regexp" + "strings" "strconv" "time" @@ -146,3 +148,19 @@ func PostReq(url, token string, jsonBody []byte) ([]byte, error) { return resBody, nil } + +// Check for invalid characters for filepath +func CheckValidChars(filename string) error { + re := regexp.MustCompile(`[\\<>"\|\x00-\x1F\x7F\!\*\'\(\)\;\:\@\&\=\+\$\,\?\%\#\[\]]`) + disallowedChars := re.FindAllString(filename, -1) + if disallowedChars != nil { + + return fmt.Errorf( + "filepath '%v' contains disallowed characters: %+v", + filename, + strings.Join(disallowedChars, ", "), + ) + } + + return nil +} diff --git a/sda-admin/helpers/helpers_test.go b/sda-admin/helpers/helpers_test.go index fa059aa74..0b2654134 100644 --- a/sda-admin/helpers/helpers_test.go +++ b/sda-admin/helpers/helpers_test.go @@ -3,6 +3,7 @@ package helpers import ( "crypto/rand" "crypto/rsa" + "fmt" "log" "net/http" "net/http/httptest" @@ -107,3 +108,15 @@ func TestPostReq(t *testing.T) { assert.Error(t, err) assert.Nil(t, body) } + +func TestInvalidCharacters(t *testing.T) { + // Test that file paths with invalid characters trigger errors + for _, badc := range "\x00\x7F\x1A:*?\\<>\"|!'();@&=+$,%#[]" { + badchar := string(badc) + testfilepath := "test" + badchar + "file" + + err := CheckValidChars(testfilepath) + assert.Error(t, err) + assert.Equal(t, fmt.Sprintf("filepath '%v' contains disallowed characters: %+v", testfilepath, badchar), err.Error()) + } +} diff --git a/sda-admin/main.go b/sda-admin/main.go index 7aaabe3d2..f382d1cb1 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -354,8 +354,14 @@ func handleFileIngestCommand() { os.Exit(1) } + err := helpers.CheckValidChars(filepath) + if err != nil { + fmt.Println("Error:", err) + os.Exit(1) + } + checkToken(token) - err := file.Ingest(apiURI, token, username, filepath) + err = file.Ingest(apiURI, token, username, filepath) if err != nil { fmt.Printf("Error: failed to ingest file, reason: %v\n", err) } else { @@ -377,8 +383,14 @@ func handleFileAccessionCommand() { os.Exit(1) } + err := helpers.CheckValidChars(filepath) + if err != nil { + fmt.Println("Error:", err) + os.Exit(1) + } + checkToken(token) - err := file.Accession(apiURI, token, username, filepath, accessionID) + err = file.Accession(apiURI, token, username, filepath, accessionID) if err != nil { fmt.Printf("Error: failed to assign accession ID to file, reason: %v\n", err) } else { From a3247fee4ac9d7c66b4e5b199e340112de8e8cee Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Wed, 4 Sep 2024 21:32:55 +0000 Subject: [PATCH 14/33] update readme for listing files --- sda-admin/README.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/sda-admin/README.md b/sda-admin/README.md index 8301a10aa..f8c03fd5b 100644 --- a/sda-admin/README.md +++ b/sda-admin/README.md @@ -21,13 +21,7 @@ Use the following command to return all users with active uploads as a JSON arra sda-admin list users ``` -## List all files, optionally filtered by a specific user. - -Use the following command to return all files belonging to the user associated with the token - -```sh -sda-admin list files -``` +## List all files for a specified user Use the following command to return all files belonging to the specified user `test@dummy.org` ```sh From 5a66fa461ce9556e7403d8c7cd4109830b84d772 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Thu, 5 Sep 2024 13:06:16 +0000 Subject: [PATCH 15/33] fix unit tests after renaming functions --- sda-admin/dataset/dataset_test.go | 16 ++++++++-------- sda-admin/file/file_test.go | 16 ++++++++-------- sda-admin/list/list_test.go | 8 ++++---- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/sda-admin/dataset/dataset_test.go b/sda-admin/dataset/dataset_test.go index 5c3fbd78c..767645cc0 100644 --- a/sda-admin/dataset/dataset_test.go +++ b/sda-admin/dataset/dataset_test.go @@ -20,7 +20,7 @@ func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, e return args.Get(0).([]byte), args.Error(1) } -func TestDatasetCreate_Success(t *testing.T) { +func TestCreate_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -34,12 +34,12 @@ func TestDatasetCreate_Success(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) - err := DatasetCreate("http://example.com", token, datasetID, accessionIDs) + err := Create("http://example.com", token, datasetID, accessionIDs) assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestDatasetCreate_PostRequestFailure(t *testing.T) { +func TestCreate_PostRequestFailure(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -53,13 +53,13 @@ func TestDatasetCreate_PostRequestFailure(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) - err := DatasetCreate("http://example.com", token, datasetID, accessionIDs) + err := Create("http://example.com", token, datasetID, accessionIDs) assert.Error(t, err) assert.EqualError(t, err, "failed to send request") mockHelpers.AssertExpectations(t) } -func TestDatasetRelease_Success(t *testing.T) { +func TestRelease_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -71,12 +71,12 @@ func TestDatasetRelease_Success(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) - err := DatasetRelease("http://example.com", token, "dataset-123") + err := Release("http://example.com", token, "dataset-123") assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestDatasetRelease_PostRequestFailure(t *testing.T) { +func TestRelease_PostRequestFailure(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -88,7 +88,7 @@ func TestDatasetRelease_PostRequestFailure(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) - err := DatasetRelease("http://example.com", token, "dataset-123") + err := Release("http://example.com", token, "dataset-123") assert.Error(t, err) assert.EqualError(t, err, "failed to send request") mockHelpers.AssertExpectations(t) diff --git a/sda-admin/file/file_test.go b/sda-admin/file/file_test.go index 851b5375b..ae442d54e 100644 --- a/sda-admin/file/file_test.go +++ b/sda-admin/file/file_test.go @@ -20,7 +20,7 @@ func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, e return args.Get(0).([]byte), args.Error(1) } -func TestFileIngest_Success(t *testing.T) { +func TestIngest_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -34,12 +34,12 @@ func TestFileIngest_Success(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) - err := FileIngest("http://example.com", token, username, filepath) + err := Ingest("http://example.com", token, username, filepath) assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestFileIngest_PostRequestFailure(t *testing.T) { +func TestIngest_PostRequestFailure(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -53,13 +53,13 @@ func TestFileIngest_PostRequestFailure(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) - err := FileIngest("http://example.com", token, username, filepath) + err := Ingest("http://example.com", token, username, filepath) assert.Error(t, err) assert.EqualError(t, err, "failed to send request") mockHelpers.AssertExpectations(t) } -func TestFileAccession_Success(t *testing.T) { +func TestAccession_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -74,12 +74,12 @@ func TestFileAccession_Success(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) - err := FileAccession("http://example.com", token, username, filepath, accessionID) + err := Accession("http://example.com", token, username, filepath, accessionID) assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestFileAccession_PostRequestFailure(t *testing.T) { +func TestAccession_PostRequestFailure(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -94,7 +94,7 @@ func TestFileAccession_PostRequestFailure(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) - err := FileAccession("http://example.com", token, username, filepath, accessionID) + err := Accession("http://example.com", token, username, filepath, accessionID) assert.Error(t, err) assert.EqualError(t, err, "failed to send request") mockHelpers.AssertExpectations(t) diff --git a/sda-admin/list/list_test.go b/sda-admin/list/list_test.go index edeebae6a..1f1e6a2e9 100644 --- a/sda-admin/list/list_test.go +++ b/sda-admin/list/list_test.go @@ -20,7 +20,7 @@ func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { return args.Get(0).([]byte), args.Error(1) } -func TestListUsers(t *testing.T) { +func TestUsers(t *testing.T) { mockHelpers := new(MockHelpers) mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(`["user1", "user2"]`), nil) @@ -29,12 +29,12 @@ func TestListUsers(t *testing.T) { defer func() { helpers.GetResponseBody = originalFunc }() helpers.GetResponseBody = mockHelpers.GetResponseBody - err := ListUsers("http://example.com", "test-token") + err := Users("http://example.com", "test-token") assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestListFiles(t *testing.T) { +func TestFiles(t *testing.T) { mockHelpers := new(MockHelpers) mockHelpers.On("GetResponseBody", "http://example.com/users/testuser/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) @@ -43,7 +43,7 @@ func TestListFiles(t *testing.T) { defer func() { helpers.GetResponseBody = originalFunc }() helpers.GetResponseBody = mockHelpers.GetResponseBody - err := ListFiles("http://example.com", "test-token", "testuser") + err := Files("http://example.com", "test-token", "testuser") assert.NoError(t, err) mockHelpers.AssertExpectations(t) } From 42146c7dab507f2998fcec726d6d66ad95789582 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Thu, 5 Sep 2024 13:38:45 +0000 Subject: [PATCH 16/33] solve double slash problem in concatenated url using path.Join --- sda-admin/dataset/dataset.go | 14 ++++---------- sda-admin/file/file.go | 13 ++++--------- sda-admin/list/list.go | 10 +++------- 3 files changed, 11 insertions(+), 26 deletions(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index af70c97ec..acdf291b9 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/url" + "path" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) @@ -17,10 +18,9 @@ type RequestBodyDataset struct { func Create(apiURI, token, datasetID string, accessionIDs []string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/dataset/create", parsedURL.Path) + parsedURL.Path = path.Join(parsedURL.Path, "dataset/create") requestBody := RequestBodyDataset{ AccessionIDs: accessionIDs, @@ -28,36 +28,30 @@ func Create(apiURI, token, datasetID string, accessionIDs []string) error { } jsonBody, err := json.Marshal(requestBody) - if err != nil { - return fmt.Errorf("failed to marshal JSON, reason: %v", err) } _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) - if err != nil { - return err } return nil } -// DatasetRelease releases a dataset for downloading +// Release releases a dataset for downloading func Release(apiURI, token, datasetID string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/dataset/release/%s", parsedURL.Path, datasetID) + parsedURL.Path = path.Join(parsedURL.Path, "dataset/release", datasetID) jsonBody := []byte("{}") _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { - return err } diff --git a/sda-admin/file/file.go b/sda-admin/file/file.go index 367dd5830..7d7f06da9 100644 --- a/sda-admin/file/file.go +++ b/sda-admin/file/file.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/url" + "path" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) @@ -23,10 +24,9 @@ type RequestBodyFileAccession struct { func Ingest(apiURI, token, username, filepath string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/file/ingest", parsedURL.Path) + parsedURL.Path = path.Join(parsedURL.Path, "file/ingest") requestBody := RequestBodyFileIngest{ Filepath: filepath, @@ -35,12 +35,11 @@ func Ingest(apiURI, token, username, filepath string) error { jsonBody, err := json.Marshal(requestBody) if err != nil { - return fmt.Errorf("failed to marshal JSON, reason: %v", err) } + _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) if err != nil { - return err } @@ -51,10 +50,9 @@ func Ingest(apiURI, token, username, filepath string) error { func Accession(apiURI, token, username, filepath, accessionID string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/file/accession", parsedURL.Path) + parsedURL.Path = path.Join(parsedURL.Path, "file/accession") requestBody := RequestBodyFileAccession{ AccessionID: accessionID, @@ -64,14 +62,11 @@ func Accession(apiURI, token, username, filepath, accessionID string) error { jsonBody, err := json.Marshal(requestBody) if err != nil { - return fmt.Errorf("failed to marshal JSON, reason: %v", err) } _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) - if err != nil { - return err } diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go index 35487da47..20e3c665d 100644 --- a/sda-admin/list/list.go +++ b/sda-admin/list/list.go @@ -3,6 +3,7 @@ package list import ( "fmt" "net/url" + "path" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" ) @@ -11,15 +12,12 @@ import ( func Users(apiURI, token string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/users", parsedURL.Path) - + parsedURL.Path = path.Join(parsedURL.Path, "users") response, err := helpers.GetResponseBody(parsedURL.String(), token) if err != nil { - return err } @@ -32,14 +30,12 @@ func Users(apiURI, token string) error { func Files(apiURI, token, username string) error { parsedURL, err := url.Parse(apiURI) if err != nil { - return err } - parsedURL.Path = fmt.Sprintf("%s/users/%s/files", parsedURL.Path, username) + parsedURL.Path = path.Join(parsedURL.Path, "users", username, "files") response, err := helpers.GetResponseBody(parsedURL.String(), token) if err != nil { - return err } From 529644e006951c724aac86d8394c90a0510f03ff Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Tue, 10 Sep 2024 09:07:11 +0000 Subject: [PATCH 17/33] implement commets from Malin --- sda-admin/helpers/helpers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sda-admin/helpers/helpers.go b/sda-admin/helpers/helpers.go index 15efe0aaa..464cbce73 100644 --- a/sda-admin/helpers/helpers.go +++ b/sda-admin/helpers/helpers.go @@ -8,8 +8,8 @@ import ( "net/http" "os" "regexp" - "strings" "strconv" + "strings" "time" "github.com/golang-jwt/jwt" @@ -19,7 +19,7 @@ import ( // CheckTokenExpiration is used to determine whether the token is expiring in less than a day func CheckTokenExpiration(accessToken string) error { - // Parse jwt token with unverifies, since we don't need to check the signatures here + // Parse jwt token with unverified, since we don't need to check the signatures here token, _, err := new(jwt.Parser).ParseUnverified(accessToken, jwt.MapClaims{}) if err != nil { return fmt.Errorf("could not parse token, reason: %s", err) @@ -78,7 +78,7 @@ func CheckTokenExpiration(accessToken string) error { // necessary for mocking in unit tests var GetResponseBody = GetBody -// GetBody gets the body of the response from the URL +// GetBody sends a GET request to the given URL and returns the body of the response func GetBody(url, token string) ([]byte, error) { req, err := http.NewRequest("GET", url, nil) if err != nil { From c3bfe4ba851ddfe30d354e15d406be72f037bc7d Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Tue, 10 Sep 2024 13:25:58 +0000 Subject: [PATCH 18/33] refactor: exit only at the main function --- sda-admin/main.go | 369 ++++++++++++++++++++++++++-------------------- 1 file changed, 209 insertions(+), 160 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index f382d1cb1..3f4c2b82f 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -19,7 +19,8 @@ var ( ) // Command-line usage -var usage = `USAGE: +var usage = ` +Usage: sda-admin [-uri URI] [-token TOKEN] [options] Commands: @@ -43,8 +44,7 @@ Additional Commands: -h, -help Show this help message ` -func listUsage() { - usageText := ` +var listUsage = ` List Users: Usage: sda-admin list users List all users in the system. @@ -58,28 +58,21 @@ Options: Use 'sda-admin help list ' for information on a specific list command. ` - fmt.Println(usageText) -} -func listUsersUsage() { - usageText := `Usage: sda-admin list users +var listUsersUsage = ` +Usage: sda-admin list users List all users in the system. ` - fmt.Println(usageText) -} -func listFilesUsage() { - usageText := `Usage: sda-admin list files -user USERNAME +var listFilesUsage = ` +Usage: sda-admin list files -user USERNAME List all files for a specified user. Options: -user USERNAME List files owned by the specified user ` - fmt.Println(usageText) -} -func fileUsage() { - usageText := ` +var fileUsage = ` Ingest a File: Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME Trigger the ingestion of a given file for a specific user. @@ -97,22 +90,18 @@ Common Options for 'file' Commands: Use 'sda-admin help file ' for information on a specific command. ` - fmt.Println(usageText) -} -func fileIngestUsage() { - usageText := `Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME +var fileIngestUsage = ` +Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME Trigger ingestion of the given file for a specific user. Options: -filepath FILEPATH Specify the path of the file to ingest. -user USERNAME Specify the username associated with the file. ` - fmt.Println(usageText) -} -func fileAccessionUsage() { - usageText := `Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID +var fileAccessionUsage = ` +Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID Assign accession ID to a file and associate it with a user. Options: @@ -120,11 +109,8 @@ Options: -user USERNAME Specify the username associated with the file. -accession-id ID Specify the accession ID to assign to the file. ` - fmt.Println(usageText) -} -func datasetUsage() { - usageText := ` +var datasetUsage = ` Create a Dataset: Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] Create a dataset from a list of accession IDs and the dataset ID. @@ -139,65 +125,57 @@ Options: Use 'sda-admin help dataset ' for information on a specific dataset command. ` - fmt.Println(usageText) -} -func datasetCreateUsage() { - usageText := `Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] +var datasetCreateUsage = ` +Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] Create a dataset from a list of accession IDs and the dataset ID. Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset. [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. ` - fmt.Println(usageText) -} -func datasetReleaseUsage() { - usageText := `Usage: sda-admin dataset release -dataset-id DATASET_ID +var datasetReleaseUsage = ` +Usage: sda-admin dataset release -dataset-id DATASET_ID Release a dataset for downloading based on its dataset ID. Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset to release. ` - fmt.Println(usageText) -} -func versionUsage() { - usageText := `Usage: sda-admin version +var versionUsage = ` +Usage: sda-admin version Show the version information for sda-admin. ` - fmt.Println(usageText) -} func printVersion() { fmt.Printf("sda-admin version %s\n", version) } -func checkToken(token string) { +func checkToken(token string) error { if err := helpers.CheckTokenExpiration(token); err != nil { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - os.Exit(1) + return err } + + return nil } -func parseFlagsAndEnv() { +func parseFlagsAndEnv() error { // Set up flags flag.StringVar(&apiURI, "uri", "", "Set the URI for the SDA server (optional if API_HOST is set)") flag.StringVar(&token, "token", "", "Set the authentication token (optional if ACCESS_TOKEN is set)") // Custom usage message flag.Usage = func() { - fmt.Println(usage) + fmt.Fprint(os.Stderr, usage) } // Parse global flags first flag.Parse() - // If no command or help command is provided, show usage - if flag.NArg() == 0 || (flag.NArg() == 1 && flag.Arg(0) == "help") { - flag.Usage() - os.Exit(0) + // If no command is provided, show usage + if flag.NArg() == 0 { + return fmt.Errorf(usage) } // Check environment variables if flags are not provided @@ -205,105 +183,127 @@ func parseFlagsAndEnv() { if apiURI == "" { apiURI = os.Getenv("API_HOST") if apiURI == "" { - fmt.Println("Error: either -uri must be provided or API_HOST environment variable must be set.") - os.Exit(1) + return fmt.Errorf("error: either -uri must be provided or API_HOST environment variable must be set.") } } if token == "" { token = os.Getenv("ACCESS_TOKEN") if token == "" { - fmt.Println("Error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") - os.Exit(1) + return fmt.Errorf("error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") } } } + + return nil } -func handleHelpCommand() { +func handleHelpCommand() error { if flag.NArg() > 1 { switch flag.Arg(1) { case "list": - handleHelpList() + if err := handleHelpList(); err != nil { + return err + } case "file": - handleHelpFile() + if err := handleHelpFile(); err != nil { + return err + } case "dataset": - handleHelpDataset() + if err := handleHelpDataset(); err != nil { + return err + } case "version": - versionUsage() - os.Exit(0) + fmt.Fprint(os.Stderr, versionUsage) default: - fmt.Printf("Unknown command '%s'.\n", flag.Arg(1)) - flag.Usage() + fmt.Fprintf(os.Stderr, "Unknown command '%s'.\n", flag.Arg(1)) + fmt.Fprint(os.Stderr, usage) + return fmt.Errorf("") } } else { - flag.Usage() + fmt.Fprint(os.Stderr, usage) } - os.Exit(0) + + return nil } -func handleHelpList() { +func handleHelpList() error { if flag.NArg() == 2 { - listUsage() + fmt.Fprint(os.Stderr, listUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "users" { - listUsersUsage() + fmt.Fprint(os.Stderr, listUsersUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "files" { - listFilesUsage() + fmt.Fprint(os.Stderr, listFilesUsage) } else { - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - listUsage() + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + fmt.Fprint(os.Stderr, listUsage) + return fmt.Errorf("") } + + return nil } -func handleHelpFile() { +func handleHelpFile() error { if flag.NArg() == 2 { - fileUsage() + fmt.Fprint(os.Stderr, fileUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "ingest" { - fileIngestUsage() + fmt.Fprint(os.Stderr, fileIngestUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "accession" { - fileAccessionUsage() + fmt.Fprint(os.Stderr, fileAccessionUsage) } else { - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - fileUsage() + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + fmt.Fprint(os.Stderr, fileUsage) + return fmt.Errorf("") } + + return nil } -func handleHelpDataset() { +func handleHelpDataset() error { if flag.NArg() == 2 { - datasetUsage() + fmt.Fprint(os.Stderr, datasetUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "create" { - datasetCreateUsage() + fmt.Fprint(os.Stderr, datasetCreateUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "release" { - datasetReleaseUsage() + fmt.Fprint(os.Stderr, datasetReleaseUsage) } else { - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - datasetUsage() + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) + fmt.Fprint(os.Stderr, datasetUsage) + return fmt.Errorf("") } + + return nil } -func handleListCommand() { +func handleListCommand() error { if flag.NArg() < 2 { - fmt.Println("Error: 'list' requires a subcommand (users, files).") - listUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: 'list' requires a subcommand (users, files).\n") + fmt.Fprint(os.Stderr, listUsage) + return fmt.Errorf("") } switch flag.Arg(1) { case "users": - checkToken(token) + if err := helpers.CheckTokenExpiration(token); err != nil { + return err + } err := list.Users(apiURI, token) if err != nil { - fmt.Printf("Error: failed to get users, reason: %v\n", err) + return fmt.Errorf("Error: failed to get users, reason: %v\n", err) } case "files": - handleListFilesCommand() + if err := handleListFilesCommand(); err != nil { + return err + } default: - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - listUsage() - os.Exit(1) + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + fmt.Fprint(os.Stderr, listUsage) + return fmt.Errorf("") } + + return nil } -func handleListFilesCommand() { +func handleListFilesCommand() error { listFilesCmd := flag.NewFlagSet("files", flag.ExitOnError) var username string listFilesCmd.StringVar(&username, "user", "", "Filter files by username") @@ -311,37 +311,47 @@ func handleListFilesCommand() { // Check if the -user flag was provided if username == "" { - fmt.Println("Error: the -user flag is required.") - listFilesUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: the -user flag is required.\n") + fmt.Fprint(os.Stderr, listFilesUsage) + return fmt.Errorf("") } - checkToken(token) - err := list.Files(apiURI, token, username) - if err != nil { - fmt.Printf("Error: failed to get files, reason: %v\n", err) + if err := helpers.CheckTokenExpiration(token); err != nil { + return err } + + if err := list.Files(apiURI, token, username); err != nil { + return fmt.Errorf("Error: failed to get files, reason: %v\n", err) + } + + return nil } -func handleFileCommand() { +func handleFileCommand() error { if flag.NArg() < 2 { - fmt.Println("Error: 'file' requires a subcommand (ingest, accession).") - fileUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: 'file' requires a subcommand (ingest, accession).\n") + fmt.Fprint(os.Stderr, fileUsage) + return fmt.Errorf("") } switch flag.Arg(1) { case "ingest": - handleFileIngestCommand() + if err := handleFileIngestCommand(); err != nil { + return err + } case "accession": - handleFileAccessionCommand() + if err := handleFileAccessionCommand(); err != nil { + return err + } default: - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - fileUsage() - os.Exit(1) + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + fmt.Fprint(os.Stderr, fileUsage) + return fmt.Errorf("") } + + return nil } -func handleFileIngestCommand() { +func handleFileIngestCommand() error { fileIngestCmd := flag.NewFlagSet("ingest", flag.ExitOnError) var filepath, username string fileIngestCmd.StringVar(&filepath, "filepath", "", "Filepath to ingest") @@ -349,27 +359,30 @@ func handleFileIngestCommand() { fileIngestCmd.Parse(flag.Args()[2:]) if filepath == "" || username == "" { - fmt.Println("Error: both -filepath and -user are required.") - fileIngestUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: both -filepath and -user are required.\n") + fmt.Fprint(os.Stderr, fileIngestUsage) + return fmt.Errorf("") } - err := helpers.CheckValidChars(filepath) - if err != nil { - fmt.Println("Error:", err) - os.Exit(1) + if err := helpers.CheckValidChars(filepath); err != nil { + return err + } + + if err := helpers.CheckTokenExpiration(token); err != nil { + return err } - checkToken(token) - err = file.Ingest(apiURI, token, username, filepath) + err := file.Ingest(apiURI, token, username, filepath) if err != nil { - fmt.Printf("Error: failed to ingest file, reason: %v\n", err) + return fmt.Errorf("Error: failed to ingest file, reason: %v\n", err) } else { fmt.Println("File ingestion triggered successfully.") } + + return nil } -func handleFileAccessionCommand() { +func handleFileAccessionCommand() error { fileAccessionCmd := flag.NewFlagSet("accession", flag.ExitOnError) var filepath, username, accessionID string fileAccessionCmd.StringVar(&filepath, "filepath", "", "Filepath to assign accession ID") @@ -378,45 +391,55 @@ func handleFileAccessionCommand() { fileAccessionCmd.Parse(flag.Args()[2:]) if filepath == "" || username == "" || accessionID == "" { - fmt.Println("Error: -filepath, -user, and -accession-id are required.") - fileAccessionUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: -filepath, -user, and -accession-id are required.\n") + fmt.Fprint(os.Stderr, fileAccessionUsage) + return fmt.Errorf("") } - err := helpers.CheckValidChars(filepath) - if err != nil { - fmt.Println("Error:", err) - os.Exit(1) + if err := helpers.CheckValidChars(filepath); err != nil { + return err } - checkToken(token) - err = file.Accession(apiURI, token, username, filepath, accessionID) + if err := helpers.CheckTokenExpiration(token); err != nil { + return err + } + + err := file.Accession(apiURI, token, username, filepath, accessionID) if err != nil { - fmt.Printf("Error: failed to assign accession ID to file, reason: %v\n", err) + return fmt.Errorf("Error: failed to assign accession ID to file, reason: %v\n", err) } else { fmt.Println("Accession ID assigned to file successfully.") } + + return nil } -func handleDatasetCommand() { +func handleDatasetCommand() error { if flag.NArg() < 2 { - fmt.Println("Error: 'dataset' requires a subcommand (create, release).") - datasetUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: 'dataset' requires a subcommand (create, release).\n") + fmt.Fprint(os.Stderr, datasetUsage) + return fmt.Errorf("") } + switch flag.Arg(1) { case "create": - handleDatasetCreateCommand() + if err := handleDatasetCreateCommand(); err != nil { + return err + } case "release": - handleDatasetReleaseCommand() + if err := handleDatasetReleaseCommand(); err != nil { + return err + } default: - fmt.Printf("Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - datasetUsage() - os.Exit(1) + fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) + fmt.Fprint(os.Stderr, datasetUsage) + return fmt.Errorf("") } + + return nil } -func handleDatasetCreateCommand() { +func handleDatasetCreateCommand() error { datasetCreateCmd := flag.NewFlagSet("create", flag.ExitOnError) var datasetID string datasetCreateCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to create") @@ -424,59 +447,85 @@ func handleDatasetCreateCommand() { accessionIDs := datasetCreateCmd.Args() // Args() returns the non-flag arguments after parsing if datasetID == "" || len(accessionIDs) == 0 { - fmt.Println("Error: -dataset-id and at least one accession ID are required.") - datasetCreateUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: -dataset-id and at least one accession ID are required.\n") + fmt.Fprint(os.Stderr, datasetCreateUsage) + return fmt.Errorf("") + } + + if err := helpers.CheckTokenExpiration(token); err != nil { + return err } - checkToken(token) err := dataset.Create(apiURI, token, datasetID, accessionIDs) if err != nil { - fmt.Printf("Error: failed to create dataset, reason: %v\n", err) + return fmt.Errorf("Error: failed to create dataset, reason: %v\n", err) } else { fmt.Println("Dataset created successfully.") } + + return nil } -func handleDatasetReleaseCommand() { +func handleDatasetReleaseCommand() error { datasetReleaseCmd := flag.NewFlagSet("release", flag.ExitOnError) var datasetID string datasetReleaseCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to release") datasetReleaseCmd.Parse(flag.Args()[2:]) if datasetID == "" { - fmt.Println("Error: -dataset-id is required.") - datasetReleaseUsage() - os.Exit(1) + fmt.Fprint(os.Stderr, "Error: -dataset-id is required.\n") + fmt.Fprint(os.Stderr, datasetReleaseUsage) + return fmt.Errorf("") + } + + if err := helpers.CheckTokenExpiration(token); err != nil { + return err } - checkToken(token) err := dataset.Release(apiURI, token, datasetID) if err != nil { - fmt.Printf("Error: failed to release dataset, reason: %v\n", err) + return fmt.Errorf("Error: failed to release dataset, reason: %v\n", err) } else { fmt.Println("Dataset released successfully.") } + + return nil } func main() { - parseFlagsAndEnv() + if err := parseFlagsAndEnv(); err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } switch flag.Arg(0) { case "help", "-h", "-help": - handleHelpCommand() + if err := handleHelpCommand(); err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } case "list": - handleListCommand() + if err := handleListCommand(); err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } case "file": - handleFileCommand() + if err := handleFileCommand(); err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } case "dataset": - handleDatasetCommand() + if err := handleDatasetCommand(); err != nil { + fmt.Fprint(os.Stderr, err) + os.Exit(1) + } case "version": printVersion() - os.Exit(0) default: - fmt.Printf("Unknown command '%s'.\n", flag.Arg(0)) - flag.Usage() + fmt.Fprintf(os.Stderr, "Unknown command '%s'.\n", flag.Arg(0)) + fmt.Fprint(os.Stderr, usage) os.Exit(1) } + + os.Exit(0) } From ac6219f49f33662faee83e54533b1997862df252 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Tue, 10 Sep 2024 20:38:32 +0000 Subject: [PATCH 19/33] refactor: use consistent commands structure --- sda-admin/README.md | 16 +- sda-admin/dataset/dataset.go | 2 +- sda-admin/file/file.go | 22 ++- sda-admin/file/file_test.go | 30 ++- sda-admin/list/list.go | 45 ----- sda-admin/main.go | 179 +++++++++--------- sda-admin/user/user.go | 27 +++ .../{list/list_test.go => user/user_test.go} | 20 +- 8 files changed, 171 insertions(+), 170 deletions(-) delete mode 100644 sda-admin/list/list.go create mode 100644 sda-admin/user/user.go rename sda-admin/{list/list_test.go => user/user_test.go} (58%) diff --git a/sda-admin/README.md b/sda-admin/README.md index f8c03fd5b..c191e6f97 100644 --- a/sda-admin/README.md +++ b/sda-admin/README.md @@ -18,14 +18,14 @@ Set the authentication token (optional if the environmental variable `ACCESS_TOK Use the following command to return all users with active uploads as a JSON array ```sh -sda-admin list users +sda-admin user list ``` ## List all files for a specified user Use the following command to return all files belonging to the specified user `test@dummy.org` ```sh -sda-admin list files -user test@dummy.org +sda-admin file list -user test@dummy.org ``` ## Ingest a file @@ -41,10 +41,10 @@ sda-admin file ingest -filepath /path/to/file.c4gh -user test@dummy.org Use the following command to assign an accession ID `my-accession-id-1` to a given file `/path/to/file.c4gh` that belongs to the user `test@dummy.org` ```sh -sda-admin file accession -filepath /path/to/file.c4gh -user test@dummy.org -accession-id my-accession-id-1 +sda-admin file set-accession -filepath /path/to/file.c4gh -user test@dummy.org -accession-id my-accession-id-1 ``` -## Create a dataset from a list of accession IDs and the dataset ID +## Create a dataset from a list of accession IDs and a dataset ID Use the following command to create a dataset `dataset001` from accession IDs `my-accession-id-1` and `my-accession-id-2` @@ -79,18 +79,18 @@ sda-admin help ### Examples -To get help on the list command: +To get help on the `file` command: ```sh -sda-admin help list +sda-admin help file ``` -To get help on the file ingest command: +To get help on the `file ingest` command: ```sh sda-admin help file ingest ``` -To get help on the dataset create command: +To get help on the `dataset create` command: ```sh sda-admin help dataset create diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index acdf291b9..ec6f24770 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -14,7 +14,7 @@ type RequestBodyDataset struct { DatasetID string `json:"dataset_id"` } -// Create creates a dataset from a list of accession IDs and the dataset ID. +// Create creates a dataset from a list of accession IDs and a dataset ID. func Create(apiURI, token, datasetID string, accessionIDs []string) error { parsedURL, err := url.Parse(apiURI) if err != nil { diff --git a/sda-admin/file/file.go b/sda-admin/file/file.go index 7d7f06da9..895bf5070 100644 --- a/sda-admin/file/file.go +++ b/sda-admin/file/file.go @@ -20,6 +20,24 @@ type RequestBodyFileAccession struct { User string `json:"user"` } +// List returns all files +func List(apiURI, token, username string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { + return err + } + parsedURL.Path = path.Join(parsedURL.Path, "users", username, "files") + + response, err := helpers.GetResponseBody(parsedURL.String(), token) + if err != nil { + return err + } + + fmt.Println(string(response)) + + return nil +} + // Ingest triggers the ingestion of a given file func Ingest(apiURI, token, username, filepath string) error { parsedURL, err := url.Parse(apiURI) @@ -46,8 +64,8 @@ func Ingest(apiURI, token, username, filepath string) error { return nil } -// Accession assigns a given file to a given accession ID -func Accession(apiURI, token, username, filepath, accessionID string) error { +// SetAccession assigns an accession ID to a specified file for a given user +func SetAccession(apiURI, token, username, filepath, accessionID string) error { parsedURL, err := url.Parse(apiURI) if err != nil { return err diff --git a/sda-admin/file/file_test.go b/sda-admin/file/file_test.go index ae442d54e..42ad532a9 100644 --- a/sda-admin/file/file_test.go +++ b/sda-admin/file/file_test.go @@ -14,12 +14,34 @@ type MockHelpers struct { mock.Mock } +// Mock the GetResponseBody function +func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { + args := m.Called(url, token) + + return args.Get(0).([]byte), args.Error(1) +} + +// Mock the PostRequest function func (m *MockHelpers) PostRequest(url, token string, jsonBody []byte) ([]byte, error) { args := m.Called(url, token, jsonBody) return args.Get(0).([]byte), args.Error(1) } +func TestList(t *testing.T) { + mockHelpers := new(MockHelpers) + mockHelpers.On("GetResponseBody", "http://example.com/users/testuser/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) + + // Replace the original GetResponseBody with the mock + originalFunc := helpers.GetResponseBody + defer func() { helpers.GetResponseBody = originalFunc }() + helpers.GetResponseBody = mockHelpers.GetResponseBody + + err := List("http://example.com", "test-token", "testuser") + assert.NoError(t, err) + mockHelpers.AssertExpectations(t) +} + func TestIngest_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest @@ -59,7 +81,7 @@ func TestIngest_PostRequestFailure(t *testing.T) { mockHelpers.AssertExpectations(t) } -func TestAccession_Success(t *testing.T) { +func TestSetAccession_Success(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -74,12 +96,12 @@ func TestAccession_Success(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) - err := Accession("http://example.com", token, username, filepath, accessionID) + err := SetAccession("http://example.com", token, username, filepath, accessionID) assert.NoError(t, err) mockHelpers.AssertExpectations(t) } -func TestAccession_PostRequestFailure(t *testing.T) { +func TestSetAccession_PostRequestFailure(t *testing.T) { mockHelpers := new(MockHelpers) originalFunc := helpers.PostRequest helpers.PostRequest = mockHelpers.PostRequest @@ -94,7 +116,7 @@ func TestAccession_PostRequestFailure(t *testing.T) { mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) - err := Accession("http://example.com", token, username, filepath, accessionID) + err := SetAccession("http://example.com", token, username, filepath, accessionID) assert.Error(t, err) assert.EqualError(t, err, "failed to send request") mockHelpers.AssertExpectations(t) diff --git a/sda-admin/list/list.go b/sda-admin/list/list.go deleted file mode 100644 index 20e3c665d..000000000 --- a/sda-admin/list/list.go +++ /dev/null @@ -1,45 +0,0 @@ -package list - -import ( - "fmt" - "net/url" - "path" - - "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" -) - -// Users returns all users -func Users(apiURI, token string) error { - parsedURL, err := url.Parse(apiURI) - if err != nil { - return err - } - parsedURL.Path = path.Join(parsedURL.Path, "users") - - response, err := helpers.GetResponseBody(parsedURL.String(), token) - if err != nil { - return err - } - - fmt.Println(string(response)) - - return nil -} - -// Files returns all files -func Files(apiURI, token, username string) error { - parsedURL, err := url.Parse(apiURI) - if err != nil { - return err - } - parsedURL.Path = path.Join(parsedURL.Path, "users", username, "files") - - response, err := helpers.GetResponseBody(parsedURL.String(), token) - if err != nil { - return err - } - - fmt.Println(string(response)) - - return nil -} diff --git a/sda-admin/main.go b/sda-admin/main.go index 3f4c2b82f..6baa71078 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -8,7 +8,7 @@ import ( "github.com/neicnordic/sensitive-data-archive/sda-admin/dataset" "github.com/neicnordic/sensitive-data-archive/sda-admin/file" "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" - "github.com/neicnordic/sensitive-data-archive/sda-admin/list" + "github.com/neicnordic/sensitive-data-archive/sda-admin/user" ) var version = "1.0.0" @@ -24,111 +24,104 @@ Usage: sda-admin [-uri URI] [-token TOKEN] [options] Commands: - list users List all users - list files -user USERNAME List all files for a specified user + user list List all users. + file list -user USERNAME List all files for a specified user. file ingest -filepath FILEPATH -user USERNAME - Trigger ingestion of the given file - file accession -filepath FILEPATH -user USERNAME -accession-id accessionID - Assign accession ID to a file + Trigger ingestion of a given file. + file set-accession -filepath FILEPATH -user USERNAME -accession-id accessionID + Assign accession ID to a file. dataset create -dataset-id DATASET_ID accessionID [accessionID ...] - Create a dataset from a list of accession IDs and the dataset ID + Create a dataset from a list of accession IDs and a dataset ID. dataset release -dataset-id DATASET_ID - Release a dataset for downloading + Release a dataset for downloading. Global Options: - -uri URI Set the URI for the API server (optional if API_HOST is set) - -token TOKEN Set the authentication token (optional if ACCESS_TOKEN is set) + -uri URI Set the URI for the API server (optional if API_HOST is set). + -token TOKEN Set the authentication token (optional if ACCESS_TOKEN is set). Additional Commands: - help Show this help message - -h, -help Show this help message + help Show this help message. + -h, -help Show this help message. ` -var listUsage = ` +var userUsage = ` List Users: - Usage: sda-admin list users + Usage: sda-admin user list List all users in the system. - -List Files: - Usage: sda-admin list files -user USERNAME - List all files for a specified user. - -Options: - -user USERNAME (For list files) List files owned by the specified user - -Use 'sda-admin help list ' for information on a specific list command. ` -var listUsersUsage = ` -Usage: sda-admin list users +var userListUsage = ` +Usage: sda-admin user list List all users in the system. ` -var listFilesUsage = ` -Usage: sda-admin list files -user USERNAME - List all files for a specified user. - -Options: - -user USERNAME List files owned by the specified user -` - var fileUsage = ` -Ingest a File: +List all files for a user: + Usage: sda-admin file list -user USERNAME + List all files for a specified user. + +Ingest a file: Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME Trigger the ingestion of a given file for a specific user. -Accession a File: - Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID - Assign an accession ID to a file and associate it with a user. - -Common Options for 'file' Commands: - -filepath FILEPATH Specify the path of the file. - -user USERNAME Specify the username associated with the file. +Set accession ID to a file: + Usage: sda-admin file set-accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID + Assign an accession ID to a file for a given user. - For 'file accession' additionally: - -accession-id ID Specify the accession ID to assign to the file. +Options: + -user USERNAME Specify the username associated with the file. + -filepath FILEPATH Specify the path of the file to ingest. + -accession-id ID Specify the accession ID to assign to the file. Use 'sda-admin help file ' for information on a specific command. ` +var fileListUsage = ` +Usage: sda-admin file list -user USERNAME + List all files for a specified user. + +Options: + -user USERNAME Specify the username associated with the files. +` + var fileIngestUsage = ` Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME - Trigger ingestion of the given file for a specific user. + Trigger the ingestion of a given file for a specific user. Options: - -filepath FILEPATH Specify the path of the file to ingest. - -user USERNAME Specify the username associated with the file. + -filepath FILEPATH Specify the path of the file to ingest. + -user USERNAME Specify the username associated with the file. ` var fileAccessionUsage = ` -Usage: sda-admin file accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID +Usage: sda-admin file set-accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID Assign accession ID to a file and associate it with a user. Options: - -filepath FILEPATH Specify the path of the file to assign the accession ID. - -user USERNAME Specify the username associated with the file. - -accession-id ID Specify the accession ID to assign to the file. + -filepath FILEPATH Specify the path of the file to assign the accession ID. + -user USERNAME Specify the username associated with the file. + -accession-id ID Specify the accession ID to assign to the file. ` var datasetUsage = ` -Create a Dataset: +Create a dataset: Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] - Create a dataset from a list of accession IDs and the dataset ID. + Create a dataset from a list of accession IDs and a dataset ID. -Release a Dataset: +Release a dataset: Usage: sda-admin dataset release -dataset-id DATASET_ID Release a dataset for downloading based on its dataset ID. Options: - -dataset-id DATASET_ID Specify the unique identifier for the dataset. - [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. + -dataset-id DATASET_ID Specify the unique identifier for the dataset. + [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. -Use 'sda-admin help dataset ' for information on a specific dataset command. +Use 'sda-admin help dataset ' for information on a specific command. ` var datasetCreateUsage = ` Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] - Create a dataset from a list of accession IDs and the dataset ID. + Create a dataset from a list of accession IDs and a dataset ID. Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset. @@ -140,7 +133,7 @@ Usage: sda-admin dataset release -dataset-id DATASET_ID Release a dataset for downloading based on its dataset ID. Options: - -dataset-id DATASET_ID Specify the unique identifier for the dataset to release. + -dataset-id DATASET_ID Specify the unique identifier for the dataset. ` var versionUsage = ` @@ -201,8 +194,8 @@ func parseFlagsAndEnv() error { func handleHelpCommand() error { if flag.NArg() > 1 { switch flag.Arg(1) { - case "list": - if err := handleHelpList(); err != nil { + case "user": + if err := handleHelpUser(); err != nil { return err } case "file": @@ -227,16 +220,14 @@ func handleHelpCommand() error { return nil } -func handleHelpList() error { +func handleHelpUser() error { if flag.NArg() == 2 { - fmt.Fprint(os.Stderr, listUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "users" { - fmt.Fprint(os.Stderr, listUsersUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "files" { - fmt.Fprint(os.Stderr, listFilesUsage) + fmt.Fprint(os.Stderr, userUsage) + } else if flag.NArg() > 2 && flag.Arg(2) == "list" { + fmt.Fprint(os.Stderr, userListUsage) } else { fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - fmt.Fprint(os.Stderr, listUsage) + fmt.Fprint(os.Stderr, userUsage) return fmt.Errorf("") } @@ -246,9 +237,11 @@ func handleHelpList() error { func handleHelpFile() error { if flag.NArg() == 2 { fmt.Fprint(os.Stderr, fileUsage) + } else if flag.NArg() > 2 && flag.Arg(2) == "list" { + fmt.Fprint(os.Stderr, fileListUsage) } else if flag.NArg() > 2 && flag.Arg(2) == "ingest" { fmt.Fprint(os.Stderr, fileIngestUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "accession" { + } else if flag.NArg() > 2 && flag.Arg(2) == "set-accession" { fmt.Fprint(os.Stderr, fileAccessionUsage) } else { fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) @@ -275,36 +268,32 @@ func handleHelpDataset() error { return nil } -func handleListCommand() error { +func handleUserCommand() error { if flag.NArg() < 2 { - fmt.Fprint(os.Stderr, "Error: 'list' requires a subcommand (users, files).\n") - fmt.Fprint(os.Stderr, listUsage) + fmt.Fprint(os.Stderr, "Error: 'user' requires a subcommand (list).\n") + fmt.Fprint(os.Stderr, userUsage) return fmt.Errorf("") } switch flag.Arg(1) { - case "users": + case "list": if err := helpers.CheckTokenExpiration(token); err != nil { return err } - err := list.Users(apiURI, token) + err := user.List(apiURI, token) if err != nil { return fmt.Errorf("Error: failed to get users, reason: %v\n", err) } - case "files": - if err := handleListFilesCommand(); err != nil { - return err - } default: fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - fmt.Fprint(os.Stderr, listUsage) + fmt.Fprint(os.Stderr, userUsage) return fmt.Errorf("") } return nil } -func handleListFilesCommand() error { - listFilesCmd := flag.NewFlagSet("files", flag.ExitOnError) +func handleFileListCommand() error { + listFilesCmd := flag.NewFlagSet("list", flag.ExitOnError) var username string listFilesCmd.StringVar(&username, "user", "", "Filter files by username") listFilesCmd.Parse(flag.Args()[2:]) @@ -312,7 +301,7 @@ func handleListFilesCommand() error { // Check if the -user flag was provided if username == "" { fmt.Fprint(os.Stderr, "Error: the -user flag is required.\n") - fmt.Fprint(os.Stderr, listFilesUsage) + fmt.Fprint(os.Stderr, fileListUsage) return fmt.Errorf("") } @@ -320,7 +309,7 @@ func handleListFilesCommand() error { return err } - if err := list.Files(apiURI, token, username); err != nil { + if err := file.List(apiURI, token, username); err != nil { return fmt.Errorf("Error: failed to get files, reason: %v\n", err) } @@ -329,16 +318,20 @@ func handleListFilesCommand() error { func handleFileCommand() error { if flag.NArg() < 2 { - fmt.Fprint(os.Stderr, "Error: 'file' requires a subcommand (ingest, accession).\n") + fmt.Fprint(os.Stderr, "Error: 'file' requires a subcommand (list, ingest, set-accession).\n") fmt.Fprint(os.Stderr, fileUsage) return fmt.Errorf("") } switch flag.Arg(1) { + case "list": + if err := handleFileListCommand(); err != nil { + return err + } case "ingest": if err := handleFileIngestCommand(); err != nil { return err } - case "accession": + case "set-accession": if err := handleFileAccessionCommand(); err != nil { return err } @@ -383,7 +376,7 @@ func handleFileIngestCommand() error { } func handleFileAccessionCommand() error { - fileAccessionCmd := flag.NewFlagSet("accession", flag.ExitOnError) + fileAccessionCmd := flag.NewFlagSet("set-accession", flag.ExitOnError) var filepath, username, accessionID string fileAccessionCmd.StringVar(&filepath, "filepath", "", "Filepath to assign accession ID") fileAccessionCmd.StringVar(&username, "user", "", "Username to associate with the file") @@ -404,7 +397,7 @@ func handleFileAccessionCommand() error { return err } - err := file.Accession(apiURI, token, username, filepath, accessionID) + err := file.SetAccession(apiURI, token, username, filepath, accessionID) if err != nil { return fmt.Errorf("Error: failed to assign accession ID to file, reason: %v\n", err) } else { @@ -494,29 +487,29 @@ func handleDatasetReleaseCommand() error { func main() { if err := parseFlagsAndEnv(); err != nil { - fmt.Fprint(os.Stderr, err) + fmt.Fprintln(os.Stderr, err) os.Exit(1) } switch flag.Arg(0) { case "help", "-h", "-help": if err := handleHelpCommand(); err != nil { - fmt.Fprint(os.Stderr, err) + fmt.Fprintln(os.Stderr, err) os.Exit(1) } - case "list": - if err := handleListCommand(); err != nil { - fmt.Fprint(os.Stderr, err) + case "user": + if err := handleUserCommand(); err != nil { + fmt.Fprintln(os.Stderr, err) os.Exit(1) } case "file": if err := handleFileCommand(); err != nil { - fmt.Fprint(os.Stderr, err) + fmt.Fprintln(os.Stderr, err) os.Exit(1) } case "dataset": if err := handleDatasetCommand(); err != nil { - fmt.Fprint(os.Stderr, err) + fmt.Fprintln(os.Stderr, err) os.Exit(1) } case "version": diff --git a/sda-admin/user/user.go b/sda-admin/user/user.go new file mode 100644 index 000000000..fdc5dafe7 --- /dev/null +++ b/sda-admin/user/user.go @@ -0,0 +1,27 @@ +package user + +import ( + "fmt" + "net/url" + "path" + + "github.com/neicnordic/sensitive-data-archive/sda-admin/helpers" +) + +// List returns all users +func List(apiURI, token string) error { + parsedURL, err := url.Parse(apiURI) + if err != nil { + return err + } + parsedURL.Path = path.Join(parsedURL.Path, "users") + + response, err := helpers.GetResponseBody(parsedURL.String(), token) + if err != nil { + return err + } + + fmt.Println(string(response)) + + return nil +} diff --git a/sda-admin/list/list_test.go b/sda-admin/user/user_test.go similarity index 58% rename from sda-admin/list/list_test.go rename to sda-admin/user/user_test.go index 1f1e6a2e9..7946f902a 100644 --- a/sda-admin/list/list_test.go +++ b/sda-admin/user/user_test.go @@ -1,4 +1,4 @@ -package list +package user import ( "testing" @@ -20,7 +20,7 @@ func (m *MockHelpers) GetResponseBody(url, token string) ([]byte, error) { return args.Get(0).([]byte), args.Error(1) } -func TestUsers(t *testing.T) { +func TestList(t *testing.T) { mockHelpers := new(MockHelpers) mockHelpers.On("GetResponseBody", "http://example.com/users", "test-token").Return([]byte(`["user1", "user2"]`), nil) @@ -29,21 +29,7 @@ func TestUsers(t *testing.T) { defer func() { helpers.GetResponseBody = originalFunc }() helpers.GetResponseBody = mockHelpers.GetResponseBody - err := Users("http://example.com", "test-token") - assert.NoError(t, err) - mockHelpers.AssertExpectations(t) -} - -func TestFiles(t *testing.T) { - mockHelpers := new(MockHelpers) - mockHelpers.On("GetResponseBody", "http://example.com/users/testuser/files", "test-token").Return([]byte(`["file1", "file2"]`), nil) - - // Replace the original GetResponseBody with the mock - originalFunc := helpers.GetResponseBody - defer func() { helpers.GetResponseBody = originalFunc }() - helpers.GetResponseBody = mockHelpers.GetResponseBody - - err := Files("http://example.com", "test-token", "testuser") + err := List("http://example.com", "test-token") assert.NoError(t, err) mockHelpers.AssertExpectations(t) } From b76c6b24c5a9c6861e1f3f02ede53d377a849ec9 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Tue, 10 Sep 2024 20:48:16 +0000 Subject: [PATCH 20/33] add sda-admin unit tests to the workflow --- .github/workflows/test.yml | 40 +++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 96b6d7bf5..8b3f1cd33 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -86,7 +86,7 @@ jobs: - name: Get dependencies run: | - cd sda-download + cd sda go get -v -t -d ./... if [ -f Gopkg.toml ]; then curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh @@ -104,3 +104,41 @@ jobs: file: ./sda/coverage.txt flags: unittests fail_ci_if_error: false + + test_sda_admin: + name: Test SDA Admin + runs-on: ubuntu-latest + strategy: + matrix: + go-version: ['1.22'] + steps: + + - name: Set up Go ${{ matrix.go-version }} + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + id: go + + - name: Check out code into the Go module directory + uses: actions/checkout@v4 + + - name: Get dependencies + run: | + cd sda-admin + go get -v -t -d ./... + if [ -f Gopkg.toml ]; then + curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh + dep ensure + fi + - name: Test + run: | + cd sda-admin + go test -v -coverprofile=coverage.txt -covermode=atomic ./... + + - name: Codecov + uses: codecov/codecov-action@v4.5.0 + with: + token: ${{ secrets.CODECOV_TOKEN }} + file: ./sda-admin/coverage.txt + flags: unittests + fail_ci_if_error: false From 82a8fbccbac0332d3ee7519d0fb86b20a98cc1fe Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 14:07:55 +0000 Subject: [PATCH 21/33] improve README --- sda-admin/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/sda-admin/README.md b/sda-admin/README.md index c191e6f97..f4e436cb3 100644 --- a/sda-admin/README.md +++ b/sda-admin/README.md @@ -16,32 +16,32 @@ Set the authentication token (optional if the environmental variable `ACCESS_TOK ## List all users -Use the following command to return all users with active uploads as a JSON array +Use the following command to return all users with active uploads ```sh sda-admin user list ``` ## List all files for a specified user -Use the following command to return all files belonging to the specified user `test@dummy.org` +Use the following command to return all files belonging to the specified user `test-user@example.org` ```sh -sda-admin file list -user test@dummy.org +sda-admin file list -user test-user@example.org ``` ## Ingest a file -Use the following command to trigger the ingesting of a given file `/path/to/file.c4gh` that belongs to the user `test@dummy.org` +Use the following command to trigger the ingesting of a given file `/path/to/file.c4gh` that belongs to the user `test-user@example.org` ```sh -sda-admin file ingest -filepath /path/to/file.c4gh -user test@dummy.org +sda-admin file ingest -filepath /path/to/file.c4gh -user test-user@example.org ``` ## Assign an accession ID to a file -Use the following command to assign an accession ID `my-accession-id-1` to a given file `/path/to/file.c4gh` that belongs to the user `test@dummy.org` +Use the following command to assign an accession ID `my-accession-id-1` to a given file `/path/to/file.c4gh` that belongs to the user `test-user@example.org` ```sh -sda-admin file set-accession -filepath /path/to/file.c4gh -user test@dummy.org -accession-id my-accession-id-1 +sda-admin file set-accession -filepath /path/to/file.c4gh -user test-user@example.org -accession-id my-accession-id-1 ``` ## Create a dataset from a list of accession IDs and a dataset ID From 6ed71f84e1827cfacc42811bed8104cb96ecfcd7 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 14:34:05 +0000 Subject: [PATCH 22/33] print all error message in the main function --- sda-admin/main.go | 66 ++++++++++++++--------------------------------- 1 file changed, 19 insertions(+), 47 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index 6baa71078..99430075f 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "flag" "fmt" "os" @@ -160,7 +161,7 @@ func parseFlagsAndEnv() error { // Custom usage message flag.Usage = func() { - fmt.Fprint(os.Stderr, usage) + fmt.Println(usage) } // Parse global flags first @@ -168,7 +169,7 @@ func parseFlagsAndEnv() error { // If no command is provided, show usage if flag.NArg() == 0 { - return fmt.Errorf(usage) + return errors.New(usage) } // Check environment variables if flags are not provided @@ -209,9 +210,8 @@ func handleHelpCommand() error { case "version": fmt.Fprint(os.Stderr, versionUsage) default: - fmt.Fprintf(os.Stderr, "Unknown command '%s'.\n", flag.Arg(1)) - fmt.Fprint(os.Stderr, usage) - return fmt.Errorf("") + return fmt.Errorf("Unknown command '%s'.\n%s", flag.Arg(1), usage) + } } else { fmt.Fprint(os.Stderr, usage) @@ -226,9 +226,7 @@ func handleHelpUser() error { } else if flag.NArg() > 2 && flag.Arg(2) == "list" { fmt.Fprint(os.Stderr, userListUsage) } else { - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - fmt.Fprint(os.Stderr, userUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) } return nil @@ -244,9 +242,7 @@ func handleHelpFile() error { } else if flag.NArg() > 2 && flag.Arg(2) == "set-accession" { fmt.Fprint(os.Stderr, fileAccessionUsage) } else { - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - fmt.Fprint(os.Stderr, fileUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) } return nil @@ -260,9 +256,7 @@ func handleHelpDataset() error { } else if flag.NArg() > 2 && flag.Arg(2) == "release" { fmt.Fprint(os.Stderr, datasetReleaseUsage) } else { - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(2), flag.Arg(1)) - fmt.Fprint(os.Stderr, datasetUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), datasetUsage) } return nil @@ -270,9 +264,7 @@ func handleHelpDataset() error { func handleUserCommand() error { if flag.NArg() < 2 { - fmt.Fprint(os.Stderr, "Error: 'user' requires a subcommand (list).\n") - fmt.Fprint(os.Stderr, userUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: 'user' requires a subcommand (list).\n%s", userUsage) } switch flag.Arg(1) { case "list": @@ -284,9 +276,7 @@ func handleUserCommand() error { return fmt.Errorf("Error: failed to get users, reason: %v\n", err) } default: - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - fmt.Fprint(os.Stderr, userUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), userUsage) } return nil @@ -300,9 +290,7 @@ func handleFileListCommand() error { // Check if the -user flag was provided if username == "" { - fmt.Fprint(os.Stderr, "Error: the -user flag is required.\n") - fmt.Fprint(os.Stderr, fileListUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: the -user flag is required.\n%s", fileListUsage) } if err := helpers.CheckTokenExpiration(token); err != nil { @@ -318,9 +306,7 @@ func handleFileListCommand() error { func handleFileCommand() error { if flag.NArg() < 2 { - fmt.Fprint(os.Stderr, "Error: 'file' requires a subcommand (list, ingest, set-accession).\n") - fmt.Fprint(os.Stderr, fileUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: 'file' requires a subcommand (list, ingest, set-accession).\n%s", fileUsage) } switch flag.Arg(1) { case "list": @@ -336,9 +322,7 @@ func handleFileCommand() error { return err } default: - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - fmt.Fprint(os.Stderr, fileUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), fileUsage) } return nil @@ -352,9 +336,7 @@ func handleFileIngestCommand() error { fileIngestCmd.Parse(flag.Args()[2:]) if filepath == "" || username == "" { - fmt.Fprint(os.Stderr, "Error: both -filepath and -user are required.\n") - fmt.Fprint(os.Stderr, fileIngestUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: both -filepath and -user are required.\n%s", fileIngestUsage) } if err := helpers.CheckValidChars(filepath); err != nil { @@ -384,9 +366,7 @@ func handleFileAccessionCommand() error { fileAccessionCmd.Parse(flag.Args()[2:]) if filepath == "" || username == "" || accessionID == "" { - fmt.Fprint(os.Stderr, "Error: -filepath, -user, and -accession-id are required.\n") - fmt.Fprint(os.Stderr, fileAccessionUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: -filepath, -user, and -accession-id are required.\n%s", fileAccessionUsage) } if err := helpers.CheckValidChars(filepath); err != nil { @@ -409,9 +389,7 @@ func handleFileAccessionCommand() error { func handleDatasetCommand() error { if flag.NArg() < 2 { - fmt.Fprint(os.Stderr, "Error: 'dataset' requires a subcommand (create, release).\n") - fmt.Fprint(os.Stderr, datasetUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: 'dataset' requires a subcommand (create, release).\n%s", datasetUsage) } switch flag.Arg(1) { @@ -424,9 +402,7 @@ func handleDatasetCommand() error { return err } default: - fmt.Fprintf(os.Stderr, "Unknown subcommand '%s' for '%s'.\n", flag.Arg(1), flag.Arg(0)) - fmt.Fprint(os.Stderr, datasetUsage) - return fmt.Errorf("") + return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), datasetUsage) } return nil @@ -440,9 +416,7 @@ func handleDatasetCreateCommand() error { accessionIDs := datasetCreateCmd.Args() // Args() returns the non-flag arguments after parsing if datasetID == "" || len(accessionIDs) == 0 { - fmt.Fprint(os.Stderr, "Error: -dataset-id and at least one accession ID are required.\n") - fmt.Fprint(os.Stderr, datasetCreateUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: -dataset-id and at least one accession ID are required.\n%s", datasetCreateUsage) } if err := helpers.CheckTokenExpiration(token); err != nil { @@ -466,9 +440,7 @@ func handleDatasetReleaseCommand() error { datasetReleaseCmd.Parse(flag.Args()[2:]) if datasetID == "" { - fmt.Fprint(os.Stderr, "Error: -dataset-id is required.\n") - fmt.Fprint(os.Stderr, datasetReleaseUsage) - return fmt.Errorf("") + return fmt.Errorf("Error: -dataset-id is required.\n%s", datasetReleaseUsage) } if err := helpers.CheckTokenExpiration(token); err != nil { From 7052542cf739225c99628a5da8e4bc15369dc795 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 14:45:24 +0000 Subject: [PATCH 23/33] add sda-admin to code-linter --- .github/workflows/code-linter.yaml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code-linter.yaml b/.github/workflows/code-linter.yaml index fce26593b..cfdd4199c 100644 --- a/.github/workflows/code-linter.yaml +++ b/.github/workflows/code-linter.yaml @@ -52,4 +52,27 @@ jobs: uses: golangci/golangci-lint-action@v6.1.0 with: args: -E bodyclose,gocritic,gofmt,gosec,govet,nestif,nlreturn,rowserrcheck -e G401,G501,G107,G115 --timeout 5m - working-directory: sda \ No newline at end of file + working-directory: sda + + lint_sda_admin: + name: Lint sda-admin code + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + go-version: ['1.22'] + steps: + - name: Set up Go ${{ matrix.go-version }} + uses: actions/setup-go@v5 + with: + go-version: ${{ matrix.go-version }} + id: go + + - name: Check out code into the Go module directory + uses: actions/checkout@v4 + + - name: Run golangci-lint + uses: golangci/golangci-lint-action@v6.1.0 + with: + args: -E bodyclose,gocritic,gofmt,gosec,govet,nestif,nlreturn,rowserrcheck -e G401,G501,G107,G115 --timeout 5m + working-directory: sda-admin From 08805ed978d77bfb58464d453dc40e81e785eb7d Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 15:01:18 +0000 Subject: [PATCH 24/33] add build, lint and test functionalities in the Makefile --- Makefile | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index f4a2012ca..9a1194b68 100644 --- a/Makefile +++ b/Makefile @@ -23,7 +23,7 @@ bootstrap: go-version-check docker-version-check GO111MODULE=off go get golang.org/x/tools/cmd/goimports # build containers -build-all: build-postgresql build-rabbitmq build-sda build-sda-download build-sda-sftp-inbox +build-all: build-postgresql build-rabbitmq build-sda build-sda-download build-sda-sftp-inbox build-sda-admin build-postgresql: @cd postgresql && docker build -t ghcr.io/neicnordic/sensitive-data-archive:PR$$(date +%F)-postgres . build-rabbitmq: @@ -34,6 +34,8 @@ build-sda-download: @cd sda-download && docker build -t ghcr.io/neicnordic/sensitive-data-archive:PR$$(date +%F)-download . build-sda-sftp-inbox: @cd sda-sftp-inbox && docker build -t ghcr.io/neicnordic/sensitive-data-archive:PR$$(date +%F)-sftp-inbox . +build-sda-admin: + @cd sda-admin && go build go-version-check: SHELL:=/bin/bash @@ -81,13 +83,16 @@ integrationtest-sda: build-all @PR_NUMBER=$$(date +%F) docker compose -f .github/integration/sda-posix-integration.yml down -v --remove-orphans # lint go code -lint-all: lint-sda lint-sda-download +lint-all: lint-sda lint-sda-download lint-sda-admin lint-sda: @echo 'Running golangci-lint in the `sda` folder' @cd sda && golangci-lint run $(LINT_INCLUDE) $(LINT_EXCLUDE) lint-sda-download: @echo 'Running golangci-lint in the `sda-download` folder' @cd sda-download && golangci-lint run $(LINT_INCLUDE) $(LINT_EXCLUDE) +lint-sda-admin: + @echo 'Running golangci-lint in the `sda-admin` folder' + @cd sda-admin && golangci-lint run $(LINT_INCLUDE) $(LINT_EXCLUDE) # run static code tests test-all: test-sda test-sda-download test-sda-sftp-inbox @@ -97,3 +102,5 @@ test-sda-download: @cd sda-download && go test ./... -count=1 test-sda-sftp-inbox: @docker run --rm -v ./sda-sftp-inbox:/inbox maven:3.9.4-eclipse-temurin-21-alpine sh -c "cd /inbox && mvn test -B" +test-sda-admin: + @cd sda-admin && go test ./... -count=1 From c7d5d31056392a3b97b78fb951de5187994c8566 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 20:57:44 +0000 Subject: [PATCH 25/33] fix linting problem --- sda-admin/dataset/dataset.go | 1 - sda-admin/helpers/helpers_test.go | 19 +++-- sda-admin/main.go | 113 +++++++++++++++++------------- 3 files changed, 76 insertions(+), 57 deletions(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index ec6f24770..88080ed87 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -57,4 +57,3 @@ func Release(apiURI, token, datasetID string) error { return nil } - diff --git a/sda-admin/helpers/helpers_test.go b/sda-admin/helpers/helpers_test.go index 0b2654134..71b971685 100644 --- a/sda-admin/helpers/helpers_test.go +++ b/sda-admin/helpers/helpers_test.go @@ -62,8 +62,11 @@ func TestTokenExpiration(t *testing.T) { func TestGetBody(t *testing.T) { // Mock server mockResponse := `{"key": "value"}` - server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - rw.Write([]byte(mockResponse)) + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { + _, err := rw.Write([]byte(mockResponse)) + if err != nil { + t.Fatalf("Failed to write response: %v", err) + } })) defer server.Close() @@ -73,7 +76,7 @@ func TestGetBody(t *testing.T) { assert.JSONEq(t, mockResponse, string(body)) // Test non-200 status code - serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { rw.WriteHeader(http.StatusInternalServerError) })) defer serverError.Close() @@ -85,11 +88,15 @@ func TestGetBody(t *testing.T) { func TestPostReq(t *testing.T) { // Mock server - mockResponse := `{"key": "value"}` + mockResponse := `{"key": "value"}` server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { assert.Equal(t, "application/json", req.Header.Get("Content-Type")) assert.Equal(t, "Bearer mock_token", req.Header.Get("Authorization")) - rw.Write([]byte(mockResponse)) + + _, err := rw.Write([]byte(mockResponse)) + if err != nil { + t.Fatalf("Failed to write response: %v", err) + } })) defer server.Close() @@ -99,7 +106,7 @@ func TestPostReq(t *testing.T) { assert.JSONEq(t, mockResponse, string(body)) // Test non-200 status code - serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + serverError := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { rw.WriteHeader(http.StatusInternalServerError) })) defer serverError.Close() diff --git a/sda-admin/main.go b/sda-admin/main.go index 99430075f..4516186cc 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -146,14 +146,6 @@ func printVersion() { fmt.Printf("sda-admin version %s\n", version) } -func checkToken(token string) error { - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - - return nil -} - func parseFlagsAndEnv() error { // Set up flags flag.StringVar(&apiURI, "uri", "", "Set the URI for the SDA server (optional if API_HOST is set)") @@ -172,20 +164,22 @@ func parseFlagsAndEnv() error { return errors.New(usage) } + if flag.Arg(0) == "help" { + return nil + } + // Check environment variables if flags are not provided - if flag.Arg(0) != "help" { + if apiURI == "" { + apiURI = os.Getenv("API_HOST") if apiURI == "" { - apiURI = os.Getenv("API_HOST") - if apiURI == "" { - return fmt.Errorf("error: either -uri must be provided or API_HOST environment variable must be set.") - } + return fmt.Errorf("error: either -uri must be provided or API_HOST environment variable must be set") } + } + if token == "" { + token = os.Getenv("ACCESS_TOKEN") if token == "" { - token = os.Getenv("ACCESS_TOKEN") - if token == "" { - return fmt.Errorf("error: either -token must be provided or ACCESS_TOKEN environment variable must be set.") - } + return fmt.Errorf("error: either -token must be provided or ACCESS_TOKEN environment variable must be set") } } @@ -221,11 +215,12 @@ func handleHelpCommand() error { } func handleHelpUser() error { - if flag.NArg() == 2 { + switch { + case flag.NArg() == 2: fmt.Fprint(os.Stderr, userUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "list" { + case flag.NArg() > 2 && flag.Arg(2) == "list": fmt.Fprint(os.Stderr, userListUsage) - } else { + default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) } @@ -233,15 +228,16 @@ func handleHelpUser() error { } func handleHelpFile() error { - if flag.NArg() == 2 { + switch { + case flag.NArg() == 2: fmt.Fprint(os.Stderr, fileUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "list" { + case flag.NArg() > 2 && flag.Arg(2) == "list": fmt.Fprint(os.Stderr, fileListUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "ingest" { + case flag.NArg() > 2 && flag.Arg(2) == "ingest": fmt.Fprint(os.Stderr, fileIngestUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "set-accession" { + case flag.NArg() > 2 && flag.Arg(2) == "set-accession": fmt.Fprint(os.Stderr, fileAccessionUsage) - } else { + default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) } @@ -249,13 +245,14 @@ func handleHelpFile() error { } func handleHelpDataset() error { - if flag.NArg() == 2 { + switch { + case flag.NArg() == 2: fmt.Fprint(os.Stderr, datasetUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "create" { + case flag.NArg() > 2 && flag.Arg(2) == "create": fmt.Fprint(os.Stderr, datasetCreateUsage) - } else if flag.NArg() > 2 && flag.Arg(2) == "release" { + case flag.NArg() > 2 && flag.Arg(2) == "release": fmt.Fprint(os.Stderr, datasetReleaseUsage) - } else { + default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), datasetUsage) } @@ -273,7 +270,7 @@ func handleUserCommand() error { } err := user.List(apiURI, token) if err != nil { - return fmt.Errorf("Error: failed to get users, reason: %v\n", err) + return fmt.Errorf("error: failed to get users, reason: %v", err) } default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), userUsage) @@ -286,7 +283,10 @@ func handleFileListCommand() error { listFilesCmd := flag.NewFlagSet("list", flag.ExitOnError) var username string listFilesCmd.StringVar(&username, "user", "", "Filter files by username") - listFilesCmd.Parse(flag.Args()[2:]) + + if err := listFilesCmd.Parse(flag.Args()[2:]); err != nil { + return fmt.Errorf("error: failed to parse command line arguments, reason: %v", err) + } // Check if the -user flag was provided if username == "" { @@ -298,7 +298,7 @@ func handleFileListCommand() error { } if err := file.List(apiURI, token, username); err != nil { - return fmt.Errorf("Error: failed to get files, reason: %v\n", err) + return fmt.Errorf("error: failed to get files, reason: %v", err) } return nil @@ -333,7 +333,10 @@ func handleFileIngestCommand() error { var filepath, username string fileIngestCmd.StringVar(&filepath, "filepath", "", "Filepath to ingest") fileIngestCmd.StringVar(&username, "user", "", "Username to associate with the file") - fileIngestCmd.Parse(flag.Args()[2:]) + + if err := fileIngestCmd.Parse(flag.Args()[2:]); err != nil { + return fmt.Errorf("error: failed to parse command line arguments, reason: %v", err) + } if filepath == "" || username == "" { return fmt.Errorf("Error: both -filepath and -user are required.\n%s", fileIngestUsage) @@ -349,11 +352,11 @@ func handleFileIngestCommand() error { err := file.Ingest(apiURI, token, username, filepath) if err != nil { - return fmt.Errorf("Error: failed to ingest file, reason: %v\n", err) - } else { - fmt.Println("File ingestion triggered successfully.") + return fmt.Errorf("error: failed to ingest file, reason: %v", err) } + fmt.Println("File ingestion triggered successfully.") + return nil } @@ -363,10 +366,13 @@ func handleFileAccessionCommand() error { fileAccessionCmd.StringVar(&filepath, "filepath", "", "Filepath to assign accession ID") fileAccessionCmd.StringVar(&username, "user", "", "Username to associate with the file") fileAccessionCmd.StringVar(&accessionID, "accession-id", "", "Accession ID to assign") - fileAccessionCmd.Parse(flag.Args()[2:]) + + if err := fileAccessionCmd.Parse(flag.Args()[2:]); err != nil { + return fmt.Errorf("error: failed to parse command line arguments, reason: %v", err) + } if filepath == "" || username == "" || accessionID == "" { - return fmt.Errorf("Error: -filepath, -user, and -accession-id are required.\n%s", fileAccessionUsage) + return fmt.Errorf("error: -filepath, -user, and -accession-id are required.\n%s", fileAccessionUsage) } if err := helpers.CheckValidChars(filepath); err != nil { @@ -379,11 +385,11 @@ func handleFileAccessionCommand() error { err := file.SetAccession(apiURI, token, username, filepath, accessionID) if err != nil { - return fmt.Errorf("Error: failed to assign accession ID to file, reason: %v\n", err) - } else { - fmt.Println("Accession ID assigned to file successfully.") + return fmt.Errorf("error: failed to assign accession ID to file, reason: %v", err) } + fmt.Println("Accession ID assigned to file successfully.") + return nil } @@ -412,7 +418,11 @@ func handleDatasetCreateCommand() error { datasetCreateCmd := flag.NewFlagSet("create", flag.ExitOnError) var datasetID string datasetCreateCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to create") - datasetCreateCmd.Parse(flag.Args()[2:]) + + if err := datasetCreateCmd.Parse(flag.Args()[2:]); err != nil { + return fmt.Errorf("error: failed to parse command line arguments, reason: %v", err) + } + accessionIDs := datasetCreateCmd.Args() // Args() returns the non-flag arguments after parsing if datasetID == "" || len(accessionIDs) == 0 { @@ -425,11 +435,11 @@ func handleDatasetCreateCommand() error { err := dataset.Create(apiURI, token, datasetID, accessionIDs) if err != nil { - return fmt.Errorf("Error: failed to create dataset, reason: %v\n", err) - } else { - fmt.Println("Dataset created successfully.") + return fmt.Errorf("error: failed to create dataset, reason: %v", err) } + fmt.Println("Dataset created successfully.") + return nil } @@ -437,10 +447,13 @@ func handleDatasetReleaseCommand() error { datasetReleaseCmd := flag.NewFlagSet("release", flag.ExitOnError) var datasetID string datasetReleaseCmd.StringVar(&datasetID, "dataset-id", "", "ID of the dataset to release") - datasetReleaseCmd.Parse(flag.Args()[2:]) + + if err := datasetReleaseCmd.Parse(flag.Args()[2:]); err != nil { + return fmt.Errorf("error: failed to parse command line arguments, reason: %v", err) + } if datasetID == "" { - return fmt.Errorf("Error: -dataset-id is required.\n%s", datasetReleaseUsage) + return fmt.Errorf("error: -dataset-id is required.\n%s", datasetReleaseUsage) } if err := helpers.CheckTokenExpiration(token); err != nil { @@ -449,11 +462,11 @@ func handleDatasetReleaseCommand() error { err := dataset.Release(apiURI, token, datasetID) if err != nil { - return fmt.Errorf("Error: failed to release dataset, reason: %v\n", err) - } else { - fmt.Println("Dataset released successfully.") + return fmt.Errorf("error: failed to release dataset, reason: %v", err) } + fmt.Println("Dataset released successfully.") + return nil } From 744d35bc034341bc4189e4b3cefeae13d7a01aba Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 21:16:38 +0000 Subject: [PATCH 26/33] do not check token on the cli side --- sda-admin/helpers/helpers.go | 66 ------------------------------- sda-admin/helpers/helpers_test.go | 50 ----------------------- sda-admin/main.go | 23 ----------- 3 files changed, 139 deletions(-) diff --git a/sda-admin/helpers/helpers.go b/sda-admin/helpers/helpers.go index 464cbce73..500f02671 100644 --- a/sda-admin/helpers/helpers.go +++ b/sda-admin/helpers/helpers.go @@ -2,79 +2,13 @@ package helpers import ( "bytes" - "encoding/json" "fmt" "io" "net/http" - "os" "regexp" - "strconv" "strings" - "time" - - "github.com/golang-jwt/jwt" ) -// Helper functions used by more than one module - -// CheckTokenExpiration is used to determine whether the token is expiring in less than a day -func CheckTokenExpiration(accessToken string) error { - // Parse jwt token with unverified, since we don't need to check the signatures here - token, _, err := new(jwt.Parser).ParseUnverified(accessToken, jwt.MapClaims{}) - if err != nil { - return fmt.Errorf("could not parse token, reason: %s", err) - } - - claims, ok := token.Claims.(jwt.MapClaims) - if !ok { - return fmt.Errorf("broken token (claims are empty): %v\nerror: %s", claims, err) - } - - // Check if the token has exp claim - if claims["exp"] == nil { - return fmt.Errorf("could not parse token, reason: no expiration date") - } - - // Parse the expiration date from token, handle cases where - // the date format is nonstandard, e.g. test tokens are used - var expiration time.Time - switch iat := claims["exp"].(type) { - case float64: - expiration = time.Unix(int64(iat), 0) - case json.Number: - tmp, _ := iat.Int64() - expiration = time.Unix(tmp, 0) - case string: - i, err := strconv.ParseInt(iat, 10, 64) - if err != nil { - return fmt.Errorf("could not parse token, reason: %s", err) - } - expiration = time.Unix(int64(i), 0) - default: - return fmt.Errorf("could not parse token, reason: unknown expiration date format") - } - - switch untilExp := time.Until(expiration); { - case untilExp < 0: - return fmt.Errorf("the provided access token has expired, please renew it") - case untilExp > 0 && untilExp < 24*time.Hour: - fmt.Fprintln( - os.Stderr, - "The provided access token expires in", - time.Until(expiration).Truncate(time.Second), - ) - fmt.Fprintln(os.Stderr, "Consider renewing the token.") - default: - fmt.Fprintln( - os.Stderr, - "The provided access token expires in", - time.Until(expiration).Truncate(time.Second), - ) - } - - return nil -} - // necessary for mocking in unit tests var GetResponseBody = GetBody diff --git a/sda-admin/helpers/helpers_test.go b/sda-admin/helpers/helpers_test.go index 71b971685..6829d1ecd 100644 --- a/sda-admin/helpers/helpers_test.go +++ b/sda-admin/helpers/helpers_test.go @@ -1,64 +1,14 @@ package helpers import ( - "crypto/rand" - "crypto/rsa" "fmt" - "log" "net/http" "net/http/httptest" "testing" - "time" - "github.com/golang-jwt/jwt" "github.com/stretchr/testify/assert" ) -// generate jwts for testing the expDate -func generateDummyToken(expDate int64) string { - // Generate a new private key - privateKey, err := rsa.GenerateKey(rand.Reader, 2048) - if err != nil { - log.Fatalf("Failed to generate private key: %s", err) - } - - // Create the Claims - claims := &jwt.StandardClaims{ - Issuer: "test", - } - if expDate != 0 { - claims = &jwt.StandardClaims{ - ExpiresAt: expDate, - Issuer: "test", - } - } - - token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) - ss, err := token.SignedString(privateKey) - if err != nil { - log.Fatalf("Failed to sign token: %s", err) - } - - return ss -} - -func TestTokenExpiration(t *testing.T) { - // Token without exp claim - token := generateDummyToken(0) - err := CheckTokenExpiration(token) - assert.EqualError(t, err, "could not parse token, reason: no expiration date") - - // Token with expired date - token = generateDummyToken(time.Now().Unix()) - err = CheckTokenExpiration(token) - assert.EqualError(t, err, "the provided access token has expired, please renew it") - - // Token with valid expiration - token = generateDummyToken(time.Now().Add(time.Hour * 72).Unix()) - err = CheckTokenExpiration(token) - assert.NoError(t, err) -} - func TestGetBody(t *testing.T) { // Mock server mockResponse := `{"key": "value"}` diff --git a/sda-admin/main.go b/sda-admin/main.go index 4516186cc..4b4671d9a 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -265,9 +265,6 @@ func handleUserCommand() error { } switch flag.Arg(1) { case "list": - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } err := user.List(apiURI, token) if err != nil { return fmt.Errorf("error: failed to get users, reason: %v", err) @@ -293,10 +290,6 @@ func handleFileListCommand() error { return fmt.Errorf("Error: the -user flag is required.\n%s", fileListUsage) } - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - if err := file.List(apiURI, token, username); err != nil { return fmt.Errorf("error: failed to get files, reason: %v", err) } @@ -346,10 +339,6 @@ func handleFileIngestCommand() error { return err } - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - err := file.Ingest(apiURI, token, username, filepath) if err != nil { return fmt.Errorf("error: failed to ingest file, reason: %v", err) @@ -379,10 +368,6 @@ func handleFileAccessionCommand() error { return err } - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - err := file.SetAccession(apiURI, token, username, filepath, accessionID) if err != nil { return fmt.Errorf("error: failed to assign accession ID to file, reason: %v", err) @@ -429,10 +414,6 @@ func handleDatasetCreateCommand() error { return fmt.Errorf("Error: -dataset-id and at least one accession ID are required.\n%s", datasetCreateUsage) } - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - err := dataset.Create(apiURI, token, datasetID, accessionIDs) if err != nil { return fmt.Errorf("error: failed to create dataset, reason: %v", err) @@ -456,10 +437,6 @@ func handleDatasetReleaseCommand() error { return fmt.Errorf("error: -dataset-id is required.\n%s", datasetReleaseUsage) } - if err := helpers.CheckTokenExpiration(token); err != nil { - return err - } - err := dataset.Release(apiURI, token, datasetID) if err != nil { return fmt.Errorf("error: failed to release dataset, reason: %v", err) From af5855aefcdbffc5b81a4ca84ebf8bb2ec8ecfc5 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Thu, 12 Sep 2024 21:43:30 +0000 Subject: [PATCH 27/33] show response from API when server returned non 200 code --- sda-admin/helpers/helpers.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/sda-admin/helpers/helpers.go b/sda-admin/helpers/helpers.go index 500f02671..9272bfa27 100644 --- a/sda-admin/helpers/helpers.go +++ b/sda-admin/helpers/helpers.go @@ -29,11 +29,7 @@ func GetBody(url, token string) ([]byte, error) { if err != nil { return nil, fmt.Errorf("failed to get response, reason: %v", err) } - - // Check the status code - if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("server returned status %d", res.StatusCode) - } + defer res.Body.Close() // Read the response body resBody, err := io.ReadAll(res.Body) @@ -41,7 +37,10 @@ func GetBody(url, token string) ([]byte, error) { return nil, fmt.Errorf("failed to read response body, reason: %v", err) } - defer res.Body.Close() + // Check the status code + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned status %d: %s", res.StatusCode, string(resBody)) + } return resBody, nil } @@ -69,17 +68,17 @@ func PostReq(url, token string, jsonBody []byte) ([]byte, error) { } defer res.Body.Close() - // Check the status code - if res.StatusCode != http.StatusOK { - return nil, fmt.Errorf("server returned status %d", res.StatusCode) - } - // Read the response body resBody, err := io.ReadAll(res.Body) if err != nil { return nil, fmt.Errorf("failed to read response body, reason: %v", err) } + // Check the status code + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("server returned status %d: %s", res.StatusCode, string(resBody)) + } + return resBody, nil } From 96bbad6462a70d1100e3c3bf1588ed49e4b91292 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Sat, 14 Sep 2024 09:48:21 +0200 Subject: [PATCH 28/33] Better logic and remove unnecessary output Co-authored-by: Joakim Bygdell --- sda-admin/main.go | 63 +++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index 4b4671d9a..afee5f448 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -20,8 +20,7 @@ var ( ) // Command-line usage -var usage = ` -Usage: +const usage = `Usage: sda-admin [-uri URI] [-token TOKEN] [options] Commands: @@ -42,8 +41,7 @@ Global Options: Additional Commands: help Show this help message. - -h, -help Show this help message. -` + -h, -help Show this help message.` var userUsage = ` List Users: @@ -187,28 +185,29 @@ func parseFlagsAndEnv() error { } func handleHelpCommand() error { - if flag.NArg() > 1 { - switch flag.Arg(1) { - case "user": - if err := handleHelpUser(); err != nil { - return err - } - case "file": - if err := handleHelpFile(); err != nil { - return err - } - case "dataset": - if err := handleHelpDataset(); err != nil { - return err - } - case "version": - fmt.Fprint(os.Stderr, versionUsage) - default: - return fmt.Errorf("Unknown command '%s'.\n%s", flag.Arg(1), usage) + if flag.NArg() == 1 { + fmt.Println(usage) + + return nil + } + switch flag.Arg(1) { + case "user": + if err := handleHelpUser(); err != nil { + return err } - } else { - fmt.Fprint(os.Stderr, usage) + case "file": + if err := handleHelpFile(); err != nil { + return err + } + case "dataset": + if err := handleHelpDataset(); err != nil { + return err + } + case "version": + fmt.Println(versionUsage) + default: + return fmt.Errorf("unknown command '%s'.\n%s", flag.Arg(1), usage) } return nil @@ -218,7 +217,7 @@ func handleHelpUser() error { switch { case flag.NArg() == 2: fmt.Fprint(os.Stderr, userUsage) - case flag.NArg() > 2 && flag.Arg(2) == "list": + case flag.Arg(2) == "list": fmt.Fprint(os.Stderr, userListUsage) default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) @@ -231,11 +230,11 @@ func handleHelpFile() error { switch { case flag.NArg() == 2: fmt.Fprint(os.Stderr, fileUsage) - case flag.NArg() > 2 && flag.Arg(2) == "list": + case flag.Arg(2) == "list": fmt.Fprint(os.Stderr, fileListUsage) - case flag.NArg() > 2 && flag.Arg(2) == "ingest": + case flag.Arg(2) == "ingest": fmt.Fprint(os.Stderr, fileIngestUsage) - case flag.NArg() > 2 && flag.Arg(2) == "set-accession": + case flag.Arg(2) == "set-accession": fmt.Fprint(os.Stderr, fileAccessionUsage) default: return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) @@ -373,8 +372,6 @@ func handleFileAccessionCommand() error { return fmt.Errorf("error: failed to assign accession ID to file, reason: %v", err) } - fmt.Println("Accession ID assigned to file successfully.") - return nil } @@ -411,7 +408,7 @@ func handleDatasetCreateCommand() error { accessionIDs := datasetCreateCmd.Args() // Args() returns the non-flag arguments after parsing if datasetID == "" || len(accessionIDs) == 0 { - return fmt.Errorf("Error: -dataset-id and at least one accession ID are required.\n%s", datasetCreateUsage) + return fmt.Errorf("error: -dataset-id and at least one accession ID are required.\n%s", datasetCreateUsage) } err := dataset.Create(apiURI, token, datasetID, accessionIDs) @@ -419,8 +416,6 @@ func handleDatasetCreateCommand() error { return fmt.Errorf("error: failed to create dataset, reason: %v", err) } - fmt.Println("Dataset created successfully.") - return nil } @@ -442,8 +437,6 @@ func handleDatasetReleaseCommand() error { return fmt.Errorf("error: failed to release dataset, reason: %v", err) } - fmt.Println("Dataset released successfully.") - return nil } From 86a60600452f4d029aaf68f5a5a9e4a61620b593 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Sat, 14 Sep 2024 08:05:03 +0000 Subject: [PATCH 29/33] Decapitalize error message --- sda-admin/main.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index afee5f448..3f2434fa3 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -220,7 +220,7 @@ func handleHelpUser() error { case flag.Arg(2) == "list": fmt.Fprint(os.Stderr, userListUsage) default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) } return nil @@ -237,7 +237,7 @@ func handleHelpFile() error { case flag.Arg(2) == "set-accession": fmt.Fprint(os.Stderr, fileAccessionUsage) default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) } return nil @@ -252,7 +252,7 @@ func handleHelpDataset() error { case flag.NArg() > 2 && flag.Arg(2) == "release": fmt.Fprint(os.Stderr, datasetReleaseUsage) default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), datasetUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), datasetUsage) } return nil @@ -260,7 +260,7 @@ func handleHelpDataset() error { func handleUserCommand() error { if flag.NArg() < 2 { - return fmt.Errorf("Error: 'user' requires a subcommand (list).\n%s", userUsage) + return fmt.Errorf("error: 'user' requires a subcommand (list).\n%s", userUsage) } switch flag.Arg(1) { case "list": @@ -269,7 +269,7 @@ func handleUserCommand() error { return fmt.Errorf("error: failed to get users, reason: %v", err) } default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), userUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), userUsage) } return nil @@ -286,7 +286,7 @@ func handleFileListCommand() error { // Check if the -user flag was provided if username == "" { - return fmt.Errorf("Error: the -user flag is required.\n%s", fileListUsage) + return fmt.Errorf("error: the -user flag is required.\n%s", fileListUsage) } if err := file.List(apiURI, token, username); err != nil { @@ -298,7 +298,7 @@ func handleFileListCommand() error { func handleFileCommand() error { if flag.NArg() < 2 { - return fmt.Errorf("Error: 'file' requires a subcommand (list, ingest, set-accession).\n%s", fileUsage) + return fmt.Errorf("error: 'file' requires a subcommand (list, ingest, set-accession).\n%s", fileUsage) } switch flag.Arg(1) { case "list": @@ -314,7 +314,7 @@ func handleFileCommand() error { return err } default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), fileUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), fileUsage) } return nil @@ -331,7 +331,7 @@ func handleFileIngestCommand() error { } if filepath == "" || username == "" { - return fmt.Errorf("Error: both -filepath and -user are required.\n%s", fileIngestUsage) + return fmt.Errorf("error: both -filepath and -user are required.\n%s", fileIngestUsage) } if err := helpers.CheckValidChars(filepath); err != nil { @@ -377,7 +377,7 @@ func handleFileAccessionCommand() error { func handleDatasetCommand() error { if flag.NArg() < 2 { - return fmt.Errorf("Error: 'dataset' requires a subcommand (create, release).\n%s", datasetUsage) + return fmt.Errorf("error: 'dataset' requires a subcommand (create, release).\n%s", datasetUsage) } switch flag.Arg(1) { @@ -390,7 +390,7 @@ func handleDatasetCommand() error { return err } default: - return fmt.Errorf("Unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), datasetUsage) + return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(1), flag.Arg(0), datasetUsage) } return nil From 2a90be2cda9d8cece53a5b919c98aec4a5025287 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Sat, 14 Sep 2024 08:53:16 +0000 Subject: [PATCH 30/33] improved url concatenation to handle when datasetID is URL like --- sda-admin/dataset/dataset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index 88080ed87..d16f74cc6 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -46,7 +46,7 @@ func Release(apiURI, token, datasetID string) error { if err != nil { return err } - parsedURL.Path = path.Join(parsedURL.Path, "dataset/release", datasetID) + parsedURL.Path = path.Join(parsedURL.Path, "dataset/release") + "/" + datasetID jsonBody := []byte("{}") _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) From e2dc2424a571db1a77bccaa359537ce96d56c390 Mon Sep 17 00:00:00 2001 From: Nanjiang Shu Date: Mon, 16 Sep 2024 12:52:08 +0200 Subject: [PATCH 31/33] Improved usage message and the logic of outputing to stdout and stderr Co-authored-by: Joakim Bygdell --- sda-admin/main.go | 90 +++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 57 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index 3f2434fa3..0be639bb8 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -20,8 +20,7 @@ var ( ) // Command-line usage -const usage = `Usage: - sda-admin [-uri URI] [-token TOKEN] [options] +const usage = `Usage: sda-admin [-uri URI] [-token TOKEN] [options] Commands: user list List all users. @@ -43,19 +42,14 @@ Additional Commands: help Show this help message. -h, -help Show this help message.` -var userUsage = ` -List Users: +var userUsage = `List Users: Usage: sda-admin user list - List all users in the system. -` + List all users in the system with ongoing submissions.` -var userListUsage = ` -Usage: sda-admin user list - List all users in the system. -` +var userListUsage = `Usage: sda-admin user list + List all users in the system with ongoing submissions.` -var fileUsage = ` -List all files for a user: +var fileUsage = `List all files for a user: Usage: sda-admin file list -user USERNAME List all files for a specified user. @@ -72,38 +66,30 @@ Options: -filepath FILEPATH Specify the path of the file to ingest. -accession-id ID Specify the accession ID to assign to the file. -Use 'sda-admin help file ' for information on a specific command. -` +Use 'sda-admin help file ' for information on a specific command.` -var fileListUsage = ` -Usage: sda-admin file list -user USERNAME +var fileListUsage = `Usage: sda-admin file list -user USERNAME List all files for a specified user. Options: - -user USERNAME Specify the username associated with the files. -` + -user USERNAME Specify the username associated with the files.` -var fileIngestUsage = ` -Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME +var fileIngestUsage = `Usage: sda-admin file ingest -filepath FILEPATH -user USERNAME Trigger the ingestion of a given file for a specific user. Options: -filepath FILEPATH Specify the path of the file to ingest. - -user USERNAME Specify the username associated with the file. -` + -user USERNAME Specify the username associated with the file.` -var fileAccessionUsage = ` -Usage: sda-admin file set-accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID +var fileAccessionUsage = `Usage: sda-admin file set-accession -filepath FILEPATH -user USERNAME -accession-id ACCESSION_ID Assign accession ID to a file and associate it with a user. Options: -filepath FILEPATH Specify the path of the file to assign the accession ID. -user USERNAME Specify the username associated with the file. - -accession-id ID Specify the accession ID to assign to the file. -` + -accession-id ID Specify the accession ID to assign to the file.` -var datasetUsage = ` -Create a dataset: +var datasetUsage = `Create a dataset: Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] Create a dataset from a list of accession IDs and a dataset ID. @@ -115,30 +101,23 @@ Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset. [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. -Use 'sda-admin help dataset ' for information on a specific command. -` +Use 'sda-admin help dataset ' for information on a specific command.` -var datasetCreateUsage = ` -Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] +var datasetCreateUsage = `Usage: sda-admin dataset create -dataset-id DATASET_ID [ACCESSION_ID ...] Create a dataset from a list of accession IDs and a dataset ID. Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset. - [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset. -` + [ACCESSION_ID ...] (For dataset create) Specify one or more accession IDs to include in the dataset.` -var datasetReleaseUsage = ` -Usage: sda-admin dataset release -dataset-id DATASET_ID +var datasetReleaseUsage = `Usage: sda-admin dataset release -dataset-id DATASET_ID Release a dataset for downloading based on its dataset ID. Options: - -dataset-id DATASET_ID Specify the unique identifier for the dataset. -` + -dataset-id DATASET_ID Specify the unique identifier for the dataset.` -var versionUsage = ` -Usage: sda-admin version - Show the version information for sda-admin. -` +var versionUsage = `Usage: sda-admin version + Show the version information for sda-admin.` func printVersion() { fmt.Printf("sda-admin version %s\n", version) @@ -216,9 +195,9 @@ func handleHelpCommand() error { func handleHelpUser() error { switch { case flag.NArg() == 2: - fmt.Fprint(os.Stderr, userUsage) + fmt.Println(userUsage) case flag.Arg(2) == "list": - fmt.Fprint(os.Stderr, userListUsage) + fmt.Println(userListUsage) default: return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), userUsage) } @@ -229,13 +208,13 @@ func handleHelpUser() error { func handleHelpFile() error { switch { case flag.NArg() == 2: - fmt.Fprint(os.Stderr, fileUsage) + fmt.Println(fileUsage) case flag.Arg(2) == "list": - fmt.Fprint(os.Stderr, fileListUsage) + fmt.Println(fileListUsage) case flag.Arg(2) == "ingest": - fmt.Fprint(os.Stderr, fileIngestUsage) + fmt.Println(fileIngestUsage) case flag.Arg(2) == "set-accession": - fmt.Fprint(os.Stderr, fileAccessionUsage) + fmt.Println(fileAccessionUsage) default: return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), fileUsage) } @@ -246,11 +225,11 @@ func handleHelpFile() error { func handleHelpDataset() error { switch { case flag.NArg() == 2: - fmt.Fprint(os.Stderr, datasetUsage) - case flag.NArg() > 2 && flag.Arg(2) == "create": - fmt.Fprint(os.Stderr, datasetCreateUsage) - case flag.NArg() > 2 && flag.Arg(2) == "release": - fmt.Fprint(os.Stderr, datasetReleaseUsage) + fmt.Println(datasetUsage) + case flag.Arg(2) == "create": + fmt.Println(datasetCreateUsage) + case flag.Arg(2) == "release": + fmt.Println(datasetReleaseUsage) default: return fmt.Errorf("unknown subcommand '%s' for '%s'.\n%s", flag.Arg(2), flag.Arg(1), datasetUsage) } @@ -343,8 +322,6 @@ func handleFileIngestCommand() error { return fmt.Errorf("error: failed to ingest file, reason: %v", err) } - fmt.Println("File ingestion triggered successfully.") - return nil } @@ -470,8 +447,7 @@ func main() { case "version": printVersion() default: - fmt.Fprintf(os.Stderr, "Unknown command '%s'.\n", flag.Arg(0)) - fmt.Fprint(os.Stderr, usage) + fmt.Fprintf(os.Stderr, "unknown command '%s'.\n%s\n", flag.Arg(0), usage) os.Exit(1) } From ae67160a698e275d02e0c622b1c8599c08f64d71 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Tue, 17 Sep 2024 13:46:53 +0000 Subject: [PATCH 32/33] refactor: remove non useful components --- sda-admin/main.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/sda-admin/main.go b/sda-admin/main.go index 0be639bb8..0b28df25a 100644 --- a/sda-admin/main.go +++ b/sda-admin/main.go @@ -39,6 +39,7 @@ Global Options: -token TOKEN Set the authentication token (optional if ACCESS_TOKEN is set). Additional Commands: + version Show the version of sda-admin. help Show this help message. -h, -help Show this help message.` @@ -116,11 +117,8 @@ var datasetReleaseUsage = `Usage: sda-admin dataset release -dataset-id DATASET_ Options: -dataset-id DATASET_ID Specify the unique identifier for the dataset.` -var versionUsage = `Usage: sda-admin version - Show the version information for sda-admin.` - func printVersion() { - fmt.Printf("sda-admin version %s\n", version) + fmt.Printf("sda-admin %s\n", version) } func parseFlagsAndEnv() error { @@ -141,7 +139,7 @@ func parseFlagsAndEnv() error { return errors.New(usage) } - if flag.Arg(0) == "help" { + if flag.Arg(0) == "help" || flag.Arg(0) == "version" { return nil } @@ -183,8 +181,6 @@ func handleHelpCommand() error { if err := handleHelpDataset(); err != nil { return err } - case "version": - fmt.Println(versionUsage) default: return fmt.Errorf("unknown command '%s'.\n%s", flag.Arg(1), usage) } From 95c6bf96b2c8765ea2a49caa5d592a0d362d7578 Mon Sep 17 00:00:00 2001 From: nanjiangshu Date: Wed, 18 Sep 2024 07:14:50 +0000 Subject: [PATCH 33/33] pass nil as the jsonBody for the dataset release function --- sda-admin/dataset/dataset.go | 4 +--- sda-admin/dataset/dataset_test.go | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/sda-admin/dataset/dataset.go b/sda-admin/dataset/dataset.go index d16f74cc6..457b35b2a 100644 --- a/sda-admin/dataset/dataset.go +++ b/sda-admin/dataset/dataset.go @@ -48,9 +48,7 @@ func Release(apiURI, token, datasetID string) error { } parsedURL.Path = path.Join(parsedURL.Path, "dataset/release") + "/" + datasetID - jsonBody := []byte("{}") - _, err = helpers.PostRequest(parsedURL.String(), token, jsonBody) - + _, err = helpers.PostRequest(parsedURL.String(), token, nil) if err != nil { return err } diff --git a/sda-admin/dataset/dataset_test.go b/sda-admin/dataset/dataset_test.go index 767645cc0..c0309b9d7 100644 --- a/sda-admin/dataset/dataset_test.go +++ b/sda-admin/dataset/dataset_test.go @@ -67,9 +67,8 @@ func TestRelease_Success(t *testing.T) { expectedURL := "http://example.com/dataset/release/dataset-123" token := "test-token" - jsonBody := []byte(`{}`) - mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(`{}`), nil) + mockHelpers.On("PostRequest", expectedURL, token, []byte(nil)).Return([]byte(`{}`), nil) err := Release("http://example.com", token, "dataset-123") assert.NoError(t, err) @@ -84,9 +83,8 @@ func TestRelease_PostRequestFailure(t *testing.T) { expectedURL := "http://example.com/dataset/release/dataset-123" token := "test-token" - jsonBody := []byte(`{}`) - mockHelpers.On("PostRequest", expectedURL, token, jsonBody).Return([]byte(nil), errors.New("failed to send request")) + mockHelpers.On("PostRequest", expectedURL, token, []byte(nil)).Return([]byte(nil), errors.New("failed to send request")) err := Release("http://example.com", token, "dataset-123") assert.Error(t, err)