BUG SearchIssue id can be bigger than Integer.MAX#180
BUG SearchIssue id can be bigger than Integer.MAX#180vootelerotov wants to merge 2 commits intospotify:masterfrom
Conversation
Represent it as Long instead of Integer. For schema (where id is specified as 'int64') see: https://docs.github.com/en/rest/search/search?apiVersion=2022-11-28#search-issues-and-pull-requests
|
Ran into this in the wild. |
|
Hmm, build is failing with |
|
It seems that a lot of PR-s now have ID-s that are bigger than |
|
Comment ID could have the same integer overflow. e.g. It maybe safer to convert all ID of |
|
Also ran into this. |
|
Yep, had the same problem. |
Abhi347
left a comment
Abhi347
left a comment
There was a problem hiding this comment.
@vootelerotov The PR is good to go if you add the test json file. Please ping me once you have done so.
Thanks for your contribution
| @Test | ||
| public void testDeserializationWithLargeIssueId() throws IOException { | ||
| final String fixture = | ||
| Resources.toString(getResource(this.getClass(), "issues-with-large-id.json"), defaultCharset()); |
There was a problem hiding this comment.
Looks like you forgot to add the issues-with-large-id.json file in the test resources. It needs to be added at src/test/resources/com/spotify/github/v3/issues/issues-with-large-id.json for this PR to pass.
There was a problem hiding this comment.
Doh :(
@Abhi347 I think #200 is a more complete solution with passing tests (thanks @sindunuragarp), happy to close this PR in favour of it.
There was a problem hiding this comment.
@Abhi347 Would be great if one of the two PRs could be merged :)
There was a problem hiding this comment.
Merge #200 Closing this now. Thanks for your contribution.
|
Closing this as another PR (#200) fixing this issue is merged |
6 participants
Represent it as Long instead of Integer.
For schema (where id is specified as 'int64') see:
https://docs.github.com/en/rest/search/search?apiVersion=2022-11-28#search-issues-and-pull-requests