From d6d73f516563970b63c432351b17dd2540153ddf Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Wed, 5 Sep 2012 18:52:37 -0700 Subject: [PATCH] A step toward using Poster in place of the retrieve* methods. I was trying to add the flavor of the retrieve method that reads into an existing instance, when I realized that there are just too many orthogonal axes here to rely on overloaded methods. That calls for a builder pattern, which we already have --- it's called Poster, but it can actually already handle GET and other HTTP requests. So I'm retiring the retrieveXYZ methods and moving the code into Poster. This is the first step. --- pom.xml | 2 +- .../org/kohsuke/github/GHCommitComment.java | 7 +- src/main/java/org/kohsuke/github/GHHook.java | 3 +- src/main/java/org/kohsuke/github/GHIssue.java | 6 +- .../org/kohsuke/github/GHOrganization.java | 2 +- .../java/org/kohsuke/github/GHRepository.java | 12 +- src/main/java/org/kohsuke/github/GHUser.java | 4 +- src/main/java/org/kohsuke/github/GitHub.java | 11 +- src/main/java/org/kohsuke/github/Poster.java | 114 +++++++++++++----- src/test/java/org/kohsuke/AppTest.java | 2 +- 10 files changed, 105 insertions(+), 58 deletions(-) diff --git a/pom.xml b/pom.xml index 80feba216..7cf0c6bc0 100644 --- a/pom.xml +++ b/pom.xml @@ -64,7 +64,7 @@ org.codehaus.jackson jackson-mapper-asl - 1.5.0 + 1.9.9 commons-io diff --git a/src/main/java/org/kohsuke/github/GHCommitComment.java b/src/main/java/org/kohsuke/github/GHCommitComment.java index f4621c2b0..44d798342 100644 --- a/src/main/java/org/kohsuke/github/GHCommitComment.java +++ b/src/main/java/org/kohsuke/github/GHCommitComment.java @@ -98,9 +98,8 @@ public class GHCommitComment { */ public void update(String body) throws IOException { GHCommitComment r = new Poster(owner.root) - .with("body",body) - .withCredential() - .to(getApiTail(),GHCommitComment.class,"PATCH"); + .with("body", body) + .withCredential().method("PATCH").to(getApiTail(), GHCommitComment.class); this.body = body; } @@ -108,7 +107,7 @@ public class GHCommitComment { * Deletes this comment. */ public void delete() throws IOException { - new Poster(owner.root).withCredential().to(getApiTail(),null,"DELETE"); + new Poster(owner.root).withCredential().method("DELETE").to(getApiTail()); } private String getApiTail() { diff --git a/src/main/java/org/kohsuke/github/GHHook.java b/src/main/java/org/kohsuke/github/GHHook.java index 94144e6f0..89e7020e7 100644 --- a/src/main/java/org/kohsuke/github/GHHook.java +++ b/src/main/java/org/kohsuke/github/GHHook.java @@ -54,7 +54,6 @@ public final class GHHook { * Deletes this hook. */ public void delete() throws IOException { - new Poster(repository.root).withCredential() - .to(String.format("/repos/%s/%s/hooks/%d",repository.getOwnerName(),repository.getName(),id),null,"DELETE"); + new Poster(repository.root).withCredential().method("DELETE").to(String.format("/repos/%s/%s/hooks/%d", repository.getOwnerName(), repository.getName(), id)); } } diff --git a/src/main/java/org/kohsuke/github/GHIssue.java b/src/main/java/org/kohsuke/github/GHIssue.java index 3224e76f2..e0d7976b7 100644 --- a/src/main/java/org/kohsuke/github/GHIssue.java +++ b/src/main/java/org/kohsuke/github/GHIssue.java @@ -26,7 +26,6 @@ package org.kohsuke.github; import java.io.IOException; import java.net.URL; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Date; @@ -116,12 +115,11 @@ public class GHIssue { * Updates the issue by adding a comment. */ public void comment(String message) throws IOException { - new Poster(root).withCredential().with("body",message).to(getApiRoute()+"/comments",null,"POST"); + new Poster(root).withCredential().with("body",message).to(getApiRoute() + "/comments"); } private void edit(String key, Object value) throws IOException { - new Poster(root).withCredential()._with(key, value) - .to(getApiRoute(),null,"PATCH"); + new Poster(root).withCredential()._with(key, value).method("PATCH").to(getApiRoute()); } /** diff --git a/src/main/java/org/kohsuke/github/GHOrganization.java b/src/main/java/org/kohsuke/github/GHOrganization.java index f15626370..21d50102c 100644 --- a/src/main/java/org/kohsuke/github/GHOrganization.java +++ b/src/main/java/org/kohsuke/github/GHOrganization.java @@ -101,7 +101,7 @@ public class GHOrganization extends GHPerson { repo_names.add(r.getName()); } post.with("repo_names",repo_names); - return post.to("/orgs/"+login+"/teams",GHTeam.class,"POST").wrapUp(this); + return post.method("POST").to("/orgs/" + login + "/teams", GHTeam.class).wrapUp(this); } public GHTeam createTeam(String name, Permission p, GHRepository... repositories) throws IOException { diff --git a/src/main/java/org/kohsuke/github/GHRepository.java b/src/main/java/org/kohsuke/github/GHRepository.java index a84840a93..0106deab4 100644 --- a/src/main/java/org/kohsuke/github/GHRepository.java +++ b/src/main/java/org/kohsuke/github/GHRepository.java @@ -253,7 +253,7 @@ public class GHRepository { private void modifyCollaborators(Collection users, String method) throws IOException { verifyMine(); for (GHUser user : users) { - new Poster(root).withCredential().to("/repos/"+owner.login+"/"+name+"/collaborators/"+user.getLogin(),null,method); + new Poster(root).withCredential().method(method).to("/repos/" + owner.login + "/" + name + "/collaborators/" + user.getLogin()); } } @@ -273,8 +273,7 @@ public class GHRepository { Poster poster = new Poster(root).withCredential(); if (!key.equals("name")) poster.with("name", name); // even when we don't change the name, we need to send it in - poster.with(key, value) - .to("/repos/" + owner.login + "/" + name, null, "PATCH"); + poster.with(key, value).method("PATCH").to("/repos/" + owner.login + "/" + name); } /** @@ -314,7 +313,7 @@ public class GHRepository { * Deletes this repository. */ public void delete() throws IOException { - new Poster(root).withCredential().to("/repos/" + owner.login +"/"+name, null, "DELETE"); + new Poster(root).withCredential().method("DELETE").to("/repos/" + owner.login + "/" + name); } /** @@ -324,7 +323,7 @@ public class GHRepository { * Newly forked repository that belong to you. */ public GHRepository fork() throws IOException { - return new Poster(root).withCredential().to("/repos/" + owner.login + "/" + name + "/forks", GHRepository.class, "POST").wrap(root); + return new Poster(root).withCredential().method("POST").to("/repos/" + owner.login + "/" + name + "/forks", GHRepository.class).wrap(root); } /** @@ -642,8 +641,7 @@ public class GHRepository { public GHMilestone createMilestone(String title, String description) throws IOException { return new Poster(root).withCredential() - .with("title", title).with("description", description) - .to("/repos/"+owner.login+"/"+name+"/milestones", GHMilestone.class,"POST").wrap(this); + .with("title", title).with("description", description).method("POST").to("/repos/" + owner.login + "/" + name + "/milestones", GHMilestone.class).wrap(this); } @Override diff --git a/src/main/java/org/kohsuke/github/GHUser.java b/src/main/java/org/kohsuke/github/GHUser.java index 07f44f71c..373a867c2 100644 --- a/src/main/java/org/kohsuke/github/GHUser.java +++ b/src/main/java/org/kohsuke/github/GHUser.java @@ -41,14 +41,14 @@ public class GHUser extends GHPerson { * Follow this user. */ public void follow() throws IOException { - new Poster(root).withCredential().to("/user/following/"+login,null,"PUT"); + new Poster(root).withCredential().method("PUT").to("/user/following/" + login); } /** * Unfollow this user. */ public void unfollow() throws IOException { - new Poster(root).withCredential().to("/user/following/"+login,null,"DELETE"); + new Poster(root).withCredential().method("DELETE").to("/user/following/" + login); } /** diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index f63cc5788..b0d2b60f2 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -170,6 +170,10 @@ public class GitHub { return new URL(tailApiUrl); } + /*package*/ Poster retrieve() { + return new Poster(this).method("GET"); + } + /*package*/ T retrieve(String tailApiUrl, Class type) throws IOException { return _retrieve(tailApiUrl, type, "GET", false); } @@ -379,7 +383,7 @@ public class GitHub { public GHUser getUser(String login) throws IOException { GHUser u = users.get(login); if (u == null) { - u = retrieve("/users/" + login, GHUser.class); + u = retrieve().to("/users/" + login, GHUser.class); u.root = this; users.put(u.getLogin(), u); } @@ -465,9 +469,10 @@ public class GitHub { * Newly created repository. */ public GHRepository createRepository(String name, String description, String homepage, boolean isPublic) throws IOException { - return new Poster(this).withCredential() + Poster poster = new Poster(this).withCredential() .with("name", name).with("description", description).with("homepage", homepage) - .with("public", isPublic ? 1 : 0).to("/user/repos", GHRepository.class,"POST").wrap(this); + .with("public", isPublic ? 1 : 0); + return poster.method("POST").to("/user/repos", GHRepository.class).wrap(this); } /** diff --git a/src/main/java/org/kohsuke/github/Poster.java b/src/main/java/org/kohsuke/github/Poster.java index d18c054b6..886947a90 100644 --- a/src/main/java/org/kohsuke/github/Poster.java +++ b/src/main/java/org/kohsuke/github/Poster.java @@ -26,6 +26,7 @@ package org.kohsuke.github; import org.apache.commons.io.IOUtils; import java.io.IOException; +import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; import java.lang.reflect.Field; @@ -36,11 +37,13 @@ import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.zip.GZIPInputStream; import static org.kohsuke.github.GitHub.*; /** - * Handles HTTP POST. + * A builder pattern for making HTTP call and parsing its output. + * * @author Kohsuke Kawaguchi */ class Poster { @@ -48,6 +51,11 @@ class Poster { private final List args = new ArrayList(); private boolean authenticate; + /** + * Request method. + */ + private String method = "POST"; + private static class Entry { String key; Object value; @@ -62,8 +70,12 @@ class Poster { this.root = root; } + /** + * Makes a request with authentication credential. + */ public Poster withCredential() { - root.requireCredential(); + // keeping it inline with retrieveWithAuth not to enforce the check + // root.requireCredential(); authenticate = true; return this; } @@ -97,12 +109,17 @@ class Poster { return this; } + public Poster method(String method) { + this.method = method; + return this; + } + public void to(String tailApiUrl) throws IOException { to(tailApiUrl,null); } /** - * POSTs the form to the specified URL. + * Sends a request to the specified URL, and parses the response into the given type via databinding. * * @throws IOException * if the server returns 4xx/5xx responses. @@ -110,54 +127,85 @@ class Poster { * {@link Reader} that reads the response. */ public T to(String tailApiUrl, Class type) throws IOException { - return to(tailApiUrl,type,"POST"); + return _to(tailApiUrl, type, null); } + /** + * Like {@link #to(String, Class)} but updates an existing object instead of creating a new instance. + */ + public T to(String tailApiUrl, T existingInstance) throws IOException { + return _to(tailApiUrl, null, existingInstance); + } + + /** + * Short for {@code method(method).to(tailApiUrl,type)} + */ + @Deprecated public T to(String tailApiUrl, Class type, String method) throws IOException { + return method(method).to(tailApiUrl,type); + } + + private T _to(String tailApiUrl, Class type, T instance) throws IOException { while (true) {// loop while API rate limit is hit HttpURLConnection uc = (HttpURLConnection) root.getApiURL(tailApiUrl).openConnection(); + uc.setRequestProperty("Accept-Encoding", "gzip"); - uc.setDoOutput(true); - uc.setRequestProperty("Content-type","application/x-www-form-urlencoded"); - if (authenticate) { - if (root.oauthAccessToken!=null) { - uc.setRequestProperty("Authorization", "token " + root.oauthAccessToken); - } else { - if (root.password==null) - throw new IllegalArgumentException("V3 API doesn't support API token"); - uc.setRequestProperty("Authorization", "Basic " + root.encodedAuthorization); + if (!method.equals("GET")) { + uc.setDoOutput(true); + uc.setRequestProperty("Content-type","application/x-www-form-urlencoded"); + if (authenticate) { + if (root.oauthAccessToken!=null) { + uc.setRequestProperty("Authorization", "token " + root.oauthAccessToken); + } else { + if (root.password==null) + throw new IllegalArgumentException("V3 API doesn't support API token"); + uc.setRequestProperty("Authorization", "Basic " + root.encodedAuthorization); + } } - } - try { - uc.setRequestMethod(method); - } catch (ProtocolException e) { - // JDK only allows one of the fixed set of verbs. Try to override that try { - Field $method = HttpURLConnection.class.getDeclaredField("method"); - $method.setAccessible(true); - $method.set(uc,method); - } catch (Exception x) { - throw (IOException)new IOException("Failed to set the custom verb").initCause(x); + uc.setRequestMethod(method); + } catch (ProtocolException e) { + // JDK only allows one of the fixed set of verbs. Try to override that + try { + Field $method = HttpURLConnection.class.getDeclaredField("method"); + $method.setAccessible(true); + $method.set(uc,method); + } catch (Exception x) { + throw (IOException)new IOException("Failed to set the custom verb").initCause(x); + } } - } - Map json = new HashMap(); - for (Entry e : args) { - json.put(e.key, e.value); + Map json = new HashMap(); + for (Entry e : args) { + json.put(e.key, e.value); + } + MAPPER.writeValue(uc.getOutputStream(),json); } - MAPPER.writeValue(uc.getOutputStream(),json); try { - InputStreamReader r = new InputStreamReader(uc.getInputStream(), "UTF-8"); + InputStreamReader r = new InputStreamReader(wrapStream(uc,uc.getInputStream()), "UTF-8"); String data = IOUtils.toString(r); - if (type==null) { - return null; - } - return MAPPER.readValue(data,type); + if (type!=null) + return MAPPER.readValue(data,type); + if (instance!=null) + return MAPPER.readerForUpdating(instance).readValue(data); + return null; } catch (IOException e) { root.handleApiError(e,uc); } } } + + /** + * Handles the "Content-Encoding" header. + */ + private InputStream wrapStream(HttpURLConnection uc, InputStream in) throws IOException { + String encoding = uc.getContentEncoding(); + if (encoding==null || in==null) return in; + if (encoding.equals("gzip")) return new GZIPInputStream(in); + + throw new UnsupportedOperationException("Unexpected Content-Encoding: "+encoding); + } + } diff --git a/src/test/java/org/kohsuke/AppTest.java b/src/test/java/org/kohsuke/AppTest.java index 8163d44be..1b3147012 100644 --- a/src/test/java/org/kohsuke/AppTest.java +++ b/src/test/java/org/kohsuke/AppTest.java @@ -87,7 +87,7 @@ public class AppTest extends TestCase { assertFalse(r.hasAdminAccess()); } - public void tryGetMyself() throws Exception { + public void testGetMyself() throws Exception { GitHub hub = GitHub.connect(); GHMyself me = hub.getMyself(); System.out.println(me);