diff --git a/src/main/java/org/kohsuke/github/GHEventInfo.java b/src/main/java/org/kohsuke/github/GHEventInfo.java index 2f3fc11a64..730aff28b2 100644 --- a/src/main/java/org/kohsuke/github/GHEventInfo.java +++ b/src/main/java/org/kohsuke/github/GHEventInfo.java @@ -142,7 +142,7 @@ public GHOrganization getOrganization() throws IOException { * if payload cannot be parsed */ public T getPayload(Class type) throws IOException { - T v = GitHubClient.MAPPER.readValue(payload.traverse(), type); + T v = GitHubClient.getMappingObjectReader().readValue(payload.traverse(), type); v.wrapUp(root); return v; } diff --git a/src/main/java/org/kohsuke/github/GHObject.java b/src/main/java/org/kohsuke/github/GHObject.java index c590444d3d..89d0aca89c 100644 --- a/src/main/java/org/kohsuke/github/GHObject.java +++ b/src/main/java/org/kohsuke/github/GHObject.java @@ -1,5 +1,6 @@ package org.kohsuke.github; +import com.fasterxml.jackson.annotation.JacksonInject; import com.infradna.tool.bridge_method_injector.WithBridgeMethods; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.commons.lang3.builder.ReflectionToStringBuilder; @@ -33,6 +34,19 @@ public abstract class GHObject { GHObject() { } + /** + * Called by Jackson + * + * @param responseInfo + * the {@link GitHubResponse.ResponseInfo} to get headers from. + */ + @JacksonInject + protected void setResponseHeaderFields(@CheckForNull GitHubResponse.ResponseInfo responseInfo) { + if (responseInfo != null) { + responseHeaderFields = responseInfo.headers(); + } + } + /** * Returns the HTTP response headers given along with the state of this object. * diff --git a/src/main/java/org/kohsuke/github/GHRateLimit.java b/src/main/java/org/kohsuke/github/GHRateLimit.java index d371f991b1..df82d24827 100644 --- a/src/main/java/org/kohsuke/github/GHRateLimit.java +++ b/src/main/java/org/kohsuke/github/GHRateLimit.java @@ -1,5 +1,6 @@ package org.kohsuke.github; +import com.fasterxml.jackson.annotation.JacksonInject; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -289,14 +290,11 @@ public static class Record { /** * The time at which the rate limit will reset. This value is calculated based on - * {@link #getResetEpochSeconds()} by calling {@link #recalculateResetDate}. If the clock on the local machine - * not synchronized with the server clock, this time value will be adjusted to match the local machine's clock. - *

- * Recalculated by calling {@link #recalculateResetDate}. - *

+ * {@link #getResetEpochSeconds()} by calling {@link #calculateResetDate}. If the clock on the local machine not + * synchronized with the server clock, this time value will be adjusted to match the local machine's clock. */ @Nonnull - private Date resetDate; + private final Date resetDate; /** * Instantiates a new Record. @@ -308,7 +306,6 @@ public static class Record { * @param resetEpochSeconds * the reset epoch seconds */ - @JsonCreator public Record(@JsonProperty(value = "limit", required = true) int limit, @JsonProperty(value = "remaining", required = true) int remaining, @JsonProperty(value = "reset", required = true) long resetEpochSeconds) { @@ -316,7 +313,7 @@ public Record(@JsonProperty(value = "limit", required = true) int limit, } /** - * Instantiates a new Record. + * Instantiates a new Record. Called by Jackson data binding or during header parsing. * * @param limit * the limit @@ -324,15 +321,22 @@ public Record(@JsonProperty(value = "limit", required = true) int limit, * the remaining * @param resetEpochSeconds * the reset epoch seconds - * @param updatedAt - * the updated at + * @param responseInfo + * the response info */ - @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD", justification = "Deprecated") - public Record(int limit, int remaining, long resetEpochSeconds, @CheckForNull String updatedAt) { + @JsonCreator + Record(@JsonProperty(value = "limit", required = true) int limit, + @JsonProperty(value = "remaining", required = true) int remaining, + @JsonProperty(value = "reset", required = true) long resetEpochSeconds, + @JacksonInject @CheckForNull GitHubResponse.ResponseInfo responseInfo) { this.limit = limit; this.remaining = remaining; this.resetEpochSeconds = resetEpochSeconds; - this.resetDate = recalculateResetDate(updatedAt); + String updatedAt = null; + if (responseInfo != null) { + updatedAt = responseInfo.headerField("Date"); + } + this.resetDate = calculateResetDate(updatedAt); } /** @@ -362,7 +366,8 @@ public Record(int limit, int remaining, long resetEpochSeconds, @CheckForNull St * a string date in RFC 1123 * @return reset date based on the passed date */ - Date recalculateResetDate(@CheckForNull String updatedAt) { + @Nonnull + private Date calculateResetDate(@CheckForNull String updatedAt) { long updatedAtEpochSeconds = createdAtEpochSeconds; if (!StringUtils.isBlank(updatedAt)) { try { @@ -379,7 +384,7 @@ Date recalculateResetDate(@CheckForNull String updatedAt) { // This may seem odd but it results in an accurate or slightly pessimistic reset date // based on system time rather than assuming the system time synchronized with the server long calculatedSecondsUntilReset = resetEpochSeconds - updatedAtEpochSeconds; - return resetDate = new Date((createdAtEpochSeconds + calculatedSecondsUntilReset) * 1000); + return new Date((createdAtEpochSeconds + calculatedSecondsUntilReset) * 1000); } /** diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index a20f09efe9..040d18459b 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -772,7 +772,7 @@ public GHGistBuilder createGist() { * the io exception */ public T parseEventPayload(Reader r, Class type) throws IOException { - T t = GitHubClient.MAPPER.readValue(r, type); + T t = GitHubClient.getMappingObjectReader().forType(type).readValue(r); t.wrapUp(this); return t; } diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java index bb4889933a..14df975588 100644 --- a/src/main/java/org/kohsuke/github/GitHubClient.java +++ b/src/main/java/org/kohsuke/github/GitHubClient.java @@ -1,8 +1,11 @@ package org.kohsuke.github; import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.ObjectReader; +import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.PropertyNamingStrategy; import com.fasterxml.jackson.databind.introspect.VisibilityChecker; import org.apache.commons.lang3.StringUtils; @@ -73,7 +76,7 @@ abstract class GitHubClient { private static final Logger LOGGER = Logger.getLogger(GitHubClient.class.getName()); - static final ObjectMapper MAPPER = new ObjectMapper(); + private static final ObjectMapper MAPPER = new ObjectMapper(); static final String GITHUB_URL = "https://api.github.com"; private static final String[] TIME_FORMATS = { "yyyy/MM/dd HH:mm:ss ZZZZ", "yyyy-MM-dd'T'HH:mm:ss'Z'", @@ -399,7 +402,6 @@ private static GitHubResponse createResponse(@Nonnull GitHubResponse.Resp // Maybe throw an exception instead? } else if (handler != null) { body = handler.apply(responseInfo); - setResponseHeaders(responseInfo, body); } return new GitHubResponse<>(responseInfo, body); } @@ -443,30 +445,6 @@ private static IOException interpretApiError(IOException e, return e; } - /** - * Sets the response headers on objects that need it. Ideally this would be handled by the objects themselves, but - * currently they do not have access to {@link GitHubResponse.ResponseInfo} after the - * - * @param responseInfo - * the response info - * @param readValue - * the object to consider adding headers to. - * @param - * type of the object - */ - private static void setResponseHeaders(GitHubResponse.ResponseInfo responseInfo, T readValue) { - if (readValue instanceof GHObject[]) { - for (GHObject ghObject : (GHObject[]) readValue) { - ghObject.responseHeaderFields = responseInfo.headers(); - } - } else if (readValue instanceof GHObject) { - ((GHObject) readValue).responseHeaderFields = responseInfo.headers(); - } else if (readValue instanceof JsonRateLimit) { - // if we're getting a GHRateLimit it needs the server date - ((JsonRateLimit) readValue).resources.getCore().recalculateResetDate(responseInfo.headerField("Date")); - } - } - protected static boolean isRateLimitResponse(@Nonnull GitHubResponse.ResponseInfo responseInfo) { return responseInfo.statusCode() == HttpURLConnection.HTTP_FORBIDDEN && "0".equals(responseInfo.headerField("X-RateLimit-Remaining")); @@ -567,7 +545,7 @@ private void noteRateLimit(@Nonnull GitHubResponse.ResponseInfo responseInfo) { return; } - GHRateLimit.Record observed = new GHRateLimit.Record(limit, remaining, reset, responseInfo.headerField("Date")); + GHRateLimit.Record observed = new GHRateLimit.Record(limit, remaining, reset, responseInfo); updateCoreRateLimit(observed); } @@ -714,4 +692,45 @@ static String printDate(Date dt) { df.setTimeZone(TimeZone.getTimeZone("GMT")); return df.format(dt); } + + /** + * Gets an {@link ObjectWriter}. + * + * @return an {@link ObjectWriter} instance that can be further configured. + */ + @Nonnull + static ObjectWriter getMappingObjectWriter() { + return MAPPER.writer(); + } + + /** + * Helper for {@link #getMappingObjectReader(GitHubResponse.ResponseInfo)} + * + * @return an {@link ObjectReader} instance that can be further configured. + */ + @Nonnull + static ObjectReader getMappingObjectReader() { + return getMappingObjectReader(null); + } + + /** + * Gets an {@link ObjectReader}. + * + * Members of {@link InjectableValues} must be present even if {@code null}, otherwise classes expecting those + * values will fail to read. This differs from regular JSONProperties which provide defaults instead of failing. + * + * Having one spot to create readers and having it take all injectable values is not a great long term solution but + * it is sufficient for this first cut. + * + * @param responseInfo + * the {@link GitHubResponse.ResponseInfo} to inject for this reader. + * @return an {@link ObjectReader} instance that can be further configured. + */ + @Nonnull + static ObjectReader getMappingObjectReader(@CheckForNull GitHubResponse.ResponseInfo responseInfo) { + InjectableValues.Std inject = new InjectableValues.Std(); + inject.addValue(GitHubResponse.ResponseInfo.class, responseInfo); + + return MAPPER.reader(inject); + } } diff --git a/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java b/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java index 0c7c3b7f5f..29e72bda86 100644 --- a/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java +++ b/src/main/java/org/kohsuke/github/GitHubHttpUrlConnectionClient.java @@ -153,7 +153,7 @@ private static void buildRequest(GitHubRequest request, HttpURLConnection connec for (GitHubRequest.Entry e : request.args()) { json.put(e.key, e.value); } - MAPPER.writeValue(connection.getOutputStream(), json); + getMappingObjectWriter().writeValue(connection.getOutputStream(), json); } } } @@ -235,4 +235,5 @@ private InputStream wrapStream(InputStream stream) throws IOException { private static final Logger LOGGER = Logger.getLogger(GitHubClient.class.getName()); } + } diff --git a/src/main/java/org/kohsuke/github/GitHubResponse.java b/src/main/java/org/kohsuke/github/GitHubResponse.java index cdb38bf3eb..50bb35d4ff 100644 --- a/src/main/java/org/kohsuke/github/GitHubResponse.java +++ b/src/main/java/org/kohsuke/github/GitHubResponse.java @@ -1,5 +1,6 @@ package org.kohsuke.github; +import com.fasterxml.jackson.databind.InjectableValues; import com.fasterxml.jackson.databind.JsonMappingException; import org.apache.commons.io.IOUtils; @@ -77,7 +78,10 @@ static T parseBody(ResponseInfo responseInfo, Class type) throws IOExcept String data = responseInfo.getBodyAsString(); try { - return GitHubClient.MAPPER.readValue(data, type); + InjectableValues.Std inject = new InjectableValues.Std(); + inject.addValue(ResponseInfo.class, responseInfo); + + return GitHubClient.getMappingObjectReader(responseInfo).forType(type).readValue(data); } catch (JsonMappingException e) { String message = "Failed to deserialize " + data; throw new IOException(message, e); @@ -102,7 +106,7 @@ static T parseBody(ResponseInfo responseInfo, T instance) throws IOException String data = responseInfo.getBodyAsString(); try { - return GitHubClient.MAPPER.readerForUpdating(instance).readValue(data); + return GitHubClient.getMappingObjectReader(responseInfo).withValueToUpdate(instance).readValue(data); } catch (JsonMappingException e) { String message = "Failed to deserialize " + data; throw new IOException(message, e); diff --git a/src/test/java/org/kohsuke/github/GHObjectTest.java b/src/test/java/org/kohsuke/github/GHObjectTest.java index d322d76568..370f13950b 100644 --- a/src/test/java/org/kohsuke/github/GHObjectTest.java +++ b/src/test/java/org/kohsuke/github/GHObjectTest.java @@ -12,5 +12,9 @@ public void test_toString() throws Exception { assertThat(org.toString(), containsString( "login=github-api-test-org,location=,blog=,email=,name=,company=,type=Organization,followers=0,following=0")); + + // getResponseHeaderFields is deprecated but we should not break it. + assertThat(org.getResponseHeaderFields(), notNullValue()); + assertThat(org.getResponseHeaderFields().get("Cache-Control").get(0), is("private, max-age=60, s-maxage=60")); } }