Skip to content

Commit

Permalink
Merge pull request #339
Browse files Browse the repository at this point in the history
  • Loading branch information
kohsuke committed Sep 9, 2017
2 parents 6178d38 + 692dccf commit d3ed8ea
Show file tree
Hide file tree
Showing 6 changed files with 201 additions and 8 deletions.
6 changes: 6 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,12 @@
<version>4.11</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-all</artifactId>
<version>1.3</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
Expand Down
34 changes: 34 additions & 0 deletions src/main/java/org/kohsuke/github/GHFileNotFoundException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.kohsuke.github;

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<String, List<String>> responseHeaderFields;

public GHFileNotFoundException() {
}

public GHFileNotFoundException(String s) {
super(s);
}

@CheckForNull
public Map<String, List<String>> getResponseHeaderFields() {
return responseHeaderFields;
}

GHFileNotFoundException withResponseHeaderFields(HttpURLConnection urlConnection) {
this.responseHeaderFields = urlConnection.getHeaderFields();
return this;
}
}
34 changes: 34 additions & 0 deletions src/main/java/org/kohsuke/github/GHIOException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package org.kohsuke.github;

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<String, List<String>> responseHeaderFields;

public GHIOException() {
}

public GHIOException(String message) {
super(message);
}

@CheckForNull
public Map<String, List<String>> getResponseHeaderFields() {
return responseHeaderFields;
}

GHIOException withResponseHeaderFields(HttpURLConnection urlConnection) {
this.responseHeaderFields = urlConnection.getHeaderFields();
return this;
}
}
23 changes: 23 additions & 0 deletions src/main/java/org/kohsuke/github/GHObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,25 @@
import org.apache.commons.lang.builder.ReflectionToStringBuilder;
import org.apache.commons.lang.builder.ToStringStyle;

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.
*/
@SuppressFBWarnings(value = {"UWF_UNWRITTEN_PUBLIC_OR_PROTECTED_FIELD", "UWF_UNWRITTEN_FIELD",
"NP_UNWRITTEN_FIELD"}, justification = "JSON API")
public abstract class GHObject {
/**
* Capture response HTTP headers on the state object.
*/
protected Map<String, List<String>> responseHeaderFields;

protected String url;
protected int id;
protected String created_at;
Expand All @@ -24,6 +32,21 @@ public abstract class GHObject {
/*package*/ GHObject() {
}

/**
* Returns the HTTP response headers given along with the state of this object.
*
* <p>
* 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<String, List<String>> getResponseHeaderFields() {
return responseHeaderFields;
}

/**
* When was this resource created?
*/
Expand Down
34 changes: 26 additions & 8 deletions src/main/java/org/kohsuke/github/Requester.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;

import javax.annotation.CheckForNull;
import javax.annotation.WillClose;
import java.io.FileNotFoundException;
import java.io.IOException;
Expand Down Expand Up @@ -277,7 +278,7 @@ private <T> T _to(String tailApiUrl, Class<T> type, T instance) throws IOExcepti
if (nextLinkMatcher.find()) {
final String link = nextLinkMatcher.group(1);
T nextResult = _to(link, type, instance);

setResponseHeaders(nextResult);
final int resultLength = Array.getLength(result);
final int nextResultLength = Array.getLength(nextResult);
T concatResult = (T) Array.newInstance(type.getComponentType(), resultLength + nextResultLength);
Expand All @@ -287,7 +288,7 @@ private <T> T _to(String tailApiUrl, Class<T> type, T instance) throws IOExcepti
}
}
}
return result;
return setResponseHeaders(result);
} catch (IOException e) {
handleApiError(e);
} finally {
Expand Down Expand Up @@ -588,6 +589,7 @@ private void setRequestMethod(HttpURLConnection uc) throws IOException {
throw new IllegalStateException("Failed to set the request method to "+method);
}

@CheckForNull
private <T> T parse(Class<T> type, T instance) throws IOException {
return parse(type, instance, 2);
}
Expand All @@ -611,20 +613,21 @@ private <T> T parse(Class<T> type, T instance, int timeouts) throws IOException
String data = IOUtils.toString(r);
if (type!=null)
try {
return MAPPER.readValue(data,type);
return setResponseHeaders(MAPPER.readValue(data, type));
} catch (JsonMappingException e) {
throw (IOException)new IOException("Failed to deserialize " +data).initCause(e);
}
if (instance!=null)
return MAPPER.readerForUpdating(instance).<T>readValue(data);
if (instance!=null) {
return setResponseHeaders(MAPPER.readerForUpdating(instance).<T>readValue(data));
}
return null;
} catch (FileNotFoundException e) {
// java.net.URLConnection handles 404 exception has FileNotFoundException, don't wrap exception in HttpException
// to preserve backward compatibility
throw e;
} catch (IOException e) {
if (e instanceof SocketTimeoutException && timeouts > 0) {
LOGGER.log(Level.INFO, "timed out accessing " + uc.getURL() + "; will try " + timeouts + " more time(s)", e);
LOGGER.log(INFO, "timed out accessing " + uc.getURL() + "; will try " + timeouts + " more time(s)", e);
return parse(type, instance, timeouts - 1);
}
throw new HttpException(responseCode, responseMessage, uc.getURL(), e);
Expand All @@ -633,6 +636,21 @@ private <T> T parse(Class<T> type, T instance, int timeouts) throws IOException
}
}

private <T> T setResponseHeaders(T readValue) {
if (readValue instanceof GHObject[]) {
for (GHObject ghObject : (GHObject[]) readValue) {
setResponseHeaders(ghObject);
}
} else if (readValue instanceof GHObject) {
setResponseHeaders((GHObject) readValue);
}
return readValue;
}

private void setResponseHeaders(GHObject readValue) {
readValue.responseHeaderFields = uc.getHeaderFields();
}

/**
* Handles the "Content-Encoding" header.
*/
Expand Down Expand Up @@ -665,13 +683,13 @@ 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
e = (IOException) new FileNotFoundException(error).initCause(e);
e = (IOException) new GHFileNotFoundException(error).withResponseHeaderFields(uc).initCause(e);
} else if (e instanceof HttpException) {
HttpException http = (HttpException) e;
e = new HttpException(error, http.getResponseCode(), http.getResponseMessage(),
http.getUrl(), e);
} else {
e = (IOException) new IOException(error).initCause(e);
e = (IOException) new GHIOException(error).withResponseHeaderFields(uc).initCause(e);
}
} finally {
IOUtils.closeQuietly(es);
Expand Down
78 changes: 78 additions & 0 deletions src/test/java/org/kohsuke/github/GHHookTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package org.kohsuke.github;

import org.apache.commons.lang.StringUtils;
import org.junit.Ignore;
import org.junit.Test;

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<String> 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<GHHook> 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<String, List<String>> 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")
);
}
}
}

0 comments on commit d3ed8ea

Please sign in to comment.