diff --git a/changelogs/unreleased/fix-pagination-link.yml b/changelogs/unreleased/fix-pagination-link.yml new file mode 100644 index 0000000000000000000000000000000000000000..d2c1fc1eb945dcfab6231f7199376b9cf9307767 --- /dev/null +++ b/changelogs/unreleased/fix-pagination-link.yml @@ -0,0 +1,5 @@ +--- +title: Fix pagination link header +merge_request: 33714 +author: Max Wittig +type: fixed diff --git a/doc/api/README.md b/doc/api/README.md index 6afed960583e0ed093607e9e0577fa45a2b89a90..6cbb99a76cb163ec20be81ed384cc10f1b4d54b2 100644 --- a/doc/api/README.md +++ b/doc/api/README.md @@ -417,10 +417,14 @@ The response header includes a link to the next page. For example: HTTP/1.1 200 OK ... Links: ; rel="next" +Link: ; rel="next" Status: 200 OK ... ``` +CAUTION: **Deprecation:** +The `Links` Header will be removed in GitLab 14.0 to be aligned with the [W3C specification](https://www.w3.org/wiki/LinkHeader) + The link to the next page contains an additional filter `id_after=42` which excludes records we have retrieved already. Note the type of filter depends on the `order_by` option used and we may have more than one additional filter. diff --git a/lib/gitlab/pagination/keyset/request_context.rb b/lib/gitlab/pagination/keyset/request_context.rb index 8c8138b307647a20602dfb0ac8b75597e2ff793c..070fa844347c8937351335b2fc9525c43613ac13 100644 --- a/lib/gitlab/pagination/keyset/request_context.rb +++ b/lib/gitlab/pagination/keyset/request_context.rb @@ -24,7 +24,9 @@ def page end def apply_headers(next_page) - request.header('Links', pagination_links(next_page)) + link = pagination_links(next_page) + request.header('Links', link) + request.header('Link', link) end private diff --git a/spec/lib/gitlab/pagination/keyset/request_context_spec.rb b/spec/lib/gitlab/pagination/keyset/request_context_spec.rb index 6cd5ccc3c196b7e0afbaa3980ad440c4f1028101..d6d5340f38b4f50e5e888219c95f9cabab84c12b 100644 --- a/spec/lib/gitlab/pagination/keyset/request_context_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/request_context_spec.rb @@ -60,9 +60,7 @@ it 'sets Links header with same host/path as the original request' do orig_uri = URI.parse(request_context.request.url) - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - + expect(request_context).to receive(:header).twice do |name, header| first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures uri = URI.parse(first_link) @@ -77,9 +75,7 @@ it 'sets Links header with a link to the next page' do orig_uri = URI.parse(request_context.request.url) - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - + expect(request_context).to receive(:header).twice do |name, header| first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures query = CGI.parse(URI.parse(first_link).query) @@ -97,9 +93,7 @@ it 'sets Links header with a link to the next page' do orig_uri = URI.parse(request_context.request.url) - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - + expect(request_context).to receive(:header).twice do |name, header| first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures query = CGI.parse(URI.parse(first_link).query) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 8a1c68916e41dea5567ebb8afacaa1a84e5d7900..6f6737864b6f89a70a0961df9e25ece89415cf46 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -597,6 +597,10 @@ expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_after=#{public_project.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_after=#{public_project.id}") end it 'contains only the first project with per_page = 1' do @@ -613,12 +617,17 @@ expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_after=#{project3.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_after=#{project3.id}") end it 'does not include a next link when the page does not have any records' do get api('/projects', current_user), params: params.merge(id_after: Project.maximum(:id)) expect(response.header).not_to include('Links') + expect(response.header).not_to include('Link') end it 'returns an empty array when the page does not have any records' do @@ -644,6 +653,10 @@ expect(response.header).to include('Links') expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include("id_before=#{project3.id}") + + expect(response.header).to include('Link') + expect(response.header['Link']).to include('pagination=keyset') + expect(response.header['Link']).to include("id_before=#{project3.id}") end it 'contains only the last project with per_page = 1' do @@ -672,6 +685,11 @@ match[1] end + link = response.header['Link'] + url = link&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match| + match[1] + end + ids += Gitlab::Json.parse(response.body).map { |p| p['id'] } end