Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Headers #339

Merged
merged 2 commits into from
Sep 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@
<configuration>
<signature>
<groupId>org.codehaus.mojo.signature</groupId>
<artifactId>java15</artifactId>
<artifactId>java16</artifactId>
<version>1.0</version>
</signature>
</configuration>
<executions>
<execution>
<id>ensure-java-1.5-class-library</id>
<id>ensure-java-1.6-class-library</id>
<phase>test</phase>
<goals>
<goal>check</goal>
Expand Down Expand Up @@ -105,6 +105,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
13 changes: 11 additions & 2 deletions src/main/java/org/kohsuke/github/GHObject.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,25 @@
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.
*/
@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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or maybe even GHObject should extend GHResponse

protected Map<String, List<String>> responseHeaderFields;

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

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

/**
* When was this resource created?
*/
Expand Down
53 changes: 43 additions & 10 deletions src/main/java/org/kohsuke/github/Requester.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -269,7 +274,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);

injectInResult(nextResult);
final int resultLength = Array.getLength(result);
final int nextResultLength = Array.getLength(nextResult);
T concatResult = (T) Array.newInstance(type.getComponentType(), resultLength + nextResultLength);
Expand All @@ -279,6 +284,7 @@ private <T> T _to(String tailApiUrl, Class<T> type, T instance) throws IOExcepti
}
}
}
injectInResult(result);
return result;
} catch (IOException e) {
handleApiError(e);
Expand Down Expand Up @@ -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> T parse(Class<T> type, T instance) throws IOException {
InputStreamReader r = null;
int responseCode = -1;
Expand All @@ -598,12 +605,17 @@ private <T> T parse(Class<T> 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).<T>readValue(data);
if (instance!=null) {
final T readValue = MAPPER.readerForUpdating(instance).<T>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
Expand All @@ -616,6 +628,26 @@ private <T> T parse(Class<T> type, T instance) throws IOException {
}
}

private <T> 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.
*/
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, List<String>> responseHeaderFields;

public GHFileNotFoundException() {
}

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

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

public GHFileNotFoundException withResponseHeaderFields(HttpURLConnection urlConnection) {
this.responseHeaderFields = urlConnection.getHeaderFields();
return this;
}
}
42 changes: 42 additions & 0 deletions src/main/java/org/kohsuke/github/exception/GHIOException.java
Original file line number Diff line number Diff line change
@@ -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<String, List<String>> 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<String, List<String>> getResponceHeaderFields() {
return responceHeaderFields;
}

public GHIOException withResponceHeaderFields(HttpURLConnection urlConnection) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

response

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KostyaSha this typo was not addressed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it before the merge

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it before the merge

this.responceHeaderFields = urlConnection.getHeaderFields();
return this;
}
}
79 changes: 79 additions & 0 deletions src/test/java/org/kohsuke/github/GHHookTest.java
Original file line number Diff line number Diff line change
@@ -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<String> scopes = Arrays.asList("repo", "read:org", "user:email", "read:repo_hook");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final in tests looks like rubbish and doesn't add anything


// 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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is ExpectedExceptionRule for that purposes :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that requires it to be rethrown.
And the next few assertions build upon it being an instance of that class, so otherwise it's a CCE.
So having a clear error message isn't that bad IMO.

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