Skip to content

Commit

Permalink
Use new endpoints to get and list references (google#1513)
Browse files Browse the repository at this point in the history 
Fixes google#1512.
  • Loading branch information
@bluekeyes
bluekeyes authored May 7, 2020
1 parent c9eb4a1 commit 1cf9115
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 158 deletions.
85 changes: 14 additions & 71 deletions github/git_refs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ package github

import (
"context"
"encoding/json"
"errors"
"fmt"
"net/url"
"strings"
Expand Down Expand Up @@ -49,30 +47,20 @@ 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
}

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
}

Expand All @@ -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
Expand Down
141 changes: 57 additions & 84 deletions github/git_refs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, `
{
Expand Down Expand Up @@ -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]
Expand All @@ -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, `
[
Expand All @@ -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{
Expand All @@ -200,60 +171,62 @@ 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, `
[
{
"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{
Expand All @@ -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)
}
}

Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 1cf9115

Please sign in to comment.