Iterator generation issues caused by incorrect or unsupported pagination options

[Not-Dhananjay-Mishra]

While trying new Iter when i run client.Users.ListAllIter it went into infinite loop, I found out that client.Users.ListAll doesn't support Page based pagination. so i decide to find if there exist more functions with similar or some sort of problems.

Here are my findings -

---

1. UsersService.ListAll don't have page acc to docs if you run ListAllIter it will go in infinite loop since its pagination is powered exclusively by the since parameter.
   

   go-github/github/users.go

   Lines 224 to 227 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/users/users#list-users
   	//
   	//meta:operation GET /users
   	func (s *UsersService) ListAll(ctx context.Context, opts *UserListOptions) ([]*User, *Response, error) {

   
   Solution : Replacing UserListOptions.ListOptions with UserListOptions.PerPage which will also result in removal of ListAllIter

---

2. PackageGetAllVersions handles two endpoints one of them doesn't support pagination acc to docs also for some reason it's Iter dosen't generated maybe because it dose not contain List as prefix.
   

   go-github/github/users_packages.go

   Lines 130 to 139 in c8fd3f7

   	// PackageGetAllVersions gets all versions of a package for a user. Passing the empty string for "user" will
   	// get versions for the authenticated user.
   	//
   	// GitHub API docs: https://docs.github.com/rest/packages/packages#list-package-versions-for-a-package-owned-by-a-user
   	//
   	// GitHub API docs: https://docs.github.com/rest/packages/packages#list-package-versions-for-a-package-owned-by-the-authenticated-user
   	//
   	//meta:operation GET /user/packages/{package_type}/{package_name}/versions
   	//meta:operation GET /users/{username}/packages/{package_type}/{package_name}/versions
   	func (s *UsersService) PackageGetAllVersions(ctx context.Context, user, packageType, packageName string, opts *PackageListOptions) ([]*PackageVersion, *Response, error) {

   
   Solution : Dividing PackageGetAllVersions into two separate functions, if Iter is required then add List to prefix of new function

---

3. SubIssueService.ListByIssue dosen't support cursor based pagination acc to docs
   

   go-github/github/sub_issue.go

   Lines 71 to 74 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/issues/sub-issues#list-sub-issues
   	//
   	//meta:operation GET /repos/{owner}/{repo}/issues/{issue_number}/sub_issues
   	func (s *SubIssueService) ListByIssue(ctx context.Context, owner, repo string, issueNumber int64, opts *IssueListOptions) ([]*SubIssue, *Response, error) {

   
   Solution : Remove IssueListOptions.ListCursorOptions

---

4. RepositoriesService.ListAllTopics also support pagination acc to docs.
   

   go-github/github/repos.go

   Lines 1928 to 1931 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/repos/repos#get-all-repository-topics
   	//
   	//meta:operation GET /repos/{owner}/{repo}/topics
   	func (s *RepositoriesService) ListAllTopics(ctx context.Context, owner, repo string) ([]string, *Response, error) {

   
   Solution : Add ListOptions

---

5. RepositoriesService.ListCustomDeploymentRuleIntegrations also support pagination acc to docs.
   btw its Iter will not generate because it doesn't return []*T kind of result
   

   go-github/github/repos_deployment_protection_rules.go

   Lines 92 to 95 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/deployments/protection-rules#list-custom-deployment-rule-integrations-available-for-an-environment
   	//
   	//meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment_protection_rules/apps
   	func (s *RepositoriesService) ListCustomDeploymentRuleIntegrations(ctx context.Context, owner, repo, environment string) (*ListCustomDeploymentRuleIntegrationsResponse, *Response, error) {

   
   Solution : Add ListOptions

---

6. RepositoriesService.ListDeploymentBranchPolicies also support pagination acc to docs.
   btw its Iter will not generate because it doesn't return []*T kind of result
   

   go-github/github/repos_deployment_branch_policies.go

   Lines 35 to 38 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/deployments/branch-policies#list-deployment-branch-policies
   	//
   	//meta:operation GET /repos/{owner}/{repo}/environments/{environment_name}/deployment-branch-policies
   	func (s *RepositoriesService) ListDeploymentBranchPolicies(ctx context.Context, owner, repo, environment string) (*DeploymentBranchPolicyResponse, *Response, error) {

   
   Solution : Add ListOptions

---

7. RepositoriesService.ListAutolinks dosen't support pagination acc to docs.
   

   go-github/github/repos_autolinks.go

   Lines 31 to 34 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/repos/autolinks#get-all-autolinks-of-a-repository
   	//
   	//meta:operation GET /repos/{owner}/{repo}/autolinks
   	func (s *RepositoriesService) ListAutolinks(ctx context.Context, owner, repo string, opts *ListOptions) ([]*Autolink, *Response, error) {

   
   Solution : Remove ListOptions

---

8. PullRequestsService.ListReviewers dosen't support pagination acc to docs.
   

   go-github/github/pulls_reviewers.go

   Lines 56 to 59 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/pulls/review-requests#get-all-requested-reviewers-for-a-pull-request
   	//
   	//meta:operation GET /repos/{owner}/{repo}/pulls/{pull_number}/requested_reviewers
   	func (s *PullRequestsService) ListReviewers(ctx context.Context, owner, repo string, number int, opts *ListOptions) (*Reviewers, *Response, error) {

   
   Solution : Remove ListOptions

---

9. OrganizationsService.ListAll don't have page acc to docs if you run ListAllIter it will go in infinite loop since its pagination is powered exclusively by the since parameter.
   

   go-github/github/orgs.go

   Lines 171 to 174 in c8fd3f7

   	// GitHub API docs: https://docs.github.com/rest/orgs/orgs#list-organizations
   	//
   	//meta:operation GET /organizations
   	func (s *OrganizationsService) ListAll(ctx context.Context, opts *OrganizationsListOptions) ([]*Organization, *Response, error) {

   
   Solution : Replacing OrganizationsListOptions.ListOptions with UserListOptions.PerPage which will also result in removal of ListAllIter

---

10. OrganizationsService.ListCodeSecurityConfigurations docs and OrganizationsService.ListCodeSecurityConfigurationRepositories docs support cursor pagination but its option are not properly made so Iter is missing for it.
    

    go-github/github/orgs_codesecurity_configurations.go

    Lines 145 to 148 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/code-security/configurations#get-code-security-configurations-for-an-organization
    	//
    	//meta:operation GET /orgs/{org}/code-security/configurations
    	func (s *OrganizationsService) ListCodeSecurityConfigurations(ctx context.Context, org string, opts *ListOrgCodeSecurityConfigurationOptions) ([]*CodeSecurityConfiguration, *Response, error) {

    
    

    go-github/github/orgs_codesecurity_configurations.go

    Lines 338 to 341 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/code-security/configurations#get-repositories-associated-with-a-code-security-configuration
    	//
    	//meta:operation GET /orgs/{org}/code-security/configurations/{configuration_id}/repositories
    	func (s *OrganizationsService) ListCodeSecurityConfigurationRepositories(ctx context.Context, org string, configurationID int64, opts *ListCodeSecurityConfigurationRepositoriesOptions) ([]*RepositoryAttachment, *Response, error) {

    
    Solution : Add ListCursorOptions to ListCodeSecurityConfigurationRepositoriesOptions and ListOrgCodeSecurityConfigurationOptions

---

11. LicensesService.List support pagination acc to docs.
    

    go-github/github/licenses.go

    Lines 63 to 66 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/licenses/licenses#get-all-commonly-used-licenses
    	//
    	//meta:operation GET /licenses
    	func (s *LicensesService) List(ctx context.Context) ([]*License, *Response, error) {

    
    Solution : Add support for ListOptions

---

12. IssuesService.List docs , IssuesService.ListByOrg docs only support normal pagination not cursor.
    

    go-github/github/issues.go

    Lines 162 to 168 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/issues/issues#list-issues-assigned-to-the-authenticated-user
    	//
    	// GitHub API docs: https://docs.github.com/rest/issues/issues#list-user-account-issues-assigned-to-the-authenticated-user
    	//
    	//meta:operation GET /issues
    	//meta:operation GET /user/issues
    	func (s *IssuesService) List(ctx context.Context, all bool, opts *IssueListOptions) ([]*Issue, *Response, error) {

    
    

    go-github/github/issues.go

    Lines 257 to 260 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/issues/issues#list-repository-issues
    	//
    	//meta:operation GET /repos/{owner}/{repo}/issues
    	func (s *IssuesService) ListByRepo(ctx context.Context, owner, repo string, opts *IssueListByRepoOptions) ([]*Issue, *Response, error) {

    
    Solution : Remove ListCursorOptions from IssueListOptions only

---

13. EnterpriseService.ListCodeSecurityConfigurations docs and EnterpriseService.ListCodeSecurityConfigurationRepositories docs support cursor pagination but its option are not properly made so Iter is missing for it.
    

    go-github/github/enterprise_codesecurity_configurations.go

    Lines 35 to 38 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/code-security/configurations#get-code-security-configurations-for-an-enterprise
    	//
    	//meta:operation GET /enterprises/{enterprise}/code-security/configurations
    	func (s *EnterpriseService) ListCodeSecurityConfigurations(ctx context.Context, enterprise string, opts *ListEnterpriseCodeSecurityConfigurationOptions) ([]*CodeSecurityConfiguration, *Response, error) {

    
    

    go-github/github/enterprise_codesecurity_configurations.go

    Lines 212 to 215 in c8fd3f7

    	// GitHub API docs: https://docs.github.com/rest/code-security/configurations#get-repositories-associated-with-an-enterprise-code-security-configuration
    	//
    	//meta:operation GET /enterprises/{enterprise}/code-security/configurations/{configuration_id}/repositories
    	func (s *EnterpriseService) ListCodeSecurityConfigurationRepositories(ctx context.Context, enterprise string, configurationID int64, opts *ListCodeSecurityConfigurationRepositoriesOptions) ([]*RepositoryAttachment, *Response, error) {

    
    Solution : Add ListCursorOptions to ListCodeSecurityConfigurationRepositoriesOptions and ListEnterpriseCodeSecurityConfigurationOptions

It take so much time to find and write all these 😵

[gmlewis]

Wow... Thank you for the detailed write-up, @Not-Dhananjay-Mishra!
Do you want to start fixing these?
One PR per endpoint is perfectly fine and maybe preferred so that each one is a focused fix.
Thoughts?

[Not-Dhananjay-Mishra]

Wow... Thank you for the detailed write-up, @Not-Dhananjay-Mishra! Do you want to start fixing these? One PR per endpoint is perfectly fine and maybe preferred so that each one is a focused fix. Thoughts?

Agreed.
Also most of these changes involve BREAKING CHANGES.

Tracking List -

• UsersService.ListAll
• PackageGetAllVersions
• SubIssueService.ListByIssue
• RepositoriesService.ListAllTopics
• RepositoriesService.ListCustomDeploymentRuleIntegrations
• RepositoriesService.ListDeploymentBranchPolicies
• RepositoriesService.ListAutolinks
• PullRequestsService.ListReviewers
• OrganizationsService.ListAll
• OrganizationsService.ListCodeSecurityConfigurations and OrganizationsService.ListCodeSecurityConfigurationRepositories
• LicensesService.List
• IssuesService.List, IssuesService.ListByOrg
• EnterpriseService.ListCodeSecurityConfigurations and EnterpriseService.ListCodeSecurityConfigurationRepositories

[Not-Dhananjay-Mishra]

Also I want to ask about one more thing, I found many endpoints which support pagination but we can't generate Iter for them because they don't return []*T type of result rather they return a struct which contains []*T type of response.
Can we manually make Iterator for them?

[gmlewis]

Also I want to ask about one more thing, I found many endpoints which support pagination but we can't generate Iter for them because they don't return []*T type of result rather they return a struct which contains []*T type of response. Can we manually make Iterator for them?

We already have one special case in gen-iterators.go and we can certainly add more.

[Not-Dhananjay-Mishra]

Also I want to ask about one more thing, I found many endpoints which support pagination but we can't generate Iter for them because they don't return []*T type of result rather they return a struct which contains []*T type of response. Can we manually make Iterator for them?

We already have one special case in gen-iterators.go and we can certainly add more.

Ah, Found it 👍

go-github/github/gen-iterators.go

Lines 128 to 130 in c8fd3f7

	var customTestJSON = map[string]string{
	"ListUserInstallations": `{"installations": []}`,
	}

[gmlewis]

Ah, Found it 👍

Maybe customTestJSON is not the best name for that map. I think we should feel free to rename it. 😅

[Not-Dhananjay-Mishra]

Maybe customTestJSON is not the best name for that map. I think we should feel free to rename it. 😅

I got confused. That why I edited 😆

[alexandear]

Thanks for the findings. I also detected in #3925 that a lot of methods won't support iteration, even though they have ListOptions in their parameter list.

Related to: #3964 #3916

[maishivamhoo123]

@gmlewis and team can you please assign this issue to me? I love to contribute to this issue.

[alexandear]

can you please assign this issue to me? I love to contribute to this issue.

You can fix these methods: IssuesService.List, IssuesService.ListByOrg, and IssuesService.ListByRepo.

Feel free to create a PR.