Update @adamint's changes to make our JSONAPI responses more easily traversible #7

21echoes · 2017-10-26T03:25:32Z

#4 was super awesome, but needed some help getting over the finish line.

In total, this PR moves away from the sad world of:

JSONObject userResponse = apiClient.fetchUser();
JSONObject user = userResponse.getJSONObject("data");
JSONArray included = userResponse.getJSONArray("included");
JSONObject pledge = null;
JSONObject campaign = null;
if (included != null) {
    for (int i = 0; i < included.length(); i++) {
        JSONObject object = included.getJSONObject(i);
        if (object.getString("type").equals("pledge") && object.getJSONObject("relationships").getJSONObject("creator").getJSONObject("data").getString("id").equals(creatorID)) {
            pledge = object;
            break;
        }
    }
    for (int i = 0; i < included.length(); i++) {
        JSONObject object = included.getJSONObject(i);
        if (object.getString("type").equals("campaign") && object.getJSONObject("relationships").getJSONObject("creator").getJSONObject("data").getString("id").equals(creatorID)) {
            campaign = object;
            break;
        }
    }
}

into the happy world of:

JSONAPIDocument<User> userResponse = apiClient.fetchUser();
User user = userResponse.get();

List<Pledge> pledges = user.getPledges()
if (pledges != null && pledges.size() > 0) {
    Pledge pledge = pledges.get(0);
}

…st have paid

… the `.fetchUser()` call, will do the other methods next

…ns()`

…ledges()`

jrsun

reviewed by @kennydo and me

jrsun · 2017-10-26T18:37:46Z

README.md

