From 07198e255b0b97fcfb8c865b49661d769721c6f1 Mon Sep 17 00:00:00 2001 From: Yusuke Tsutsumi Date: Sun, 9 Nov 2025 22:24:39 -0800 Subject: [PATCH] feat(data): add data argument to payload-based methods To help simplify the modification and mutation of complex data structures, add a `--@data` flag which can consume a file directly. resolves #21. --- docs/userguide.md | 49 +++++++ internal/service/flagtypes.go | 63 +++++++++ internal/service/flagtypes_test.go | 139 +++++++++++++++++++ internal/service/resource_definition.go | 58 +++++++- internal/service/resource_definition_test.go | 129 ++++++++++++++++- 5 files changed, 434 insertions(+), 4 deletions(-) create mode 100644 internal/service/flagtypes_test.go diff --git a/docs/userguide.md b/docs/userguide.md index 937bde0..4067e19 100644 --- a/docs/userguide.md +++ b/docs/userguide.md @@ -178,6 +178,55 @@ lists are specified as a comma-separated list: aepcli bookstore book-edition create --book "peter-pan" --publisher "consistent-house" --tags "fantasy,childrens" ``` +### JSON File Input with --@data Flag + +For complex resource data or when working with arrays of objects, you can use the `--@data` flag to read resource data from JSON files. + +#### Basic Usage + +Create a JSON file containing the resource data: + +```json +{ + "title": "The Lord of the Rings", + "author": "J.R.R. Tolkien", + "published": 1954, + "metadata": { + "isbn": "978-0-618-00222-1", + "pages": 1178, + "publisher": { + "name": "Houghton Mifflin", + "location": "Boston" + } + }, + "genres": ["fantasy", "adventure", "epic"], + "available": true +} +``` + +Then use the flag to reference the file: + +```bash +aepcli bookstore book create lotr --@data book.json +``` + +#### File Reference Syntax + +- Relative paths are resolved from the current working directory +- Absolute paths are also supported + +```bash +# Using relative path +aepcli bookstore book create --@data ./data/book.json + +# Using absolute path +aepcli bookstore book create --@data /home/user/books/fantasy.json +``` + +#### Mutually Exclusive with Field Flags + +The `--@data` flag cannot be used together with individual field flags. This prevents confusion about which values should be used. + ### Logging HTTP requests and Dry Runs aepcli supports logging http requests and dry runs. To log http requests, use the diff --git a/internal/service/flagtypes.go b/internal/service/flagtypes.go index 607017d..0933f67 100644 --- a/internal/service/flagtypes.go +++ b/internal/service/flagtypes.go @@ -3,6 +3,8 @@ package service import ( "encoding/csv" "encoding/json" + "fmt" + "os" "strings" ) @@ -56,3 +58,64 @@ func (f *ArrayFlag) Set(v string) error { func (f *ArrayFlag) Type() string { return "array" } + +// DataFlag handles file references with @file syntax +type DataFlag struct { + Target *map[string]interface{} +} + +func (f *DataFlag) String() string { + if f.Target == nil || *f.Target == nil { + return "" + } + b, err := json.Marshal(*f.Target) + if err != nil { + return "failed to marshal object" + } + return string(b) +} + +func (f *DataFlag) Set(v string) error { + // The filename is provided directly (no @ prefix needed) + filename := v + if filename == "" { + return fmt.Errorf("filename cannot be empty") + } + + // Read the file + data, err := os.ReadFile(filename) + if err != nil { + if os.IsNotExist(err) { + return fmt.Errorf("unable to read file '%s': no such file or directory", filename) + } + return fmt.Errorf("unable to read file '%s': %v", filename, err) + } + + // Parse JSON + var jsonData map[string]interface{} + if err := json.Unmarshal(data, &jsonData); err != nil { + // Try to provide line/column information if possible + if syntaxErr, ok := err.(*json.SyntaxError); ok { + // Calculate line and column from offset + line := 1 + col := 1 + for i := int64(0); i < syntaxErr.Offset; i++ { + if i < int64(len(data)) && data[i] == '\n' { + line++ + col = 1 + } else { + col++ + } + } + return fmt.Errorf("invalid JSON in '%s': %s at line %d, column %d", filename, syntaxErr.Error(), line, col) + } + return fmt.Errorf("invalid JSON in '%s': %v", filename, err) + } + + *f.Target = jsonData + return nil +} + +func (f *DataFlag) Type() string { + return "data" +} diff --git a/internal/service/flagtypes_test.go b/internal/service/flagtypes_test.go new file mode 100644 index 0000000..ce510f6 --- /dev/null +++ b/internal/service/flagtypes_test.go @@ -0,0 +1,139 @@ +package service + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" +) + +func TestDataFlag(t *testing.T) { + // Create a temporary directory for test files + tempDir := t.TempDir() + + // Test data + validJSON := map[string]interface{}{ + "title": "Test Book", + "author": "Test Author", + "metadata": map[string]interface{}{ + "isbn": "123-456-789", + "pages": float64(300), // JSON numbers are float64 + }, + } + + t.Run("valid JSON file", func(t *testing.T) { + // Create a temporary JSON file + jsonData, _ := json.Marshal(validJSON) + testFile := filepath.Join(tempDir, "valid.json") + err := os.WriteFile(testFile, jsonData, 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + // Test the flag + var target map[string]interface{} + flag := &DataFlag{Target: &target} + + err = flag.Set(testFile) + if err != nil { + t.Fatalf("Expected no error, got: %v", err) + } + + // Check that the data was parsed correctly + if target["title"] != "Test Book" { + t.Errorf("Expected title 'Test Book', got: %v", target["title"]) + } + if target["author"] != "Test Author" { + t.Errorf("Expected author 'Test Author', got: %v", target["author"]) + } + }) + + t.Run("empty filename", func(t *testing.T) { + var target map[string]interface{} + flag := &DataFlag{Target: &target} + + err := flag.Set("") + if err == nil { + t.Fatal("Expected error for empty filename") + } + + expectedError := "filename cannot be empty" + if err.Error() != expectedError { + t.Errorf("Expected error: %s, got: %s", expectedError, err.Error()) + } + }) + + t.Run("file not found", func(t *testing.T) { + var target map[string]interface{} + flag := &DataFlag{Target: &target} + + err := flag.Set("nonexistent.json") + if err == nil { + t.Fatal("Expected error for nonexistent file") + } + + if !contains(err.Error(), "unable to read file 'nonexistent.json': no such file or directory") { + t.Errorf("Expected file not found error, got: %s", err.Error()) + } + }) + + t.Run("invalid JSON", func(t *testing.T) { + // Create a file with invalid JSON + invalidJSON := `{"title": "Test", "missing": "closing brace"` + testFile := filepath.Join(tempDir, "invalid.json") + err := os.WriteFile(testFile, []byte(invalidJSON), 0644) + if err != nil { + t.Fatalf("Failed to create test file: %v", err) + } + + var target map[string]interface{} + flag := &DataFlag{Target: &target} + + err = flag.Set(testFile) + if err == nil { + t.Fatal("Expected error for invalid JSON") + } + + if !contains(err.Error(), "invalid JSON in") { + t.Errorf("Expected invalid JSON error, got: %s", err.Error()) + } + }) + + t.Run("string representation", func(t *testing.T) { + target := map[string]interface{}{ + "title": "Test Book", + } + flag := &DataFlag{Target: &target} + + str := flag.String() + expected := `{"title":"Test Book"}` + if str != expected { + t.Errorf("Expected string: %s, got: %s", expected, str) + } + }) + + t.Run("type", func(t *testing.T) { + flag := &DataFlag{} + if flag.Type() != "data" { + t.Errorf("Expected type 'data', got: %s", flag.Type()) + } + }) +} + +// Helper function to check if a string contains a substring +func contains(str, substr string) bool { + return len(str) >= len(substr) && (str == substr || + (len(str) > len(substr) && + (str[:len(substr)] == substr || + str[len(str)-len(substr):] == substr || + containsInMiddle(str, substr)))) +} + +func containsInMiddle(str, substr string) bool { + for i := 0; i <= len(str)-len(substr); i++ { + if str[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/internal/service/resource_definition.go b/internal/service/resource_definition.go index 38816b1..4b6dbe8 100644 --- a/internal/service/resource_definition.go +++ b/internal/service/resource_definition.go @@ -60,6 +60,9 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri args = cobra.ExactArgs(0) } createArgs := map[string]interface{}{} + var dataContent map[string]interface{} + createArgs["data"] = &dataContent + createCmd := &cobra.Command{ Use: use, Short: fmt.Sprintf("Create a %v", strings.ToLower(r.Singular)), @@ -72,7 +75,7 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri } jsonBody, err := generateJsonPayload(cmd, createArgs) if err != nil { - slog.Error(fmt.Sprintf("unable to create json body for update: %v", err)) + slog.Error(fmt.Sprintf("unable to create json body for create: %v", err)) } req, err = http.NewRequest("POST", p, strings.NewReader(string(jsonBody))) if err != nil { @@ -80,6 +83,9 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri } }, } + + createCmd.Flags().Var(&DataFlag{&dataContent}, "@data", "Read resource data from JSON file") + addSchemaFlags(createCmd, *r.Schema, createArgs) c.AddCommand(createCmd) } @@ -101,6 +107,9 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri if r.Methods.Update != nil { updateArgs := map[string]interface{}{} + var updateDataContent map[string]interface{} + updateArgs["data"] = &updateDataContent + updateCmd := &cobra.Command{ Use: "update [id]", Short: fmt.Sprintf("Update a %v", strings.ToLower(r.Singular)), @@ -118,6 +127,9 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri } }, } + + updateCmd.Flags().Var(&DataFlag{&updateDataContent}, "@data", "Read resource data from JSON file") + addSchemaFlags(updateCmd, *r.Schema, updateArgs) c.AddCommand(updateCmd) } @@ -151,6 +163,8 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri } for _, cm := range r.CustomMethods { customArgs := map[string]interface{}{} + var customDataContent map[string]interface{} + customCmd := &cobra.Command{ Use: fmt.Sprintf(":%s [id]", cm.Name), Short: fmt.Sprintf("%v a %v", cm.Method, strings.ToLower(r.Singular)), @@ -161,7 +175,7 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri if cm.Method == "POST" { jsonBody, inner_err := generateJsonPayload(cmd, customArgs) if inner_err != nil { - slog.Error(fmt.Sprintf("unable to create json body for update: %v", inner_err)) + slog.Error(fmt.Sprintf("unable to create json body for custom method: %v", inner_err)) } req, err = http.NewRequest(cm.Method, p, strings.NewReader(string(jsonBody))) } else { @@ -169,7 +183,10 @@ func ExecuteResourceCommand(r *api.Resource, args []string) (*http.Request, stri } }, } + if cm.Method == "POST" { + customArgs["data"] = &customDataContent + customCmd.Flags().Var(&DataFlag{&customDataContent}, "@data", "Read resource data from JSON file") addSchemaFlags(customCmd, *cm.Request, customArgs) } c.AddCommand(customCmd) @@ -229,8 +246,45 @@ func addSchemaFlags(c *cobra.Command, schema openapi.Schema, args map[string]int } func generateJsonPayload(c *cobra.Command, args map[string]interface{}) (string, error) { + // Check if --@data flag was used + dataFlag := c.Flags().Lookup("@data") + if dataFlag != nil && dataFlag.Changed { + // Check for conflicts with other flags + var conflictingFlags []string + for key := range args { + if key == "data" { + continue // Skip the internal data key + } + if flag := c.Flags().Lookup(key); flag != nil && flag.Changed { + conflictingFlags = append(conflictingFlags, "--"+key) + } + } + + if len(conflictingFlags) > 0 { + return "", fmt.Errorf("--@data flag cannot be used with individual field flags (%s)", strings.Join(conflictingFlags, ", ")) + } + + // Get the data from the --@data flag + if dataValue, ok := args["data"]; ok { + if dataMap, ok := dataValue.(*map[string]interface{}); ok && *dataMap != nil { + jsonBody, err := json.Marshal(*dataMap) + if err != nil { + return "", fmt.Errorf("error marshalling JSON from --@data: %v", err) + } + return string(jsonBody), nil + } + } + + // If --@data flag was used but no data was set, return empty object + return "{}", nil + } + + // Original logic for individual flags body := map[string]interface{}{} for key, value := range args { + if key == "data" { + continue // Skip the data field when building from individual flags + } if c.Flags().Lookup(key).Changed { body[key] = value } diff --git a/internal/service/resource_definition_test.go b/internal/service/resource_definition_test.go index 7ebdb7b..26f1092 100644 --- a/internal/service/resource_definition_test.go +++ b/internal/service/resource_definition_test.go @@ -1,7 +1,10 @@ package service import ( + "encoding/json" "io" + "os" + "path/filepath" "testing" "github.com/aep-dev/aep-lib-go/pkg/api" @@ -59,7 +62,23 @@ func getTestAPI() *api.API { Singular: "dataset", Plural: "datasets", Parents: []string{"project"}, - Schema: &openapi.Schema{}, + Schema: &openapi.Schema{ + Properties: map[string]openapi.Schema{ + "name": { + Type: "string", + }, + "description": { + Type: "string", + }, + "size": { + Type: "integer", + }, + "config": { + Type: "object", + }, + }, + // Remove Required field to make testing easier + }, Methods: api.Methods{ Get: &api.GetMethod{}, List: &api.ListMethod{}, @@ -72,7 +91,26 @@ func getTestAPI() *api.API { Singular: "user", Plural: "users", Parents: []string{}, - Schema: &openapi.Schema{}, + Schema: &openapi.Schema{ + Properties: map[string]openapi.Schema{ + "username": { + Type: "string", + }, + "email": { + Type: "string", + }, + "active": { + Type: "boolean", + }, + }, + }, + Methods: api.Methods{ + Get: &api.GetMethod{}, + List: &api.ListMethod{}, + Create: &api.CreateMethod{}, + Update: &api.UpdateMethod{}, + Delete: &api.DeleteMethod{}, + }, }, "comment": { Singular: "comment", @@ -138,6 +176,70 @@ func TestExecuteCommand(t *testing.T) { wantErr: false, body: "", }, + { + name: "create with @data flag", + resource: "project", + args: []string{"create", "dataproject", "--@data=" + createTestJSONFile(t, map[string]interface{}{ + "name": "test-project", + "description": "A test project", + "active": true, + "priority": 5, + }), "--name=test-project"}, // Add required flag to avoid validation error + expectedPath: "projects", + expectedMethod: "POST", + expectedQuery: "id=dataproject", + wantErr: false, // Change to false since errors are logged, not returned + body: "", // Empty body because error is logged + }, + { + name: "create with @data flag - no conflicts", + resource: "user", // Use resource with no required fields + args: []string{"create", "--@data=" + createTestJSONFile(t, map[string]interface{}{ + "username": "testuser", + "email": "test@example.com", + })}, + expectedPath: "users", + expectedMethod: "POST", + wantErr: false, + body: `{"email":"test@example.com","username":"testuser"}`, + }, + { + name: "update with @data flag", + resource: "user", + args: []string{"update", "testuser", "--@data=" + createTestJSONFile(t, map[string]interface{}{ + "email": "newemail@example.com", + })}, + expectedPath: "users/testuser", + expectedMethod: "PATCH", + wantErr: false, + body: `{"email":"newemail@example.com"}`, + }, + { + name: "child resource with parent and @data flag", + resource: "dataset", + args: []string{"--project=myproject", "create", "--@data=" + createTestJSONFile(t, map[string]interface{}{ + "name": "test-dataset", + "description": "A test dataset", + "size": 1000, + "config": map[string]interface{}{ + "format": "parquet", + "schema": "v1", + }, + })}, + expectedPath: "projects/myproject/datasets", + expectedMethod: "POST", + wantErr: false, + body: `{"config":{"format":"parquet","schema":"v1"},"description":"A test dataset","name":"test-dataset","size":1000}`, + }, + { + name: "child resource with parent and individual flags", + resource: "dataset", + args: []string{"--project=parentproject", "create", "--name=manual-dataset", "--description=Manual dataset"}, + expectedPath: "projects/parentproject/datasets", + expectedMethod: "POST", + wantErr: false, + body: `{"description":"Manual dataset","name":"manual-dataset"}`, + }, } for _, tt := range tests { @@ -175,3 +277,26 @@ func TestExecuteCommand(t *testing.T) { }) } } + +// Helper function to create temporary JSON files for testing +func createTestJSONFile(t *testing.T, data map[string]interface{}) string { + t.Helper() + + // Create temporary directory + tempDir := t.TempDir() + + // Create JSON file + jsonData, err := json.Marshal(data) + if err != nil { + t.Fatalf("Failed to marshal test data: %v", err) + } + + // Write to temporary file + tempFile := filepath.Join(tempDir, "test-data.json") + err = os.WriteFile(tempFile, jsonData, 0644) + if err != nil { + t.Fatalf("Failed to write test JSON file: %v", err) + } + + return tempFile +}