From e8798a9e8444ca9ac6993ae7a392942326ee0793 Mon Sep 17 00:00:00 2001
From: Michael Becker <11881043-wandering_person@users.noreply.gitlab.com>
Date: Fri, 12 May 2023 13:24:43 +0700
Subject: [PATCH 1/2] Fix edges in KeysetPaginationHelpers spec helper and add
specs
This is a followup change from a comment on [MR 116344][0]
We think it makes more sense to return an empty array from this helper
method rather than a `nil`
resolves: https://gitlab.com/gitlab-org/gitlab/-/issues/404981
[0]:https://gitlab.com/gitlab-org/gitlab/-/merge_requests/116344
---
.../helpers/keyset_pagination_helpers.rb | 7 +-
.../helpers/keyset_pagination_helpers_spec.rb | 84 +++++++++++++++++++
2 files changed, 89 insertions(+), 2 deletions(-)
create mode 100644 spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
diff --git a/spec/support/helpers/keyset_pagination_helpers.rb b/spec/support/helpers/keyset_pagination_helpers.rb
index 4bc20098e8cbe9..4a476c47fda46a 100644
--- a/spec/support/helpers/keyset_pagination_helpers.rb
+++ b/spec/support/helpers/keyset_pagination_helpers.rb
@@ -7,14 +7,17 @@ def pagination_links(response)
link.split(',').filter_map do |link|
match = link.match(/<(?<url>.*)>; rel="(?<rel>\w+)"/)
- break nil unless match
+ next unless match
{ url: match[:url], rel: match[:rel] }
end
end
def pagination_params_from_next_url(response)
- next_url = pagination_links(response).find { |link| link[:rel] == 'next' }[:url]
+ next_link = pagination_links(response).find { |link| link[:rel] == 'next' }
+ next_url = next_link&.fetch(:url)
+ return unless next_url
+
Rack::Utils.parse_query(URI.parse(next_url).query)
end
end
diff --git a/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb b/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
new file mode 100644
index 00000000000000..4dad087208b496
--- /dev/null
+++ b/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe KeysetPaginationHelpers, feature_category: :api do
+ include described_class
+
+ let(:headers) { { 'LINK' => "<#{url}>; rel=\"#{rel}\"" } }
+ let(:response) { instance_double('HTTParty::Response', headers: headers) }
+ let(:rel) { 'next' }
+ let(:url) do
+ 'http://127.0.0.1:3000/api/v4/projects/7/audit_eve' \
+ 'nts?cursor=eyJpZCI6IjYyMjAiLCJfa2QiOiJuIn0%3D&id=7&o' \
+ 'rder_by=id&page=1&pagination=keyset&per_page=2'
+ end
+
+ describe '#pagination_links' do
+ subject { pagination_links(response) }
+
+ let(:expected_result) { [{ url: url, rel: rel }] }
+
+ it { is_expected.to eq expected_result }
+
+ context 'with a partially malformed LINK header' do
+ let(:headers) do
+ { 'LINK' => "<#{url}>; rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" }
+ end
+
+ it { is_expected.to eq expected_result }
+ end
+
+ context 'with a malformed LINK header' do
+ let(:headers) { { 'LINK' => "rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" } }
+ let(:expected_result) { [] }
+
+ it { is_expected.to eq expected_result }
+ end
+ end
+
+ describe '#pagination_params_from_next_url' do
+ subject { pagination_params_from_next_url(response) }
+
+ let(:expected_result) do
+ {
+ 'cursor' => 'eyJpZCI6IjYyMjAiLCJfa2QiOiJuIn0=',
+ 'id' => '7',
+ 'order_by' => 'id',
+ 'page' => '1',
+ 'pagination' => 'keyset',
+ 'per_page' => '2'
+ }
+ end
+
+ it { is_expected.to eq expected_result }
+
+ context 'with both prev and next rel links' do
+ let(:prev_url) do
+ 'http://127.0.0.1:3000/api/v4/projects/7/audit_eve' \
+ 'nts?cursor=foocursor&id=8&o' \
+ 'rder_by=id&page=0&pagination=keyset&per_page=2'
+ end
+
+ let(:headers) do
+ { 'LINK' => "<#{url}>; rel=\"next\", <#{prev_url}>; rel=\"prev\"" }
+ end
+
+ it { is_expected.to eq expected_result }
+ end
+
+ context 'with a partially malformed LINK header' do
+ let(:headers) do
+ { 'LINK' => "<#{url}>; rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" }
+ end
+
+ it { is_expected.to eq expected_result }
+ end
+
+ context 'with a malformed LINK header' do
+ let(:headers) { { 'LINK' => "rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" } }
+
+ it { is_expected.to be nil }
+ end
+ end
+end
--
GitLab
From e78061aeebd5a9e827654a4959775258468cb6a0 Mon Sep 17 00:00:00 2001
From: Michael Becker <11881043-wandering_person@users.noreply.gitlab.com>
Date: Mon, 15 May 2023 14:39:35 +0700
Subject: [PATCH 2/2] MR Feedback
---
.../helpers/keyset_pagination_helpers_spec.rb | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb b/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
index 4dad087208b496..ec63f33776c708 100644
--- a/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
+++ b/spec/support_specs/helpers/keyset_pagination_helpers_spec.rb
@@ -5,7 +5,7 @@
RSpec.describe KeysetPaginationHelpers, feature_category: :api do
include described_class
- let(:headers) { { 'LINK' => "<#{url}>; rel=\"#{rel}\"" } }
+ let(:headers) { { 'LINK' => %(<#{url}>; rel="#{rel}") } }
let(:response) { instance_double('HTTParty::Response', headers: headers) }
let(:rel) { 'next' }
let(:url) do
@@ -22,15 +22,17 @@
it { is_expected.to eq expected_result }
context 'with a partially malformed LINK header' do
+ # malformed as the regxe is expecting the url to be surrounded by `<>`
let(:headers) do
- { 'LINK' => "<#{url}>; rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" }
+ { 'LINK' => %(<#{url}>; rel="next", GARBAGE, #{url}; rel="prev") }
end
it { is_expected.to eq expected_result }
end
context 'with a malformed LINK header' do
- let(:headers) { { 'LINK' => "rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" } }
+ # malformed as the regxe is expecting the url to be surrounded by `<>`
+ let(:headers) { { 'LINK' => %(rel="next", GARBAGE, #{url}; rel="prev") } }
let(:expected_result) { [] }
it { is_expected.to eq expected_result }
@@ -61,22 +63,24 @@
end
let(:headers) do
- { 'LINK' => "<#{url}>; rel=\"next\", <#{prev_url}>; rel=\"prev\"" }
+ { 'LINK' => %(<#{url}>; rel="next", <#{prev_url}>; rel="prev") }
end
it { is_expected.to eq expected_result }
end
context 'with a partially malformed LINK header' do
+ # malformed as the regxe is expecting the url to be surrounded by `<>`
let(:headers) do
- { 'LINK' => "<#{url}>; rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" }
+ { 'LINK' => %(<#{url}>; rel="next", GARBAGE, #{url}; rel="prev") }
end
it { is_expected.to eq expected_result }
end
context 'with a malformed LINK header' do
- let(:headers) { { 'LINK' => "rel=\"next\", GARBAGE, #{url}; rel=\"prev\"" } }
+ # malformed as the regxe is expecting the url to be surrounded by `<>`
+ let(:headers) { { 'LINK' => %(rel="next", GARBAGE, #{url}; rel="prev") } }
it { is_expected.to be nil }
end
--
GitLab