From 344ff88701416eba8da9301539df9d18784cbe2d Mon Sep 17 00:00:00 2001 From: MaxHuang22 Date: Fri, 10 Apr 2026 17:49:41 +0800 Subject: [PATCH] feat: add --file flag for multipart/form-data file uploads (#395) * feat(cmdutil): add shared file upload helpers Add ParseFileFlag, ValidateFileFlag, and BuildFormdata to support multipart file upload via --file flag across raw API and meta API commands. Change-Id: Ib724cf8b055b0b314af11d8d830f38559dac60eb * feat(api): add --file flag for multipart/form-data file uploads Add --file flag to `lark-cli api` command enabling file upload via multipart/form-data. The flag accepts [field=]path format and supports stdin (-). Includes mutual exclusion validation with --output, --page-all, and GET method. Dry-run mode shows file metadata instead of building actual formdata. Change-Id: Icf34aba5da3a558219a97a583e8f6aa951ded199 * feat(service): add --file flag with auto-detection from metadata Add file upload support to meta API service method commands. The --file flag is conditionally registered only for methods whose metadata declares file-type fields (POST/PUT/PATCH/DELETE). The default field name is auto-detected from metadata when exactly one file field exists. Change-Id: Ibbf04eb42341ba11bb1fd9750e63bc1d0eacd08d * feat(schema): show file upload indicators in method detail display Add hasFileFields helper to detect file-type fields in requestBody metadata. Modify printMethodDetail to display [file upload] tag on --data line, --file flag description with default field name, and --file in CLI example for methods that accept file uploads. Change-Id: Iae3bc14fe07e16a8b5f6a50a2b3592d6d8490ed9 * fix: address code review findings for file upload feature - ParseFileFlag: change idx >= 0 to idx > 0 to prevent empty field name when input like "=photo.jpg" is passed - BuildFormdata: read file into bytes.Reader with defer Close to prevent file handle leak on later errors - BuildFormdata: remove unused ctx parameter from signature and callers - Eliminate duplicated dry-run logic by having buildAPIRequest and buildServiceRequest return FileUploadMeta when in dry-run mode, removing ~60 lines of copy-pasted URL building and validation code Change-Id: I27b9534fd0eaefce40390f6e723dd0c04a2cdf80 * fix: address PR review findings - Remove opts.File=="" guard on dual-stdin check so --file photo.jpg --params - --data - correctly reports an error instead of silently dropping --data content (P1 bug in both api.go and service.go) - Extract shared DetectFileFields into cmdutil, deduplicate detectFileFields (service.go) and hasFileFields (schema.go) - Show "" instead of empty path in dry-run output for --file - Change-Id: Iccc5d879165ea6a3d04f0425ec6a5018a10e72e1 * fix: reject non-object --data with --file and improve multi-file schema - --data with --file now requires a JSON object; arrays/strings/numbers are rejected with a clear error instead of being silently dropped - Schema display for multi-file methods shows explicit field=path syntax and lists valid field names instead of advertising a false default Change-Id: I0facdb3ad86f68cb125c7ea109a33714fd91dba0 --- cmd/api/api.go | 80 +++++-- cmd/api/api_test.go | 96 ++++++++ cmd/schema/schema.go | 31 ++- cmd/schema/schema_test.go | 121 ++++++++++ cmd/service/service.go | 97 ++++++-- cmd/service/service_test.go | 139 ++++++++++++ internal/cmdutil/dryrun.go | 45 ++++ internal/cmdutil/fileupload.go | 130 +++++++++++ internal/cmdutil/fileupload_test.go | 338 ++++++++++++++++++++++++++++ 9 files changed, 1043 insertions(+), 34 deletions(-) create mode 100644 internal/cmdutil/fileupload.go create mode 100644 internal/cmdutil/fileupload_test.go diff --git a/cmd/api/api.go b/cmd/api/api.go index b26c1167e..1fe651d03 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -41,6 +41,7 @@ type APIOptions struct { Format string JqExpr string DryRun bool + File string } var urlPrefixRe = regexp.MustCompile(`https?://[^/]+(/open-apis/.+)`) @@ -87,6 +88,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*APIOptions) error) *cobra.Command cmd.Flags().StringVar(&opts.Format, "format", "json", "output format: json|ndjson|table|csv") cmd.Flags().StringVarP(&opts.JqExpr, "jq", "q", "", "jq expression to filter JSON output") cmd.Flags().BoolVar(&opts.DryRun, "dry-run", false, "print request without executing") + cmd.Flags().StringVar(&opts.File, "file", "", "file to upload as multipart/form-data ([field=]path, supports - for stdin)") cmd.ValidArgsFunction = func(_ *cobra.Command, args []string, _ string) ([]string, cobra.ShellCompDirective) { if len(args) == 0 { @@ -105,20 +107,24 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*APIOptions) error) *cobra.Command } // buildAPIRequest validates flags and builds a RawApiRequest. -func buildAPIRequest(opts *APIOptions) (client.RawApiRequest, error) { - // stdin is an io.Reader consumed at most once. Only one of --params/--data - // may use "-" (stdin); the conflict check below prevents silent data loss. +// When dryRun is true and a file is provided, file reading is skipped and +// FileUploadMeta is returned instead so the caller can render dry-run output. +func buildAPIRequest(opts *APIOptions) (client.RawApiRequest, *cmdutil.FileUploadMeta, error) { stdin := opts.Factory.IOStreams.In - if opts.Params == "-" && opts.Data == "-" { - return client.RawApiRequest{}, output.ErrValidation("--params and --data cannot both read from stdin (-)") + + // Validate --file mutual exclusions first. + if err := cmdutil.ValidateFileFlag(opts.File, opts.Params, opts.Data, opts.Output, opts.PageAll, opts.Method); err != nil { + return client.RawApiRequest{}, nil, err } + + // stdin conflict: --params and --data cannot both read from stdin, regardless of --file. + if opts.Params == "-" && opts.Data == "-" { + return client.RawApiRequest{}, nil, output.ErrValidation("--params and --data cannot both read from stdin (-)") + } + params, err := cmdutil.ParseJSONMap(opts.Params, "--params", stdin) if err != nil { - return client.RawApiRequest{}, err - } - data, err := cmdutil.ParseOptionalBody(opts.Method, opts.Data, stdin) - if err != nil { - return client.RawApiRequest{}, err + return client.RawApiRequest{}, nil, err } if opts.PageSize > 0 { params["page_size"] = opts.PageSize @@ -128,14 +134,53 @@ func buildAPIRequest(opts *APIOptions) (client.RawApiRequest, error) { Method: opts.Method, URL: normalisePath(opts.Path), Params: params, - Data: data, As: opts.As, } - // WithFileDownload tells the SDK to skip CodeError parsing on 200 OK. - if opts.Output != "" { - request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileDownload()) + + if opts.File != "" { + // File upload path: build formdata. + fieldName, filePath, isStdin := cmdutil.ParseFileFlag(opts.File, "file") + + // Parse --data as JSON map for form fields (not as body). + var dataFields any + if opts.Data != "" { + dataFields, err = cmdutil.ParseOptionalBody(opts.Method, opts.Data, stdin) + if err != nil { + return client.RawApiRequest{}, nil, err + } + if _, ok := dataFields.(map[string]any); !ok { + return client.RawApiRequest{}, nil, output.ErrValidation("--data must be a JSON object when used with --file") + } + } + + if opts.DryRun { + return request, &cmdutil.FileUploadMeta{ + FieldName: fieldName, FilePath: filePath, FormFields: dataFields, + }, nil + } + + fd, err := cmdutil.BuildFormdata( + opts.Factory.ResolveFileIO(opts.Ctx), + fieldName, filePath, isStdin, stdin, dataFields, + ) + if err != nil { + return client.RawApiRequest{}, nil, err + } + request.Data = fd + request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileUpload()) + } else { + // Normal path: JSON body. + data, err := cmdutil.ParseOptionalBody(opts.Method, opts.Data, stdin) + if err != nil { + return client.RawApiRequest{}, nil, err + } + request.Data = data + if opts.Output != "" { + request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileDownload()) + } } - return request, nil + + return request, nil, nil } func apiRun(opts *APIOptions) error { @@ -153,7 +198,7 @@ func apiRun(opts *APIOptions) error { return err } - request, err := buildAPIRequest(opts) + request, fileMeta, err := buildAPIRequest(opts) if err != nil { return err } @@ -164,6 +209,9 @@ func apiRun(opts *APIOptions) error { } if opts.DryRun { + if fileMeta != nil { + return cmdutil.PrintDryRunWithFile(f.IOStreams.Out, request, config, opts.Format, fileMeta.FieldName, fileMeta.FilePath, fileMeta.FormFields) + } return apiDryRun(f, request, config, opts.Format) } // Identity info is now included in the JSON envelope; skip stderr printing. diff --git a/cmd/api/api_test.go b/cmd/api/api_test.go index 94d553209..9f5be7022 100644 --- a/cmd/api/api_test.go +++ b/cmd/api/api_test.go @@ -5,6 +5,7 @@ package api import ( "errors" + "os" "sort" "strings" "testing" @@ -706,3 +707,98 @@ func TestApiCmd_MethodUppercase(t *testing.T) { t.Errorf("expected method POST (uppercased), got %s", gotOpts.Method) } } + +func TestApiCmd_FileFlagParsing(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + var gotOpts *APIOptions + cmd := NewCmdApi(f, func(opts *APIOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs([]string{"POST", "/open-apis/test", "--file", "image=photo.jpg", "--data", `{"image_type":"message"}`}) + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotOpts.File != "image=photo.jpg" { + t.Errorf("expected File = %q, got %q", "image=photo.jpg", gotOpts.File) + } +} + +func TestApiCmd_FileAndOutputConflict(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + cmd := NewCmdApi(f, func(opts *APIOptions) error { + return apiRun(opts) + }) + cmd.SetArgs([]string{"POST", "/open-apis/test", "--as", "bot", "--file", "photo.jpg", "--output", "out.json"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for --file with --output") + } + if !strings.Contains(err.Error(), "mutually exclusive") { + t.Errorf("expected mutual exclusion error, got: %v", err) + } +} + +func TestApiCmd_FileWithGET(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + cmd := NewCmdApi(f, func(opts *APIOptions) error { + return apiRun(opts) + }) + cmd.SetArgs([]string{"GET", "/open-apis/test", "--as", "bot", "--file", "photo.jpg"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for --file with GET") + } + if !strings.Contains(err.Error(), "requires POST") { + t.Errorf("expected method error, got: %v", err) + } +} + +func TestApiCmd_FileStdinConflictWithData(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + cmd := NewCmdApi(f, func(opts *APIOptions) error { + return apiRun(opts) + }) + cmd.SetArgs([]string{"POST", "/open-apis/test", "--as", "bot", "--file", "-", "--data", "-"}) + err := cmd.Execute() + if err == nil { + t.Fatal("expected error for --file stdin with --data stdin") + } + if !strings.Contains(err.Error(), "cannot both read from stdin") { + t.Errorf("expected stdin conflict error, got: %v", err) + } +} + +func TestApiCmd_DryRunWithFile(t *testing.T) { + tmpDir := t.TempDir() + tmpFile := tmpDir + "/test.jpg" + if err := os.WriteFile(tmpFile, []byte("fake-image"), 0600); err != nil { + t.Fatal(err) + } + + f, stdout, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu, + }) + cmd := NewCmdApi(f, nil) + cmd.SetArgs([]string{"POST", "/open-apis/im/v1/images", "--file", "image=" + tmpFile, "--data", `{"image_type":"message"}`, "--dry-run", "--as", "bot"}) + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "image") { + t.Errorf("expected dry-run output to mention file field, got: %s", out) + } + if !strings.Contains(out, "Dry Run") { + t.Errorf("expected dry-run header, got: %s", out) + } +} diff --git a/cmd/schema/schema.go b/cmd/schema/schema.go index 81daee134..f45d6e816 100644 --- a/cmd/schema/schema.go +++ b/cmd/schema/schema.go @@ -73,6 +73,12 @@ func printResourceList(w io.Writer, spec map[string]interface{}) { fmt.Fprintf(w, "%sUsage: lark-cli schema %s..%s\n", output.Dim, name, output.Reset) } +// hasFileFields returns true if any requestBody field has type "file". +func hasFileFields(method map[string]interface{}) (bool, []string) { + names := cmdutil.DetectFileFields(method) + return len(names) > 0, names +} + func printMethodDetail(w io.Writer, spec map[string]interface{}, resName, methodName string, method map[string]interface{}) { servicePath := registry.GetStrFromMap(spec, "servicePath") specName := registry.GetStrFromMap(spec, "name") @@ -80,6 +86,7 @@ func printMethodDetail(w io.Writer, spec map[string]interface{}, resName, method fullPath := servicePath + "/" + methodPath httpMethod := registry.GetStrFromMap(method, "httpMethod") desc := registry.GetStrFromMap(method, "description") + isFileUpload, fileFieldNames := hasFileFields(method) fmt.Fprintf(w, "%s%s.%s.%s%s\n\n", output.Bold, specName, resName, methodName, output.Reset) @@ -138,11 +145,25 @@ func printMethodDetail(w io.Writer, spec map[string]interface{}, resName, method if len(params) == 0 { fmt.Fprintf(w, "%sParameters:%s\n\n", output.Bold, output.Reset) } - fmt.Fprintf(w, " %s--data%s %soptional%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) + fileUploadTag := "" + if isFileUpload { + fileUploadTag = fmt.Sprintf(" %s[file upload]%s", output.Yellow, output.Reset) + } + fmt.Fprintf(w, " %s--data%s %soptional%s%s\n", output.Cyan, output.Reset, output.Dim, output.Reset, fileUploadTag) requestBody, _ := method["requestBody"].(map[string]interface{}) if len(requestBody) > 0 { printNestedFields(w, requestBody, " ", "") } + + if isFileUpload { + if len(fileFieldNames) == 1 { + fmt.Fprintf(w, "\n %s--file%s <[field=]path> %sfile upload%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) + fmt.Fprintf(w, " Upload file as multipart/form-data. Default field: %q\n", fileFieldNames[0]) + } else { + fmt.Fprintf(w, "\n %s--file%s %sfile upload%s\n", output.Cyan, output.Reset, output.Dim, output.Reset) + fmt.Fprintf(w, " Upload file as multipart/form-data. Fields: %s\n", strings.Join(fileFieldNames, ", ")) + } + } fmt.Fprintln(w) } @@ -184,7 +205,13 @@ func printMethodDetail(w io.Writer, spec map[string]interface{}, resName, method } // CLI example - fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s\n", output.Bold, output.Reset, specName, resName, methodName) + if isFileUpload && len(fileFieldNames) == 1 { + fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s --file \n", output.Bold, output.Reset, specName, resName, methodName) + } else if isFileUpload { + fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s --file \n", output.Bold, output.Reset, specName, resName, methodName) + } else { + fmt.Fprintf(w, "%sCLI:%s lark-cli %s %s %s\n", output.Bold, output.Reset, specName, resName, methodName) + } // Docs if docUrl := registry.GetStrFromMap(method, "docUrl"); docUrl != "" { diff --git a/cmd/schema/schema_test.go b/cmd/schema/schema_test.go index 2f36159a6..639822acc 100644 --- a/cmd/schema/schema_test.go +++ b/cmd/schema/schema_test.go @@ -4,6 +4,7 @@ package schema import ( + "bytes" "strings" "testing" @@ -61,3 +62,123 @@ func TestSchemaCmd_UnknownService(t *testing.T) { t.Errorf("expected 'Unknown service' error, got: %v", err) } } + +func TestPrintMethodDetail_FileUpload(t *testing.T) { + spec := map[string]interface{}{ + "name": "im", + "servicePath": "/open-apis/im/v1", + } + method := map[string]interface{}{ + "path": "images", + "httpMethod": "POST", + "description": "Upload an image", + "requestBody": map[string]interface{}{ + "image_type": map[string]interface{}{ + "type": "string", + "required": true, + }, + "image": map[string]interface{}{ + "type": "file", + "required": true, + }, + }, + "accessTokens": []interface{}{"user", "tenant"}, + } + + var buf bytes.Buffer + printMethodDetail(&buf, spec, "images", "create", method) + out := buf.String() + + if !strings.Contains(out, "file upload") { + t.Errorf("expected 'file upload' marker in output, got:\n%s", out) + } + if !strings.Contains(out, "--file") { + t.Errorf("expected '--file' in output, got:\n%s", out) + } + if !strings.Contains(out, `"image"`) { + t.Errorf("expected default field name 'image' in output, got:\n%s", out) + } + if !strings.Contains(out, "--file ") { + t.Errorf("expected CLI example with --file , got:\n%s", out) + } +} + +func TestPrintMethodDetail_NoFileUpload(t *testing.T) { + spec := map[string]interface{}{ + "name": "calendar", + "servicePath": "/open-apis/calendar/v4", + } + method := map[string]interface{}{ + "path": "events", + "httpMethod": "POST", + "description": "Create an event", + "requestBody": map[string]interface{}{ + "summary": map[string]interface{}{ + "type": "string", + "required": true, + }, + }, + } + + var buf bytes.Buffer + printMethodDetail(&buf, spec, "events", "create", method) + out := buf.String() + + if strings.Contains(out, "file upload") { + t.Errorf("did not expect 'file upload' marker for non-file method, got:\n%s", out) + } + if strings.Contains(out, "--file") { + t.Errorf("did not expect '--file' for non-file method, got:\n%s", out) + } +} + +func TestHasFileFields(t *testing.T) { + tests := []struct { + name string + method map[string]interface{} + wantBool bool + wantFields []string + }{ + { + name: "has file field", + method: map[string]interface{}{ + "requestBody": map[string]interface{}{ + "image": map[string]interface{}{"type": "file"}, + "name": map[string]interface{}{"type": "string"}, + }, + }, + wantBool: true, + wantFields: []string{"image"}, + }, + { + name: "no file field", + method: map[string]interface{}{ + "requestBody": map[string]interface{}{ + "name": map[string]interface{}{"type": "string"}, + }, + }, + wantBool: false, + wantFields: nil, + }, + { + name: "no requestBody", + method: map[string]interface{}{}, + wantBool: false, + wantFields: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, names := hasFileFields(tt.method) + if got != tt.wantBool { + t.Errorf("hasFileFields() = %v, want %v", got, tt.wantBool) + } + if tt.wantFields == nil && names != nil { + t.Errorf("expected nil names, got %v", names) + } + if tt.wantFields != nil && len(names) != len(tt.wantFields) { + t.Errorf("expected %d field names, got %d", len(tt.wantFields), len(names)) + } + }) + } +} diff --git a/cmd/service/service.go b/cmd/service/service.go index 592a73874..5639075bd 100644 --- a/cmd/service/service.go +++ b/cmd/service/service.go @@ -111,6 +111,13 @@ type ServiceMethodOptions struct { Format string JqExpr string DryRun bool + File string // --file flag value + FileFields []string // auto-detected file field names from metadata +} + +// detectFileFields delegates to the shared cmdutil.DetectFileFields helper. +func detectFileFields(method map[string]interface{}) []string { + return cmdutil.DetectFileFields(method) } func registerMethod(parent *cobra.Command, spec map[string]interface{}, method map[string]interface{}, name string, resName string, f *cmdutil.Factory) { @@ -161,6 +168,16 @@ func NewCmdServiceMethod(f *cmdutil.Factory, spec, method map[string]interface{} cmd.Flags().StringVarP(&opts.JqExpr, "jq", "q", "", "jq expression to filter JSON output") cmd.Flags().BoolVar(&opts.DryRun, "dry-run", false, "print request without executing") + // Conditionally register --file for methods with file-type fields. + fileFields := detectFileFields(method) + opts.FileFields = fileFields + if len(fileFields) > 0 { + switch httpMethod { + case "POST", "PUT", "PATCH", "DELETE": + cmd.Flags().StringVar(&opts.File, "file", "", "file to upload ([field=]path, supports - for stdin)") + } + } + _ = cmd.RegisterFlagCompletionFunc("as", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { return []string{"user", "bot"}, cobra.ShellCompDirectiveNoFileComp }) @@ -212,12 +229,15 @@ func serviceMethodRun(opts *ServiceMethodOptions) error { } } - request, err := buildServiceRequest(opts) + request, fileMeta, err := buildServiceRequest(opts) if err != nil { return err } if opts.DryRun { + if fileMeta != nil { + return cmdutil.PrintDryRunWithFile(f.IOStreams.Out, request, config, opts.Format, fileMeta.FieldName, fileMeta.FilePath, fileMeta.FormFields) + } return serviceDryRun(f, request, config, opts.Format) } @@ -303,7 +323,9 @@ func checkServiceScopes(ctx context.Context, cred *credential.CredentialProvider } // buildServiceRequest parses flags, builds the URL with path/query params, and returns a RawApiRequest. -func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, error) { +// When dryRun is true and a file is provided, file reading is skipped and +// FileUploadMeta is returned instead so the caller can render dry-run output. +func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, *cmdutil.FileUploadMeta, error) { spec := opts.Spec method := opts.Method schemaPath := opts.SchemaPath @@ -312,12 +334,17 @@ func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, erro // stdin is an io.Reader consumed at most once. Only one of --params/--data // may use "-" (stdin); the conflict check below prevents silent data loss. stdin := opts.Factory.IOStreams.In + + // Validate --file mutual exclusions. + if err := cmdutil.ValidateFileFlag(opts.File, opts.Params, opts.Data, opts.Output, opts.PageAll, httpMethod); err != nil { + return client.RawApiRequest{}, nil, err + } if opts.Params == "-" && opts.Data == "-" { - return client.RawApiRequest{}, output.ErrValidation("--params and --data cannot both read from stdin (-)") + return client.RawApiRequest{}, nil, output.ErrValidation("--params and --data cannot both read from stdin (-)") } params, err := cmdutil.ParseJSONMap(opts.Params, "--params", stdin) if err != nil { - return client.RawApiRequest{}, err + return client.RawApiRequest{}, nil, err } url := registry.GetStrFromMap(spec, "servicePath") + "/" + registry.GetStrFromMap(method, "path") @@ -330,13 +357,13 @@ func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, erro } val, ok := params[name] if !ok || util.IsEmptyValue(val) { - return client.RawApiRequest{}, output.ErrWithHint(output.ExitValidation, "validation", + return client.RawApiRequest{}, nil, output.ErrWithHint(output.ExitValidation, "validation", fmt.Sprintf("missing required path parameter: %s", name), fmt.Sprintf("lark-cli schema %s", schemaPath)) } valStr := fmt.Sprintf("%v", val) if err := validate.ResourceName(valStr, name); err != nil { - return client.RawApiRequest{}, output.ErrValidation("%s", err) + return client.RawApiRequest{}, nil, output.ErrValidation("%s", err) } url = strings.Replace(url, "{"+name+"}", validate.EncodePathSegment(valStr), 1) delete(params, name) @@ -352,7 +379,7 @@ func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, erro required, _ := p["required"].(bool) isPaginationParam := opts.PageAll && (name == "page_token" || name == "page_size") if required && !isPaginationParam && (!exists || util.IsEmptyValue(value)) { - return client.RawApiRequest{}, output.ErrWithHint(output.ExitValidation, "validation", + return client.RawApiRequest{}, nil, output.ErrWithHint(output.ExitValidation, "validation", fmt.Sprintf("missing required query parameter: %s", name), fmt.Sprintf("lark-cli schema %s", schemaPath)) } @@ -366,22 +393,60 @@ func buildServiceRequest(opts *ServiceMethodOptions) (client.RawApiRequest, erro } } - data, err := cmdutil.ParseOptionalBody(httpMethod, opts.Data, stdin) - if err != nil { - return client.RawApiRequest{}, err - } - request := client.RawApiRequest{ Method: httpMethod, URL: url, Params: queryParams, - Data: data, As: opts.As, } - if opts.Output != "" { - request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileDownload()) + + if opts.File != "" { + // File upload: determine default field name from metadata. + defaultField := "file" + if len(opts.FileFields) == 1 { + defaultField = opts.FileFields[0] + } + fieldName, filePath, isStdin := cmdutil.ParseFileFlag(opts.File, defaultField) + + // Parse --data as form fields. + var dataFields any + if opts.Data != "" { + dataFields, err = cmdutil.ParseOptionalBody(httpMethod, opts.Data, stdin) + if err != nil { + return client.RawApiRequest{}, nil, err + } + if _, ok := dataFields.(map[string]any); !ok { + return client.RawApiRequest{}, nil, output.ErrValidation("--data must be a JSON object when used with --file") + } + } + + if opts.DryRun { + return request, &cmdutil.FileUploadMeta{ + FieldName: fieldName, FilePath: filePath, FormFields: dataFields, + }, nil + } + + fd, err := cmdutil.BuildFormdata( + opts.Factory.ResolveFileIO(opts.Ctx), + fieldName, filePath, isStdin, stdin, dataFields, + ) + if err != nil { + return client.RawApiRequest{}, nil, err + } + request.Data = fd + request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileUpload()) + } else { + data, err := cmdutil.ParseOptionalBody(httpMethod, opts.Data, stdin) + if err != nil { + return client.RawApiRequest{}, nil, err + } + request.Data = data + if opts.Output != "" { + request.ExtraOpts = append(request.ExtraOpts, larkcore.WithFileDownload()) + } } - return request, nil + + return request, nil, nil } func serviceDryRun(f *cmdutil.Factory, request client.RawApiRequest, config *core.CliConfig, format string) error { diff --git a/cmd/service/service_test.go b/cmd/service/service_test.go index c424610dc..f9adc5d47 100644 --- a/cmd/service/service_test.go +++ b/cmd/service/service_test.go @@ -4,6 +4,7 @@ package service import ( + "os" "strings" "testing" @@ -710,6 +711,144 @@ func TestScopeAwareChecker_ScopeError_BotMode(t *testing.T) { } } +// ── file upload ── + +func imImageMethod() map[string]interface{} { + return map[string]interface{}{ + "path": "images", + "httpMethod": "POST", + "requestBody": map[string]interface{}{ + "image_type": map[string]interface{}{ + "type": "string", + "required": true, + }, + "image": map[string]interface{}{ + "type": "file", + "required": true, + }, + }, + "accessTokens": []interface{}{"user", "tenant"}, + } +} + +func imSpec() map[string]interface{} { + return map[string]interface{}{ + "name": "im", + "servicePath": "/open-apis/im/v1", + } +} + +func TestServiceMethod_FileFlagRegistered(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, imSpec(), imImageMethod(), "create", "images", nil) + flag := cmd.Flags().Lookup("file") + if flag == nil { + t.Fatal("expected --file flag to be registered for file upload method") + } +} + +func TestServiceMethod_FileFlagNotRegistered(t *testing.T) { + f, _, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, driveSpec(), driveMethod("POST", nil), "copy", "files", nil) + flag := cmd.Flags().Lookup("file") + if flag != nil { + t.Fatal("expected --file flag NOT to be registered for non-file method") + } +} + +func TestServiceMethod_FileFlagNotRegisteredForGET(t *testing.T) { + getMethod := map[string]interface{}{ + "path": "images", + "httpMethod": "GET", + "requestBody": map[string]interface{}{ + "image": map[string]interface{}{ + "type": "file", + }, + }, + } + f, _, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, imSpec(), getMethod, "get", "images", nil) + flag := cmd.Flags().Lookup("file") + if flag != nil { + t.Fatal("expected --file flag NOT to be registered for GET method") + } +} + +func TestServiceMethod_FileUpload_DryRun(t *testing.T) { + tmpDir := t.TempDir() + tmpFile := tmpDir + "/test.jpg" + if err := os.WriteFile(tmpFile, []byte("fake-image"), 0600); err != nil { + t.Fatal(err) + } + + f, stdout, _, _ := cmdutil.TestFactory(t, testConfig) + cmd := NewCmdServiceMethod(f, imSpec(), imImageMethod(), "create", "images", nil) + cmd.SetArgs([]string{ + "--file", "image=" + tmpFile, + "--data", `{"image_type":"message"}`, + "--dry-run", + "--as", "bot", + }) + err := cmd.Execute() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + out := stdout.String() + if !strings.Contains(out, "image") { + t.Errorf("expected dry-run output to mention file field, got: %s", out) + } + if !strings.Contains(out, "Dry Run") { + t.Errorf("expected dry-run header, got: %s", out) + } +} + +func TestDetectFileFields(t *testing.T) { + tests := []struct { + name string + method map[string]interface{} + want []string + }{ + { + name: "single file field", + method: map[string]interface{}{ + "requestBody": map[string]interface{}{ + "image": map[string]interface{}{"type": "file"}, + "name": map[string]interface{}{"type": "string"}, + }, + }, + want: []string{"image"}, + }, + { + name: "no file fields", + method: map[string]interface{}{ + "requestBody": map[string]interface{}{ + "name": map[string]interface{}{"type": "string"}, + }, + }, + want: nil, + }, + { + name: "no requestBody", + method: map[string]interface{}{}, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := detectFileFields(tt.method) + if len(got) != len(tt.want) { + t.Errorf("detectFileFields() = %v, want %v", got, tt.want) + return + } + for i := range got { + if got[i] != tt.want[i] { + t.Errorf("detectFileFields()[%d] = %q, want %q", i, got[i], tt.want[i]) + } + } + }) + } +} + // ── helpers ── func isExitError(err error, target **output.ExitError) bool { diff --git a/internal/cmdutil/dryrun.go b/internal/cmdutil/dryrun.go index 57302a60b..f047d3fb2 100644 --- a/internal/cmdutil/dryrun.go +++ b/internal/cmdutil/dryrun.go @@ -215,6 +215,51 @@ func encodeParams(params map[string]interface{}) string { return vals.Encode() } +// PrintDryRunWithFile outputs a dry-run summary for file upload requests. +// Instead of serializing the Formdata body, it shows file metadata. +func PrintDryRunWithFile(w io.Writer, request client.RawApiRequest, config *core.CliConfig, format, fileField, filePath string, formFields any) error { + dr := NewDryRunAPI() + switch request.Method { + case "POST": + dr.POST(request.URL) + case "PUT": + dr.PUT(request.URL) + case "PATCH": + dr.PATCH(request.URL) + case "DELETE": + dr.DELETE(request.URL) + default: + dr.GET(request.URL) + } + if len(request.Params) > 0 { + dr.Params(request.Params) + } + filePathDisplay := filePath + if filePathDisplay == "" { + filePathDisplay = "" + } + fileInfo := map[string]any{ + "file": map[string]string{"field": fileField, "path": filePathDisplay}, + } + if formFields != nil { + fileInfo["form_fields"] = formFields + } + fileInfo["options"] = []string{"WithFileUpload"} + dr.Body(fileInfo) + dr.Set("as", string(request.As)) + dr.Set("appId", config.AppID) + if config.UserOpenId != "" { + dr.Set("userOpenId", config.UserOpenId) + } + fmt.Fprintln(w, "=== Dry Run ===") + if format == "pretty" { + fmt.Fprint(w, dr.Format()) + } else { + output.PrintJson(w, dr) + } + return nil +} + // PrintDryRun outputs a standardised dry-run summary using DryRunAPI. // When format is "pretty", outputs human-readable text; otherwise JSON. func PrintDryRun(w io.Writer, request client.RawApiRequest, config *core.CliConfig, format string) error { diff --git a/internal/cmdutil/fileupload.go b/internal/cmdutil/fileupload.go new file mode 100644 index 000000000..541c1fde1 --- /dev/null +++ b/internal/cmdutil/fileupload.go @@ -0,0 +1,130 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdutil + +import ( + "bytes" + "fmt" + "io" + "strings" + + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/registry" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" +) + +// DetectFileFields returns field names with type "file" in the method's requestBody. +func DetectFileFields(method map[string]interface{}) []string { + rb, _ := method["requestBody"].(map[string]interface{}) + var fields []string + for name, field := range rb { + f, _ := field.(map[string]interface{}) + if registry.GetStrFromMap(f, "type") == "file" { + fields = append(fields, name) + } + } + return fields +} + +// ParseFileFlag parses a --file flag value into its components. +// The format is either "path" or "field=path". When no explicit "field=" +// prefix is present, defaultField is used as the field name. +// A path of "-" indicates stdin; in that case filePath is empty and isStdin is true. +func ParseFileFlag(raw, defaultField string) (fieldName, filePath string, isStdin bool) { + if idx := strings.IndexByte(raw, '='); idx > 0 { + fieldName = raw[:idx] + filePath = raw[idx+1:] + } else { + fieldName = defaultField + filePath = raw + } + if filePath == "-" { + return fieldName, "", true + } + return fieldName, filePath, false +} + +// ValidateFileFlag checks mutual exclusion rules for the --file flag. +// Returns nil if file is empty (flag not provided). +func ValidateFileFlag(file, params, data, outputPath string, pageAll bool, httpMethod string) error { + if file == "" { + return nil + } + + _, filePath, isStdin := ParseFileFlag(file, "file") + if !isStdin && filePath == "" { + return output.ErrValidation("--file: empty file path") + } + + if outputPath != "" { + return output.ErrValidation("--file and --output are mutually exclusive") + } + if pageAll { + return output.ErrValidation("--file and --page-all are mutually exclusive") + } + if isStdin && data == "-" { + return output.ErrValidation("--file and --data cannot both read from stdin") + } + if isStdin && params == "-" { + return output.ErrValidation("--file and --params cannot both read from stdin") + } + + switch httpMethod { + case "POST", "PUT", "PATCH", "DELETE": + default: + return output.ErrValidation("--file requires POST, PUT, PATCH, or DELETE method") + } + + return nil +} + +// FileUploadMeta holds file upload metadata for dry-run display. +// Returned by request builders when dry-run mode skips actual file reading. +type FileUploadMeta struct { + FieldName string + FilePath string + FormFields any +} + +// BuildFormdata constructs a multipart form data payload for file upload. +// If isStdin is true, the file content is read from stdin. +// Top-level keys from dataJSON are added as text form fields. +func BuildFormdata(fileIO fileio.FileIO, fieldName, filePath string, isStdin bool, stdin io.Reader, dataJSON any) (*larkcore.Formdata, error) { + fd := larkcore.NewFormdata() + + if isStdin { + if stdin == nil { + return nil, output.ErrValidation("--file: stdin is not available") + } + data, err := io.ReadAll(stdin) + if err != nil { + return nil, output.ErrValidation("--file: failed to read stdin: %v", err) + } + if len(data) == 0 { + return nil, output.ErrValidation("--file: stdin is empty") + } + fd.AddFile(fieldName, bytes.NewReader(data)) + } else { + f, err := fileIO.Open(filePath) + if err != nil { + return nil, output.ErrValidation("cannot open file: %s", filePath) + } + defer f.Close() + data, err := io.ReadAll(f) + if err != nil { + return nil, output.ErrValidation("--file: failed to read %s: %v", filePath, err) + } + fd.AddFile(fieldName, bytes.NewReader(data)) + } + + // Add top-level JSON keys as text form fields. + if m, ok := dataJSON.(map[string]any); ok { + for k, v := range m { + fd.AddField(k, fmt.Sprintf("%v", v)) + } + } + + return fd, nil +} diff --git a/internal/cmdutil/fileupload_test.go b/internal/cmdutil/fileupload_test.go new file mode 100644 index 000000000..abd3861c3 --- /dev/null +++ b/internal/cmdutil/fileupload_test.go @@ -0,0 +1,338 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package cmdutil + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/larksuite/cli/internal/vfs/localfileio" +) + +func TestParseFileFlag(t *testing.T) { + tests := []struct { + name string + raw string + defaultField string + wantField string + wantPath string + wantStdin bool + }{ + { + name: "simple filename uses default field", + raw: "photo.jpg", + defaultField: "file", + wantField: "file", + wantPath: "photo.jpg", + wantStdin: false, + }, + { + name: "simple filename with custom default", + raw: "photo.jpg", + defaultField: "image", + wantField: "image", + wantPath: "photo.jpg", + wantStdin: false, + }, + { + name: "explicit field prefix", + raw: "image=photo.jpg", + defaultField: "file", + wantField: "image", + wantPath: "photo.jpg", + wantStdin: false, + }, + { + name: "stdin bare", + raw: "-", + defaultField: "file", + wantField: "file", + wantPath: "", + wantStdin: true, + }, + { + name: "stdin with field prefix", + raw: "image=-", + defaultField: "file", + wantField: "image", + wantPath: "", + wantStdin: true, + }, + { + name: "path with equals sign (only first equals splits)", + raw: "field=path/to/file=1.jpg", + defaultField: "file", + wantField: "field", + wantPath: "path/to/file=1.jpg", + wantStdin: false, + }, + { + name: "absolute path no prefix", + raw: "/tmp/photo.jpg", + defaultField: "file", + wantField: "file", + wantPath: "/tmp/photo.jpg", + wantStdin: false, + }, + { + name: "absolute path with field prefix", + raw: "image=/tmp/photo.jpg", + defaultField: "file", + wantField: "image", + wantPath: "/tmp/photo.jpg", + wantStdin: false, + }, + { + name: "empty field prefix falls through to default", + raw: "=photo.jpg", + defaultField: "file", + wantField: "file", + wantPath: "=photo.jpg", + wantStdin: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + field, path, isStdin := ParseFileFlag(tt.raw, tt.defaultField) + if field != tt.wantField { + t.Errorf("field = %q, want %q", field, tt.wantField) + } + if path != tt.wantPath { + t.Errorf("path = %q, want %q", path, tt.wantPath) + } + if isStdin != tt.wantStdin { + t.Errorf("isStdin = %v, want %v", isStdin, tt.wantStdin) + } + }) + } +} + +func TestValidateFileFlag(t *testing.T) { + tests := []struct { + name string + file string + params string + data string + outputPath string + pageAll bool + httpMethod string + wantErr string // empty means no error + }{ + { + name: "empty file is valid", + file: "", + httpMethod: "GET", + wantErr: "", + }, + { + name: "empty file path", + file: "field=", + httpMethod: "POST", + wantErr: "--file: empty file path", + }, + { + name: "file with output", + file: "photo.jpg", + outputPath: "out.json", + httpMethod: "POST", + wantErr: "--file and --output are mutually exclusive", + }, + { + name: "file with page-all", + file: "photo.jpg", + pageAll: true, + httpMethod: "POST", + wantErr: "--file and --page-all are mutually exclusive", + }, + { + name: "stdin file with stdin data", + file: "-", + data: "-", + httpMethod: "POST", + wantErr: "--file and --data cannot both read from stdin", + }, + { + name: "stdin file with stdin params", + file: "-", + params: "-", + httpMethod: "POST", + wantErr: "--file and --params cannot both read from stdin", + }, + { + name: "file with GET method", + file: "photo.jpg", + httpMethod: "GET", + wantErr: "--file requires POST, PUT, PATCH, or DELETE method", + }, + { + name: "file with POST method", + file: "photo.jpg", + httpMethod: "POST", + wantErr: "", + }, + { + name: "file with PUT method", + file: "photo.jpg", + httpMethod: "PUT", + wantErr: "", + }, + { + name: "file with PATCH method", + file: "photo.jpg", + httpMethod: "PATCH", + wantErr: "", + }, + { + name: "file with DELETE method", + file: "photo.jpg", + httpMethod: "DELETE", + wantErr: "", + }, + { + name: "stdin with field prefix and data stdin", + file: "image=-", + data: "-", + httpMethod: "POST", + wantErr: "--file and --data cannot both read from stdin", + }, + { + name: "stdin with field prefix and params stdin", + file: "image=-", + params: "-", + httpMethod: "POST", + wantErr: "--file and --params cannot both read from stdin", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateFileFlag(tt.file, tt.params, tt.data, tt.outputPath, tt.pageAll, tt.httpMethod) + if tt.wantErr == "" { + if err != nil { + t.Errorf("unexpected error: %v", err) + } + return + } + if err == nil { + t.Fatalf("expected error containing %q, got nil", tt.wantErr) + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Errorf("error = %q, want containing %q", err.Error(), tt.wantErr) + } + }) + } +} + +func TestBuildFormdata(t *testing.T) { + fio := &localfileio.LocalFileIO{} + + t.Run("stdin success", func(t *testing.T) { + stdin := bytes.NewReader([]byte("file-content-here")) + fd, err := BuildFormdata(fio, "file", "", true, stdin, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fd == nil { + t.Fatal("expected non-nil Formdata") + } + }) + + t.Run("stdin nil reader", func(t *testing.T) { + _, err := BuildFormdata(fio, "file", "", true, nil, nil) + if err == nil { + t.Fatal("expected error for nil stdin") + } + if !strings.Contains(err.Error(), "stdin is not available") { + t.Errorf("error = %q, want containing %q", err.Error(), "stdin is not available") + } + }) + + t.Run("stdin empty", func(t *testing.T) { + stdin := bytes.NewReader([]byte{}) + _, err := BuildFormdata(fio, "file", "", true, stdin, nil) + if err == nil { + t.Fatal("expected error for empty stdin") + } + if !strings.Contains(err.Error(), "stdin is empty") { + t.Errorf("error = %q, want containing %q", err.Error(), "stdin is empty") + } + }) + + t.Run("file open success", func(t *testing.T) { + dir := t.TempDir() + TestChdir(t, dir) + + if err := os.WriteFile(filepath.Join(dir, "test.txt"), []byte("hello"), 0600); err != nil { + t.Fatalf("failed to create test file: %v", err) + } + + fd, err := BuildFormdata(fio, "photo", "test.txt", false, nil, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fd == nil { + t.Fatal("expected non-nil Formdata") + } + }) + + t.Run("file not found", func(t *testing.T) { + dir := t.TempDir() + TestChdir(t, dir) + + _, err := BuildFormdata(fio, "file", "nonexistent.txt", false, nil, nil) + if err == nil { + t.Fatal("expected error for missing file") + } + if !strings.Contains(err.Error(), "cannot open file:") { + t.Errorf("error = %q, want containing %q", err.Error(), "cannot open file:") + } + }) + + t.Run("dataJSON fields added", func(t *testing.T) { + dir := t.TempDir() + TestChdir(t, dir) + + if err := os.WriteFile(filepath.Join(dir, "upload.bin"), []byte("data"), 0600); err != nil { + t.Fatalf("failed to create test file: %v", err) + } + + dataJSON := map[string]any{ + "file_name": "report.pdf", + "parent_type": "doc_image", + "size": 1024, + } + + fd, err := BuildFormdata(fio, "file", "upload.bin", false, nil, dataJSON) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fd == nil { + t.Fatal("expected non-nil Formdata") + } + }) + + t.Run("dataJSON nil is fine", func(t *testing.T) { + stdin := bytes.NewReader([]byte("content")) + fd, err := BuildFormdata(fio, "file", "", true, stdin, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fd == nil { + t.Fatal("expected non-nil Formdata") + } + }) + + t.Run("dataJSON non-map is ignored", func(t *testing.T) { + stdin := bytes.NewReader([]byte("content")) + fd, err := BuildFormdata(fio, "file", "", true, stdin, "not-a-map") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if fd == nil { + t.Fatal("expected non-nil Formdata") + } + }) +}