@@ -13,60 +13,52 @@ Get the artifact from [Maven](http://search.maven.org/#search|ga|1|g%3A%22com.pa

 Step 1. Get your client_id and client_secret
 ---
-Visit the [OAuth Documentation Page](https://www.patreon.com/oauth2/documentation)
+Visit the [Patreon OAuth Platform Documentation Page](https://www.patreon.com/platform)


since the docs refer to https://docs.patreon.com, we should call this the Patreon Platform Page or something. Also, this link should be https://www.patreon.com/portal or similar.

jrsun · 2017-10-26T18:41:15Z

README.md

nit: capitalize visit

jrsun · 2017-10-26T18:46:31Z

src/main/java/com/patreon/PatreonAPI.java

+     */
+    public JSONAPIDocument<List<Campaign>> fetchCampaigns() throws IOException {
+        return converter.readDocumentCollection(
+            getDataStream("current_user/campaigns?include=rewards,creator,goals"),


maybe allow passing custom include and/or fields?

definitely want to do that, but i think it's out of scope for this PR 👍

jrsun · 2017-10-26T19:02:02Z

src/main/java/com/patreon/PatreonAPI.java

+     * @return JSONAPIDocument<List<Pledge>> containing pledges & associated data
+     * @throws IOException Thrown when the GET request failed
+     */
+    public JSONAPIDocument<List<Pledge>> fetchPageOfPledges(String campaignId, int pageSize, String pageCursor) throws IOException {


seems like this interface requires users who want pagination to parse out cursors from the next link, and pass back in to this method. could we hide that logic and provide an iterable interface?

it would be nice to still expose a startCursor to allow users to start iteration from a certain point.

👍 agreed that a helper method to get the link out would be nice (it's what python does, for instance)

jrsun · 2017-10-26T19:03:34Z

src/main/java/com/patreon/PatreonAPI.java

+            connection.setRequestProperty("Authorization", "Bearer ".concat(this.accessToken));
+            return connection.getInputStream();
+        } catch (IOException e) {
+            System.out.println(e.getMessage());


use SLF4J? ping @kennydo for example

jrsun · 2017-10-26T19:08:26Z

src/main/java/com/patreon/PatreonOAuth.java

+    }
+
+    public String getDefaultOauthUrl(String redirectUri) {
+        return "https://www.patreon.com/oauth2/authorize?response_type=code&client_id=" + clientID + "&redirect_uri=" + redirectUri;


can we use https://hc.apache.org/httpcomponents-client-ga/httpclient/apidocs/org/apache/http/client/utils/URIBuilder.html#addParameter(java.lang.String,%20java.lang.String) to prevent arbitrary redirectUri?

same for PatreonAPI.java urls?

it might be nice to use URIBuilder to construct the oauth flow urls as well, and then pass the result into Jsoup.connect

jrsun · 2017-10-26T19:14:36Z

src/main/java/com/patreon/PatreonOAuth.java

+        private String scope;
+        private String token_type;
+
+        public Token(String access_token, String refresh_token, int expires_in, String scope, String token_type) {


gson supports mapping field names to other capitalization schemes: https://stackoverflow.com/questions/2370745/convert-json-style-properties-names-to-java-camelcase-names-with-gson/11810183#11810183

yup, that's what I use for parsing most PatreonAPI stuff. how are you recommending we use that here?

jrsun · 2017-10-26T19:16:34Z

src/test/java/com/patreon/PatreonAPITest.java

+        JSONAPIDocument<List<Campaign>> campaignResponse = api.fetchCampaigns();
+        Assert.assertEquals(1, campaignResponse.get().size());
+        Campaign campaign = campaignResponse.get().get(0);
+        Assert.assertEquals("70261", campaign.getId());


pull expected campaign data into constant?

👍 there's more extensive stuff to do rewriting all tests -- right now they require you to add a real access_token, rather than mocking network

jrsun · 2017-10-26T19:23:54Z

src/test/java/com/patreon/PatreonOAuthTest.java

@@ -0,0 +1,26 @@
+package com.patreon;


is this file incomplete?

yes, this and the other test are both placeholders

…ding

jrsun · 2017-10-26T22:14:24Z

src/main/java/com/patreon/PatreonAPI.java

+
+    private InputStream getDataStream(String suffix) {
+        try {
+            String prefix = "https://api.patreon.com/oauth2/api/";


sorry for missing this, but i believe we encourage patreon.com/api over api.patreon.com because it saves an OPTIONS request

when you're not in a browser, there's no OPTIONS request, and this library is never used in a browser

jrsun · 2017-10-26T22:17:48Z

src/main/java/com/patreon/PatreonAPI.java

+     * @throws IOException Thrown when the GET request failed
+     */
+    public JSONAPIDocument<List<Pledge>> fetchPageOfPledges(String campaignId, int pageSize, String pageCursor) throws IOException {
+        String url = String.format("campaigns/%s/pledges?page%%5Bcount%%5D=%s", campaignId, pageSize);


nit: can this be new URIBuilder().setPath(blah).addParameter('page%5Bcount%5D', pageSize)

jrsun · 2017-10-26T22:18:14Z

src/main/java/com/patreon/PatreonAPI.java

+        if (pageCursor != null) {
+            try {
+                String escapedCursor = URLEncoder.encode(pageCursor, "UTF-8");
+                url.concat(String.format("&page%%5Bcursor%%5D=%s", escapedCursor));


same as above

jrsun

🥇

jrsun · 2017-10-27T01:11:00Z

src/main/java/com/patreon/PatreonAPI.java

+        return null;
+    }
+
+    public List<Pledge> fetchAllPledges(String campaignId) throws IOException {


this is bomb!!

jrsun · 2017-10-27T01:11:23Z

src/main/java/com/patreon/PatreonAPI.java

+        );
+    }
+
+    public String getCursorFromPledgesResponse(JSONAPIDocument<List<Pledge>> pledgesResponse) {


optional, possibly in a later PR: it would be really cool to make this resource-independent.

jrsun · 2017-10-27T01:14:44Z

src/main/java/com/patreon/PatreonAPI.java

+
+    private InputStream getDataStream(String suffix) throws IOException {
+        try {
+            String prefix = "https://api.patreon.com/oauth2/api/";


think this should be www.patreon.com/api

i'd really love to keep our platform API at api.patreon.com and have www.patreon.com/api be an in-house implementation detail for our js clients

adamint and others added 22 commits October 3, 2017 01:42

wrapper update

3545663

change campaign endpoint name, change readme to reflect new changes

d1548c9

added way to get users' social connections, will do goals later

23a11dc

added methods to obtain goals, rewards, and cards

b40570b

Rework to make more flexible

43c05a8

Adds PatreonUserAttributes#getDiscordId and removes a test println

7e0393f

Changed readme to reflect new style

aefdc70

Added some documentation for major methods

6b14bb6

Add getAttributes for pledges

aea3da3

patron_pays_fees is always false

45c107b

found better solution - if declined_since is null, then the patron mu…

c4dfb49

…st have paid

update, testing oauth

b410ca5

quick readme update

7f980b0

resource folder clean-up, simple file moving version

3e9cef2

Delete the CombinedModel and LinkedModel in favor of the BaseResource

57a018f

Start moving to a more standard usage of jsonapi-converter. Works for…

98742e2

… the `.fetchUser()` call, will do the other methods next

Move to a more standard usage of jsonapi-converter for `.fetchCampaig…

27f6954

…ns()`

Move to a more standard usage of jsonapi-converter for `.fetchPageOfP…

2f2ac6b

…ledges()`

Clean up unneeded classes

63f8256

simplify folder structure and class names

6435f73

tests that were used to prove out new factoring

0c26bcf

update README.md

7876779

21echoes requested review from kennydo and jrsun October 26, 2017 03:25

Merge branch 'master' into adamint-master

3bc61ae

21echoes mentioned this pull request Oct 26, 2017

Wrapper rewrite and modernization #4

Closed

jrsun suggested changes Oct 26, 2017

View reviewed changes

21echoes added 3 commits October 26, 2017 14:43

PR feedback: README clean-up

5a9a9f7

PR feedback: clean up the PatreonOAuth class, especially the URL buil…

a0cb01d

…ding

PR feedback: a better logging system for PatreonAPI issues

29ee98d

21echoes added 3 commits October 26, 2017 14:48

lint our pom.xml

6f19951

fix typos

9e68143

properly configure Log4J

995a958

21echoes requested a review from jrsun October 26, 2017 22:00

jrsun reviewed Oct 26, 2017

View reviewed changes

add a fetchAllPledges method

e7a70c7

21echoes force-pushed the adamint-master branch from b8043fc to e7a70c7 Compare October 27, 2017 00:26

port to URIBuilder for all URI building

99f9372

21echoes requested a review from jrsun October 27, 2017 00:32

jrsun approved these changes Oct 27, 2017

View reviewed changes

21echoes added 4 commits October 27, 2017 10:57

genericize the getNextCursorFromDocument function

501ac6d

little readme tweak

42ed5d7

version bump

665262b

bump paging size for fetchAllPledges

156174a

21echoes merged commit c84328d into master Oct 27, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update @adamint's changes to make our JSONAPI responses more easily traversible #7

Update @adamint's changes to make our JSONAPI responses more easily traversible #7

21echoes commented Oct 26, 2017

jrsun left a comment

jrsun Oct 26, 2017

jrsun Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 26, 2017

jrsun Oct 26, 2017

jrsun Oct 26, 2017

jrsun Oct 26, 2017

jrsun Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 26, 2017

jrsun Oct 26, 2017

21echoes Oct 27, 2017 •

edited

Loading

jrsun Oct 26, 2017

jrsun Oct 26, 2017

jrsun left a comment

jrsun Oct 27, 2017

jrsun Oct 27, 2017

jrsun Oct 27, 2017

jrsun Oct 27, 2017

21echoes Oct 27, 2017

Update @adamint's changes to make our JSONAPI responses more easily traversible #7

Update @adamint's changes to make our JSONAPI responses more easily traversible #7

Conversation

21echoes commented Oct 26, 2017

jrsun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

21echoes Oct 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrsun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

21echoes Oct 27, 2017 •

edited

Loading