Petersoj / alpaca-java

A Java API for Alpaca, the commission free, algo friendly, stock trading broker. https://alpaca.markets
https://petersoj.github.io/alpaca-java/
MIT License
198 stars 84 forks source link

Update Polygon API for new v2 endpoints (#17) #18

Closed Petersoj closed 4 years ago

Petersoj commented 4 years ago

This PR is for #17. There is somewhat of a problem though. The following aggregate API response has two fields with a different case, that is, "T" and "t" in the "results" section:

{
  "ticker": "AAPL",
  "status": "OK",
  "adjusted": true,
  "queryCount": 55,
  "resultsCount": 2,
  "results": [
    {
      "T": "AAPL",
      "v": 31315282,
      "o": 102.87,
      "c": 103.74,
      "h": 103.82,
      "l": 102.65,
      "t": 1549314000000,
      "n": 4
    }
  ]
}

This now produces the error:

Execution failed for task ':alpaca-java:generateJsonSchema2Pojo'.
> trying to create the same field twice: t

It appears to be the makeLowerCamelCase() method in jsonSchema2Pojo which makes the first character in a field name lowercase regardless of propertyWordDelimiters being set to nothing (which is supposed to prevent alterations to JSON fields names when generating the JSON). This happens with all versions of the jsonschema2pojo gradle plugin too... I'm thinking of opening a PR in jsonschema2pojo to fix this for single characters. I was looking into just using JSON Schemas for that particular class, but that would get tedious after Polygon changes their API responses. Any ideas @mainstringargs?

Petersoj commented 4 years ago

Opened a PR in jsonschema2pojo here: https://github.com/joelittlejohn/jsonschema2pojo/pull/1031

mainstringargs commented 4 years ago

I think I may have seen this previously in another scenario, but I can't remember exactly which file it effected. I believe what I did was manually grab the 'ticker' and not include the 'T' in the example json file. You can see some of the hackery here: https://github.com/mainstringargs/alpaca-java/blob/fbbd3d111c113259431564d77961f75c15fcb61a/src/main/java/io/github/mainstringargs/polygon/rest/PolygonRequest.java in 'getResponseObject' method. Hopefully they will fix the issue in the jsonschema2pojo library itself, that would be great.

Petersoj commented 4 years ago

@mainstringargs Yeah that's pretty annoying... I've finished my PR over there. Hopefully they merge soon because I don't want to do that hackery haha. I also want to keep the objects as exposed as possible for people who are interfacing with this library and the ask price, size, and exchange JSON keys are now just the upper case of the bid price, size, and exchange keys so there will be even more hackery...

mainstringargs commented 4 years ago

Hey -- any feedback from the jsonschema2pojo people about rolling it in?

Petersoj commented 4 years ago

@mainstringargs nothing yet. Looks like I’ll have to just do the hack. I’ll also look into using JSON schemas instead of JSON sources and see if I can generate unique getters and setters that way. I’ll work on this tomorrow and start the Polygon Websockets PR this weekend.

mainstringargs commented 4 years ago

No rush, was more curious than anything

On Wed, Oct 23, 2019 at 2:02 PM Jacob Peterson notifications@github.com wrote:

@mainstringargs https://github.com/mainstringargs nothing yet. Looks like I’ll have to just do the hack. I’ll also look into using JSON schemas instead of JSON sources and see if I can generate unique getters and setters that way. I’ll work on this tomorrow and start the Polygon Websockets PR this weekend.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mainstringargs/alpaca-java/pull/18?email_source=notifications&email_token=ABVXM4VHPIKWWYV5WNFWBMDQQCNVFA5CNFSM4JCQVZJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOECCQT7I#issuecomment-545589757, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVXM4QJQ7JV2ITFMDJX4DDQQCNVFANCNFSM4JCQVZJQ .

Petersoj commented 4 years ago

Looks like it might be a while before that PR gets merged. We'll just have to go the with the hack...

Petersoj commented 4 years ago

I'm going to close this PR and reopen a new one with the changes to master so that I can resolve the issue with getter/setter name conflicts by implemented the changes in #24.