fix(api): follow the pagination Link URL without rebuilding the query

Description

glab api --paginate rebuilds the query string on every page, so each page after the first is requested with the caller's fields appended a second time onto the URL the server built: httpRequest runs the query builder over an absolute path, the pagination loop re-passes the same fields with the Link header's rel="next" target, and parseQuery concatenates instead of merging by key.

glab api --paginate -X GET -F 'ids=[1,2]' groups/x/issues

Page two goes out as ...?ids[]=1&ids[]=2&page=2&per_page=100&ids[]=1&ids[]=2, four values where the Link header asked for two, and each further page adds two more. A caller-supplied page survives too, as ?page=2&per_page=100&page=1, so the outcome depends on how the server resolves the repeat: a server taking the last occurrence answers page one again and the loop does not terminate. gitlab.com sorts duplicates instead, and there a stale -f 'page=5' won over the Link header's page two.

The fix follows the rel="next" target as given, which both the offset and keyset pagination docs ask for: "Be sure to use these links instead of generating your own URLs". Once the loop adopts a Link URL it stops reapplying fields, and only for a query method: an inferred POST carries its fields in the body and has to keep sending them, which --paginate -f a=b projects reaches because validate runs before the method is inferred.

The GET/DELETE test that decides whether fields become a query is now isQueryMethod, shared by httpRequest, the pagination loop and warnOnLegacyRawArrays. GraphQL pagination is unaffected: its path never becomes absolute and its cursor travels in the fields map.

!3776 changes field parsing in the same part of http.go, so the second to land resolves a small conflict and can fold its GET/DELETE check onto isQueryMethod.

Resolves #8537

How has this been tested?

  • Test_apiRun_paginationREST_followsLinkURLVerbatim covers five cases: an array field, the same with ndjson output, a scalar field, a caller-supplied page, and a field with no method flag. Each asserts the exact page-two and page-three request URIs against the Link header the mock served, plus method, body and stdout on all three.
  • Test_NewCmdApi/pagination with fields pins that validate accepts --paginate with fields and no method flag, and Test_httpRequest/DELETE with params pins isQueryMethod's DELETE arm.
  • Reverting api.go and http.go to main with the new test kept fails 4 of the 5 cases, as does deleting requestBody = nil; removing isQueryMethod(method) so the body is always dropped fails only the fifth.
  • go test ./internal/commands/api/... passes with 0 failures, make lint 0 issues. Also checked against gitlab.com with read-only GETs.

Screenshots (if appropriate):

Not applicable.

Edited by Andrew Dunn

Merge request reports

Loading
Loading