Merge pull request #339

This commit is contained in:
Kohsuke Kawaguchi
2017-09-09 13:03:55 -07:00
6 changed files with 201 additions and 8 deletions

View File

@@ -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>

View File

@@ -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;
}
}

View File

@@ -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;
}
}

View File

@@ -5,10 +5,13 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
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.
@@ -16,6 +19,11 @@ import java.util.Date;
@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;
@@ -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?
*/

View File

@@ -28,6 +28,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
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;
@@ -277,7 +278,7 @@ class Requester {
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);
@@ -287,7 +288,7 @@ class Requester {
}
}
}
return result;
return setResponseHeaders(result);
} catch (IOException e) {
handleApiError(e);
} finally {
@@ -588,6 +589,7 @@ class Requester {
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);
}
@@ -611,12 +613,13 @@ class Requester {
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
@@ -624,7 +627,7 @@ class Requester {
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);
@@ -633,6 +636,21 @@ class Requester {
}
}
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.
*/
@@ -665,13 +683,13 @@ class Requester {
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);

View File

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