diff --git a/github/git_refs.go b/github/git_refs.go index a6269e5a9e4..94053aaea1c 100644 --- a/github/git_refs.go +++ b/github/git_refs.go @@ -7,8 +7,6 @@ package github import ( "context" - "encoding/json" - "errors" "fmt" "net/url" "strings" @@ -49,16 +47,12 @@ type updateRefRequest struct { Force *bool `json:"force"` } -// GetRef fetches a single Reference object for a given Git ref. -// If there is no exact match, GetRef will return an error. +// GetRef fetches a single reference in a repository. // -// Note: The GitHub API can return multiple matches. -// If you wish to use this functionality please use the GetRefs() method. -// -// GitHub API docs: https://developer.github.com/v3/git/refs/#get-a-reference +// GitHub API docs: https://developer.github.com/v3/git/refs/#get-a-single-reference func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref string) (*Reference, *Response, error) { ref = strings.TrimPrefix(ref, "refs/") - u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(ref)) + u := fmt.Sprintf("repos/%v/%v/git/ref/%v", owner, repo, refURLEscape(ref)) req, err := s.client.NewRequest("GET", u, nil) if err != nil { return nil, nil, err @@ -66,13 +60,7 @@ func (s *GitService) GetRef(ctx context.Context, owner string, repo string, ref r := new(Reference) resp, err := s.client.Do(ctx, req, r) - if _, ok := err.(*json.UnmarshalTypeError); ok { - // Multiple refs, means there wasn't an exact match. - return nil, resp, errors.New("multiple matches found for this ref") - } else if _, ok := err.(*ErrorResponse); ok && resp.StatusCode == 404 { - // No ref, there was no match for the ref - return nil, resp, errors.New("no match found for this ref") - } else if err != nil { + if err != nil { return nil, resp, err } @@ -89,69 +77,24 @@ func refURLEscape(ref string) string { return strings.Join(parts, "/") } -// GetRefs fetches a slice of Reference objects for a given Git ref. -// If there is an exact match, only that ref is returned. -// If there is no exact match, GitHub returns all refs that start with ref. -// If returned error is nil, there will be at least 1 ref returned. -// For example: -// -// "heads/featureA" -> ["refs/heads/featureA"] // Exact match, single ref is returned. -// "heads/feature" -> ["refs/heads/featureA", "refs/heads/featureB"] // All refs that start with ref. -// "heads/notexist" -> [] // Returns an error. -// -// GitHub API docs: https://developer.github.com/v3/git/refs/#get-a-reference -func (s *GitService) GetRefs(ctx context.Context, owner string, repo string, ref string) ([]*Reference, *Response, error) { - ref = strings.TrimPrefix(ref, "refs/") - u := fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, refURLEscape(ref)) - req, err := s.client.NewRequest("GET", u, nil) - if err != nil { - return nil, nil, err - } - - var rawJSON json.RawMessage - resp, err := s.client.Do(ctx, req, &rawJSON) - if err != nil { - return nil, resp, err - } - - // Prioritize the most common case: a single returned ref. - r := new(Reference) - singleUnmarshalError := json.Unmarshal(rawJSON, r) - if singleUnmarshalError == nil { - return []*Reference{r}, resp, nil - } - - // Attempt to unmarshal multiple refs. - var rs []*Reference - multipleUnmarshalError := json.Unmarshal(rawJSON, &rs) - if multipleUnmarshalError == nil { - if len(rs) == 0 { - return nil, resp, fmt.Errorf("unexpected response from GitHub API: an array of refs with length 0") - } - return rs, resp, nil - } - - return nil, resp, fmt.Errorf("unmarshalling failed for both single and multiple refs: %s and %s", singleUnmarshalError, multipleUnmarshalError) -} - // ReferenceListOptions specifies optional parameters to the -// GitService.ListRefs method. +// GitService.ListMatchingRefs method. type ReferenceListOptions struct { - Type string `url:"-"` + Ref string `url:"-"` ListOptions } -// ListRefs lists all refs in a repository. +// ListMatchingRefs lists references in a repository that match a supplied ref. +// Use an empty ref to list all references. // -// GitHub API docs: https://developer.github.com/v3/git/refs/#get-all-references -func (s *GitService) ListRefs(ctx context.Context, owner, repo string, opts *ReferenceListOptions) ([]*Reference, *Response, error) { - var u string - if opts != nil && opts.Type != "" { - u = fmt.Sprintf("repos/%v/%v/git/refs/%v", owner, repo, opts.Type) - } else { - u = fmt.Sprintf("repos/%v/%v/git/refs", owner, repo) +// GitHub API docs: https://developer.github.com/v3/git/refs/#list-matching-references +func (s *GitService) ListMatchingRefs(ctx context.Context, owner, repo string, opts *ReferenceListOptions) ([]*Reference, *Response, error) { + var ref string + if opts != nil { + ref = strings.TrimPrefix(opts.Ref, "refs/") } + u := fmt.Sprintf("repos/%v/%v/git/matching-refs/%v", owner, repo, refURLEscape(ref)) u, err := addOptions(u, opts) if err != nil { return nil, nil, err diff --git a/github/git_refs_test.go b/github/git_refs_test.go index 22dc8eb6247..73c193b20f1 100644 --- a/github/git_refs_test.go +++ b/github/git_refs_test.go @@ -19,7 +19,7 @@ func TestGitService_GetRef_singleRef(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/ref/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, ` { @@ -64,75 +64,44 @@ func TestGitService_GetRef_noRefs(t *testing.T) { mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") w.WriteHeader(http.StatusNotFound) - fmt.Fprint(w, "[]") }) - _, _, err := client.Git.GetRef(context.Background(), "o", "r", "refs/heads/b") - want := "no match found for this ref" - if err.Error() != want { - t.Errorf("Git.GetRef returned %+v, want %+v", err, want) + ref, resp, err := client.Git.GetRef(context.Background(), "o", "r", "refs/heads/b") + if err == nil { + t.Errorf("Expected HTTP 404 response") + } + if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want { + t.Errorf("Git.GetRef returned status %d, want %d", got, want) + } + if ref != nil { + t.Errorf("Git.GetRef return %+v, want nil", ref) } } -func TestGitService_GetRef_multipleRefs(t *testing.T) { +func TestGitService_ListMatchingRefs_singleRef(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/matching-refs/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, ` [ { - "ref": "refs/heads/booger", - "url": "https://api.github.com/repos/o/r/git/refs/heads/booger", - "object": { - "type": "commit", - "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", - "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" - } - }, - { - "ref": "refs/heads/bandsaw", - "url": "https://api.github.com/repos/o/r/git/refs/heads/bandsaw", + "ref": "refs/heads/b", + "url": "https://api.github.com/repos/o/r/git/refs/heads/b", "object": { "type": "commit", - "sha": "612077ae6dffb4d2fbd8ce0cccaa58893b07b5ac", - "url": "https://api.github.com/repos/o/r/git/commits/612077ae6dffb4d2fbd8ce0cccaa58893b07b5ac" + "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", + "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" } } - ] - `) - }) - - _, _, err := client.Git.GetRef(context.Background(), "o", "r", "refs/heads/b") - want := "multiple matches found for this ref" - if err.Error() != want { - t.Errorf("Git.GetRef returned %+v, want %+v", err, want) - } - -} - -func TestGitService_GetRefs_singleRef(t *testing.T) { - client, mux, _, teardown := setup() - defer teardown() - - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { - testMethod(t, r, "GET") - fmt.Fprint(w, ` - { - "ref": "refs/heads/b", - "url": "https://api.github.com/repos/o/r/git/refs/heads/b", - "object": { - "type": "commit", - "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", - "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" - } - }`) + ]`) }) - refs, _, err := client.Git.GetRefs(context.Background(), "o", "r", "refs/heads/b") + opts := &ReferenceListOptions{Ref: "refs/heads/b"} + refs, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", opts) if err != nil { - t.Fatalf("Git.GetRefs returned error: %v", err) + t.Fatalf("Git.ListMatchingRefs returned error: %v", err) } ref := refs[0] @@ -146,20 +115,21 @@ func TestGitService_GetRefs_singleRef(t *testing.T) { }, } if !reflect.DeepEqual(ref, want) { - t.Errorf("Git.GetRefs returned %+v, want %+v", ref, want) + t.Errorf("Git.ListMatchingRefs returned %+v, want %+v", ref, want) } // without 'refs/' prefix - if _, _, err := client.Git.GetRefs(context.Background(), "o", "r", "heads/b"); err != nil { - t.Errorf("Git.GetRefs returned error: %v", err) + opts = &ReferenceListOptions{Ref: "heads/b"} + if _, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", opts); err != nil { + t.Errorf("Git.ListMatchingRefs returned error: %v", err) } } -func TestGitService_GetRefs_multipleRefs(t *testing.T) { +func TestGitService_ListMatchingRefs_multipleRefs(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/matching-refs/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, ` [ @@ -185,9 +155,10 @@ func TestGitService_GetRefs_multipleRefs(t *testing.T) { `) }) - refs, _, err := client.Git.GetRefs(context.Background(), "o", "r", "refs/heads/b") + opts := &ReferenceListOptions{Ref: "refs/heads/b"} + refs, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", opts) if err != nil { - t.Errorf("Git.GetRefs returned error: %v", err) + t.Errorf("Git.ListMatchingRefs returned error: %v", err) } want := &Reference{ @@ -200,33 +171,35 @@ func TestGitService_GetRefs_multipleRefs(t *testing.T) { }, } if !reflect.DeepEqual(refs[0], want) { - t.Errorf("Git.GetRefs returned %+v, want %+v", refs[0], want) + t.Errorf("Git.ListMatchingRefs returned %+v, want %+v", refs[0], want) } } -// TestGitService_GetRefs_noRefs tests for behaviour resulting from an unexpected GH response. This should never actually happen. -func TestGitService_GetRefs_noRefs(t *testing.T) { +func TestGitService_ListMatchingRefs_noRefs(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/matching-refs/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, "[]") }) - _, _, err := client.Git.GetRefs(context.Background(), "o", "r", "refs/heads/b") - want := "unexpected response from GitHub API: an array of refs with length 0" - if err.Error() != want { - t.Errorf("Git.GetRefs returned %+v, want %+v", err, want) + opts := &ReferenceListOptions{Ref: "refs/heads/b"} + refs, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", opts) + if err != nil { + t.Errorf("Git.ListMatchingRefs returned error: %v", err) } + if len(refs) != 0 { + t.Errorf("Git.ListMatchingRefs returned %+v, want an empty slice", refs) + } } -func TestGitService_ListRefs(t *testing.T) { +func TestGitService_ListMatchingRefs_allRefs(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/matching-refs/", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") fmt.Fprint(w, ` [ @@ -234,26 +207,26 @@ func TestGitService_ListRefs(t *testing.T) { "ref": "refs/heads/branchA", "url": "https://api.github.com/repos/o/r/git/refs/heads/branchA", "object": { - "type": "commit", - "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", - "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" + "type": "commit", + "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", + "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" } }, { "ref": "refs/heads/branchB", "url": "https://api.github.com/repos/o/r/git/refs/heads/branchB", "object": { - "type": "commit", - "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", - "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" + "type": "commit", + "sha": "aa218f56b14c9653891f9e74264a383fa43fefbd", + "url": "https://api.github.com/repos/o/r/git/commits/aa218f56b14c9653891f9e74264a383fa43fefbd" } } ]`) }) - refs, _, err := client.Git.ListRefs(context.Background(), "o", "r", nil) + refs, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", nil) if err != nil { - t.Errorf("Git.ListRefs returned error: %v", err) + t.Errorf("Git.ListMatchingRefs returned error: %v", err) } want := []*Reference{ @@ -277,29 +250,29 @@ func TestGitService_ListRefs(t *testing.T) { }, } if !reflect.DeepEqual(refs, want) { - t.Errorf("Git.ListRefs returned %+v, want %+v", refs, want) + t.Errorf("Git.ListMatchingRefs returned %+v, want %+v", refs, want) } } -func TestGitService_ListRefs_options(t *testing.T) { +func TestGitService_ListMatchingRefs_options(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/t", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/matching-refs/t", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") testFormValues(t, r, values{"page": "2"}) fmt.Fprint(w, `[{"ref": "r"}]`) }) - opts := &ReferenceListOptions{Type: "t", ListOptions: ListOptions{Page: 2}} - refs, _, err := client.Git.ListRefs(context.Background(), "o", "r", opts) + opts := &ReferenceListOptions{Ref: "t", ListOptions: ListOptions{Page: 2}} + refs, _, err := client.Git.ListMatchingRefs(context.Background(), "o", "r", opts) if err != nil { - t.Errorf("Git.ListRefs returned error: %v", err) + t.Errorf("Git.ListMatchingRefs returned error: %v", err) } want := []*Reference{{Ref: String("r")}} if !reflect.DeepEqual(refs, want) { - t.Errorf("Git.ListRefs returned %+v, want %+v", refs, want) + t.Errorf("Git.ListMatchingRefs returned %+v, want %+v", refs, want) } } @@ -450,7 +423,7 @@ func TestGitService_GetRef_pathEscape(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - mux.HandleFunc("/repos/o/r/git/refs/heads/b", func(w http.ResponseWriter, r *http.Request) { + mux.HandleFunc("/repos/o/r/git/ref/heads/b", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") if strings.Contains(r.URL.RawPath, "%2F") { t.Errorf("RawPath still contains escaped / as %%2F: %v", r.URL.RawPath) diff --git a/update-urls/main.go b/update-urls/main.go index 73c2e9ca3aa..3c07812467d 100644 --- a/update-urls/main.go +++ b/update-urls/main.go @@ -61,9 +61,6 @@ var ( "AppsService.FindRepositoryInstallationByID": true, "AuthorizationsService.CreateImpersonation": true, "AuthorizationsService.DeleteImpersonation": true, - "GitService.GetRef": true, - "GitService.GetRefs": true, - "GitService.ListRefs": true, "MarketplaceService.marketplaceURI": true, "OrganizationsService.GetByID": true, "RepositoriesService.DeletePreReceiveHook": true,