Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add affiliation filter for collaborators #981

Merged
merged 7 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 45 additions & 2 deletions src/main/java/org/kohsuke/github/GHRepository.java
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,13 @@ public int getSize() {
return size;
}

/**
* Affiliation of a repository collaborator
*/
public enum CollaboratorAffiliation {
ALL, DIRECT, OUTSIDE
}

/**
* Gets the collaborators on this repository. This set always appear to include the owner.
*
Expand All @@ -823,7 +830,7 @@ public int getSize() {
*/
@WithBridgeMethods(Set.class)
public GHPersonSet<GHUser> getCollaborators() throws IOException {
return new GHPersonSet<GHUser>(listCollaborators().toList());
return new GHPersonSet<GHUser>(listCollaborators(CollaboratorAffiliation.ALL).toList());
}

/**
Expand All @@ -834,7 +841,22 @@ public GHPersonSet<GHUser> getCollaborators() throws IOException {
* the io exception
*/
public PagedIterable<GHUser> listCollaborators() throws IOException {
return listUsers("collaborators");
return listCollaborators(CollaboratorAffiliation.ALL);
Irialad marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Lists up the collaborators on this repository.
*
* @param affiliation
* Filter users by affiliation
* @return Users paged iterable
* @throws IOException
* the io exception
*/
public PagedIterable<GHUser> listCollaborators(CollaboratorAffiliation affiliation) throws IOException {
Map<String, Object> args = new HashMap<>();
args.put("affiliation", affiliation.toString().toLowerCase());
Irialad marked this conversation as resolved.
Show resolved Hide resolved
return listUsers("collaborators", args);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work?

Suggested change
return listUsers("collaborators", args);
return listUsers("collaborators",
Collections.singletonMap("affiliation", affiliation);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a look at how I've streamlined things now and tell me what you think.

}

/**
Expand Down Expand Up @@ -873,10 +895,25 @@ public boolean hasAssignee(GHUser u) throws IOException {
* the io exception
*/
public Set<String> getCollaboratorNames() throws IOException {
return getCollaboratorNames(CollaboratorAffiliation.ALL);
}

/**
* Gets the names of the collaborators on this repository. This method deviates from the principle of this library
* but it works a lot faster than {@link #getCollaborators()}.
*
* @param affiliation
* Filter users by affiliation
* @return the collaborator names
* @throws IOException
* the io exception
*/
public Set<String> getCollaboratorNames(CollaboratorAffiliation affiliation) throws IOException {
Set<String> r = new HashSet<>();
// no initializer - we just want to the logins
PagedIterable<GHUser> users = root.createRequest()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This probably also use listUsers(...) right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

listUsers does a few extra things, so that would negatively impact the speed of what is ostensibly the "fast" alternative. If that's a desirable change, I'm happy to make it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't the fast alternative, just a convenient one. Go ahead and change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the javadoc in error and I should update that while I'm in here?

    /**
     * Gets the names of the collaborators on this repository. This method deviates from the principle of this library
     * but it works a lot faster than {@link #getCollaborators()}.
     *
     * @return the collaborator names
     * @throws IOException
     *             the io exception
     */
    public Set<String> getCollaboratorNames() throws IOException```

.withUrlPath(getApiTailUrl("collaborators"))
.with("affiliation", affiliation.toString().toLowerCase())
.toIterable(GHUser[].class, null);
for (GHUser u : users.toArray()) {
r.add(u.login);
Expand Down Expand Up @@ -2088,8 +2125,14 @@ public PagedIterable<GHStargazer> listStargazers2() {
}

private PagedIterable<GHUser> listUsers(final String suffix) {
Map<String, Object> defaultArgs = Collections.EMPTY_MAP;
return listUsers(suffix, defaultArgs);
}

private PagedIterable<GHUser> listUsers(final String suffix, Map<String, Object> args) {
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
return root.createRequest()
.withUrlPath(getApiTailUrl(suffix))
.with(args)
.toIterable(GHUser[].class, item -> item.wrapUp(root));
}

Expand Down
33 changes: 24 additions & 9 deletions src/main/java/org/kohsuke/github/GitHubRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private GitHubRequest(@Nonnull List<Entry> args,

/**
* Create a new {@link Builder}.
*
*
Irialad marked this conversation as resolved.
Show resolved Hide resolved
* @return a new {@link Builder}.
*/
public static Builder<?> newBuilder() {
Expand Down Expand Up @@ -165,7 +165,7 @@ public Map<String, Object> injectedMappingValues() {

/**
* The base GitHub API URL for this request represented as a {@link String}
*
*
* @return the url string
*/
@Nonnull
Expand All @@ -176,7 +176,7 @@ public String apiUrl() {
/**
* The url path to be added to the {@link #apiUrl()} for this request. If this does not start with a "/", it instead
* represents the full url string for this request.
*
*
* @return a url path or full url string
*/
@Nonnull
Expand All @@ -186,7 +186,7 @@ public String urlPath() {

/**
* The content type to to be sent by this request.
*
*
* @return the content type.
*/
@Nonnull
Expand All @@ -196,7 +196,7 @@ public String contentType() {

/**
* The {@link InputStream} to be sent as the body of this request.
*
*
* @return the {@link InputStream}.
*/
@CheckForNull
Expand All @@ -206,7 +206,7 @@ public InputStream body() {

/**
* The {@link URL} for this request. This is the actual URL the {@link GitHubClient} will send this request to.
*
*
* @return the request {@link URL}
*/
@Nonnull
Expand All @@ -216,7 +216,7 @@ public URL url() {

/**
* Whether arguments for this request should be included in the URL or in the body of the request.
*
*
* @return true if the arguements should be sent in the body of the request.
*/
public boolean inBody() {
Expand All @@ -226,7 +226,7 @@ public boolean inBody() {
/**
* Create a {@link Builder} from this request. Initial values of the builder will be the same as this
* {@link GitHubRequest}.
*
*
* @return a {@link Builder} based on this request.
*/
public Builder<?> toBuilder() {
Expand Down Expand Up @@ -346,7 +346,7 @@ private Builder(@Nonnull List<Entry> args,

/**
* Builds a {@link GitHubRequest} from this builder.
*
*
* @return a {@link GitHubRequest}
* @throws MalformedURLException
* if the GitHub API URL cannot be constructed
Expand Down Expand Up @@ -437,6 +437,21 @@ public B withPreview(String name) {
return withHeader("Accept", name);
}

/**
* With requester.
*
* @param Map
* map of key value pairs to add
* @return the request builder
*/
public B with(Map<String, Object> map) {
Irialad marked this conversation as resolved.
Show resolved Hide resolved
for (Map.Entry<String, Object> entry : map.entrySet()) {
with(entry.getKey(), entry.getValue());
}

return (B) this;
}

/**
* With requester.
*
Expand Down
11 changes: 11 additions & 0 deletions src/test/java/org/kohsuke/github/GHRepositoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.fasterxml.jackson.databind.JsonMappingException;
import org.apache.commons.io.IOUtils;
import org.junit.Ignore;
import org.junit.Test;

import java.io.FileNotFoundException;
Expand Down Expand Up @@ -645,6 +646,16 @@ public void listCollaborators() throws Exception {
assertThat(collaborators.size(), greaterThan(10));
}

@Test
@Ignore("Data not cached")
Irialad marked this conversation as resolved.
Show resolved Hide resolved
public void listCollaboratorsFiltered() throws Exception {
GHRepository repo = getRepository();
List<GHUser> allCollaborators = repo.listCollaborators().toList();
List<GHUser> filteredCollaborators = repo.listCollaborators(GHRepository.CollaboratorAffiliation.OUTSIDE)
.toList();
assertThat(allCollaborators.size(), greaterThan(filteredCollaborators.size()));
}

@Test
public void getCheckRuns() throws Exception {
final int expectedCount = 8;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"id": "bce97482-6a11-44e5-a112-29230b142636",
"name": "repos_hub4j-test-org_jenkins_collaborators",
"request": {
"url": "/repos/hub4j-test-org/jenkins/collaborators",
"url": "/repos/hub4j-test-org/jenkins/collaborators?affiliation=all",
Irialad marked this conversation as resolved.
Show resolved Hide resolved
"method": "GET",
"headers": {
"Accept": {
Expand Down Expand Up @@ -44,4 +44,4 @@
"uuid": "bce97482-6a11-44e5-a112-29230b142636",
"persistent": true,
"insertionIndex": 4
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"id": "ddaa8229-c0ae-4df6-90ed-08425bfe71f2",
"name": "repos_hub4j-test-org_github-api_collaborators",
"request": {
"url": "/repos/hub4j-test-org/github-api/collaborators",
"url": "/repos/hub4j-test-org/github-api/collaborators?affiliation=all",
Irialad marked this conversation as resolved.
Show resolved Hide resolved
"method": "GET",
"headers": {
"Accept": {
Expand Down Expand Up @@ -38,4 +38,4 @@
"uuid": "ddaa8229-c0ae-4df6-90ed-08425bfe71f2",
"persistent": true,
"insertionIndex": 5
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"id": "2b8badfb-52b8-4304-a9a5-66b80274e93d",
"name": "repos_hub4j-test-org_github-api_collaborators",
"request": {
"url": "/repos/hub4j-test-org/github-api/collaborators",
"url": "/repos/hub4j-test-org/github-api/collaborators?affiliation=all",
Irialad marked this conversation as resolved.
Show resolved Hide resolved
"method": "GET",
"headers": {
"Accept": {
Expand Down Expand Up @@ -44,4 +44,4 @@
"uuid": "2b8badfb-52b8-4304-a9a5-66b80274e93d",
"persistent": true,
"insertionIndex": 4
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"id": "b0680d17-cd3b-4ec0-a857-d352c7167e94",
"name": "repos_hub4j-test-org_github-api_collaborators",
"request": {
"url": "/repos/hub4j-test-org/github-api/collaborators",
"url": "/repos/hub4j-test-org/github-api/collaborators?affiliation=all",
Irialad marked this conversation as resolved.
Show resolved Hide resolved
"method": "GET",
"headers": {
"Accept": {
Expand Down Expand Up @@ -44,4 +44,4 @@
"uuid": "b0680d17-cd3b-4ec0-a857-d352c7167e94",
"persistent": true,
"insertionIndex": 4
}
}