From 9735dbccdbc3898ec367afa9284041ffe40295ef Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sat, 4 Nov 2023 15:28:38 -0500 Subject: [PATCH 1/2] Create ListByUser and ListByAuthenticatedUser methods --- github/examples_test.go | 6 +- github/repos.go | 148 ++++++++++++++++++++++++++------- github/repos_test.go | 66 +++++++-------- test/integration/repos_test.go | 20 +++-- 4 files changed, 166 insertions(+), 74 deletions(-) diff --git a/github/examples_test.go b/github/examples_test.go index ab319151786..9338e0b6170 100644 --- a/github/examples_test.go +++ b/github/examples_test.go @@ -49,14 +49,14 @@ func ExampleRepositoriesService_GetReadme() { fmt.Printf("google/go-github README:\n%v\n", content) } -func ExampleRepositoriesService_List() { +func ExampleRepositoriesService_ListByUser() { client := github.NewClient(nil) user := "willnorris" - opt := &github.RepositoryListOptions{Type: "owner", Sort: "updated", Direction: "desc"} + opt := &github.RepositoryListByUserOptions{Type: "owner", Sort: "updated", Direction: "desc"} ctx := context.Background() - repos, _, err := client.Repositories.List(ctx, user, opt) + repos, _, err := client.Repositories.ListByUser(ctx, user, opt) if err != nil { fmt.Println(err) } diff --git a/github/repos.go b/github/repos.go index b16cd3b8159..5fcf219b3cf 100644 --- a/github/repos.go +++ b/github/repos.go @@ -173,33 +173,19 @@ type BranchListOptions struct { // RepositoryListOptions specifies the optional parameters to the // RepositoriesService.List method. type RepositoryListOptions struct { - // Visibility of repositories to list. Can be one of all, public, or private. - // Default: all + // See RepositoryListByAuthenticatedUserOptions.Visibility Visibility string `url:"visibility,omitempty"` - // List repos of given affiliation[s]. - // Comma-separated list of values. Can include: - // * owner: Repositories that are owned by the authenticated user. - // * collaborator: Repositories that the user has been added to as a - // collaborator. - // * organization_member: Repositories that the user has access to through - // being a member of an organization. This includes every repository on - // every team that the user is on. - // Default: owner,collaborator,organization_member + // See RepositoryListByAuthenticatedUserOptions.Affiliation Affiliation string `url:"affiliation,omitempty"` - // Type of repositories to list. - // Can be one of all, owner, public, private, member. Default: all - // Will cause a 422 error if used in the same request as visibility or - // affiliation. + // See RepositoryListByUserOptions.Type or RepositoryListByAuthenticatedUserOptions.Type Type string `url:"type,omitempty"` - // How to sort the repository list. Can be one of created, updated, pushed, - // full_name. Default: full_name + // See RepositoryListByUserOptions.Sort or RepositoryListByAuthenticatedUserOptions.Sort Sort string `url:"sort,omitempty"` - // Direction in which to sort repositories. Can be one of asc or desc. - // Default: when using full_name: asc; otherwise desc + // See RepositoryListByUserOptions.Direction or RepositoryListByAuthenticatedUserOptions.Direction Direction string `url:"direction,omitempty"` ListOptions @@ -262,8 +248,10 @@ func (d DependabotSecurityUpdates) String() string { return Stringify(d) } -// List the repositories for a user. Passing the empty string will list -// repositories for the authenticated user. +// List calls either RepositoriesService.ListByUser or RepositoriesService.ListByAuthenticatedUser +// depending on whether user is empty. +// +// Deprecated: Use RepositoriesService.ListByUser or RepositoriesService.ListByAuthenticatedUser instead. // // GitHub API docs: https://docs.github.com/rest/repos/repos#list-repositories-for-a-user // GitHub API docs: https://docs.github.com/rest/repos/repos#list-repositories-for-the-authenticated-user @@ -271,12 +259,55 @@ func (d DependabotSecurityUpdates) String() string { //meta:operation GET /user/repos //meta:operation GET /users/{username}/repos func (s *RepositoriesService) List(ctx context.Context, user string, opts *RepositoryListOptions) ([]*Repository, *Response, error) { - var u string - if user != "" { - u = fmt.Sprintf("users/%v/repos", user) - } else { - u = "user/repos" + if opts == nil { + opts = &RepositoryListOptions{} } + if user != "" { + return s.ListByUser(ctx, user, &RepositoryListByUserOptions{ + Type: opts.Type, + Sort: opts.Sort, + Direction: opts.Direction, + ListOptions: opts.ListOptions, + }) + } + return s.ListByAuthenticatedUser(ctx, &RepositoryListByAuthenticatedUserOptions{ + Visibility: opts.Visibility, + Affiliation: opts.Affiliation, + Type: opts.Type, + Sort: opts.Sort, + Direction: opts.Direction, + ListOptions: opts.ListOptions, + }) +} + +// RepositoryListByUserOptions specifies the optional parameters to the +// RepositoriesService.ListByUser method. +type RepositoryListByUserOptions struct { + // Limit results to repositories of the specified type. + // Default: owner + // Can be one of: all, owner, member + Type string `url:"type,omitempty"` + + // The property to sort the results by. + // Default: full_name + // Can be one of: created, updated, pushed, full_name + Sort string `url:"sort,omitempty"` + + // The order to sort by. + // Default: asc when using full_name, otherwise desc. + // Can be one of: asc, desc + Direction string `url:"direction,omitempty"` + + ListOptions +} + +// ListByUser lists public repositories for the specified user. +// +// GitHub API docs: https://docs.github.com/rest/repos/repos#list-repositories-for-a-user +// +//meta:operation GET /users/{username}/repos +func (s *RepositoriesService) ListByUser(ctx context.Context, user string, opts *RepositoryListByUserOptions) ([]*Repository, *Response, error) { + u := fmt.Sprintf("users/%v/repos", user) u, err := addOptions(u, opts) if err != nil { return nil, nil, err @@ -287,9 +318,68 @@ func (s *RepositoriesService) List(ctx context.Context, user string, opts *Repos return nil, nil, err } - // TODO: remove custom Accept headers when APIs fully launch. - acceptHeaders := []string{mediaTypeTopicsPreview, mediaTypeRepositoryVisibilityPreview} - req.Header.Set("Accept", strings.Join(acceptHeaders, ", ")) + var repos []*Repository + resp, err := s.client.Do(ctx, req, &repos) + if err != nil { + return nil, resp, err + } + + return repos, resp, nil +} + +// RepositoryListByAuthenticatedUserOptions specifies the optional parameters to the +// RepositoriesService.ListByAuthenticatedUser method. +type RepositoryListByAuthenticatedUserOptions struct { + // Limit results to repositories with the specified visibility. + // Default: all + // Can be one of: all, public, private + Visibility string `url:"visibility,omitempty"` + + // List repos of given affiliation[s]. + // Comma-separated list of values. Can include: + // * owner: Repositories that are owned by the authenticated user. + // * collaborator: Repositories that the user has been added to as a + // collaborator. + // * organization_member: Repositories that the user has access to through + // being a member of an organization. This includes every repository on + // every team that the user is on. + // Default: owner,collaborator,organization_member + Affiliation string `url:"affiliation,omitempty"` + + // Limit results to repositories of the specified type. Will cause a 422 error if + // used in the same request as visibility or affiliation. + // Default: all + // Can be one of: all, owner, public, private, member + Type string `url:"type,omitempty"` + + // The property to sort the results by. + // Default: full_name + // Can be one of: created, updated, pushed, full_name + Sort string `url:"sort,omitempty"` + + // Direction in which to sort repositories. Can be one of asc or desc. + // Default: when using full_name: asc; otherwise desc + Direction string `url:"direction,omitempty"` + + ListOptions +} + +// ListByAuthenticatedUser lists repositories for the authenticated user. +// +// GitHub API docs: https://docs.github.com/rest/repos/repos#list-repositories-for-the-authenticated-user +// +//meta:operation GET /user/repos +func (s *RepositoriesService) ListByAuthenticatedUser(ctx context.Context, opts *RepositoryListByAuthenticatedUserOptions) ([]*Repository, *Response, error) { + u := "user/repos" + u, err := addOptions(u, opts) + if err != nil { + return nil, nil, err + } + + req, err := s.client.NewRequest("GET", u, nil) + if err != nil { + return nil, nil, err + } var repos []*Repository resp, err := s.client.Do(ctx, req, &repos) diff --git a/github/repos_test.go b/github/repos_test.go index 2bdc1b4289d..a69f8006f16 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -18,36 +18,30 @@ import ( "github.com/google/go-cmp/cmp" ) -func TestRepositoriesService_List_authenticatedUser(t *testing.T) { +func TestRepositoriesService_ListByAuthenticatedUser(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - wantAcceptHeaders := []string{mediaTypeTopicsPreview, mediaTypeRepositoryVisibilityPreview} mux.HandleFunc("/user/repos", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testHeader(t, r, "Accept", strings.Join(wantAcceptHeaders, ", ")) fmt.Fprint(w, `[{"id":1},{"id":2}]`) }) ctx := context.Background() - got, _, err := client.Repositories.List(ctx, "", nil) + got, _, err := client.Repositories.ListByAuthenticatedUser(ctx, nil) if err != nil { t.Errorf("Repositories.List returned error: %v", err) } want := []*Repository{{ID: Int64(1)}, {ID: Int64(2)}} if !cmp.Equal(got, want) { - t.Errorf("Repositories.List returned %+v, want %+v", got, want) + t.Errorf("Repositories.ListByAuthenticatedUser returned %+v, want %+v", got, want) } - const methodName = "List" - testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.List(ctx, "\n", &RepositoryListOptions{}) - return err - }) + const methodName = "ListByAuthenticatedUser" testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.List(ctx, "", nil) + got, resp, err := client.Repositories.ListByAuthenticatedUser(ctx, nil) if got != nil { t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) } @@ -55,78 +49,84 @@ func TestRepositoriesService_List_authenticatedUser(t *testing.T) { }) } -func TestRepositoriesService_List_specifiedUser(t *testing.T) { +func TestRepositoriesService_ListByUser(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - wantAcceptHeaders := []string{mediaTypeTopicsPreview, mediaTypeRepositoryVisibilityPreview} mux.HandleFunc("/users/u/repos", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testHeader(t, r, "Accept", strings.Join(wantAcceptHeaders, ", ")) testFormValues(t, r, values{ - "visibility": "public", - "affiliation": "owner,collaborator", - "sort": "created", - "direction": "asc", - "page": "2", + "sort": "created", + "direction": "asc", + "page": "2", }) fmt.Fprint(w, `[{"id":1}]`) }) - opt := &RepositoryListOptions{ - Visibility: "public", - Affiliation: "owner,collaborator", + opt := &RepositoryListByUserOptions{ Sort: "created", Direction: "asc", ListOptions: ListOptions{Page: 2}, } ctx := context.Background() - repos, _, err := client.Repositories.List(ctx, "u", opt) + repos, _, err := client.Repositories.ListByUser(ctx, "u", opt) if err != nil { t.Errorf("Repositories.List returned error: %v", err) } want := []*Repository{{ID: Int64(1)}} if !cmp.Equal(repos, want) { - t.Errorf("Repositories.List returned %+v, want %+v", repos, want) + t.Errorf("Repositories.ListByUser returned %+v, want %+v", repos, want) } + + const methodName = "ListByUser" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.ListByUser(ctx, "\n", nil) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.ListByUser(ctx, "u", nil) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) } -func TestRepositoriesService_List_specifiedUser_type(t *testing.T) { +func TestRepositoriesService_ListByUser_type(t *testing.T) { client, mux, _, teardown := setup() defer teardown() - wantAcceptHeaders := []string{mediaTypeTopicsPreview, mediaTypeRepositoryVisibilityPreview} mux.HandleFunc("/users/u/repos", func(w http.ResponseWriter, r *http.Request) { testMethod(t, r, "GET") - testHeader(t, r, "Accept", strings.Join(wantAcceptHeaders, ", ")) testFormValues(t, r, values{ "type": "owner", }) fmt.Fprint(w, `[{"id":1}]`) }) - opt := &RepositoryListOptions{ + opt := &RepositoryListByUserOptions{ Type: "owner", } ctx := context.Background() - repos, _, err := client.Repositories.List(ctx, "u", opt) + repos, _, err := client.Repositories.ListByUser(ctx, "u", opt) if err != nil { - t.Errorf("Repositories.List returned error: %v", err) + t.Errorf("Repositories.ListByUser returned error: %v", err) } want := []*Repository{{ID: Int64(1)}} if !cmp.Equal(repos, want) { - t.Errorf("Repositories.List returned %+v, want %+v", repos, want) + t.Errorf("Repositories.ListByUser returned %+v, want %+v", repos, want) } } -func TestRepositoriesService_List_invalidUser(t *testing.T) { +func TestRepositoriesService_ListByUser_invalidUser(t *testing.T) { client, _, _, teardown := setup() defer teardown() ctx := context.Background() - _, _, err := client.Repositories.List(ctx, "%", nil) + _, _, err := client.Repositories.ListByUser(ctx, "%", nil) testURLParseError(t, err) } diff --git a/test/integration/repos_test.go b/test/integration/repos_test.go index b0fa75cb8fc..39a99fef98f 100644 --- a/test/integration/repos_test.go +++ b/test/integration/repos_test.go @@ -157,29 +157,31 @@ func TestRepositories_EditBranches(t *testing.T) { } } -func TestRepositories_List(t *testing.T) { - if !checkAuth("TestRepositories_List") { +func TestRepositories_ListByAuthenticatedUser(t *testing.T) { + if !checkAuth("TestRepositories_ListByAuthenticatedUser") { return } - _, _, err := client.Repositories.List(context.Background(), "", nil) + _, _, err := client.Repositories.ListByAuthenticatedUser(context.Background(), nil) if err != nil { - t.Fatalf("Repositories.List('') returned error: %v", err) + t.Fatalf("Repositories.ListByAuthenticatedUser() returned error: %v", err) } +} - _, _, err = client.Repositories.List(context.Background(), "google", nil) +func TestRepositories_ListByUser(t *testing.T) { + _, _, err := client.Repositories.ListByUser(context.Background(), "google", nil) if err != nil { - t.Fatalf("Repositories.List('google') returned error: %v", err) + t.Fatalf("Repositories.ListByUser('google') returned error: %v", err) } - opt := github.RepositoryListOptions{Sort: "created"} - repos, _, err := client.Repositories.List(context.Background(), "google", &opt) + opt := github.RepositoryListByUserOptions{Sort: "created"} + repos, _, err := client.Repositories.ListByUser(context.Background(), "google", &opt) if err != nil { t.Fatalf("Repositories.List('google') with Sort opt returned error: %v", err) } for i, repo := range repos { if i > 0 && (*repos[i-1].CreatedAt).Time.Before((*repo.CreatedAt).Time) { - t.Fatalf("Repositories.List('google') with default descending Sort returned incorrect order") + t.Fatalf("Repositories.ListByUser('google') with default descending Sort returned incorrect order") } } } From 11a893336a23506ae7ad15c370a9c11f6086da4d Mon Sep 17 00:00:00 2001 From: Will Roden Date: Sat, 4 Nov 2023 15:45:38 -0500 Subject: [PATCH 2/2] fix testBadOptions --- github/repos_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/github/repos_test.go b/github/repos_test.go index a69f8006f16..fa49ddd4f5c 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -81,7 +81,7 @@ func TestRepositoriesService_ListByUser(t *testing.T) { const methodName = "ListByUser" testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.ListByUser(ctx, "\n", nil) + _, _, err = client.Repositories.ListByUser(ctx, "\n", &RepositoryListByUserOptions{}) return err })