From 66fce794278f392e4b007f1da6c7c5a9f4146d8f Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 27 Feb 2020 06:31:48 -0800 Subject: [PATCH 1/3] Add ability to inject other local bindings --- .../java/org/kohsuke/github/GHEventInfo.java | 2 +- src/main/java/org/kohsuke/github/GitHub.java | 4 +- .../java/org/kohsuke/github/GitHubClient.java | 23 ++++++++-- .../org/kohsuke/github/GitHubRequest.java | 46 +++++++++++++++++-- 4 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHEventInfo.java b/src/main/java/org/kohsuke/github/GHEventInfo.java index 730aff28b2..2cca187f76 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.getMappingObjectReader().readValue(payload.traverse(), type); + T v = GitHubClient.getMappingObjectReader(root).readValue(payload.traverse(), type); v.wrapUp(root); return v; } diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 2e5d3510c0..53276d87d2 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.getMappingObjectReader().forType(type).readValue(r); + T t = GitHubClient.getMappingObjectReader(this).forType(type).readValue(r); t.wrapUp(this); return t; } @@ -1184,7 +1184,7 @@ GitHubClient getClient() { @Nonnull Requester createRequest() { - return new Requester(client); + return new Requester(client).inject(this); } GHUser intern(GHUser user) throws IOException { diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java index 14df975588..3dfcbeedb0 100644 --- a/src/main/java/org/kohsuke/github/GitHubClient.java +++ b/src/main/java/org/kohsuke/github/GitHubClient.java @@ -706,11 +706,16 @@ static ObjectWriter getMappingObjectWriter() { /** * Helper for {@link #getMappingObjectReader(GitHubResponse.ResponseInfo)} * + * @param root + * the root GitHub object for this reader + * * @return an {@link ObjectReader} instance that can be further configured. */ @Nonnull - static ObjectReader getMappingObjectReader() { - return getMappingObjectReader(null); + static ObjectReader getMappingObjectReader(@Nonnull GitHub root) { + ObjectReader reader = getMappingObjectReader((GitHubResponse.ResponseInfo) null); + ((InjectableValues.Std) reader.getInjectableValues()).addValue(GitHub.class, root); + return reader; } /** @@ -724,13 +729,21 @@ static ObjectReader getMappingObjectReader() { * * @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); + Map injected = new HashMap<>(); + + // Required or many things break + injected.put(GitHubResponse.ResponseInfo.class.getName(), null); + injected.put(GitHub.class.getName(), null); - return MAPPER.reader(inject); + if (responseInfo != null) { + injected.put(GitHubResponse.ResponseInfo.class.getName(), responseInfo); + injected.putAll(responseInfo.request().injected()); + } + return MAPPER.reader(new InjectableValues.Std(injected)); } } diff --git a/src/main/java/org/kohsuke/github/GitHubRequest.java b/src/main/java/org/kohsuke/github/GitHubRequest.java index 9ab1b94a7b..938b4987ba 100644 --- a/src/main/java/org/kohsuke/github/GitHubRequest.java +++ b/src/main/java/org/kohsuke/github/GitHubRequest.java @@ -40,6 +40,7 @@ class GitHubRequest { private static final List METHODS_WITHOUT_BODY = asList("GET", "DELETE"); private final List args; private final Map headers; + private final Map injected; private final String apiUrl; private final String urlPath; private final String method; @@ -50,6 +51,7 @@ class GitHubRequest { private GitHubRequest(@Nonnull List args, @Nonnull Map headers, + @Nonnull Map injected, @Nonnull String apiUrl, @Nonnull String urlPath, @Nonnull String method, @@ -57,6 +59,7 @@ private GitHubRequest(@Nonnull List args, boolean forceBody) throws MalformedURLException { this.args = Collections.unmodifiableList(new ArrayList<>(args)); this.headers = Collections.unmodifiableMap(new LinkedHashMap<>(headers)); + this.injected = Collections.unmodifiableMap(new LinkedHashMap<>(injected)); this.apiUrl = apiUrl; this.urlPath = urlPath; this.method = method; @@ -136,6 +139,16 @@ public Map headers() { return headers; } + /** + * The headers for this request. + * + * @return the {@link Map} of headers + */ + @Nonnull + public Map injected() { + return injected; + } + /** * The base GitHub API URL for this request represented as a {@link String} * @@ -203,7 +216,7 @@ public boolean inBody() { * @return a {@link Builder} based on this request. */ public Builder toBuilder() { - return new Builder<>(args, headers, apiUrl, urlPath, method, body, forceBody); + return new Builder<>(args, headers, injected, apiUrl, urlPath, method, body, forceBody); } private String buildTailApiUrl() { @@ -248,6 +261,12 @@ static class Builder> { @Nonnull private final Map headers; + /** + * Injected local data map + */ + @Nonnull + private final Map injected; + /** * The base GitHub API for this request. */ @@ -268,11 +287,19 @@ static class Builder> { * Create a new {@link GitHubRequest.Builder} */ protected Builder() { - this(new ArrayList<>(), new LinkedHashMap<>(), GitHubClient.GITHUB_URL, "/", "GET", null, false); + this(new ArrayList<>(), + new LinkedHashMap<>(), + new LinkedHashMap<>(), + GitHubClient.GITHUB_URL, + "/", + "GET", + null, + false); } private Builder(@Nonnull List args, @Nonnull Map headers, + @Nonnull Map injected, @Nonnull String apiUrl, @Nonnull String urlPath, @Nonnull String method, @@ -280,6 +307,7 @@ private Builder(@Nonnull List args, boolean forceBody) { this.args = new ArrayList<>(args); this.headers = new LinkedHashMap<>(headers); + this.injected = new LinkedHashMap<>(injected); this.apiUrl = apiUrl; this.urlPath = urlPath; this.method = method; @@ -295,7 +323,7 @@ private Builder(@Nonnull List args, * if the GitHub API URL cannot be constructed */ public GitHubRequest build() throws MalformedURLException { - return new GitHubRequest(args, headers, apiUrl, urlPath, method, body, forceBody); + return new GitHubRequest(args, headers, injected, apiUrl, urlPath, method, body, forceBody); } /** @@ -338,6 +366,18 @@ public B withHeader(String name, String value) { return (B) this; } + /** + * Object to inject into binding. + * + * @param value + * the value + * @return the request builder + */ + public B inject(Object value) { + this.injected.put(value.getClass().getName(), value); + return (B) this; + } + public B withPreview(String name) { return withHeader("Accept", name); } From 307508b7a059922b336d07a7897c6f7e1a712e3b Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 27 Feb 2020 08:38:29 -0800 Subject: [PATCH 2/3] Move GHGist from late binding to inject bind --- src/main/java/org/kohsuke/github/GHGist.java | 62 +++++++------------ .../org/kohsuke/github/GHGistBuilder.java | 2 +- .../org/kohsuke/github/GHGistUpdater.java | 2 +- src/main/java/org/kohsuke/github/GHUser.java | 2 +- src/main/java/org/kohsuke/github/GitHub.java | 2 +- 5 files changed, 26 insertions(+), 44 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHGist.java b/src/main/java/org/kohsuke/github/GHGist.java index 7e7c00d42b..a9059ecea8 100644 --- a/src/main/java/org/kohsuke/github/GHGist.java +++ b/src/main/java/org/kohsuke/github/GHGist.java @@ -1,12 +1,13 @@ package org.kohsuke.github; +import com.fasterxml.jackson.annotation.JacksonInject; +import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.commons.lang3.StringUtils; import java.io.IOException; import java.net.URL; import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.Map.Entry; @@ -19,8 +20,9 @@ * @see documentation */ public class GHGist extends GHObject { - /* package almost final */ GHUser owner; - /* package almost final */ GitHub root; + + final GHUser owner; + final GitHub root; private String forks_url, commits_url, id, git_pull_url, git_push_url, html_url; @@ -33,7 +35,19 @@ public class GHGist extends GHObject { private String comments_url; - private Map files = new HashMap(); + private final Map files; + + @JsonCreator + private GHGist(@JacksonInject GitHub root, + @JsonProperty("owner") GHUser owner, + @JsonProperty("files") Map files) { + this.root = root; + for (Entry e : files.entrySet()) { + e.getValue().fileName = e.getKey(); + } + this.files = Collections.unmodifiableMap(files); + this.owner = root.getUser(owner); + } /** * Gets owner. @@ -43,7 +57,7 @@ public class GHGist extends GHObject { * the io exception */ public GHUser getOwner() throws IOException { - return root.intern(owner); + return owner; } /** @@ -139,31 +153,7 @@ public GHGistFile getFile(String name) { * @return the files */ public Map getFiles() { - return Collections.unmodifiableMap(files); - } - - GHGist wrapUp(GHUser owner) { - this.owner = owner; - this.root = owner.root; - wrapUp(); - return this; - } - - /** - * Used when caller obtains {@link GHGist} without knowing its owner. A partially constructed owner object is - * interned. - */ - GHGist wrapUp(GitHub root) { - this.owner = root.getUser(owner); - this.root = root; - wrapUp(); - return this; - } - - private void wrapUp() { - for (Entry e : files.entrySet()) { - e.getValue().fileName = e.getKey(); - } + return files; } String getApiTailUrl(String tail) { @@ -213,7 +203,7 @@ public boolean isStarred() throws IOException { * the io exception */ public GHGist fork() throws IOException { - return root.createRequest().method("POST").withUrlPath(getApiTailUrl("forks")).fetch(GHGist.class).wrapUp(root); + return root.createRequest().method("POST").withUrlPath(getApiTailUrl("forks")).fetch(GHGist.class); } /** @@ -222,9 +212,7 @@ public GHGist fork() throws IOException { * @return the paged iterable */ public PagedIterable listForks() { - return root.createRequest() - .withUrlPath(getApiTailUrl("forks")) - .toIterable(GHGist[].class, item -> item.wrapUp(root)); + return root.createRequest().withUrlPath(getApiTailUrl("forks")).toIterable(GHGist[].class, null); } /** @@ -263,10 +251,4 @@ public boolean equals(Object o) { public int hashCode() { return id.hashCode(); } - - GHGist wrap(GHUser owner) { - this.owner = owner; - this.root = owner.root; - return this; - } } diff --git a/src/main/java/org/kohsuke/github/GHGistBuilder.java b/src/main/java/org/kohsuke/github/GHGistBuilder.java index 95ad403847..f8e04e0df3 100644 --- a/src/main/java/org/kohsuke/github/GHGistBuilder.java +++ b/src/main/java/org/kohsuke/github/GHGistBuilder.java @@ -72,6 +72,6 @@ public GHGistBuilder file(String fileName, String content) { */ public GHGist create() throws IOException { req.with("files", files); - return req.withUrlPath("/gists").fetch(GHGist.class).wrapUp(root); + return req.withUrlPath("/gists").fetch(GHGist.class); } } diff --git a/src/main/java/org/kohsuke/github/GHGistUpdater.java b/src/main/java/org/kohsuke/github/GHGistUpdater.java index 344d6b9335..d686065420 100644 --- a/src/main/java/org/kohsuke/github/GHGistUpdater.java +++ b/src/main/java/org/kohsuke/github/GHGistUpdater.java @@ -94,6 +94,6 @@ public GHGistUpdater description(String desc) { */ public GHGist update() throws IOException { builder.with("files", files); - return builder.method("PATCH").withUrlPath(base.getApiTailUrl("")).fetch(GHGist.class).wrap(base.owner); + return builder.method("PATCH").withUrlPath(base.getApiTailUrl("")).fetch(GHGist.class); } } diff --git a/src/main/java/org/kohsuke/github/GHUser.java b/src/main/java/org/kohsuke/github/GHUser.java index 89542f1ce2..cbc62b9956 100644 --- a/src/main/java/org/kohsuke/github/GHUser.java +++ b/src/main/java/org/kohsuke/github/GHUser.java @@ -217,7 +217,7 @@ public PagedIterable listEvents() throws IOException { public PagedIterable listGists() throws IOException { return root.createRequest() .withUrlPath(String.format("/users/%s/gists", login)) - .toIterable(GHGist[].class, item -> item.wrapUp(this)); + .toIterable(GHGist[].class, null); } @Override diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 53276d87d2..c1da14c9ef 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -743,7 +743,7 @@ public List getEvents() throws IOException { * the io exception */ public GHGist getGist(String id) throws IOException { - return createRequest().withUrlPath("/gists/" + id).fetch(GHGist.class).wrapUp(this); + return createRequest().withUrlPath("/gists/" + id).fetch(GHGist.class); } /** From 6d3904fbbd4285a9ee811ce8602426262f33570d Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 27 Feb 2020 14:27:57 -0800 Subject: [PATCH 3/3] Change name of inject methods --- src/main/java/org/kohsuke/github/GitHub.java | 2 +- .../java/org/kohsuke/github/GitHubClient.java | 2 +- .../org/kohsuke/github/GitHubRequest.java | 38 +++++++++++++------ 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index c1da14c9ef..1d215cb855 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -1184,7 +1184,7 @@ GitHubClient getClient() { @Nonnull Requester createRequest() { - return new Requester(client).inject(this); + return new Requester(client).injectMappingValue(this); } GHUser intern(GHUser user) throws IOException { diff --git a/src/main/java/org/kohsuke/github/GitHubClient.java b/src/main/java/org/kohsuke/github/GitHubClient.java index 3dfcbeedb0..1d38e5fb63 100644 --- a/src/main/java/org/kohsuke/github/GitHubClient.java +++ b/src/main/java/org/kohsuke/github/GitHubClient.java @@ -742,7 +742,7 @@ static ObjectReader getMappingObjectReader(@CheckForNull GitHubResponse.Response if (responseInfo != null) { injected.put(GitHubResponse.ResponseInfo.class.getName(), responseInfo); - injected.putAll(responseInfo.request().injected()); + injected.putAll(responseInfo.request().injectedMappingValues()); } return MAPPER.reader(new InjectableValues.Std(injected)); } diff --git a/src/main/java/org/kohsuke/github/GitHubRequest.java b/src/main/java/org/kohsuke/github/GitHubRequest.java index 938b4987ba..c487e10289 100644 --- a/src/main/java/org/kohsuke/github/GitHubRequest.java +++ b/src/main/java/org/kohsuke/github/GitHubRequest.java @@ -1,5 +1,6 @@ package org.kohsuke.github; +import edu.umd.cs.findbugs.annotations.NonNull; import org.apache.commons.lang3.StringUtils; import java.io.InputStream; @@ -40,7 +41,7 @@ class GitHubRequest { private static final List METHODS_WITHOUT_BODY = asList("GET", "DELETE"); private final List args; private final Map headers; - private final Map injected; + private final Map injectedMappingValues; private final String apiUrl; private final String urlPath; private final String method; @@ -51,7 +52,7 @@ class GitHubRequest { private GitHubRequest(@Nonnull List args, @Nonnull Map headers, - @Nonnull Map injected, + @Nonnull Map injectedMappingValues, @Nonnull String apiUrl, @Nonnull String urlPath, @Nonnull String method, @@ -59,7 +60,7 @@ private GitHubRequest(@Nonnull List args, boolean forceBody) throws MalformedURLException { this.args = Collections.unmodifiableList(new ArrayList<>(args)); this.headers = Collections.unmodifiableMap(new LinkedHashMap<>(headers)); - this.injected = Collections.unmodifiableMap(new LinkedHashMap<>(injected)); + this.injectedMappingValues = Collections.unmodifiableMap(new LinkedHashMap<>(injectedMappingValues)); this.apiUrl = apiUrl; this.urlPath = urlPath; this.method = method; @@ -145,8 +146,8 @@ public Map headers() { * @return the {@link Map} of headers */ @Nonnull - public Map injected() { - return injected; + public Map injectedMappingValues() { + return injectedMappingValues; } /** @@ -216,7 +217,7 @@ public boolean inBody() { * @return a {@link Builder} based on this request. */ public Builder toBuilder() { - return new Builder<>(args, headers, injected, apiUrl, urlPath, method, body, forceBody); + return new Builder<>(args, headers, injectedMappingValues, apiUrl, urlPath, method, body, forceBody); } private String buildTailApiUrl() { @@ -265,7 +266,7 @@ static class Builder> { * Injected local data map */ @Nonnull - private final Map injected; + private final Map injectedMappingValues; /** * The base GitHub API for this request. @@ -299,7 +300,7 @@ protected Builder() { private Builder(@Nonnull List args, @Nonnull Map headers, - @Nonnull Map injected, + @Nonnull Map injectedMappingValues, @Nonnull String apiUrl, @Nonnull String urlPath, @Nonnull String method, @@ -307,7 +308,7 @@ private Builder(@Nonnull List args, boolean forceBody) { this.args = new ArrayList<>(args); this.headers = new LinkedHashMap<>(headers); - this.injected = new LinkedHashMap<>(injected); + this.injectedMappingValues = new LinkedHashMap<>(injectedMappingValues); this.apiUrl = apiUrl; this.urlPath = urlPath; this.method = method; @@ -323,7 +324,7 @@ private Builder(@Nonnull List args, * if the GitHub API URL cannot be constructed */ public GitHubRequest build() throws MalformedURLException { - return new GitHubRequest(args, headers, injected, apiUrl, urlPath, method, body, forceBody); + return new GitHubRequest(args, headers, injectedMappingValues, apiUrl, urlPath, method, body, forceBody); } /** @@ -373,8 +374,21 @@ public B withHeader(String name, String value) { * the value * @return the request builder */ - public B inject(Object value) { - this.injected.put(value.getClass().getName(), value); + public B injectMappingValue(@NonNull Object value) { + return injectMappingValue(value.getClass().getName(), value); + } + + /** + * Object to inject into binding. + * + * @param name + * the name + * @param value + * the value + * @return the request builder + */ + public B injectMappingValue(@NonNull String name, Object value) { + this.injectedMappingValues.put(name, value); return (B) this; }