From 55b00a87f619250ddcd0784ada74fa10625e9980 Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Fri, 10 Feb 2017 03:28:06 +0300 Subject: [PATCH 1/4] Set 1.6 level. I'm not so old. Signed-off-by: Kanstantsin Shautsou --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 462816255a..0e908df6fe 100644 --- a/pom.xml +++ b/pom.xml @@ -41,13 +41,13 @@ org.codehaus.mojo.signature - java15 + java16 1.0 - ensure-java-1.5-class-library + ensure-java-1.6-class-library test check From be081eec3ff24317bb6423a11230712c2107e2b3 Mon Sep 17 00:00:00 2001 From: Kanstantsin Shautsou Date: Fri, 10 Feb 2017 03:33:18 +0300 Subject: [PATCH 2/4] Inject responce headers in GHObject and Exceptions. GH has specific to GET/POST headers required for analysing in case of error. Signed-off-by: Kanstantsin Shautsou --- pom.xml | 6 ++ .../java/org/kohsuke/github/GHObject.java | 13 ++- .../java/org/kohsuke/github/Requester.java | 53 ++++++++++--- .../exception/GHFileNotFoundException.java | 34 ++++++++ .../github/exception/GHIOException.java | 42 ++++++++++ .../java/org/kohsuke/github/GHHookTest.java | 79 +++++++++++++++++++ 6 files changed, 215 insertions(+), 12 deletions(-) create mode 100644 src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java create mode 100644 src/main/java/org/kohsuke/github/exception/GHIOException.java create mode 100644 src/test/java/org/kohsuke/github/GHHookTest.java diff --git a/pom.xml b/pom.xml index 0e908df6fe..9f372d703d 100644 --- a/pom.xml +++ b/pom.xml @@ -105,6 +105,12 @@ 4.11 test + + org.hamcrest + hamcrest-all + 1.3 + test + com.fasterxml.jackson.core jackson-databind diff --git a/src/main/java/org/kohsuke/github/GHObject.java b/src/main/java/org/kohsuke/github/GHObject.java index a81d913a5c..3a1cde1908 100644 --- a/src/main/java/org/kohsuke/github/GHObject.java +++ b/src/main/java/org/kohsuke/github/GHObject.java @@ -3,14 +3,15 @@ import com.infradna.tool.bridge_method_injector.WithBridgeMethods; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.lang.builder.ReflectionToStringBuilder; -import org.apache.commons.lang.builder.ToStringBuilder; import org.apache.commons.lang.builder.ToStringStyle; -import org.apache.commons.lang.reflect.FieldUtils; +import javax.annotation.CheckForNull; import java.io.IOException; import java.lang.reflect.Field; import java.net.URL; import java.util.Date; +import java.util.List; +import java.util.Map; /** * Most (all?) domain objects in GitHub seems to have these 4 properties. @@ -18,6 +19,9 @@ @SuppressFBWarnings(value = {"UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD", "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD"}, justification = "JSON API") public abstract class GHObject { + // not data but information related to data from responce + protected Map> responseHeaderFields; + protected String url; protected int id; protected String created_at; @@ -26,6 +30,11 @@ public abstract class GHObject { /*package*/ GHObject() { } + @CheckForNull + public Map> getResponseHeaderFields() { + return responseHeaderFields; + } + /** * When was this resource created? */ diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java index db70fbd64b..64aefe68fe 100644 --- a/src/main/java/org/kohsuke/github/Requester.java +++ b/src/main/java/org/kohsuke/github/Requester.java @@ -25,6 +25,10 @@ import com.fasterxml.jackson.databind.JsonMappingException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.apache.commons.io.IOUtils; +import org.kohsuke.github.exception.GHFileNotFoundException; +import org.kohsuke.github.exception.GHIOException; + import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -48,17 +52,18 @@ import java.util.Locale; import java.util.Map; import java.util.NoSuchElementException; -import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.zip.GZIPInputStream; + +import javax.annotation.CheckForNull; import javax.annotation.WillClose; -import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; import static java.util.Arrays.asList; -import static java.util.logging.Level.*; +import static java.util.logging.Level.FINE; +import static java.util.logging.Level.FINEST; import static org.kohsuke.github.GitHub.MAPPER; /** @@ -269,7 +274,7 @@ private T _to(String tailApiUrl, Class type, T instance) throws IOExcepti if (nextLinkMatcher.find()) { final String link = nextLinkMatcher.group(1); T nextResult = _to(link, type, instance); - + injectInResult(nextResult); final int resultLength = Array.getLength(result); final int nextResultLength = Array.getLength(nextResult); T concatResult = (T) Array.newInstance(type.getComponentType(), resultLength + nextResultLength); @@ -279,6 +284,7 @@ private T _to(String tailApiUrl, Class type, T instance) throws IOExcepti } } } + injectInResult(result); return result; } catch (IOException e) { handleApiError(e); @@ -579,6 +585,7 @@ private void setRequestMethod(HttpURLConnection uc) throws IOException { throw new IllegalStateException("Failed to set the request method to "+method); } + @CheckForNull private T parse(Class type, T instance) throws IOException { InputStreamReader r = null; int responseCode = -1; @@ -598,12 +605,17 @@ private T parse(Class type, T instance) throws IOException { String data = IOUtils.toString(r); if (type!=null) try { - return MAPPER.readValue(data,type); + final T readValue = MAPPER.readValue(data, type); + injectInResult(readValue); + return readValue; } catch (JsonMappingException e) { throw (IOException)new IOException("Failed to deserialize " +data).initCause(e); } - if (instance!=null) - return MAPPER.readerForUpdating(instance).readValue(data); + if (instance!=null) { + final T readValue = MAPPER.readerForUpdating(instance).readValue(data); + injectInResult(readValue); + return readValue; + } return null; } catch (FileNotFoundException e) { // java.net.URLConnection handles 404 exception has FileNotFoundException, don't wrap exception in HttpException @@ -616,6 +628,26 @@ private T parse(Class type, T instance) throws IOException { } } + private void injectInResult(T readValue) { + if (readValue instanceof GHObject[]) { + for (GHObject ghObject : (GHObject[]) readValue) { + injectInResult(ghObject); + } + } else if (readValue instanceof GHObject) { + injectInResult((GHObject) readValue); + } + } + + private void injectInResult(GHObject readValue) { + try { + final Field field = GHObject.class.getDeclaredField("responseHeaderFields"); + field.setAccessible(true); + field.set(readValue, uc.getHeaderFields()); + } catch (NoSuchFieldException ignore) { + } catch (IllegalAccessException ignore) { + } + } + /** * Handles the "Content-Encoding" header. */ @@ -663,15 +695,16 @@ private InputStream wrapStream(InputStream in) throws IOException { String error = IOUtils.toString(es, "UTF-8"); if (e instanceof FileNotFoundException) { // pass through 404 Not Found to allow the caller to handle it intelligently - throw (IOException) new FileNotFoundException(error).initCause(e); + throw (IOException) new GHFileNotFoundException(error).withResponseHeaderFields(uc).initCause(e); } else if (e instanceof HttpException) { HttpException http = (HttpException) e; throw (IOException) new HttpException(error, http.getResponseCode(), http.getResponseMessage(), http.getUrl(), e); } else { - throw (IOException) new IOException(error).initCause(e); + throw (IOException) new GHIOException(error).withResponceHeaderFields(uc).initCause(e); } - } else + } else { throw e; + } } finally { IOUtils.closeQuietly(es); } diff --git a/src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java b/src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java new file mode 100644 index 0000000000..27606de595 --- /dev/null +++ b/src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java @@ -0,0 +1,34 @@ +package org.kohsuke.github.exception; + +import javax.annotation.CheckForNull; +import java.io.FileNotFoundException; +import java.net.HttpURLConnection; +import java.util.List; +import java.util.Map; + +/** + * Request/responce contains useful metadata. + * Custom exception allows store info for next diagnostics. + * + * @author Kanstantsin Shautsou + */ +public class GHFileNotFoundException extends FileNotFoundException { + protected Map> responseHeaderFields; + + public GHFileNotFoundException() { + } + + public GHFileNotFoundException(String s) { + super(s); + } + + @CheckForNull + public Map> getResponseHeaderFields() { + return responseHeaderFields; + } + + public GHFileNotFoundException withResponseHeaderFields(HttpURLConnection urlConnection) { + this.responseHeaderFields = urlConnection.getHeaderFields(); + return this; + } +} diff --git a/src/main/java/org/kohsuke/github/exception/GHIOException.java b/src/main/java/org/kohsuke/github/exception/GHIOException.java new file mode 100644 index 0000000000..7bb614092e --- /dev/null +++ b/src/main/java/org/kohsuke/github/exception/GHIOException.java @@ -0,0 +1,42 @@ +package org.kohsuke.github.exception; + +import javax.annotation.CheckForNull; +import java.io.IOException; +import java.net.HttpURLConnection; +import java.util.List; +import java.util.Map; + +/** + * Request/responce contains useful metadata. + * Custom exception allows store info for next diagnostics. + * + * @author Kanstantsin Shautsou + */ +public class GHIOException extends IOException { + protected Map> responceHeaderFields; + + public GHIOException() { + } + + public GHIOException(String message) { + super(message); + } + + public GHIOException(String message, Throwable cause) { + super(message, cause); + } + + public GHIOException(Throwable cause) { + super(cause); + } + + @CheckForNull + public Map> getResponceHeaderFields() { + return responceHeaderFields; + } + + public GHIOException withResponceHeaderFields(HttpURLConnection urlConnection) { + this.responceHeaderFields = urlConnection.getHeaderFields(); + return this; + } +} diff --git a/src/test/java/org/kohsuke/github/GHHookTest.java b/src/test/java/org/kohsuke/github/GHHookTest.java new file mode 100644 index 0000000000..2cbab48cb7 --- /dev/null +++ b/src/test/java/org/kohsuke/github/GHHookTest.java @@ -0,0 +1,79 @@ +package org.kohsuke.github; + +import org.apache.commons.lang.StringUtils; +import org.junit.Ignore; +import org.junit.Test; +import org.kohsuke.github.exception.GHFileNotFoundException; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.hasValue; +import static org.hamcrest.core.IsInstanceOf.instanceOf; +import static org.junit.Assert.assertThat; + + +/** + * @author Kanstantsin Shautsou + */ +public class GHHookTest { + + @Ignore + @Test + public void exposeResponceHeaders() throws Exception { + String user1Login = "KostyaSha-auto"; + String user1Pass = "secret"; + + String clientId = "90140219451"; + String clientSecret = "1451245425"; + + String orgRepo = "KostyaSha-org/test"; + + // some login based user that has access to application + final GitHub gitHub = GitHub.connectUsingPassword(user1Login, user1Pass); + gitHub.getMyself(); + + // we request read + final List scopes = Arrays.asList("repo", "read:org", "user:email", "read:repo_hook"); + + // application creates token with scopes + final GHAuthorization auth = gitHub.createOrGetAuth(clientId, clientSecret, scopes, "", ""); + String token = auth.getToken(); + if (StringUtils.isEmpty(token)) { + gitHub.deleteAuth(auth.getId()); + token = gitHub.createOrGetAuth(clientId, clientSecret, scopes, "", "").getToken(); + } + + /// now create connection using token + final GitHub gitHub2 = GitHub.connectUsingOAuth(token); + // some repo in organisation + final GHRepository repository = gitHub2.getRepository(orgRepo); + + // doesn't fail because we have read access + final List hooks = repository.getHooks(); + + try { + // fails because application isn't approved in organisation and you can find it only after doing real call + final GHHook hook = repository.createHook( + "my-hook", + singletonMap("url", "http://localhost"), + singletonList(GHEvent.PUSH), + true + ); + } catch (IOException ex) { + assertThat(ex, instanceOf(GHFileNotFoundException.class)); + final GHFileNotFoundException ghFileNotFoundException = (GHFileNotFoundException) ex; + final Map> responseHeaderFields = ghFileNotFoundException.getResponseHeaderFields(); + assertThat(responseHeaderFields, hasKey("X-Accepted-OAuth-Scopes")); + assertThat(responseHeaderFields.get("X-Accepted-OAuth-Scopes"), + hasItem("admin:repo_hook, public_repo, repo, write:repo_hook") + ); + } + } +} From 92caf986835b711cd602792c5faa47bed99970c6 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 9 Sep 2017 12:57:52 -0700 Subject: [PATCH 3/4] Reverting java1.6 change which I assume is accidental. Not that I really care about Java5 but I think that change should be done separatel & intentionally --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 9f372d703d..fe8e0faff7 100644 --- a/pom.xml +++ b/pom.xml @@ -41,13 +41,13 @@ org.codehaus.mojo.signature - java16 + java15 1.0 - ensure-java-1.6-class-library + ensure-java-1.5-class-library test check From 692dccf110ceb2281c1d8ac887b6a66c70387f3a Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 9 Sep 2017 13:03:18 -0700 Subject: [PATCH 4/4] Massaging the change a bit --- .../GHFileNotFoundException.java | 4 +-- .../github/{exception => }/GHIOException.java | 12 +++---- .../java/org/kohsuke/github/GHObject.java | 16 +++++++-- .../java/org/kohsuke/github/Requester.java | 34 ++++++------------- .../java/org/kohsuke/github/GHHookTest.java | 1 - 5 files changed, 33 insertions(+), 34 deletions(-) rename src/main/java/org/kohsuke/github/{exception => }/GHFileNotFoundException.java (85%) rename src/main/java/org/kohsuke/github/{exception => }/GHIOException.java (66%) diff --git a/src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java b/src/main/java/org/kohsuke/github/GHFileNotFoundException.java similarity index 85% rename from src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java rename to src/main/java/org/kohsuke/github/GHFileNotFoundException.java index 27606de595..9f4b37c560 100644 --- a/src/main/java/org/kohsuke/github/exception/GHFileNotFoundException.java +++ b/src/main/java/org/kohsuke/github/GHFileNotFoundException.java @@ -1,4 +1,4 @@ -package org.kohsuke.github.exception; +package org.kohsuke.github; import javax.annotation.CheckForNull; import java.io.FileNotFoundException; @@ -27,7 +27,7 @@ public Map> getResponseHeaderFields() { return responseHeaderFields; } - public GHFileNotFoundException withResponseHeaderFields(HttpURLConnection urlConnection) { + GHFileNotFoundException withResponseHeaderFields(HttpURLConnection urlConnection) { this.responseHeaderFields = urlConnection.getHeaderFields(); return this; } diff --git a/src/main/java/org/kohsuke/github/exception/GHIOException.java b/src/main/java/org/kohsuke/github/GHIOException.java similarity index 66% rename from src/main/java/org/kohsuke/github/exception/GHIOException.java rename to src/main/java/org/kohsuke/github/GHIOException.java index 7bb614092e..5cdc9e541a 100644 --- a/src/main/java/org/kohsuke/github/exception/GHIOException.java +++ b/src/main/java/org/kohsuke/github/GHIOException.java @@ -1,4 +1,4 @@ -package org.kohsuke.github.exception; +package org.kohsuke.github; import javax.annotation.CheckForNull; import java.io.IOException; @@ -13,7 +13,7 @@ * @author Kanstantsin Shautsou */ public class GHIOException extends IOException { - protected Map> responceHeaderFields; + protected Map> responseHeaderFields; public GHIOException() { } @@ -31,12 +31,12 @@ public GHIOException(Throwable cause) { } @CheckForNull - public Map> getResponceHeaderFields() { - return responceHeaderFields; + public Map> getResponseHeaderFields() { + return responseHeaderFields; } - public GHIOException withResponceHeaderFields(HttpURLConnection urlConnection) { - this.responceHeaderFields = urlConnection.getHeaderFields(); + GHIOException withResponseHeaderFields(HttpURLConnection urlConnection) { + this.responseHeaderFields = urlConnection.getHeaderFields(); return this; } } diff --git a/src/main/java/org/kohsuke/github/GHObject.java b/src/main/java/org/kohsuke/github/GHObject.java index 3a1cde1908..96e41a2559 100644 --- a/src/main/java/org/kohsuke/github/GHObject.java +++ b/src/main/java/org/kohsuke/github/GHObject.java @@ -19,7 +19,9 @@ @SuppressFBWarnings(value = {"UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD", "UWF_UNWRITTEN_FIELD", "NP_UNWRITTEN_FIELD"}, justification = "JSON API") public abstract class GHObject { - // not data but information related to data from responce + /** + * Capture response HTTP headers on the state object. + */ protected Map> responseHeaderFields; protected String url; @@ -30,7 +32,17 @@ public abstract class GHObject { /*package*/ GHObject() { } - @CheckForNull + /** + * Returns the HTTP response headers given along with the state of this object. + * + *

+ * Some of the HTTP headers have nothing to do with the object, for example "Cache-Control" + * and others are different depending on how this object was retrieved. + * + * This method was added as a kind of hack to allow the caller to retrieve OAuth scopes and such. + * Use with caution. The method might be removed in the future. + */ + @CheckForNull @Deprecated public Map> getResponseHeaderFields() { return responseHeaderFields; } diff --git a/src/main/java/org/kohsuke/github/Requester.java b/src/main/java/org/kohsuke/github/Requester.java index 64aefe68fe..57ce89f0b1 100644 --- a/src/main/java/org/kohsuke/github/Requester.java +++ b/src/main/java/org/kohsuke/github/Requester.java @@ -26,8 +26,6 @@ import com.fasterxml.jackson.databind.JsonMappingException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.io.IOUtils; -import org.kohsuke.github.exception.GHFileNotFoundException; -import org.kohsuke.github.exception.GHIOException; import java.io.FileNotFoundException; import java.io.IOException; @@ -274,7 +272,7 @@ private T _to(String tailApiUrl, Class type, T instance) throws IOExcepti if (nextLinkMatcher.find()) { final String link = nextLinkMatcher.group(1); T nextResult = _to(link, type, instance); - injectInResult(nextResult); + setResponseHeaders(nextResult); final int resultLength = Array.getLength(result); final int nextResultLength = Array.getLength(nextResult); T concatResult = (T) Array.newInstance(type.getComponentType(), resultLength + nextResultLength); @@ -284,8 +282,7 @@ private T _to(String tailApiUrl, Class type, T instance) throws IOExcepti } } } - injectInResult(result); - return result; + return setResponseHeaders(result); } catch (IOException e) { handleApiError(e); } finally { @@ -605,16 +602,12 @@ private T parse(Class type, T instance) throws IOException { String data = IOUtils.toString(r); if (type!=null) try { - final T readValue = MAPPER.readValue(data, type); - injectInResult(readValue); - return readValue; + return setResponseHeaders(MAPPER.readValue(data, type)); } catch (JsonMappingException e) { throw (IOException)new IOException("Failed to deserialize " +data).initCause(e); } if (instance!=null) { - final T readValue = MAPPER.readerForUpdating(instance).readValue(data); - injectInResult(readValue); - return readValue; + return setResponseHeaders(MAPPER.readerForUpdating(instance).readValue(data)); } return null; } catch (FileNotFoundException e) { @@ -628,24 +621,19 @@ private T parse(Class type, T instance) throws IOException { } } - private void injectInResult(T readValue) { + private T setResponseHeaders(T readValue) { if (readValue instanceof GHObject[]) { for (GHObject ghObject : (GHObject[]) readValue) { - injectInResult(ghObject); + setResponseHeaders(ghObject); } } else if (readValue instanceof GHObject) { - injectInResult((GHObject) readValue); + setResponseHeaders((GHObject) readValue); } + return readValue; } - private void injectInResult(GHObject readValue) { - try { - final Field field = GHObject.class.getDeclaredField("responseHeaderFields"); - field.setAccessible(true); - field.set(readValue, uc.getHeaderFields()); - } catch (NoSuchFieldException ignore) { - } catch (IllegalAccessException ignore) { - } + private void setResponseHeaders(GHObject readValue) { + readValue.responseHeaderFields = uc.getHeaderFields(); } /** @@ -700,7 +688,7 @@ private InputStream wrapStream(InputStream in) throws IOException { HttpException http = (HttpException) e; throw (IOException) new HttpException(error, http.getResponseCode(), http.getResponseMessage(), http.getUrl(), e); } else { - throw (IOException) new GHIOException(error).withResponceHeaderFields(uc).initCause(e); + throw (IOException) new GHIOException(error).withResponseHeaderFields(uc).initCause(e); } } else { throw e; diff --git a/src/test/java/org/kohsuke/github/GHHookTest.java b/src/test/java/org/kohsuke/github/GHHookTest.java index 2cbab48cb7..b27484b5e5 100644 --- a/src/test/java/org/kohsuke/github/GHHookTest.java +++ b/src/test/java/org/kohsuke/github/GHHookTest.java @@ -3,7 +3,6 @@ import org.apache.commons.lang.StringUtils; import org.junit.Ignore; import org.junit.Test; -import org.kohsuke.github.exception.GHFileNotFoundException; import java.io.IOException; import java.util.Arrays;