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.

Sign up for GitHub

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

Adding possiblity to get ssh keys #544

Merged
merged 4 commits into from
Sep 30, 2019
Merged

Adding possiblity to get ssh keys #544

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
884eae2
Adding possiblity to get ssh keys
Sep 17, 2019
22afb76
Adding wiremocks
Sep 17, 2019
a1a9153
Merge branch 'master' into getKeysForUser
bitwiseman Sep 25, 2019
55f9c40
Merge branch 'master' into getKeysForUser
Sep 27, 2019
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
7 changes: 5 additions & 2 deletions src/main/java/org/kohsuke/github/GHUser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@
import com.infradna.tool.bridge_method_injector.WithBridgeMethods;

import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import java.util.*;

/**
* Represents an user of GitHub.
Expand All @@ -36,6 +35,10 @@
*/
public class GHUser extends GHPerson {

public List<GHKey> listKeys() throws IOException {
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public List<GHKey> listKeys() throws IOException {
public List<GHKey> getKeys() throws IOException {

return Collections.unmodifiableList(Arrays.asList(root.retrieve().to("/users/"+login+"/keys", GHKey[].class)));
Copy link
Member

Choose a reason for hiding this comment

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

It is possible for there to be multiple pages of keys? I know it is unlikely for some one to have more than a page of keys, but if it is possible it might be better to use a PagedIterable.

Copy link
Author

Choose a reason for hiding this comment

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

I think it is a over kill using PagedIterable, but it is of course possible. Most people only have a single ssh key and less than 10.

Personally I think it better to return a list and instead read all the keys in listKeys() . I can of course iterate until all key have been read

Copy link
Member

Choose a reason for hiding this comment

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

@arngrimur-seal
I see your point.

Okay, I just did a quick review of the code base. Generally, list* methods return PagedIterable<T>. If there is a method that returns a List<T> or a T[] it is called get*.

I think this completely misleading naming, but I'd rather be consistent with the existing naming pattern. So, let's change the name to getKeys(). Then we're good to go.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure I understand, are we good to merge or do you want me to update the code so we make sure all keys has been rtrieved?

Copy link
Member

Choose a reason for hiding this comment

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

Added a suggestion comment above.

}

/**
* Follow this user.
*/
Expand Down
26 changes: 24 additions & 2 deletions src/test/java/org/kohsuke/github/UserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import org.junit.Test;

import java.io.IOException;
import java.util.HashSet;
import java.util.Set;
import java.util.*;

/**
* @author Kohsuke Kawaguchi
Expand All @@ -27,4 +26,27 @@ private Set<GHUser> count30(PagedIterable<GHUser> l) {
assertEquals(30, users.size());
return users;
}

@Test
public void getKeys() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

This is really great. How was it using this test framework and takeSnapshot? I just added it and would appreciate any feedback you can offer.

Copy link
Author

Choose a reason for hiding this comment

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

It took a while before I understood the framework but then it was simple. Deleting and generating the test data has drawbacks since it is easy do forget about that step but the advantage with test taking lots of time and that data is constant is a greater benefit.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, well, if you have any suggestions for how to make it easier (such as improvements to the CONTRIBUTING.md) that would be great.

GHUser u = gitHub.getUser("rtyler");
List<GHKey> ghKeys = new ArrayList<>(u.listKeys());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
List<GHKey> ghKeys = new ArrayList<>(u.listKeys());
List<GHKey> ghKeys = new ArrayList<>(u.getKeys());


assertEquals(3, ghKeys.size());
Collections.sort(ghKeys, new Comparator<GHKey>() {
@Override
public int compare(GHKey ghKey, GHKey t1) {
return ghKey.getId() - t1.getId();
}
});
assertEquals(1066173, ghKeys.get(0).getId());
assertEquals("ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAueiy12T5bvFhsc9YjfLc3aVIxgySd3gDxQWy/bletIoZL8omKmzocBYJ7F58U1asoyfWsy2ToTOY8jJp1eToXmbD6L5+xvHba0A7djYh9aQRrFam7doKQ0zp0ZSUF6+R1v0OM4nnWqK4n2ECIYd+Bdzrp+xA5+XlW3ZSNzlnW2BeWznzmgRMcp6wI+zQ9GMHWviR1cxpml5Z6wrxTZ0aX91btvnNPqoOGva976B6e6403FOEkkIFTk6CC1TFKwc/VjbqxYBg4kU0JhiTP+iEZibcQrYjWdYUgAotYbFVe5/DneHMLNsMPdeihba4PUwt62rXyNegenuCRmCntLcaFQ==",
ghKeys.get(0).getKey());
assertEquals(28136459, ghKeys.get(1 ).getId());
assertEquals( "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDTU0s5OKCC6VpKZGL9NJD4mNLY0AtujkVB1JkkuQ4OkMi2YGUHJtGhTbTwEVhNxpm0x2dM5KSzse6MLDYuGBW0qkE/VVuD9+9I73hbq461KqP0+WlupNh+Qc86kbiLBDv64+vWc+50mp1dbINpoM5xvaPYxgjnemydPv7vu5bhCHBugW7aN8VcLgfFgcp8vZCEanMtd3hIRjRU8v8Skk233ZGu1bXkG8iIOBQPabvEtZ0VDMg9pT3Q1R6lnnKqfCwHXd6zP6uAtejFSxvKRGKpu3OLGQMHwk7NlImVuhkVdaEFBq7pQtpOaGuP2eLKcN1wy5jsTYE+ZB6pvHCi2ecb",
ghKeys.get(1).getKey());
assertEquals(31452581, ghKeys.get(2).getId());
assertEquals("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC3JhH2FZBDmHLjXTcBoV6tdcYKmsQ7sgu8k1RsUhwxGsXm65+Cuas6GcMVoA1DncKfJGQkulHDFiTxIROIBmedh9/otHWBlZ4HqYZ4MQ1A8W5quULkXwX/kF+UdRBUxFvjigibEbuHB+LARVxRRzFlPnTSE9rAfAv8OOEsb3lNUGT/IGhN8w1vwe8GclB90tgqN1RBDgrVqwLFwn5AfrW9kUIa2f2oT4RjYu1OrhKhVIIzfHADo85aD+s8wEhqwI96BCJG3qTWrypoHwBUoj1O6Ak5CGc1iKz9o8XyTMjudRt2ddCjfOtxsuwSlTbVtQXJGIpgKviX1sgh4pPvGh7BVAFP+mdAK4F+mEugDnuj47GO/K5KGGDRCL56kh9+h28l4q/+fZvp7DhtmSN2EzrVAdQFskF8yY/6Xit/aAvjeKm03DcjbylSXbG26EJefaLHlwYFq2mUFRMak25wuuCZS71GF3RC3Sl/bMoxBKRYkyfYtGafeaYTFNGn8Dbd+hfVUCz31ebI8cvmlQR5b5AbCre3T7HTVgw8FKbAxWRf1Fio56PnqHsj+sT1KVj255Zo1F8iD9GrgERSVAlkh5bY/CKszQ8ZSd01c9Qp2a47/gR7XAAbxhzGHP+cSOlrqDlJ24fbPtcpVsM0llqKUcxpmoOBFNboRmE1QqnSmAf9ww==",
ghKeys.get(2).getKey());
}
}
1 change: 1 addition & 0 deletions .../UserTest/wiremock/getKeys/__files/users_rtyler-11f4235a-9149-4727-bb9b-91b6dce384b5.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"login":"rtyler","id":26594,"node_id":"MDQ6VXNlcjI2NTk0","avatar_url":"https://avatars0.githubusercontent.com/u/26594?v=4","gravatar_id":"","url":"https://api.github.com/users/rtyler","html_url":"https://github.com/rtyler","followers_url":"https://api.github.com/users/rtyler/followers","following_url":"https://api.github.com/users/rtyler/following{/other_user}","gists_url":"https://api.github.com/users/rtyler/gists{/gist_id}","starred_url":"https://api.github.com/users/rtyler/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rtyler/subscriptions","organizations_url":"https://api.github.com/users/rtyler/orgs","repos_url":"https://api.github.com/users/rtyler/repos","events_url":"https://api.github.com/users/rtyler/events{/privacy}","received_events_url":"https://api.github.com/users/rtyler/received_events","type":"User","site_admin":false,"name":"R. Tyler Croy","company":null,"blog":"https://brokenco.de/","location":"California","email":null,"hireable":null,"bio":"person","public_repos":239,"public_gists":470,"followers":397,"following":41,"created_at":"2008-09-28T07:28:42Z","updated_at":"2019-04-24T00:04:36Z"}
1 change: 1 addition & 0 deletions ...Test/wiremock/getKeys/__files/users_rtyler_keys-395dc00a-5234-4866-815c-d7b49cdd0157.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"id":1066173,"key":"ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAueiy12T5bvFhsc9YjfLc3aVIxgySd3gDxQWy/bletIoZL8omKmzocBYJ7F58U1asoyfWsy2ToTOY8jJp1eToXmbD6L5+xvHba0A7djYh9aQRrFam7doKQ0zp0ZSUF6+R1v0OM4nnWqK4n2ECIYd+Bdzrp+xA5+XlW3ZSNzlnW2BeWznzmgRMcp6wI+zQ9GMHWviR1cxpml5Z6wrxTZ0aX91btvnNPqoOGva976B6e6403FOEkkIFTk6CC1TFKwc/VjbqxYBg4kU0JhiTP+iEZibcQrYjWdYUgAotYbFVe5/DneHMLNsMPdeihba4PUwt62rXyNegenuCRmCntLcaFQ=="},{"id":28136459,"key":"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDTU0s5OKCC6VpKZGL9NJD4mNLY0AtujkVB1JkkuQ4OkMi2YGUHJtGhTbTwEVhNxpm0x2dM5KSzse6MLDYuGBW0qkE/VVuD9+9I73hbq461KqP0+WlupNh+Qc86kbiLBDv64+vWc+50mp1dbINpoM5xvaPYxgjnemydPv7vu5bhCHBugW7aN8VcLgfFgcp8vZCEanMtd3hIRjRU8v8Skk233ZGu1bXkG8iIOBQPabvEtZ0VDMg9pT3Q1R6lnnKqfCwHXd6zP6uAtejFSxvKRGKpu3OLGQMHwk7NlImVuhkVdaEFBq7pQtpOaGuP2eLKcN1wy5jsTYE+ZB6pvHCi2ecb"},{"id":31452581,"key":"ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAACAQC3JhH2FZBDmHLjXTcBoV6tdcYKmsQ7sgu8k1RsUhwxGsXm65+Cuas6GcMVoA1DncKfJGQkulHDFiTxIROIBmedh9/otHWBlZ4HqYZ4MQ1A8W5quULkXwX/kF+UdRBUxFvjigibEbuHB+LARVxRRzFlPnTSE9rAfAv8OOEsb3lNUGT/IGhN8w1vwe8GclB90tgqN1RBDgrVqwLFwn5AfrW9kUIa2f2oT4RjYu1OrhKhVIIzfHADo85aD+s8wEhqwI96BCJG3qTWrypoHwBUoj1O6Ak5CGc1iKz9o8XyTMjudRt2ddCjfOtxsuwSlTbVtQXJGIpgKviX1sgh4pPvGh7BVAFP+mdAK4F+mEugDnuj47GO/K5KGGDRCL56kh9+h28l4q/+fZvp7DhtmSN2EzrVAdQFskF8yY/6Xit/aAvjeKm03DcjbylSXbG26EJefaLHlwYFq2mUFRMak25wuuCZS71GF3RC3Sl/bMoxBKRYkyfYtGafeaYTFNGn8Dbd+hfVUCz31ebI8cvmlQR5b5AbCre3T7HTVgw8FKbAxWRf1Fio56PnqHsj+sT1KVj255Zo1F8iD9GrgERSVAlkh5bY/CKszQ8ZSd01c9Qp2a47/gR7XAAbxhzGHP+cSOlrqDlJ24fbPtcpVsM0llqKUcxpmoOBFNboRmE1QqnSmAf9ww=="}]
38 changes: 38 additions & 0 deletions ...UserTest/wiremock/getKeys/mappings/users_rtyler-11f4235a-9149-4727-bb9b-91b6dce384b5.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
{
"id" : "11f4235a-9149-4727-bb9b-91b6dce384b5",
"name" : "users_rtyler",
"request" : {
"url" : "/users/rtyler",
"method" : "GET"
},
"response" : {
"status" : 200,
"bodyFileName" : "users_rtyler-11f4235a-9149-4727-bb9b-91b6dce384b5.json",
"headers" : {
"Date" : "Tue, 17 Sep 2019 12:55:38 GMT",
"Content-Type" : "application/json; charset=utf-8",
"Server" : "GitHub.com",
"Status" : "200 OK",
"X-RateLimit-Limit" : "60",
"X-RateLimit-Remaining" : "42",
"X-RateLimit-Reset" : "1568727505",
"Cache-Control" : "public, max-age=60, s-maxage=60",
"Vary" : [ "Accept", "Accept-Encoding" ],
"ETag" : "W/\"c51918a9f877202da96a3904b2445a4f\"",
"Last-Modified" : "Wed, 24 Apr 2019 00:04:36 GMT",
"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" : "EA4A:354F9:5E84C1B:7289588:5D80D7C9"
}
},
"uuid" : "11f4235a-9149-4727-bb9b-91b6dce384b5",
"persistent" : true,
"insertionIndex" : 1
}
37 changes: 37 additions & 0 deletions ...est/wiremock/getKeys/mappings/users_rtyler_keys-395dc00a-5234-4866-815c-d7b49cdd0157.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"id" : "395dc00a-5234-4866-815c-d7b49cdd0157",
"name" : "users_rtyler_keys",
"request" : {
"url" : "/users/rtyler/keys",
"method" : "GET"
},
"response" : {
"status" : 200,
"bodyFileName" : "users_rtyler_keys-395dc00a-5234-4866-815c-d7b49cdd0157.json",
"headers" : {
"Date" : "Tue, 17 Sep 2019 12:55:38 GMT",
"Content-Type" : "application/json; charset=utf-8",
"Server" : "GitHub.com",
"Status" : "200 OK",
"X-RateLimit-Limit" : "60",
"X-RateLimit-Remaining" : "41",
"X-RateLimit-Reset" : "1568727505",
"Cache-Control" : "public, max-age=60, s-maxage=60",
"Vary" : [ "Accept", "Accept-Encoding" ],
"ETag" : "W/\"257fbc4218f6908012a1054fb084df45\"",
"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" : "EA4A:354F9:5E84CA9:7289633:5D80D7CA"
}
},
"uuid" : "395dc00a-5234-4866-815c-d7b49cdd0157",
"persistent" : true,
"insertionIndex" : 2
}