diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee15e25..0fd93aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: ['1.26', '1.27'] + go-version: ['1.27'] steps: - name: Checkout code uses: actions/checkout@v4 @@ -50,7 +50,7 @@ jobs: run: go test -short -v -race -count=1 -coverprofile=coverage.out ./... - name: Upload coverage - if: matrix.go-version == '1.26' + if: matrix.go-version == '1.27' uses: codecov/codecov-action@v4 with: file: ./coverage.out @@ -67,7 +67,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.26' + go-version: '1.27' - name: Run golangci-lint uses: golangci/golangci-lint-action@v7 @@ -85,7 +85,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.26' + go-version: '1.27' - name: Check formatting run: | @@ -111,7 +111,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.26' + go-version: '1.27' - name: Run integration tests env: @@ -134,7 +134,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: '1.26' + go-version: '1.27' - name: Build run: go build ./... diff --git a/.golangci.yml b/.golangci.yml index 43c2b5f..4d9d56c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ version: "2" run: - go: "1.26" + go: "1.27" output: formats: text: diff --git a/CHANGELOG.md b/CHANGELOG.md index 246c7ce..70681ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,78 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [v0.16.0] - 2026-11-11 +### Fixed +- **Retry-After 三道闸（issue #4）**：`getRetryWaitTime` 的 `Retry-After` + 分支此前直接返回未校验的秒数——3600 秒照等（越过 `MaxBackoff`）、 + 负数秒产出负时长（`time.After` 立即触发，退避退化成忙等）、 + HTTP-date 形式经 `Atoi` 必然失败被静默忽略。现在统一走 + `retryAfterDuration`：非正值（负数/零/过去的日期）与不可解析值 + 回落指数退避；delay-seconds 与 HTTP-date（`http.ParseTime`）两种 + RFC 9110 形式都支持；解析结果与指数退避共用同一道 `MaxBackoff` + 钳制；超大秒数在 Duration 转换前封顶防溢出。零值 + `InitialBackoff`/`MaxBackoff` 按文档默认（1s/30s）归一。 +- **Paginate 页数封顶（issue #5）**：服务端异常恒返满页时，旧实现 + 300ms 可打出 12 万次请求、只能靠 ctx 止损。现在游走以 + `maxPaginatePages = 50`（与 go-gongfeng 对齐）封顶，触顶 yield + 哨兵错误 `ErrTooManyPages`（errors.Is 可判定）而非静默结束—— + 「封顶截断」与「取完」可区分。注：未做「与上一页内容相同即停」 + 的短路——合法数据里跨页全同（如 200 条同名项）无法与服务端 + 忽略分页参数区分，误杀面大于收益，封顶已把损害有界化。 +- **383 处端点路径统一转义（issue #6）**：全仓端点路径从裸 + `fmt.Sprintf` 收编到新 helper `pathf`——路径侧的字符串实参一律 + `url.PathEscape`（`/`→`%2F`、`?`→`%3F`、`#`→`%23`，调用方控制 + 的 owner/repo/org 被隔离在自己的段内，不再能插层级或改写端点）； + `?` 后的 query 实参不动；`%d` 等数值动词原样；仓库文件路径这类 + 天然含 `/` 的参数走 `pathSegs`（逐段转义、保留分隔符）。顺带把 + 六处裸拼 query 值（search 三件的 `q=`/过滤参数、events 的 + year/next、namespace 的 path、archive 的 ref/format、contents/raw + 的 ref）改为 `url.QueryEscape`/`url.Values`。 +- **响应体读取有界（issue #7）**：client/upload/oauth 全部 9 处 + `io.ReadAll` 收口到 `readBodyLimited`（`io.LimitReader` 实现）， + 分层默认上限：JSON 端点 16 MiB、二进制下载（raw 文件/release + 资产/artifact/日志归档）1 GiB；超限报 + `ResponseBodyTooLargeError`（`IsResponseBodyTooLarge` 判定）而非 + 无界分配（实测 256MB 响应曾致堆 +569.7MB）。 +- **client 字段读侧持锁（issue #8）**：`baseURL`/`httpClient` 的 + 读点（doRequest/doRawRequest/doFormRequest/doRawBodyRequest/ + retry 循环/upload）此前全部裸读，与持锁写的 + `SetHTTPClient`/`SetBaseURL` 构成 data race（interface 撕裂可致 + runtime crash）。现在统一经 `requestSnapshot`（RLock 下取值）， + 与仓内 `retryPolicy`/hooks 的既有读法对齐。 +- **upload 走共享管线（issue #9）**：`UploadFileReader`/ + `UploadImageReader` 此前自建请求直发——审计 hook 从不触发、 + 429/5xx 从不重试，与 CHANGELOG「All HTTP request methods now run + request/response hooks and support retry policy」的承诺相悖。现在 + 构造 multipart body 后走 `doBufferedRequestAt`（鉴权 → request + hook → 重试 → response hook → 有界读体）。同时修复一个连带的 + 潜伏缺陷：重试第 2+ 次尝试此前复用已耗尽的 body reader（发空 + body），现在经 `req.GetBody` 重放——JSON POST 与 multipart 上传 + 的重试都因此真正可重入（有测试断言三次尝试 body 逐字一致）。 +- **ListRepositories 不再丢 Type/Sort（issue #10）**： + `ListRepositoriesOptions` 新增自有 `toQuery`（遮蔽嵌入的 + `ListOptions.toQuery`），把 `type`/`sort` 一并上 wire 且经 + `QueryEscape`；`ListRepositories`（含 Owner 分支）与 + `ListCurrentUserRepositories` 收敛到同一 helper，杜绝同端点同 + 结构体再次分叉。 + +### Changed +- **请求执行核心合并**：doRequestAt/doFormRequest/doRawBodyRequest + 五份复制粘贴的管线（鉴权→hook→重试→hook→读体→解码）合并为 + `doBufferedRequestAt` 单核；doRawRequest/doRawRequestAt 共享 + `doRawRequestWith`。行为差异收敛：form 请求对空 body 从「json + unmarshal 报错」变为「跳过解码」（与其余路径一致）。 +- **地板提升**：go 指令 1.26→1.27（CI matrix/lint/fmt/integration/ + release 与 .golangci.yml 同步）；删除手写 `min`，用内建； + `for range n`、`slices.Clone` 就位。本仓零第三方依赖， + `go get -u ./...` 无可升项。 + +### Added +- `SetMaxResponseBodySize`/`MaxResponseBodySize`：全局覆盖响应体 + 读上限（<=0 恢复分层默认）。 +- `ResponseBodyTooLargeError`/`IsResponseBodyTooLarge`、 + `ErrTooManyPages` 哨兵，供调用方程序化判定。 + ## [v0.15.0] - 2026-11-06 ### Added - **`FindActionRunByHeadSHA`（issue #2 的诚实解法）**：v8 wire 的 runs 列表 diff --git a/README.md b/README.md index 41579cd..0936e13 100644 --- a/README.md +++ b/README.md @@ -965,7 +965,9 @@ if err != nil { ### Rate Limit 自动重试 -配置自动重试策略，遇到 429/5xx 自动等待重试： +配置自动重试策略，遇到 429/5xx 自动等待重试。服务端返回 `Retry-After` +（delay-seconds 或 HTTP-date 均可）时优先采用该等待，但与指数退避共用 +`MaxBackoff` 上限；非正值与不可解析值回落指数退避： ```go client.SetRetryPolicy(gitcode.RetryPolicy{ @@ -979,7 +981,10 @@ client.SetRetryPolicy(gitcode.RetryPolicy{ ### 自动分页 -自动遍历所有分页，无需手动处理 `Page` 参数： +自动遍历所有分页，无需手动处理 `Page` 参数。分页游走以 50 页封顶 +（`per_page=100` × 50 页远超任何真实列表）：触顶时迭代器 yield 哨兵错误 +`gitcode.ErrTooManyPages`（`errors.Is` 可判定），表示结果被截断而非取完—— +服务端异常恒返满页时不会无限打请求： ```go // 收集所有结果到一个 slice diff --git a/actions_v8.go b/actions_v8.go index 79b60c1..94e6918 100644 --- a/actions_v8.go +++ b/actions_v8.go @@ -25,7 +25,6 @@ import ( "context" "encoding/base64" "errors" - "fmt" "net/http" "net/url" "strconv" @@ -76,7 +75,7 @@ type ListWorkflowsResponse struct { // GET /v8/repos/{owner}/{repo}/actions/workflows (page/per_page). func (c *Client) ListActionWorkflows(ctx context.Context, owner, repo string, opts ListOptions) (*ListWorkflowsResponse, error) { var resp ListWorkflowsResponse - err := c.doRequestAt(ctx, c.v8BaseURL(), http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/workflows?%s", owner, repo, opts.toQuery()), nil, &resp) + err := c.doRequestAt(ctx, c.v8BaseURL(), http.MethodGet, pathf("/repos/%s/%s/actions/workflows?%s", owner, repo, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -107,7 +106,7 @@ func (c *Client) DispatchActionWorkflow(ctx context.Context, owner, repo, workfl opts.Inputs = map[string]string{} } var resp DispatchActionWorkflowResult - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/workflows/%s/dispatches", owner, repo, workflowID), opts, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/workflows/%s/dispatches", owner, repo, workflowID), opts, &resp) if err != nil { return nil, err } @@ -143,7 +142,7 @@ type WorkflowValidation struct { func (c *Client) ValidateActionWorkflowYAML(ctx context.Context, owner, repo string, yamlContent []byte) (*WorkflowValidation, error) { body := map[string]string{"base64_content": base64.StdEncoding.EncodeToString(yamlContent)} var resp WorkflowValidation - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/workflows/validate", owner, repo), body, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/workflows/validate", owner, repo), body, &resp) if err != nil { return nil, err } @@ -226,7 +225,7 @@ type ListWorkflowRunsResponse struct { // GET /v8/repos/{owner}/{repo}/actions/runs. func (c *Client) ListActionRuns(ctx context.Context, owner, repo string, opts ListWorkflowRunsOptions) (*ListWorkflowRunsResponse, error) { var resp ListWorkflowRunsResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runs?%s", owner, repo, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runs?%s", owner, repo, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -367,7 +366,7 @@ type WorkflowRunDetail struct { // GET /v8/repos/{owner}/{repo}/actions/runs/{run_id}. func (c *Client) GetActionRun(ctx context.Context, owner, repo, runID string) (*WorkflowRunDetail, error) { var resp WorkflowRunDetail - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runs/%s", owner, repo, runID), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runs/%s", owner, repo, runID), nil, &resp) if err != nil { return nil, err } @@ -384,7 +383,7 @@ type stopRetryRerunResult struct { // POST /v8/repos/{owner}/{repo}/actions/runs/{run_id}/stop → {success}. func (c *Client) StopActionRun(ctx context.Context, owner, repo, runID string) (bool, error) { var resp stopRetryRerunResult - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/stop", owner, repo, runID), nil, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/runs/%s/stop", owner, repo, runID), nil, &resp) return resp.Success, err } @@ -393,7 +392,7 @@ func (c *Client) StopActionRun(ctx context.Context, owner, repo, runID string) ( // POST /v8/repos/{owner}/{repo}/actions/runs/{run_id}/retry → {success}. func (c *Client) RetryActionRun(ctx context.Context, owner, repo, runID string) (bool, error) { var resp stopRetryRerunResult - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/retry", owner, repo, runID), nil, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/runs/%s/retry", owner, repo, runID), nil, &resp) return resp.Success, err } @@ -402,7 +401,7 @@ func (c *Client) RetryActionRun(ctx context.Context, owner, repo, runID string) // POST /v8/repos/{owner}/{repo}/actions/runs/{run_id}/rerun → {success}. func (c *Client) RerunActionRun(ctx context.Context, owner, repo, runID string) (bool, error) { var resp stopRetryRerunResult - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/rerun", owner, repo, runID), nil, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/runs/%s/rerun", owner, repo, runID), nil, &resp) return resp.Success, err } @@ -418,7 +417,7 @@ type ListActionJobsResponse struct { // GET /v8/repos/{owner}/{repo}/actions/runs/{run_id}/jobs. func (c *Client) ListActionRunJobs(ctx context.Context, owner, repo, runID string) (*ListActionJobsResponse, error) { var resp ListActionJobsResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/jobs", owner, repo, runID), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runs/%s/jobs", owner, repo, runID), nil, &resp) if err != nil { return nil, err } @@ -431,7 +430,7 @@ func (c *Client) ListActionRunJobs(ctx context.Context, owner, repo, runID strin // GET /v8/repos/{owner}/{repo}/actions/runs/{run_id}/jobs/{job_id}. func (c *Client) GetActionJob(ctx context.Context, owner, repo, runID, jobID string) (*ActionJob, error) { var job ActionJob - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/jobs/%s", owner, repo, runID, jobID), nil, &job) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runs/%s/jobs/%s", owner, repo, runID, jobID), nil, &job) if err != nil { return nil, err } @@ -462,7 +461,7 @@ type ActionJobLogs struct { // POST /v8/repos/{owner}/{repo}/actions/runs/{run_id}/jobs/{job_id}/logs. func (c *Client) GetActionJobLogs(ctx context.Context, owner, repo, runID, jobID string, opts GetActionJobLogsOptions) (*ActionJobLogs, error) { var resp ActionJobLogs - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/jobs/%s/logs", owner, repo, runID, jobID), opts, &resp) + err := c.doV8Request(ctx, http.MethodPost, pathf("/repos/%s/%s/actions/runs/%s/jobs/%s/logs", owner, repo, runID, jobID), opts, &resp) if err != nil { return nil, err } @@ -474,7 +473,7 @@ func (c *Client) GetActionJobLogs(ctx context.Context, owner, repo, runID, jobID // GET /v8/repos/{owner}/{repo}/actions/runs/{run_id}/jobs/{job_id}/download-log. func (c *Client) DownloadActionJobLogs(ctx context.Context, owner, repo, runID, jobID string) ([]byte, error) { return c.doRawRequestAt(ctx, c.v8BaseURL(), http.MethodGet, - fmt.Sprintf("/repos/%s/%s/actions/runs/%s/jobs/%s/download-log", owner, repo, runID, jobID)) + pathf("/repos/%s/%s/actions/runs/%s/jobs/%s/download-log", owner, repo, runID, jobID)) } // --- Artifacts --- @@ -525,7 +524,7 @@ type ListActionArtifactsResponse struct { // GET /v8/repos/{owner}/{repo}/actions/artifacts. func (c *Client) ListActionArtifacts(ctx context.Context, owner, repo string, opts ListActionArtifactsOptions) (*ListActionArtifactsResponse, error) { var resp ListActionArtifactsResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/artifacts?%s", owner, repo, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/artifacts?%s", owner, repo, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -537,7 +536,7 @@ func (c *Client) ListActionArtifacts(ctx context.Context, owner, repo string, op // GET /v8/repos/{owner}/{repo}/actions/runs/{run_id}/artifacts. func (c *Client) ListActionRunArtifacts(ctx context.Context, owner, repo, runID string, opts ListOptions) (*ListActionArtifactsResponse, error) { var resp ListActionArtifactsResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runs/%s/artifacts?%s", owner, repo, runID, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runs/%s/artifacts?%s", owner, repo, runID, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -549,7 +548,7 @@ func (c *Client) ListActionRunArtifacts(ctx context.Context, owner, repo, runID // GET /v8/repos/{owner}/{repo}/actions/artifacts/{artifact_id}. func (c *Client) GetActionArtifact(ctx context.Context, owner, repo, artifactID string) (*ActionArtifact, error) { var artifact ActionArtifact - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/artifacts/%s", owner, repo, artifactID), nil, &artifact) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/artifacts/%s", owner, repo, artifactID), nil, &artifact) if err != nil { return nil, err } @@ -560,7 +559,7 @@ func (c *Client) GetActionArtifact(ctx context.Context, owner, repo, artifactID // // DELETE /v8/repos/{owner}/{repo}/actions/artifacts/{artifact_id}. func (c *Client) DeleteActionArtifact(ctx context.Context, owner, repo, artifactID string) error { - return c.doV8Request(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/actions/artifacts/%s", owner, repo, artifactID), nil, nil) + return c.doV8Request(ctx, http.MethodDelete, pathf("/repos/%s/%s/actions/artifacts/%s", owner, repo, artifactID), nil, nil) } // DownloadActionArtifact downloads an artifact archive in format (the @@ -569,7 +568,7 @@ func (c *Client) DeleteActionArtifact(ctx context.Context, owner, repo, artifact // GET /v8/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/{archive_format}. func (c *Client) DownloadActionArtifact(ctx context.Context, owner, repo, artifactID, format string) ([]byte, error) { return c.doRawRequestAt(ctx, c.v8BaseURL(), http.MethodGet, - fmt.Sprintf("/repos/%s/%s/actions/artifacts/%s/%s", owner, repo, artifactID, format)) + pathf("/repos/%s/%s/actions/artifacts/%s/%s", owner, repo, artifactID, format)) } // --- Runners --- @@ -620,7 +619,7 @@ type ListActionRunnersResponse struct { // GET /v8/repos/{owner}/{repo}/actions/runners. func (c *Client) ListRepoActionRunners(ctx context.Context, owner, repo string, opts ListActionRunnersOptions) (*ListActionRunnersResponse, error) { var resp ListActionRunnersResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/runners?%s", owner, repo, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/runners?%s", owner, repo, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -633,7 +632,7 @@ func (c *Client) ListRepoActionRunners(ctx context.Context, owner, repo string, // envelopes mirror the repo host-runners shape. func (c *Client) listRepoRunnersVariant(ctx context.Context, owner, repo, suffix string, opts ListActionRunnersOptions) (*ListActionRunnersResponse, error) { var resp ListActionRunnersResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/actions/%s?%s", owner, repo, suffix, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/repos/%s/%s/actions/%s?%s", owner, repo, suffix, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -684,7 +683,7 @@ type ListOrgRunnerGroupsResponse struct { // GET /v8/orgs/{org}/actions/runner-groups. func (c *Client) ListOrgActionRunnerGroups(ctx context.Context, org string, opts ListActionRunnersOptions) (*ListOrgRunnerGroupsResponse, error) { var resp ListOrgRunnerGroupsResponse - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/actions/runner-groups?%s", org, opts.toQuery()), nil, &resp) + err := c.doV8Request(ctx, http.MethodGet, pathf("/orgs/%s/actions/runner-groups?%s", org, opts.toQuery()), nil, &resp) if err != nil { return nil, err } @@ -696,7 +695,7 @@ func (c *Client) ListOrgActionRunnerGroups(ctx context.Context, org string, opts // GET /v8/orgs/{org}/actions/runner-groups/{group_id}. func (c *Client) GetActionRunnerGroup(ctx context.Context, org, groupID string) (*ActionRunnerGroup, error) { var group ActionRunnerGroup - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/actions/runner-groups/%s", org, groupID), nil, &group) + err := c.doV8Request(ctx, http.MethodGet, pathf("/orgs/%s/actions/runner-groups/%s", org, groupID), nil, &group) if err != nil { return nil, err } diff --git a/branches.go b/branches.go index 23ce3a8..6a26ad9 100644 --- a/branches.go +++ b/branches.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -49,7 +48,7 @@ type BranchProtection struct { func (c *Client) ListBranches(ctx context.Context, owner, repo string) ([]*Branch, error) { var branches []*Branch - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/branches?per_page=100", owner, repo), nil, &branches) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/branches?per_page=100", owner, repo), nil, &branches) if err != nil { return nil, err } @@ -58,7 +57,7 @@ func (c *Client) ListBranches(ctx context.Context, owner, repo string) ([]*Branc func (c *Client) GetBranch(ctx context.Context, owner, repo, branch string) (*Branch, error) { var b Branch - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, &b) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, &b) if err != nil { return nil, err } @@ -73,7 +72,7 @@ type CreateBranchOptions struct { func (c *Client) CreateBranch(ctx context.Context, owner, repo string, opts CreateBranchOptions) (*Branch, error) { var b Branch - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/branches", owner, repo), opts, &b) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/branches", owner, repo), opts, &b) if err != nil { return nil, err } @@ -81,7 +80,7 @@ func (c *Client) CreateBranch(ctx context.Context, owner, repo string, opts Crea } func (c *Client) DeleteBranch(ctx context.Context, owner, repo, branch string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, nil) } type ListBranchProtectionOptions struct { @@ -89,7 +88,7 @@ type ListBranchProtectionOptions struct { } func (c *Client) DeleteBranchProtection(ctx context.Context, owner, repo, name string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/branches/%s/setting", owner, repo, name), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/branches/%s/setting", owner, repo, name), nil, nil) } type CommitComparison struct { @@ -102,7 +101,7 @@ type CommitComparison struct { func (c *Client) CompareCommits(ctx context.Context, owner, repo, base, head string) (*CommitComparison, error) { var cmp CommitComparison - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/compare/%s...%s", owner, repo, base, head), nil, &cmp) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/compare/%s...%s", owner, repo, base, head), nil, &cmp) if err != nil { return nil, err } @@ -111,7 +110,7 @@ func (c *Client) CompareCommits(ctx context.Context, owner, repo, base, head str func (c *Client) GetCommit(ctx context.Context, owner, repo, sha string) (*Commit, error) { var commit Commit - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits/%s", owner, repo, sha), nil, &commit) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits/%s", owner, repo, sha), nil, &commit) if err != nil { return nil, err } @@ -137,7 +136,7 @@ func (c *Client) ListCommits(ctx context.Context, owner, repo string, opts ListC if opts.Until != "" { query += "&until=" + opts.Until } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits?%s", owner, repo, query), nil, &commits) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits?%s", owner, repo, query), nil, &commits) if err != nil { return nil, err } @@ -150,5 +149,5 @@ type UpdateBranchProtectionOptions struct { } func (c *Client) UpdateBranchProtection(ctx context.Context, owner, repo, wildcard string, opts UpdateBranchProtectionOptions) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/branches/%s/setting", owner, repo, wildcard), opts, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/branches/%s/setting", owner, repo, wildcard), opts, nil) } diff --git a/client.go b/client.go index 1ec869f..c57c0ea 100644 --- a/client.go +++ b/client.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -23,8 +24,50 @@ const ( // EnvGitToken is the environment variable for the GitCode API token. EnvGitToken = "GITCODE_TOKEN" + + // defaultMaxJSONBodyBytes caps how much of a JSON API response is read + // into memory. JSON payloads are small; anything past this means a + // broken or hostile server, and refusing beats OOM-ing the process + // (issue #7). + defaultMaxJSONBodyBytes int64 = 16 << 20 // 16 MiB + + // defaultMaxRawBodyBytes caps binary downloads (raw files, release + // assets, artifacts, log archives). These are legitimately large, so + // the ceiling is far higher — but still a ceiling. + defaultMaxRawBodyBytes int64 = 1 << 30 // 1 GiB ) +// ResponseBodyTooLargeError is returned when a response body exceeds the +// configured read limit instead of being read unbounded into memory. +type ResponseBodyTooLargeError struct { + // Limit is the maximum number of bytes that would have been read. + Limit int64 +} + +func (e *ResponseBodyTooLargeError) Error() string { + return fmt.Sprintf("gitcode: response body exceeds %d-byte limit", e.Limit) +} + +// IsResponseBodyTooLarge checks if an error is a ResponseBodyTooLargeError. +func IsResponseBodyTooLarge(err error) bool { + var target *ResponseBodyTooLargeError + return errors.As(err, &target) +} + +// readBodyLimited reads r fully into memory but never more than limit +// bytes; past the limit it stops and reports ResponseBodyTooLargeError so a +// broken or hostile server cannot trigger unbounded allocation (issue #7). +func readBodyLimited(r io.Reader, limit int64) ([]byte, error) { + b, err := io.ReadAll(io.LimitReader(r, limit+1)) + if err != nil { + return b, err + } + if int64(len(b)) > limit { + return b, &ResponseBodyTooLargeError{Limit: limit} + } + return b, nil +} + // Client is the GitCode API client. type Client struct { baseURL string @@ -33,6 +76,11 @@ type Client struct { authStyle AuthStyle mu sync.RWMutex + // maxBodyBytes overrides the per-response body read limit for every + // request; 0 means "use the tiered defaults" (defaultMaxJSONBodyBytes + // for JSON endpoints, defaultMaxRawBodyBytes for binary downloads). + maxBodyBytes int64 + // Retry policy for failed requests. retryPolicy *RetryPolicy @@ -122,6 +170,54 @@ func (c *Client) BaseURL() string { return c.baseURL } +// SetMaxResponseBodySize caps how many bytes of any response body are read +// into memory. Values <= 0 restore the tiered defaults (16 MiB for JSON +// endpoints, 1 GiB for binary downloads). The limit is enforced on every +// request path; exceeding it fails with ResponseBodyTooLargeError (issue #7). +func (c *Client) SetMaxResponseBodySize(n int64) { + c.mu.Lock() + defer c.mu.Unlock() + if n <= 0 { + n = 0 + } + c.maxBodyBytes = n +} + +// MaxResponseBodySize returns the configured response body limit; 0 means +// the tiered defaults are in effect. +func (c *Client) MaxResponseBodySize() int64 { + c.mu.RLock() + defer c.mu.RUnlock() + return c.maxBodyBytes +} + +// jsonBodyLimit is the read limit for JSON API responses. +func (c *Client) jsonBodyLimit() int64 { + if n := c.MaxResponseBodySize(); n > 0 { + return n + } + return defaultMaxJSONBodyBytes +} + +// rawBodyLimit is the read limit for binary download responses. +func (c *Client) rawBodyLimit() int64 { + if n := c.MaxResponseBodySize(); n > 0 { + return n + } + return defaultMaxRawBodyBytes +} + +// requestSnapshot captures the request-time mutable settings under the read +// lock. Every read of baseURL/httpClient on the request path goes through +// it so that concurrent SetHTTPClient/SetBaseURL calls cannot race the +// readers — the setters write under the same mutex, and until now the +// readers did not hold it (issue #8). +func (c *Client) requestSnapshot() (base string, hc *http.Client) { + c.mu.RLock() + defer c.mu.RUnlock() + return c.baseURL, c.httpClient +} + // setAuthHeader sets the authentication header on the request. func (c *Client) setAuthHeader(req *http.Request) { c.mu.RLock() @@ -143,7 +239,8 @@ func (c *Client) setAuthHeader(req *http.Request) { // doRequest executes an HTTP request and unmarshals the result. func (c *Client) doRequest(ctx context.Context, method, path string, body interface{}, result interface{}) error { - return c.doRequestAt(ctx, c.baseURL, method, path, body, result) + base, _ := c.requestSnapshot() + return c.doRequestAt(ctx, base, method, path, body, result) } // doRequestAt executes an HTTP request against an explicit base URL and @@ -151,13 +248,29 @@ func (c *Client) doRequest(ctx context.Context, method, path string, body interf // (doRequest) and the v8 API (actions_v8.go), whose paths differ only by // the base. func (c *Client) doRequestAt(ctx context.Context, base, method, path string, body interface{}, result interface{}) error { - var reqBody io.Reader + var raw []byte if body != nil { - raw, err := json.Marshal(body) + var err error + raw, err = json.Marshal(body) if err != nil { return err } - reqBody = bytes.NewReader(raw) + } + return c.doBufferedRequestAt(ctx, base, method, path, "application/json", raw, result, c.jsonBodyLimit()) +} + +// doBufferedRequestAt is the single shared execution pipeline: +// auth header → request hooks → retry loop → response hooks → +// size-limited body read → status check → JSON decode. +// +// Every request shape — JSON bodies, form encoding, multipart uploads — +// funnels through it, so request/response hooks and the retry policy apply +// uniformly (issue #9: uploads used to bypass both). body may be nil (no +// request body); limit caps the response body read (issue #7). +func (c *Client) doBufferedRequestAt(ctx context.Context, base, method, path, contentType string, body []byte, result interface{}, limit int64) error { + var reqBody io.Reader + if body != nil { + reqBody = bytes.NewReader(body) } req, err := http.NewRequestWithContext(ctx, method, base+path, reqBody) @@ -166,7 +279,9 @@ func (c *Client) doRequestAt(ctx context.Context, base, method, path string, bod } c.setAuthHeader(req) - req.Header.Set("Content-Type", "application/json") + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } // Run request hooks if err := c.runRequestHooks(req); err != nil { @@ -185,7 +300,7 @@ func (c *Client) doRequestAt(ctx context.Context, base, method, path string, bod return fmt.Errorf("response hook error: %w", err) } - respBody, err := io.ReadAll(resp.Body) + respBody, err := readBodyLimited(resp.Body, limit) if err != nil { return err } @@ -194,11 +309,9 @@ func (c *Client) doRequestAt(ctx context.Context, base, method, path string, bod return newAPIError(method, path, resp.StatusCode, string(respBody)) } - if result != nil && resp.StatusCode != http.StatusNoContent { - if len(respBody) > 0 { - if err := json.Unmarshal(respBody, result); err != nil { - return err - } + if result != nil && resp.StatusCode != http.StatusNoContent && len(respBody) > 0 { + if err := json.Unmarshal(respBody, result); err != nil { + return err } } @@ -212,114 +325,52 @@ func (c *Client) doRawRequestAt(ctx context.Context, base, method, path string) if err != nil { return nil, err } - c.setAuthHeader(req) - if err := c.runRequestHooks(req); err != nil { - return nil, fmt.Errorf("request hook error: %w", err) - } - resp, err := c.doRequestWithRetry(ctx, req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - if err := c.runResponseHooks(resp); err != nil { - return nil, fmt.Errorf("response hook error: %w", err) - } - body, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - if resp.StatusCode >= 400 { - return nil, newAPIError(method, path, resp.StatusCode, string(body[:min(len(body), 200)])) - } - return body, nil + return c.doRawRequestWith(ctx, req, method, path) } // doRawRequest executes a raw HTTP request and returns the response body. func (c *Client) doRawRequest(ctx context.Context, method, path string) ([]byte, error) { - req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, nil) + base, _ := c.requestSnapshot() + req, err := http.NewRequestWithContext(ctx, method, base+path, nil) if err != nil { return nil, err } + return c.doRawRequestWith(ctx, req, method, path) +} +// doRawRequestWith runs an already-built request through the shared +// pipeline (hooks + retry) and returns the raw, size-limited body. +func (c *Client) doRawRequestWith(ctx context.Context, req *http.Request, method, path string) ([]byte, error) { c.setAuthHeader(req) - - // Run request hooks if err := c.runRequestHooks(req); err != nil { return nil, fmt.Errorf("request hook error: %w", err) } - - // Execute request with retry resp, err := c.doRequestWithRetry(ctx, req) if err != nil { return nil, err } defer resp.Body.Close() - - // Run response hooks if err := c.runResponseHooks(resp); err != nil { return nil, fmt.Errorf("response hook error: %w", err) } - - body, err := io.ReadAll(resp.Body) + body, err := readBodyLimited(resp.Body, c.rawBodyLimit()) if err != nil { return nil, err } - if resp.StatusCode >= 400 { return nil, newAPIError(method, path, resp.StatusCode, string(body[:min(len(body), 200)])) } - return body, nil } // doFormRequest executes a form-encoded HTTP request. func (c *Client) doFormRequest(ctx context.Context, method, path string, params url.Values, result interface{}) error { - var reqBody io.Reader + var raw []byte if params != nil { - reqBody = strings.NewReader(params.Encode()) + raw = []byte(params.Encode()) } - - req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, reqBody) - if err != nil { - return err - } - - c.setAuthHeader(req) - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - - // Run request hooks - if err := c.runRequestHooks(req); err != nil { - return fmt.Errorf("request hook error: %w", err) - } - - // Execute request with retry - resp, err := c.doRequestWithRetry(ctx, req) - if err != nil { - return err - } - defer resp.Body.Close() - - // Run response hooks - if err := c.runResponseHooks(resp); err != nil { - return fmt.Errorf("response hook error: %w", err) - } - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return err - } - - if resp.StatusCode >= 400 { - return newAPIError(method, path, resp.StatusCode, string(respBody)) - } - - if result != nil && resp.StatusCode != http.StatusNoContent { - if err := json.Unmarshal(respBody, result); err != nil { - return err - } - } - - return nil + base, _ := c.requestSnapshot() + return c.doBufferedRequestAt(ctx, base, method, path, "application/x-www-form-urlencoded", raw, result, c.jsonBodyLimit()) } // doRawBodyRequest executes an HTTP request with a raw JSON body. @@ -328,55 +379,8 @@ func (c *Client) doRawBodyRequest(ctx context.Context, method, path string, body if err != nil { return err } - - req, err := http.NewRequestWithContext(ctx, method, c.baseURL+path, bytes.NewReader(raw)) - if err != nil { - return err - } - - c.setAuthHeader(req) - req.Header.Set("Content-Type", "application/json") - - // Run request hooks - if err := c.runRequestHooks(req); err != nil { - return fmt.Errorf("request hook error: %w", err) - } - - // Execute request with retry - resp, err := c.doRequestWithRetry(ctx, req) - if err != nil { - return err - } - defer resp.Body.Close() - - // Run response hooks - if err := c.runResponseHooks(resp); err != nil { - return fmt.Errorf("response hook error: %w", err) - } - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return err - } - - if resp.StatusCode >= 400 { - return newAPIError(method, path, resp.StatusCode, string(respBody)) - } - - if result != nil && resp.StatusCode != http.StatusNoContent && len(respBody) > 0 { - if err := json.Unmarshal(respBody, result); err != nil { - return err - } - } - - return nil -} - -func min(a, b int) int { - if a < b { - return a - } - return b + base, _ := c.requestSnapshot() + return c.doBufferedRequestAt(ctx, base, method, path, "application/json", raw, result, c.jsonBodyLimit()) } // isNotFoundError checks if an error is a 404 Not Found response. @@ -427,7 +431,7 @@ func (c *Client) GetCurrentUser(ctx context.Context) (*User, error) { // GetUser returns a user by username. func (c *Client) GetUser(ctx context.Context, username string) (*User, error) { var user User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s", username), nil, &user) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s", username), nil, &user) if err != nil { return nil, err } diff --git a/commits_enhanced.go b/commits_enhanced.go index 5a3c550..ecf23c2 100644 --- a/commits_enhanced.go +++ b/commits_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -10,12 +9,12 @@ import ( // // GET /repos/{owner}/{repo}/commits/{sha}.diff func (c *Client) GetCommitDiff(ctx context.Context, owner, repo, sha string) ([]byte, error) { - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits/%s.diff", owner, repo, sha)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits/%s.diff", owner, repo, sha)) } // GetCommitPatch returns the patch of a commit. // // GET /repos/{owner}/{repo}/commits/{sha}.patch func (c *Client) GetCommitPatch(ctx context.Context, owner, repo, sha string) ([]byte, error) { - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits/%s.patch", owner, repo, sha)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits/%s.patch", owner, repo, sha)) } diff --git a/dashboard_kanban.go b/dashboard_kanban.go index ecb0fd2..1d7b657 100644 --- a/dashboard_kanban.go +++ b/dashboard_kanban.go @@ -64,7 +64,7 @@ func (c *Client) ListKanbanBoards(ctx context.Context, org string, opts ListKanb q.Set("search", opts.Search) } var list KanbanBoardList - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/org/%s/kanban/list?%s", org, q.Encode()), nil, &list) + err := c.doRequest(ctx, http.MethodGet, pathf("/org/%s/kanban/list?%s", org, q.Encode()), nil, &list) if err != nil { return nil, err } @@ -76,7 +76,7 @@ func (c *Client) ListKanbanBoards(ctx context.Context, org string, opts ListKanb // GET /v5/org/{org}/kanban/{kanban_id}/detail (live-verified). func (c *Client) GetKanbanBoard(ctx context.Context, org, kanbanID string) (*KanbanBoard, error) { var board KanbanBoard - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/org/%s/kanban/%s/detail", org, kanbanID), nil, &board) + err := c.doRequest(ctx, http.MethodGet, pathf("/org/%s/kanban/%s/detail", org, kanbanID), nil, &board) if err != nil { return nil, err } @@ -140,7 +140,7 @@ func (c *Client) ListKanbanBoardItems(ctx context.Context, org, kanbanID string, setIf("title", opts.Title) setIf("source_iids", opts.SourceIIDs) var items []*KanbanBoardItem - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/org/%s/kanban/%s/item_list?%s", org, kanbanID, q.Encode()), nil, &items) + err := c.doRequest(ctx, http.MethodGet, pathf("/org/%s/kanban/%s/item_list?%s", org, kanbanID, q.Encode()), nil, &items) if err != nil { return nil, err } @@ -160,7 +160,7 @@ type KanbanItemRef struct { // POST /v5/org/{org}/kanban/{kanban_id}/add_item (live-verified; the // response is an empty object per the docs). func (c *Client) AddToKanbanBoard(ctx context.Context, org, kanbanID string, items []KanbanItemRef) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/org/%s/kanban/%s/add_item", org, kanbanID), items, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/org/%s/kanban/%s/add_item", org, kanbanID), items, nil) } // UpdateKanbanBoardStatus sets a board open/closed (state: "open" or @@ -168,7 +168,7 @@ func (c *Client) AddToKanbanBoard(ctx context.Context, org, kanbanID string, ite // // PUT /v5/org/{org}/kanban/{kanban_id}/state (live-verified). func (c *Client) UpdateKanbanBoardStatus(ctx context.Context, org, kanbanID, state string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/org/%s/kanban/%s/state", org, kanbanID), map[string]string{"state": state}, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/org/%s/kanban/%s/state", org, kanbanID), map[string]string{"state": state}, nil) } // RemoveFromKanbanBoard removes repo issues/PRs from a board. @@ -176,5 +176,5 @@ func (c *Client) UpdateKanbanBoardStatus(ctx context.Context, org, kanbanID, sta // DELETE /v5/org/{org}/kanban/{kanban_id}/remove_item (live-verified; the // body is an array per the docs). func (c *Client) RemoveFromKanbanBoard(ctx context.Context, org, kanbanID string, items []KanbanItemRef) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/org/%s/kanban/%s/remove_item", org, kanbanID), items, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/org/%s/kanban/%s/remove_item", org, kanbanID), items, nil) } diff --git a/enterprise.go b/enterprise.go index bb0c092..1dcb8d3 100644 --- a/enterprise.go +++ b/enterprise.go @@ -19,7 +19,6 @@ package gitcode import ( "context" - "fmt" "net/http" "strings" ) @@ -37,7 +36,7 @@ type OrgEnterprise struct { // GET /v8/org/{org}/enterprise. func (c *Client) GetOrgEnterprise(ctx context.Context, org string) (*OrgEnterprise, error) { var ent OrgEnterprise - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/org/%s/enterprise", org), nil, &ent) + err := c.doV8Request(ctx, http.MethodGet, pathf("/org/%s/enterprise", org), nil, &ent) if err != nil { return nil, err } @@ -69,7 +68,7 @@ type EnterpriseCustomizedRole struct { // GET /v8/enterprise/{enterprise_id}/customized_roles. func (c *Client) ListEnterpriseCustomizedRoles(ctx context.Context, enterpriseID string) ([]*EnterpriseCustomizedRole, error) { var roles []*EnterpriseCustomizedRole - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprise/%s/customized_roles", enterpriseID), nil, &roles) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprise/%s/customized_roles", enterpriseID), nil, &roles) if err != nil { return nil, err } @@ -122,7 +121,7 @@ type UpdateEnterpriseMilestoneOptions struct { // GET /v8/enterprises/{enterprise_id}/milestones. func (c *Client) ListEnterpriseMilestones(ctx context.Context, enterpriseID string, opts ListOptions) ([]*EnterpriseMilestone, error) { var milestones []*EnterpriseMilestone - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/milestones?%s", enterpriseID, opts.toQuery()), nil, &milestones) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprises/%s/milestones?%s", enterpriseID, opts.toQuery()), nil, &milestones) if err != nil { return nil, err } @@ -134,7 +133,7 @@ func (c *Client) ListEnterpriseMilestones(ctx context.Context, enterpriseID stri // GET /v8/enterprises/{enterprise_id}/milestones/{milestone_id}. func (c *Client) GetEnterpriseMilestone(ctx context.Context, enterpriseID, milestoneID string) (*EnterpriseMilestone, error) { var milestone EnterpriseMilestone - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), nil, &milestone) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), nil, &milestone) if err != nil { return nil, err } @@ -146,7 +145,7 @@ func (c *Client) GetEnterpriseMilestone(ctx context.Context, enterpriseID, miles // POST /v8/enterprises/{enterprise_id}/milestones. func (c *Client) CreateEnterpriseMilestone(ctx context.Context, enterpriseID string, opts CreateEnterpriseMilestoneOptions) (*EnterpriseMilestone, error) { var milestone EnterpriseMilestone - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/enterprises/%s/milestones", enterpriseID), opts, &milestone) + err := c.doV8Request(ctx, http.MethodPost, pathf("/enterprises/%s/milestones", enterpriseID), opts, &milestone) if err != nil { return nil, err } @@ -159,7 +158,7 @@ func (c *Client) CreateEnterpriseMilestone(ctx context.Context, enterpriseID str // PUT /v8/enterprises/{enterprise_id}/milestones/{milestone_id}. func (c *Client) UpdateEnterpriseMilestone(ctx context.Context, enterpriseID, milestoneID string, opts UpdateEnterpriseMilestoneOptions) (*EnterpriseMilestone, error) { var milestone EnterpriseMilestone - err := c.doV8Request(ctx, http.MethodPut, fmt.Sprintf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), opts, &milestone) + err := c.doV8Request(ctx, http.MethodPut, pathf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), opts, &milestone) if err != nil { return nil, err } @@ -170,7 +169,7 @@ func (c *Client) UpdateEnterpriseMilestone(ctx context.Context, enterpriseID, mi // // DELETE /v8/enterprises/{enterprise_id}/milestones/{milestone_id}. func (c *Client) DeleteEnterpriseMilestone(ctx context.Context, enterpriseID, milestoneID string) error { - return c.doV8Request(ctx, http.MethodDelete, fmt.Sprintf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), nil, nil) + return c.doV8Request(ctx, http.MethodDelete, pathf("/enterprises/%s/milestones/%s", enterpriseID, milestoneID), nil, nil) } // --- Labels --- @@ -208,7 +207,7 @@ type UpdateEnterpriseLabelOptions struct { // POST /v8/enterprises/{enterprise_id}/label (singular on the wire). func (c *Client) CreateEnterpriseLabel(ctx context.Context, enterpriseID string, opts CreateEnterpriseLabelOptions) (*EnterpriseLabel, error) { var label EnterpriseLabel - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/enterprises/%s/label", enterpriseID), opts, &label) + err := c.doV8Request(ctx, http.MethodPost, pathf("/enterprises/%s/label", enterpriseID), opts, &label) if err != nil { return nil, err } @@ -220,7 +219,7 @@ func (c *Client) CreateEnterpriseLabel(ctx context.Context, enterpriseID string, // PUT /v8/enterprises/{enterprise_id}/label/{label_id}. func (c *Client) UpdateEnterpriseLabel(ctx context.Context, enterpriseID, labelID string, opts UpdateEnterpriseLabelOptions) (*EnterpriseLabel, error) { var label EnterpriseLabel - err := c.doV8Request(ctx, http.MethodPut, fmt.Sprintf("/enterprises/%s/label/%s", enterpriseID, labelID), opts, &label) + err := c.doV8Request(ctx, http.MethodPut, pathf("/enterprises/%s/label/%s", enterpriseID, labelID), opts, &label) if err != nil { return nil, err } @@ -231,7 +230,7 @@ func (c *Client) UpdateEnterpriseLabel(ctx context.Context, enterpriseID, labelI // // DELETE /v8/enterprises/{enterprise_id}/label/{label_id}. func (c *Client) DeleteEnterpriseLabel(ctx context.Context, enterpriseID, labelID string) error { - return c.doV8Request(ctx, http.MethodDelete, fmt.Sprintf("/enterprises/%s/label/%s", enterpriseID, labelID), nil, nil) + return c.doV8Request(ctx, http.MethodDelete, pathf("/enterprises/%s/label/%s", enterpriseID, labelID), nil, nil) } // --- Members --- @@ -272,7 +271,7 @@ type EnterpriseMemberOptions struct { // POST /v8/enterprises/{enterprise_id}/memberships/{username}. func (c *Client) InviteEnterpriseMember(ctx context.Context, enterpriseID, username string, opts EnterpriseMemberOptions) (*EnterpriseMember, error) { var member EnterpriseMember - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/enterprises/%s/memberships/%s", enterpriseID, username), opts, &member) + err := c.doV8Request(ctx, http.MethodPost, pathf("/enterprises/%s/memberships/%s", enterpriseID, username), opts, &member) if err != nil { return nil, err } @@ -285,7 +284,7 @@ func (c *Client) InviteEnterpriseMember(ctx context.Context, enterpriseID, usern // PUT /v8/enterprises/{enterprise_id}/members/{username}. func (c *Client) UpdateEnterpriseMemberPermission(ctx context.Context, enterpriseID, username string, opts EnterpriseMemberOptions) (*EnterpriseMember, error) { var member EnterpriseMember - err := c.doV8Request(ctx, http.MethodPut, fmt.Sprintf("/enterprises/%s/members/%s", enterpriseID, username), opts, &member) + err := c.doV8Request(ctx, http.MethodPut, pathf("/enterprises/%s/members/%s", enterpriseID, username), opts, &member) if err != nil { return nil, err } @@ -299,7 +298,7 @@ func (c *Client) UpdateEnterpriseMemberPermission(ctx context.Context, enterpris // GET /v8/enterprises/{enterprise_id}/members. func (c *Client) ListEnterpriseMembers(ctx context.Context, enterpriseID string, opts ListOptions) ([]*EnterpriseMember, error) { var members []*EnterpriseMember - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/members?%s", enterpriseID, opts.toQuery()), nil, &members) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprises/%s/members?%s", enterpriseID, opts.toQuery()), nil, &members) if err != nil { return nil, err } @@ -311,7 +310,7 @@ func (c *Client) ListEnterpriseMembers(ctx context.Context, enterpriseID string, // GET /v8/enterprises/{enterprise_id}/members/{username}. func (c *Client) GetEnterpriseMember(ctx context.Context, enterpriseID, username string) (*EnterpriseMember, error) { var member EnterpriseMember - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/members/%s", enterpriseID, username), nil, &member) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprises/%s/members/%s", enterpriseID, username), nil, &member) if err != nil { return nil, err } @@ -323,7 +322,7 @@ func (c *Client) GetEnterpriseMember(ctx context.Context, enterpriseID, username // // DELETE /v8/enterprises/{enterprise_id}/members/{usernames}. func (c *Client) DeleteEnterpriseMember(ctx context.Context, enterpriseID string, usernames ...string) error { - return c.doV8Request(ctx, http.MethodDelete, fmt.Sprintf("/enterprises/%s/members/%s", enterpriseID, strings.Join(usernames, ",")), nil, nil) + return c.doV8Request(ctx, http.MethodDelete, pathf("/enterprises/%s/members/%s", enterpriseID, strings.Join(usernames, ",")), nil, nil) } // --- Issue custom fields --- @@ -371,7 +370,7 @@ type EnterpriseCustomField struct { // GET /v8/enterprises/{enterprise_id}/issue_extend_field. func (c *Client) ListEnterpriseIssueCustomFields(ctx context.Context, enterpriseID string, opts ListOptions) ([]*EnterpriseCustomField, error) { var fields []*EnterpriseCustomField - err := c.doV8Request(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issue_extend_field?%s", enterpriseID, opts.toQuery()), nil, &fields) + err := c.doV8Request(ctx, http.MethodGet, pathf("/enterprises/%s/issue_extend_field?%s", enterpriseID, opts.toQuery()), nil, &fields) if err != nil { return nil, err } @@ -496,7 +495,7 @@ func (c *Client) ListEnterpriseIssues(ctx context.Context, enterpriseID string, opts.PerPage = 20 } var issues []*EnterpriseIssue - err := c.doV8Request(ctx, http.MethodPost, fmt.Sprintf("/enterprises/%s/issues", enterpriseID), opts, &issues) + err := c.doV8Request(ctx, http.MethodPost, pathf("/enterprises/%s/issues", enterpriseID), opts, &issues) if err != nil { return nil, err } diff --git a/enterprise_issues.go b/enterprise_issues.go index b7f9ebc..930355a 100644 --- a/enterprise_issues.go +++ b/enterprise_issues.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "net/url" "strconv" @@ -39,7 +38,7 @@ func userIssuesQuery(opts ListUserIssuesOptions) string { func (c *Client) ListUserIssues(ctx context.Context, opts ListUserIssuesOptions) ([]*Issue, error) { var issues []*Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/issues?%s", userIssuesQuery(opts)), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/issues?%s", userIssuesQuery(opts)), nil, &issues) if err != nil { return nil, err } @@ -48,7 +47,7 @@ func (c *Client) ListUserIssues(ctx context.Context, opts ListUserIssuesOptions) func (c *Client) ListOrgIssues(ctx context.Context, org string, opts ListUserIssuesOptions) ([]*Issue, error) { var issues []*Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/issues?%s", org, userIssuesQuery(opts)), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/issues?%s", org, userIssuesQuery(opts)), nil, &issues) if err != nil { return nil, err } @@ -57,7 +56,7 @@ func (c *Client) ListOrgIssues(ctx context.Context, org string, opts ListUserIss func (c *Client) ListRepoAllIssueComments(ctx context.Context, owner, repo string, opts ListOptions) ([]*IssueComment, error) { var comments []*IssueComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/comments?%s", owner, repo, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/comments?%s", owner, repo, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -66,7 +65,7 @@ func (c *Client) ListRepoAllIssueComments(ctx context.Context, owner, repo strin func (c *Client) GetIssueComment(ctx context.Context, owner, repo string, commentID int64) (*IssueComment, error) { var comment IssueComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), nil, &comment) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), nil, &comment) if err != nil { return nil, err } @@ -82,7 +81,7 @@ type IssueOperateLog struct { func (c *Client) GetIssueOperateLogs(ctx context.Context, owner, repo string, number int) ([]*IssueOperateLog, error) { var logs []*IssueOperateLog - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/operate_logs", owner, repo, number), nil, &logs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/operate_logs", owner, repo, number), nil, &logs) if err != nil { return nil, err } @@ -91,7 +90,7 @@ func (c *Client) GetIssueOperateLogs(ctx context.Context, owner, repo string, nu func (c *Client) GetIssueLinkedPRs(ctx context.Context, owner, repo string, number int) ([]*PullRequest, error) { var prs []*PullRequest - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/pull_requests", owner, repo, number), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/pull_requests", owner, repo, number), nil, &prs) if err != nil { return nil, err } diff --git a/enterprise_v5.go b/enterprise_v5.go index e5e7946..3f6878c 100644 --- a/enterprise_v5.go +++ b/enterprise_v5.go @@ -9,7 +9,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -30,7 +29,7 @@ type EnterpriseIssueStatus struct { // GET /enterprises/{enterprise}/issue-statuses (v5). func (c *Client) ListEnterpriseIssueStatuses(ctx context.Context, enterprise string) ([]*EnterpriseIssueStatus, error) { var statuses []*EnterpriseIssueStatus - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issue-statuses", enterprise), nil, &statuses) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/issue-statuses", enterprise), nil, &statuses) if err != nil { return nil, err } @@ -44,7 +43,7 @@ func (c *Client) ListEnterpriseIssueStatuses(ctx context.Context, enterprise str // get-api-v-5-enterprises-enterprise-issues-number). func (c *Client) GetEnterpriseIssue(ctx context.Context, enterprise string, number int) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issues/%d", enterprise, number), nil, &issue) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/issues/%d", enterprise, number), nil, &issue) if err != nil { return nil, err } @@ -57,7 +56,7 @@ func (c *Client) GetEnterpriseIssue(ctx context.Context, enterprise string, numb // get-api-v-5-enterprises-enterprise-issues-number-comments). func (c *Client) ListEnterpriseIssueComments(ctx context.Context, enterprise string, number int, opts ListOptions) ([]*IssueComment, error) { var comments []*IssueComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issues/%d/comments?%s", enterprise, number, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/issues/%d/comments?%s", enterprise, number, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -71,7 +70,7 @@ func (c *Client) ListEnterpriseIssueComments(ctx context.Context, enterprise str // get-api-v-5-enterprises-enterprise-issues-issue-id-labels). func (c *Client) ListEnterpriseIssueLabels(ctx context.Context, enterprise string, issueID int64) ([]*Label, error) { var labels []*Label - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issues/%d/labels", enterprise, issueID), nil, &labels) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/issues/%d/labels", enterprise, issueID), nil, &labels) if err != nil { return nil, err } diff --git a/git_refs.go b/git_refs.go index 0367486..663b6c1 100644 --- a/git_refs.go +++ b/git_refs.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -37,7 +36,7 @@ type UpdateReferenceOptions struct { // GET /repos/{owner}/{repo}/git/refs func (c *Client) ListGitReferences(ctx context.Context, owner, repo string, opts ListOptions) ([]*GitReference, error) { var refs []*GitReference - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/git/refs?%s", owner, repo, opts.toQuery()), nil, &refs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/git/refs?%s", owner, repo, opts.toQuery()), nil, &refs) if err != nil { return nil, err } @@ -49,7 +48,7 @@ func (c *Client) ListGitReferences(ctx context.Context, owner, repo string, opts // GET /repos/{owner}/{repo}/git/refs/{ref} func (c *Client) GetGitReference(ctx context.Context, owner, repo, ref string) (*GitReference, error) { var gitRef GitReference - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/git/refs/%s", owner, repo, ref), nil, &gitRef) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/git/refs/%s", owner, repo, ref), nil, &gitRef) if err != nil { return nil, err } @@ -61,7 +60,7 @@ func (c *Client) GetGitReference(ctx context.Context, owner, repo, ref string) ( // POST /repos/{owner}/{repo}/git/refs func (c *Client) CreateGitReference(ctx context.Context, owner, repo string, opts CreateReferenceOptions) (*GitReference, error) { var gitRef GitReference - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/git/refs", owner, repo), opts, &gitRef) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/git/refs", owner, repo), opts, &gitRef) if err != nil { return nil, err } @@ -73,7 +72,7 @@ func (c *Client) CreateGitReference(ctx context.Context, owner, repo string, opt // PATCH /repos/{owner}/{repo}/git/refs/{ref} func (c *Client) UpdateGitReference(ctx context.Context, owner, repo, ref string, opts UpdateReferenceOptions) (*GitReference, error) { var gitRef GitReference - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/git/refs/%s", owner, repo, ref), opts, &gitRef) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/git/refs/%s", owner, repo, ref), opts, &gitRef) if err != nil { return nil, err } @@ -84,7 +83,7 @@ func (c *Client) UpdateGitReference(ctx context.Context, owner, repo, ref string // // DELETE /repos/{owner}/{repo}/git/refs/{ref} func (c *Client) DeleteGitReference(ctx context.Context, owner, repo, ref string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/git/refs/%s", owner, repo, ref), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/git/refs/%s", owner, repo, ref), nil, nil) } // ListGitRefSubPaths lists references filtered by prefix (e.g. "heads/", "tags/"). @@ -92,7 +91,7 @@ func (c *Client) DeleteGitReference(ctx context.Context, owner, repo, ref string // GET /repos/{owner}/{repo}/git/refs/{refPrefix} func (c *Client) ListGitRefSubPaths(ctx context.Context, owner, repo, refPrefix string) ([]*GitReference, error) { var refs []*GitReference - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/git/refs/%s", owner, repo, refPrefix), nil, &refs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/git/refs/%s", owner, repo, refPrefix), nil, &refs) if err != nil { return nil, err } diff --git a/git_tags.go b/git_tags.go index 6dad782..9a48289 100644 --- a/git_tags.go +++ b/git_tags.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -35,7 +34,7 @@ type CreateAnnotatedTagOptions struct { // GET /repos/{owner}/{repo}/git/tags/{sha} func (c *Client) GetAnnotatedTag(ctx context.Context, owner, repo, sha string) (*AnnotatedTag, error) { var tag AnnotatedTag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/git/tags/%s", owner, repo, sha), nil, &tag) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/git/tags/%s", owner, repo, sha), nil, &tag) if err != nil { return nil, err } @@ -47,7 +46,7 @@ func (c *Client) GetAnnotatedTag(ctx context.Context, owner, repo, sha string) ( // POST /repos/{owner}/{repo}/git/tags func (c *Client) CreateAnnotatedTag(ctx context.Context, owner, repo string, opts CreateAnnotatedTagOptions) (*AnnotatedTag, error) { var tag AnnotatedTag - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/git/tags", owner, repo), opts, &tag) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/git/tags", owner, repo), opts, &tag) if err != nil { return nil, err } @@ -72,7 +71,7 @@ type TagWithRelease struct { // GET /repos/{owner}/{repo}/tags func (c *Client) ListTagsWithOptions(ctx context.Context, owner, repo string, opts ListOptions) ([]*Tag, error) { var tags []*Tag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/tags?%s", owner, repo, opts.toQuery()), nil, &tags) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/tags?%s", owner, repo, opts.toQuery()), nil, &tags) if err != nil { return nil, err } @@ -84,7 +83,7 @@ func (c *Client) ListTagsWithOptions(ctx context.Context, owner, repo string, op // GET /repos/{owner}/{repo}/tags/{tag} func (c *Client) GetTag(ctx context.Context, owner, repo, tagName string) (*Tag, error) { var tag Tag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/tags/%s", owner, repo, tagName), nil, &tag) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/tags/%s", owner, repo, tagName), nil, &tag) if err != nil { return nil, err } @@ -96,7 +95,7 @@ func (c *Client) GetTag(ctx context.Context, owner, repo, tagName string) (*Tag, // POST /repos/{owner}/{repo}/tags func (c *Client) CreateTag(ctx context.Context, owner, repo string, opts CreateTagOptions) (*Tag, error) { var tag Tag - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/tags", owner, repo), opts, &tag) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/tags", owner, repo), opts, &tag) if err != nil { return nil, err } @@ -115,7 +114,7 @@ type CreateTagOptions struct { // GET /repos/{owner}/{repo}/releases/tags/{tag} func (c *Client) GetReleaseByTag(ctx context.Context, owner, repo, tag string) (*Release, error) { var release Release - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/tags/%s", owner, repo, tag), nil, &release) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/tags/%s", owner, repo, tag), nil, &release) if err != nil { return nil, err } @@ -127,7 +126,7 @@ func (c *Client) GetReleaseByTag(ctx context.Context, owner, repo, tag string) ( // PATCH /repos/{owner}/{repo}/releases/{id} func (c *Client) UpdateRelease(ctx context.Context, owner, repo string, releaseID int64, opts UpdateReleaseOptions) (*Release, error) { var release Release - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/releases/%d", owner, repo, releaseID), opts, &release) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/releases/%d", owner, repo, releaseID), opts, &release) if err != nil { return nil, err } @@ -149,7 +148,7 @@ type UpdateReleaseOptions struct { // GET /repos/{owner}/{repo}/releases/{id} func (c *Client) GetRelease(ctx context.Context, owner, repo string, releaseID int64) (*Release, error) { var release Release - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/%d", owner, repo, releaseID), nil, &release) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/%d", owner, repo, releaseID), nil, &release) if err != nil { return nil, err } @@ -160,7 +159,7 @@ func (c *Client) GetRelease(ctx context.Context, owner, repo string, releaseID i // // DELETE /repos/{owner}/{repo}/releases/{id} func (c *Client) DeleteReleaseByID(ctx context.Context, owner, repo string, releaseID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/releases/%d", owner, repo, releaseID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/releases/%d", owner, repo, releaseID), nil, nil) } // ReleaseAsset represents a release asset. @@ -180,7 +179,7 @@ type ReleaseAsset struct { // GET /repos/{owner}/{repo}/releases/{id}/assets func (c *Client) ListReleaseAssets(ctx context.Context, owner, repo string, releaseID int64, opts ListOptions) ([]*ReleaseAsset, error) { var assets []*ReleaseAsset - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/%d/assets?%s", owner, repo, releaseID, opts.toQuery()), nil, &assets) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/%d/assets?%s", owner, repo, releaseID, opts.toQuery()), nil, &assets) if err != nil { return nil, err } @@ -192,7 +191,7 @@ func (c *Client) ListReleaseAssets(ctx context.Context, owner, repo string, rele // GET /repos/{owner}/{repo}/releases/assets/{id} func (c *Client) GetReleaseAsset(ctx context.Context, owner, repo string, assetID int64) (*ReleaseAsset, error) { var asset ReleaseAsset - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID), nil, &asset) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID), nil, &asset) if err != nil { return nil, err } @@ -203,5 +202,5 @@ func (c *Client) GetReleaseAsset(ctx context.Context, owner, repo string, assetI // // DELETE /repos/{owner}/{repo}/releases/assets/{id} func (c *Client) DeleteReleaseAsset(ctx context.Context, owner, repo string, assetID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID), nil, nil) } diff --git a/go.mod b/go.mod index 9d5b9a2..9dd21cd 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/yi-nology/go-gitcode -go 1.26 +go 1.27 diff --git a/hardening_test.go b/hardening_test.go new file mode 100644 index 0000000..89fa962 --- /dev/null +++ b/hardening_test.go @@ -0,0 +1,674 @@ +package gitcode + +// Hardening tests for issues #4-#10: Retry-After gates, paginate page cap, +// path-segment escaping, response body limits, client field read locking, +// upload pipeline reuse, and ListRepositories filter params. + +import ( + "context" + "errors" + "fmt" + "io" + "math" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" +) + +// --- issue #4: Retry-After three gates --- + +func TestRetryAfterDuration(t *testing.T) { + now := time.Date(2026, 11, 11, 12, 0, 0, 0, time.UTC) + future := now.Add(time.Hour) + past := now.Add(-time.Hour) + + tests := []struct { + name string + header string + want time.Duration + ok bool + }{ + {"delay-seconds", "5", 5 * time.Second, true}, + {"zero seconds rejected", "0", 0, false}, + {"negative seconds rejected", "-5", 0, false}, + {"not a number", "soon", 0, false}, + {"empty-ish garbage", " ", 0, false}, + {"http-date future", future.Format(http.TimeFormat), time.Hour, true}, + {"http-date past rejected", past.Format(http.TimeFormat), 0, false}, + {"http-date exactly now rejected", now.Format(http.TimeFormat), 0, false}, + {"overflowing seconds clamped", "9223372036854", time.Duration(math.MaxInt64), true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp := &http.Response{Header: http.Header{}} + if tt.header != "" { + resp.Header.Set("Retry-After", tt.header) + } + got, ok := retryAfterDuration(resp, now) + if ok != tt.ok { + t.Fatalf("ok = %v, want %v", ok, tt.ok) + } + if ok && got != tt.want { + t.Errorf("duration = %v, want %v", got, tt.want) + } + }) + } + + if _, ok := retryAfterDuration(nil, now); ok { + t.Error("nil response must not parse") + } + resp := &http.Response{Header: http.Header{}} + if _, ok := retryAfterDuration(resp, now); ok { + t.Error("missing header must not parse") + } +} + +func TestGetRetryWaitTimeRetryAfter(t *testing.T) { + clampedPolicy := RetryPolicy{ + MaxRetries: 3, + InitialBackoff: 1 * time.Second, + MaxBackoff: 30 * time.Second, + Multiplier: 2.0, + } + future := time.Now().Add(2 * time.Hour).UTC() + + tests := []struct { + name string + policy RetryPolicy + header string + attempt int + want time.Duration + }{ + {"retry-after within bounds", clampedPolicy, "5", 0, 5 * time.Second}, + {"retry-after clamped to MaxBackoff", clampedPolicy, "3600", 0, 30 * time.Second}, + {"negative falls back to exponential", clampedPolicy, "-5", 0, 1 * time.Second}, + {"zero falls back to exponential", clampedPolicy, "0", 0, 1 * time.Second}, + {"garbage falls back to exponential", clampedPolicy, "soon", 0, 1 * time.Second}, + {"http-date clamped to MaxBackoff", clampedPolicy, future.Format(http.TimeFormat), 0, 30 * time.Second}, + {"unset MaxBackoff uses documented default", RetryPolicy{MaxRetries: 3, InitialBackoff: 1 * time.Second, Multiplier: 2}, "3600", 0, 30 * time.Second}, + {"exponential attempt 0", clampedPolicy, "", 0, 1 * time.Second}, + {"exponential attempt 3", clampedPolicy, "", 3, 8 * time.Second}, + {"exponential clamped at attempt 10", clampedPolicy, "", 10, 30 * time.Second}, + {"zero InitialBackoff normalized", RetryPolicy{MaxRetries: 3, MaxBackoff: 30 * time.Second, Multiplier: 2}, "", 0, 1 * time.Second}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewClient("tok") + c.SetRetryPolicy(tt.policy) + resp := &http.Response{Header: http.Header{}} + if tt.header != "" { + resp.Header.Set("Retry-After", tt.header) + } + got := c.getRetryWaitTime(tt.attempt, resp) + if got != tt.want { + t.Errorf("getRetryWaitTime() = %v, want %v", got, tt.want) + } + }) + } + + c := NewClient("tok") // no policy + if got := c.getRetryWaitTime(0, nil); got != 0 { + t.Errorf("no policy: got %v, want 0", got) + } +} + +// TestRetryAfterHeaderHonoredOn429 wires a real 429-then-200 exchange and +// asserts the SDK waits for the (small) Retry-After instead of busy-looping. +func TestRetryAfterHeaderHonoredOn429(t *testing.T) { + var attempts int + var waits []time.Duration + start := time.Now() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts++ + if attempts == 1 { + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + fmt.Fprint(w, `{"message":"rate limited"}`) + return + } + waits = append(waits, time.Since(start)) + fmt.Fprint(w, `{"login":"u"}`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + c.SetRetryPolicy(RetryPolicy{MaxRetries: 1, RetryableStatusCodes: []int{429}}) + if _, err := c.GetCurrentUser(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if attempts != 2 { + t.Fatalf("attempts = %d, want 2", attempts) + } + if len(waits) != 1 || waits[0] < 900*time.Millisecond { + t.Errorf("second attempt came too early: %v (want >= ~1s Retry-After)", waits) + } +} + +// --- issue #5: paginate page cap --- + +func TestPaginatePageCap(t *testing.T) { + calls := 0 + fetch := func(opts ListOptions) ([]*Issue, error) { + calls++ + // Server pathology: always a full page, cursor never advances. + out := make([]*Issue, opts.PerPage) + for i := range out { + out[i] = &Issue{} + } + return out, nil + } + + var pages int + var lastErr error + for items, err := range Paginate(context.Background(), fetch) { + if err != nil { + lastErr = err + break + } + pages++ + _ = items + } + if pages != maxPaginatePages { + t.Errorf("pages yielded = %d, want %d", pages, maxPaginatePages) + } + if calls != maxPaginatePages { + t.Errorf("fetch calls = %d, want %d", calls, maxPaginatePages) + } + if !errors.Is(lastErr, ErrTooManyPages) { + t.Errorf("terminal error = %v, want ErrTooManyPages", lastErr) + } +} + +func TestPaginateEndsNormally(t *testing.T) { + fetch := func(opts ListOptions) ([]string, error) { + if opts.Page > 1 { + return nil, nil + } + return []string{"a", "b"}, nil // short page: end of list + } + var got []string + var errs int + for items, err := range Paginate(context.Background(), fetch) { + if err != nil { + errs++ + continue + } + got = append(got, items...) + } + if errs != 0 { + t.Errorf("errors = %d, want 0", errs) + } + if strings.Join(got, ",") != "a,b" { + t.Errorf("items = %v, want [a b]", got) + } +} + +// --- issue #6: path segment escaping --- + +func TestPathfEscaping(t *testing.T) { + tests := []struct { + name string + got string + want string + }{ + {"plain identifiers unchanged", pathf("/repos/%s/%s", "owner", "repo"), "/repos/owner/repo"}, + {"slash confined to segment", pathf("/repos/%s/%s", "evil/../admin", "repo"), "/repos/evil%2F..%2Fadmin/repo"}, + {"question mark escaped", pathf("/orgs/%s/hooks", "org?x=1"), "/orgs/org%3Fx=1/hooks"}, + {"fragment escaped", pathf("/repos/%s", "o#w"), "/repos/o%23w"}, + {"space escaped", pathf("/repos/%s/%s", "a b", "r"), "/repos/a%20b/r"}, + {"int verb untouched", pathf("/repos/%s/pulls/%d", "o", 42), "/repos/o/pulls/42"}, + {"query verbs untouched", pathf("/repos/%s?%s", "o", "page=1&x=%2F"), "/repos/o?page=1&x=%2F"}, + {"multi-segment file path", pathf("/repos/%s/contents/%s", "o", pathSegs("docs/READ ME.md")), "/repos/o/contents/docs/READ%20ME.md"}, + {"pathSegs on plain segment", string(pathSegs("plain")), "plain"}, + {"literal percent", pathf("/a%%b/%s", "c"), "/a%b/c"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.got != tt.want { + t.Errorf("got %q, want %q", tt.got, tt.want) + } + }) + } +} + +func TestPathSegEquivalence(t *testing.T) { + if pathSeg("x") != string(pathSegs("x")) { + t.Error("single-segment inputs should agree between pathSeg and pathSegs") + } +} + +// TestEndpointPathIsolation asserts the wire form: a caller-controlled +// identifier stays inside its own path segment instead of adding levels or +// spilling into the query string. +func TestEndpointPathIsolation(t *testing.T) { + tests := []struct { + name string + owner string + wantRequestURI string + }{ + {"plain", "good-org", "/api/v5/repos/good-org/repo"}, + {"dotdot slash", "evil/../admin", "/api/v5/repos/evil%2F..%2Fadmin/repo"}, + {"query injection", "org?x=1", "/api/v5/repos/org%3Fx=1/repo"}, + {"fragment", "o#w", "/api/v5/repos/o%23w/repo"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotURI string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotURI = r.RequestURI + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{}`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + _, _ = c.GetRepository(context.Background(), tt.owner, "repo") + if gotURI != tt.wantRequestURI { + t.Errorf("request URI = %q, want %q", gotURI, tt.wantRequestURI) + } + }) + } +} + +func TestRawFilePathSegments(t *testing.T) { + var gotURI string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotURI = r.RequestURI + fmt.Fprint(w, "raw") + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + if _, err := c.GetRawFile(context.Background(), "o", "r", "docs/READ ME.md", "main"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := "/api/v5/repos/o/r/raw/docs/READ%20ME.md?ref=main" + if gotURI != want { + t.Errorf("request URI = %q, want %q", gotURI, want) + } +} + +// --- issue #7: response body limits --- + +func TestResponseBodyLimit(t *testing.T) { + bigBody := strings.Repeat("x", 1<<20) // 1 MiB, far over the small limits below + + tests := []struct { + name string + limit int64 + call func(*Client) error + wantBig bool + }{ + { + name: "json endpoint over custom limit", + limit: 1024, + call: func(c *Client) error { + _, err := c.GetRepository(context.Background(), "o", "r") + return err + }, + wantBig: true, + }, + { + name: "raw download over custom limit", + limit: 1024, + call: func(c *Client) error { + _, err := c.GetRawFile(context.Background(), "o", "r", "f", "main") + return err + }, + wantBig: true, + }, + { + name: "upload response over custom limit", + limit: 8, + call: func(c *Client) error { + _, err := c.UploadFileBytes(context.Background(), "o", "r", "f.txt", []byte("data")) + return err + }, + wantBig: true, + }, + { + name: "raw download under limit passes", + limit: int64(len(bigBody)) + 10, + call: func(c *Client) error { + b, err := c.GetRawFile(context.Background(), "o", "r", "f", "") + if err == nil && len(b) != len(bigBody) { + return fmt.Errorf("short read: %d", len(b)) + } + return err + }, + wantBig: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/octet-stream") + fmt.Fprint(w, bigBody) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + c.SetMaxResponseBodySize(tt.limit) + err := tt.call(c) + if tt.wantBig { + if !IsResponseBodyTooLarge(err) { + t.Fatalf("error = %v, want ResponseBodyTooLargeError", err) + } + } else if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + +func TestMaxResponseBodySizeSetter(t *testing.T) { + c := NewClient("tok") + if got := c.MaxResponseBodySize(); got != 0 { + t.Errorf("default limit = %d, want 0 (tiered defaults)", got) + } + c.SetMaxResponseBodySize(4096) + if got := c.MaxResponseBodySize(); got != 4096 { + t.Errorf("limit = %d, want 4096", got) + } + if got := c.jsonBodyLimit(); got != 4096 { + t.Errorf("jsonBodyLimit = %d, want 4096", got) + } + if got := c.rawBodyLimit(); got != 4096 { + t.Errorf("rawBodyLimit = %d, want 4096", got) + } + c.SetMaxResponseBodySize(0) + if got := c.jsonBodyLimit(); got != defaultMaxJSONBodyBytes { + t.Errorf("jsonBodyLimit after reset = %d, want %d", got, defaultMaxJSONBodyBytes) + } + if got := c.rawBodyLimit(); got != defaultMaxRawBodyBytes { + t.Errorf("rawBodyLimit after reset = %d, want %d", got, defaultMaxRawBodyBytes) + } +} + +func TestReadBodyLimited(t *testing.T) { + tests := []struct { + name string + input string + limit int64 + wantErr bool + }{ + {"under limit", "abc", 10, false}, + {"exactly at limit", "abcdefghij", 10, false}, + {"over limit", "abcdefghijk", 10, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + b, err := readBodyLimited(strings.NewReader(tt.input), tt.limit) + if tt.wantErr { + if !IsResponseBodyTooLarge(err) { + t.Fatalf("error = %v, want ResponseBodyTooLargeError", err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(b) != tt.input { + t.Errorf("body = %q, want %q", b, tt.input) + } + }) + } +} + +// --- issue #8: concurrent setters vs requests --- + +// TestConcurrentSettersAndRequests must be run with -race: before the +// requestSnapshot fix, SetHTTPClient/SetBaseURL (locked writes) raced the +// bare reads of c.baseURL / c.httpClient on the request path. +func TestConcurrentSettersAndRequests(t *testing.T) { + srv := httptest.NewServer(jsonHandler(http.StatusOK, map[string]string{"login": "u"})) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + + const rounds = 100 + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for i := range rounds { + c.SetHTTPClient(&http.Client{Timeout: 10 * time.Second}) + c.SetBaseURL(srv.URL + "/api/v5") + _ = i + } + }() + go func() { + defer wg.Done() + for range rounds { + if _, err := c.GetCurrentUser(context.Background()); err != nil { + t.Errorf("GetCurrentUser: %v", err) + return + } + } + }() + wg.Wait() +} + +// --- issue #9: uploads through the shared pipeline --- + +func TestUploadRunsHooksAndRetries(t *testing.T) { + var attempts, reqHooks, respHooks int + var lastFileContent string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasPrefix(r.URL.Path, "/api/v5/repos/o/r/file/upload") { + w.WriteHeader(http.StatusNotFound) + return + } + attempts++ + if attempts < 3 { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprint(w, `{"error_msg":"boom"}`) + return + } + if err := r.ParseMultipartForm(1 << 20); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + file, _, err := r.FormFile("file") + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + defer file.Close() + raw, err := io.ReadAll(file) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + lastFileContent = string(raw) + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"file_path":"/uploads/a.txt"}`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + c.SetRetryPolicy(RetryPolicy{ + MaxRetries: 2, + InitialBackoff: time.Millisecond, + MaxBackoff: 2 * time.Millisecond, + RetryableStatusCodes: []int{http.StatusInternalServerError}, + }) + c.AddRequestHook(func(*http.Request) error { reqHooks++; return nil }) + c.AddResponseHook(func(*http.Response) error { respHooks++; return nil }) + + res, err := c.UploadFileBytes(context.Background(), "o", "r", "a.txt", []byte("hello upload")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if attempts != 3 { + t.Errorf("server attempts = %d, want 3 (1 + 2 retries with replayed body)", attempts) + } + if lastFileContent != "hello upload" { + t.Errorf("uploaded content on final attempt = %q, want %q", lastFileContent, "hello upload") + } + if reqHooks != 1 { + t.Errorf("request hook calls = %d, want 1", reqHooks) + } + if respHooks != 1 { + t.Errorf("response hook calls = %d, want 1", respHooks) + } + if res.FilePath != "/uploads/a.txt" { + t.Errorf("FilePath = %q, want /uploads/a.txt", res.FilePath) + } +} + +func TestUploadImageRunsHooks(t *testing.T) { + var reqHooks int + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reqHooks++ + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"file_path":"/img/x.png"}`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + c.AddRequestHook(func(*http.Request) error { return nil }) + hooked := 0 + c.AddRequestHook(func(*http.Request) error { hooked++; return nil }) + + if _, err := c.UploadImageReader(context.Background(), "o", "r", "x.png", strings.NewReader("png")); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if hooked != 1 { + t.Errorf("request hook calls = %d, want 1 (image upload used to bypass hooks entirely)", hooked) + } +} + +// TestJSONBodyReplayOnRetry guards the retry body reset for regular JSON +// POSTs: the retried request must carry the same body as the first. +func TestJSONBodyReplayOnRetry(t *testing.T) { + var bodies []string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + b := make([]byte, r.ContentLength) + _, _ = r.Body.Read(b) + bodies = append(bodies, string(b)) + if len(bodies) < 3 { + w.WriteHeader(http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"number":1}`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + c.SetRetryPolicy(RetryPolicy{ + MaxRetries: 2, + InitialBackoff: time.Millisecond, + MaxBackoff: 2 * time.Millisecond, + RetryableStatusCodes: []int{http.StatusInternalServerError}, + }) + if _, err := c.CreateIssue(context.Background(), "o", "r", CreateIssueOptions{Title: "t"}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(bodies) != 3 { + t.Fatalf("attempts = %d, want 3", len(bodies)) + } + for i, b := range bodies { + if b == "" || b == "null" { + t.Errorf("attempt %d sent an empty body: %q", i+1, b) + } + if b != bodies[0] { + t.Errorf("attempt %d body %q differs from attempt 1 %q", i+1, b, bodies[0]) + } + } +} + +// --- issue #10: ListRepositories Type/Sort --- + +func TestListRepositoriesFiltersOnWire(t *testing.T) { + tests := []struct { + name string + call func(*Client) error + // wantPath is the endpoint path prefix the request must hit. + wantPath string + }{ + { + name: "ListRepositories authenticated user", + call: func(c *Client) error { + _, err := c.ListRepositories(context.Background(), ListRepositoriesOptions{Type: "owner", Sort: "created"}) + return err + }, + wantPath: "/api/v5/user/repos", + }, + { + name: "ListRepositories owner branch", + call: func(c *Client) error { + _, err := c.ListRepositories(context.Background(), ListRepositoriesOptions{Owner: "someuser", Type: "owner", Sort: "created"}) + return err + }, + wantPath: "/api/v5/users/someuser/repos", + }, + { + name: "ListCurrentUserRepositories", + call: func(c *Client) error { + _, err := c.ListCurrentUserRepositories(context.Background(), ListRepositoriesOptions{Type: "owner", Sort: "created"}) + return err + }, + wantPath: "/api/v5/user/repos", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var gotPath string + var gotQuery url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `[]`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + if err := tt.call(c); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotPath != tt.wantPath { + t.Errorf("path = %q, want %q", gotPath, tt.wantPath) + } + if got := gotQuery.Get("type"); got != "owner" { + t.Errorf("type filter = %q, want %q (silently dropped before issue #10 fix)", got, "owner") + } + if got := gotQuery.Get("sort"); got != "created" { + t.Errorf("sort filter = %q, want %q", got, "created") + } + if got := gotQuery.Get("page"); got != "1" { + t.Errorf("page = %q, want 1", got) + } + if got := gotQuery.Get("per_page"); got != "20" { + t.Errorf("per_page = %q, want 20", got) + } + }) + } +} + +func TestListRepositoriesOptionsNoFilters(t *testing.T) { + var gotQuery url.Values + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotQuery = r.URL.Query() + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `[]`) + })) + defer srv.Close() + + c := NewClientWithBaseURL(srv.URL+"/api/v5", "tok") + if _, err := c.ListRepositories(context.Background(), ListRepositoriesOptions{}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := gotQuery["type"]; ok { + t.Errorf("unexpected type filter: %v", gotQuery["type"]) + } + if _, ok := gotQuery["sort"]; ok { + t.Errorf("unexpected sort filter: %v", gotQuery["sort"]) + } +} diff --git a/issues.go b/issues.go index af28bd6..6fa265b 100644 --- a/issues.go +++ b/issues.go @@ -151,7 +151,7 @@ func (c *Client) ListIssues(ctx context.Context, owner, repo string, opts ListIs if opts.Since != "" { q.Set("since", opts.Since) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues?%s", owner, repo, q.Encode()), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues?%s", owner, repo, q.Encode()), nil, &issues) if err != nil { return nil, err } @@ -160,7 +160,7 @@ func (c *Client) ListIssues(ctx context.Context, owner, repo string, opts ListIs func (c *Client) GetIssue(ctx context.Context, owner, repo string, number int) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d", owner, repo, number), nil, &issue) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d", owner, repo, number), nil, &issue) if err != nil { return nil, err } @@ -169,7 +169,7 @@ func (c *Client) GetIssue(ctx context.Context, owner, repo string, number int) ( func (c *Client) CreateIssue(ctx context.Context, owner, repo string, opts CreateIssueOptions) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues", owner, repo), opts, &issue) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues", owner, repo), opts, &issue) if err != nil { return nil, err } @@ -178,7 +178,7 @@ func (c *Client) CreateIssue(ctx context.Context, owner, repo string, opts Creat func (c *Client) UpdateIssue(ctx context.Context, owner, repo string, number int, opts UpdateIssueOptions) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/issues/%d", owner, repo, number), opts, &issue) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/issues/%d", owner, repo, number), opts, &issue) if err != nil { return nil, err } @@ -234,7 +234,7 @@ func (c *Client) ListIssueCommentsWithOptions(ctx context.Context, owner, repo s if opts.Since != "" { q.Set("since", opts.Since) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/comments?%s", owner, repo, number, q.Encode()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/comments?%s", owner, repo, number, q.Encode()), nil, &comments) if err != nil { return nil, err } @@ -243,7 +243,7 @@ func (c *Client) ListIssueCommentsWithOptions(ctx context.Context, owner, repo s func (c *Client) CreateIssueComment(ctx context.Context, owner, repo string, number int, body string) (*IssueComment, error) { var comment IssueComment - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/%d/comments", owner, repo, number), map[string]string{"body": body}, &comment) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/%d/comments", owner, repo, number), map[string]string{"body": body}, &comment) if err != nil { return nil, err } @@ -252,7 +252,7 @@ func (c *Client) CreateIssueComment(ctx context.Context, owner, repo string, num func (c *Client) UpdateIssueComment(ctx context.Context, owner, repo string, commentID int64, body string) (*IssueComment, error) { var comment IssueComment - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), map[string]string{"body": body}, &comment) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), map[string]string{"body": body}, &comment) if err != nil { return nil, err } @@ -260,7 +260,7 @@ func (c *Client) UpdateIssueComment(ctx context.Context, owner, repo string, com } func (c *Client) DeleteIssueComment(ctx context.Context, owner, repo string, commentID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/comments/%d", owner, repo, commentID), nil, nil) } // ListIssueLabels lists a repository's labels (server-default first page). @@ -273,7 +273,7 @@ func (c *Client) ListIssueLabels(ctx context.Context, owner, repo string) ([]*La // pagination (GET /repos/{owner}/{repo}/labels). func (c *Client) ListIssueLabelsWithOptions(ctx context.Context, owner, repo string, opts ListOptions) ([]*Label, error) { var labels []*Label - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/labels?%s", owner, repo, opts.toQuery()), nil, &labels) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/labels?%s", owner, repo, opts.toQuery()), nil, &labels) if err != nil { return nil, err } @@ -283,7 +283,7 @@ func (c *Client) ListIssueLabelsWithOptions(ctx context.Context, owner, repo str func (c *Client) CreateIssueLabel(ctx context.Context, owner, repo string, name, color string) (*Label, error) { var label Label params := url.Values{"name": {name}, "color": {"#" + strings.TrimPrefix(color, "#")}} - err := c.doFormRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/labels", owner, repo), params, &label) + err := c.doFormRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/labels", owner, repo), params, &label) if err != nil { return nil, err } @@ -291,15 +291,15 @@ func (c *Client) CreateIssueLabel(ctx context.Context, owner, repo string, name, } func (c *Client) DeleteIssueLabel(ctx context.Context, owner, repo string, name string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/labels/%s", owner, repo, name), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/labels/%s", owner, repo, name), nil, nil) } func (c *Client) AddIssueLabels(ctx context.Context, owner, repo string, number int, labels []string) error { - return c.doRawBodyRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/%d/labels", owner, repo, number), labels, nil) + return c.doRawBodyRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/%d/labels", owner, repo, number), labels, nil) } func (c *Client) RemoveIssueLabel(ctx context.Context, owner, repo string, number int, name string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/labels/%s", owner, repo, number, name), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/labels/%s", owner, repo, number, name), nil, nil) } func (c *Client) ListMilestones(ctx context.Context, owner, repo string) ([]*Milestone, error) { @@ -311,5 +311,5 @@ func (c *Client) CreateMilestone(ctx context.Context, owner, repo string, title, } func (c *Client) DeleteMilestone(ctx context.Context, owner, repo string, number int) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/milestones/%d", owner, repo, number), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/milestones/%d", owner, repo, number), nil, nil) } diff --git a/issues_enhanced.go b/issues_enhanced.go index 770ad6d..1fa8a25 100644 --- a/issues_enhanced.go +++ b/issues_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -20,7 +19,7 @@ type IssueRelatedBranch struct { // GET /repos/{owner}/{repo}/issues/{number}/related-branches func (c *Client) ListIssueRelatedBranches(ctx context.Context, owner, repo string, number int) ([]*IssueRelatedBranch, error) { var branches []*IssueRelatedBranch - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/related-branches", owner, repo, number), nil, &branches) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/related-branches", owner, repo, number), nil, &branches) if err != nil { return nil, err } @@ -31,7 +30,7 @@ func (c *Client) ListIssueRelatedBranches(ctx context.Context, owner, repo strin // // PUT /repos/{owner}/{repo}/issues/{number}/related-branches func (c *Client) SetIssueRelatedBranches(ctx context.Context, owner, repo string, number int, branches []string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/issues/%d/related-branches", owner, repo, number), map[string]interface{}{"branches": branches}, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/issues/%d/related-branches", owner, repo, number), map[string]interface{}{"branches": branches}, nil) } // --- Issue Kanban Values --- @@ -48,7 +47,7 @@ type KanbanValue struct { // // PUT /repos/{owner}/{repo}/issues/{number}/kanban-values func (c *Client) UpdateIssueKanbanValues(ctx context.Context, owner, repo string, number int, values []KanbanValue) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/issues/%d/kanban-values", owner, repo, number), values, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/issues/%d/kanban-values", owner, repo, number), values, nil) } // --- Issue Modify History --- @@ -70,7 +69,7 @@ type ModifyHistoryEntry struct { // GET /repos/{owner}/{repo}/issues/{number}/modify-history func (c *Client) ListIssueModifyHistory(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*ModifyHistoryEntry, error) { var history []*ModifyHistoryEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/modify-history?%s", owner, repo, number, opts.toQuery()), nil, &history) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/modify-history?%s", owner, repo, number, opts.toQuery()), nil, &history) if err != nil { return nil, err } @@ -82,7 +81,7 @@ func (c *Client) ListIssueModifyHistory(ctx context.Context, owner, repo string, // GET /repos/{owner}/{repo}/issues/comment/{comment_id}/modify-history func (c *Client) ListIssueCommentModifyHistory(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*ModifyHistoryEntry, error) { var history []*ModifyHistoryEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/comment/%d/modify-history?%s", owner, repo, commentID, opts.toQuery()), nil, &history) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/comment/%d/modify-history?%s", owner, repo, commentID, opts.toQuery()), nil, &history) if err != nil { return nil, err } @@ -104,7 +103,7 @@ type IssueUserReaction struct { // GET /repos/{owner}/{repo}/issues/{number}/user-reactions func (c *Client) ListIssueUserReactions(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*IssueUserReaction, error) { var reactions []*IssueUserReaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/user-reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/user-reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -116,7 +115,7 @@ func (c *Client) ListIssueUserReactions(ctx context.Context, owner, repo string, // GET /repos/{owner}/{repo}/issues/comment/{comment_id}/user-reactions func (c *Client) ListIssueCommentUserReactions(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*IssueUserReaction, error) { var reactions []*IssueUserReaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/comment/%d/user-reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/comment/%d/user-reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } diff --git a/labels.go b/labels.go index c9008f9..f8f625c 100644 --- a/labels.go +++ b/labels.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -13,7 +12,7 @@ type UpdateLabelOptions struct { func (c *Client) UpdateIssueLabel(ctx context.Context, owner, repo, originalName string, opts UpdateLabelOptions) (*Label, error) { var label Label - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/labels/%s", owner, repo, originalName), opts, &label) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/labels/%s", owner, repo, originalName), opts, &label) if err != nil { return nil, err } @@ -21,9 +20,9 @@ func (c *Client) UpdateIssueLabel(ctx context.Context, owner, repo, originalName } func (c *Client) ReplaceIssueLabels(ctx context.Context, owner, repo string, number int, labels []string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/issues/%d/labels", owner, repo, number), labels, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/issues/%d/labels", owner, repo, number), labels, nil) } func (c *Client) RemoveAllIssueLabels(ctx context.Context, owner, repo string, number int) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/labels", owner, repo, number), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/labels", owner, repo, number), nil, nil) } diff --git a/milestones.go b/milestones.go index 6cd4111..5992b40 100644 --- a/milestones.go +++ b/milestones.go @@ -28,7 +28,7 @@ func (c *Client) ListMilestonesWithOptions(ctx context.Context, owner, repo stri if opts.Direction != "" { q.Set("direction", opts.Direction) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/milestones?%s", owner, repo, q.Encode()), nil, &milestones) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/milestones?%s", owner, repo, q.Encode()), nil, &milestones) if err != nil { return nil, err } @@ -37,7 +37,7 @@ func (c *Client) ListMilestonesWithOptions(ctx context.Context, owner, repo stri func (c *Client) GetMilestone(ctx context.Context, owner, repo string, number int) (*Milestone, error) { var milestone Milestone - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/milestones/%d", owner, repo, number), nil, &milestone) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/milestones/%d", owner, repo, number), nil, &milestone) if err != nil { return nil, err } @@ -59,7 +59,7 @@ type UpdateMilestoneOptions struct { func (c *Client) UpdateMilestone(ctx context.Context, owner, repo string, number int, opts UpdateMilestoneOptions) (*Milestone, error) { var milestone Milestone - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/milestones/%d", owner, repo, number), opts, &milestone) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/milestones/%d", owner, repo, number), opts, &milestone) if err != nil { return nil, err } @@ -75,7 +75,7 @@ type CreateMilestoneOptions struct { func (c *Client) CreateMilestoneWithOptions(ctx context.Context, owner, repo string, opts CreateMilestoneOptions) (*Milestone, error) { var milestone Milestone - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/milestones", owner, repo), opts, &milestone) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/milestones", owner, repo), opts, &milestone) if err != nil { return nil, err } diff --git a/misc.go b/misc.go index 5fb99d1..49b2d3f 100644 --- a/misc.go +++ b/misc.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -25,7 +24,7 @@ func (c *Client) ListGitignoreTemplates(ctx context.Context) ([]string, error) { // GET /gitignore/templates/{name} func (c *Client) GetGitignoreTemplate(ctx context.Context, name string) (*GitignoreTemplate, error) { var template GitignoreTemplate - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/gitignore/templates/%s", name), nil, &template) + err := c.doRequest(ctx, http.MethodGet, pathf("/gitignore/templates/%s", name), nil, &template) if err != nil { return nil, err } @@ -57,7 +56,7 @@ func (c *Client) ListLicenseTemplates(ctx context.Context) ([]*LicenseTemplate, // GET /licenses/{name} func (c *Client) GetLicenseTemplate(ctx context.Context, name string) (*LicenseTemplate, error) { var template LicenseTemplate - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/licenses/%s", name), nil, &template) + err := c.doRequest(ctx, http.MethodGet, pathf("/licenses/%s", name), nil, &template) if err != nil { return nil, err } @@ -97,7 +96,7 @@ func (c *Client) ListLabelTemplates(ctx context.Context) ([]string, error) { // GET /label/templates/{name} func (c *Client) GetLabelTemplate(ctx context.Context, name string) ([]*Label, error) { var labels []*Label - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/label/templates/%s", name), nil, &labels) + err := c.doRequest(ctx, http.MethodGet, pathf("/label/templates/%s", name), nil, &labels) if err != nil { return nil, err } @@ -146,14 +145,10 @@ func (c *Client) RenderMarkdownRaw(ctx context.Context, markdown string) (string // GET /user/repos func (c *Client) ListCurrentUserRepositories(ctx context.Context, opts ListRepositoriesOptions) ([]*Repository, error) { var repos []*Repository - query := opts.toQuery() - if opts.Type != "" { - query += "&type=" + opts.Type - } - if opts.Sort != "" { - query += "&sort=" + opts.Sort - } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/repos?%s", query), nil, &repos) + // opts.toQuery() is ListRepositoriesOptions.toQuery, which already + // carries type/sort — the same helper ListRepositories uses, so the + // two entry points cannot diverge again (issue #10). + err := c.doRequest(ctx, http.MethodGet, pathf("/user/repos?%s", opts.toQuery()), nil, &repos) if err != nil { return nil, err } @@ -165,7 +160,7 @@ func (c *Client) ListCurrentUserRepositories(ctx context.Context, opts ListRepos // GET /users/{username}/repos func (c *Client) ListUserRepositories(ctx context.Context, username string, opts ListOptions) ([]*Repository, error) { var repos []*Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s/repos?%s", username, opts.toQuery()), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s/repos?%s", username, opts.toQuery()), nil, &repos) if err != nil { return nil, err } @@ -198,7 +193,7 @@ type ExtendedUser struct { // GET /users/{username} func (c *Client) GetExtendedUser(ctx context.Context, username string) (*ExtendedUser, error) { var user ExtendedUser - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s", username), nil, &user) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s", username), nil, &user) if err != nil { return nil, err } diff --git a/notifications_enhanced.go b/notifications_enhanced.go index 4874331..1d17a70 100644 --- a/notifications_enhanced.go +++ b/notifications_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -59,7 +58,7 @@ func (c *Client) ListNotificationsWithOptions(ctx context.Context, opts ListNoti if opts.Status != "" { query += "&status=" + opts.Status } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/notifications?%s", query), nil, ¬ifications) + err := c.doRequest(ctx, http.MethodGet, pathf("/notifications?%s", query), nil, ¬ifications) if err != nil { return nil, err } @@ -71,7 +70,7 @@ func (c *Client) ListNotificationsWithOptions(ctx context.Context, opts ListNoti // GET /notifications/threads/{id} func (c *Client) GetNotificationThread(ctx context.Context, threadID int64) (*NotificationThread, error) { var thread NotificationThread - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/notifications/threads/%d", threadID), nil, &thread) + err := c.doRequest(ctx, http.MethodGet, pathf("/notifications/threads/%d", threadID), nil, &thread) if err != nil { return nil, err } @@ -82,7 +81,7 @@ func (c *Client) GetNotificationThread(ctx context.Context, threadID int64) (*No // // PATCH /notifications/threads/{id} func (c *Client) MarkNotificationThreadAsRead(ctx context.Context, threadID int64) error { - return c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/notifications/threads/%d", threadID), nil, nil) + return c.doRequest(ctx, http.MethodPatch, pathf("/notifications/threads/%d", threadID), nil, nil) } // MarkNotificationsAsRead marks all notifications as read. @@ -110,7 +109,7 @@ func (c *Client) ListRepoNotifications(ctx context.Context, owner, repo string, if opts.Status != "" { query += "&status=" + opts.Status } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/notifications?%s", owner, repo, query), nil, ¬ifications) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/notifications?%s", owner, repo, query), nil, ¬ifications) if err != nil { return nil, err } @@ -121,5 +120,5 @@ func (c *Client) ListRepoNotifications(ctx context.Context, owner, repo string, // // PUT /repos/{owner}/{repo}/notifications func (c *Client) MarkRepoNotificationsAsRead(ctx context.Context, owner, repo string, opts MarkNotificationsOptions) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/notifications", owner, repo), opts, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/notifications", owner, repo), opts, nil) } diff --git a/oauth.go b/oauth.go index 34ce6fd..f5f7f02 100644 --- a/oauth.go +++ b/oauth.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "io" "net/http" "net/url" "strings" @@ -99,7 +98,10 @@ func (o *OAuthClient) doTokenRequest(ctx context.Context, data url.Values) (*OAu } defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + // Token responses are tiny JSON documents; read them with the same + // bounded reader as the API client so a hostile token endpoint cannot + // trigger unbounded allocation (issue #7). + body, err := readBodyLimited(resp.Body, defaultMaxJSONBodyBytes) if err != nil { return nil, err } diff --git a/org_labels.go b/org_labels.go index a5326e2..033217f 100644 --- a/org_labels.go +++ b/org_labels.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -39,7 +38,7 @@ type UpdateOrgLabelOptions struct { // GET /orgs/{org}/labels func (c *Client) ListOrgLabels(ctx context.Context, org string, opts ListOptions) ([]*OrgLabel, error) { var labels []*OrgLabel - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/labels?%s", org, opts.toQuery()), nil, &labels) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/labels?%s", org, opts.toQuery()), nil, &labels) if err != nil { return nil, err } @@ -51,7 +50,7 @@ func (c *Client) ListOrgLabels(ctx context.Context, org string, opts ListOptions // GET /orgs/{org}/labels/{id} func (c *Client) GetOrgLabel(ctx context.Context, org string, labelID int64) (*OrgLabel, error) { var label OrgLabel - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/labels/%d", org, labelID), nil, &label) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/labels/%d", org, labelID), nil, &label) if err != nil { return nil, err } @@ -63,7 +62,7 @@ func (c *Client) GetOrgLabel(ctx context.Context, org string, labelID int64) (*O // POST /orgs/{org}/labels func (c *Client) CreateOrgLabel(ctx context.Context, org string, opts CreateOrgLabelOptions) (*OrgLabel, error) { var label OrgLabel - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/orgs/%s/labels", org), opts, &label) + err := c.doRequest(ctx, http.MethodPost, pathf("/orgs/%s/labels", org), opts, &label) if err != nil { return nil, err } @@ -75,7 +74,7 @@ func (c *Client) CreateOrgLabel(ctx context.Context, org string, opts CreateOrgL // PATCH /orgs/{org}/labels/{id} func (c *Client) UpdateOrgLabel(ctx context.Context, org string, labelID int64, opts UpdateOrgLabelOptions) (*OrgLabel, error) { var label OrgLabel - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/orgs/%s/labels/%d", org, labelID), opts, &label) + err := c.doRequest(ctx, http.MethodPatch, pathf("/orgs/%s/labels/%d", org, labelID), opts, &label) if err != nil { return nil, err } @@ -86,5 +85,5 @@ func (c *Client) UpdateOrgLabel(ctx context.Context, org string, labelID int64, // // DELETE /orgs/{org}/labels/{id} func (c *Client) DeleteOrgLabel(ctx context.Context, org string, labelID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s/labels/%d", org, labelID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/orgs/%s/labels/%d", org, labelID), nil, nil) } diff --git a/org_members_enhanced.go b/org_members_enhanced.go index 8c79832..3322c55 100644 --- a/org_members_enhanced.go +++ b/org_members_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -11,7 +10,7 @@ import ( // GET /orgs/{org}/public_members func (c *Client) ListOrgPublicMembers(ctx context.Context, org string, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/public_members?%s", org, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/public_members?%s", org, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -22,7 +21,7 @@ func (c *Client) ListOrgPublicMembers(ctx context.Context, org string, opts List // // GET /orgs/{org}/public_members/{username} func (c *Client) IsOrgPublicMember(ctx context.Context, org, username string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/public_members/%s", org, username)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/orgs/%s/public_members/%s", org, username)) if err != nil { if isNotFoundError(err) { return false, nil @@ -36,14 +35,14 @@ func (c *Client) IsOrgPublicMember(ctx context.Context, org, username string) (b // // PUT /orgs/{org}/public_members/{username} func (c *Client) PublicizeOrgMembership(ctx context.Context, org, username string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/orgs/%s/public_members/%s", org, username), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/orgs/%s/public_members/%s", org, username), nil, nil) } // ConcealOrgMembership makes the authenticated user's membership private. // // DELETE /orgs/{org}/public_members/{username} func (c *Client) ConcealOrgMembership(ctx context.Context, org, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s/public_members/%s", org, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/orgs/%s/public_members/%s", org, username), nil, nil) } // ListOrgBlockedUsers lists all blocked users of an organization. @@ -51,7 +50,7 @@ func (c *Client) ConcealOrgMembership(ctx context.Context, org, username string) // GET /orgs/{org}/blocks func (c *Client) ListOrgBlockedUsers(ctx context.Context, org string, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/blocks?%s", org, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/blocks?%s", org, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -62,7 +61,7 @@ func (c *Client) ListOrgBlockedUsers(ctx context.Context, org string, opts ListO // // GET /orgs/{org}/blocks/{username} func (c *Client) IsOrgBlockedUser(ctx context.Context, org, username string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/blocks/%s", org, username)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/orgs/%s/blocks/%s", org, username)) if err != nil { if isNotFoundError(err) { return false, nil @@ -76,14 +75,14 @@ func (c *Client) IsOrgBlockedUser(ctx context.Context, org, username string) (bo // // PUT /orgs/{org}/blocks/{username} func (c *Client) BlockOrgUser(ctx context.Context, org, username string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/orgs/%s/blocks/%s", org, username), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/orgs/%s/blocks/%s", org, username), nil, nil) } // UnblockOrgUser unblocks a user from an organization. // // DELETE /orgs/{org}/blocks/{username} func (c *Client) UnblockOrgUser(ctx context.Context, org, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s/blocks/%s", org, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/orgs/%s/blocks/%s", org, username), nil, nil) } // CreateOrganization creates a new organization. @@ -113,5 +112,5 @@ type CreateOrgOptions struct { // // DELETE /orgs/{org} func (c *Client) DeleteOrganization(ctx context.Context, org string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s", org), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/orgs/%s", org), nil, nil) } diff --git a/org_teams.go b/org_teams.go index 31c82c8..e872cc9 100644 --- a/org_teams.go +++ b/org_teams.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -54,7 +53,7 @@ type TeamMember struct { // GET /orgs/{org}/teams func (c *Client) ListOrgTeams(ctx context.Context, org string, opts ListOptions) ([]*Team, error) { var teams []*Team - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/teams?%s", org, opts.toQuery()), nil, &teams) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/teams?%s", org, opts.toQuery()), nil, &teams) if err != nil { return nil, err } @@ -66,7 +65,7 @@ func (c *Client) ListOrgTeams(ctx context.Context, org string, opts ListOptions) // GET /teams/{team_id} func (c *Client) GetTeam(ctx context.Context, teamID int64) (*Team, error) { var team Team - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/teams/%d", teamID), nil, &team) + err := c.doRequest(ctx, http.MethodGet, pathf("/teams/%d", teamID), nil, &team) if err != nil { return nil, err } @@ -78,7 +77,7 @@ func (c *Client) GetTeam(ctx context.Context, teamID int64) (*Team, error) { // POST /orgs/{org}/teams func (c *Client) CreateTeam(ctx context.Context, org string, opts CreateTeamOptions) (*Team, error) { var team Team - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/orgs/%s/teams", org), opts, &team) + err := c.doRequest(ctx, http.MethodPost, pathf("/orgs/%s/teams", org), opts, &team) if err != nil { return nil, err } @@ -90,7 +89,7 @@ func (c *Client) CreateTeam(ctx context.Context, org string, opts CreateTeamOpti // PATCH /teams/{team_id} func (c *Client) UpdateTeam(ctx context.Context, teamID int64, opts UpdateTeamOptions) (*Team, error) { var team Team - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/teams/%d", teamID), opts, &team) + err := c.doRequest(ctx, http.MethodPatch, pathf("/teams/%d", teamID), opts, &team) if err != nil { return nil, err } @@ -101,7 +100,7 @@ func (c *Client) UpdateTeam(ctx context.Context, teamID int64, opts UpdateTeamOp // // DELETE /teams/{team_id} func (c *Client) DeleteTeam(ctx context.Context, teamID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/teams/%d", teamID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/teams/%d", teamID), nil, nil) } // ListTeamMembers lists all members of a team. @@ -109,7 +108,7 @@ func (c *Client) DeleteTeam(ctx context.Context, teamID int64) error { // GET /teams/{team_id}/members func (c *Client) ListTeamMembers(ctx context.Context, teamID int64, opts ListOptions) ([]*TeamMember, error) { var members []*TeamMember - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/teams/%d/members?%s", teamID, opts.toQuery()), nil, &members) + err := c.doRequest(ctx, http.MethodGet, pathf("/teams/%d/members?%s", teamID, opts.toQuery()), nil, &members) if err != nil { return nil, err } @@ -121,7 +120,7 @@ func (c *Client) ListTeamMembers(ctx context.Context, teamID int64, opts ListOpt // GET /teams/{team_id}/members/{username} func (c *Client) GetTeamMember(ctx context.Context, teamID int64, username string) (*User, error) { var user User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/teams/%d/members/%s", teamID, username), nil, &user) + err := c.doRequest(ctx, http.MethodGet, pathf("/teams/%d/members/%s", teamID, username), nil, &user) if err != nil { return nil, err } @@ -132,14 +131,14 @@ func (c *Client) GetTeamMember(ctx context.Context, teamID int64, username strin // // PUT /teams/{team_id}/members/{username} func (c *Client) AddTeamMember(ctx context.Context, teamID int64, username string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/teams/%d/members/%s", teamID, username), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/teams/%d/members/%s", teamID, username), nil, nil) } // RemoveTeamMember removes a member from a team. // // DELETE /teams/{team_id}/members/{username} func (c *Client) RemoveTeamMember(ctx context.Context, teamID int64, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/teams/%d/members/%s", teamID, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/teams/%d/members/%s", teamID, username), nil, nil) } // ListTeamRepositories lists all repositories of a team. @@ -147,7 +146,7 @@ func (c *Client) RemoveTeamMember(ctx context.Context, teamID int64, username st // GET /teams/{team_id}/repos func (c *Client) ListTeamRepositories(ctx context.Context, teamID int64, opts ListOptions) ([]*Repository, error) { var repos []*Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/teams/%d/repos?%s", teamID, opts.toQuery()), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/teams/%d/repos?%s", teamID, opts.toQuery()), nil, &repos) if err != nil { return nil, err } @@ -158,12 +157,12 @@ func (c *Client) ListTeamRepositories(ctx context.Context, teamID int64, opts Li // // PUT /teams/{team_id}/repos/{org}/{repo} func (c *Client) AddTeamRepository(ctx context.Context, teamID int64, org, repo string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/teams/%d/repos/%s/%s", teamID, org, repo), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/teams/%d/repos/%s/%s", teamID, org, repo), nil, nil) } // RemoveTeamRepository removes a repository from a team. // // DELETE /teams/{team_id}/repos/{org}/{repo} func (c *Client) RemoveTeamRepository(ctx context.Context, teamID int64, org, repo string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/teams/%d/repos/%s/%s", teamID, org, repo), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/teams/%d/repos/%s/%s", teamID, org, repo), nil, nil) } diff --git a/orgs.go b/orgs.go index 9ba0b49..79c5062 100644 --- a/orgs.go +++ b/orgs.go @@ -2,13 +2,12 @@ package gitcode import ( "context" - "fmt" "net/http" ) func (c *Client) ListUserOrganizations(ctx context.Context, username string, opts ListOptions) ([]*Organization, error) { var orgs []*Organization - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s/orgs?%s", username, opts.toQuery()), nil, &orgs) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s/orgs?%s", username, opts.toQuery()), nil, &orgs) if err != nil { return nil, err } @@ -21,7 +20,7 @@ func (c *Client) ListOrganizationsWithOptions(ctx context.Context, admin bool, o if admin { query += "&admin=true" } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/orgs?%s", query), nil, &orgs) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/orgs?%s", query), nil, &orgs) if err != nil { return nil, err } @@ -39,7 +38,7 @@ type OrgMemberDetail struct { func (c *Client) GetOrgMemberDetail(ctx context.Context, org, username string) (*OrgMemberDetail, error) { var detail OrgMemberDetail - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/members/%s", org, username), nil, &detail) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/members/%s", org, username), nil, &detail) if err != nil { return nil, err } @@ -48,7 +47,7 @@ func (c *Client) GetOrgMemberDetail(ctx context.Context, org, username string) ( func (c *Client) GetOrgInfo(ctx context.Context, org string) (*Organization, error) { var o Organization - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s", org), nil, &o) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s", org), nil, &o) if err != nil { return nil, err } @@ -65,7 +64,7 @@ type UpdateOrgOptions struct { func (c *Client) UpdateOrganization(ctx context.Context, org string, opts UpdateOrgOptions) (*Organization, error) { var o Organization - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/orgs/%s", org), opts, &o) + err := c.doRequest(ctx, http.MethodPatch, pathf("/orgs/%s", org), opts, &o) if err != nil { return nil, err } @@ -79,7 +78,7 @@ type InviteMemberOptions struct { func (c *Client) InviteOrgMember(ctx context.Context, org, username string, opts InviteMemberOptions) (*User, error) { var user User - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/orgs/%s/memberships/%s", org, username), opts, &user) + err := c.doRequest(ctx, http.MethodPost, pathf("/orgs/%s/memberships/%s", org, username), opts, &user) if err != nil { return nil, err } @@ -87,7 +86,7 @@ func (c *Client) InviteOrgMember(ctx context.Context, org, username string, opts } func (c *Client) RemoveOrgMember(ctx context.Context, org, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/orgs/%s/memberships/%s", org, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/orgs/%s/memberships/%s", org, username), nil, nil) } type OrgFollowers struct { @@ -100,7 +99,7 @@ type OrgFollowers struct { func (c *Client) ListOrgFollowers(ctx context.Context, org string, opts ListOptions) ([]*OrgFollowers, error) { var followers []*OrgFollowers - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/followers?%s", org, opts.toQuery()), nil, &followers) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/followers?%s", org, opts.toQuery()), nil, &followers) if err != nil { return nil, err } @@ -123,7 +122,7 @@ func (c *Client) ListOrgMembers(ctx context.Context, org, role string, opts List if role != "" { query += "&role=" + role } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/members?%s", org, query), nil, &members) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/members?%s", org, query), nil, &members) if err != nil { return nil, err } @@ -148,7 +147,7 @@ type UserMembership struct { func (c *Client) GetUserMembership(ctx context.Context, org string) (*UserMembership, error) { var membership UserMembership - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/memberships/orgs/%s", org), nil, &membership) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/memberships/orgs/%s", org), nil, &membership) if err != nil { return nil, err } @@ -156,7 +155,7 @@ func (c *Client) GetUserMembership(ctx context.Context, org string) (*UserMember } func (c *Client) ExitOrganization(ctx context.Context, org string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/user/memberships/orgs/%s", org), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/user/memberships/orgs/%s", org), nil, nil) } type IssueExtendSetting struct { @@ -173,7 +172,7 @@ type IssueExtendSetting struct { func (c *Client) GetOrgIssueExtendSettings(ctx context.Context, org string) ([]*IssueExtendSetting, error) { var settings []*IssueExtendSetting - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/issue/extend/settings", org), nil, &settings) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/issue/extend/settings", org), nil, &settings) if err != nil { return nil, err } diff --git a/orgs_enhanced.go b/orgs_enhanced.go index 08706bf..9cee1f3 100644 --- a/orgs_enhanced.go +++ b/orgs_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -22,7 +21,7 @@ type OrgCustomizedRole struct { // GET /org/{org}/customized-roles func (c *Client) ListOrgCustomizedRoles(ctx context.Context, org string) ([]*OrgCustomizedRole, error) { var roles []*OrgCustomizedRole - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/org/%s/customized-roles", org), nil, &roles) + err := c.doRequest(ctx, http.MethodGet, pathf("/org/%s/customized-roles", org), nil, &roles) if err != nil { return nil, err } @@ -72,7 +71,7 @@ type OrgDiscussionCommentReply struct { // GET /orgs/{org}/discuss func (c *Client) ListOrgDiscussions(ctx context.Context, org string, opts ListOptions) ([]*OrgDiscussion, error) { var discussions []*OrgDiscussion - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/discuss?%s", org, opts.toQuery()), nil, &discussions) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/discuss?%s", org, opts.toQuery()), nil, &discussions) if err != nil { return nil, err } @@ -84,7 +83,7 @@ func (c *Client) ListOrgDiscussions(ctx context.Context, org string, opts ListOp // GET /orgs/{org}/discuss/{number} func (c *Client) GetOrgDiscussion(ctx context.Context, org string, number int) (*OrgDiscussion, error) { var discussion OrgDiscussion - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/discuss/%d", org, number), nil, &discussion) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/discuss/%d", org, number), nil, &discussion) if err != nil { return nil, err } @@ -96,7 +95,7 @@ func (c *Client) GetOrgDiscussion(ctx context.Context, org string, number int) ( // GET /orgs/{org}/discuss/{number}/comment func (c *Client) ListOrgDiscussionComments(ctx context.Context, org string, number int, opts ListOptions) ([]*OrgDiscussionComment, error) { var comments []*OrgDiscussionComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/discuss/%d/comment?%s", org, number, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/discuss/%d/comment?%s", org, number, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -108,7 +107,7 @@ func (c *Client) ListOrgDiscussionComments(ctx context.Context, org string, numb // GET /orgs/{org}/discuss/{number}/comment/{comment_id}/reply func (c *Client) ListOrgDiscussionCommentReplies(ctx context.Context, org string, number int, commentID int64, opts ListOptions) ([]*OrgDiscussionCommentReply, error) { var replies []*OrgDiscussionCommentReply - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/discuss/%d/comment/%d/reply?%s", org, number, commentID, opts.toQuery()), nil, &replies) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/discuss/%d/comment/%d/reply?%s", org, number, commentID, opts.toQuery()), nil, &replies) if err != nil { return nil, err } diff --git a/pagination.go b/pagination.go index 79062f4..364f430 100644 --- a/pagination.go +++ b/pagination.go @@ -2,9 +2,26 @@ package gitcode import ( "context" + "errors" + "fmt" "iter" ) +// maxPaginatePages bounds the page walk: per_page=100 × 50 pages is far +// beyond any real list, and a server that keeps returning full pages +// without the data advancing (broken cursor, ignored pagination params, +// cached response) used to walk forever — 125k requests in a 300ms repro — +// with only the caller's ctx as a brake (issue #5). The value aligns with +// the sister repo go-gongfeng's maxPaginatePages. +const maxPaginatePages = 50 + +// ErrTooManyPages is yielded by Paginate when the page walk hits +// maxPaginatePages and the last page was still full — the iteration was +// capped, not completed, and the caller is looking at a truncated list. +// Distinguish it with errors.Is; treating it as fatal or as "good enough" +// is the caller's call. +var ErrTooManyPages = errors.New("gitcode: paginate exceeded the page cap; results are truncated") + // PageResult represents a single page of results. type PageResult[T any] struct { Items []T @@ -22,12 +39,23 @@ type PageResult[T any] struct { // // process issue // } // } +// +// The walk is bounded: at most maxPaginatePages pages are fetched. When the +// cap is hit on a still-full page the iterator yields ErrTooManyPages +// instead of ending silently, so a truncated result set cannot masquerade +// as a complete one (issue #5). func Paginate[T any](ctx context.Context, fetch func(ListOptions) ([]T, error)) iter.Seq2[[]T, error] { return func(yield func([]T, error) bool) { - page := 1 perPage := 100 // max per page for efficiency - for { + for page := 1; page <= maxPaginatePages; page++ { + // Check context cancellation + select { + case <-ctx.Done(): + return + default: + } + items, err := fetch(ListOptions{Page: page, PerPage: perPage}) if err != nil { yield(nil, err) @@ -46,16 +74,11 @@ func Paginate[T any](ctx context.Context, fetch func(ListOptions) ([]T, error)) if len(items) < perPage { return } - - page++ - - // Check context cancellation - select { - case <-ctx.Done(): - return - default: - } } + + // The cap is reached and the last page was still full: the caller + // must know the list was truncated. + yield(nil, fmt.Errorf("%w after %d pages of %d items", ErrTooManyPages, maxPaginatePages, perPage)) } } diff --git a/path.go b/path.go new file mode 100644 index 0000000..dd7925e --- /dev/null +++ b/path.go @@ -0,0 +1,88 @@ +package gitcode + +import ( + "fmt" + "net/url" + "slices" + "strings" +) + +// escapedPath marks an argument that is already percent-escaped and must be +// inserted into the path verbatim (multi-segment file paths escaped by +// pathSegs). pathf leaves values of this type untouched so they are not +// double-escaped. +type escapedPath string + +// pathSeg escapes a single path segment (owner, repo, org, branch, sha…). +// +// Callers control these values and they must not be able to alter the shape +// of the request path: url.PathEscape turns "/" into %2F, "?" into %3F, +// "#" into %23 — a malicious or merely unusual identifier stays confined to +// its own segment instead of adding path levels or spilling into the query +// string (issue #6). +func pathSeg(s string) string { + return url.PathEscape(s) +} + +// pathSegs escapes a slash-bearing path argument (repository file paths) +// segment by segment: the separating slashes are preserved — the server +// must keep seeing distinct segments — while every other reserved character +// is percent-escaped. The result is an escapedPath so pathf inserts it +// verbatim. +func pathSegs(s string) escapedPath { + parts := strings.Split(s, "/") + for i, p := range parts { + parts[i] = url.PathEscape(p) + } + return escapedPath(strings.Join(parts, "/")) +} + +// pathf builds an endpoint path from a fmt-style format string, escaping +// every string argument as a single path segment. It is the single place +// where endpoint paths are assembled (issue #6: 383 call sites used to +// concatenate caller-controlled values raw). +// +// Rules: +// - Verbs before the first "?" in the format belong to the path: string +// args are escaped with url.PathEscape (pathSeg); non-string args +// (ints via %d) are formatted as-is. +// - Verbs after the first "?" belong to the query string and are left +// untouched — callers pass pre-encoded query values (toQuery, Encode). +// - Arguments of type escapedPath (from pathSegs) are inserted verbatim, +// never double-escaped. +func pathf(format string, args ...any) string { + if qi := strings.IndexByte(format, '?'); qi >= 0 { + escaped := slices.Clone(args) + escapePathVerbs(format[:qi], escaped) + return fmt.Sprintf(format, escaped...) + } + escapePathVerbs(format, args) + return fmt.Sprintf(format, args...) +} + +// escapePathVerbs escapes, in place, the string arguments bound to the +// format verbs contained in the given (path-only) portion of a format +// string. +func escapePathVerbs(pathFormat string, args []any) { + arg := 0 + for i := 0; i < len(pathFormat); i++ { + if pathFormat[i] != '%' || i+1 >= len(pathFormat) { + continue + } + switch pathFormat[i+1] { + case '%': + i++ + case 's', 'd', 'v', 'q', 'x', 'X', 'o', 'b', 'f', 'g', 'e': + if arg < len(args) { + switch v := args[arg].(type) { + case escapedPath: + // already escaped (pathSegs); keep verbatim + case string: + args[arg] = pathSeg(v) + } + } + arg++ + i++ + } + } +} diff --git a/pulls.go b/pulls.go index 234abca..b0d7def 100644 --- a/pulls.go +++ b/pulls.go @@ -138,7 +138,7 @@ func (c *Client) ListPullRequests(ctx context.Context, owner, repo string, opts if opts.Base != "" { query += "&base=" + opts.Base } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls?%s", owner, repo, query), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls?%s", owner, repo, query), nil, &prs) if err != nil { return nil, err } @@ -147,7 +147,7 @@ func (c *Client) ListPullRequests(ctx context.Context, owner, repo string, opts func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { var pr PullRequest - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d", owner, repo, number), nil, &pr) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d", owner, repo, number), nil, &pr) if err != nil { return nil, err } @@ -156,7 +156,7 @@ func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number func (c *Client) CreatePullRequest(ctx context.Context, owner, repo string, opts CreatePullRequestOptions) (*PullRequest, error) { var pr PullRequest - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls", owner, repo), opts, &pr) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls", owner, repo), opts, &pr) if err != nil { return nil, err } @@ -165,7 +165,7 @@ func (c *Client) CreatePullRequest(ctx context.Context, owner, repo string, opts func (c *Client) UpdatePullRequest(ctx context.Context, owner, repo string, number int, opts UpdatePullRequestOptions) (*PullRequest, error) { var pr PullRequest - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/pulls/%d", owner, repo, number), opts, &pr) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/pulls/%d", owner, repo, number), opts, &pr) if err != nil { return nil, err } @@ -209,11 +209,11 @@ func (c *Client) MergePullRequest(ctx context.Context, owner, repo string, numbe body["merge_method"] = "squash" } } - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/pulls/%d/merge", owner, repo, number), body, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/pulls/%d/merge", owner, repo, number), body, nil) } func (c *Client) IsPullRequestMerged(ctx context.Context, owner, repo string, number int) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/merge", owner, repo, number)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/merge", owner, repo, number)) if err != nil { if strings.Contains(err.Error(), "404") { return false, nil @@ -225,7 +225,7 @@ func (c *Client) IsPullRequestMerged(ctx context.Context, owner, repo string, nu func (c *Client) ListPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]*PullRequestFile, error) { var files []*PullRequestFile - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/files", owner, repo, number), nil, &files) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/files", owner, repo, number), nil, &files) if err != nil { return nil, err } @@ -234,7 +234,7 @@ func (c *Client) ListPullRequestFiles(ctx context.Context, owner, repo string, n func (c *Client) ListPullRequestComments(ctx context.Context, owner, repo string, number int) ([]*PullRequestComment, error) { var comments []*PullRequestComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), nil, &comments) if err != nil { return nil, err } @@ -243,7 +243,7 @@ func (c *Client) ListPullRequestComments(ctx context.Context, owner, repo string func (c *Client) CreatePullRequestComment(ctx context.Context, owner, repo string, number int, body, path string, position, commitID string) (*PullRequestComment, error) { var comment PullRequestComment - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), map[string]interface{}{ + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), map[string]interface{}{ "body": body, "path": path, "position": position, @@ -257,7 +257,7 @@ func (c *Client) CreatePullRequestComment(ctx context.Context, owner, repo strin func (c *Client) CreatePullRequestInlineComment(ctx context.Context, owner, repo string, number int, opts CreatePullRequestInlineCommentOptions) (*PullRequestComment, error) { var comment PullRequestComment - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), opts, &comment) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/comments", owner, repo, number), opts, &comment) if err != nil { return nil, err } @@ -266,7 +266,7 @@ func (c *Client) CreatePullRequestInlineComment(ctx context.Context, owner, repo func (c *Client) UpdatePullRequestComment(ctx context.Context, owner, repo string, commentID string, body string) (*PullRequestComment, error) { var comment PullRequestComment - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/pulls/comments/%s", owner, repo, commentID), map[string]string{"body": body}, &comment) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/pulls/comments/%s", owner, repo, commentID), map[string]string{"body": body}, &comment) if err != nil { return nil, err } @@ -274,12 +274,12 @@ func (c *Client) UpdatePullRequestComment(ctx context.Context, owner, repo strin } func (c *Client) DeletePullRequestComment(ctx context.Context, owner, repo string, commentID string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/comments/%s", owner, repo, commentID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/comments/%s", owner, repo, commentID), nil, nil) } func (c *Client) ListPullRequestReviews(ctx context.Context, owner, repo string, number int) ([]*PullRequestReview, error) { var reviews []*PullRequestReview - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews", owner, repo, number), nil, &reviews) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/reviews", owner, repo, number), nil, &reviews) if err != nil { return nil, err } @@ -288,7 +288,7 @@ func (c *Client) ListPullRequestReviews(ctx context.Context, owner, repo string, func (c *Client) CreatePullRequestReview(ctx context.Context, owner, repo string, number int, body, event string) (*PullRequestReview, error) { var review PullRequestReview - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews", owner, repo, number), map[string]interface{}{ + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/reviews", owner, repo, number), map[string]interface{}{ "body": body, "event": event, }, &review) @@ -300,7 +300,7 @@ func (c *Client) CreatePullRequestReview(ctx context.Context, owner, repo string func (c *Client) ListPullRequestCommits(ctx context.Context, owner, repo string, number int) ([]*Commit, error) { var commits []*Commit - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/commits", owner, repo, number), nil, &commits) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/commits", owner, repo, number), nil, &commits) if err != nil { return nil, err } @@ -309,7 +309,7 @@ func (c *Client) ListPullRequestCommits(ctx context.Context, owner, repo string, func (c *Client) ListPullRequestLabels(ctx context.Context, owner, repo string, number int) ([]*Label, error) { var labels []*Label - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), nil, &labels) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), nil, &labels) if err != nil { return nil, err } @@ -317,15 +317,15 @@ func (c *Client) ListPullRequestLabels(ctx context.Context, owner, repo string, } func (c *Client) AddPullRequestLabels(ctx context.Context, owner, repo string, number int, labels []string) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), labels, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), labels, nil) } func (c *Client) RemovePullRequestLabel(ctx context.Context, owner, repo string, number int, name string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/labels/%s", owner, repo, number, name), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/labels/%s", owner, repo, number, name), nil, nil) } func (c *Client) ReplacePullRequestLabels(ctx context.Context, owner, repo string, number int, labels []string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), labels, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/pulls/%d/labels", owner, repo, number), labels, nil) } // AssignPullRequestReviewers 指派用户审查 PR。 @@ -336,18 +336,18 @@ func (c *Client) ReplacePullRequestLabels(ctx context.Context, owner, repo strin // 跨平台 CR 指派人统一面时不要按 GitHub 血统把此方法接到 assignee 语义上。 // assignees 是用户个人空间地址，逗号分隔。 func (c *Client) AssignPullRequestReviewers(ctx context.Context, owner, repo string, number int, assignees string) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/assignees", owner, repo, number), map[string]string{"assignees": assignees}, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/assignees", owner, repo, number), map[string]string{"assignees": assignees}, nil) } // UnassignPullRequestReviewers 取消用户的 PR 审查指派。语义同 // AssignPullRequestReviewers——wire 词表 assignees，平台语义是审查人， // 与 GitHub/Gitea 的 assignee（经办人）相反。 func (c *Client) UnassignPullRequestReviewers(ctx context.Context, owner, repo string, number int, assignees string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/assignees?assignees=%s", owner, repo, number, assignees), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/assignees?assignees=%s", owner, repo, number, assignees), nil, nil) } func (c *Client) AssignPullRequestTesters(ctx context.Context, owner, repo string, number int, testers string) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/testers", owner, repo, number), map[string]string{"testers": testers}, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/testers", owner, repo, number), map[string]string{"testers": testers}, nil) } func (c *Client) HandlePullRequestTest(ctx context.Context, owner, repo string, number int, force bool) error { @@ -355,7 +355,7 @@ func (c *Client) HandlePullRequestTest(ctx context.Context, owner, repo string, if force { body["force"] = true } - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/test", owner, repo, number), body, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/test", owner, repo, number), body, nil) } func (c *Client) HandlePullRequestReview(ctx context.Context, owner, repo string, number int, force bool) error { @@ -363,7 +363,7 @@ func (c *Client) HandlePullRequestReview(ctx context.Context, owner, repo string if force { body["force"] = true } - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/review", owner, repo, number), body, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/review", owner, repo, number), body, nil) } func (c *Client) ResetPullRequestTestStatus(ctx context.Context, owner, repo string, number int, resetAll bool) error { @@ -371,7 +371,7 @@ func (c *Client) ResetPullRequestTestStatus(ctx context.Context, owner, repo str if resetAll { body["reset_all"] = true } - return c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/pulls/%d/testers", owner, repo, number), body, nil) + return c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/pulls/%d/testers", owner, repo, number), body, nil) } // ResetPullRequestReviewStatus 重置 PR 审查状态。wire 路径词表是 @@ -383,7 +383,7 @@ func (c *Client) ResetPullRequestReviewStatus(ctx context.Context, owner, repo s if resetAll { body["reset_all"] = true } - return c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/pulls/%d/assignees", owner, repo, number), body, nil) + return c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/pulls/%d/assignees", owner, repo, number), body, nil) } type PROperateLog struct { @@ -395,7 +395,7 @@ type PROperateLog struct { func (c *Client) GetPullRequestOperateLogs(ctx context.Context, owner, repo string, number int) ([]*PROperateLog, error) { var logs []*PROperateLog - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/operate_logs", owner, repo, number), nil, &logs) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/operate_logs", owner, repo, number), nil, &logs) if err != nil { return nil, err } @@ -404,7 +404,7 @@ func (c *Client) GetPullRequestOperateLogs(ctx context.Context, owner, repo stri func (c *Client) GetPullRequestLinkedIssues(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*Issue, error) { var issues []*Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/issues?%s", owner, repo, number, opts.toQuery()), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/issues?%s", owner, repo, number, opts.toQuery()), nil, &issues) if err != nil { return nil, err } @@ -413,7 +413,7 @@ func (c *Client) GetPullRequestLinkedIssues(ctx context.Context, owner, repo str func (c *Client) GetPullRequestComment(ctx context.Context, owner, repo string, commentID int64) (*PullRequestComment, error) { var comment PullRequestComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/comments/%d", owner, repo, commentID), nil, &comment) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/comments/%d", owner, repo, commentID), nil, &comment) if err != nil { return nil, err } @@ -443,7 +443,7 @@ func (c *Client) ListEnterprisePullRequests(ctx context.Context, enterprise stri if opts.Direction != "" { query += "&direction=" + opts.Direction } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/pull_requests?%s", enterprise, query), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/pull_requests?%s", enterprise, query), nil, &prs) if err != nil { return nil, err } @@ -465,7 +465,7 @@ func (c *Client) ListOrgPullRequests(ctx context.Context, org string, opts ListE if opts.Direction != "" { query += "&direction=" + opts.Direction } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/org/%s/pull_requests?%s", org, query), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/org/%s/pull_requests?%s", org, query), nil, &prs) if err != nil { return nil, err } @@ -474,7 +474,7 @@ func (c *Client) ListOrgPullRequests(ctx context.Context, org string, opts ListE func (c *Client) GetEnterpriseIssueLinkedPRs(ctx context.Context, enterprise string, number int) ([]*PullRequest, error) { var prs []*PullRequest - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/enterprises/%s/issues/%d/pull_requests", enterprise, number), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/enterprises/%s/issues/%d/pull_requests", enterprise, number), nil, &prs) if err != nil { return nil, err } diff --git a/pulls_enhanced.go b/pulls_enhanced.go index c6a74fa..1b0bbf4 100644 --- a/pulls_enhanced.go +++ b/pulls_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -13,14 +12,14 @@ import ( // // POST /repos/{owner}/{repo}/pulls/{number}/linked-issues func (c *Client) LinkPullRequestIssue(ctx context.Context, owner, repo string, number int, issueNumber int) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/linked-issues", owner, repo, number), map[string]int{"issue_number": issueNumber}, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/linked-issues", owner, repo, number), map[string]int{"issue_number": issueNumber}, nil) } // UnlinkPullRequestIssue removes an issue link from a pull request. // // DELETE /repos/{owner}/{repo}/pulls/{number}/issues func (c *Client) UnlinkPullRequestIssue(ctx context.Context, owner, repo string, number int, issueNumber int) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/issues?issue_number=%d", owner, repo, number, issueNumber), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/issues?issue_number=%d", owner, repo, number, issueNumber), nil, nil) } // --- PR Tester Management --- @@ -29,7 +28,7 @@ func (c *Client) UnlinkPullRequestIssue(ctx context.Context, owner, repo string, // // DELETE /repos/{owner}/{repo}/pulls/{number}/testers func (c *Client) UnassignPullRequestTesters(ctx context.Context, owner, repo string, number int, testers string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/testers?testers=%s", owner, repo, number, testers), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/testers?testers=%s", owner, repo, number, testers), nil, nil) } // ListPullRequestAvailableTesters lists users available as testers for a pull request. @@ -37,7 +36,7 @@ func (c *Client) UnassignPullRequestTesters(ctx context.Context, owner, repo str // GET /repos/{owner}/{repo}/pulls/{number}/option-approval-testers func (c *Client) ListPullRequestAvailableTesters(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/option-approval-testers?%s", owner, repo, number, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/option-approval-testers?%s", owner, repo, number, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -50,14 +49,14 @@ func (c *Client) ListPullRequestAvailableTesters(ctx context.Context, owner, rep // // POST /repos/{owner}/{repo}/pulls/{number}/approval-reviewers func (c *Client) AssignPullRequestApprovalReviewers(ctx context.Context, owner, repo string, number int, reviewers string) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/approval-reviewers", owner, repo, number), map[string]string{"reviewers": reviewers}, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/approval-reviewers", owner, repo, number), map[string]string{"reviewers": reviewers}, nil) } // UnassignPullRequestApprovalReviewers removes approval reviewers from a pull request. // // DELETE /repos/{owner}/{repo}/pulls/{number}/approval-reviewers func (c *Client) UnassignPullRequestApprovalReviewers(ctx context.Context, owner, repo string, number int, reviewers string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/approval-reviewers?reviewers=%s", owner, repo, number, reviewers), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/approval-reviewers?reviewers=%s", owner, repo, number, reviewers), nil, nil) } // ListPullRequestAvailableReviewers lists users available as approval reviewers for a pull request. @@ -65,7 +64,7 @@ func (c *Client) UnassignPullRequestApprovalReviewers(ctx context.Context, owner // GET /repos/{owner}/{repo}/pulls/{number}/option-approval-reviewers func (c *Client) ListPullRequestAvailableReviewers(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/option-approval-reviewers?%s", owner, repo, number, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/option-approval-reviewers?%s", owner, repo, number, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -90,7 +89,7 @@ type PRDiscussionComment struct { // POST /repos/{owner}/{repo}/pulls/{number}/discussions/{discussions_id}/comments func (c *Client) ReplyPullRequestComment(ctx context.Context, owner, repo string, number int, discussionID string, body string) (*PRDiscussionComment, error) { var comment PRDiscussionComment - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/discussions/%s/comments", owner, repo, number, discussionID), map[string]string{"body": body}, &comment) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/discussions/%s/comments", owner, repo, number, discussionID), map[string]string{"body": body}, &comment) if err != nil { return nil, err } @@ -101,7 +100,7 @@ func (c *Client) ReplyPullRequestComment(ctx context.Context, owner, repo string // // PUT /repos/{owner}/{repo}/pulls/{number}/comments/discussions/{id} func (c *Client) ResolvePullRequestDiscussion(ctx context.Context, owner, repo string, number int, discussionID string, resolved bool) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/pulls/%d/comments/discussions/%s", owner, repo, number, discussionID), map[string]bool{"resolved": resolved}, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/pulls/%d/comments/discussions/%s", owner, repo, number, discussionID), map[string]bool{"resolved": resolved}, nil) } // --- PR Modify History --- @@ -111,7 +110,7 @@ func (c *Client) ResolvePullRequestDiscussion(ctx context.Context, owner, repo s // GET /repos/{owner}/{repo}/pulls/{number}/modify-history func (c *Client) ListPullRequestModifyHistory(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*ModifyHistoryEntry, error) { var history []*ModifyHistoryEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/modify-history?%s", owner, repo, number, opts.toQuery()), nil, &history) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/modify-history?%s", owner, repo, number, opts.toQuery()), nil, &history) if err != nil { return nil, err } @@ -123,7 +122,7 @@ func (c *Client) ListPullRequestModifyHistory(ctx context.Context, owner, repo s // GET /repos/{owner}/{repo}/pulls/comment/{comment_id}/modify-history func (c *Client) ListPullRequestCommentModifyHistory(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*ModifyHistoryEntry, error) { var history []*ModifyHistoryEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/comment/%d/modify-history?%s", owner, repo, commentID, opts.toQuery()), nil, &history) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/comment/%d/modify-history?%s", owner, repo, commentID, opts.toQuery()), nil, &history) if err != nil { return nil, err } @@ -137,7 +136,7 @@ func (c *Client) ListPullRequestCommentModifyHistory(ctx context.Context, owner, // GET /repos/{owner}/{repo}/pulls/{number}/user-reactions func (c *Client) ListPullRequestUserReactions(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*IssueUserReaction, error) { var reactions []*IssueUserReaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/user-reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/user-reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -149,7 +148,7 @@ func (c *Client) ListPullRequestUserReactions(ctx context.Context, owner, repo s // GET /repos/{owner}/{repo}/pulls/comment/{comment_id}/user-reactions func (c *Client) ListPullRequestCommentUserReactions(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*IssueUserReaction, error) { var reactions []*IssueUserReaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/comment/%d/user-reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/comment/%d/user-reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -162,7 +161,7 @@ func (c *Client) ListPullRequestCommentUserReactions(ctx context.Context, owner, // // POST /repos/{owner}/{repo}/pulls/{number}/refresh-position func (c *Client) RefreshPullRequestCommentPosition(ctx context.Context, owner, repo string, number int) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/refresh-position", owner, repo, number), nil, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/refresh-position", owner, repo, number), nil, nil) } // --- PR Files JSON --- @@ -186,7 +185,7 @@ type PullRequestFileChange struct { // GET /repos/{owner}/{repo}/pulls/{number}/files.json func (c *Client) ListPullRequestFilesJSON(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*PullRequestFileChange, error) { var files []*PullRequestFileChange - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/files.json?%s", owner, repo, number, opts.toQuery()), nil, &files) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/files.json?%s", owner, repo, number, opts.toQuery()), nil, &files) if err != nil { return nil, err } diff --git a/releases_enhanced.go b/releases_enhanced.go index 64b138d..8224762 100644 --- a/releases_enhanced.go +++ b/releases_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -11,7 +10,7 @@ import ( // GET /repos/{owner}/{repo}/releases/latest func (c *Client) GetLatestRelease(ctx context.Context, owner, repo string) (*Release, error) { var release Release - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/latest", owner, repo), nil, &release) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/latest", owner, repo), nil, &release) if err != nil { return nil, err } @@ -33,7 +32,7 @@ func (c *Client) GetReleaseUploadURL(ctx context.Context, owner, repo string, re if label != "" { body["label"] = label } - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/releases/%d/assets/upload_url", owner, repo, releaseID), body, &result) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/releases/%d/assets/upload_url", owner, repo, releaseID), body, &result) if err != nil { return nil, err } @@ -44,5 +43,5 @@ func (c *Client) GetReleaseUploadURL(ctx context.Context, owner, repo string, re // // GET /repos/{owner}/{repo}/releases/assets/{id} func (c *Client) DownloadReleaseAsset(ctx context.Context, owner, repo string, assetID int64) ([]byte, error) { - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases/assets/%d", owner, repo, assetID)) } diff --git a/repos.go b/repos.go index a12d469..1a35c7e 100644 --- a/repos.go +++ b/repos.go @@ -2,8 +2,8 @@ package gitcode import ( "context" - "fmt" "net/http" + "net/url" "time" ) @@ -57,11 +57,30 @@ type ListRepositoriesOptions struct { Sort string `json:"sort,omitempty"` } +// toQuery renders the pagination plus the type/sort filter parameters. +// +// It is defined on ListRepositoriesOptions (shadowing the embedded +// ListOptions.toQuery) so that every consumer of the struct — +// ListRepositories and ListCurrentUserRepositories, which hit the same +// /user/repos endpoint — builds the identical query. ListRepositories used +// to drop Type/Sort silently while its sibling sent them (issue #10); +// funneling both through this method keeps them from diverging again. +func (o ListRepositoriesOptions) toQuery() string { + q := o.ListOptions.toQuery() + if o.Type != "" { + q += "&type=" + url.QueryEscape(o.Type) + } + if o.Sort != "" { + q += "&sort=" + url.QueryEscape(o.Sort) + } + return q +} + func (c *Client) ListRepositories(ctx context.Context, opts ListRepositoriesOptions) ([]*Repository, error) { var repos []*Repository - path := fmt.Sprintf("/user/repos?%s", opts.toQuery()) + path := pathf("/user/repos?%s", opts.toQuery()) if opts.Owner != "" { - path = fmt.Sprintf("/users/%s/repos?%s", opts.Owner, opts.toQuery()) + path = pathf("/users/%s/repos?%s", opts.Owner, opts.toQuery()) } err := c.doRequest(ctx, http.MethodGet, path, nil, &repos) if err != nil { @@ -72,7 +91,7 @@ func (c *Client) ListRepositories(ctx context.Context, opts ListRepositoriesOpti func (c *Client) GetRepository(ctx context.Context, owner, repo string) (*Repository, error) { var r Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s", owner, repo), nil, &r) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s", owner, repo), nil, &r) if err != nil { return nil, err } @@ -90,7 +109,7 @@ func (c *Client) CreateRepository(ctx context.Context, opts CreateRepositoryOpti func (c *Client) UpdateRepository(ctx context.Context, owner, repo string, opts UpdateRepositoryOptions) (*Repository, error) { var r Repository - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s", owner, repo), opts, &r) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s", owner, repo), opts, &r) if err != nil { return nil, err } @@ -98,12 +117,12 @@ func (c *Client) UpdateRepository(ctx context.Context, owner, repo string, opts } func (c *Client) DeleteRepository(ctx context.Context, owner, repo string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s", owner, repo), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s", owner, repo), nil, nil) } func (c *Client) ForkRepository(ctx context.Context, owner, repo string, opts *CreateRepositoryOptions) (*Repository, error) { var r Repository - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/forks", owner, repo), opts, &r) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/forks", owner, repo), opts, &r) if err != nil { return nil, err } @@ -126,11 +145,11 @@ type RepositoryContent struct { func (c *Client) GetRepositoryContent(ctx context.Context, owner, repo, path, ref string) (*RepositoryContent, error) { var content RepositoryContent - url := fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path) + reqPath := pathf("/repos/%s/%s/contents/%s", owner, repo, pathSegs(path)) if ref != "" { - url += "?ref=" + ref + reqPath += "?ref=" + url.QueryEscape(ref) } - err := c.doRequest(ctx, http.MethodGet, url, nil, &content) + err := c.doRequest(ctx, http.MethodGet, reqPath, nil, &content) if err != nil { return nil, err } @@ -139,11 +158,11 @@ func (c *Client) GetRepositoryContent(ctx context.Context, owner, repo, path, re func (c *Client) ListRepositoryContents(ctx context.Context, owner, repo, path, ref string) ([]*RepositoryContent, error) { var contents []*RepositoryContent - url := fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path) + reqPath := pathf("/repos/%s/%s/contents/%s", owner, repo, pathSegs(path)) if ref != "" { - url += "?ref=" + ref + reqPath += "?ref=" + url.QueryEscape(ref) } - err := c.doRequest(ctx, http.MethodGet, url, nil, &contents) + err := c.doRequest(ctx, http.MethodGet, reqPath, nil, &contents) if err != nil { return nil, err } @@ -176,7 +195,7 @@ type FileResult struct { func (c *Client) CreateFile(ctx context.Context, owner, repo, path string, opts CreateFileOptions) (*FileResult, error) { var result FileResult - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path), opts, &result) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/contents/%s", owner, repo, pathSegs(path)), opts, &result) if err != nil { return nil, err } @@ -185,7 +204,7 @@ func (c *Client) CreateFile(ctx context.Context, owner, repo, path string, opts func (c *Client) UpdateFile(ctx context.Context, owner, repo, path string, opts UpdateFileOptions) (*FileResult, error) { var result FileResult - err := c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path), opts, &result) + err := c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/contents/%s", owner, repo, pathSegs(path)), opts, &result) if err != nil { return nil, err } @@ -194,7 +213,7 @@ func (c *Client) UpdateFile(ctx context.Context, owner, repo, path string, opts func (c *Client) DeleteFile(ctx context.Context, owner, repo, path string, opts DeleteFileOptions) (*FileResult, error) { var result FileResult - err := c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/contents/%s", owner, repo, path), opts, &result) + err := c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/contents/%s", owner, repo, pathSegs(path)), opts, &result) if err != nil { return nil, err } @@ -210,7 +229,7 @@ type Tag struct { func (c *Client) ListTags(ctx context.Context, owner, repo string) ([]*Tag, error) { var tags []*Tag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/tags?per_page=100", owner, repo), nil, &tags) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/tags?per_page=100", owner, repo), nil, &tags) if err != nil { return nil, err } @@ -241,7 +260,7 @@ type CreateReleaseOptions struct { func (c *Client) ListReleases(ctx context.Context, owner, repo string) ([]*Release, error) { var releases []*Release - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/releases?per_page=100", owner, repo), nil, &releases) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/releases?per_page=100", owner, repo), nil, &releases) if err != nil { return nil, err } @@ -250,7 +269,7 @@ func (c *Client) ListReleases(ctx context.Context, owner, repo string) ([]*Relea func (c *Client) CreateRelease(ctx context.Context, owner, repo string, opts CreateReleaseOptions) (*Release, error) { var r Release - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/releases", owner, repo), opts, &r) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/releases", owner, repo), opts, &r) if err != nil { return nil, err } @@ -258,7 +277,7 @@ func (c *Client) CreateRelease(ctx context.Context, owner, repo string, opts Cre } func (c *Client) DeleteRelease(ctx context.Context, owner, repo string, tagName string) error { - err := c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/releases/tags/%s", owner, repo, tagName), nil, nil) + err := c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/releases/tags/%s", owner, repo, tagName), nil, nil) if err == nil { return nil } @@ -266,7 +285,7 @@ func (c *Client) DeleteRelease(ctx context.Context, owner, repo string, tagName } func (c *Client) DeleteTag(ctx context.Context, owner, repo, tagName string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/tags/%s", owner, repo, tagName), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/tags/%s", owner, repo, tagName), nil, nil) } type Contributor struct { @@ -278,7 +297,7 @@ type Contributor struct { func (c *Client) ListContributors(ctx context.Context, owner, repo string) ([]*Contributor, error) { var contributors []*Contributor - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/contributors?per_page=100", owner, repo), nil, &contributors) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/contributors?per_page=100", owner, repo), nil, &contributors) if err != nil { return nil, err } @@ -289,7 +308,7 @@ type Language map[string]int func (c *Client) GetLanguages(ctx context.Context, owner, repo string) (Language, error) { var lang Language - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/languages", owner, repo), nil, &lang) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/languages", owner, repo), nil, &lang) if err != nil { return nil, err } @@ -297,11 +316,11 @@ func (c *Client) GetLanguages(ctx context.Context, owner, repo string) (Language } func (c *Client) GetRawFile(ctx context.Context, owner, repo, path, ref string) ([]byte, error) { - url := fmt.Sprintf("/repos/%s/%s/raw/%s", owner, repo, path) + reqPath := pathf("/repos/%s/%s/raw/%s", owner, repo, pathSegs(path)) if ref != "" { - url += "?ref=" + ref + reqPath += "?ref=" + url.QueryEscape(ref) } - return c.doRawRequest(ctx, http.MethodGet, url) + return c.doRawRequest(ctx, http.MethodGet, reqPath) } type GitTree struct { @@ -322,7 +341,7 @@ type GitTreeEntry struct { func (c *Client) GetTree(ctx context.Context, owner, repo, sha string, recursive bool) (*GitTree, error) { var tree GitTree - url := fmt.Sprintf("/repos/%s/%s/git/trees/%s", owner, repo, sha) + url := pathf("/repos/%s/%s/git/trees/%s", owner, repo, sha) if recursive { url += "?recursive=1" } @@ -343,7 +362,7 @@ type GitBlob struct { func (c *Client) GetBlob(ctx context.Context, owner, repo, sha string) (*GitBlob, error) { var blob GitBlob - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/git/blobs/%s", owner, repo, sha), nil, &blob) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/git/blobs/%s", owner, repo, sha), nil, &blob) if err != nil { return nil, err } @@ -352,7 +371,7 @@ func (c *Client) GetBlob(ctx context.Context, owner, repo, sha string) (*GitBlob func (c *Client) ListForks(ctx context.Context, owner, repo string, opts ListOptions) ([]*Repository, error) { var forks []*Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/forks?%s", owner, repo, opts.toQuery()), nil, &forks) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/forks?%s", owner, repo, opts.toQuery()), nil, &forks) if err != nil { return nil, err } @@ -361,7 +380,7 @@ func (c *Client) ListForks(ctx context.Context, owner, repo string, opts ListOpt func (c *Client) ListWatchers(ctx context.Context, owner, repo string, opts ListOptions) ([]*User, error) { var watchers []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/subscribers?%s", owner, repo, opts.toQuery()), nil, &watchers) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/subscribers?%s", owner, repo, opts.toQuery()), nil, &watchers) if err != nil { return nil, err } @@ -370,7 +389,7 @@ func (c *Client) ListWatchers(ctx context.Context, owner, repo string, opts List func (c *Client) ListStargazers(ctx context.Context, owner, repo string, opts ListOptions) ([]*User, error) { var stargazers []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/stargazers?%s", owner, repo, opts.toQuery()), nil, &stargazers) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/stargazers?%s", owner, repo, opts.toQuery()), nil, &stargazers) if err != nil { return nil, err } @@ -390,7 +409,7 @@ type ContributorStatistic struct { func (c *Client) GetContributorStatistics(ctx context.Context, owner, repo string) ([]*ContributorStatistic, error) { var stats []*ContributorStatistic - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/contributors/statistic", owner, repo), nil, &stats) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/contributors/statistic", owner, repo), nil, &stats) if err != nil { return nil, err } @@ -408,7 +427,7 @@ type RepoEvent struct { func (c *Client) ListRepoEvents(ctx context.Context, owner, repo string, opts ListOptions) ([]*RepoEvent, error) { var events []*RepoEvent - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/events?%s", owner, repo, opts.toQuery()), nil, &events) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/events?%s", owner, repo, opts.toQuery()), nil, &events) if err != nil { return nil, err } @@ -428,7 +447,7 @@ type RepoSettings struct { func (c *Client) GetRepoSettings(ctx context.Context, owner, repo string) (*RepoSettings, error) { var settings RepoSettings - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/repo_settings", owner, repo), nil, &settings) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/repo_settings", owner, repo), nil, &settings) if err != nil { return nil, err } @@ -437,7 +456,7 @@ func (c *Client) GetRepoSettings(ctx context.Context, owner, repo string) (*Repo func (c *Client) UpdateRepoSettings(ctx context.Context, owner, repo string, opts *RepoSettings) (*RepoSettings, error) { var settings RepoSettings - err := c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/repo_settings", owner, repo), opts, &settings) + err := c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/repo_settings", owner, repo), opts, &settings) if err != nil { return nil, err } @@ -462,7 +481,7 @@ type CreateOrgRepoOptions struct { func (c *Client) CreateOrgRepository(ctx context.Context, org string, opts CreateOrgRepoOptions) (*Repository, error) { var r Repository - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/orgs/%s/repos", org), opts, &r) + err := c.doRequest(ctx, http.MethodPost, pathf("/orgs/%s/repos", org), opts, &r) if err != nil { return nil, err } @@ -475,7 +494,7 @@ func (c *Client) ListOrgRepositories(ctx context.Context, org, repoType string, if repoType != "" { query += "&type=" + repoType } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/repos?%s", org, query), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/repos?%s", org, query), nil, &repos) if err != nil { return nil, err } @@ -487,7 +506,7 @@ type TransferRepoOptions struct { } func (c *Client) TransferRepository(ctx context.Context, owner, repo string, opts TransferRepoOptions) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/transfer", owner, repo), opts, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/transfer", owner, repo), opts, nil) } type ArchiveStatus struct { @@ -496,7 +515,7 @@ type ArchiveStatus struct { func (c *Client) GetArchiveStatus(ctx context.Context, owner, repo string) (*ArchiveStatus, error) { var status ArchiveStatus - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/transition", owner, repo), nil, &status) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/transition", owner, repo), nil, &status) if err != nil { return nil, err } @@ -504,7 +523,7 @@ func (c *Client) GetArchiveStatus(ctx context.Context, owner, repo string) (*Arc } func (c *Client) ArchiveRepository(ctx context.Context, owner, repo string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/transition", owner, repo), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/transition", owner, repo), nil, nil) } type UpdateMemberOptions struct { @@ -512,7 +531,7 @@ type UpdateMemberOptions struct { } func (c *Client) UpdateRepoMember(ctx context.Context, owner, repo, username string, opts UpdateMemberOptions) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/members/%s", owner, repo, username), opts, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/members/%s", owner, repo, username), opts, nil) } type FileUploadResult struct { @@ -522,7 +541,7 @@ type FileUploadResult struct { func (c *Client) UploadFile(ctx context.Context, owner, repo string, filePath string) (*FileUploadResult, error) { var result FileUploadResult body := map[string]string{"file": filePath} - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/file/upload", owner, repo), body, &result) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/file/upload", owner, repo), body, &result) if err != nil { return nil, err } @@ -532,7 +551,7 @@ func (c *Client) UploadFile(ctx context.Context, owner, repo string, filePath st func (c *Client) UploadImage(ctx context.Context, owner, repo string, filePath string) (*FileUploadResult, error) { var result FileUploadResult body := map[string]string{"file": filePath} - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/img/upload", owner, repo), body, &result) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/img/upload", owner, repo), body, &result) if err != nil { return nil, err } @@ -547,7 +566,7 @@ type PushConfig struct { func (c *Client) GetPushConfig(ctx context.Context, owner, repo string) (*PushConfig, error) { var config PushConfig - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/push_config", owner, repo), nil, &config) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/push_config", owner, repo), nil, &config) if err != nil { return nil, err } @@ -555,7 +574,7 @@ func (c *Client) GetPushConfig(ctx context.Context, owner, repo string) (*PushCo } func (c *Client) UpdatePushConfig(ctx context.Context, owner, repo string, config *PushConfig) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/push_config", owner, repo), config, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/push_config", owner, repo), config, nil) } type PRSettings struct { @@ -565,7 +584,7 @@ type PRSettings struct { func (c *Client) GetPRSettings(ctx context.Context, owner, repo string) (*PRSettings, error) { var settings PRSettings - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pull_request_settings", owner, repo), nil, &settings) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pull_request_settings", owner, repo), nil, &settings) if err != nil { return nil, err } @@ -573,7 +592,7 @@ func (c *Client) GetPRSettings(ctx context.Context, owner, repo string) (*PRSett } func (c *Client) UpdatePRSettings(ctx context.Context, owner, repo string, settings *PRSettings) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/pull_request_settings", owner, repo), settings, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/pull_request_settings", owner, repo), settings, nil) } type FileListEntry struct { @@ -584,7 +603,7 @@ type FileListEntry struct { func (c *Client) ListFiles(ctx context.Context, owner, repo string) ([]*FileListEntry, error) { var files []*FileListEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/file_list", owner, repo), nil, &files) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/file_list", owner, repo), nil, &files) if err != nil { return nil, err } @@ -598,7 +617,7 @@ type ModuleSetting struct { } func (c *Client) SetModuleSetting(ctx context.Context, owner, repo string, setting ModuleSetting) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/module/setting", owner, repo), setting, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/module/setting", owner, repo), setting, nil) } type CustomizedRole struct { @@ -609,7 +628,7 @@ type CustomizedRole struct { func (c *Client) GetCustomizedRoles(ctx context.Context, owner, repo string) ([]*CustomizedRole, error) { var roles []*CustomizedRole - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/customized_roles", owner, repo), nil, &roles) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/customized_roles", owner, repo), nil, &roles) if err != nil { return nil, err } @@ -622,7 +641,7 @@ type ReviewerConfig struct { } func (c *Client) UpdateReviewerConfig(ctx context.Context, owner, repo string, config ReviewerConfig) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/reviewer", owner, repo), config, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/reviewer", owner, repo), config, nil) } type DownloadStatistic struct { @@ -632,7 +651,7 @@ type DownloadStatistic struct { func (c *Client) GetDownloadStatistics(ctx context.Context, owner, repo string) ([]*DownloadStatistic, error) { var stats []*DownloadStatistic - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/download_statistics", owner, repo), nil, &stats) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/download_statistics", owner, repo), nil, &stats) if err != nil { return nil, err } @@ -640,9 +659,9 @@ func (c *Client) GetDownloadStatistics(ctx context.Context, owner, repo string) } func (c *Client) UpdateOrgRepoStatus(ctx context.Context, org, repo, status string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/org/%s/repo/%s/status", org, repo), map[string]string{"status": status}, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/org/%s/repo/%s/status", org, repo), map[string]string{"status": status}, nil) } func (c *Client) TransferToOrg(ctx context.Context, org, repo, newOwner string) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/org/%s/projects/%s/transfer", org, repo), map[string]string{"new_owner": newOwner}, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/org/%s/projects/%s/transfer", org, repo), map[string]string{"new_owner": newOwner}, nil) } diff --git a/repos_archive.go b/repos_archive.go index 899e171..25d8c32 100644 --- a/repos_archive.go +++ b/repos_archive.go @@ -2,8 +2,8 @@ package gitcode import ( "context" - "fmt" "net/http" + "net/url" ) // GetRepositoryArchive downloads a repository archive (tar.gz, zip, etc.). @@ -11,33 +11,33 @@ import ( // GET /repos/{owner}/{repo}/archive/{archive} func (c *Client) GetRepositoryArchive(ctx context.Context, owner, repo, archive string) ([]byte, error) { // archive can be: master.tar.gz, main.zip, v1.0.0.tar.gz, etc. - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/archive/%s", owner, repo, archive)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/archive/%s", owner, repo, archive)) } // DownloadRepository downloads a repository as a zip or tarball. // // GET /repos/{owner}/{repo}/{format} func (c *Client) DownloadRepository(ctx context.Context, owner, repo, format string) ([]byte, error) { - // format can be: repository/archive.zip, repository/archive.tar.gz - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/%s", owner, repo, format)) + // format can be: repository/archive.zip, repository/archive.tar.gz — + // slash-bearing, so it goes through pathSegs (per-segment escaping that + // keeps the separators). + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/%s", owner, repo, pathSegs(format))) } // GenerateRepositoryArchive generates a repository archive. // // GET /repos/{owner}/{repo}/archive func (c *Client) GenerateRepositoryArchive(ctx context.Context, owner, repo, ref, format string) ([]byte, error) { - path := fmt.Sprintf("/repos/%s/%s/archive", owner, repo) - if ref != "" || format != "" { - path += "?" - if ref != "" { - path += "ref=" + ref - } - if format != "" { - if ref != "" { - path += "&" - } - path += "format=" + format - } + path := pathf("/repos/%s/%s/archive", owner, repo) + q := url.Values{} + if ref != "" { + q.Set("ref", ref) + } + if format != "" { + q.Set("format", format) + } + if len(q) > 0 { + path += "?" + q.Encode() } return c.doRawRequest(ctx, http.MethodGet, path) } @@ -58,7 +58,7 @@ type ParticipationStats struct { // GET /repos/{owner}/{repo}/stats/participation func (c *Client) GetRepoParticipation(ctx context.Context, owner, repo string) (*ParticipationStats, error) { var stats ParticipationStats - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/stats/participation", owner, repo), nil, &stats) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/stats/participation", owner, repo), nil, &stats) if err != nil { return nil, err } @@ -73,7 +73,7 @@ type CodeFrequencyEntry []int // GET /repos/{owner}/{repo}/stats/code_frequency func (c *Client) GetRepoCodeFrequency(ctx context.Context, owner, repo string) ([]CodeFrequencyEntry, error) { var entries []CodeFrequencyEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/stats/code_frequency", owner, repo), nil, &entries) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/stats/code_frequency", owner, repo), nil, &entries) if err != nil { return nil, err } @@ -92,7 +92,7 @@ type CommitActivity struct { // GET /repos/{owner}/{repo}/stats/commit_activity func (c *Client) GetRepoCommitActivity(ctx context.Context, owner, repo string) ([]*CommitActivity, error) { var activity []*CommitActivity - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/stats/commit_activity", owner, repo), nil, &activity) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/stats/commit_activity", owner, repo), nil, &activity) if err != nil { return nil, err } @@ -107,7 +107,7 @@ type PunchCardEntry []int // GET /repos/{owner}/{repo}/stats/punch_card func (c *Client) GetRepoPunchCard(ctx context.Context, owner, repo string) ([]PunchCardEntry, error) { var entries []PunchCardEntry - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/stats/punch_card", owner, repo), nil, &entries) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/stats/punch_card", owner, repo), nil, &entries) if err != nil { return nil, err } diff --git a/repos_assignees.go b/repos_assignees.go index 277d6e2..0e16dcc 100644 --- a/repos_assignees.go +++ b/repos_assignees.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -11,7 +10,7 @@ import ( // GET /repos/{owner}/{repo}/assignees func (c *Client) ListRepoAssignees(ctx context.Context, owner, repo string, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/assignees?%s", owner, repo, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/assignees?%s", owner, repo, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -22,7 +21,7 @@ func (c *Client) ListRepoAssignees(ctx context.Context, owner, repo string, opts // // GET /repos/{owner}/{repo}/assignees/{username} func (c *Client) IsRepoAssignee(ctx context.Context, owner, repo, username string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/assignees/%s", owner, repo, username)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/assignees/%s", owner, repo, username)) if err != nil { if isNotFoundError(err) { return false, nil @@ -42,7 +41,7 @@ type IssueAssigneeResult struct { // POST /repos/{owner}/{repo}/issues/{index}/assignees func (c *Client) AddIssueAssignees(ctx context.Context, owner, repo string, number int, assignees []string) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/%d/assignees", owner, repo, number), map[string]interface{}{"assignees": assignees}, &issue) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/%d/assignees", owner, repo, number), map[string]interface{}{"assignees": assignees}, &issue) if err != nil { return nil, err } @@ -54,7 +53,7 @@ func (c *Client) AddIssueAssignees(ctx context.Context, owner, repo string, numb // DELETE /repos/{owner}/{repo}/issues/{index}/assignees func (c *Client) RemoveIssueAssignees(ctx context.Context, owner, repo string, number int, assignees []string) (*Issue, error) { var issue Issue - err := c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/assignees", owner, repo, number), map[string]interface{}{"assignees": assignees}, &issue) + err := c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/assignees", owner, repo, number), map[string]interface{}{"assignees": assignees}, &issue) if err != nil { return nil, err } @@ -76,7 +75,7 @@ type RepoBranchInfo struct { // GET /repos/{owner}/{repo}/branches func (c *Client) ListRepoBranchesPaginated(ctx context.Context, owner, repo string, opts ListOptions) ([]*RepoBranchInfo, error) { var branches []*RepoBranchInfo - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/branches?%s", owner, repo, opts.toQuery()), nil, &branches) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/branches?%s", owner, repo, opts.toQuery()), nil, &branches) if err != nil { return nil, err } @@ -88,7 +87,7 @@ func (c *Client) ListRepoBranchesPaginated(ctx context.Context, owner, repo stri // GET /repos/{owner}/{repo}/branches/{branch} func (c *Client) GetRepoBranchDetail(ctx context.Context, owner, repo, branch string) (*RepoBranchInfo, error) { var b RepoBranchInfo - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, &b) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/branches/%s", owner, repo, branch), nil, &b) if err != nil { return nil, err } diff --git a/repos_collaborators.go b/repos_collaborators.go index 2506129..baa865a 100644 --- a/repos_collaborators.go +++ b/repos_collaborators.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -32,7 +31,7 @@ type AddCollaboratorOptions struct { // GET /repos/{owner}/{repo}/collaborators func (c *Client) ListCollaborators(ctx context.Context, owner, repo string, opts ListOptions) ([]*Collaborator, error) { var collaborators []*Collaborator - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/collaborators?%s", owner, repo, opts.toQuery()), nil, &collaborators) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/collaborators?%s", owner, repo, opts.toQuery()), nil, &collaborators) if err != nil { return nil, err } @@ -43,7 +42,7 @@ func (c *Client) ListCollaborators(ctx context.Context, owner, repo string, opts // // GET /repos/{owner}/{repo}/collaborators/{username} func (c *Client) IsCollaborator(ctx context.Context, owner, repo, username string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/collaborators/%s", owner, repo, username)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/collaborators/%s", owner, repo, username)) if err != nil { if isNotFoundError(err) { return false, nil @@ -57,14 +56,14 @@ func (c *Client) IsCollaborator(ctx context.Context, owner, repo, username strin // // PUT /repos/{owner}/{repo}/collaborators/{username} func (c *Client) AddCollaborator(ctx context.Context, owner, repo, username string, opts *AddCollaboratorOptions) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/collaborators/%s", owner, repo, username), opts, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/collaborators/%s", owner, repo, username), opts, nil) } // RemoveCollaborator removes a collaborator from a repository. // // DELETE /repos/{owner}/{repo}/collaborators/{username} func (c *Client) RemoveCollaborator(ctx context.Context, owner, repo, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/collaborators/%s", owner, repo, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/collaborators/%s", owner, repo, username), nil, nil) } // GetCollaboratorPermission gets the permission of a collaborator for a repository. @@ -72,7 +71,7 @@ func (c *Client) RemoveCollaborator(ctx context.Context, owner, repo, username s // GET /repos/{owner}/{repo}/collaborators/{username}/permission func (c *Client) GetCollaboratorPermission(ctx context.Context, owner, repo, username string) (*CollaboratorPermission, error) { var perm CollaboratorPermission - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/collaborators/%s/permission", owner, repo, username), nil, &perm) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/collaborators/%s/permission", owner, repo, username), nil, &perm) if err != nil { return nil, err } diff --git a/repos_commits_comments.go b/repos_commits_comments.go index 62cfc42..66af5ea 100644 --- a/repos_commits_comments.go +++ b/repos_commits_comments.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -40,7 +39,7 @@ type UpdateCommitCommentOptions struct { // GET /repos/{owner}/{repo}/commits/{sha}/comments func (c *Client) ListCommitComments(ctx context.Context, owner, repo, sha string, opts ListOptions) ([]*CommitComment, error) { var comments []*CommitComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits/%s/comments?%s", owner, repo, sha, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits/%s/comments?%s", owner, repo, sha, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -52,7 +51,7 @@ func (c *Client) ListCommitComments(ctx context.Context, owner, repo, sha string // GET /repos/{owner}/{repo}/comments func (c *Client) ListRepoCommitComments(ctx context.Context, owner, repo string, opts ListOptions) ([]*CommitComment, error) { var comments []*CommitComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/comments?%s", owner, repo, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/comments?%s", owner, repo, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -64,7 +63,7 @@ func (c *Client) ListRepoCommitComments(ctx context.Context, owner, repo string, // GET /repos/{owner}/{repo}/comments/{id} func (c *Client) GetCommitComment(ctx context.Context, owner, repo string, commentID int64) (*CommitComment, error) { var comment CommitComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/comments/%d", owner, repo, commentID), nil, &comment) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/comments/%d", owner, repo, commentID), nil, &comment) if err != nil { return nil, err } @@ -76,7 +75,7 @@ func (c *Client) GetCommitComment(ctx context.Context, owner, repo string, comme // POST /repos/{owner}/{repo}/commits/{sha}/comments func (c *Client) CreateCommitComment(ctx context.Context, owner, repo, sha string, opts CreateCommitCommentOptions) (*CommitComment, error) { var comment CommitComment - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/commits/%s/comments", owner, repo, sha), opts, &comment) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/commits/%s/comments", owner, repo, sha), opts, &comment) if err != nil { return nil, err } @@ -88,7 +87,7 @@ func (c *Client) CreateCommitComment(ctx context.Context, owner, repo, sha strin // PATCH /repos/{owner}/{repo}/comments/{id} func (c *Client) UpdateCommitComment(ctx context.Context, owner, repo string, commentID int64, opts UpdateCommitCommentOptions) (*CommitComment, error) { var comment CommitComment - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/comments/%d", owner, repo, commentID), opts, &comment) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/comments/%d", owner, repo, commentID), opts, &comment) if err != nil { return nil, err } @@ -99,5 +98,5 @@ func (c *Client) UpdateCommitComment(ctx context.Context, owner, repo string, co // // DELETE /repos/{owner}/{repo}/comments/{id} func (c *Client) DeleteCommitComment(ctx context.Context, owner, repo string, commentID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/comments/%d", owner, repo, commentID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/comments/%d", owner, repo, commentID), nil, nil) } diff --git a/repos_deploy_keys.go b/repos_deploy_keys.go index 284cd7d..7c191d5 100644 --- a/repos_deploy_keys.go +++ b/repos_deploy_keys.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -29,7 +28,7 @@ type CreateDeployKeyOptions struct { // GET /repos/{owner}/{repo}/keys func (c *Client) ListDeployKeys(ctx context.Context, owner, repo string, opts ListOptions) ([]*DeployKey, error) { var keys []*DeployKey - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/keys?%s", owner, repo, opts.toQuery()), nil, &keys) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/keys?%s", owner, repo, opts.toQuery()), nil, &keys) if err != nil { return nil, err } @@ -41,7 +40,7 @@ func (c *Client) ListDeployKeys(ctx context.Context, owner, repo string, opts Li // GET /repos/{owner}/{repo}/keys/{id} func (c *Client) GetDeployKey(ctx context.Context, owner, repo string, keyID int64) (*DeployKey, error) { var key DeployKey - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/keys/%d", owner, repo, keyID), nil, &key) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/keys/%d", owner, repo, keyID), nil, &key) if err != nil { return nil, err } @@ -53,7 +52,7 @@ func (c *Client) GetDeployKey(ctx context.Context, owner, repo string, keyID int // POST /repos/{owner}/{repo}/keys func (c *Client) CreateDeployKey(ctx context.Context, owner, repo string, opts CreateDeployKeyOptions) (*DeployKey, error) { var key DeployKey - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/keys", owner, repo), opts, &key) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/keys", owner, repo), opts, &key) if err != nil { return nil, err } @@ -64,5 +63,5 @@ func (c *Client) CreateDeployKey(ctx context.Context, owner, repo string, opts C // // DELETE /repos/{owner}/{repo}/keys/{id} func (c *Client) DeleteDeployKey(ctx context.Context, owner, repo string, keyID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/keys/%d", owner, repo, keyID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/keys/%d", owner, repo, keyID), nil, nil) } diff --git a/repos_discussions.go b/repos_discussions.go index 7aeb469..7cab0f3 100644 --- a/repos_discussions.go +++ b/repos_discussions.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -48,7 +47,7 @@ type DiscussionCommentReply struct { // GET /repos/{owner}/{repo}/discuss func (c *Client) ListDiscussions(ctx context.Context, owner, repo string, opts ListOptions) ([]*Discussion, error) { var discussions []*Discussion - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/discuss?%s", owner, repo, opts.toQuery()), nil, &discussions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/discuss?%s", owner, repo, opts.toQuery()), nil, &discussions) if err != nil { return nil, err } @@ -60,7 +59,7 @@ func (c *Client) ListDiscussions(ctx context.Context, owner, repo string, opts L // GET /repos/{owner}/{repo}/discuss/{number} func (c *Client) GetDiscussion(ctx context.Context, owner, repo string, number int) (*Discussion, error) { var discussion Discussion - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/discuss/%d", owner, repo, number), nil, &discussion) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/discuss/%d", owner, repo, number), nil, &discussion) if err != nil { return nil, err } @@ -72,7 +71,7 @@ func (c *Client) GetDiscussion(ctx context.Context, owner, repo string, number i // GET /repos/{owner}/{repo}/discuss/{number}/comment func (c *Client) ListDiscussionComments(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*DiscussionComment, error) { var comments []*DiscussionComment - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/discuss/%d/comment?%s", owner, repo, number, opts.toQuery()), nil, &comments) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/discuss/%d/comment?%s", owner, repo, number, opts.toQuery()), nil, &comments) if err != nil { return nil, err } @@ -84,7 +83,7 @@ func (c *Client) ListDiscussionComments(ctx context.Context, owner, repo string, // GET /repos/{owner}/{repo}/discuss/{number}/comment/{comment_id}/reply func (c *Client) ListDiscussionCommentReplies(ctx context.Context, owner, repo string, number int, commentID int64, opts ListOptions) ([]*DiscussionCommentReply, error) { var replies []*DiscussionCommentReply - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/discuss/%d/comment/%d/reply?%s", owner, repo, number, commentID, opts.toQuery()), nil, &replies) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/discuss/%d/comment/%d/reply?%s", owner, repo, number, commentID, opts.toQuery()), nil, &replies) if err != nil { return nil, err } @@ -106,7 +105,7 @@ type ForkSyncStatus struct { // GET /repos/{owner}/{repo}/sync-repo func (c *Client) GetForkSyncStatus(ctx context.Context, owner, repo string) (*ForkSyncStatus, error) { var status ForkSyncStatus - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/sync-repo", owner, repo), nil, &status) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/sync-repo", owner, repo), nil, &status) if err != nil { return nil, err } @@ -117,7 +116,7 @@ func (c *Client) GetForkSyncStatus(ctx context.Context, owner, repo string) (*Fo // // PUT /repos/{owner}/{repo}/sync-repo func (c *Client) SyncForkRepository(ctx context.Context, owner, repo string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/sync-repo", owner, repo), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/sync-repo", owner, repo), nil, nil) } // --- Remote Mirrors --- @@ -137,7 +136,7 @@ type RemoteMirror struct { // GET /repos/{owner}/{repo}/repo-remote-mirror func (c *Client) GetRepoRemoteMirror(ctx context.Context, owner, repo string) (*RemoteMirror, error) { var mirror RemoteMirror - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/repo-remote-mirror", owner, repo), nil, &mirror) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/repo-remote-mirror", owner, repo), nil, &mirror) if err != nil { return nil, err } @@ -149,7 +148,7 @@ func (c *Client) GetRepoRemoteMirror(ctx context.Context, owner, repo string) (* // GET /repos/{owner}/{repo}/push-remote-mirrors func (c *Client) ListPushRemoteMirrors(ctx context.Context, owner, repo string, opts ListOptions) ([]*RemoteMirror, error) { var mirrors []*RemoteMirror - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/push-remote-mirrors?%s", owner, repo, opts.toQuery()), nil, &mirrors) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/push-remote-mirrors?%s", owner, repo, opts.toQuery()), nil, &mirrors) if err != nil { return nil, err } @@ -172,7 +171,7 @@ type RepoLicense struct { // GET /repos/{owner}/{repo}/license func (c *Client) GetRepoLicense(ctx context.Context, owner, repo string) (*RepoLicense, error) { var license RepoLicense - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/license", owner, repo), nil, &license) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/license", owner, repo), nil, &license) if err != nil { return nil, err } @@ -191,8 +190,8 @@ type RepoCLA struct { // // GET /repos/{owner}/{repo}/clas func (c *Client) ListRepoCLAs(ctx context.Context, owner, repo string) ([]*RepoCLA, error) { - var claList []*RepoCLA //nolint:misspell // "clas" is the API endpoint name - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/clas", owner, repo), nil, &claList) //nolint:misspell // API endpoint + var claList []*RepoCLA //nolint:misspell // "clas" is the API endpoint name + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/clas", owner, repo), nil, &claList) //nolint:misspell // API endpoint if err != nil { return nil, err } @@ -204,7 +203,7 @@ func (c *Client) ListRepoCLAs(ctx context.Context, owner, repo string) ([]*RepoC // PUT /repos/{owner}/{repo}/clas func (c *Client) ConfigureRepoCLA(ctx context.Context, owner, repo string, opts *RepoCLA) (*RepoCLA, error) { var cla RepoCLA - err := c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/clas", owner, repo), opts, &cla) //nolint:misspell // API endpoint + err := c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/clas", owner, repo), opts, &cla) //nolint:misspell // API endpoint if err != nil { return nil, err } diff --git a/repos_invitations.go b/repos_invitations.go index 540bdac..f201224 100644 --- a/repos_invitations.go +++ b/repos_invitations.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -23,14 +22,14 @@ type RepoInvitation struct { // // PATCH /user/repository_invitations/{id} func (c *Client) AcceptRepoInvitation(ctx context.Context, invitationID int64) error { - return c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/user/repository_invitations/%d", invitationID), nil, nil) + return c.doRequest(ctx, http.MethodPatch, pathf("/user/repository_invitations/%d", invitationID), nil, nil) } // DeclineRepoInvitation declines a repository invitation. // // DELETE /user/repository_invitations/{id} func (c *Client) DeclineRepoInvitation(ctx context.Context, invitationID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/user/repository_invitations/%d", invitationID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/user/repository_invitations/%d", invitationID), nil, nil) } // ListPendingRepoInvitations lists pending repository invitations for the authenticated user. @@ -38,7 +37,7 @@ func (c *Client) DeclineRepoInvitation(ctx context.Context, invitationID int64) // GET /user/repository_invitations func (c *Client) ListPendingRepoInvitations(ctx context.Context, opts ListOptions) ([]*RepoInvitation, error) { var invitations []*RepoInvitation - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/repository_invitations?%s", opts.toQuery()), nil, &invitations) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/repository_invitations?%s", opts.toQuery()), nil, &invitations) if err != nil { return nil, err } diff --git a/repos_issues_timeline.go b/repos_issues_timeline.go index fe86667..e75a786 100644 --- a/repos_issues_timeline.go +++ b/repos_issues_timeline.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -31,7 +30,7 @@ type IssueTimelineEvent struct { // GET /repos/{owner}/{repo}/issues/{index}/timeline func (c *Client) ListIssueTimelineEvents(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*IssueTimelineEvent, error) { var events []*IssueTimelineEvent - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/timeline?%s", owner, repo, number, opts.toQuery()), nil, &events) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/timeline?%s", owner, repo, number, opts.toQuery()), nil, &events) if err != nil { return nil, err } @@ -51,7 +50,7 @@ type IssueSubscriber struct { // GET /repos/{owner}/{repo}/issues/{index}/subscribers func (c *Client) ListIssueSubscribers(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*IssueSubscriber, error) { var subscribers []*IssueSubscriber - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/subscribers?%s", owner, repo, number, opts.toQuery()), nil, &subscribers) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/subscribers?%s", owner, repo, number, opts.toQuery()), nil, &subscribers) if err != nil { return nil, err } @@ -62,14 +61,14 @@ func (c *Client) ListIssueSubscribers(ctx context.Context, owner, repo string, n // // PUT /repos/{owner}/{repo}/issues/{index}/subscribers/{username} func (c *Client) SubscribeToIssue(ctx context.Context, owner, repo string, number int, username string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/issues/%d/subscribers/%s", owner, repo, number, username), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/issues/%d/subscribers/%s", owner, repo, number, username), nil, nil) } // UnsubscribeFromIssue unsubscribes the authenticated user from an issue. // // DELETE /repos/{owner}/{repo}/issues/{index}/subscribers/{username} func (c *Client) UnsubscribeFromIssue(ctx context.Context, owner, repo string, number int, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/subscribers/%s", owner, repo, number, username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/subscribers/%s", owner, repo, number, username), nil, nil) } // IssueDependency represents a dependency between issues. @@ -83,7 +82,7 @@ type IssueDependency struct { // GET /repos/{owner}/{repo}/issues/{index}/dependencies func (c *Client) ListIssueDependencies(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*IssueDependency, error) { var deps []*IssueDependency - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/dependencies?%s", owner, repo, number, opts.toQuery()), nil, &deps) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/dependencies?%s", owner, repo, number, opts.toQuery()), nil, &deps) if err != nil { return nil, err } @@ -95,7 +94,7 @@ func (c *Client) ListIssueDependencies(ctx context.Context, owner, repo string, // POST /repos/{owner}/{repo}/issues/{index}/dependencies func (c *Client) CreateIssueDependency(ctx context.Context, owner, repo string, number int, dependsOnNumber int) (*IssueDependency, error) { var dep IssueDependency - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/%d/dependencies", owner, repo, number), map[string]int{"depends_on": dependsOnNumber}, &dep) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/%d/dependencies", owner, repo, number), map[string]int{"depends_on": dependsOnNumber}, &dep) if err != nil { return nil, err } @@ -106,7 +105,7 @@ func (c *Client) CreateIssueDependency(ctx context.Context, owner, repo string, // // DELETE /repos/{owner}/{repo}/issues/{index}/dependencies/{dependency} func (c *Client) DeleteIssueDependency(ctx context.Context, owner, repo string, number int, dependencyNumber int) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/dependencies/%d", owner, repo, number, dependencyNumber), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/dependencies/%d", owner, repo, number, dependencyNumber), nil, nil) } // ListIssueBlockingIssues lists all issues that this issue blocks. @@ -114,7 +113,7 @@ func (c *Client) DeleteIssueDependency(ctx context.Context, owner, repo string, // GET /repos/{owner}/{repo}/issues/{index}/blocks func (c *Client) ListIssueBlockingIssues(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*Issue, error) { var issues []*Issue - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/blocks?%s", owner, repo, number, opts.toQuery()), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/blocks?%s", owner, repo, number, opts.toQuery()), nil, &issues) if err != nil { return nil, err } diff --git a/repos_protected_tags.go b/repos_protected_tags.go index 3d56c24..3fc1b56 100644 --- a/repos_protected_tags.go +++ b/repos_protected_tags.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -33,7 +32,7 @@ type UpdateProtectedTagOptions struct { // GET /repos/{owner}/{repo}/protected-tags func (c *Client) ListProtectedTags(ctx context.Context, owner, repo string, opts ListOptions) ([]*ProtectedTag, error) { var tags []*ProtectedTag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/protected-tags?%s", owner, repo, opts.toQuery()), nil, &tags) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/protected-tags?%s", owner, repo, opts.toQuery()), nil, &tags) if err != nil { return nil, err } @@ -45,7 +44,7 @@ func (c *Client) ListProtectedTags(ctx context.Context, owner, repo string, opts // GET /repos/{owner}/{repo}/protected-tags/{tag} func (c *Client) GetProtectedTag(ctx context.Context, owner, repo, tag string) (*ProtectedTag, error) { var pt ProtectedTag - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), nil, &pt) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), nil, &pt) if err != nil { return nil, err } @@ -57,7 +56,7 @@ func (c *Client) GetProtectedTag(ctx context.Context, owner, repo, tag string) ( // POST /repos/{owner}/{repo}/protected-tags func (c *Client) CreateProtectedTag(ctx context.Context, owner, repo string, opts CreateProtectedTagOptions) (*ProtectedTag, error) { var pt ProtectedTag - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/protected-tags", owner, repo), opts, &pt) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/protected-tags", owner, repo), opts, &pt) if err != nil { return nil, err } @@ -69,7 +68,7 @@ func (c *Client) CreateProtectedTag(ctx context.Context, owner, repo string, opt // PUT /repos/{owner}/{repo}/protected-tags/{tag} func (c *Client) UpdateProtectedTag(ctx context.Context, owner, repo, tag string, opts UpdateProtectedTagOptions) (*ProtectedTag, error) { var pt ProtectedTag - err := c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), opts, &pt) + err := c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), opts, &pt) if err != nil { return nil, err } @@ -80,5 +79,5 @@ func (c *Client) UpdateProtectedTag(ctx context.Context, owner, repo, tag string // // DELETE /repos/{owner}/{repo}/protected-tags/{tag} func (c *Client) DeleteProtectedTag(ctx context.Context, owner, repo, tag string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/protected-tags/%s", owner, repo, tag), nil, nil) } diff --git a/repos_reactions.go b/repos_reactions.go index 2578e43..9c96273 100644 --- a/repos_reactions.go +++ b/repos_reactions.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -39,7 +38,7 @@ type CreateReactionOptions struct { // GET /repos/{owner}/{repo}/issues/{index}/reactions func (c *Client) ListIssueReactions(ctx context.Context, owner, repo string, number int, opts ListOptions) ([]*Reaction, error) { var reactions []*Reaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/%d/reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/%d/reactions?%s", owner, repo, number, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -51,7 +50,7 @@ func (c *Client) ListIssueReactions(ctx context.Context, owner, repo string, num // POST /repos/{owner}/{repo}/issues/{index}/reactions func (c *Client) CreateIssueReaction(ctx context.Context, owner, repo string, number int, content string) (*Reaction, error) { var reaction Reaction - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/%d/reactions", owner, repo, number), CreateReactionOptions{Content: content}, &reaction) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/%d/reactions", owner, repo, number), CreateReactionOptions{Content: content}, &reaction) if err != nil { return nil, err } @@ -62,7 +61,7 @@ func (c *Client) CreateIssueReaction(ctx context.Context, owner, repo string, nu // // DELETE /repos/{owner}/{repo}/issues/{index}/reactions/{id} func (c *Client) DeleteIssueReaction(ctx context.Context, owner, repo string, number int, reactionID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/%d/reactions/%d", owner, repo, number, reactionID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/%d/reactions/%d", owner, repo, number, reactionID), nil, nil) } // --- Issue Comment Reactions --- @@ -72,7 +71,7 @@ func (c *Client) DeleteIssueReaction(ctx context.Context, owner, repo string, nu // GET /repos/{owner}/{repo}/issues/comments/{id}/reactions func (c *Client) ListIssueCommentReactions(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*Reaction, error) { var reactions []*Reaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issues/comments/%d/reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issues/comments/%d/reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -84,7 +83,7 @@ func (c *Client) ListIssueCommentReactions(ctx context.Context, owner, repo stri // POST /repos/{owner}/{repo}/issues/comments/{id}/reactions func (c *Client) CreateIssueCommentReaction(ctx context.Context, owner, repo string, commentID int64, content string) (*Reaction, error) { var reaction Reaction - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/issues/comments/%d/reactions", owner, repo, commentID), CreateReactionOptions{Content: content}, &reaction) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/issues/comments/%d/reactions", owner, repo, commentID), CreateReactionOptions{Content: content}, &reaction) if err != nil { return nil, err } @@ -95,7 +94,7 @@ func (c *Client) CreateIssueCommentReaction(ctx context.Context, owner, repo str // // DELETE /repos/{owner}/{repo}/issues/comments/{id}/reactions/{id} func (c *Client) DeleteIssueCommentReaction(ctx context.Context, owner, repo string, commentID, reactionID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/issues/comments/%d/reactions/%d", owner, repo, commentID, reactionID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/issues/comments/%d/reactions/%d", owner, repo, commentID, reactionID), nil, nil) } // --- Pull Request Comment Reactions --- @@ -105,7 +104,7 @@ func (c *Client) DeleteIssueCommentReaction(ctx context.Context, owner, repo str // GET /repos/{owner}/{repo}/pulls/comments/{id}/reactions func (c *Client) ListPullRequestCommentReactions(ctx context.Context, owner, repo string, commentID int64, opts ListOptions) ([]*Reaction, error) { var reactions []*Reaction - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/comments/%d/reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/comments/%d/reactions?%s", owner, repo, commentID, opts.toQuery()), nil, &reactions) if err != nil { return nil, err } @@ -117,7 +116,7 @@ func (c *Client) ListPullRequestCommentReactions(ctx context.Context, owner, rep // POST /repos/{owner}/{repo}/pulls/comments/{id}/reactions func (c *Client) CreatePullRequestCommentReaction(ctx context.Context, owner, repo string, commentID int64, content string) (*Reaction, error) { var reaction Reaction - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/comments/%d/reactions", owner, repo, commentID), CreateReactionOptions{Content: content}, &reaction) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/comments/%d/reactions", owner, repo, commentID), CreateReactionOptions{Content: content}, &reaction) if err != nil { return nil, err } @@ -128,5 +127,5 @@ func (c *Client) CreatePullRequestCommentReaction(ctx context.Context, owner, re // // DELETE /repos/{owner}/{repo}/pulls/comments/{id}/reactions/{id} func (c *Client) DeletePullRequestCommentReaction(ctx context.Context, owner, repo string, commentID, reactionID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/comments/%d/reactions/%d", owner, repo, commentID, reactionID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/comments/%d/reactions/%d", owner, repo, commentID, reactionID), nil, nil) } diff --git a/repos_reviewers.go b/repos_reviewers.go index d2eb36f..0c72a48 100644 --- a/repos_reviewers.go +++ b/repos_reviewers.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -19,7 +18,7 @@ type Reviewer struct { // GET /repos/{owner}/{repo}/reviewers func (c *Client) ListRepoReviewers(ctx context.Context, owner, repo string, opts ListOptions) ([]*Reviewer, error) { var reviewers []*Reviewer - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/reviewers?%s", owner, repo, opts.toQuery()), nil, &reviewers) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/reviewers?%s", owner, repo, opts.toQuery()), nil, &reviewers) if err != nil { return nil, err } @@ -37,7 +36,7 @@ type PullRequestReviewRequest struct { // GET /repos/{owner}/{repo}/pulls/{index}/requested_reviewers func (c *Client) ListPullRequestReviewers(ctx context.Context, owner, repo string, number int) ([]*Reviewer, error) { var reviewers []*Reviewer - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), nil, &reviewers) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), nil, &reviewers) if err != nil { return nil, err } @@ -48,21 +47,21 @@ func (c *Client) ListPullRequestReviewers(ctx context.Context, owner, repo strin // // POST /repos/{owner}/{repo}/pulls/{index}/requested_reviewers func (c *Client) RequestPullRequestReviewers(ctx context.Context, owner, repo string, number int, opts PullRequestReviewRequest) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), opts, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), opts, nil) } // RemovePullRequestReviewer removes a requested reviewer from a pull request. // // DELETE /repos/{owner}/{repo}/pulls/{index}/requested_reviewers func (c *Client) RemovePullRequestReviewer(ctx context.Context, owner, repo string, number int, opts PullRequestReviewRequest) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), opts, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/pulls/%d/requested_reviewers", owner, repo, number), opts, nil) } // DismissPullRequestReview dismisses a pull request review. // // PUT /repos/{owner}/{repo}/pulls/{index}/reviews/{id}/dismissals func (c *Client) DismissPullRequestReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews/%d/dismissals", owner, repo, number, reviewID), map[string]string{"message": message}, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/pulls/%d/reviews/%d/dismissals", owner, repo, number, reviewID), map[string]string{"message": message}, nil) } // SubmitPullRequestReview submits a pull request review. @@ -70,7 +69,7 @@ func (c *Client) DismissPullRequestReview(ctx context.Context, owner, repo strin // POST /repos/{owner}/{repo}/pulls/{index}/reviews/{id} func (c *Client) SubmitPullRequestReview(ctx context.Context, owner, repo string, number int, reviewID int64, body, event string) (*PullRequestReview, error) { var review PullRequestReview - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews/%d", owner, repo, number, reviewID), map[string]string{ + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/pulls/%d/reviews/%d", owner, repo, number, reviewID), map[string]string{ "body": body, "event": event, }, &review) @@ -85,7 +84,7 @@ func (c *Client) SubmitPullRequestReview(ctx context.Context, owner, repo string // GET /repos/{owner}/{repo}/pulls/{index}/reviews/{id} func (c *Client) GetPullRequestReview(ctx context.Context, owner, repo string, number int, reviewID int64) (*PullRequestReview, error) { var review PullRequestReview - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d/reviews/%d", owner, repo, number, reviewID), nil, &review) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d/reviews/%d", owner, repo, number, reviewID), nil, &review) if err != nil { return nil, err } @@ -101,12 +100,12 @@ type PullRequestDiff struct { // // GET /repos/{owner}/{repo}/pulls/{index}.diff func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) ([]byte, error) { - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d.diff", owner, repo, number)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d.diff", owner, repo, number)) } // GetPullRequestPatch returns the patch of a pull request. // // GET /repos/{owner}/{repo}/pulls/{index}.patch func (c *Client) GetPullRequestPatch(ctx context.Context, owner, repo string, number int) ([]byte, error) { - return c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/pulls/%d.patch", owner, repo, number)) + return c.doRawRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/pulls/%d.patch", owner, repo, number)) } diff --git a/repos_statuses.go b/repos_statuses.go index a0edb93..779c62f 100644 --- a/repos_statuses.go +++ b/repos_statuses.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" "time" ) @@ -43,7 +42,7 @@ type CombinedStatus struct { // GET /repos/{owner}/{repo}/statuses/{sha} func (c *Client) ListCommitStatuses(ctx context.Context, owner, repo, sha string, opts ListOptions) ([]*CommitStatus, error) { var statuses []*CommitStatus - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/statuses/%s?%s", owner, repo, sha, opts.toQuery()), nil, &statuses) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/statuses/%s?%s", owner, repo, sha, opts.toQuery()), nil, &statuses) if err != nil { return nil, err } @@ -55,7 +54,7 @@ func (c *Client) ListCommitStatuses(ctx context.Context, owner, repo, sha string // POST /repos/{owner}/{repo}/statuses/{sha} func (c *Client) CreateCommitStatus(ctx context.Context, owner, repo, sha string, opts CreateCommitStatusOptions) (*CommitStatus, error) { var status CommitStatus - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/statuses/%s", owner, repo, sha), opts, &status) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/statuses/%s", owner, repo, sha), opts, &status) if err != nil { return nil, err } @@ -67,7 +66,7 @@ func (c *Client) CreateCommitStatus(ctx context.Context, owner, repo, sha string // GET /repos/{owner}/{repo/commits/{sha}/status func (c *Client) GetCombinedStatus(ctx context.Context, owner, repo, sha string) (*CombinedStatus, error) { var status CombinedStatus - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/commits/%s/status", owner, repo, sha), nil, &status) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/commits/%s/status", owner, repo, sha), nil, &status) if err != nil { return nil, err } diff --git a/repos_templates.go b/repos_templates.go index 5e9fa1b..ebc1e9c 100644 --- a/repos_templates.go +++ b/repos_templates.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -28,7 +27,7 @@ type PRMergeTemplate struct { // GET /repos/{owner}/{repo}/issue_templates func (c *Client) ListIssueTemplates(ctx context.Context, owner, repo string) ([]*IssueTemplate, error) { var templates []*IssueTemplate - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issue_templates", owner, repo), nil, &templates) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issue_templates", owner, repo), nil, &templates) if err != nil { return nil, err } @@ -40,7 +39,7 @@ func (c *Client) ListIssueTemplates(ctx context.Context, owner, repo string) ([] // GET /repos/{owner}/{repo}/issue_templates/{name} func (c *Client) GetIssueTemplate(ctx context.Context, owner, repo, name string) (*IssueTemplate, error) { var template IssueTemplate - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/issue_templates/%s", owner, repo, name), nil, &template) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/issue_templates/%s", owner, repo, name), nil, &template) if err != nil { return nil, err } @@ -52,7 +51,7 @@ func (c *Client) GetIssueTemplate(ctx context.Context, owner, repo, name string) // GET /repos/{owner}/{repo}/merge_templates func (c *Client) ListPullRequestMergeTemplates(ctx context.Context, owner, repo string) ([]*PRMergeTemplate, error) { var templates []*PRMergeTemplate - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/merge_templates", owner, repo), nil, &templates) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/merge_templates", owner, repo), nil, &templates) if err != nil { return nil, err } diff --git a/repos_topics.go b/repos_topics.go index 7d6a5dd..c872f11 100644 --- a/repos_topics.go +++ b/repos_topics.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -16,7 +15,7 @@ type RepositoryTopics struct { // GET /repos/{owner}/{repo}/topics func (c *Client) ListRepositoryTopics(ctx context.Context, owner, repo string) ([]string, error) { var result RepositoryTopics - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/topics", owner, repo), nil, &result) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/topics", owner, repo), nil, &result) if err != nil { return nil, err } @@ -28,7 +27,7 @@ func (c *Client) ListRepositoryTopics(ctx context.Context, owner, repo string) ( // PUT /repos/{owner}/{repo}/topics func (c *Client) UpdateRepositoryTopics(ctx context.Context, owner, repo string, topics []string) ([]string, error) { var result RepositoryTopics - err := c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/topics", owner, repo), map[string]interface{}{"topics": topics}, &result) + err := c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/topics", owner, repo), map[string]interface{}{"topics": topics}, &result) if err != nil { return nil, err } @@ -39,12 +38,12 @@ func (c *Client) UpdateRepositoryTopics(ctx context.Context, owner, repo string, // // PUT /repos/{owner}/{repo}/topics/{topic} func (c *Client) AddRepositoryTopic(ctx context.Context, owner, repo, topic string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/repos/%s/%s/topics/%s", owner, repo, topic), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/repos/%s/%s/topics/%s", owner, repo, topic), nil, nil) } // DeleteRepositoryTopic removes a single topic from a repository. // // DELETE /repos/{owner}/{repo}/topics/{topic} func (c *Client) DeleteRepositoryTopic(ctx context.Context, owner, repo, topic string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/topics/%s", owner, repo, topic), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/topics/%s", owner, repo, topic), nil, nil) } diff --git a/repos_wiki.go b/repos_wiki.go index 4f64cbf..c74bdbc 100644 --- a/repos_wiki.go +++ b/repos_wiki.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -38,7 +37,7 @@ type UpdateWikiPageOptions struct { // GET /repos/{owner}/{repo}/wiki/pages func (c *Client) ListWikiPages(ctx context.Context, owner, repo string, opts ListOptions) ([]*WikiPage, error) { var pages []*WikiPage - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/wiki/pages?%s", owner, repo, opts.toQuery()), nil, &pages) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/wiki/pages?%s", owner, repo, opts.toQuery()), nil, &pages) if err != nil { return nil, err } @@ -50,7 +49,7 @@ func (c *Client) ListWikiPages(ctx context.Context, owner, repo string, opts Lis // GET /repos/{owner}/{repo}/wiki/page/{pageName} func (c *Client) GetWikiPage(ctx context.Context, owner, repo, pageName string) (*WikiPage, error) { var page WikiPage - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), nil, &page) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), nil, &page) if err != nil { return nil, err } @@ -62,7 +61,7 @@ func (c *Client) GetWikiPage(ctx context.Context, owner, repo, pageName string) // POST /repos/{owner}/{repo}/wiki/new func (c *Client) CreateWikiPage(ctx context.Context, owner, repo string, opts CreateWikiPageOptions) (*WikiPage, error) { var page WikiPage - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/wiki/new", owner, repo), opts, &page) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/wiki/new", owner, repo), opts, &page) if err != nil { return nil, err } @@ -74,7 +73,7 @@ func (c *Client) CreateWikiPage(ctx context.Context, owner, repo string, opts Cr // PATCH /repos/{owner}/{repo}/wiki/page/{pageName} func (c *Client) UpdateWikiPage(ctx context.Context, owner, repo, pageName string, opts UpdateWikiPageOptions) (*WikiPage, error) { var page WikiPage - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), opts, &page) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), opts, &page) if err != nil { return nil, err } @@ -85,5 +84,5 @@ func (c *Client) UpdateWikiPage(ctx context.Context, owner, repo, pageName strin // // DELETE /repos/{owner}/{repo}/wiki/page/{pageName} func (c *Client) DeleteWikiPage(ctx context.Context, owner, repo, pageName string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/wiki/page/%s", owner, repo, pageName), nil, nil) } diff --git a/retry.go b/retry.go index 7de8269..08d2676 100644 --- a/retry.go +++ b/retry.go @@ -15,6 +15,9 @@ type RetryPolicy struct { // InitialBackoff is the initial wait time before the first retry. Default: 1s. InitialBackoff time.Duration // MaxBackoff is the maximum wait time between retries. Default: 30s. + // The clamp applies to the exponential backoff **and** to a server-sent + // Retry-After: a hostile or misbehaving server cannot park the client + // for an arbitrary amount of time (issue #4). MaxBackoff time.Duration // Multiplier is the backoff multiplier for exponential backoff. Default: 2.0. Multiplier float64 @@ -59,6 +62,56 @@ func (c *Client) shouldRetry(statusCode int) bool { return false } +// defaultInitialBackoff / defaultMaxBackoff fill in the documented defaults +// when a custom policy leaves the corresponding field unset (issue #4: a +// zero MaxBackoff used to clamp every wait to 0). +const ( + defaultInitialBackoff = 1 * time.Second + defaultMaxBackoff = 30 * time.Second +) + +// maxRetryAfterSeconds bounds delay-seconds before the Duration conversion +// so a huge Retry-After cannot overflow time.Duration. +const maxRetryAfterSeconds = math.MaxInt64 / int64(time.Second) + +// retryAfterDuration parses the Retry-After header (issue #4). It accepts +// both RFC 9110 forms — delay-seconds and HTTP-date — and applies three +// gates before the value is used: +// +// 1. non-positive values (negative or zero seconds, a date in the past) +// are rejected instead of producing a negative duration, which used to +// make time.After fire immediately and turn rate-limit backoff into a +// busy loop; +// 2. the parsed value is clamped by the caller to MaxBackoff; +// 3. unparseable values report ok=false and the caller falls back to +// exponential backoff. +func retryAfterDuration(resp *http.Response, now time.Time) (time.Duration, bool) { + if resp == nil { + return 0, false + } + v := resp.Header.Get("Retry-After") + if v == "" { + return 0, false + } + if seconds, err := strconv.Atoi(v); err == nil { + if seconds <= 0 { + return 0, false + } + if int64(seconds) > maxRetryAfterSeconds { + return time.Duration(math.MaxInt64), true + } + return time.Duration(seconds) * time.Second, true + } + // HTTP-date form: wait until the given instant. + if t, err := http.ParseTime(v); err == nil { + if d := t.Sub(now); d > 0 { + return d, true + } + return 0, false + } + return 0, false +} + // getRetryWaitTime calculates the wait time for a given retry attempt. func (c *Client) getRetryWaitTime(attempt int, resp *http.Response) time.Duration { c.mu.RLock() @@ -69,21 +122,25 @@ func (c *Client) getRetryWaitTime(attempt int, resp *http.Response) time.Duratio return 0 } - // Check Retry-After header first (for 429 responses) - if resp != nil { - if retryAfter := resp.Header.Get("Retry-After"); retryAfter != "" { - if seconds, err := strconv.Atoi(retryAfter); err == nil { - return time.Duration(seconds) * time.Second - } - } + initial := policy.InitialBackoff + if initial <= 0 { + initial = defaultInitialBackoff + } + maxBackoff := policy.MaxBackoff + if maxBackoff <= 0 { + maxBackoff = defaultMaxBackoff } - // Exponential backoff - backoff := float64(policy.InitialBackoff) * math.Pow(policy.Multiplier, float64(attempt)) - if backoff > float64(policy.MaxBackoff) { - backoff = float64(policy.MaxBackoff) + // Check Retry-After header first (for 429 responses): a valid + // server-provided delay wins over the exponential curve, but shares + // the same MaxBackoff ceiling (issue #4). + if d, ok := retryAfterDuration(resp, time.Now()); ok { + return min(d, maxBackoff) } - return time.Duration(backoff) + + // Exponential backoff + backoff := time.Duration(float64(initial) * math.Pow(policy.Multiplier, float64(attempt))) + return min(backoff, maxBackoff) } // doRequestWithRetry executes an HTTP request with retry logic. @@ -97,12 +154,24 @@ func (c *Client) doRequestWithRetry(ctx context.Context, req *http.Request) (*ht maxRetries = policy.MaxRetries } + // The HTTP client is read under the same lock the setters write under; + // a bare read here raced SetHTTPClient (issue #8). + _, hc := c.requestSnapshot() + var resp *http.Response var err error for attempt := 0; attempt <= maxRetries; attempt++ { - // Clone the request for retries (body can be nil for retries) if attempt > 0 { + // Replay the request body: the previous attempt consumed it. + // http.NewRequestWithContext wires GetBody for *bytes.Reader / + // *strings.Reader bodies, which is exactly what every request + // builder in this package produces. + if req.GetBody != nil { + if body, bodyErr := req.GetBody(); bodyErr == nil { + req.Body = body + } + } waitTime := c.getRetryWaitTime(attempt-1, resp) select { case <-ctx.Done(): @@ -111,7 +180,7 @@ func (c *Client) doRequestWithRetry(ctx context.Context, req *http.Request) (*ht } } - resp, err = c.httpClient.Do(req) + resp, err = hc.Do(req) if err != nil { return nil, err } diff --git a/search.go b/search.go index e5ca82a..7666bf1 100644 --- a/search.go +++ b/search.go @@ -2,8 +2,8 @@ package gitcode import ( "context" - "fmt" "net/http" + "net/url" "time" ) @@ -50,23 +50,23 @@ type SearchRepositoryResult struct { func (c *Client) SearchRepositories(ctx context.Context, opts SearchRepositoriesOptions) ([]*SearchRepositoryResult, error) { var repos []*SearchRepositoryResult - query := fmt.Sprintf("q=%s&%s", opts.Query, opts.toQuery()) + query := "q=" + url.QueryEscape(opts.Query) + "&" + opts.toQuery() if opts.Sort != "" { - query += "&sort=" + opts.Sort + query += "&sort=" + url.QueryEscape(opts.Sort) } if opts.Order != "" { - query += "&order=" + opts.Order + query += "&order=" + url.QueryEscape(opts.Order) } if opts.Owner != "" { - query += "&owner=" + opts.Owner + query += "&owner=" + url.QueryEscape(opts.Owner) } if opts.Fork != "" { - query += "&fork=" + opts.Fork + query += "&fork=" + url.QueryEscape(opts.Fork) } if opts.Language != "" { - query += "&language=" + opts.Language + query += "&language=" + url.QueryEscape(opts.Language) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/search/repositories?%s", query), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/search/repositories?%s", query), nil, &repos) if err != nil { return nil, err } @@ -107,20 +107,20 @@ type SearchIssueResult struct { func (c *Client) SearchIssues(ctx context.Context, opts SearchIssuesOptions) ([]*SearchIssueResult, error) { var issues []*SearchIssueResult - query := fmt.Sprintf("q=%s&%s", opts.Query, opts.toQuery()) + query := "q=" + url.QueryEscape(opts.Query) + "&" + opts.toQuery() if opts.Sort != "" { - query += "&sort=" + opts.Sort + query += "&sort=" + url.QueryEscape(opts.Sort) } if opts.Order != "" { - query += "&order=" + opts.Order + query += "&order=" + url.QueryEscape(opts.Order) } if opts.Repo != "" { - query += "&repo=" + opts.Repo + query += "&repo=" + url.QueryEscape(opts.Repo) } if opts.State != "" { - query += "&state=" + opts.State + query += "&state=" + url.QueryEscape(opts.State) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/search/issues?%s", query), nil, &issues) + err := c.doRequest(ctx, http.MethodGet, pathf("/search/issues?%s", query), nil, &issues) if err != nil { return nil, err } @@ -145,14 +145,14 @@ type SearchUserResult struct { func (c *Client) SearchUsers(ctx context.Context, opts SearchUsersOptions) ([]*SearchUserResult, error) { var users []*SearchUserResult - query := fmt.Sprintf("q=%s&%s", opts.Query, opts.toQuery()) + query := "q=" + url.QueryEscape(opts.Query) + "&" + opts.toQuery() if opts.Sort != "" { - query += "&sort=" + opts.Sort + query += "&sort=" + url.QueryEscape(opts.Sort) } if opts.Order != "" { - query += "&order=" + opts.Order + query += "&order=" + url.QueryEscape(opts.Order) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/search/users?%s", query), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/search/users?%s", query), nil, &users) if err != nil { return nil, err } diff --git a/types.go b/types.go index 5ebde19..a797c6c 100644 --- a/types.go +++ b/types.go @@ -185,7 +185,7 @@ func (c *Client) ListOrganizations(ctx context.Context) ([]*Organization, error) func (c *Client) GetOrganization(ctx context.Context, org string) (*Organization, error) { var o Organization - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s", org), nil, &o) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s", org), nil, &o) if err != nil { return nil, err } @@ -194,7 +194,7 @@ func (c *Client) GetOrganization(ctx context.Context, org string) (*Organization func (c *Client) ListOrganizationMembers(ctx context.Context, org string) ([]*Member, error) { var members []*Member - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/orgs/%s/members", org), nil, &members) + err := c.doRequest(ctx, http.MethodGet, pathf("/orgs/%s/members", org), nil, &members) if err != nil { return nil, err } @@ -211,15 +211,15 @@ func (c *Client) ListNotifications(ctx context.Context) ([]*Notification, error) } func (c *Client) StarRepository(ctx context.Context, owner, repo string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/user/starred/%s/%s", owner, repo), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/user/starred/%s/%s", owner, repo), nil, nil) } func (c *Client) UnstarRepository(ctx context.Context, owner, repo string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/user/starred/%s/%s", owner, repo), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/user/starred/%s/%s", owner, repo), nil, nil) } func (c *Client) IsRepositoryStarred(ctx context.Context, owner, repo string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/user/starred/%s/%s", owner, repo)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/user/starred/%s/%s", owner, repo)) if err != nil { if strings.Contains(err.Error(), "404") { return false, nil diff --git a/upload.go b/upload.go index 73d0b22..0670da8 100644 --- a/upload.go +++ b/upload.go @@ -3,7 +3,6 @@ package gitcode import ( "bytes" "context" - "encoding/json" "fmt" "io" "mime/multipart" @@ -11,54 +10,50 @@ import ( "path/filepath" ) -// UploadFileReader uploads a file using multipart form data. -// This is the proper way to upload file contents (as opposed to UploadFile which takes a path). -// -// POST /repos/{owner}/{repo}/file/upload -func (c *Client) UploadFileReader(ctx context.Context, owner, repo, filename string, reader io.Reader) (*FileUploadResult, error) { - if err := validateOwnerRepo(owner, repo); err != nil { - return nil, err - } - +// multipartBody builds a multipart form with a single "file" field and +// returns the body together with its Content-Type. The body is a plain +// []byte so it can be replayed across retries (doRequestWithRetry resets +// the body via req.GetBody, which http.NewRequestWithContext wires for +// *bytes.Reader bodies). +func multipartBody(filename string, reader io.Reader) (body []byte, contentType string, err error) { var buf bytes.Buffer writer := multipart.NewWriter(&buf) part, err := writer.CreateFormFile("file", filename) if err != nil { - return nil, fmt.Errorf("create form file: %w", err) + return nil, "", fmt.Errorf("create form file: %w", err) } - if _, err := io.Copy(part, reader); err != nil { - return nil, fmt.Errorf("copy file content: %w", err) + return nil, "", fmt.Errorf("copy file content: %w", err) } - - writer.Close() - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+fmt.Sprintf("/repos/%s/%s/file/upload", owner, repo), &buf) - if err != nil { - return nil, err + if err := writer.Close(); err != nil { + return nil, "", fmt.Errorf("close multipart writer: %w", err) } + return buf.Bytes(), writer.FormDataContentType(), nil +} - c.setAuthHeader(req) - req.Header.Set("Content-Type", writer.FormDataContentType()) - - resp, err := c.httpClient.Do(req) - if err != nil { +// UploadFileReader uploads a file using multipart form data. +// This is the proper way to upload file contents (as opposed to UploadFile which takes a path). +// +// POST /repos/{owner}/{repo}/file/upload +// +// Uploads run through the shared request pipeline (doBufferedRequestAt): +// request/response hooks fire and the retry policy applies, exactly like +// every other request method (issue #9). +func (c *Client) UploadFileReader(ctx context.Context, owner, repo, filename string, reader io.Reader) (*FileUploadResult, error) { + if err := validateOwnerRepo(owner, repo); err != nil { return nil, err } - defer resp.Body.Close() - respBody, err := io.ReadAll(resp.Body) + body, contentType, err := multipartBody(filename, reader) if err != nil { return nil, err } - if resp.StatusCode >= 400 { - return nil, newAPIError(http.MethodPost, req.URL.Path, resp.StatusCode, string(respBody)) - } - var result FileUploadResult - if err := json.Unmarshal(respBody, &result); err != nil { + base, _ := c.requestSnapshot() + if err := c.doBufferedRequestAt(ctx, base, http.MethodPost, + pathf("/repos/%s/%s/file/upload", owner, repo), contentType, body, &result, c.jsonBodyLimit()); err != nil { return nil, err } return &result, nil @@ -67,50 +62,24 @@ func (c *Client) UploadFileReader(ctx context.Context, owner, repo, filename str // UploadImageReader uploads an image using multipart form data. // // POST /repos/{owner}/{repo}/img/upload +// +// Uploads run through the shared request pipeline (doBufferedRequestAt): +// request/response hooks fire and the retry policy applies, exactly like +// every other request method (issue #9). func (c *Client) UploadImageReader(ctx context.Context, owner, repo, filename string, reader io.Reader) (*FileUploadResult, error) { if err := validateOwnerRepo(owner, repo); err != nil { return nil, err } - var buf bytes.Buffer - writer := multipart.NewWriter(&buf) - - part, err := writer.CreateFormFile("file", filename) - if err != nil { - return nil, fmt.Errorf("create form file: %w", err) - } - - if _, err := io.Copy(part, reader); err != nil { - return nil, fmt.Errorf("copy file content: %w", err) - } - - writer.Close() - - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.baseURL+fmt.Sprintf("/repos/%s/%s/img/upload", owner, repo), &buf) - if err != nil { - return nil, err - } - - c.setAuthHeader(req) - req.Header.Set("Content-Type", writer.FormDataContentType()) - - resp, err := c.httpClient.Do(req) - if err != nil { - return nil, err - } - defer resp.Body.Close() - - respBody, err := io.ReadAll(resp.Body) + body, contentType, err := multipartBody(filename, reader) if err != nil { return nil, err } - if resp.StatusCode >= 400 { - return nil, newAPIError(http.MethodPost, req.URL.Path, resp.StatusCode, string(respBody)) - } - var result FileUploadResult - if err := json.Unmarshal(respBody, &result); err != nil { + base, _ := c.requestSnapshot() + if err := c.doBufferedRequestAt(ctx, base, http.MethodPost, + pathf("/repos/%s/%s/img/upload", owner, repo), contentType, body, &result, c.jsonBodyLimit()); err != nil { return nil, err } return &result, nil diff --git a/user_followers.go b/user_followers.go index 5918aa6..58ff323 100644 --- a/user_followers.go +++ b/user_followers.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -11,7 +10,7 @@ import ( // GET /users/{username}/followers func (c *Client) ListUserFollowers(ctx context.Context, username string, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s/followers?%s", username, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s/followers?%s", username, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -23,7 +22,7 @@ func (c *Client) ListUserFollowers(ctx context.Context, username string, opts Li // GET /user/followers func (c *Client) ListCurrentUserFollowers(ctx context.Context, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/followers?%s", opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/followers?%s", opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -35,7 +34,7 @@ func (c *Client) ListCurrentUserFollowers(ctx context.Context, opts ListOptions) // GET /users/{username}/following func (c *Client) ListUserFollowing(ctx context.Context, username string, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s/following?%s", username, opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s/following?%s", username, opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -47,7 +46,7 @@ func (c *Client) ListUserFollowing(ctx context.Context, username string, opts Li // GET /user/following func (c *Client) ListCurrentUserFollowing(ctx context.Context, opts ListOptions) ([]*User, error) { var users []*User - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/following?%s", opts.toQuery()), nil, &users) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/following?%s", opts.toQuery()), nil, &users) if err != nil { return nil, err } @@ -58,7 +57,7 @@ func (c *Client) ListCurrentUserFollowing(ctx context.Context, opts ListOptions) // // GET /user/following/{username} func (c *Client) IsFollowing(ctx context.Context, username string) (bool, error) { - _, err := c.doRawRequest(ctx, http.MethodGet, fmt.Sprintf("/user/following/%s", username)) + _, err := c.doRawRequest(ctx, http.MethodGet, pathf("/user/following/%s", username)) if err != nil { if isNotFoundError(err) { return false, nil @@ -72,12 +71,12 @@ func (c *Client) IsFollowing(ctx context.Context, username string) (bool, error) // // PUT /user/following/{username} func (c *Client) FollowUser(ctx context.Context, username string) error { - return c.doRequest(ctx, http.MethodPut, fmt.Sprintf("/user/following/%s", username), nil, nil) + return c.doRequest(ctx, http.MethodPut, pathf("/user/following/%s", username), nil, nil) } // UnfollowUser unfollows a user. // // DELETE /user/following/{username} func (c *Client) UnfollowUser(ctx context.Context, username string) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/user/following/%s", username), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/user/following/%s", username), nil, nil) } diff --git a/users.go b/users.go index ec1324e..396fcde 100644 --- a/users.go +++ b/users.go @@ -2,8 +2,8 @@ package gitcode import ( "context" - "fmt" "net/http" + "net/url" "time" ) @@ -36,7 +36,7 @@ type CreateSSHKeyOptions struct { func (c *Client) ListSSHKeys(ctx context.Context, opts ListOptions) ([]*SSHKey, error) { var keys []*SSHKey - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/keys?%s", opts.toQuery()), nil, &keys) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/keys?%s", opts.toQuery()), nil, &keys) if err != nil { return nil, err } @@ -45,7 +45,7 @@ func (c *Client) ListSSHKeys(ctx context.Context, opts ListOptions) ([]*SSHKey, func (c *Client) GetSSHKey(ctx context.Context, id int64) (*SSHKey, error) { var key SSHKey - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/keys/%d", id), nil, &key) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/keys/%d", id), nil, &key) if err != nil { return nil, err } @@ -62,7 +62,7 @@ func (c *Client) CreateSSHKey(ctx context.Context, opts CreateSSHKeyOptions) (*S } func (c *Client) DeleteSSHKey(ctx context.Context, id int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/user/keys/%d", id), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/user/keys/%d", id), nil, nil) } type UserEvent struct { @@ -91,19 +91,16 @@ type UserEventsResponse struct { func (c *Client) GetUserEvents(ctx context.Context, username, year, next string) (*UserEventsResponse, error) { var resp UserEventsResponse - query := "" + q := url.Values{} if year != "" { - query += "year=" + year + q.Set("year", year) } if next != "" { - if query != "" { - query += "&" - } - query += "next=" + next + q.Set("next", next) } - path := fmt.Sprintf("/users/%s/events", username) - if query != "" { - path += "?" + query + path := pathf("/users/%s/events", username) + if len(q) > 0 { + path += "?" + q.Encode() } err := c.doRequest(ctx, http.MethodGet, path, nil, &resp) if err != nil { @@ -122,12 +119,12 @@ func (c *Client) ListStarredRepositories(ctx context.Context, opts ListStarredRe var repos []*Repository query := opts.toQuery() if opts.Sort != "" { - query += "&sort=" + opts.Sort + query += "&sort=" + url.QueryEscape(opts.Sort) } if opts.Direction != "" { - query += "&direction=" + opts.Direction + query += "&direction=" + url.QueryEscape(opts.Direction) } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/starred?%s", query), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/starred?%s", query), nil, &repos) if err != nil { return nil, err } @@ -144,7 +141,7 @@ type Namespace struct { func (c *Client) GetNamespace(ctx context.Context, path string) (*Namespace, error) { var ns Namespace - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/namespace?path=%s", path), nil, &ns) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/namespace?path=%s", url.QueryEscape(path)), nil, &ns) if err != nil { return nil, err } diff --git a/users_enhanced.go b/users_enhanced.go index eb2efd8..15620c1 100644 --- a/users_enhanced.go +++ b/users_enhanced.go @@ -2,7 +2,6 @@ package gitcode import ( "context" - "fmt" "net/http" ) @@ -13,7 +12,7 @@ import ( // GET /users/{username}/subscriptions func (c *Client) ListUserWatchedRepositories(ctx context.Context, username string, opts ListOptions) ([]*Repository, error) { var repos []*Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/%s/subscriptions?%s", username, opts.toQuery()), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/%s/subscriptions?%s", username, opts.toQuery()), nil, &repos) if err != nil { return nil, err } @@ -25,7 +24,7 @@ func (c *Client) ListUserWatchedRepositories(ctx context.Context, username strin // GET /user/subscriptions func (c *Client) ListCurrentUserWatchedRepositories(ctx context.Context, opts ListOptions) ([]*Repository, error) { var repos []*Repository - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/user/subscriptions?%s", opts.toQuery()), nil, &repos) + err := c.doRequest(ctx, http.MethodGet, pathf("/user/subscriptions?%s", opts.toQuery()), nil, &repos) if err != nil { return nil, err } @@ -72,7 +71,7 @@ func (c *Client) ListUserPullRequests(ctx context.Context, opts ListPullRequests if opts.Direction != "" { query += "&direction=" + opts.Direction } - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/users/merge-requests?%s", query), nil, &prs) + err := c.doRequest(ctx, http.MethodGet, pathf("/users/merge-requests?%s", query), nil, &prs) if err != nil { return nil, err } diff --git a/webhooks.go b/webhooks.go index cce542b..55cc111 100644 --- a/webhooks.go +++ b/webhooks.go @@ -3,7 +3,6 @@ package gitcode import ( "context" "encoding/json" - "fmt" "net/http" "time" ) @@ -84,7 +83,7 @@ func (c *Client) ListWebhooks(ctx context.Context, owner, repo string) ([]*Webho // pagination (GET /repos/{owner}/{repo}/hooks). func (c *Client) ListWebhooksWithOptions(ctx context.Context, owner, repo string, opts ListOptions) ([]*Webhook, error) { var hooks []*Webhook - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/hooks?%s", owner, repo, opts.toQuery()), nil, &hooks) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/hooks?%s", owner, repo, opts.toQuery()), nil, &hooks) if err != nil { return nil, err } @@ -93,7 +92,7 @@ func (c *Client) ListWebhooksWithOptions(ctx context.Context, owner, repo string func (c *Client) GetWebhook(ctx context.Context, owner, repo string, hookID int64) (*Webhook, error) { var hook Webhook - err := c.doRequest(ctx, http.MethodGet, fmt.Sprintf("/repos/%s/%s/hooks/%d", owner, repo, hookID), nil, &hook) + err := c.doRequest(ctx, http.MethodGet, pathf("/repos/%s/%s/hooks/%d", owner, repo, hookID), nil, &hook) if err != nil { return nil, err } @@ -102,7 +101,7 @@ func (c *Client) GetWebhook(ctx context.Context, owner, repo string, hookID int6 func (c *Client) CreateWebhook(ctx context.Context, owner, repo string, opts CreateWebhookOptions) (*Webhook, error) { var hook Webhook - err := c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/hooks", owner, repo), opts, &hook) + err := c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/hooks", owner, repo), opts, &hook) if err != nil { return nil, err } @@ -111,7 +110,7 @@ func (c *Client) CreateWebhook(ctx context.Context, owner, repo string, opts Cre func (c *Client) UpdateWebhook(ctx context.Context, owner, repo string, hookID int64, opts UpdateWebhookOptions) (*Webhook, error) { var hook Webhook - err := c.doRequest(ctx, http.MethodPatch, fmt.Sprintf("/repos/%s/%s/hooks/%d", owner, repo, hookID), opts, &hook) + err := c.doRequest(ctx, http.MethodPatch, pathf("/repos/%s/%s/hooks/%d", owner, repo, hookID), opts, &hook) if err != nil { return nil, err } @@ -119,11 +118,11 @@ func (c *Client) UpdateWebhook(ctx context.Context, owner, repo string, hookID i } func (c *Client) DeleteWebhook(ctx context.Context, owner, repo string, hookID int64) error { - return c.doRequest(ctx, http.MethodDelete, fmt.Sprintf("/repos/%s/%s/hooks/%d", owner, repo, hookID), nil, nil) + return c.doRequest(ctx, http.MethodDelete, pathf("/repos/%s/%s/hooks/%d", owner, repo, hookID), nil, nil) } func (c *Client) TestWebhook(ctx context.Context, owner, repo string, hookID int64) error { - return c.doRequest(ctx, http.MethodPost, fmt.Sprintf("/repos/%s/%s/hooks/%d/tests", owner, repo, hookID), nil, nil) + return c.doRequest(ctx, http.MethodPost, pathf("/repos/%s/%s/hooks/%d/tests", owner, repo, hookID), nil, nil) } type WebhookEvent struct {