From 6aad80904a366bcc640b4f8f913b33e8738727d0 Mon Sep 17 00:00:00 2001 From: PauloMigAlmeida Date: Thu, 14 Nov 2019 15:52:43 +1300 Subject: [PATCH 1/6] Implement Meta endpoint Signed-off-by: PauloMigAlmeida --- src/main/java/org/kohsuke/github/GHMeta.java | 82 +++++++++++++++++ src/main/java/org/kohsuke/github/GitHub.java | 13 +++ .../java/org/kohsuke/github/GitHubTest.java | 18 ++-- ...-d7546658-0019-4aac-ad1a-502d7abd8050.json | 87 +++++++++++++++++++ .../getMeta/mappings/meta-1-d75466.json | 40 +++++++++ 5 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/kohsuke/github/GHMeta.java create mode 100644 src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/__files/meta-d7546658-0019-4aac-ad1a-502d7abd8050.json create mode 100644 src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/mappings/meta-1-d75466.json diff --git a/src/main/java/org/kohsuke/github/GHMeta.java b/src/main/java/org/kohsuke/github/GHMeta.java new file mode 100644 index 0000000000..6343012c1e --- /dev/null +++ b/src/main/java/org/kohsuke/github/GHMeta.java @@ -0,0 +1,82 @@ +package org.kohsuke.github; + +import com.fasterxml.jackson.annotation.JsonProperty; + +import java.util.List; + +/** + * Class that wraps the list of GitHub's IP addresses. + * + * @author Paulo Miguel Almeida + * + * @see GitHub#getMeta() + */ + +public class GHMeta { + + @JsonProperty("verifiable_password_authentication") + private boolean verifiablePasswordAuthentication; + private List hooks; + private List git; + private List web; + private List api; + private List pages; + private List importer; + + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + public void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { + this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; + } + + public List getHooks() { + return hooks; + } + + public void setHooks(List hooks) { + this.hooks = hooks; + } + + public List getGit() { + return git; + } + + public void setGit(List git) { + this.git = git; + } + + public List getWeb() { + return web; + } + + public void setWeb(List web) { + this.web = web; + } + + public List getApi() { + return api; + } + + public void setApi(List api) { + this.api = api; + } + + public List getPages() { + return pages; + } + + public void setPages(List pages) { + this.pages = pages; + } + + public List getImporter() { + return importer; + } + + public void setImporter(List importer) { + this.importer = importer; + } + +} diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 94ffd6f0b6..be753af230 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -802,6 +802,19 @@ public boolean isCredentialValid() { } } + /** + * Provides a list of GitHub's IP addresses. + * + * @see Get Meta + * + * @return an instance of {@link GHMeta} + * @throws IOException if the credentials supplied are invalid or if you're trying to access it as a GitHub App + * via the JWT authentication + */ + public GHMeta getMeta() throws IOException { + return retrieve().to("/meta", GHMeta.class); + } + /*package*/ GHUser intern(GHUser user) throws IOException { if (user==null) return user; diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index bd37714b41..df9dedfb77 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -1,18 +1,12 @@ package org.kohsuke.github; -import java.io.FileNotFoundException; import java.io.IOException; -import java.lang.reflect.Field; import java.util.*; import com.google.common.collect.Iterables; -import org.apache.commons.io.IOUtils; -import org.junit.Ignore; import org.junit.Test; import static org.hamcrest.CoreMatchers.*; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; /** * Unit test for {@link GitHub}. @@ -79,4 +73,16 @@ public void testListMyAuthorizations() throws IOException assertNotNull(auth.getAppName()); } } + + @Test + public void getMeta() throws IOException{ + GHMeta meta = gitHub.getMeta(); + assertTrue(meta.isVerifiablePasswordAuthentication()); + assertEquals(19, meta.getApi().size()); + assertEquals(19, meta.getGit().size()); + assertEquals(3, meta.getHooks().size()); + assertEquals(6, meta.getImporter().size()); + assertEquals(6, meta.getPages().size()); + assertEquals(19, meta.getWeb().size()); + } } diff --git a/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/__files/meta-d7546658-0019-4aac-ad1a-502d7abd8050.json b/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/__files/meta-d7546658-0019-4aac-ad1a-502d7abd8050.json new file mode 100644 index 0000000000..02db520957 --- /dev/null +++ b/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/__files/meta-d7546658-0019-4aac-ad1a-502d7abd8050.json @@ -0,0 +1,87 @@ +{ + "verifiable_password_authentication": true, + "hooks": [ + "192.30.252.0/22", + "185.199.108.0/22", + "140.82.112.0/20" + ], + "web": [ + "192.30.252.0/22", + "185.199.108.0/22", + "140.82.112.0/20", + "13.114.40.48/32", + "13.229.188.59/32", + "13.234.176.102/32", + "13.234.210.38/32", + "13.236.229.21/32", + "13.237.44.5/32", + "13.250.177.223/32", + "15.164.81.167/32", + "18.194.104.89/32", + "18.195.85.27/32", + "35.159.8.160/32", + "52.192.72.89/32", + "52.64.108.95/32", + "52.69.186.44/32", + "52.74.223.119/32", + "52.78.231.108/32" + ], + "api": [ + "192.30.252.0/22", + "185.199.108.0/22", + "140.82.112.0/20", + "13.209.163.61/32", + "13.230.158.120/32", + "13.233.76.15/32", + "13.234.168.60/32", + "13.236.14.80/32", + "13.238.54.232/32", + "13.250.168.23/32", + "13.250.94.254/32", + "18.179.245.253/32", + "18.194.201.191/32", + "18.195.135.122/32", + "52.58.203.252/32", + "52.63.231.178/32", + "52.69.239.207/32", + "54.169.195.247/32", + "54.180.75.25/32" + ], + "git": [ + "192.30.252.0/22", + "185.199.108.0/22", + "140.82.112.0/20", + "13.114.40.48/32", + "13.229.188.59/32", + "13.234.176.102/32", + "13.234.210.38/32", + "13.236.229.21/32", + "13.237.44.5/32", + "13.250.177.223/32", + "15.164.81.167/32", + "18.194.104.89/32", + "18.195.85.27/32", + "35.159.8.160/32", + "52.192.72.89/32", + "52.64.108.95/32", + "52.69.186.44/32", + "52.74.223.119/32", + "52.78.231.108/32" + ], + "pages": [ + "192.30.252.153/32", + "192.30.252.154/32", + "185.199.108.153/32", + "185.199.109.153/32", + "185.199.110.153/32", + "185.199.111.153/32" + ], + "importer": [ + "54.87.5.173", + "54.166.52.62", + "23.20.92.3", + "192.30.252.0/22", + "185.199.108.0/22", + "140.82.112.0/20" + ] +} \ No newline at end of file diff --git a/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/mappings/meta-1-d75466.json b/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/mappings/meta-1-d75466.json new file mode 100644 index 0000000000..37c886c2cc --- /dev/null +++ b/src/test/resources/org/kohsuke/github/GitHubTest/wiremock/getMeta/mappings/meta-1-d75466.json @@ -0,0 +1,40 @@ +{ + "id": "d7546658-0019-4aac-ad1a-502d7abd8050", + "name": "meta", + "request": { + "url": "/meta", + "method": "GET" + }, + "response": { + "status": 200, + "bodyFileName": "meta-d7546658-0019-4aac-ad1a-502d7abd8050.json", + "headers": { + "Date": "Thu, 14 Nov 2019 02:46:14 GMT", + "Content-Type": "application/json; charset=utf-8", + "Server": "GitHub.com", + "Status": "200 OK", + "X-RateLimit-Limit": "60", + "X-RateLimit-Remaining": "59", + "X-RateLimit-Reset": "1573703174", + "Cache-Control": "public, max-age=60, s-maxage=60", + "Vary": [ + "Accept", + "Accept-Encoding" + ], + "ETag": "W/\"eb00a8c05920e4cb21eb0d018c339fe1\"", + "X-GitHub-Media-Type": "unknown, github.v3", + "Access-Control-Expose-Headers": "ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type", + "Access-Control-Allow-Origin": "*", + "Strict-Transport-Security": "max-age=31536000; includeSubdomains; preload", + "X-Frame-Options": "deny", + "X-Content-Type-Options": "nosniff", + "X-XSS-Protection": "1; mode=block", + "Referrer-Policy": "origin-when-cross-origin, strict-origin-when-cross-origin", + "Content-Security-Policy": "default-src 'none'", + "X-GitHub-Request-Id": "FC2E:3B7E:AD9E9:C9032:5DCCBFF6" + } + }, + "uuid": "d7546658-0019-4aac-ad1a-502d7abd8050", + "persistent": true, + "insertionIndex": 1 +} \ No newline at end of file From d5ba0eebcbd0d73c3cd0bbaa600441fb1e99b32e Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 14 Nov 2019 09:12:28 -0800 Subject: [PATCH 2/6] Run code formatting --- src/main/java/org/kohsuke/github/GHMeta.java | 1 + src/main/java/org/kohsuke/github/GitHub.java | 5 +++-- src/test/java/org/kohsuke/github/GitHubTest.java | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHMeta.java b/src/main/java/org/kohsuke/github/GHMeta.java index 6343012c1e..b80c27764d 100644 --- a/src/main/java/org/kohsuke/github/GHMeta.java +++ b/src/main/java/org/kohsuke/github/GHMeta.java @@ -10,6 +10,7 @@ * @author Paulo Miguel Almeida * * @see GitHub#getMeta() + * @see Get Meta */ public class GHMeta { diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 175e36ef09..ed17ee46f3 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -808,8 +808,9 @@ public boolean isCredentialValid() { * @see Get Meta * * @return an instance of {@link GHMeta} - * @throws IOException if the credentials supplied are invalid or if you're trying to access it as a GitHub App - * via the JWT authentication + * @throws IOException + * if the credentials supplied are invalid or if you're trying to access it as a GitHub App via the JWT + * authentication */ public GHMeta getMeta() throws IOException { return retrieve().to("/meta", GHMeta.class); diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index b7b62764bc..b26ba1fad8 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -75,7 +75,7 @@ public void testListMyAuthorizations() throws IOException { } @Test - public void getMeta() throws IOException{ + public void getMeta() throws IOException { GHMeta meta = gitHub.getMeta(); assertTrue(meta.isVerifiablePasswordAuthentication()); assertEquals(19, meta.getApi().size()); From 6c2ce83b45e53f2abb9e3397784828596a5531c4 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 14 Nov 2019 10:29:40 -0800 Subject: [PATCH 3/6] Make GHMeta fields not externally settable --- src/main/java/org/kohsuke/github/GHMeta.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GHMeta.java b/src/main/java/org/kohsuke/github/GHMeta.java index b80c27764d..ebb96c6011 100644 --- a/src/main/java/org/kohsuke/github/GHMeta.java +++ b/src/main/java/org/kohsuke/github/GHMeta.java @@ -28,7 +28,7 @@ public boolean isVerifiablePasswordAuthentication() { return verifiablePasswordAuthentication; } - public void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { + void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; } @@ -36,7 +36,7 @@ public List getHooks() { return hooks; } - public void setHooks(List hooks) { + void setHooks(List hooks) { this.hooks = hooks; } @@ -44,7 +44,7 @@ public List getGit() { return git; } - public void setGit(List git) { + void setGit(List git) { this.git = git; } @@ -52,7 +52,7 @@ public List getWeb() { return web; } - public void setWeb(List web) { + void setWeb(List web) { this.web = web; } @@ -60,7 +60,7 @@ public List getApi() { return api; } - public void setApi(List api) { + void setApi(List api) { this.api = api; } @@ -68,7 +68,7 @@ public List getPages() { return pages; } - public void setPages(List pages) { + void setPages(List pages) { this.pages = pages; } @@ -76,7 +76,7 @@ public List getImporter() { return importer; } - public void setImporter(List importer) { + void setImporter(List importer) { this.importer = importer; } From 7d1e977ffe32093cb5284ed7f03b5e88de0b202d Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 14 Nov 2019 12:07:56 -0800 Subject: [PATCH 4/6] Make GHMeta fields effectively final --- src/main/java/org/kohsuke/github/GHMeta.java | 45 +- src/main/java/org/kohsuke/github/GitHub.java | 17 + .../example/dataobject/GHMetaExamples.java | 475 ++++++++++++++++++ .../java/org/kohsuke/github/GitHubTest.java | 21 + 4 files changed, 522 insertions(+), 36 deletions(-) create mode 100644 src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java diff --git a/src/main/java/org/kohsuke/github/GHMeta.java b/src/main/java/org/kohsuke/github/GHMeta.java index ebb96c6011..b2a9e22fe1 100644 --- a/src/main/java/org/kohsuke/github/GHMeta.java +++ b/src/main/java/org/kohsuke/github/GHMeta.java @@ -2,6 +2,8 @@ import com.fasterxml.jackson.annotation.JsonProperty; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; /** @@ -22,62 +24,33 @@ public class GHMeta { private List web; private List api; private List pages; - private List importer; + private List importer = new ArrayList<>(); public boolean isVerifiablePasswordAuthentication() { return verifiablePasswordAuthentication; } - void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { - this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; - } - public List getHooks() { - return hooks; - } - - void setHooks(List hooks) { - this.hooks = hooks; + return Collections.unmodifiableList(hooks); } public List getGit() { - return git; - } - - void setGit(List git) { - this.git = git; + return Collections.unmodifiableList(git); } public List getWeb() { - return web; - } - - void setWeb(List web) { - this.web = web; + return Collections.unmodifiableList(web); } public List getApi() { - return api; - } - - void setApi(List api) { - this.api = api; + return Collections.unmodifiableList(api); } public List getPages() { - return pages; - } - - void setPages(List pages) { - this.pages = pages; + return Collections.unmodifiableList(pages); } public List getImporter() { - return importer; + return Collections.unmodifiableList(importer); } - - void setImporter(List importer) { - this.importer = importer; - } - } diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index ed17ee46f3..2e6bd91a42 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -33,6 +33,7 @@ import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; +import org.kohsuke.github.example.dataobject.GHMetaExamples; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -816,6 +817,22 @@ public GHMeta getMeta() throws IOException { return retrieve().to("/meta", GHMeta.class); } + /** + * TEST-ONLY + *

+ * Provides a list of GitHub's IP addresses. For testing and example purposes only. + * + * @see Get Meta + * + * @return an instance of {@link GHMeta} + * @throws IOException + * if the credentials supplied are invalid or if you're trying to access it as a GitHub App via the JWT + * authentication + */ + GHMetaExamples.GHMetaExample getMetaExample(Class clazz) throws IOException { + return retrieve().to("/meta", clazz); + } + GHUser intern(GHUser user) throws IOException { if (user == null) return user; diff --git a/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java b/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java new file mode 100644 index 0000000000..faf6d6382c --- /dev/null +++ b/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java @@ -0,0 +1,475 @@ +package org.kohsuke.github.example.dataobject; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonSetter; +import org.kohsuke.github.GitHub; + +import javax.annotation.Nonnull; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +/** + * {@link org.kohsuke.github.GHMeta} wraps the list of GitHub's IP addresses. + *

+ * This class is used to show examples of different ways to create simple read-only data objects. + * For data objects that can be modified, perform actions, or get other objects we'll need other examples. + *

+ * IMPORTANT: There is no one right way to do this, but there are better and worse. + *

    + *
  • Better: {@link GHMetaGettersUnmodifiable} is a good balance of clarity and brevity
    • + *
  • Worse: {@link GHMetaPublic} exposes setters that are not needed, making it unclear that fields are actually read-only
    • + * + * @author Liam Newman + * + * @see org.kohsuke.github.GHMeta + * @see Get Meta + */ + +public final class GHMetaExamples { + + /** + * All GHMeta data objects should expose these values. + * + * @author Liam Newman + */ + public interface GHMetaExample { + boolean isVerifiablePasswordAuthentication(); + + List getHooks(); + + List getGit(); + + List getWeb(); + + List getApi(); + + List getPages(); + + List getImporter(); + } + + /** + * This version uses public getters and setters and leaves it up to Jackson how it wants to fill them. + *

    + * Pro: + *

      + *
    • Easy to create
      • + *
    • Not much code
      • + *
    • Mininal annotations
      • + *
    + * Con: + *
      + *
    • Exposes public setters for fields that should not be changed
      • + *
    • Lists modifiable when they should not be changed
      • + *
    • Jackson generally doesn't call the setters, it just sets the fields directly
      • + *
    + * + * @author Paulo Miguel Almeida + * @see org.kohsuke.github.GHMeta + */ + + public static class GHMetaPublic implements GHMetaExample { + + @JsonProperty("verifiable_password_authentication") + private boolean verifiablePasswordAuthentication; + private List hooks; + private List git; + private List web; + private List api; + private List pages; + private List importer; + + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + public void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { + this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; + } + + public List getHooks() { + return hooks; + } + + public void setHooks(List hooks) { + this.hooks = hooks; + } + + public List getGit() { + return git; + } + + public void setGit(List git) { + this.git = git; + } + + public List getWeb() { + return web; + } + + public void setWeb(List web) { + this.web = web; + } + + public List getApi() { + return api; + } + + public void setApi(List api) { + this.api = api; + } + + public List getPages() { + return pages; + } + + public void setPages(List pages) { + this.pages = pages; + } + + public List getImporter() { + return importer; + } + + public void setImporter(List importer) { + this.importer = importer; + } + + } + + /** + * This version uses public getters and shows that package or private setters both can be used by jackson. + * You can check this by running in debug and setting break points in the setters. + * + *

    + * Pro: + *

      + *
    • Easy to create
      • + *
    • Not much code
      • + *
    • Some annotations
      • + *
    + * Con: + *
      + *
    • Exposes some package setters for fields that should not be changed, better than public
      • + *
    • Lists modifiable when they should not be changed
      • + *
    + * + * @author Liam Newman + * @see org.kohsuke.github.GHMeta + */ + + public static class GHMetaPackage implements GHMetaExample { + + private boolean verifiablePasswordAuthentication; + private List hooks; + private List git; + private List web; + private List api; + private List pages; + + /** + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore + * getters and setters. + */ + @JsonProperty + private List importer; + + + @JsonProperty("verifiable_password_authentication") + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + private void setVerifiablePasswordAuthentication(boolean verifiablePasswordAuthentication) { + this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; + } + + @JsonProperty + public List getHooks() { + return hooks; + } + + /** + * Setters can be private (or package local) and will still be called by Jackson. + * The {@link JsonProperty} can got on the getter or setter and still work. + * + * @param hooks list of hooks + */ + private void setHooks(List hooks) { + this.hooks = hooks; + } + + public List getGit() { + return git; + } + + /** + * Since we mostly use Jackson for deserialization, {@link JsonSetter} is also okay, but + * {@link JsonProperty} is preferred. + * + * @param git list of git addresses + */ + @JsonSetter + void setGit(List git) { + this.git = git; + } + + public List getWeb() { + return web; + } + + /** + * The {@link JsonProperty} can got on the getter or setter and still work. + * + * @param web list of web addresses + */ + void setWeb(List web) { + this.web = web; + } + + @JsonProperty + public List getApi() { + return api; + } + + void setApi(List api) { + this.api = api; + } + + @JsonProperty + public List getPages() { + return pages; + } + + void setPages(List pages) { + this.pages = pages; + } + + /** + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore + * getters and setters. + * + * @return list of importer addresses + */ + public List getImporter() { + return importer; + } + + /** + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore + * getters and setters. + * + * @param importer list of importer addresses + */ + void setImporter(List importer) { + this.importer = importer; + } + + } + + /** + * This version uses only public getters and returns unmodifiable lists. + * + * + *

    + * Pro: + *

      + *
    • Very Easy to create
      • + *
    • Minimal code
      • + *
    • Mininal annotations
      • + *
    • Fields effectively final and lists unmodifiable
      • + *
    + * Con: + *
      + *
    • Effectively final is not quite really final
      • + *
    • If one of the lists were missing (an option member, for example), it will throw NPE but we could mitigate by checking for null or assigning a default.
      • + *
    + * + * @author Liam Newman + * @see org.kohsuke.github.GHMeta + */ + public static class GHMetaGettersUnmodifiable implements GHMetaExample { + + @JsonProperty("verifiable_password_authentication") + private boolean verifiablePasswordAuthentication; + private List hooks; + private List git; + private List web; + private List api; + private List pages; + /** + * If this were an optional member, we could fill it with an empty list by default. + */ + private List importer = new ArrayList<>(); + + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + public List getHooks() { + return Collections.unmodifiableList(hooks); + } + + public List getGit() { + return Collections.unmodifiableList(git); + } + + public List getWeb() { + return Collections.unmodifiableList(web); + } + + public List getApi() { + return Collections.unmodifiableList(api); + } + + public List getPages() { + return Collections.unmodifiableList(pages); + } + + public List getImporter() { + return Collections.unmodifiableList(importer); + } + } + + /** + * This version uses only public getters and returns unmodifiable lists and has final fields + *

    + * Pro: + *

      + *
    • Moderate amount of code
      • + *
    • More annotations
      • + *
    • Fields final and lists unmodifiable
      • + *
    + * Con: + *
      + *
    • Extra allocations - default array lists will be replaced by Jackson (yes, even though they are final)
      • + *
    • Added constructor is annoying
      • + *
    • If this object could be refreshed or populated, then the final is misleading (and possibly buggy)
      • + *
    + * + * @author Liam Newman + * @see org.kohsuke.github.GHMeta + */ + public static class GHMetaGettersFinal implements GHMetaExample { + + private final boolean verifiablePasswordAuthentication; + private final List hooks = new ArrayList<>(); + private final List git = new ArrayList<>(); + private final List web = new ArrayList<>(); + private final List api = new ArrayList<>(); + private final List pages = new ArrayList<>(); + private final List importer = new ArrayList<>(); + + @JsonCreator + private GHMetaGettersFinal(@JsonProperty("verifiable_password_authentication") + boolean verifiablePasswordAuthentication) { + // boolean fields when final seem to be really final, so we have to switch to constructor + this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; + } + + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + public List getHooks() { + return Collections.unmodifiableList(hooks); + } + + public List getGit() { + return Collections.unmodifiableList(git); + } + + public List getWeb() { + return Collections.unmodifiableList(web); + } + + public List getApi() { + return Collections.unmodifiableList(api); + } + + public List getPages() { + return Collections.unmodifiableList(pages); + } + + public List getImporter() { + return Collections.unmodifiableList(importer); + } + } + + /** + * This version uses only public getters and returns unmodifiable lists + *

    + * Pro: + *

      + *
    • Fields final and lists unmodifiable
      • + *
    • Construction behavior can be controlled - if values depended on each other or needed to be set in a specific order, this could do that.
      • + *
    + * Con: + *
      + *
    • There is no way you'd know about this without some research
      • + *
    • Specific annotations needed
      • + *
    • Brittle and verbose - not friendly to optional fields or large number of fields
      • + *
    + * + * @author Liam Newman + * @see org.kohsuke.github.GHMeta + */ + public static class GHMetaGettersFinalCreator implements GHMetaExample { + + private final boolean verifiablePasswordAuthentication; + private final List hooks; + private final List git; + private final List web; + private final List api; + private final List pages; + private final List importer; + + @JsonCreator + private GHMetaGettersFinalCreator(@Nonnull @JsonProperty("hooks") List hooks, + @Nonnull @JsonProperty("git") List git, + @Nonnull @JsonProperty("web") Listweb, + @Nonnull @JsonProperty("api")List api, + @Nonnull @JsonProperty("pages") Listpages, + @Nonnull @JsonProperty("importer")List importer, + @JsonProperty("verifiable_password_authentication") boolean verifiablePasswordAuthentication) { + this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; + this.hooks = Collections.unmodifiableList(hooks); + this.git = Collections.unmodifiableList(git); + this.web = Collections.unmodifiableList(web); + this.api = Collections.unmodifiableList(api); + this.pages = Collections.unmodifiableList(pages); + this.importer = Collections.unmodifiableList(importer); + } + + + public boolean isVerifiablePasswordAuthentication() { + return verifiablePasswordAuthentication; + } + + public List getHooks() { + return hooks; + } + + public List getGit() { + return git; + } + + public List getWeb() { + return web; + } + + public List getApi() { + return api; + } + + public List getPages() { + return pages; + } + + public List getImporter() { + return importer; + } + } +} \ No newline at end of file diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index b26ba1fad8..fa3cc42c68 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -5,6 +5,7 @@ import com.google.common.collect.Iterables; import org.junit.Test; +import org.kohsuke.github.example.dataobject.GHMetaExamples; import static org.hamcrest.CoreMatchers.*; @@ -84,5 +85,25 @@ public void getMeta() throws IOException { assertEquals(6, meta.getImporter().size()); assertEquals(6, meta.getPages().size()); assertEquals(19, meta.getWeb().size()); + + // Also test examples here + Class[] examples = new Class[] { + GHMetaExamples.GHMetaPublic.class, + GHMetaExamples.GHMetaPackage.class, + GHMetaExamples.GHMetaGettersUnmodifiable.class, + GHMetaExamples.GHMetaGettersFinal.class, + GHMetaExamples.GHMetaGettersFinalCreator.class, + }; + + for (Class metaClass : examples) { + GHMetaExamples.GHMetaExample metaExample = gitHub.getMetaExample(metaClass); + assertTrue(metaExample.isVerifiablePasswordAuthentication()); + assertEquals(19, metaExample.getApi().size()); + assertEquals(19, metaExample.getGit().size()); + assertEquals(3, metaExample.getHooks().size()); + assertEquals(6, metaExample.getImporter().size()); + assertEquals(6, metaExample.getPages().size()); + assertEquals(19, metaExample.getWeb().size()); + } } } From 7e05ce38cf980be759901d80d4f5609b4333fc18 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 14 Nov 2019 12:39:40 -0800 Subject: [PATCH 5/6] Add examples of different ways to create data objects --- src/main/java/org/kohsuke/github/GitHub.java | 3 +- .../example/dataobject/GHMetaExamples.java | 116 +++++++++--------- .../java/org/kohsuke/github/GitHubTest.java | 10 +- 3 files changed, 63 insertions(+), 66 deletions(-) diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index 2e6bd91a42..cfc3a8fd44 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -829,7 +829,8 @@ public GHMeta getMeta() throws IOException { * if the credentials supplied are invalid or if you're trying to access it as a GitHub App via the JWT * authentication */ - GHMetaExamples.GHMetaExample getMetaExample(Class clazz) throws IOException { + GHMetaExamples.GHMetaExample getMetaExample(Class clazz) + throws IOException { return retrieve().to("/meta", clazz); } diff --git a/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java b/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java index faf6d6382c..7d36950e65 100644 --- a/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java +++ b/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java @@ -13,13 +13,14 @@ /** * {@link org.kohsuke.github.GHMeta} wraps the list of GitHub's IP addresses. *

    - * This class is used to show examples of different ways to create simple read-only data objects. - * For data objects that can be modified, perform actions, or get other objects we'll need other examples. + * This class is used to show examples of different ways to create simple read-only data objects. For data objects that + * can be modified, perform actions, or get other objects we'll need other examples. *

    * IMPORTANT: There is no one right way to do this, but there are better and worse. *

      - *
    • Better: {@link GHMetaGettersUnmodifiable} is a good balance of clarity and brevity
      • - *
    • Worse: {@link GHMetaPublic} exposes setters that are not needed, making it unclear that fields are actually read-only
      • + *
    • Better: {@link GHMetaGettersUnmodifiable} is a good balance of clarity and brevity
      • + *
    • Worse: {@link GHMetaPublic} exposes setters that are not needed, making it unclear that fields are actually + * read-only
      • * * @author Liam Newman * @@ -55,15 +56,15 @@ public interface GHMetaExample { *

      * Pro: *

        - *
      • Easy to create
        • - *
      • Not much code
        • - *
      • Mininal annotations
        • + *
      • Easy to create
        • + *
      • Not much code
        • + *
      • Mininal annotations
        • *
      * Con: *
        - *
      • Exposes public setters for fields that should not be changed
        • - *
      • Lists modifiable when they should not be changed
        • - *
      • Jackson generally doesn't call the setters, it just sets the fields directly
        • + *
      • Exposes public setters for fields that should not be changed
        • + *
      • Lists modifiable when they should not be changed
        • + *
      • Jackson generally doesn't call the setters, it just sets the fields directly
        • *
      * * @author Paulo Miguel Almeida @@ -140,20 +141,20 @@ public void setImporter(List importer) { } /** - * This version uses public getters and shows that package or private setters both can be used by jackson. - * You can check this by running in debug and setting break points in the setters. + * This version uses public getters and shows that package or private setters both can be used by jackson. You can + * check this by running in debug and setting break points in the setters. * *

      * Pro: *

        - *
      • Easy to create
        • - *
      • Not much code
        • - *
      • Some annotations
        • + *
      • Easy to create
        • + *
      • Not much code
        • + *
      • Some annotations
        • *
      * Con: *
        - *
      • Exposes some package setters for fields that should not be changed, better than public
        • - *
      • Lists modifiable when they should not be changed
        • + *
      • Exposes some package setters for fields that should not be changed, better than public
        • + *
      • Lists modifiable when they should not be changed
        • *
      * * @author Liam Newman @@ -170,13 +171,11 @@ public static class GHMetaPackage implements GHMetaExample { private List pages; /** - * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore - * getters and setters. + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore getters and setters. */ @JsonProperty private List importer; - @JsonProperty("verifiable_password_authentication") public boolean isVerifiablePasswordAuthentication() { return verifiablePasswordAuthentication; @@ -192,10 +191,11 @@ public List getHooks() { } /** - * Setters can be private (or package local) and will still be called by Jackson. - * The {@link JsonProperty} can got on the getter or setter and still work. + * Setters can be private (or package local) and will still be called by Jackson. The {@link JsonProperty} can + * got on the getter or setter and still work. * - * @param hooks list of hooks + * @param hooks + * list of hooks */ private void setHooks(List hooks) { this.hooks = hooks; @@ -206,10 +206,11 @@ public List getGit() { } /** - * Since we mostly use Jackson for deserialization, {@link JsonSetter} is also okay, but - * {@link JsonProperty} is preferred. + * Since we mostly use Jackson for deserialization, {@link JsonSetter} is also okay, but {@link JsonProperty} is + * preferred. * - * @param git list of git addresses + * @param git + * list of git addresses */ @JsonSetter void setGit(List git) { @@ -223,7 +224,8 @@ public List getWeb() { /** * The {@link JsonProperty} can got on the getter or setter and still work. * - * @param web list of web addresses + * @param web + * list of web addresses */ void setWeb(List web) { this.web = web; @@ -248,8 +250,7 @@ void setPages(List pages) { } /** - * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore - * getters and setters. + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore getters and setters. * * @return list of importer addresses */ @@ -258,10 +259,10 @@ public List getImporter() { } /** - * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore - * getters and setters. + * Missing {@link JsonProperty} or having it on the field will cause Jackson to ignore getters and setters. * - * @param importer list of importer addresses + * @param importer + * list of importer addresses */ void setImporter(List importer) { this.importer = importer; @@ -276,15 +277,16 @@ void setImporter(List importer) { *

      * Pro: *

        - *
      • Very Easy to create
        • - *
      • Minimal code
        • - *
      • Mininal annotations
        • - *
      • Fields effectively final and lists unmodifiable
        • + *
      • Very Easy to create
        • + *
      • Minimal code
        • + *
      • Mininal annotations
        • + *
      • Fields effectively final and lists unmodifiable
        • *
      * Con: *
        - *
      • Effectively final is not quite really final
        • - *
      • If one of the lists were missing (an option member, for example), it will throw NPE but we could mitigate by checking for null or assigning a default.
        • + *
      • Effectively final is not quite really final
        • + *
      • If one of the lists were missing (an option member, for example), it will throw NPE but we could mitigate by + * checking for null or assigning a default.
        • *
      * * @author Liam Newman @@ -338,15 +340,15 @@ public List getImporter() { *

      * Pro: *

        - *
      • Moderate amount of code
        • - *
      • More annotations
        • - *
      • Fields final and lists unmodifiable
        • + *
      • Moderate amount of code
        • + *
      • More annotations
        • + *
      • Fields final and lists unmodifiable
        • *
      * Con: *
        - *
      • Extra allocations - default array lists will be replaced by Jackson (yes, even though they are final)
        • - *
      • Added constructor is annoying
        • - *
      • If this object could be refreshed or populated, then the final is misleading (and possibly buggy)
        • + *
      • Extra allocations - default array lists will be replaced by Jackson (yes, even though they are final)
        • + *
      • Added constructor is annoying
        • + *
      • If this object could be refreshed or populated, then the final is misleading (and possibly buggy)
        • *
      * * @author Liam Newman @@ -363,8 +365,8 @@ public static class GHMetaGettersFinal implements GHMetaExample { private final List importer = new ArrayList<>(); @JsonCreator - private GHMetaGettersFinal(@JsonProperty("verifiable_password_authentication") - boolean verifiablePasswordAuthentication) { + private GHMetaGettersFinal( + @JsonProperty("verifiable_password_authentication") boolean verifiablePasswordAuthentication) { // boolean fields when final seem to be really final, so we have to switch to constructor this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; } @@ -403,14 +405,15 @@ public List getImporter() { *

      * Pro: *

        - *
      • Fields final and lists unmodifiable
        • - *
      • Construction behavior can be controlled - if values depended on each other or needed to be set in a specific order, this could do that.
        • + *
      • Fields final and lists unmodifiable
        • + *
      • Construction behavior can be controlled - if values depended on each other or needed to be set in a specific + * order, this could do that.
        • *
      * Con: *
        - *
      • There is no way you'd know about this without some research
        • - *
      • Specific annotations needed
        • - *
      • Brittle and verbose - not friendly to optional fields or large number of fields
        • + *
      • There is no way you'd know about this without some research
        • + *
      • Specific annotations needed
        • + *
      • Brittle and verbose - not friendly to optional fields or large number of fields
        • *
      * * @author Liam Newman @@ -428,12 +431,10 @@ public static class GHMetaGettersFinalCreator implements GHMetaExample { @JsonCreator private GHMetaGettersFinalCreator(@Nonnull @JsonProperty("hooks") List hooks, - @Nonnull @JsonProperty("git") List git, - @Nonnull @JsonProperty("web") Listweb, - @Nonnull @JsonProperty("api")List api, - @Nonnull @JsonProperty("pages") Listpages, - @Nonnull @JsonProperty("importer")List importer, - @JsonProperty("verifiable_password_authentication") boolean verifiablePasswordAuthentication) { + @Nonnull @JsonProperty("git") List git, @Nonnull @JsonProperty("web") List web, + @Nonnull @JsonProperty("api") List api, @Nonnull @JsonProperty("pages") List pages, + @Nonnull @JsonProperty("importer") List importer, + @JsonProperty("verifiable_password_authentication") boolean verifiablePasswordAuthentication) { this.verifiablePasswordAuthentication = verifiablePasswordAuthentication; this.hooks = Collections.unmodifiableList(hooks); this.git = Collections.unmodifiableList(git); @@ -443,7 +444,6 @@ private GHMetaGettersFinalCreator(@Nonnull @JsonProperty("hooks") List h this.importer = Collections.unmodifiableList(importer); } - public boolean isVerifiablePasswordAuthentication() { return verifiablePasswordAuthentication; } diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index fa3cc42c68..ec73d8a90c 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -87,13 +87,9 @@ public void getMeta() throws IOException { assertEquals(19, meta.getWeb().size()); // Also test examples here - Class[] examples = new Class[] { - GHMetaExamples.GHMetaPublic.class, - GHMetaExamples.GHMetaPackage.class, - GHMetaExamples.GHMetaGettersUnmodifiable.class, - GHMetaExamples.GHMetaGettersFinal.class, - GHMetaExamples.GHMetaGettersFinalCreator.class, - }; + Class[] examples = new Class[] { GHMetaExamples.GHMetaPublic.class, GHMetaExamples.GHMetaPackage.class, + GHMetaExamples.GHMetaGettersUnmodifiable.class, GHMetaExamples.GHMetaGettersFinal.class, + GHMetaExamples.GHMetaGettersFinalCreator.class, }; for (Class metaClass : examples) { GHMetaExamples.GHMetaExample metaExample = gitHub.getMetaExample(metaClass); From fd436cf5b2cbf75544aacec4a9a3926c8df769c3 Mon Sep 17 00:00:00 2001 From: Liam Newman Date: Thu, 14 Nov 2019 13:01:35 -0800 Subject: [PATCH 6/6] Fixes for build failures --- src/main/java/org/kohsuke/github/GitHub.java | 20 ------------------- ...MetaExamples.java => ReadOnlyObjects.java} | 4 ++-- .../java/org/kohsuke/github/GitHubTest.java | 11 +++++----- 3 files changed, 8 insertions(+), 27 deletions(-) rename src/main/java/org/kohsuke/github/example/dataobject/{GHMetaExamples.java => ReadOnlyObjects.java} (99%) diff --git a/src/main/java/org/kohsuke/github/GitHub.java b/src/main/java/org/kohsuke/github/GitHub.java index cfc3a8fd44..875cb314c2 100644 --- a/src/main/java/org/kohsuke/github/GitHub.java +++ b/src/main/java/org/kohsuke/github/GitHub.java @@ -26,14 +26,11 @@ import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.MapperFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.PropertyNamingStrategy; import com.fasterxml.jackson.databind.introspect.VisibilityChecker.Std; import com.infradna.tool.bridge_method_injector.WithBridgeMethods; import org.apache.commons.codec.Charsets; import org.apache.commons.codec.binary.Base64; import org.apache.commons.io.IOUtils; -import org.apache.commons.lang3.StringUtils; -import org.kohsuke.github.example.dataobject.GHMetaExamples; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; @@ -817,23 +814,6 @@ public GHMeta getMeta() throws IOException { return retrieve().to("/meta", GHMeta.class); } - /** - * TEST-ONLY - *

      - * Provides a list of GitHub's IP addresses. For testing and example purposes only. - * - * @see Get Meta - * - * @return an instance of {@link GHMeta} - * @throws IOException - * if the credentials supplied are invalid or if you're trying to access it as a GitHub App via the JWT - * authentication - */ - GHMetaExamples.GHMetaExample getMetaExample(Class clazz) - throws IOException { - return retrieve().to("/meta", clazz); - } - GHUser intern(GHUser user) throws IOException { if (user == null) return user; diff --git a/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java b/src/main/java/org/kohsuke/github/example/dataobject/ReadOnlyObjects.java similarity index 99% rename from src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java rename to src/main/java/org/kohsuke/github/example/dataobject/ReadOnlyObjects.java index 7d36950e65..e56f405979 100644 --- a/src/main/java/org/kohsuke/github/example/dataobject/GHMetaExamples.java +++ b/src/main/java/org/kohsuke/github/example/dataobject/ReadOnlyObjects.java @@ -3,7 +3,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonSetter; -import org.kohsuke.github.GitHub; import javax.annotation.Nonnull; import java.util.ArrayList; @@ -21,6 +20,7 @@ *

    • Better: {@link GHMetaGettersUnmodifiable} is a good balance of clarity and brevity
      • *
    • Worse: {@link GHMetaPublic} exposes setters that are not needed, making it unclear that fields are actually * read-only
      • + *
    * * @author Liam Newman * @@ -28,7 +28,7 @@ * @see Get Meta */ -public final class GHMetaExamples { +public final class ReadOnlyObjects { /** * All GHMeta data objects should expose these values. diff --git a/src/test/java/org/kohsuke/github/GitHubTest.java b/src/test/java/org/kohsuke/github/GitHubTest.java index ec73d8a90c..81cfeba27b 100644 --- a/src/test/java/org/kohsuke/github/GitHubTest.java +++ b/src/test/java/org/kohsuke/github/GitHubTest.java @@ -5,7 +5,7 @@ import com.google.common.collect.Iterables; import org.junit.Test; -import org.kohsuke.github.example.dataobject.GHMetaExamples; +import org.kohsuke.github.example.dataobject.ReadOnlyObjects; import static org.hamcrest.CoreMatchers.*; @@ -87,12 +87,13 @@ public void getMeta() throws IOException { assertEquals(19, meta.getWeb().size()); // Also test examples here - Class[] examples = new Class[] { GHMetaExamples.GHMetaPublic.class, GHMetaExamples.GHMetaPackage.class, - GHMetaExamples.GHMetaGettersUnmodifiable.class, GHMetaExamples.GHMetaGettersFinal.class, - GHMetaExamples.GHMetaGettersFinalCreator.class, }; + Class[] examples = new Class[] { ReadOnlyObjects.GHMetaPublic.class, ReadOnlyObjects.GHMetaPackage.class, + ReadOnlyObjects.GHMetaGettersUnmodifiable.class, ReadOnlyObjects.GHMetaGettersFinal.class, + ReadOnlyObjects.GHMetaGettersFinalCreator.class, }; for (Class metaClass : examples) { - GHMetaExamples.GHMetaExample metaExample = gitHub.getMetaExample(metaClass); + ReadOnlyObjects.GHMetaExample metaExample = gitHub.retrieve().to("/meta", + (Class) metaClass); assertTrue(metaExample.isVerifiablePasswordAuthentication()); assertEquals(19, metaExample.getApi().size()); assertEquals(19, metaExample.getGit().size());