Skip to content

Commit f788b0d

Browse files
author
Travis Tomsu
authored
Adds pagination support to Github UserRolesProvider (#130)
1 parent a6fd53a commit f788b0d

File tree

2 files changed

+37
-20
lines changed

2 files changed

+37
-20
lines changed

fiat-github/src/main/java/com/netflix/spinnaker/fiat/roles/github/GithubTeamsUserRolesProvider.java

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333

3434
import java.util.ArrayList;
3535
import java.util.Collection;
36-
import java.util.Collections;
3736
import java.util.HashMap;
3837
import java.util.List;
3938
import java.util.Map;
@@ -84,7 +83,8 @@ public List<Role> loadRoles(String userName) {
8483
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
8584
return new ArrayList<>();
8685
} else if (e.getResponse().getStatus() == 404) {
87-
log.error(String.format("Could not find the GitHub organization %s",
86+
log.error(String.format("%s is not a member of GitHub organization %s",
87+
userName,
8888
gitHubProperties.getOrganization()),
8989
e);
9090
return new ArrayList<>();
@@ -105,26 +105,40 @@ public List<Role> loadRoles(String userName) {
105105

106106
// Get teams of the current user
107107
List<GitHubMaster.Team> teams = new ArrayList<>();
108-
try {
109-
teams = master.getGitHubClient().getOrgTeams(gitHubProperties.getOrganization(),
110-
gitHubProperties.paginationValue);
111-
} catch (RetrofitError e) {
112-
log.error(String.format("RetrofitError %s %s ",
113-
e.getResponse().getStatus(),
114-
e.getResponse().getReason()),
115-
e);
116-
if (e.getKind() == RetrofitError.Kind.NETWORK) {
117-
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
118-
} else if (e.getResponse().getStatus() == 404) {
119-
log.error("404 when getting teams");
120-
return result;
121-
} else if (e.getResponse().getStatus() == 401) {
122-
log.error(String.format("Cannot get GitHub organization %s teams: Not authorized.",
123-
gitHubProperties.getOrganization()),
108+
int page = 1;
109+
boolean hasMorePages = true;
110+
111+
do {
112+
try {
113+
log.debug("Requesting page " + page + " of teams.");
114+
List<GitHubMaster.Team> teamsPage = master.getGitHubClient()
115+
.getOrgTeams(gitHubProperties.getOrganization(),
116+
page++,
117+
gitHubProperties.paginationValue);
118+
teams.addAll(teamsPage);
119+
if (teamsPage.size() != gitHubProperties.paginationValue) {
120+
hasMorePages = false;
121+
}
122+
log.debug("Got " + teamsPage.size() + " teams back. hasMorePages: " + hasMorePages);
123+
} catch (RetrofitError e) {
124+
hasMorePages = false;
125+
log.error(String.format("RetrofitError %s %s ",
126+
e.getResponse().getStatus(),
127+
e.getResponse().getReason()),
124128
e);
125-
return result;
129+
if (e.getKind() == RetrofitError.Kind.NETWORK) {
130+
log.error(String.format("Could not find the server %s", master.getBaseUrl()), e);
131+
} else if (e.getResponse().getStatus() == 404) {
132+
log.error("404 when getting teams");
133+
return result;
134+
} else if (e.getResponse().getStatus() == 401) {
135+
log.error(String.format("Cannot get GitHub organization %s teams: Not authorized.",
136+
gitHubProperties.getOrganization()),
137+
e);
138+
return result;
139+
}
126140
}
127-
}
141+
} while (hasMorePages);
128142

129143
log.debug("Found " + teams.size() + " teams in org.");
130144
teams.forEach(t -> {
@@ -180,4 +194,6 @@ public Map<String, Collection<Role>> multiLoadRoles(Collection<String> userEmail
180194

181195
return emailGroupsMap;
182196
}
197+
198+
183199
}

fiat-github/src/main/java/com/netflix/spinnaker/fiat/roles/github/client/GitHubClient.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ Response isMemberOfOrganization(@Path("org") String org,
4141

4242
@GET("/orgs/{org}/teams")
4343
List<GitHubMaster.Team> getOrgTeams(@Path("org") String org,
44+
@Query("page") int page,
4445
@Query("per_page") int paginationValue);
4546

4647
@GET("/teams/{idTeam}/memberships/{username}")

0 commit comments

Comments
 (0)