From 2416393ed544fa2b27135ceecb010dc4b994d84b Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Mon, 17 Aug 2026 18:48:10 +0530 Subject: [PATCH] Add cross-forge commit ref resolution --- README.md | 16 +++- bitbucket/commits.go | 17 ++++ commits.go | 87 +++++++++++++++++++ commits_test.go | 142 +++++++++++++++++++++++++++++++ forge.go | 1 + forges_test.go | 23 +++++ gerrit/stubs.go | 6 ++ gitea/commits.go | 47 ++++++++++ gitea/commits_test.go | 98 +++++++++++++++++++++ github/commits.go | 64 +++++++------- github/commits_test.go | 66 ++++++++++++++ gitlab/commits.go | 44 ++++++++++ gitlab/commits_test.go | 92 ++++++++++++++++++++ internal/cli/pr_checkout_test.go | 1 + tangled/tangled.go | 1 + tangled/unsupported.go | 6 ++ 16 files changed, 679 insertions(+), 32 deletions(-) create mode 100644 bitbucket/commits.go create mode 100644 commits.go create mode 100644 commits_test.go create mode 100644 gitea/commits.go create mode 100644 gitea/commits_test.go create mode 100644 gitlab/commits.go create mode 100644 gitlab/commits_test.go diff --git a/README.md b/README.md index 82497b3..1af1238 100644 --- a/README.md +++ b/README.md @@ -155,7 +155,7 @@ client := forges.NewClient( repo, err := client.FetchRepository(ctx, "https://github.com/octocat/hello-world") ``` -The `Forge` interface exposes services for repos, issues, pull requests, reviews, releases, CI, branches, labels, milestones, deploy keys, secrets, notifications, files, collaborators, and commit statuses. Each backend implements these using its native SDK. +The `Forge` interface exposes services for repos, issues, pull requests, reviews, releases, CI, branches, labels, milestones, deploy keys, secrets, notifications, files, collaborators, commit statuses and commits. Each backend implements these using its native SDK. ```go f, _ := client.ForgeFor("github.com") @@ -193,7 +193,19 @@ p, _ := purl.Parse("pkg:npm/lodash?repository_url=https://github.com/lodash/loda repo, err := client.FetchRepositoryFromPURL(ctx, p) ``` -GitHub refs can be resolved to full commit SHAs without listing every tag: +Branches, tags and abbreviated commit refs can be resolved to full commit SHAs without listing every tag. GitHub, GitLab and Gitea/Forgejo implement this; the remaining backends return `forges.ErrNotSupported`. + +```go +f, _ := client.ForgeFor("gitlab.com") +sha, err := f.Commits().ResolveCommit(ctx, "gitlab-org", "gitlab", "v17.0.0") + +// or route by repository URL +sha, err = client.ResolveCommit(ctx, "https://github.com/actions/checkout", "v4.2.1") +``` + +A ref that does not exist returns `forges.ErrNotFound`. A ref that is already a full 40-character SHA is normalized and returned without a network request. + +The GitHub backend also exposes a standalone resolver for callers that do not need the full `Forge` interface: ```go import githubforge "github.com/git-pkgs/forge/github" diff --git a/bitbucket/commits.go b/bitbucket/commits.go new file mode 100644 index 0000000..92ae34f --- /dev/null +++ b/bitbucket/commits.go @@ -0,0 +1,17 @@ +package bitbucket + +import ( + "context" + + forge "github.com/git-pkgs/forge" +) + +type bitbucketCommitService struct{} + +func (f *bitbucketForge) Commits() forge.CommitService { + return &bitbucketCommitService{} +} + +func (s *bitbucketCommitService) ResolveCommit(_ context.Context, _, _, _ string) (string, error) { + return "", forge.ErrNotSupported +} diff --git a/commits.go b/commits.go new file mode 100644 index 0000000..df4dcf8 --- /dev/null +++ b/commits.go @@ -0,0 +1,87 @@ +package forges + +import ( + "context" + "errors" + "fmt" + "strings" +) + +// FullCommitSHALength is the number of hexadecimal characters in a full SHA-1 +// commit ID. +const FullCommitSHALength = 40 + +// ErrCommitRefRequired is returned when owner, repo or ref is empty. +var ErrCommitRefRequired = errors.New("resolve commit: owner, repo and ref are required") + +// CommitService resolves repository refs to immutable commit SHAs. Callers use +// it to pin a mutable ref, such as an action reference or a package source +// revision, to the exact commit it currently points at without listing every +// branch or tag on the repository. +type CommitService interface { + // ResolveCommit returns the full commit SHA that ref points at in + // owner/repo. ref may be a branch name, a tag name, an abbreviated + // commit SHA or a full commit SHA. Implementations return ErrNotFound + // when the ref does not exist on the repository. They return + // ErrNotSupported when the forge exposes no ref resolution endpoint. + ResolveCommit(ctx context.Context, owner, repo, ref string) (string, error) +} + +// IsFullCommitSHA reports whether ref is already a full-length hexadecimal +// SHA-1. Backends call it to skip a network round trip for refs that are +// immutable to begin with. +func IsFullCommitSHA(ref string) bool { + if len(ref) != FullCommitSHALength { + return false + } + for _, char := range ref { + if (char < '0' || char > '9') && (char < 'a' || char > 'f') && (char < 'A' || char > 'F') { + return false + } + } + return true +} + +// ValidateCommitRef rejects the empty arguments every CommitService +// implementation has to guard against before issuing a request. +func ValidateCommitRef(owner, repo, ref string) error { + if owner == "" || repo == "" || ref == "" { + return ErrCommitRefRequired + } + return nil +} + +// CommitRefError wraps err with the repository and ref being resolved so every +// backend reports resolution failures the same way. +func CommitRefError(owner, repo, ref string, err error) error { + return fmt.Errorf("resolve %s/%s ref %q: %w", owner, repo, ref, err) +} + +// ResolvedCommitSHA normalizes a commit SHA a forge API returned for +// owner/repo ref. It trims surrounding whitespace, lowercases the value and +// rejects anything that is not a full hexadecimal SHA-1, so callers never +// receive a value they cannot pin against. +func ResolvedCommitSHA(owner, repo, ref, sha string) (string, error) { + sha = strings.TrimSpace(sha) + if sha == "" { + return "", CommitRefError(owner, repo, ref, errors.New("empty SHA in response")) + } + if !IsFullCommitSHA(sha) { + return "", CommitRefError(owner, repo, ref, fmt.Errorf("invalid full SHA %q in response", sha)) + } + return strings.ToLower(sha), nil +} + +// ResolveCommit resolves ref to a full commit SHA for the repository at +// repoURL, routing to the forge registered for that URL's domain. +func (c *Client) ResolveCommit(ctx context.Context, repoURL, ref string) (string, error) { + domain, owner, repo, err := ParseRepoURL(repoURL) + if err != nil { + return "", err + } + f, err := c.forgeFor(domain) + if err != nil { + return "", err + } + return f.Commits().ResolveCommit(ctx, owner, repo, ref) +} diff --git a/commits_test.go b/commits_test.go new file mode 100644 index 0000000..2639353 --- /dev/null +++ b/commits_test.go @@ -0,0 +1,142 @@ +package forges + +import ( + "context" + "errors" + "strings" + "testing" +) + +const ( + testCommitSHA = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + testCommitSHAUpper = "8E8C483DB84B4BEE98B60C0593521ED34D9990E8" +) + +func TestIsFullCommitSHA(t *testing.T) { + tests := []struct { + name string + ref string + want bool + }{ + {name: "lowercase full SHA", ref: testCommitSHA, want: true}, + {name: "uppercase full SHA", ref: testCommitSHAUpper, want: true}, + {name: "mixed case full SHA", ref: "8e8C483db84B4bee98b60C0593521ed34d9990E8", want: true}, + {name: "abbreviated SHA", ref: "8e8c483", want: false}, + {name: "branch name", ref: "main", want: false}, + {name: "tag name", ref: "v4.2.1", want: false}, + {name: "empty", ref: "", want: false}, + {name: "too short by one", ref: testCommitSHA[1:], want: false}, + {name: "too long by one", ref: testCommitSHA + "0", want: false}, + {name: "right length with non-hex character", ref: strings.Repeat("g", FullCommitSHALength), want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsFullCommitSHA(tt.ref); got != tt.want { + t.Errorf("IsFullCommitSHA(%q) = %v, want %v", tt.ref, got, tt.want) + } + }) + } +} + +func TestValidateCommitRef(t *testing.T) { + tests := []struct { + name string + owner, repo, ref string + wantErr bool + }{ + {name: "all set", owner: "actions", repo: "checkout", ref: "v4", wantErr: false}, + {name: "empty owner", owner: "", repo: "checkout", ref: "v4", wantErr: true}, + {name: "empty repo", owner: "actions", repo: "", ref: "v4", wantErr: true}, + {name: "empty ref", owner: "actions", repo: "checkout", ref: "", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateCommitRef(tt.owner, tt.repo, tt.ref) + if tt.wantErr && !errors.Is(err, ErrCommitRefRequired) { + t.Errorf("ValidateCommitRef() error = %v, want ErrCommitRefRequired", err) + } + if !tt.wantErr && err != nil { + t.Errorf("ValidateCommitRef() error = %v, want nil", err) + } + }) + } +} + +func TestResolvedCommitSHA(t *testing.T) { + t.Run("normalizes case and whitespace", func(t *testing.T) { + got, err := ResolvedCommitSHA("actions", "checkout", "v4", " "+testCommitSHAUpper+"\n") + if err != nil { + t.Fatalf("ResolvedCommitSHA() error = %v", err) + } + if got != testCommitSHA { + t.Errorf("ResolvedCommitSHA() = %q, want %q", got, testCommitSHA) + } + }) + + t.Run("rejects empty SHA", func(t *testing.T) { + if _, err := ResolvedCommitSHA("actions", "checkout", "v4", " \n"); err == nil { + t.Fatal("ResolvedCommitSHA() error = nil, want empty SHA error") + } + }) + + t.Run("rejects abbreviated SHA", func(t *testing.T) { + if _, err := ResolvedCommitSHA("actions", "checkout", "v4", "deadbeef"); err == nil { + t.Fatal("ResolvedCommitSHA() error = nil, want invalid SHA error") + } + }) +} + +func TestCommitRefErrorUnwraps(t *testing.T) { + err := CommitRefError("actions", "checkout", "missing", ErrNotFound) + if !errors.Is(err, ErrNotFound) { + t.Fatalf("CommitRefError() = %v, want an error wrapping ErrNotFound", err) + } + for _, want := range []string{"actions/checkout", `"missing"`} { + if !strings.Contains(err.Error(), want) { + t.Errorf("CommitRefError() = %q, want it to mention %s", err.Error(), want) + } + } +} + +func TestClientResolveCommitRoutes(t *testing.T) { + mock := &mockForge{commitService: &mockCommitService{sha: testCommitSHA}} + c := &Client{ + forges: map[string]Forge{"example.com": mock}, + tokens: make(map[string]string), + } + + sha, err := c.ResolveCommit(context.Background(), "https://example.com/test/repo", "v1.0.0") + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != testCommitSHA { + t.Errorf("ResolveCommit() = %q, want %q", sha, testCommitSHA) + } + + cs := mock.commitService + if cs.lastOwner != "test" || cs.lastRepo != "repo" || cs.lastRef != "v1.0.0" { + t.Errorf("backend received owner=%q repo=%q ref=%q, want test/repo at v1.0.0", + cs.lastOwner, cs.lastRepo, cs.lastRef) + } +} + +func TestClientResolveCommitPropagatesBackendError(t *testing.T) { + mock := &mockForge{commitService: &mockCommitService{err: ErrNotFound}} + c := &Client{ + forges: map[string]Forge{"example.com": mock}, + tokens: make(map[string]string), + } + + if _, err := c.ResolveCommit(context.Background(), "https://example.com/test/repo", "nope"); !errors.Is(err, ErrNotFound) { + t.Errorf("ResolveCommit() error = %v, want ErrNotFound", err) + } +} + +func TestClientResolveCommitUnregisteredDomain(t *testing.T) { + c := NewClient() + if _, err := c.ResolveCommit(context.Background(), "https://example.com/test/repo", "v1.0.0"); err == nil { + t.Error("ResolveCommit() error = nil, want error for unregistered domain") + } +} diff --git a/forge.go b/forge.go index 877efa6..cffd986 100644 --- a/forge.go +++ b/forge.go @@ -73,6 +73,7 @@ type Forge interface { Files() FileService Collaborators() CollaboratorService CommitStatuses() CommitStatusService + Commits() CommitService GetRateLimit(ctx context.Context) (*RateLimit, error) // ParsePath parses URL path segments into a resource reference. ParsePath(pathParts []string) (*ResourceRef, error) diff --git a/forges_test.go b/forges_test.go index 1334608..0981ee8 100644 --- a/forges_test.go +++ b/forges_test.go @@ -620,6 +620,7 @@ type mockForge struct { deployKeyService *mockDeployKeyService secretService *mockSecretService reviewService *mockReviewService + commitService *mockCommitService } func (m *mockForge) Repos() RepoService { @@ -724,6 +725,13 @@ func (m *mockForge) CommitStatuses() CommitStatusService { return &mockCommitStatusService{} } +func (m *mockForge) Commits() CommitService { + if m.commitService != nil { + return m.commitService + } + return &mockCommitService{} +} + func (m *mockForge) GetRateLimit(_ context.Context) (*RateLimit, error) { return nil, ErrNotSupported } @@ -766,6 +774,21 @@ func (m *mockCommitStatusService) Set(_ context.Context, _, _, _ string, _ SetCo return nil, nil } +type mockCommitService struct { + sha string + err error + lastOwner string + lastRepo string + lastRef string +} + +func (m *mockCommitService) ResolveCommit(_ context.Context, owner, repo, ref string) (string, error) { + m.lastOwner = owner + m.lastRepo = repo + m.lastRef = ref + return m.sha, m.err +} + type mockRepoService struct { repo *Repository repos []Repository diff --git a/gerrit/stubs.go b/gerrit/stubs.go index 5bef12d..80d5596 100644 --- a/gerrit/stubs.go +++ b/gerrit/stubs.go @@ -18,6 +18,7 @@ type unsupportedSecretService struct{} type unsupportedNotificationService struct{} type unsupportedCollaboratorService struct{} type unsupportedCommitStatusService struct{} +type unsupportedCommitService struct{} func (f *gerritForge) Issues() forge.IssueService { return &unsupportedIssueService{} } func (f *gerritForge) Labels() forge.LabelService { return &unsupportedLabelService{} } @@ -35,6 +36,7 @@ func (f *gerritForge) Collaborators() forge.CollaboratorService { func (f *gerritForge) CommitStatuses() forge.CommitStatusService { return &unsupportedCommitStatusService{} } +func (f *gerritForge) Commits() forge.CommitService { return &unsupportedCommitService{} } func (f *gerritForge) GetRateLimit(context.Context) (*forge.RateLimit, error) { return nil, forge.ErrNotSupported } @@ -206,3 +208,7 @@ func (s *unsupportedCommitStatusService) List(context.Context, string, string, s func (s *unsupportedCommitStatusService) Set(context.Context, string, string, string, forge.SetCommitStatusOpts) (*forge.CommitStatus, error) { return nil, forge.ErrNotSupported } + +func (s *unsupportedCommitService) ResolveCommit(context.Context, string, string, string) (string, error) { + return "", forge.ErrNotSupported +} diff --git a/gitea/commits.go b/gitea/commits.go new file mode 100644 index 0000000..d2f9d64 --- /dev/null +++ b/gitea/commits.go @@ -0,0 +1,47 @@ +package gitea + +import ( + "context" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "strings" + + "code.gitea.io/sdk/gitea" +) + +type giteaCommitService struct { + client *gitea.Client +} + +func (f *giteaForge) Commits() forge.CommitService { + return &giteaCommitService{client: f.client} +} + +// ResolveCommit returns the full commit SHA for ref in owner/repo. Gitea and +// Forgejo accept a git ref or a commit SHA on the single-commit endpoint and +// resolve branches, tags and abbreviated SHAs the same way git does. +// +// The context is unused because the Gitea SDK only takes a context on the +// client itself, which is shared across calls, so a per-request deadline +// cannot be applied without racing other callers. +func (s *giteaCommitService) ResolveCommit(_ context.Context, owner, repo, ref string) (string, error) { + if err := forge.ValidateCommitRef(owner, repo, ref); err != nil { + return "", err + } + if forge.IsFullCommitSHA(ref) { + return strings.ToLower(ref), nil + } + + commit, resp, err := s.client.GetSingleCommit(owner, repo, ref) + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return "", forge.CommitRefError(owner, repo, ref, forge.ErrNotFound) + } + return "", forge.CommitRefError(owner, repo, ref, err) + } + if commit == nil || commit.CommitMeta == nil { + return "", forge.CommitRefError(owner, repo, ref, errors.New("empty response")) + } + return forge.ResolvedCommitSHA(owner, repo, ref, commit.SHA) +} diff --git a/gitea/commits_test.go b/gitea/commits_test.go new file mode 100644 index 0000000..1c255d6 --- /dev/null +++ b/gitea/commits_test.go @@ -0,0 +1,98 @@ +package gitea + +import ( + "context" + "encoding/json" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +const ( + giteaCommitSHA = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + giteaCommitSHAUpper = "8E8C483DB84B4BEE98B60C0593521ED34D9990E8" +) + +func TestGiteaResolveCommit(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v1/version", giteaVersionHandler) + mux.HandleFunc("GET /api/v1/repos/testorg/testrepo/git/commits/v1.0.0", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "sha": giteaCommitSHAUpper, + "url": "https://gitea.example.com/testorg/testrepo/commit/" + giteaCommitSHA, + }) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + sha, err := f.Commits().ResolveCommit(context.Background(), "testorg", "testrepo", "v1.0.0") + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != giteaCommitSHA { + t.Errorf("ResolveCommit() = %q, want %q", sha, giteaCommitSHA) + } +} + +func TestGiteaResolveCommitSkipsRequestForFullSHA(t *testing.T) { + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + requests.Add(1) + })) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + sha, err := f.Commits().ResolveCommit(context.Background(), "testorg", "testrepo", giteaCommitSHAUpper) + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != giteaCommitSHA { + t.Errorf("ResolveCommit() = %q, want normalized %q", sha, giteaCommitSHA) + } + if got := requests.Load(); got != 0 { + t.Errorf("ResolveCommit() made %d requests for a full SHA, want 0", got) + } +} + +func TestGiteaResolveCommitNotFound(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v1/version", giteaVersionHandler) + mux.HandleFunc("/", http.NotFound) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + _, err := f.Commits().ResolveCommit(context.Background(), "testorg", "testrepo", "missing") + if !errors.Is(err, forge.ErrNotFound) { + t.Errorf("ResolveCommit() error = %v, want ErrNotFound", err) + } +} + +func TestGiteaResolveCommitRejectsShortSHAInResponse(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v1/version", giteaVersionHandler) + mux.HandleFunc("GET /api/v1/repos/testorg/testrepo/git/commits/v1.0.0", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"sha": "8e8c483"}) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + if _, err := f.Commits().ResolveCommit(context.Background(), "testorg", "testrepo", "v1.0.0"); err == nil { + t.Fatal("ResolveCommit() error = nil, want invalid SHA error") + } +} + +func TestGiteaResolveCommitRequiresArguments(t *testing.T) { + f := New("https://gitea.example.com", "", nil) + if _, err := f.Commits().ResolveCommit(context.Background(), "", "testrepo", "v1.0.0"); !errors.Is(err, forge.ErrCommitRefRequired) { + t.Errorf("ResolveCommit() error = %v, want ErrCommitRefRequired", err) + } +} diff --git a/github/commits.go b/github/commits.go index 3741b4d..88c8f07 100644 --- a/github/commits.go +++ b/github/commits.go @@ -2,6 +2,7 @@ package github import ( "context" + "errors" "fmt" "net/http" "net/url" @@ -14,9 +15,7 @@ import ( // DefaultAPIBaseURL is the public GitHub REST API base URL. const DefaultAPIBaseURL = "https://api.github.com/" -const fullCommitSHALength = 40 - -// CommitResolver resolves GitHub branch, tag, and abbreviated commit refs to +// CommitResolver resolves GitHub branch, tag and abbreviated commit refs to // full commit SHAs. Full 40-character hexadecimal SHAs are returned without a // network request. type CommitResolver struct { @@ -48,7 +47,7 @@ func NewCommitResolverWithBase(baseURL, token string, client *http.Client) (*Com return nil, fmt.Errorf("parse GitHub API base URL: unsupported scheme %q", base.Scheme) } if base.Host == "" { - return nil, fmt.Errorf("parse GitHub API base URL: host is required") + return nil, errors.New("parse GitHub API base URL: host is required") } api.BaseURL = base @@ -58,42 +57,47 @@ func NewCommitResolverWithBase(baseURL, token string, client *http.Client) (*Com // ResolveCommit returns the full commit SHA for ref in owner/repo. GitHub's // commit endpoint dereferences both lightweight and annotated tags. func (r *CommitResolver) ResolveCommit(ctx context.Context, owner, repo, ref string) (string, error) { - if owner == "" || repo == "" || ref == "" { - return "", fmt.Errorf("resolve GitHub commit: owner, repo, and ref are required") + if err := forge.ValidateCommitRef(owner, repo, ref); err != nil { + return "", err } - if isFullCommitSHA(ref) { + if forge.IsFullCommitSHA(ref) { return strings.ToLower(ref), nil } if r == nil || r.client == nil { - return "", fmt.Errorf("resolve GitHub commit: resolver is nil") + return "", errors.New("resolve GitHub commit: resolver is nil") } + return resolveCommit(ctx, r.client, owner, repo, ref) +} - sha, response, err := r.client.Repositories.GetCommitSHA1(ctx, owner, repo, ref, "") - if err != nil { - if response != nil && response.StatusCode == http.StatusNotFound { - return "", fmt.Errorf("resolve %s/%s ref %q: %w", owner, repo, ref, forge.ErrNotFound) - } - return "", fmt.Errorf("resolve %s/%s ref %q: %w", owner, repo, ref, err) - } - sha = strings.TrimSpace(sha) - if sha == "" { - return "", fmt.Errorf("resolve %s/%s ref %q: empty SHA in response", owner, repo, ref) +type gitHubCommitService struct { + client *gh.Client +} + +func (f *gitHubForge) Commits() forge.CommitService { + return &gitHubCommitService{client: f.client} +} + +// ResolveCommit returns the full commit SHA for ref in owner/repo. +func (s *gitHubCommitService) ResolveCommit(ctx context.Context, owner, repo, ref string) (string, error) { + if err := forge.ValidateCommitRef(owner, repo, ref); err != nil { + return "", err } - if !isFullCommitSHA(sha) { - return "", fmt.Errorf("resolve %s/%s ref %q: invalid full SHA in response", owner, repo, ref) + if forge.IsFullCommitSHA(ref) { + return strings.ToLower(ref), nil } - return strings.ToLower(sha), nil + return resolveCommit(ctx, s.client, owner, repo, ref) } -// isFullCommitSHA reports whether ref is a full-length hexadecimal SHA-1. -func isFullCommitSHA(ref string) bool { - if len(ref) != fullCommitSHALength { - return false - } - for _, char := range ref { - if (char < '0' || char > '9') && (char < 'a' || char > 'f') && (char < 'A' || char > 'F') { - return false +// resolveCommit asks the GitHub commit endpoint for the full SHA behind ref. +// Callers have already rejected empty arguments and short-circuited refs that +// are full SHAs already. +func resolveCommit(ctx context.Context, client *gh.Client, owner, repo, ref string) (string, error) { + sha, response, err := client.Repositories.GetCommitSHA1(ctx, owner, repo, ref, "") + if err != nil { + if response != nil && response.StatusCode == http.StatusNotFound { + return "", forge.CommitRefError(owner, repo, ref, forge.ErrNotFound) } + return "", forge.CommitRefError(owner, repo, ref, err) } - return true + return forge.ResolvedCommitSHA(owner, repo, ref, sha) } diff --git a/github/commits_test.go b/github/commits_test.go index e03531b..9481c6d 100644 --- a/github/commits_test.go +++ b/github/commits_test.go @@ -108,4 +108,70 @@ func TestCommitResolverErrors(t *testing.T) { t.Fatal("NewCommitResolverWithBase() error = nil, want invalid URL error") } }) + + t.Run("missing arguments", func(t *testing.T) { + resolver := NewCommitResolver("", nil) + if _, err := resolver.ResolveCommit(context.Background(), "actions", "checkout", ""); !errors.Is(err, forge.ErrCommitRefRequired) { + t.Errorf("ResolveCommit() error = %v, want ErrCommitRefRequired", err) + } + }) +} + +func TestGitHubCommitServiceResolveCommit(t *testing.T) { + const responseSHA = "8E8C483DB84B4BEE98B60C0593521ED34D9990E8" + const wantSHA = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v3/repos/octocat/hello-world/commits/v4.2.1", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Accept") != "application/vnd.github.v3.sha" { + t.Errorf("Accept = %q, want SHA media type", r.Header.Get("Accept")) + } + _, _ = w.Write([]byte(responseSHA)) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := NewWithBase(srv.URL, "test-token", srv.Client()) + sha, err := f.Commits().ResolveCommit(context.Background(), "octocat", "hello-world", "v4.2.1") + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != wantSHA { + t.Errorf("ResolveCommit() = %q, want %q", sha, wantSHA) + } +} + +func TestGitHubCommitServiceNotFound(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + defer srv.Close() + + f := NewWithBase(srv.URL, "test-token", srv.Client()) + _, err := f.Commits().ResolveCommit(context.Background(), "octocat", "hello-world", "missing") + if !errors.Is(err, forge.ErrNotFound) { + t.Errorf("ResolveCommit() error = %v, want ErrNotFound", err) + } +} + +func TestGitHubCommitServiceSkipsRequestForFullSHA(t *testing.T) { + const sha = "8E8C483DB84B4BEE98B60C0593521ED34D9990E8" + const want = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + requests.Add(1) + })) + defer srv.Close() + + f := NewWithBase(srv.URL, "test-token", srv.Client()) + got, err := f.Commits().ResolveCommit(context.Background(), "octocat", "hello-world", sha) + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if got != want { + t.Errorf("ResolveCommit() = %q, want normalized %q", got, want) + } + if n := requests.Load(); n != 0 { + t.Errorf("ResolveCommit() made %d requests for a full SHA, want 0", n) + } } diff --git a/gitlab/commits.go b/gitlab/commits.go new file mode 100644 index 0000000..88bd584 --- /dev/null +++ b/gitlab/commits.go @@ -0,0 +1,44 @@ +package gitlab + +import ( + "context" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "strings" + + gitlab "gitlab.com/gitlab-org/api/client-go" +) + +type gitLabCommitService struct { + client *gitlab.Client +} + +func (f *gitLabForge) Commits() forge.CommitService { + return &gitLabCommitService{client: f.client} +} + +// ResolveCommit returns the full commit SHA for ref in owner/repo. GitLab's +// single-commit endpoint accepts a branch name, a tag name or a commit SHA, +// and it dereferences annotated tags to the commit they point at. +func (s *gitLabCommitService) ResolveCommit(ctx context.Context, owner, repo, ref string) (string, error) { + if err := forge.ValidateCommitRef(owner, repo, ref); err != nil { + return "", err + } + if forge.IsFullCommitSHA(ref) { + return strings.ToLower(ref), nil + } + + pid := owner + "/" + repo + commit, resp, err := s.client.Commits.GetCommit(pid, ref, nil, gitlab.WithContext(ctx)) + if err != nil { + if resp != nil && resp.StatusCode == http.StatusNotFound { + return "", forge.CommitRefError(owner, repo, ref, forge.ErrNotFound) + } + return "", forge.CommitRefError(owner, repo, ref, err) + } + if commit == nil { + return "", forge.CommitRefError(owner, repo, ref, errors.New("empty response")) + } + return forge.ResolvedCommitSHA(owner, repo, ref, commit.ID) +} diff --git a/gitlab/commits_test.go b/gitlab/commits_test.go new file mode 100644 index 0000000..6863c63 --- /dev/null +++ b/gitlab/commits_test.go @@ -0,0 +1,92 @@ +package gitlab + +import ( + "context" + "encoding/json" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +const ( + gitLabCommitSHA = "8e8c483db84b4bee98b60c0593521ed34d9990e8" + gitLabCommitSHAUpper = "8E8C483DB84B4BEE98B60C0593521ED34D9990E8" +) + +func TestGitLabResolveCommit(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v4/projects/mygroup%2Fmyrepo/repository/commits/v1.0.0", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": gitLabCommitSHAUpper, + "short_id": "8e8c483", + }) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + sha, err := f.Commits().ResolveCommit(context.Background(), "mygroup", "myrepo", "v1.0.0") + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != gitLabCommitSHA { + t.Errorf("ResolveCommit() = %q, want %q", sha, gitLabCommitSHA) + } +} + +func TestGitLabResolveCommitSkipsRequestForFullSHA(t *testing.T) { + var requests atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { + requests.Add(1) + })) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + sha, err := f.Commits().ResolveCommit(context.Background(), "mygroup", "myrepo", gitLabCommitSHAUpper) + if err != nil { + t.Fatalf("ResolveCommit() error = %v", err) + } + if sha != gitLabCommitSHA { + t.Errorf("ResolveCommit() = %q, want normalized %q", sha, gitLabCommitSHA) + } + if got := requests.Load(); got != 0 { + t.Errorf("ResolveCommit() made %d requests for a full SHA, want 0", got) + } +} + +func TestGitLabResolveCommitNotFound(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + _, err := f.Commits().ResolveCommit(context.Background(), "mygroup", "myrepo", "missing") + if !errors.Is(err, forge.ErrNotFound) { + t.Errorf("ResolveCommit() error = %v, want ErrNotFound", err) + } +} + +func TestGitLabResolveCommitRejectsShortSHAInResponse(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v4/projects/mygroup%2Fmyrepo/repository/commits/v1.0.0", func(w http.ResponseWriter, _ *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"id": "8e8c483"}) + }) + + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", nil) + if _, err := f.Commits().ResolveCommit(context.Background(), "mygroup", "myrepo", "v1.0.0"); err == nil { + t.Fatal("ResolveCommit() error = nil, want invalid SHA error") + } +} + +func TestGitLabResolveCommitRequiresArguments(t *testing.T) { + f := New("https://gitlab.example.com", "", nil) + if _, err := f.Commits().ResolveCommit(context.Background(), "mygroup", "myrepo", ""); !errors.Is(err, forge.ErrCommitRefRequired) { + t.Errorf("ResolveCommit() error = %v, want ErrCommitRefRequired", err) + } +} diff --git a/internal/cli/pr_checkout_test.go b/internal/cli/pr_checkout_test.go index 0e7923d..a41f3d9 100644 --- a/internal/cli/pr_checkout_test.go +++ b/internal/cli/pr_checkout_test.go @@ -94,6 +94,7 @@ func (m *mockForge) Reviews() forges.ReviewService { return nil } func (m *mockForge) Files() forges.FileService { return nil } func (m *mockForge) Collaborators() forges.CollaboratorService { return nil } func (m *mockForge) CommitStatuses() forges.CommitStatusService { return nil } +func (m *mockForge) Commits() forges.CommitService { return nil } func (m *mockForge) GetRateLimit(_ context.Context) (*forges.RateLimit, error) { return nil, forges.ErrNotSupported } diff --git a/tangled/tangled.go b/tangled/tangled.go index 172737c..8953a83 100644 --- a/tangled/tangled.go +++ b/tangled/tangled.go @@ -63,6 +63,7 @@ func (f *tangledForge) Collaborators() forges.CollaboratorService { func (f *tangledForge) CommitStatuses() forges.CommitStatusService { return unsupportedCommitStatusService{} } +func (f *tangledForge) Commits() forges.CommitService { return unsupportedCommitService{} } func (f *tangledForge) GetRateLimit(context.Context) (*forges.RateLimit, error) { return nil, forges.ErrNotSupported diff --git a/tangled/unsupported.go b/tangled/unsupported.go index 93a9190..fc5cd9f 100644 --- a/tangled/unsupported.go +++ b/tangled/unsupported.go @@ -250,3 +250,9 @@ func (unsupportedCommitStatusService) List(context.Context, string, string, stri func (unsupportedCommitStatusService) Set(context.Context, string, string, string, forges.SetCommitStatusOpts) (*forges.CommitStatus, error) { return nil, forges.ErrNotSupported } + +type unsupportedCommitService struct{} + +func (unsupportedCommitService) ResolveCommit(context.Context, string, string, string) (string, error) { + return "", forges.ErrNotSupported +}