Commit ab70437a authored by James Fargher's avatar James Fargher

Merge branch '220312-remove-links-header-from-keyset-based-pagination' into 'master'

Remove Links header

See merge request gitlab-org/gitlab!62388
parents 93afbe07 47203a78
...@@ -13,7 +13,6 @@ module Gitlab ...@@ -13,7 +13,6 @@ module Gitlab
def add_next_page_header(query_params) def add_next_page_header(query_params)
link = next_page_link(page_href(query_params)) link = next_page_link(page_href(query_params))
header('Links', link)
header('Link', link) header('Link', link)
end end
......
...@@ -108,7 +108,6 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do ...@@ -108,7 +108,6 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do
let(:expected_next_page_link) { %Q(<#{incoming_api_projects_url}?#{query.merge(page_token: branch2.name).to_query}>; rel="next") } let(:expected_next_page_link) { %Q(<#{incoming_api_projects_url}?#{query.merge(page_token: branch2.name).to_query}>; rel="next") }
it 'uses keyset pagination and adds link headers' do it 'uses keyset pagination and adds link headers' do
expect(request_context).to receive(:header).with('Links', expected_next_page_link)
expect(request_context).to receive(:header).with('Link', expected_next_page_link) expect(request_context).to receive(:header).with('Link', expected_next_page_link)
pager.paginate(finder) pager.paginate(finder)
...@@ -119,7 +118,6 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do ...@@ -119,7 +118,6 @@ RSpec.describe Gitlab::Pagination::GitalyKeysetPager do
let(:branches) { [branch1] } let(:branches) { [branch1] }
it 'uses keyset pagination without link headers' do it 'uses keyset pagination without link headers' do
expect(request_context).not_to receive(:header).with('Links', anything)
expect(request_context).not_to receive(:header).with('Link', anything) expect(request_context).not_to receive(:header).with('Link', anything)
pager.paginate(finder) pager.paginate(finder)
......
...@@ -57,14 +57,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -57,14 +57,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do
subject { described_class.new(request_context).apply_headers(next_page) } subject { described_class.new(request_context).apply_headers(next_page) }
it 'sets Links header with same host/path as the original request' do it 'sets Link 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).twice do |name, header| expect(request_context).to receive(:header).once do |name, header|
first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures
uri = URI.parse(first_link) uri = URI.parse(first_link)
expect(name).to eq('Link')
expect(uri.host).to eq(orig_uri.host) expect(uri.host).to eq(orig_uri.host)
expect(uri.path).to eq(orig_uri.path) expect(uri.path).to eq(orig_uri.path)
end end
...@@ -72,14 +73,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -72,14 +73,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do
subject subject
end end
it 'sets Links header with a link to the next page' do it 'sets Link 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).twice do |name, header| expect(request_context).to receive(:header).once do |name, header|
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)
expect(name).to eq('Link')
expect(query.except('id_after')).to eq(CGI.parse(orig_uri.query).except('id_after')) expect(query.except('id_after')).to eq(CGI.parse(orig_uri.query).except('id_after'))
expect(query['id_after']).to eq(['42']) expect(query['id_after']).to eq(['42'])
end end
...@@ -90,14 +92,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do ...@@ -90,14 +92,15 @@ RSpec.describe Gitlab::Pagination::Keyset::RequestContext do
context 'with descending order' do context 'with descending order' do
let(:next_page) { double('next page', order_by: { id: :desc }, lower_bounds: { id: 42 }) } let(:next_page) { double('next page', order_by: { id: :desc }, lower_bounds: { id: 42 }) }
it 'sets Links header with a link to the next page' do it 'sets Link 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).twice do |name, header| expect(request_context).to receive(:header).once do |name, header|
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)
expect(name).to eq('Link')
expect(query.except('id_before')).to eq(CGI.parse(orig_uri.query).except('id_before')) expect(query.except('id_before')).to eq(CGI.parse(orig_uri.query).except('id_before'))
expect(query['id_before']).to eq(['42']) expect(query['id_before']).to eq(['42'])
end end
......
...@@ -105,7 +105,7 @@ RSpec.describe API::Branches do ...@@ -105,7 +105,7 @@ RSpec.describe API::Branches do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response).to match_response_schema('public_api/v4/branches') expect(response).to match_response_schema('public_api/v4/branches')
expect(response.headers).not_to include('Link', 'Links') expect(response.headers).not_to include('Link')
branch_names = json_response.map { |x| x['name'] } branch_names = json_response.map { |x| x['name'] }
expect(branch_names).to match_array(project.repository.branch_names) expect(branch_names).to match_array(project.repository.branch_names)
end end
...@@ -116,7 +116,7 @@ RSpec.describe API::Branches do ...@@ -116,7 +116,7 @@ RSpec.describe API::Branches do
get api(route, current_user), params: base_params.merge(per_page: 2) get api(route, current_user), params: base_params.merge(per_page: 2)
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(response.headers).to include('Link', 'Links') expect(response.headers).to include('Link')
expect(json_response.count).to eq 2 expect(json_response.count).to eq 2
check_merge_status(json_response) check_merge_status(json_response)
......
...@@ -742,10 +742,6 @@ RSpec.describe API::Projects do ...@@ -742,10 +742,6 @@ RSpec.describe API::Projects do
it 'includes a pagination header with link to the next page' do it 'includes a pagination header with link to the next page' do
get api('/projects', current_user), params: params get api('/projects', current_user), params: params
expect(response.header).to include('Links')
expect(response.header['Links']).to include('pagination=keyset')
expect(response.header['Links']).to include("id_after=#{first_project_id}")
expect(response.header).to include('Link') expect(response.header).to include('Link')
expect(response.header['Link']).to include('pagination=keyset') expect(response.header['Link']).to include('pagination=keyset')
expect(response.header['Link']).to include("id_after=#{first_project_id}") expect(response.header['Link']).to include("id_after=#{first_project_id}")
...@@ -762,10 +758,6 @@ RSpec.describe API::Projects do ...@@ -762,10 +758,6 @@ RSpec.describe API::Projects do
it 'still includes a link if the end has reached and there is no more data after this page' do it 'still includes a link if the end has reached and there is no more data after this page' do
get api('/projects', current_user), params: params.merge(id_after: project2.id) get api('/projects', current_user), params: params.merge(id_after: project2.id)
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).to include('Link')
expect(response.header['Link']).to include('pagination=keyset') expect(response.header['Link']).to include('pagination=keyset')
expect(response.header['Link']).to include("id_after=#{project3.id}") expect(response.header['Link']).to include("id_after=#{project3.id}")
...@@ -774,7 +766,6 @@ RSpec.describe API::Projects do ...@@ -774,7 +766,6 @@ RSpec.describe API::Projects do
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('Link') expect(response.header).not_to include('Link')
end end
...@@ -798,10 +789,6 @@ RSpec.describe API::Projects do ...@@ -798,10 +789,6 @@ RSpec.describe API::Projects do
it 'includes a pagination header with link to the next page' do it 'includes a pagination header with link to the next page' do
get api('/projects', current_user), params: params get api('/projects', current_user), params: params
expect(response.header).to include('Links')
expect(response.header['Links']).to include('pagination=keyset')
expect(response.header['Links']).to include("id_before=#{last_project_id}")
expect(response.header).to include('Link') expect(response.header).to include('Link')
expect(response.header['Link']).to include('pagination=keyset') expect(response.header['Link']).to include('pagination=keyset')
expect(response.header['Link']).to include("id_before=#{last_project_id}") expect(response.header['Link']).to include("id_before=#{last_project_id}")
...@@ -828,11 +815,6 @@ RSpec.describe API::Projects do ...@@ -828,11 +815,6 @@ RSpec.describe API::Projects do
requests += 1 requests += 1
get api(url, current_user), params: params get api(url, current_user), params: params
links = response.header['Links']
url = links&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match|
match[1]
end
link = response.header['Link'] link = response.header['Link']
url = link&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match| url = link&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match|
match[1] match[1]
......
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