Bitbucket list files fix #154
Conversation
| @@ -83,6 +83,10 @@ func (s *contentService) Delete(ctx context.Context, repo, path string, params * | |||
|
|
|||
| func (s *contentService) List(ctx context.Context, repo, path, ref string, opts scm.ListOptions) ([]*scm.ContentInfo, *scm.Response, error) { | |||
| endpoint := fmt.Sprintf("/2.0/repositories/%s/src/%s/%s?%s", repo, ref, path, encodeListOptions(opts)) | |||
| if opts.URL != "" { | |||
There was a problem hiding this comment.
This is incorrect, if you look at the encodeListOptions function it allows the user to pass through a page number https://github.com/mohitg0795/go-scm/blob/21de0f1a1424e92ac3a2c34aa26519a9f1f464d2/scm/driver/bitbucket/util.go#L27
There was a problem hiding this comment.
It doesn't get respected for bitbucket. Bitbucket doesn't support page number based pagination. It only supports url based pagination. Whatever page number you may pass, it will always provide you first page of output.
Thus, not tinkering with current logic, I just added support for url based pagination. If someone provides page url, then it overrides everything and it is used to parse the results.
There was a problem hiding this comment.
can you provide an example of the url you would pass.
i am extremely hesitant to merge this kind of change. as the URL parameter ignores all other parameters. at this point it would be better to use a simple http request.
this makes using bitbucket totally different to other git providers, meaning end users will miss this nuance.
We are having a team meeting later and will discuss this PR then
There was a problem hiding this comment.
There was a problem hiding this comment.
This is a sample URL.
mockNextPageUri := "https://api.bitbucket.org/2.0/repositories/atlassian/atlaskit/src/master/packages/activity?pageLen=3&page=RPfL"
Also, I've tested it by integrating with bitbucket realtime and got this URL during that process.
Further, I'm not tampering any behaviour. I'm just enhancing current support and infact fixing current behaviour. There's no page num based support for bitbucket. It results into HTTP 500 if you pass any page number except 0. You can further read on bitbucket documentation around this API. It only supports URL based pagination. It returns next page complete URL when we try to fetch 0th page and consequently it just works recursively in same way for other pages.
Let me know if we need to come on a call to finalize on this change. Appreciate your help. Thanks.
There was a problem hiding this comment.
@tphoney unlike GitHub or GitLab, Bitbucket does not support passing page numbers as query parameters for all endpoints; some endpoints require an after query parameter while others require an encoded page query parameter that is not an integer. Because pagination is not uniform across endpoints, Bitbucket includes the next url in the response body. The Bitbucket driver supports passing in this next url to list functions so that it can be used to fetch the next page in the resultset, regardless of how pagination is implemented. You can see an example here:
https://github.com/drone/go-scm/blob/master/scm/driver/bitbucket/repo.go#L99:L101
https://github.com/drone/go-scm/blob/master/scm/driver/bitbucket/testdata/repos.json#L109
In some cases the page and pagelen query parameters should work just fine (I am not sure about this particular endpoint). However, using the next url is guaranteed to always work, which means there is no harm in allowing this form of pagination for all Bitbucket list endpoints.
|
After the team review, this is the best solution available. Due to bitbucket. |
Currently, we only return first page of list files for bitbucket. It doesn't respect the incoming page number argument.
Bitbucket works on URL based pagination. Thus, added support if incoming page request has url, then it should be used, otherwise first page response will be returned.