From fdcf74eaf27dd455e41af4150ea78649952b3b20 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sat, 18 Apr 2020 14:23:06 +0200 Subject: [PATCH 1/6] Add global node_id to GHObject + GHTeam extends GHObject --- .../java/org/kohsuke/github/GHObject.java | 12 ++++++++++++ .../org/kohsuke/github/GHOrganization.java | 2 +- src/main/java/org/kohsuke/github/GHTeam.java | 19 ++++++++----------- src/main/java/org/kohsuke/github/GitHub.java | 2 +- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHObject.java b/src/main/java/org/kohsuke/github/GHObject.java index 58be3907bc..3504fc3823 100644 --- a/src/main/java/org/kohsuke/github/GHObject.java +++ b/src/main/java/org/kohsuke/github/GHObject.java @@ -28,6 +28,7 @@ public abstract class GHObject { protected String url; protected long id; + protected String node_id; protected String created_at; protected String updated_at; @@ -123,6 +124,17 @@ public long getId() { return id; } + /** + * Get Global node_id from Github object. + * + * @see Using Global Node IDs + * + * @return Global Node ID. + */ + public String getNodeId() { + return node_id; + } + @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", justification = "Bridge method of getId") private Object longToStringOrInt(long id, Class type) { if (type == String.class) diff --git a/src/main/java/org/kohsuke/github/GHOrganization.java b/src/main/java/org/kohsuke/github/GHOrganization.java index 2d955df874..2ea2411c49 100644 --- a/src/main/java/org/kohsuke/github/GHOrganization.java +++ b/src/main/java/org/kohsuke/github/GHOrganization.java @@ -139,7 +139,7 @@ public PagedIterable listTeams() throws IOException { * * @see documentation */ - public GHTeam getTeam(int teamId) throws IOException { + public GHTeam getTeam(long teamId) throws IOException { return root.createRequest() .withUrlPath(String.format("/organizations/%d/team/%d", id, teamId)) .fetch(GHTeam.class) diff --git a/src/main/java/org/kohsuke/github/GHTeam.java b/src/main/java/org/kohsuke/github/GHTeam.java index 9854f3e863..aae670410a 100644 --- a/src/main/java/org/kohsuke/github/GHTeam.java +++ b/src/main/java/org/kohsuke/github/GHTeam.java @@ -1,6 +1,7 @@ package org.kohsuke.github; import java.io.IOException; +import java.net.URL; import java.util.Map; import java.util.Set; import java.util.TreeMap; @@ -10,14 +11,14 @@ * * @author Kohsuke Kawaguchi */ -public class GHTeam implements Refreshable { +public class GHTeam extends GHObject implements Refreshable { + private String html_url; private String name; private String permission; private String slug; private String description; private Privacy privacy; - private int id; private GHOrganization organization; // populated by GET /user/teams where Teams+Orgs are returned together protected /* final */ GitHub root; @@ -129,15 +130,6 @@ public void setPrivacy(Privacy privacy) throws IOException { root.createRequest().method("PATCH").with("privacy", privacy).withUrlPath(api("")).send(); } - /** - * Gets id. - * - * @return the id - */ - public int getId() { - return id; - } - /** * Retrieves the current members. * @@ -321,4 +313,9 @@ public GHOrganization getOrganization() throws IOException { public void refresh() throws IOException { root.createRequest().withUrlPath(api("")).fetchInto(this).wrapUp(root); } + + @Override + public URL getHtmlUrl() { + return GitHubClient.parseURL(html_url); + } } diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index f05d92f9c4..797f804914 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -731,7 +731,7 @@ public Map> getMyTeams() throws IOException { * @see deprecation notice */ @Deprecated - public GHTeam getTeam(int id) throws IOException { + public GHTeam getTeam(long id) throws IOException { return createRequest().withUrlPath("/teams/" + id).fetch(GHTeam.class).wrapUp(this); } From e05348463ce3ee51e0159431e91fea987619839d Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sat, 18 Apr 2020 14:32:51 +0200 Subject: [PATCH 2/6] Fix javadoc --- src/main/java/org/kohsuke/github/GitHub.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 797f804914..a916e07aac 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -727,7 +727,7 @@ public Map> getMyTeams() throws IOException { * @throws IOException * the io exception * - * @deprecated Use {@link GHOrganization#getTeam(int)} + * @deprecated Use {@link GHOrganization#getTeam(long)} * @see deprecation notice */ @Deprecated From f77eb33029746536aa26ac72ed6b13ad8d03cf1f Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sun, 19 Apr 2020 11:18:29 +0200 Subject: [PATCH 3/6] Add deprecated method --- .../java/org/kohsuke/github/GHOrganization.java | 17 ++++++++++++++++- src/main/java/org/kohsuke/github/GitHub.java | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHOrganization.java b/src/main/java/org/kohsuke/github/GHOrganization.java index 2ea2411c49..70382e2cbf 100644 --- a/src/main/java/org/kohsuke/github/GHOrganization.java +++ b/src/main/java/org/kohsuke/github/GHOrganization.java @@ -127,7 +127,7 @@ public PagedIterable listTeams() throws IOException { .withUrlPath(String.format("/orgs/%s/teams", login)) .toIterable(GHTeam[].class, item -> item.wrapUp(this)); } - + /** * Gets a single team by ID. * @@ -145,6 +145,21 @@ public GHTeam getTeam(long teamId) throws IOException { .fetch(GHTeam.class) .wrapUp(this); } + + /** + * Gets a single team by ID. + * + * @param teamId + * id of the team that we want to query for + * @return the team + * @throws IOException + * the io exception + * + * @deprecated Use {@link GHOrganization#getTeam(long)} + */ + public GHTeam getTeam(int teamId) throws IOException { + return getTeam((long)teamId); + } /** * Finds a team that has the given name in its {@link GHTeam#getName()} diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index a916e07aac..4684b96ec3 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -731,7 +731,7 @@ public Map> getMyTeams() throws IOException { * @see deprecation notice */ @Deprecated - public GHTeam getTeam(long id) throws IOException { + public GHTeam getTeam(int id) throws IOException { return createRequest().withUrlPath("/teams/" + id).fetch(GHTeam.class).wrapUp(this); } From a3073ec14ee13355d8edb591d75cd2a9bec259eb Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sun, 19 Apr 2020 11:19:48 +0200 Subject: [PATCH 4/6] Fix formatting --- src/main/java/org/kohsuke/github/GHOrganization.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHOrganization.java b/src/main/java/org/kohsuke/github/GHOrganization.java index 70382e2cbf..5de0cf35ca 100644 --- a/src/main/java/org/kohsuke/github/GHOrganization.java +++ b/src/main/java/org/kohsuke/github/GHOrganization.java @@ -127,7 +127,7 @@ public PagedIterable listTeams() throws IOException { .withUrlPath(String.format("/orgs/%s/teams", login)) .toIterable(GHTeam[].class, item -> item.wrapUp(this)); } - + /** * Gets a single team by ID. * @@ -145,7 +145,7 @@ public GHTeam getTeam(long teamId) throws IOException { .fetch(GHTeam.class) .wrapUp(this); } - + /** * Gets a single team by ID. * @@ -158,7 +158,7 @@ public GHTeam getTeam(long teamId) throws IOException { * @deprecated Use {@link GHOrganization#getTeam(long)} */ public GHTeam getTeam(int teamId) throws IOException { - return getTeam((long)teamId); + return getTeam((long) teamId); } /** From b45f353fa95b59d061e65269cc9e75dbca1e3000 Mon Sep 17 00:00:00 2001 From: Karol Lassak Date: Sun, 19 Apr 2020 11:38:36 +0200 Subject: [PATCH 5/6] Fix tests + add deprecation to one of methods --- src/main/java/org/kohsuke/github/GHProject.java | 5 +++-- src/test/java/org/kohsuke/github/AppTest.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHProject.java b/src/main/java/org/kohsuke/github/GHProject.java index 1c183a67cb..e2f017c789 100644 --- a/src/main/java/org/kohsuke/github/GHProject.java +++ b/src/main/java/org/kohsuke/github/GHProject.java @@ -42,7 +42,6 @@ public class GHProject extends GHObject { private String owner_url; private String html_url; - private String node_id; private String name; private String body; private int number; @@ -105,10 +104,12 @@ public URL getOwnerUrl() { /** * Gets node id. * + * @deprecated Use {@link GHObject#getNodeId()} * @return the node id */ + @Deprecated public String getNode_id() { - return node_id; + return getNodeId(); } /** diff --git a/src/test/java/org/kohsuke/github/AppTest.java b/src/test/java/org/kohsuke/github/AppTest.java index d2b498cb74..c0e0174e44 100755 --- a/src/test/java/org/kohsuke/github/AppTest.java +++ b/src/test/java/org/kohsuke/github/AppTest.java @@ -291,7 +291,7 @@ public void testShouldFetchTeam() throws Exception { GHOrganization organization = gitHub.getOrganization(GITHUB_API_TEST_ORG); GHTeam teamByName = organization.getTeams().get("Core Developers"); - GHTeam teamById = gitHub.getTeam(teamByName.getId()); + GHTeam teamById = gitHub.getTeam((int) teamByName.getId()); assertNotNull(teamById); assertEquals(teamByName.getId(), teamById.getId()); From 87f37e9f1c62fc4b71862924b25803dc81b41166 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Tue, 21 Apr 2020 17:34:15 -0700 Subject: [PATCH 6/6] Formatting and improved bridge method tests --- .../java/org/kohsuke/github/GHObject.java | 22 +++--- .../org/kohsuke/github/GHOrganization.java | 19 +++--- src/main/java/org/kohsuke/github/GitHub.java | 2 +- src/test/java/org/kohsuke/github/AppTest.java | 7 ++ .../org/kohsuke/github/BridgeMethodTest.java | 67 +++++++++++++------ 5 files changed, 76 insertions(+), 41 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHObject.java b/src/main/java/org/kohsuke/github/GHObject.java index 3504fc3823..c637690f4f 100644 --- a/src/main/java/org/kohsuke/github/GHObject.java +++ b/src/main/java/org/kohsuke/github/GHObject.java @@ -114,6 +114,17 @@ public Date getUpdatedAt() throws IOException { return GitHubClient.parseDate(updated_at); } + /** + * Get Global node_id from Github object. + * + * @see Using Global Node IDs + * + * @return Global Node ID. + */ + public String getNodeId() { + return node_id; + } + /** * Gets id. * @@ -124,17 +135,6 @@ public long getId() { return id; } - /** - * Get Global node_id from Github object. - * - * @see Using Global Node IDs - * - * @return Global Node ID. - */ - public String getNodeId() { - return node_id; - } - @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", justification = "Bridge method of getId") private Object longToStringOrInt(long id, Class type) { if (type == String.class) diff --git a/src/main/java/org/kohsuke/github/GHOrganization.java b/src/main/java/org/kohsuke/github/GHOrganization.java index 5de0cf35ca..7dc2720436 100644 --- a/src/main/java/org/kohsuke/github/GHOrganization.java +++ b/src/main/java/org/kohsuke/github/GHOrganization.java @@ -137,13 +137,11 @@ public PagedIterable listTeams() throws IOException { * @throws IOException * the io exception * - * @see documentation + * @deprecated Use {@link GHOrganization#getTeam(long)} */ - public GHTeam getTeam(long teamId) throws IOException { - return root.createRequest() - .withUrlPath(String.format("/organizations/%d/team/%d", id, teamId)) - .fetch(GHTeam.class) - .wrapUp(this); + @Deprecated + public GHTeam getTeam(int teamId) throws IOException { + return getTeam((long) teamId); } /** @@ -155,10 +153,13 @@ public GHTeam getTeam(long teamId) throws IOException { * @throws IOException * the io exception * - * @deprecated Use {@link GHOrganization#getTeam(long)} + * @see documentation */ - public GHTeam getTeam(int teamId) throws IOException { - return getTeam((long) teamId); + public GHTeam getTeam(long teamId) throws IOException { + return root.createRequest() + .withUrlPath(String.format("/organizations/%d/team/%d", id, teamId)) + .fetch(GHTeam.class) + .wrapUp(this); } /** diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 4684b96ec3..73a08df1bf 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -413,7 +413,7 @@ public GHRateLimit rateLimit() throws IOException { * @throws IOException * the io exception */ - @WithBridgeMethods(GHUser.class) + @WithBridgeMethods(value = GHUser.class) public GHMyself getMyself() throws IOException { client.requireCredential(); synchronized (this) { diff --git a/src/test/java/org/kohsuke/github/AppTest.java b/src/test/java/org/kohsuke/github/AppTest.java index c0e0174e44..d9fbcc1c7e 100755 --- a/src/test/java/org/kohsuke/github/AppTest.java +++ b/src/test/java/org/kohsuke/github/AppTest.java @@ -308,6 +308,13 @@ public void testShouldFetchTeamFromOrganization() throws Exception { assertEquals(teamByName.getId(), teamById.getId()); assertEquals(teamByName.getDescription(), teamById.getDescription()); + + GHTeam teamById2 = organization.getTeam((int) teamByName.getId()); + assertNotNull(teamById2); + + assertEquals(teamByName.getId(), teamById2.getId()); + assertEquals(teamByName.getDescription(), teamById2.getDescription()); + } @Ignore("Needs mocking check") diff --git a/src/test/java/org/kohsuke/github/BridgeMethodTest.java b/src/test/java/org/kohsuke/github/BridgeMethodTest.java index 0256b72515..78238333d7 100644 --- a/src/test/java/org/kohsuke/github/BridgeMethodTest.java +++ b/src/test/java/org/kohsuke/github/BridgeMethodTest.java @@ -5,12 +5,15 @@ import java.io.IOException; import java.lang.reflect.Method; +import java.net.URL; import java.util.ArrayList; import java.util.Date; import java.util.List; +import java.util.Set; -import static org.hamcrest.CoreMatchers.is; -import static org.hamcrest.Matchers.equalTo; +import javax.annotation.Nonnull; + +import static org.hamcrest.Matchers.*; /** * @author Kohsuke Kawaguchi @@ -18,26 +21,50 @@ public class BridgeMethodTest extends Assert { @Test - public void lastStatus() throws IOException { - GHObject obj = new GHIssue(); - - List createdAtMethods = new ArrayList<>(); - for (Method method : obj.getClass().getMethods()) { - if (method.getName().equalsIgnoreCase("getCreatedAt")) { - if (method.getReturnType() == Date.class) { - createdAtMethods.add(0, method); - } else { - createdAtMethods.add(method); - } - } - } + public void testBridgeMethods() throws IOException { + + // Some would say this is redundant, given that bridge methods are so thin anyway + // In the interest of maintaining binary compatibility, we'll do this anyway for a sampling of methods + + // Something odd here + // verifyBridgeMethods(new GHCommit(), "getAuthor", GHCommit.GHAuthor.class, GitUser.class); + // verifyBridgeMethods(new GHCommit(), "getCommitter", GHCommit.GHAuthor.class, GitUser.class); + + verifyBridgeMethods(GHIssue.class, "getCreatedAt", Date.class, String.class); + verifyBridgeMethods(GHIssue.class, "getId", int.class, long.class, String.class); + verifyBridgeMethods(GHIssue.class, "getUrl", String.class, URL.class); + + verifyBridgeMethods(GHOrganization.class, "getHtmlUrl", String.class, URL.class); + verifyBridgeMethods(GHOrganization.class, "getId", int.class, long.class, String.class); + verifyBridgeMethods(GHOrganization.class, "getUrl", String.class, URL.class); - assertThat(createdAtMethods.size(), equalTo(2)); + verifyBridgeMethods(GHRepository.class, "getCollaborators", GHPersonSet.class, Set.class); + verifyBridgeMethods(GHRepository.class, "getHtmlUrl", String.class, URL.class); + verifyBridgeMethods(GHRepository.class, "getId", int.class, long.class, String.class); + verifyBridgeMethods(GHRepository.class, "getUrl", String.class, URL.class); - assertThat(createdAtMethods.get(0).getParameterCount(), equalTo(0)); - assertThat(createdAtMethods.get(1).getParameterCount(), equalTo(0)); + verifyBridgeMethods(GHUser.class, "getFollows", GHPersonSet.class, Set.class); + verifyBridgeMethods(GHUser.class, "getFollowers", GHPersonSet.class, Set.class); + verifyBridgeMethods(GHUser.class, "getOrganizations", GHPersonSet.class, Set.class); + verifyBridgeMethods(GHUser.class, "getId", int.class, long.class, String.class); + + verifyBridgeMethods(GHTeam.class, "getId", int.class, long.class, String.class); + + // verifyBridgeMethods(GitHub.class, "getMyself", GHMyself.class, GHUser.class); + + } + + void verifyBridgeMethods(@Nonnull Class targetClass, @Nonnull String methodName, Class... returnTypes) { + List> foundMethods = new ArrayList<>(); + Method[] methods = targetClass.getMethods(); + for (Method method : methods) { + if (method.getName().equalsIgnoreCase(methodName)) { + // Bridge methods are only + assertThat(method.getParameterCount(), equalTo(0)); + foundMethods.add(method.getReturnType()); + } + } - assertThat(createdAtMethods.get(0).getReturnType(), is(Date.class)); - assertThat(createdAtMethods.get(1).getReturnType(), is(String.class)); + assertThat(foundMethods, containsInAnyOrder(returnTypes)); } }