Commit 0901d4b2 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'fix/pagination-link' into 'master'

fix(api): include standard Link header in keyset pagination

Closes #218504

See merge request gitlab-org/gitlab!33714
parents c7b7b6d9 e8e3ee94
---
title: Fix pagination link header
merge_request: 33714
author: Max Wittig
type: fixed
...@@ -417,10 +417,14 @@ The response header includes a link to the next page. For example: ...@@ -417,10 +417,14 @@ The response header includes a link to the next page. For example:
HTTP/1.1 200 OK HTTP/1.1 200 OK
... ...
Links: <https://gitlab.example.com/api/v4/projects?pagination=keyset&per_page=50&order_by=id&sort=asc&id_after=42>; rel="next" Links: <https://gitlab.example.com/api/v4/projects?pagination=keyset&per_page=50&order_by=id&sort=asc&id_after=42>; rel="next"
Link: <https://gitlab.example.com/api/v4/projects?pagination=keyset&per_page=50&order_by=id&sort=asc&id_after=42>; rel="next"
Status: 200 OK 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. 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. Note the type of filter depends on the `order_by` option used and we may have more than one additional filter.
......
...@@ -24,7 +24,9 @@ module Gitlab ...@@ -24,7 +24,9 @@ module Gitlab
end end
def apply_headers(next_page) 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 end
private private
......
...@@ -60,9 +60,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -60,9 +60,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do
it 'sets Links header with same host/path as the original request' do it 'sets Links header with same host/path as the original request' do
orig_uri = URI.parse(request_context.request.url) orig_uri = URI.parse(request_context.request.url)
expect(request_context).to receive(:header) do |name, header| expect(request_context).to receive(:header).twice do |name, header|
expect(name).to eq('Links')
first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures
uri = URI.parse(first_link) uri = URI.parse(first_link)
...@@ -77,9 +75,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -77,9 +75,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do
it 'sets Links header with a link to the next page' do it 'sets Links header with a link to the next page' do
orig_uri = URI.parse(request_context.request.url) orig_uri = URI.parse(request_context.request.url)
expect(request_context).to receive(:header) do |name, header| expect(request_context).to receive(:header).twice do |name, header|
expect(name).to eq('Links')
first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures
query = CGI.parse(URI.parse(first_link).query) query = CGI.parse(URI.parse(first_link).query)
...@@ -97,9 +93,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -97,9 +93,7 @@ describe Gitlab::Pagination::Keyset::RequestContext do
it 'sets Links header with a link to the next page' do it 'sets Links header with a link to the next page' do
orig_uri = URI.parse(request_context.request.url) orig_uri = URI.parse(request_context.request.url)
expect(request_context).to receive(:header) do |name, header| expect(request_context).to receive(:header).twice do |name, header|
expect(name).to eq('Links')
first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures
query = CGI.parse(URI.parse(first_link).query) query = CGI.parse(URI.parse(first_link).query)
......
...@@ -597,6 +597,10 @@ describe API::Projects do ...@@ -597,6 +597,10 @@ describe API::Projects do
expect(response.header).to include('Links') expect(response.header).to include('Links')
expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include('pagination=keyset')
expect(response.header['Links']).to include("id_after=#{public_project.id}") 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 end
it 'contains only the first project with per_page = 1' do it 'contains only the first project with per_page = 1' do
...@@ -613,12 +617,17 @@ describe API::Projects do ...@@ -613,12 +617,17 @@ describe API::Projects do
expect(response.header).to include('Links') expect(response.header).to include('Links')
expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include('pagination=keyset')
expect(response.header['Links']).to include("id_after=#{project3.id}") 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 end
it 'does not include a next link when the page does not have any records' do 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)) 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('Links')
expect(response.header).not_to include('Link')
end end
it 'returns an empty array when the page does not have any records' do it 'returns an empty array when the page does not have any records' do
...@@ -644,6 +653,10 @@ describe API::Projects do ...@@ -644,6 +653,10 @@ describe API::Projects do
expect(response.header).to include('Links') expect(response.header).to include('Links')
expect(response.header['Links']).to include('pagination=keyset') expect(response.header['Links']).to include('pagination=keyset')
expect(response.header['Links']).to include("id_before=#{project3.id}") 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 end
it 'contains only the last project with per_page = 1' do it 'contains only the last project with per_page = 1' do
...@@ -672,6 +685,11 @@ describe API::Projects do ...@@ -672,6 +685,11 @@ describe API::Projects do
match[1] match[1]
end 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'] } ids += Gitlab::Json.parse(response.body).map { |p| p['id'] }
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment