From 9c7de767e948812f50f2a68ebaa92b837a97e7c7 Mon Sep 17 00:00:00 2001 From: George Gastaldi <gegastaldi@gmail.com> Date: Sat, 25 Jul 2020 01:17:16 -0300 Subject: [PATCH 1/2] Fixes modifyCollaborators for multiple users Fixes #868 --- .../java/org/kohsuke/github/GHRepository.java | 14 +++--- .../org/kohsuke/github/GHRepositoryTest.java | 1 + .../__files/users_jimmysombrero2.json | 45 +++++++++++++++++ ...thub-api_collaborators_jimmysombrero2.json | 48 +++++++++++++++++++ .../mappings/users_jimmysombrero2-7.json | 45 +++++++++++++++++ 5 files changed, 146 insertions(+), 7 deletions(-) create mode 100644 src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/__files/users_jimmysombrero2.json create mode 100644 src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/repos_hub4j-test-org_github-api_collaborators_jimmysombrero2.json create mode 100644 src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/users_jimmysombrero2-7.json diff --git a/src/main/java/org/kohsuke/github/GHRepository.java b/src/main/java/org/kohsuke/github/GHRepository.java index 50adf06852..8315b0dcd8 100644 --- a/src/main/java/org/kohsuke/github/GHRepository.java +++ b/src/main/java/org/kohsuke/github/GHRepository.java @@ -46,6 +46,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -979,13 +980,12 @@ public void removeCollaborators(Collection<GHUser> users) throws IOException { private void modifyCollaborators(@NonNull Collection<GHUser> users, @NonNull String method, @CheckForNull GHOrganization.Permission permission) throws IOException { - Requester requester = root.createRequest().method(method); - - if (permission != null) { - requester = requester.with("permission", permission).inBody(); - } - - for (GHUser user : users) { + // Make sure that the users collection doesn't have any duplicates + for (GHUser user : new LinkedHashSet<GHUser>(users)) { + Requester requester = root.createRequest().method(method); + if (permission != null) { + requester = requester.with("permission", permission).inBody(); + } requester.withUrlPath(getApiTailUrl("collaborators/" + user.getLogin())).send(); } } diff --git a/src/test/java/org/kohsuke/github/GHRepositoryTest.java b/src/test/java/org/kohsuke/github/GHRepositoryTest.java index b7df6d69f3..2c579370dd 100644 --- a/src/test/java/org/kohsuke/github/GHRepositoryTest.java +++ b/src/test/java/org/kohsuke/github/GHRepositoryTest.java @@ -191,6 +191,7 @@ public void addCollaborators() throws Exception { List<GHUser> users = new ArrayList<GHUser>(); users.add(user); + users.add(gitHub.getUser("jimmysombrero2")); repo.addCollaborators(users, GHOrganization.Permission.PUSH); GHPersonSet<GHUser> collabs = repo.getCollaborators(); diff --git a/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/__files/users_jimmysombrero2.json b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/__files/users_jimmysombrero2.json new file mode 100644 index 0000000000..9d2d78fa72 --- /dev/null +++ b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/__files/users_jimmysombrero2.json @@ -0,0 +1,45 @@ +{ + "login": "jimmysombrero2", + "id": 12157727, + "node_id": "MDQ6VXNlcjEyMTU3NzI3", + "avatar_url": "https://avatars3.githubusercontent.com/u/12157727?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/jimmysombrero", + "html_url": "https://github.com/jimmysombrero", + "followers_url": "https://api.github.com/users/jimmysombrero/followers", + "following_url": "https://api.github.com/users/jimmysombrero/following{/other_user}", + "gists_url": "https://api.github.com/users/jimmysombrero/gists{/gist_id}", + "starred_url": "https://api.github.com/users/jimmysombrero/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/jimmysombrero/subscriptions", + "organizations_url": "https://api.github.com/users/jimmysombrero/orgs", + "repos_url": "https://api.github.com/users/jimmysombrero/repos", + "events_url": "https://api.github.com/users/jimmysombrero/events{/privacy}", + "received_events_url": "https://api.github.com/users/jimmysombrero/received_events", + "type": "User", + "site_admin": false, + "name": null, + "company": null, + "blog": "", + "location": null, + "email": null, + "hireable": null, + "bio": null, + "public_repos": 4, + "public_gists": 0, + "followers": 1, + "following": 0, + "created_at": "2015-04-28T17:47:19Z", + "updated_at": "2020-02-02T04:43:58Z", + "private_gists": 0, + "total_private_repos": 0, + "owned_private_repos": 0, + "disk_usage": 19, + "collaborators": 0, + "two_factor_authentication": false, + "plan": { + "name": "free", + "space": 976562499, + "collaborators": 0, + "private_repos": 10000 + } +} \ No newline at end of file diff --git a/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/repos_hub4j-test-org_github-api_collaborators_jimmysombrero2.json b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/repos_hub4j-test-org_github-api_collaborators_jimmysombrero2.json new file mode 100644 index 0000000000..a538fc44b3 --- /dev/null +++ b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/repos_hub4j-test-org_github-api_collaborators_jimmysombrero2.json @@ -0,0 +1,48 @@ +{ + "id": "3d80b19e-05f4-4ad9-a610-5f573abc4363", + "name": "repos_hub4j-test-org_github-api_collaborators_jimmysombrero2", + "request": { + "url": "/repos/hub4j-test-org/github-api/collaborators/jimmysombrero2", + "method": "PUT", + "headers": { + "Accept": { + "equalTo": "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" + } + }, + "bodyPatterns": [ + { + "equalToJson": "{\"permission\":\"push\"}", + "ignoreArrayOrder": true, + "ignoreExtraElements": true + } + ] + }, + "response": { + "status": 201, + "body": "[]", + "headers": { + "Server": "GitHub.com", + "Date": "Sun, 02 Feb 2020 04:59:39 GMT", + "Content-Type": "application/json; charset=utf-8", + "Status": "201 Created", + "X-RateLimit-Limit": "5000", + "X-RateLimit-Remaining": "4892", + "X-RateLimit-Reset": "1580620983", + "X-OAuth-Scopes": "admin:enterprise, admin:gpg_key, admin:org, admin:org_hook, admin:public_key, admin:repo_hook, delete:packages, delete_repo, gist, notifications, read:packages, repo, user, workflow, write:discussion, write:packages", + "X-Accepted-OAuth-Scopes": "", + "X-GitHub-Media-Type": "unknown, github.v3", + "Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type", + "Access-Control-Allow-Origin": "*", + "Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload", + "X-Frame-Options": "deny", + "X-Content-Type-Options": "nosniff", + "X-XSS-Protection": "1; mode=block", + "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin", + "Content-Security-Policy": "default-src 'none'", + "X-GitHub-Request-Id": "C903:768E:16C4966:2BA11B0:5E36573B" + } + }, + "uuid": "3d80b19e-05f4-4ad9-a610-5f573abc4363", + "persistent": true, + "insertionIndex": 4 +} \ No newline at end of file diff --git a/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/users_jimmysombrero2-7.json b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/users_jimmysombrero2-7.json new file mode 100644 index 0000000000..9b27581e40 --- /dev/null +++ b/src/test/resources/org/kohsuke/github/GHRepositoryTest/wiremock/addCollaborators/mappings/users_jimmysombrero2-7.json @@ -0,0 +1,45 @@ +{ + "id": "6685376c-451b-486d-88cd-502af9a7c5d2", + "name": "users_jimmysombrero2", + "request": { + "url": "/users/jimmysombrero2", + "method": "GET", + "headers": { + "Accept": { + "equalTo": "text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2" + } + } + }, + "response": { + "status": 200, + "bodyFileName": "users_jimmysombrero2.json", + "headers": { + "Server": "GitHub.com", + "Date": "Mon, 03 Feb 2020 03:50:14 GMT", + "Content-Type": "application/json; charset=utf-8", + "Status": "200 OK", + "X-RateLimit-Limit": "5000", + "X-RateLimit-Remaining": "4907", + "X-RateLimit-Reset": "1580704799", + "Cache-Control": "private, max-age=60, s-maxage=60", + "Vary": "Accept, Authorization, Cookie, X-GitHub-OTP", + "ETag": "W/\"7a0206b47e995649c88218afeb2266a6\"", + "Last-Modified": "Sun, 02 Feb 2020 04:43:58 GMT", + "X-OAuth-Scopes": "admin:enterprise, admin:gpg_key, admin:org, admin:org_hook, admin:public_key, admin:repo_hook, delete:packages, delete_repo, gist, notifications, read:packages, repo, user, workflow, write:discussion, write:packages", + "X-Accepted-OAuth-Scopes": "", + "X-GitHub-Media-Type": "unknown, github.v3", + "Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type", + "Access-Control-Allow-Origin": "*", + "Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload", + "X-Frame-Options": "deny", + "X-Content-Type-Options": "nosniff", + "X-XSS-Protection": "1; mode=block", + "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin", + "Content-Security-Policy": "default-src 'none'", + "X-GitHub-Request-Id": "CDFE:768D:1021273:26DC788:5E379876" + } + }, + "uuid": "6685376c-451b-486d-88cd-502af9a7c5d1", + "persistent": true, + "insertionIndex": 6 +} \ No newline at end of file From fb1adbd1eff55b203fff3bad6185e5e3a9e35d51 Mon Sep 17 00:00:00 2001 From: Liam Newman <bitwiseman@gmail.com> Date: Mon, 27 Jul 2020 13:12:41 -0700 Subject: [PATCH 2/2] Make withUrlPath() overwrite instead of append I had some ideas about having multiple calls to apend to build up paths, but it turns out that idea is pretty bad. `with*()` methods should overwrite when called for the same field. If we want to create and , we can do that later. --- src/main/java/org/kohsuke/github/GHRepository.java | 9 +++++---- src/main/java/org/kohsuke/github/GitHubRequest.java | 8 ++------ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHRepository.java b/src/main/java/org/kohsuke/github/GHRepository.java index 8315b0dcd8..b4e056cac8 100644 --- a/src/main/java/org/kohsuke/github/GHRepository.java +++ b/src/main/java/org/kohsuke/github/GHRepository.java @@ -980,12 +980,13 @@ public void removeCollaborators(Collection<GHUser> users) throws IOException { private void modifyCollaborators(@NonNull Collection<GHUser> users, @NonNull String method, @CheckForNull GHOrganization.Permission permission) throws IOException { + Requester requester = root.createRequest().method(method); + if (permission != null) { + requester = requester.with("permission", permission).inBody(); + } + // Make sure that the users collection doesn't have any duplicates for (GHUser user : new LinkedHashSet<GHUser>(users)) { - Requester requester = root.createRequest().method(method); - if (permission != null) { - requester = requester.with("permission", permission).inBody(); - } requester.withUrlPath(getApiTailUrl("collaborators/" + user.getLogin())).send(); } } diff --git a/src/main/java/org/kohsuke/github/GitHubRequest.java b/src/main/java/org/kohsuke/github/GitHubRequest.java index f7b4d2a4dc..1148073b25 100644 --- a/src/main/java/org/kohsuke/github/GitHubRequest.java +++ b/src/main/java/org/kohsuke/github/GitHubRequest.java @@ -671,13 +671,9 @@ public B withUrlPath(@Nonnull String urlPath, @Nonnull String... urlPathItems) { tailUrlPath += "/" + String.join("/", urlPathItems); } - if (this.urlPath.endsWith("/")) { - tailUrlPath = StringUtils.stripStart(tailUrlPath, "/"); - } else { - tailUrlPath = StringUtils.prependIfMissing(tailUrlPath, "/"); - } + tailUrlPath = StringUtils.prependIfMissing(tailUrlPath, "/"); - this.urlPath += urlPathEncode(tailUrlPath); + this.urlPath = urlPathEncode(tailUrlPath); return (B) this; }