diff --git a/github/github.go b/github/github.go index 2c2d1b4..14b5328 100644 --- a/github/github.go +++ b/github/github.go @@ -13,9 +13,12 @@ import ( "io/ioutil" "net/http" "net/url" + "reflect" "strconv" "strings" "time" + + "github.com/google/go-querystring/query" ) const ( @@ -65,10 +68,32 @@ type Client struct { // support pagination. type ListOptions struct { // For paginated result sets, page of results to retrieve. - Page int + Page int `url:"page,omitempty"` // For paginated result sets, the number of results to include per page. - PerPage int + PerPage int `url:"per_page,omitempty"` +} + +// addOptions adds the parameters in opt as URL query parameters to s. opt +// must be a struct whose fields may contain "url" tags. +func addOptions(s string, opt interface{}) (string, error) { + v := reflect.ValueOf(opt) + if v.Kind() == reflect.Ptr && v.IsNil() { + return s, nil + } + + u, err := url.Parse(s) + if err != nil { + return s, err + } + + qs, err := query.Values(opt) + if err != nil { + return s, err + } + + u.RawQuery = qs.Encode() + return u.String(), nil } // NewClient returns a new GitHub API client. If a nil httpClient is diff --git a/github/github_test.go b/github/github_test.go index 8612c3b..0bde882 100644 --- a/github/github_test.go +++ b/github/github_test.go @@ -57,10 +57,14 @@ func testMethod(t *testing.T, r *http.Request, want string) { type values map[string]string func testFormValues(t *testing.T, r *http.Request, values values) { - for key, want := range values { - if v := r.FormValue(key); v != want { - t.Errorf("Request parameter %v = %v, want %v", key, v, want) - } + want := url.Values{} + for k, v := range values { + want.Add(k, v) + } + + r.ParseForm() + if !reflect.DeepEqual(want, r.Form) { + t.Errorf("Request parameters = %v, want %v", r.Form, want) } } diff --git a/github/repos.go b/github/repos.go index 6ce7ac4..c5246c7 100644 --- a/github/repos.go +++ b/github/repos.go @@ -5,11 +5,7 @@ package github -import ( - "fmt" - "net/url" - "strconv" -) +import "fmt" // RepositoriesService handles communication with the repository related // methods of the GitHub API. @@ -59,18 +55,17 @@ func (r Repository) String() string { type RepositoryListOptions struct { // Type of repositories to list. Possible values are: all, owner, public, // private, member. Default is "all". - Type string + Type string `url:"type,omitempty"` // How to sort the repository list. Possible values are: created, updated, // pushed, full_name. Default is "full_name". - Sort string + Sort string `url:"sort,omitempty"` // Direction in which to sort repositories. Possible values are: asc, desc. // Default is "asc" when sort is "full_name", otherwise default is "desc". - Direction string + Direction string `url:"direction,omitempty"` - // For paginated result sets, page of results to retrieve. - Page int + ListOptions } // List the repositories for a user. Passing the empty string will list @@ -84,14 +79,9 @@ func (s *RepositoriesService) List(user string, opt *RepositoryListOptions) ([]R } else { u = "user/repos" } - if opt != nil { - params := url.Values{ - "type": []string{opt.Type}, - "sort": []string{opt.Sort}, - "direction": []string{opt.Direction}, - "page": []string{strconv.Itoa(opt.Page)}, - } - u += "?" + params.Encode() + u, err := addOptions(u, opt) + if err != nil { + return nil, nil, err } req, err := s.client.NewRequest("GET", u, nil) @@ -109,10 +99,9 @@ func (s *RepositoriesService) List(user string, opt *RepositoryListOptions) ([]R type RepositoryListByOrgOptions struct { // Type of repositories to list. Possible values are: all, public, private, // forks, sources, member. Default is "all". - Type string + Type string `url:"type,omitempty"` - // For paginated result sets, page of results to retrieve. - Page int + ListOptions } // ListByOrg lists the repositories for an organization. @@ -120,12 +109,9 @@ type RepositoryListByOrgOptions struct { // GitHub API docs: http://developer.github.com/v3/repos/#list-organization-repositories func (s *RepositoriesService) ListByOrg(org string, opt *RepositoryListByOrgOptions) ([]Repository, *Response, error) { u := fmt.Sprintf("orgs/%v/repos", org) - if opt != nil { - params := url.Values{ - "type": []string{opt.Type}, - "page": []string{strconv.Itoa(opt.Page)}, - } - u += "?" + params.Encode() + u, err := addOptions(u, opt) + if err != nil { + return nil, nil, err } req, err := s.client.NewRequest("GET", u, nil) @@ -142,19 +128,18 @@ func (s *RepositoriesService) ListByOrg(org string, opt *RepositoryListByOrgOpti // RepositoriesService.ListAll method. type RepositoryListAllOptions struct { // ID of the last repository seen - Since int + Since int `url:"since,omitempty"` + + ListOptions } // ListAll lists all GitHub repositories in the order that they were created. // -// GitHub API docs: http://developer.github.com/v3/repos/#list-all-repositories +// GitHub API docs: http://developer.github.com/v3/repos/#list-all-public-repositories func (s *RepositoriesService) ListAll(opt *RepositoryListAllOptions) ([]Repository, *Response, error) { - u := "repositories" - if opt != nil { - params := url.Values{ - "since": []string{strconv.Itoa(opt.Since)}, - } - u += "?" + params.Encode() + u, err := addOptions("repositories", opt) + if err != nil { + return nil, nil, err } req, err := s.client.NewRequest("GET", u, nil) diff --git a/github/repos_test.go b/github/repos_test.go index 444a312..d30d32c 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -49,7 +49,7 @@ func TestRepositoriesService_List_specifiedUser(t *testing.T) { fmt.Fprint(w, `[{"id":1}]`) }) - opt := &RepositoryListOptions{"owner", "created", "asc", 2} + opt := &RepositoryListOptions{"owner", "created", "asc", ListOptions{Page: 2}} repos, _, err := client.Repositories.List("u", opt) if err != nil { t.Errorf("Repositories.List returned error: %v", err) @@ -79,7 +79,7 @@ func TestRepositoriesService_ListByOrg(t *testing.T) { fmt.Fprint(w, `[{"id":1}]`) }) - opt := &RepositoryListByOrgOptions{"forks", 2} + opt := &RepositoryListByOrgOptions{"forks", ListOptions{Page: 2}} repos, _, err := client.Repositories.ListByOrg("o", opt) if err != nil { t.Errorf("Repositories.ListByOrg returned error: %v", err) @@ -102,11 +102,15 @@ func TestRepositoriesService_ListAll(t *testing.T) { mux.HandleFunc("/repositories", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testFormValues(t, r, values{"since": "1"}) + testFormValues(t, r, values{ + "since": "1", + "page": "2", + "per_page": "3", + }) fmt.Fprint(w, `[{"id":1}]`) }) - opt := &RepositoryListAllOptions{1} + opt := &RepositoryListAllOptions{1, ListOptions{2, 3}} repos, _, err := client.Repositories.ListAll(opt) if err != nil { t.Errorf("Repositories.ListAll returned error: %v", err) diff --git a/github/users.go b/github/users.go index c092bbc..1414aa5 100644 --- a/github/users.go +++ b/github/users.go @@ -7,8 +7,7 @@ package github import ( "fmt" - "net/url" - "strconv" + "time" ) @@ -83,19 +82,16 @@ func (s *UsersService) Edit(user *User) (*User, *Response, error) { // method. type UserListOptions struct { // ID of the last user seen - Since int + Since int `url:"since,omitempty"` } // ListAll lists all GitHub users. // // GitHub API docs: http://developer.github.com/v3/users/#get-all-users func (s *UsersService) ListAll(opt *UserListOptions) ([]User, *Response, error) { - u := "users" - if opt != nil { - params := url.Values{ - "since": []string{strconv.Itoa(opt.Since)}, - } - u += "?" + params.Encode() + u, err := addOptions("users", opt) + if err != nil { + return nil, nil, err } req, err := s.client.NewRequest("GET", u, nil)