diff --git a/githubapi/client.go b/githubapi/client.go index 327684c..f8374fe 100644 --- a/githubapi/client.go +++ b/githubapi/client.go @@ -342,28 +342,34 @@ func (client *Client) createCommit(ctx context.Context, req cpgo.UpsertFileReque // updateHeadRef force-updates the branch ref, creating it when absent. func (client *Client) updateHeadRef(ctx context.Context, repository cpgo.RepositoryRef, headBranch string, commitSHA string) (bool, error) { - _, _, err := client.githubClient.Git.UpdateRef(ctx, repository.Owner, repository.Name, "heads/"+headBranch, github.UpdateRef{ - SHA: commitSHA, - Force: new(true), - }) + refName := "heads/" + headBranch + _, _, err := client.githubClient.Git.GetRef(ctx, repository.Owner, repository.Name, refName) if err == nil { + _, _, err = client.githubClient.Git.UpdateRef(ctx, repository.Owner, repository.Name, refName, github.UpdateRef{ + SHA: commitSHA, + Force: new(true), + }) + if err != nil { + return false, fmt.Errorf("force update branch ref: %w", err) + } + return false, nil } - if !isNotFound(err) && !isReferenceMissing(err) { - return false, fmt.Errorf("force update branch ref: %w", err) + if !isNotFound(err) { + return false, fmt.Errorf("get head branch ref: %w", err) } _, _, err = client.githubClient.Git.CreateRef(ctx, repository.Owner, repository.Name, github.CreateRef{ - Ref: "refs/heads/" + headBranch, + Ref: "refs/" + refName, SHA: commitSHA, }) if err == nil { return true, nil } - // The branch may have been created concurrently after the initial update attempt. - _, _, updateErr := client.githubClient.Git.UpdateRef(ctx, repository.Owner, repository.Name, "heads/"+headBranch, github.UpdateRef{ + // The branch may have been created concurrently after the lookup. + _, _, updateErr := client.githubClient.Git.UpdateRef(ctx, repository.Owner, repository.Name, refName, github.UpdateRef{ SHA: commitSHA, Force: new(true), }) @@ -387,29 +393,6 @@ func isNotFound(err error) bool { return githubError.Response.StatusCode == http.StatusNotFound } -func isReferenceMissing(err error) bool { - var githubError *github.ErrorResponse - if !errors.As(err, &githubError) { - return false - } - - if githubError.Response == nil || githubError.Response.StatusCode != http.StatusUnprocessableEntity { - return false - } - - if strings.Contains(strings.ToLower(githubError.Message), "reference does not exist") { - return true - } - - for _, item := range githubError.Errors { - if strings.Contains(strings.ToLower(item.Message), "reference does not exist") { - return true - } - } - - return false -} - func validateRepositoryRef(repository cpgo.RepositoryRef) error { if strings.TrimSpace(repository.Owner) == "" { return fmt.Errorf("repository owner is required") diff --git a/githubapi/client_test.go b/githubapi/client_test.go index 8a26ec6..257e2df 100644 --- a/githubapi/client_test.go +++ b/githubapi/client_test.go @@ -133,6 +133,7 @@ func TestClientFindOpenByHead(t *testing.T) { func TestClientUpsertFileAndForceBranch(t *testing.T) { encodedProfile := base64.StdEncoding.EncodeToString([]byte("new-profile")) createRefCalled := false + updateRefCalled := false githubClient := newGitHubClient(t, http.HandlerFunc(func(response http.ResponseWriter, req *http.Request) { switch req.URL.Path { @@ -162,9 +163,18 @@ func TestClientUpsertFileAndForceBranch(t *testing.T) { _, _ = response.Write([]byte(`{"sha":"tree-sha"}`)) case "/repos/acme/payments/git/commits": _, _ = response.Write([]byte(`{"sha":"commit-sha"}`)) - case "/repos/acme/payments/git/refs/heads/cpgo": - response.WriteHeader(http.StatusUnprocessableEntity) - _, _ = response.Write([]byte(`{"message":"Reference does not exist","errors":[]}`)) + case "/repos/acme/payments/git/ref/heads/cpgo": + switch req.Method { + case http.MethodGet: + response.WriteHeader(http.StatusNotFound) + _, _ = response.Write([]byte(`{"message":"Not Found"}`)) + case http.MethodPatch: + updateRefCalled = true + response.WriteHeader(http.StatusForbidden) + _, _ = response.Write([]byte(`{"message":"Resource not accessible by integration"}`)) + default: + t.Fatalf("unexpected request method: %s", req.Method) + } case "/repos/acme/payments/git/refs": createRefCalled = true var payload struct { @@ -216,6 +226,54 @@ func TestClientUpsertFileAndForceBranch(t *testing.T) { if !createRefCalled { t.Fatalf("expected create ref call") } + + if updateRefCalled { + t.Fatalf("expected missing branch to be created without update attempt") + } +} + +func TestClientUpdateHeadRef(t *testing.T) { + githubClient := newGitHubClient(t, http.HandlerFunc(func(response http.ResponseWriter, req *http.Request) { + switch { + case req.Method == http.MethodGet && req.URL.Path == "/repos/acme/payments/git/ref/heads/cpgo": + _, _ = response.Write([]byte(`{"ref":"refs/heads/cpgo","object":{"type":"commit","sha":"old-commit"}}`)) + case req.Method == http.MethodPatch && req.URL.Path == "/repos/acme/payments/git/refs/heads/cpgo": + var payload struct { + SHA string `json:"sha"` + Force bool `json:"force"` + } + if err := json.NewDecoder(req.Body).Decode(&payload); err != nil { + t.Fatalf("decode update ref request: %v", err) + } + + if payload.SHA != "new-commit" { + t.Fatalf("expected new-commit, got %s", payload.SHA) + } + + if !payload.Force { + t.Fatalf("expected forced update") + } + + _, _ = response.Write([]byte(`{"ref":"refs/heads/cpgo","object":{"type":"commit","sha":"new-commit"}}`)) + default: + t.Fatalf("unexpected request: %s %s", req.Method, req.URL.Path) + } + })) + + client := mustNewClient(t, githubClient) + isBranchCreated, err := client.updateHeadRef( + context.Background(), + cpgo.RepositoryRef{Owner: "acme", Name: "payments"}, + "cpgo", + "new-commit", + ) + if err != nil { + t.Fatalf("update head ref: %v", err) + } + + if isBranchCreated { + t.Fatalf("expected existing branch update") + } } func mustNewClient(t *testing.T, githubClient *github.Client) *Client {