Merge pull request hub4j#791 from ingwarsw/global_node_id

Add global node_id to GHObject + GHTeam extends GHObject

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;
 
@@ -113,6 +114,17 @@ public Date getUpdatedAt() throws IOException {
         return GitHubClient.parseDate(updated_at);
     }
 
+    /**
+     * Get Global node_id from Github object.
+     *
+     * @see <a href="https://developer.github.com/v4/guides/using-global-node-ids/">Using Global Node IDs</a>
+     *
+     * @return Global Node ID.
+     */
+    public String getNodeId() {
+        return node_id;
+    }
+
     /**
      * Gets id.
      *

src/main/java/org/kohsuke/github/GHOrganization.java

@@ -137,9 +137,25 @@ public PagedIterable<GHTeam> listTeams() throws IOException {
      * @throws IOException
      *             the io exception
      *
-     * @see <a href= "https://developer.github.com/v3/teams/#get-team-by-name">documentation</a>
+     * @deprecated Use {@link GHOrganization#getTeam(long)}
      */
+    @Deprecated
     public GHTeam getTeam(int teamId) throws IOException {
+        return getTeam((long) teamId);
+    }
+
+    /**
+     * 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
+     *
+     * @see <a href= "https://developer.github.com/v3/teams/#get-team-by-name">documentation</a>
+     */
+    public GHTeam getTeam(long teamId) throws IOException {
         return root.createRequest()
                 .withUrlPath(String.format("/organizations/%d/team/%d", id, teamId))
                 .fetch(GHTeam.class)

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();
     }
 
     /**

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);
+    }
 }

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) {
@@ -727,7 +727,7 @@ public Map<String, Set<GHTeam>> getMyTeams() throws IOException {
      * @throws IOException
      *             the io exception
      * 
-     * @deprecated Use {@link GHOrganization#getTeam(int)}
+     * @deprecated Use {@link GHOrganization#getTeam(long)}
      * @see <a href= "https://developer.github.com/v3/teams/#get-team-legacy">deprecation notice</a>
      */
     @Deprecated

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());
@@ -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")

src/test/java/org/kohsuke/github/BridgeMethodTest.java

@@ -5,39 +5,66 @@
 
 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
  */
 public class BridgeMethodTest extends Assert {
 
     @Test
-    public void lastStatus() throws IOException {
-        GHObject obj = new GHIssue();
-
-        List<Method> 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<Class<?>> 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));
     }
 }