diff --git a/README.md b/README.md index 0cb79bf..3818cf0 100644 --- a/README.md +++ b/README.md @@ -101,7 +101,7 @@ The official [GitLink](https://www.gitlink.org.cn) CLI tool — built for humans | 🔀 PR | Create, merge, review pull requests, view changed files | | 👥 Member | List, add, remove repository members, change roles, create and accept invite links | | 🌿 Branch | Create, delete, list, protect, unprotect branches | -| 🏷️ Release | Create, edit, update, view, delete releases | +| 🏷️ Release | Create, edit, update, view, delete releases, and manage release assets | | 🏢 Org | Manage organizations, members, teams | | 🔧 CI | View builds, logs, CI/CD operations | | ⚙️ Pipeline | Run, inspect, enable, disable, delete pipeline workflows and logs | @@ -442,6 +442,16 @@ gitlink-cli release +create --owner Gitlink --repo forgeplus -t v1.0.0 -n "v1.0. # View a release gitlink-cli release +view --owner Gitlink --repo forgeplus -i +# List assets attached to a release +gitlink-cli release +assets --owner Gitlink --repo forgeplus -i + +# Attach or detach existing asset IDs while preserving release metadata +gitlink-cli release +attach --owner Gitlink --repo forgeplus -i --attachment-ids 12,34 --dry-run +gitlink-cli release +detach --owner Gitlink --repo forgeplus -i --attachment-ids 34 --dry-run + +# Upload a local file and attach it to the release in one step +gitlink-cli release +upload --owner Gitlink --repo forgeplus -i --file dist/gitlink-cli_linux_amd64.tar.gz --asset-name gitlink-cli-linux-amd64.tar.gz --description "Linux binary" --dry-run + # Get edit data and update while preserving unspecified fields gitlink-cli release +edit --owner Gitlink --repo forgeplus -i gitlink-cli release +update --owner Gitlink --repo forgeplus -i -b "Updated changelog" --dry-run diff --git a/README.zh-CN.md b/README.zh-CN.md index 8a67276..c8a0541 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -109,7 +109,7 @@ | 🔀 PR | 创建、合并、Review Pull Request，查看变更文件 | | 👥 成员 | 列出、添加、移除仓库成员，调整角色，生成和接受邀请链接 | | 🌿 分支 | 创建、删除、保护分支 | -| 🏷️ 发布 | 创建、编辑、更新、查看、删除 Release | +| 🏷️ 发布 | 创建、编辑、更新、查看、删除 Release，并管理发布资产 | | 🏢 组织 | 管理组织、成员、团队 | | 🔧 CI | 查看构建、日志、CI/CD 操作 | | ⚙️ Pipeline | 运行、查看、启停、删除流水线工作流并查询日志 | @@ -476,6 +476,16 @@ gitlink-cli release +create --owner Gitlink --repo forgeplus -t v1.0.0 -n "v1.0. # 查看 Release gitlink-cli release +view --owner Gitlink --repo forgeplus -i +# 列出 Release 已绑定的资产 +gitlink-cli release +assets --owner Gitlink --repo forgeplus -i + +# 绑定或移除已有附件 ID，同时保留 Release 其他字段 +gitlink-cli release +attach --owner Gitlink --repo forgeplus -i --attachment-ids 12,34 --dry-run +gitlink-cli release +detach --owner Gitlink --repo forgeplus -i --attachment-ids 34 --dry-run + +# 上传本地文件并一步绑定到 Release +gitlink-cli release +upload --owner Gitlink --repo forgeplus -i --file dist/gitlink-cli_linux_amd64.tar.gz --asset-name gitlink-cli-linux-amd64.tar.gz --description "Linux 二进制包" --dry-run + # 获取编辑数据并保留未传字段更新 gitlink-cli release +edit --owner Gitlink --repo forgeplus -i gitlink-cli release +update --owner Gitlink --repo forgeplus -i -b "更新后的内容" --dry-run diff --git a/doc/changes/release-asset-shortcuts.md b/doc/changes/release-asset-shortcuts.md new file mode 100644 index 0000000..f632bcf --- /dev/null +++ b/doc/changes/release-asset-shortcuts.md @@ -0,0 +1,25 @@ +# Release Asset Shortcuts + +Submitter: Mengz + +This change fills the missing release asset workflow in `gitlink-cli`, so users can upload files, attach existing assets, inspect attached assets, and detach assets without dropping the rest of the release metadata. + +## Commands + +- Add `release +assets` to inspect the assets currently attached to a release. +- Add `release +attach` to merge existing attachment IDs into a release. +- Add `release +detach` to remove attachment IDs from a release. +- Add `release +upload` to upload a local file and attach it to a release in one command. + +## Behavior + +- Add multipart upload support to `internal/client` and expose it through `shortcuts/common.RuntimeContext`. +- Keep the `RuntimeContext` encapsulation instead of calling the low-level client directly from shortcuts. +- Preserve existing release fields when attaching, detaching, or uploading assets, so these commands do not accidentally overwrite `name`, `tag_name`, `body`, `target_commitish`, `draft`, or `prerelease`. +- Accept release tags for asset operations by resolving them to the internal release version ID before write operations. +- Clean up the uploaded attachment automatically if the follow-up release update fails, avoiding orphaned assets. + +## Verification + +- Add multipart client tests covering field values, uploaded filename, content type, and file body. +- Add release shortcut tests covering asset listing, attach/detach dry-run behavior, attachment merging, upload-and-attach flow, and cleanup after failed release update. diff --git a/internal/client/client.go b/internal/client/client.go index be81ded..3dbb05f 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -7,9 +7,9 @@ import ( "io" "mime/multipart" "net/http" + "net/textproto" "net/url" - "os" - "path/filepath" + "sort" "strings" "github.com/gitlink-org/gitlink-cli/internal/auth" @@ -29,6 +29,13 @@ type APIError struct { Message string } +type MultipartFile struct { + FieldName string + FileName string + ContentType string + Reader io.Reader +} + func (e *APIError) Error() string { return fmt.Sprintf("[%v] %s", e.Code, e.Message) } @@ -45,96 +52,57 @@ func New() (*Client, error) { } func (c *Client) Do(method, path string, body interface{}, query url.Values) (*output.Envelope, error) { - path = normalizeAPIPath(c.BaseURL, path) - - // Append .json suffix if not already present (GitLink API convention) - // Handle paths that may already contain query strings (e.g., /path?key=val) - if idx := strings.Index(path, "?"); idx != -1 { - basePath := path[:idx] - queryStr := path[idx:] - if shouldAppendJSONSuffix(basePath) { - path = basePath + ".json" + queryStr - } - } else if shouldAppendJSONSuffix(path) { - path += ".json" - } - fullURL := c.BaseURL + path - if len(query) > 0 { - sep := "?" - if strings.Contains(fullURL, "?") { - sep = "&" - } - fullURL += sep + query.Encode() - } - - // Replace path params var bodyReader io.Reader + contentType := "" if body != nil { data, err := json.Marshal(body) if err != nil { return nil, err } bodyReader = bytes.NewReader(data) + contentType = "application/json" } - req, err := http.NewRequest(method, fullURL, bodyReader) + return c.doRequest(method, path, bodyReader, contentType, query) +} + +func (c *Client) PostMultipart(path string, fields map[string]string, files []MultipartFile) (*output.Envelope, error) { + if len(files) == 0 { + return nil, fmt.Errorf("at least one multipart file is required") + } + + reader, writer := io.Pipe() + form := multipart.NewWriter(writer) + + go func() { + writeErr := writeMultipartBody(form, fields, files) + closeErr := form.Close() + if writeErr == nil { + writeErr = closeErr + } + if writeErr != nil { + _ = writer.CloseWithError(writeErr) + return + } + _ = writer.Close() + }() + + return c.doRequest(http.MethodPost, path, reader, form.FormDataContentType(), nil) +} + +func (c *Client) doRequest(method, path string, body io.Reader, contentType string, query url.Values) (*output.Envelope, error) { + fullURL := c.apiURL(path, query) + + req, err := http.NewRequest(method, fullURL, body) if err != nil { return nil, err } - - return c.doRequest(req) -} - -func (c *Client) PostMultipart(path, fileField, filePath string, fields map[string]string) (*output.Envelope, error) { - path = normalizeAPIPath(c.BaseURL, path) - if idx := strings.Index(path, "?"); idx != -1 { - basePath := path[:idx] - queryStr := path[idx:] - if shouldAppendJSONSuffix(basePath) { - path = basePath + ".json" + queryStr - } - } else if shouldAppendJSONSuffix(path) { - path += ".json" - } - fullURL := c.BaseURL + path - - file, err := os.Open(filePath) - if err != nil { - return nil, fmt.Errorf("open file: %w", err) - } - defer file.Close() - - var body bytes.Buffer - writer := multipart.NewWriter(&body) - - part, err := writer.CreateFormFile(fileField, filepath.Base(filePath)) - if err != nil { - return nil, fmt.Errorf("create form file: %w", err) - } - if _, err := io.Copy(part, file); err != nil { - return nil, fmt.Errorf("copy file: %w", err) - } - for key, value := range fields { - if err := writer.WriteField(key, value); err != nil { - return nil, fmt.Errorf("write form field %s: %w", key, err) - } - } - if err := writer.Close(); err != nil { - return nil, fmt.Errorf("close multipart writer: %w", err) + if contentType != "" { + req.Header.Set("Content-Type", contentType) } - req, err := http.NewRequest("POST", fullURL, &body) - if err != nil { - return nil, err - } - req.Header.Set("Content-Type", writer.FormDataContentType()) - - return c.doRequest(req) -} - -func (c *Client) doRequest(req *http.Request) (*output.Envelope, error) { if c.Debug { - fmt.Printf("-> %s %s\n", req.Method, req.URL.String()) + fmt.Printf("-> %s %s\n", method, fullURL) } resp, err := c.HTTP.Do(req) @@ -149,15 +117,97 @@ func (c *Client) doRequest(req *http.Request) (*output.Envelope, error) { } if c.Debug { - fmt.Printf("<- %d %s\n", resp.StatusCode, string(respData[:min(len(respData), 200)])) + previewLen := len(respData) + if previewLen > 200 { + previewLen = 200 + } + fmt.Printf("<- %d %s\n", resp.StatusCode, string(respData[:previewLen])) } + return parseResponseEnvelope(resp.StatusCode, respData) +} + +func writeMultipartBody(form *multipart.Writer, fields map[string]string, files []MultipartFile) error { + fieldNames := make([]string, 0, len(fields)) + for name := range fields { + fieldNames = append(fieldNames, name) + } + sort.Strings(fieldNames) + + for _, name := range fieldNames { + if err := form.WriteField(name, fields[name]); err != nil { + return err + } + } + + for _, file := range files { + if file.Reader == nil { + return fmt.Errorf("multipart file reader is required") + } + part, err := createMultipartPart(form, file) + if err != nil { + return err + } + if _, err := io.Copy(part, file.Reader); err != nil { + return err + } + } + + return nil +} + +func createMultipartPart(form *multipart.Writer, file MultipartFile) (io.Writer, error) { + fieldName := strings.TrimSpace(file.FieldName) + if fieldName == "" { + fieldName = "file" + } + fileName := strings.TrimSpace(file.FileName) + if fileName == "" { + fileName = fieldName + } + if strings.TrimSpace(file.ContentType) == "" { + return form.CreateFormFile(fieldName, fileName) + } + + header := make(textproto.MIMEHeader) + header.Set("Content-Disposition", fmt.Sprintf(`form-data; name=%q; filename=%q`, fieldName, fileName)) + header.Set("Content-Type", file.ContentType) + return form.CreatePart(header) +} + +func (c *Client) apiURL(path string, query url.Values) string { + path = normalizeAPIPath(c.BaseURL, path) + + // Append .json suffix if not already present (GitLink API convention) + // Handle paths that may already contain query strings (e.g., /path?key=val) + if idx := strings.Index(path, "?"); idx != -1 { + basePath := path[:idx] + queryStr := path[idx:] + if shouldAppendJSONSuffix(basePath) { + path = basePath + ".json" + queryStr + } + } else if shouldAppendJSONSuffix(path) { + path += ".json" + } + + fullURL := c.BaseURL + path + if len(query) > 0 { + sep := "?" + if strings.Contains(fullURL, "?") { + sep = "&" + } + fullURL += sep + query.Encode() + } + return fullURL +} + +func parseResponseEnvelope(statusCode int, respData []byte) (*output.Envelope, error) { // Check HTTP-level errors - if resp.StatusCode >= 400 { + if statusCode >= 400 { return nil, &APIError{ - StatusCode: resp.StatusCode, - Code: resp.StatusCode, - Message: fmt.Sprintf("HTTP %d: %s", resp.StatusCode, strings.TrimSpace(string(respData))), + StatusCode: statusCode, + Code: statusCode, + Message: fmt.Sprintf("HTTP %d: %s", statusCode, strings.TrimSpace(string(respData))), } } diff --git a/internal/client/client_test.go b/internal/client/client_test.go index b7d08e9..aa2a857 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -2,11 +2,14 @@ package client import ( "encoding/json" + "io" + "mime" "net/http" "net/http/httptest" "net/url" "os" "path/filepath" + "strings" "testing" ) @@ -312,6 +315,87 @@ func TestClientDoWithBody(t *testing.T) { } } +func TestClientPostMultipart(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Fatalf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/api/attachments.json" { + t.Fatalf("expected path /api/attachments.json, got %s", r.URL.Path) + } + + mediaType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil { + t.Fatalf("parse media type: %v", err) + } + if mediaType != "multipart/form-data" { + t.Fatalf("Content-Type = %q, want multipart/form-data", mediaType) + } + + if err := r.ParseMultipartForm(1 << 20); err != nil { + t.Fatalf("ParseMultipartForm: %v", err) + } + if got := r.FormValue("description"); got != "release asset" { + t.Fatalf("description = %q, want %q", got, "release asset") + } + + files := r.MultipartForm.File["file"] + if len(files) != 1 { + t.Fatalf("expected 1 uploaded file, got %d", len(files)) + } + if files[0].Filename != "asset.zip" { + t.Fatalf("filename = %q, want %q", files[0].Filename, "asset.zip") + } + if got := files[0].Header.Get("Content-Type"); got != "application/octet-stream" { + t.Fatalf("part Content-Type = %q, want %q", got, "application/octet-stream") + } + + file, err := files[0].Open() + if err != nil { + t.Fatalf("open multipart file: %v", err) + } + defer file.Close() + + data, err := io.ReadAll(file) + if err != nil { + t.Fatalf("read multipart file: %v", err) + } + if string(data) != "binary content" { + t.Fatalf("file body = %q, want %q", string(data), "binary content") + } + + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"id":"asset-1"}`)) + })) + defer server.Close() + + c := &Client{HTTP: server.Client(), BaseURL: server.URL} + env, err := c.PostMultipart("/api/attachments", map[string]string{ + "description": "release asset", + }, []MultipartFile{ + { + FieldName: "file", + FileName: "asset.zip", + ContentType: "application/octet-stream", + Reader: strings.NewReader("binary content"), + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !env.OK { + t.Fatal("expected OK=true") + } +} + +func TestClientPostMultipartRequiresFile(t *testing.T) { + c := &Client{HTTP: &http.Client{}, BaseURL: "https://gitlink.example.com"} + _, err := c.PostMultipart("/attachments", nil, nil) + if err == nil { + t.Fatal("expected error when no multipart file is provided") + } +} + func TestClientGet(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != "GET" { diff --git a/internal/i18n/locales/en-US.json b/internal/i18n/locales/en-US.json index b3c153e..54c5d25 100644 --- a/internal/i18n/locales/en-US.json +++ b/internal/i18n/locales/en-US.json @@ -79,9 +79,13 @@ "cmd.profile.role.short": "Show a user's role positioning", "cmd.profile.short": "User profile and statistics operations", "cmd.release.create.short": "Create a release", + "cmd.release.assets.short": "List release assets", + "cmd.release.attach.short": "Attach assets to a release", "cmd.release.delete.short": "Delete a release", + "cmd.release.detach.short": "Detach assets from a release", "cmd.release.list.short": "List releases", "cmd.release.short": "Release operations", + "cmd.release.upload.short": "Upload an asset and attach it to a release", "cmd.release.view.short": "View release details", "cmd.repo.create.short": "Create a new repository", "cmd.repo.delete.short": "Delete a repository", diff --git a/internal/i18n/locales/zh-CN.json b/internal/i18n/locales/zh-CN.json index 9c129e8..44706bc 100644 --- a/internal/i18n/locales/zh-CN.json +++ b/internal/i18n/locales/zh-CN.json @@ -79,9 +79,13 @@ "cmd.profile.role.short": "显示用户角色定位", "cmd.profile.short": "用户画像与统计操作", "cmd.release.create.short": "创建发布", + "cmd.release.assets.short": "列出发布资产", + "cmd.release.attach.short": "为发布绑定资产", "cmd.release.delete.short": "删除发布", + "cmd.release.detach.short": "从发布移除资产", "cmd.release.list.short": "列出发布", "cmd.release.short": "发布操作", + "cmd.release.upload.short": "上传文件并绑定到发布", "cmd.release.view.short": "查看发布详情", "cmd.repo.create.short": "创建新仓库", "cmd.repo.delete.short": "删除仓库", diff --git a/shortcuts/common/types.go b/shortcuts/common/types.go index e89614b..100e467 100644 --- a/shortcuts/common/types.go +++ b/shortcuts/common/types.go @@ -90,9 +90,9 @@ func (ctx *RuntimeContext) CallAPIWithQuery(method, path string, query url.Value return ctx.Client.Do(method, path, nil, query) } -// PostMultipart uploads a file with multipart/form-data through the shared client. -func (ctx *RuntimeContext) PostMultipart(path, fileField, filePath string, fields map[string]string) (*output.Envelope, error) { - return ctx.Client.PostMultipart(path, fileField, filePath, fields) +// PostMultipart uploads multipart/form-data through the shared runtime client. +func (ctx *RuntimeContext) PostMultipart(path string, fields map[string]string, files []client.MultipartFile) (*output.Envelope, error) { + return ctx.Client.PostMultipart(path, fields, files) } // PaginateAll fetches all pages. @@ -100,11 +100,6 @@ func (ctx *RuntimeContext) PaginateAll(path string, params url.Values) ([]json.R return ctx.Client.PaginateAll(path, params) } -// Download fetches raw bytes from an API, attachment, or archive URL. -func (ctx *RuntimeContext) Download(path string) (*client.DownloadResult, error) { - return ctx.Client.Download(path) -} - // Output prints the envelope in the configured format. func (ctx *RuntimeContext) Output(env *output.Envelope) error { return output.Print(env, ctx.Format) @@ -115,20 +110,6 @@ func (ctx *RuntimeContext) OutputData(data interface{}) error { return output.Print(output.SuccessEnvelope(data, nil), ctx.Format) } -// DefaultBranch fetches the repository's default branch, falling back to "master". -func (ctx *RuntimeContext) DefaultBranch() (string, error) { - env, err := ctx.CallAPI("GET", ctx.RepoPath(), nil) - if err != nil { - return "", err - } - if data, ok := env.Data.(map[string]interface{}); ok { - if branch, ok := data["default_branch"].(string); ok && branch != "" { - return branch, nil - } - } - return "master", nil -} - // RepoPath returns the API path prefix for the current owner/repo. func (ctx *RuntimeContext) RepoPath() string { return fmt.Sprintf("/%s/%s", ctx.Owner, ctx.Repo) diff --git a/shortcuts/release/assets.go b/shortcuts/release/assets.go index 768da55..e6474fe 100644 --- a/shortcuts/release/assets.go +++ b/shortcuts/release/assets.go @@ -4,221 +4,444 @@ import ( "fmt" "os" "path/filepath" - "strconv" "strings" + "github.com/gitlink-org/gitlink-cli/internal/client" + "github.com/gitlink-org/gitlink-cli/internal/i18n" "github.com/gitlink-org/gitlink-cli/internal/output" "github.com/gitlink-org/gitlink-cli/shortcuts/common" ) -type releaseAsset struct { - ID string `json:"id"` - Title string `json:"title"` - Filesize string `json:"filesize,omitempty"` - Description string `json:"description,omitempty"` - URL string `json:"url"` +type releaseTarget struct { + Identifier string + VersionID string + View map[string]interface{} + Edit map[string]interface{} } -func runReleaseAssets(ctx *common.RuntimeContext) error { - release, err := fetchRelease(ctx) - if err != nil { - return err - } - return ctx.Output(output.SuccessEnvelope(releaseAssetsPayload(release), nil)) -} - -func runReleaseDownload(ctx *common.RuntimeContext) error { - release, err := fetchRelease(ctx) - if err != nil { - return err - } - sourceURL, filename, sourceType, err := selectDownloadSource(ctx, release) - if err != nil { - return err - } - - result, err := ctx.Download(sourceURL) - if err != nil { - return err - } - outPath, err := resolveOutputPath(ctx.Arg("output"), filename) - if err != nil { - return err - } - if ctx.Arg("force") != "true" { - if _, err := os.Stat(outPath); err == nil { - return fmt.Errorf("output file already exists: %s (use --force to overwrite)", outPath) - } else if !os.IsNotExist(err) { - return err - } - } - if err := os.MkdirAll(filepath.Dir(outPath), 0755); err != nil { - return err - } - if err := os.WriteFile(outPath, result.Data, 0644); err != nil { - return err - } - - return ctx.Output(output.SuccessEnvelope(map[string]interface{}{ - "path": outPath, - "bytes": len(result.Data), - "source": sourceURL, - "source_type": sourceType, - "content_type": result.ContentType, - }, nil)) -} - -func fetchRelease(ctx *common.RuntimeContext) (map[string]interface{}, error) { - if err := ctx.ResolveOwnerRepo(); err != nil { - return nil, err - } - id, _ := ctx.RequireArg("id") - env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/releases/%s", ctx.RepoPath(), id), nil) - if err != nil { - return nil, err - } - data, ok := env.Data.(map[string]interface{}) - if !ok { - return nil, fmt.Errorf("unexpected release response shape") - } - return data, nil -} - -func releaseAssetsPayload(release map[string]interface{}) map[string]interface{} { - return map[string]interface{}{ - "version_id": stringValue(release["version_id"]), - "tag_name": stringValue(release["tag_name"]), - "name": stringValue(release["name"]), - "attachments": releaseAttachments(release), - "archives": map[string]string{ - "zip": stringValue(release["zipball_url"]), - "tar": stringValue(release["tarball_url"]), +func releaseAssetShortcuts(tr *i18n.Translator) []*common.Shortcut { + return []*common.Shortcut{ + { + Name: "assets", + Description: tr.T("cmd.release.assets.short"), + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: tr.T("flag.release.id_or_tag"), Required: true}, + }, + Run: runAssets, + }, + { + Name: "attach", + Description: tr.T("cmd.release.attach.short"), + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: tr.T("flag.release.id_or_tag"), Required: true}, + {Name: "attachment-ids", Usage: "Comma-separated attachment IDs", Required: true}, + {Name: "dry-run", Usage: "Preview the attach request without changing release state", Bool: true, Default: "false"}, + }, + Run: runAttach, + }, + { + Name: "detach", + Description: tr.T("cmd.release.detach.short"), + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: tr.T("flag.release.id_or_tag"), Required: true}, + {Name: "attachment-ids", Usage: "Comma-separated attachment IDs", Required: true}, + {Name: "dry-run", Usage: "Preview the detach request without changing release state", Bool: true, Default: "false"}, + }, + Run: runDetach, + }, + { + Name: "upload", + Description: tr.T("cmd.release.upload.short"), + Flags: []common.Flag{ + {Name: "id", Short: "i", Usage: tr.T("flag.release.id_or_tag"), Required: true}, + {Name: "file", Usage: "Local file path", Required: true}, + {Name: "asset-name", Usage: "Override the uploaded asset filename"}, + {Name: "description", Usage: "Attachment description"}, + {Name: "dry-run", Usage: "Preview the upload request without changing release state", Bool: true, Default: "false"}, + }, + Run: runUpload, }, } } -func selectDownloadSource(ctx *common.RuntimeContext, release map[string]interface{}) (string, string, string, error) { - archive := strings.ToLower(strings.TrimSpace(ctx.Arg("archive"))) - assetSelector := strings.TrimSpace(ctx.Arg("asset")) - if archive != "" && assetSelector != "" { - return "", "", "", fmt.Errorf("--asset and --archive cannot be used together") +func runAssets(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err } - if archive != "" { - return selectArchiveSource(ctx, release, archive) - } - - assets := releaseAttachments(release) - asset, err := selectAsset(assets, assetSelector) + identifier, err := ctx.RequireArg("id") if err != nil { - return "", "", "", err + return err } - return asset.URL, safeFilename(asset.Title, "attachment-"+asset.ID), "asset", nil + + view, err := fetchReleaseView(ctx, identifier) + if err != nil { + return err + } + target := &releaseTarget{ + Identifier: identifier, + VersionID: releaseVersionID(identifier, view), + View: view, + } + + result := releaseActionResult(ctx, target, "list_release_assets") + result["attachment_ids"] = releaseAttachmentIDs(view) + result["attachments"] = releaseAttachments(view) + return ctx.OutputData(result) } -func selectArchiveSource(ctx *common.RuntimeContext, release map[string]interface{}, archive string) (string, string, string, error) { - tag := stringValue(release["tag_name"]) - if tag == "" { - tag = ctx.Arg("id") +func runAttach(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err } - switch archive { - case "zip": - u := stringValue(release["zipball_url"]) - if u == "" { - return "", "", "", fmt.Errorf("release does not provide zipball_url") + identifier, err := ctx.RequireArg("id") + if err != nil { + return err + } + requestedIDs, err := parseReleaseAttachmentIDs(ctx.Arg("attachment-ids")) + if err != nil { + return err + } + + target, err := resolveReleaseTarget(ctx, identifier) + if err != nil { + return err + } + + currentIDs := releaseAttachmentIDs(target.Edit) + nextIDs, addedIDs := mergeReleaseAttachmentIDs(currentIDs, requestedIDs) + result := releaseActionResult(ctx, target, "attach_release_assets") + result["dry_run"] = ctx.Arg("dry-run") == "true" + result["changed"] = len(addedIDs) > 0 + result["current_attachment_ids"] = currentIDs + result["requested_attachment_ids"] = requestedIDs + result["added_attachment_ids"] = addedIDs + result["attachment_ids"] = nextIDs + if len(addedIDs) == 0 || ctx.Arg("dry-run") == "true" { + return ctx.OutputData(result) + } + + env, err := updateReleaseAttachments(ctx, target.VersionID, target.Edit, nextIDs) + if err != nil { + return err + } + result["release"] = env.Data + return ctx.OutputData(result) +} + +func runDetach(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + identifier, err := ctx.RequireArg("id") + if err != nil { + return err + } + requestedIDs, err := parseReleaseAttachmentIDs(ctx.Arg("attachment-ids")) + if err != nil { + return err + } + + target, err := resolveReleaseTarget(ctx, identifier) + if err != nil { + return err + } + + currentIDs := releaseAttachmentIDs(target.Edit) + nextIDs, removedIDs := removeReleaseAttachmentIDs(currentIDs, requestedIDs) + result := releaseActionResult(ctx, target, "detach_release_assets") + result["dry_run"] = ctx.Arg("dry-run") == "true" + result["changed"] = len(removedIDs) > 0 + result["current_attachment_ids"] = currentIDs + result["requested_attachment_ids"] = requestedIDs + result["removed_attachment_ids"] = removedIDs + result["attachment_ids"] = nextIDs + if len(removedIDs) == 0 || ctx.Arg("dry-run") == "true" { + return ctx.OutputData(result) + } + + env, err := updateReleaseAttachments(ctx, target.VersionID, target.Edit, nextIDs) + if err != nil { + return err + } + result["release"] = env.Data + return ctx.OutputData(result) +} + +func runUpload(ctx *common.RuntimeContext) error { + if err := ctx.ResolveOwnerRepo(); err != nil { + return err + } + identifier, err := ctx.RequireArg("id") + if err != nil { + return err + } + filePath, uploadName, size, err := resolveUploadFile(ctx.Arg("file"), ctx.Arg("asset-name")) + if err != nil { + return err + } + + target, err := resolveReleaseTarget(ctx, identifier) + if err != nil { + return err + } + + result := releaseActionResult(ctx, target, "upload_release_asset") + result["dry_run"] = ctx.Arg("dry-run") == "true" + result["current_attachment_ids"] = releaseAttachmentIDs(target.Edit) + result["file"] = map[string]interface{}{ + "path": filePath, + "asset_name": uploadName, + "size_bytes": size, + "description": strings.TrimSpace(ctx.Arg("description")), + } + if ctx.Arg("dry-run") == "true" { + return ctx.OutputData(result) + } + + file, err := os.Open(filePath) + if err != nil { + return fmt.Errorf("open asset file: %w", err) + } + defer file.Close() + + fields := map[string]string{} + if description := strings.TrimSpace(ctx.Arg("description")); description != "" { + fields["description"] = description + } + uploadEnv, err := ctx.PostMultipart("/attachments", fields, []client.MultipartFile{ + { + FieldName: "file", + FileName: uploadName, + Reader: file, + }, + }) + if err != nil { + return fmt.Errorf("upload asset: %w", err) + } + + attachment, err := releaseMap(uploadEnv.Data, "attachment upload response") + if err != nil { + return err + } + attachmentID := releaseIDString(attachment["id"]) + if attachmentID == "" { + return fmt.Errorf("attachment upload response did not include an attachment ID") + } + + nextIDs, _ := mergeReleaseAttachmentIDs(releaseAttachmentIDs(target.Edit), []string{attachmentID}) + releaseEnv, err := updateReleaseAttachments(ctx, target.VersionID, target.Edit, nextIDs) + if err != nil { + if cleanupErr := deleteAttachment(ctx, attachmentID); cleanupErr != nil { + return fmt.Errorf("attach uploaded asset to release: %w (cleanup failed: %v)", err, cleanupErr) } - return u, safeFilename(fmt.Sprintf("%s-%s.zip", ctx.Repo, tag), "release.zip"), "archive", nil - case "tar", "tar.gz", "tgz": - u := stringValue(release["tarball_url"]) - if u == "" { - return "", "", "", fmt.Errorf("release does not provide tarball_url") - } - return u, safeFilename(fmt.Sprintf("%s-%s.tar.gz", ctx.Repo, tag), "release.tar.gz"), "archive", nil - default: - return "", "", "", fmt.Errorf("--archive must be zip or tar") + return fmt.Errorf("attach uploaded asset to release: %w", err) + } + + result["attachment_ids"] = nextIDs + result["uploaded_attachment_id"] = attachmentID + result["attachment"] = attachment + result["release"] = releaseEnv.Data + return ctx.OutputData(result) +} + +func resolveReleaseTarget(ctx *common.RuntimeContext, identifier string) (*releaseTarget, error) { + view, err := fetchReleaseView(ctx, identifier) + if err != nil { + return nil, fmt.Errorf("fetch release: %w", err) + } + + versionID := releaseVersionID(identifier, view) + if versionID == "" { + return nil, fmt.Errorf("failed to resolve release version ID from %q", identifier) + } + + edit, err := fetchReleaseEdit(ctx, versionID) + if err != nil { + return nil, fmt.Errorf("fetch release edit data: %w", err) + } + + return &releaseTarget{ + Identifier: identifier, + VersionID: versionID, + View: view, + Edit: edit, + }, nil +} + +func fetchReleaseView(ctx *common.RuntimeContext, id string) (map[string]interface{}, error) { + env, err := ctx.CallAPI("GET", fmt.Sprintf("%s/releases/%s", ctx.RepoPath(), id), nil) + if err != nil { + return nil, err + } + return releaseMap(env.Data, "release data") +} + +func updateReleaseAttachments(ctx *common.RuntimeContext, versionID string, current map[string]interface{}, attachmentIDs []string) (*output.Envelope, error) { + payload, err := releaseCurrentPayload(current, attachmentIDs) + if err != nil { + return nil, err + } + return ctx.CallAPI("PUT", fmt.Sprintf("%s/releases/%s", ctx.RepoPath(), versionID), payload) +} + +func releaseActionResult(ctx *common.RuntimeContext, target *releaseTarget, action string) map[string]interface{} { + return map[string]interface{}{ + "repository": fmt.Sprintf("%s/%s", ctx.Owner, ctx.Repo), + "release_ref": target.Identifier, + "release_id": target.VersionID, + "tag_name": releaseString(target.View, "tag_name"), + "release_name": firstReleaseValue(releaseString(target.View, "name"), releaseString(target.Edit, "name")), + "action": action, } } -func selectAsset(assets []releaseAsset, selector string) (releaseAsset, error) { - if len(assets) == 0 { - return releaseAsset{}, fmt.Errorf("release has no attachments") +func releaseVersionID(identifier string, view map[string]interface{}) string { + if id := releaseIDString(view["version_id"]); id != "" { + return id } - if selector == "" { - if len(assets) == 1 { - return assets[0], nil - } - return releaseAsset{}, fmt.Errorf("release has multiple attachments; specify --asset by id or title") + if isNumericReleaseID(identifier) { + return strings.TrimSpace(identifier) } - for _, asset := range assets { - if asset.ID == selector || asset.Title == selector { - return asset, nil - } - } - return releaseAsset{}, fmt.Errorf("release attachment %q not found", selector) + return "" } -func releaseAttachments(release map[string]interface{}) []releaseAsset { - raw, ok := release["attachments"].([]interface{}) +func isNumericReleaseID(value string) bool { + value = strings.TrimSpace(value) + if value == "" { + return false + } + for _, r := range value { + if r < '0' || r > '9' { + return false + } + } + return true +} + +func releaseMap(data interface{}, name string) (map[string]interface{}, error) { + result, ok := data.(map[string]interface{}) if !ok { + return nil, fmt.Errorf("failed to parse %s", name) + } + return result, nil +} + +func releaseCurrentPayload(current map[string]interface{}, attachmentIDs []string) (map[string]interface{}, error) { + name := releaseString(current, "name") + if name == "" { + return nil, fmt.Errorf("required release name is missing in remote data") + } + tag := releaseString(current, "tag_name") + if tag == "" { + return nil, fmt.Errorf("required release tag is missing in remote data") + } + + ids := make([]string, len(attachmentIDs)) + copy(ids, attachmentIDs) + return map[string]interface{}{ + "name": name, + "tag_name": tag, + "body": releaseString(current, "body"), + "target_commitish": releaseString(current, "target_commitish"), + "draft": releaseBoolValue(current, "draft", false), + "prerelease": releaseBoolValue(current, "prerelease", false), + "attachment_ids": ids, + }, nil +} + +func releaseAttachments(values map[string]interface{}) []map[string]interface{} { + if values == nil { return nil } - assets := make([]releaseAsset, 0, len(raw)) - for _, item := range raw { - m, ok := item.(map[string]interface{}) - if !ok { + + switch attachments := values["attachments"].(type) { + case []interface{}: + items := make([]map[string]interface{}, 0, len(attachments)) + for _, attachment := range attachments { + item, ok := attachment.(map[string]interface{}) + if !ok { + continue + } + items = append(items, item) + } + return items + case []map[string]interface{}: + return attachments + default: + return nil + } +} + +func mergeReleaseAttachmentIDs(currentIDs, requestedIDs []string) ([]string, []string) { + merged := make([]string, 0, len(currentIDs)+len(requestedIDs)) + seen := map[string]bool{} + + for _, id := range currentIDs { + if seen[id] { continue } - assets = append(assets, releaseAsset{ - ID: stringValue(m["id"]), - Title: stringValue(m["title"]), - Filesize: stringValue(m["filesize"]), - Description: stringValue(m["description"]), - URL: stringValue(m["url"]), - }) + seen[id] = true + merged = append(merged, id) } - return assets -} -func resolveOutputPath(out, filename string) (string, error) { - if strings.TrimSpace(out) == "" { - out = "." - } - if info, err := os.Stat(out); err == nil && info.IsDir() { - return filepath.Join(out, filename), nil - } else if err != nil && !os.IsNotExist(err) { - return "", err - } - if strings.HasSuffix(out, string(os.PathSeparator)) || strings.HasSuffix(out, "/") { - return filepath.Join(out, filename), nil - } - return out, nil -} - -func safeFilename(name, fallback string) string { - name = strings.TrimSpace(name) - if name == "" { - name = fallback - } - name = filepath.Base(strings.ReplaceAll(name, "\\", "/")) - if name == "." || name == string(os.PathSeparator) || name == "" { - return fallback - } - return name -} - -func stringValue(v interface{}) string { - switch x := v.(type) { - case string: - return x - case float64: - if x == float64(int64(x)) { - return strconv.FormatInt(int64(x), 10) + added := make([]string, 0, len(requestedIDs)) + for _, id := range requestedIDs { + if seen[id] { + continue } - return strconv.FormatFloat(x, 'f', -1, 64) - case int: - return strconv.Itoa(x) - case int64: - return strconv.FormatInt(x, 10) - default: - return "" + seen[id] = true + merged = append(merged, id) + added = append(added, id) } + + return merged, added +} + +func removeReleaseAttachmentIDs(currentIDs, requestedIDs []string) ([]string, []string) { + removeSet := map[string]bool{} + for _, id := range requestedIDs { + removeSet[id] = true + } + + remaining := make([]string, 0, len(currentIDs)) + removed := make([]string, 0, len(requestedIDs)) + for _, id := range currentIDs { + if removeSet[id] { + removed = append(removed, id) + continue + } + remaining = append(remaining, id) + } + + return remaining, removed +} + +func resolveUploadFile(path, assetName string) (string, string, int64, error) { + cleanPath := filepath.Clean(strings.TrimSpace(path)) + if cleanPath == "." || cleanPath == "" { + return "", "", 0, fmt.Errorf("--file is required") + } + + info, err := os.Stat(cleanPath) + if err != nil { + return "", "", 0, fmt.Errorf("stat asset file: %w", err) + } + if info.IsDir() { + return "", "", 0, fmt.Errorf("--file must point to a file, got directory %q", cleanPath) + } + + uploadName := strings.TrimSpace(assetName) + if uploadName == "" { + uploadName = filepath.Base(cleanPath) + } + if strings.ContainsAny(uploadName, `/\`) { + return "", "", 0, fmt.Errorf("--asset-name must be a filename, got %q", uploadName) + } + + return cleanPath, uploadName, info.Size(), nil +} + +func deleteAttachment(ctx *common.RuntimeContext, attachmentID string) error { + _, err := ctx.CallAPI("DELETE", fmt.Sprintf("/attachments/%s", attachmentID), nil) + return err } diff --git a/shortcuts/release/assets_test.go b/shortcuts/release/assets_test.go new file mode 100644 index 0000000..b294abe --- /dev/null +++ b/shortcuts/release/assets_test.go @@ -0,0 +1,216 @@ +package release + +import ( + "io" + "net/http" + "os" + "path/filepath" + "testing" +) + +func TestReleaseAssets(t *testing.T) { + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + assertReleaseRequest(t, r, "GET", "/owner/repo/releases/v1.0.json") + writeReleaseJSON(t, w, releaseViewFixture("7")) + }) + defer server.Close() + + if err := runReleaseShortcut(t, server, "assets", map[string]string{"id": "v1.0"}); err != nil { + t.Fatalf("assets shortcut failed: %v", err) + } +} + +func TestReleaseAttachDryRunDoesNotWrite(t *testing.T) { + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/v1.0.json": + writeReleaseJSON(t, w, releaseViewFixture("7")) + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/7/edit.json": + writeReleaseJSON(t, w, releaseEditFixture()) + default: + t.Fatalf("dry-run should not write, got %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runReleaseShortcut(t, server, "attach", map[string]string{ + "id": "v1.0", + "attachment-ids": "34,56", + "dry-run": "true", + }) + if err != nil { + t.Fatalf("attach dry-run failed: %v", err) + } +} + +func TestReleaseAttachWritesMergedAttachmentIDs(t *testing.T) { + var payload map[string]interface{} + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/v1.0.json": + writeReleaseJSON(t, w, releaseViewFixture("7")) + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/7/edit.json": + writeReleaseJSON(t, w, releaseEditFixture()) + case r.Method == http.MethodPut && r.URL.Path == "/owner/repo/releases/7.json": + payload = decodeReleaseJSON(t, r) + writeReleaseJSON(t, w, map[string]interface{}{"status": 0, "message": "updated"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runReleaseShortcut(t, server, "attach", map[string]string{ + "id": "v1.0", + "attachment-ids": "34,56", + }) + if err != nil { + t.Fatalf("attach shortcut failed: %v", err) + } + + assertReleaseEqual(t, payload["name"], "Old release") + assertReleaseEqual(t, payload["tag_name"], "v1.0.0") + assertReleaseStringSlice(t, payload["attachment_ids"], []string{"12", "34", "56"}) +} + +func TestReleaseDetachWritesRemainingAttachmentIDs(t *testing.T) { + var payload map[string]interface{} + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/v1.0.json": + writeReleaseJSON(t, w, releaseViewFixture("7")) + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/7/edit.json": + writeReleaseJSON(t, w, releaseEditFixture()) + case r.Method == http.MethodPut && r.URL.Path == "/owner/repo/releases/7.json": + payload = decodeReleaseJSON(t, r) + writeReleaseJSON(t, w, map[string]interface{}{"status": 0, "message": "updated"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runReleaseShortcut(t, server, "detach", map[string]string{ + "id": "v1.0", + "attachment-ids": "12", + }) + if err != nil { + t.Fatalf("detach shortcut failed: %v", err) + } + + assertReleaseStringSlice(t, payload["attachment_ids"], []string{"34"}) +} + +func TestReleaseUploadUploadsAndAttachesAsset(t *testing.T) { + var payload map[string]interface{} + path := filepath.Join(t.TempDir(), "artifact.zip") + if err := os.WriteFile(path, []byte("artifact-bytes"), 0600); err != nil { + t.Fatalf("write test asset: %v", err) + } + + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/v1.0.json": + writeReleaseJSON(t, w, releaseViewFixture("7")) + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/7/edit.json": + writeReleaseJSON(t, w, releaseEditFixture()) + case r.Method == http.MethodPost && r.URL.Path == "/attachments.json": + if err := r.ParseMultipartForm(1 << 20); err != nil { + t.Fatalf("ParseMultipartForm: %v", err) + } + if got := r.FormValue("description"); got != "CLI binary" { + t.Fatalf("description = %q, want %q", got, "CLI binary") + } + files := r.MultipartForm.File["file"] + if len(files) != 1 { + t.Fatalf("expected 1 uploaded file, got %d", len(files)) + } + if files[0].Filename != "gitlink-cli.zip" { + t.Fatalf("filename = %q, want %q", files[0].Filename, "gitlink-cli.zip") + } + file, err := files[0].Open() + if err != nil { + t.Fatalf("open multipart file: %v", err) + } + defer file.Close() + data, err := io.ReadAll(file) + if err != nil { + t.Fatalf("read multipart file: %v", err) + } + if string(data) != "artifact-bytes" { + t.Fatalf("uploaded body = %q, want %q", string(data), "artifact-bytes") + } + writeReleaseJSON(t, w, map[string]interface{}{"id": "asset-99", "title": "gitlink-cli.zip"}) + case r.Method == http.MethodPut && r.URL.Path == "/owner/repo/releases/7.json": + payload = decodeReleaseJSON(t, r) + writeReleaseJSON(t, w, map[string]interface{}{"status": 0, "message": "updated"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runReleaseShortcut(t, server, "upload", map[string]string{ + "id": "v1.0", + "file": path, + "asset-name": "gitlink-cli.zip", + "description": "CLI binary", + }) + if err != nil { + t.Fatalf("upload shortcut failed: %v", err) + } + + assertReleaseStringSlice(t, payload["attachment_ids"], []string{"12", "34", "asset-99"}) +} + +func TestReleaseUploadCleansUpOnAttachFailure(t *testing.T) { + path := filepath.Join(t.TempDir(), "artifact.zip") + if err := os.WriteFile(path, []byte("artifact-bytes"), 0600); err != nil { + t.Fatalf("write test asset: %v", err) + } + + cleanupCalled := false + server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/v1.0.json": + writeReleaseJSON(t, w, releaseViewFixture("7")) + case r.Method == http.MethodGet && r.URL.Path == "/owner/repo/releases/7/edit.json": + writeReleaseJSON(t, w, releaseEditFixture()) + case r.Method == http.MethodPost && r.URL.Path == "/attachments.json": + writeReleaseJSON(t, w, map[string]interface{}{"id": "asset-99", "title": "artifact.zip"}) + case r.Method == http.MethodPut && r.URL.Path == "/owner/repo/releases/7.json": + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte("update failed")) + case r.Method == http.MethodDelete && r.URL.Path == "/attachments/asset-99.json": + cleanupCalled = true + writeReleaseJSON(t, w, map[string]interface{}{"status": 0, "message": "deleted"}) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + defer server.Close() + + err := runReleaseShortcut(t, server, "upload", map[string]string{ + "id": "v1.0", + "file": path, + }) + if err == nil { + t.Fatal("expected upload shortcut to fail when release update fails") + } + if !cleanupCalled { + t.Fatal("expected uploaded attachment cleanup to run after release update failure") + } +} + +func releaseViewFixture(versionID string) map[string]interface{} { + return map[string]interface{}{ + "version_id": versionID, + "id": "release-gid", + "tag_name": "v1.0.0", + "name": "Old release", + "attachments": []map[string]interface{}{ + {"id": 12, "title": "a.zip"}, + {"id": "34", "title": "b.zip"}, + }, + } +} diff --git a/shortcuts/release/release.go b/shortcuts/release/release.go index ee9e2a6..cf028da 100644 --- a/shortcuts/release/release.go +++ b/shortcuts/release/release.go @@ -13,7 +13,7 @@ import ( func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { tr := shortcutTranslator(translators...) - return []*common.Shortcut{ + shortcuts := []*common.Shortcut{ { Name: "list", Description: tr.T("cmd.release.list.short"), @@ -237,6 +237,7 @@ func Shortcuts(translators ...*i18n.Translator) []*common.Shortcut { Run: runAutoNotes, }, } + return append(shortcuts, releaseAssetShortcuts(tr)...) } func shortcutTranslator(translators ...*i18n.Translator) *i18n.Translator { @@ -383,12 +384,16 @@ func releaseBoolFromArgsOrMap(ctx *common.RuntimeContext, name string, current m if ctx.Arg(name) != "" { return releaseBoolArg(ctx, name, defaultValue) } + return releaseBoolValue(current, name, defaultValue), nil +} + +func releaseBoolValue(current map[string]interface{}, name string, defaultValue bool) bool { if current != nil { if value, ok := current[name].(bool); ok { - return value, nil + return value } } - return defaultValue, nil + return defaultValue } func parseReleaseAttachmentIDs(value string) ([]string, error) { @@ -413,20 +418,10 @@ func parseReleaseAttachmentIDs(value string) ([]string, error) { } func releaseAttachmentIDs(current map[string]interface{}) []string { - if current == nil { - return nil - } - attachments, ok := current["attachments"].([]interface{}) - if !ok { - return nil - } + attachments := releaseAttachments(current) ids := make([]string, 0, len(attachments)) for _, attachment := range attachments { - item, ok := attachment.(map[string]interface{}) - if !ok { - continue - } - if id := releaseIDString(item["id"]); id != "" { + if id := releaseIDString(attachment["id"]); id != "" { ids = append(ids, id) } } diff --git a/shortcuts/release/release_test.go b/shortcuts/release/release_test.go index 13c6fe6..0d0ef3b 100644 --- a/shortcuts/release/release_test.go +++ b/shortcuts/release/release_test.go @@ -5,8 +5,6 @@ import ( "fmt" "net/http" "net/http/httptest" - "os" - "path/filepath" "reflect" "testing" @@ -100,147 +98,6 @@ func TestReleaseView(t *testing.T) { } } -func TestReleaseAssets(t *testing.T) { - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - assertReleaseRequest(t, r, "GET", "/owner/repo/releases/7.json") - writeReleaseJSON(t, w, releaseDetail(serverURL(r), []map[string]interface{}{ - {"id": float64(10), "title": "app.zip", "filesize": "12 KB", "url": "/api/attachments/10"}, - })) - }) - defer server.Close() - - if err := runReleaseShortcut(t, server, "assets", map[string]string{"id": "7"}); err != nil { - t.Fatalf("assets failed: %v", err) - } -} - -func TestReleaseDownloadAssetByName(t *testing.T) { - dir := t.TempDir() - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/owner/repo/releases/7.json": - writeReleaseJSON(t, w, releaseDetail(serverURL(r), []map[string]interface{}{ - {"id": float64(10), "title": "app.zip", "filesize": "12 KB", "url": "/api/attachments/10"}, - })) - case "/api/attachments/10": - w.Header().Set("Content-Type", "application/zip") - _, _ = w.Write([]byte("asset bytes")) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - err := runReleaseShortcut(t, server, "download", map[string]string{ - "id": "7", - "asset": "app.zip", - "output": dir, - }) - if err != nil { - t.Fatalf("download asset failed: %v", err) - } - data, err := os.ReadFile(filepath.Join(dir, "app.zip")) - if err != nil { - t.Fatalf("read downloaded file: %v", err) - } - if string(data) != "asset bytes" { - t.Fatalf("downloaded data = %q", data) - } -} - -func TestReleaseDownloadSingleAssetByDefault(t *testing.T) { - dir := t.TempDir() - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/owner/repo/releases/7.json": - writeReleaseJSON(t, w, releaseDetail(serverURL(r), []map[string]interface{}{ - {"id": float64(10), "title": "../unsafe.txt", "url": "/owner/repo/releases/download/v1/unsafe.txt"}, - })) - case "/owner/repo/releases/download/v1/unsafe.txt": - _, _ = w.Write([]byte("safe")) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - err := runReleaseShortcut(t, server, "download", map[string]string{ - "id": "7", - "output": dir, - }) - if err != nil { - t.Fatalf("download default asset failed: %v", err) - } - if _, err := os.Stat(filepath.Join(dir, "unsafe.txt")); err != nil { - t.Fatalf("expected sanitized file: %v", err) - } -} - -func TestReleaseDownloadArchiveZip(t *testing.T) { - out := filepath.Join(t.TempDir(), "source.zip") - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/owner/repo/releases/7.json": - writeReleaseJSON(t, w, releaseDetail(serverURL(r), nil)) - case "/archive/v1.0.zip": - _, _ = w.Write([]byte("zip bytes")) - default: - t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) - } - }) - defer server.Close() - - err := runReleaseShortcut(t, server, "download", map[string]string{ - "id": "7", - "archive": "zip", - "output": out, - }) - if err != nil { - t.Fatalf("download archive failed: %v", err) - } - data, err := os.ReadFile(out) - if err != nil { - t.Fatalf("read archive: %v", err) - } - if string(data) != "zip bytes" { - t.Fatalf("archive data = %q", data) - } -} - -func TestReleaseDownloadRequiresAssetWhenMultipleAttachments(t *testing.T) { - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - writeReleaseJSON(t, w, releaseDetail(serverURL(r), []map[string]interface{}{ - {"id": float64(10), "title": "a.zip", "url": "/api/attachments/10"}, - {"id": float64(11), "title": "b.zip", "url": "/api/attachments/11"}, - })) - }) - defer server.Close() - - err := runReleaseShortcut(t, server, "download", map[string]string{"id": "7", "output": t.TempDir()}) - if err == nil { - t.Fatal("expected error when multiple attachments need --asset") - } -} - -func TestReleaseDownloadDoesNotOverwriteWithoutForce(t *testing.T) { - dir := t.TempDir() - out := filepath.Join(dir, "app.zip") - if err := os.WriteFile(out, []byte("exists"), 0644); err != nil { - t.Fatalf("seed output file: %v", err) - } - server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { - writeReleaseJSON(t, w, releaseDetail(serverURL(r), []map[string]interface{}{ - {"id": float64(10), "title": "app.zip", "url": "/api/attachments/10"}, - })) - }) - defer server.Close() - - err := runReleaseShortcut(t, server, "download", map[string]string{"id": "7", "output": dir}) - if err == nil { - t.Fatal("expected overwrite protection error") - } -} - func TestReleaseUpdatePreservesExistingFields(t *testing.T) { var payload map[string]interface{} server := newReleaseTestServer(t, func(w http.ResponseWriter, r *http.Request) { @@ -472,7 +329,7 @@ func TestReleaseShortcutNames(t *testing.T) { for _, shortcut := range Shortcuts() { got[shortcut.Name] = true } - want := []string{"list", "create", "edit", "view", "assets", "download", "update", "delete"} + want := []string{"list", "create", "edit", "view", "update", "delete", "assets", "attach", "detach", "upload"} for _, name := range want { if !got[name] { t.Fatalf("missing shortcut %q in %v", name, got) @@ -534,25 +391,6 @@ func releaseEditFixture() map[string]interface{} { } } -func releaseDetail(baseURL string, attachments []map[string]interface{}) map[string]interface{} { - rawAttachments := make([]interface{}, 0, len(attachments)) - for _, attachment := range attachments { - rawAttachments = append(rawAttachments, attachment) - } - return map[string]interface{}{ - "version_id": float64(7), - "tag_name": "v1.0", - "name": "v1.0", - "tarball_url": baseURL + "/archive/v1.0.tar.gz", - "zipball_url": baseURL + "/archive/v1.0.zip", - "attachments": rawAttachments, - } -} - -func serverURL(r *http.Request) string { - return "http://" + r.Host -} - func assertReleaseRequest(t *testing.T, r *http.Request, method, path string) { t.Helper() if r.Method != method || r.URL.Path != path { @@ -612,8 +450,10 @@ func ExampleShortcuts() { // create // edit // view - // assets - // download // update // delete + // assets + // attach + // detach + // upload }