From 7efe0bde9dc363de57fc350dbcdb0b099bd1329a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 5 Sep 2024 16:05:38 -0400 Subject: [PATCH] Add `have_http_link_header` matcher and set header values as strings (#31010) --- .../concerns/account_controller_concern.rb | 2 +- app/controllers/concerns/api/pagination.rb | 2 +- app/controllers/statuses_controller.rb | 4 ++- .../account_controller_concern_spec.rb | 13 +++----- spec/requests/accounts_spec.rb | 3 +- spec/requests/api/v1/timelines/home_spec.rb | 5 +-- spec/requests/api/v1/timelines/list_spec.rb | 5 +-- spec/requests/link_headers_spec.rb | 24 +++----------- spec/support/matchers/api_pagination.rb | 2 +- spec/support/matchers/http_link_header.rb | 33 +++++++++++++++++++ 10 files changed, 54 insertions(+), 39 deletions(-) create mode 100644 spec/support/matchers/http_link_header.rb diff --git a/app/controllers/concerns/account_controller_concern.rb b/app/controllers/concerns/account_controller_concern.rb index d63bcc85c9..b75f3e3581 100644 --- a/app/controllers/concerns/account_controller_concern.rb +++ b/app/controllers/concerns/account_controller_concern.rb @@ -20,7 +20,7 @@ module AccountControllerConcern webfinger_account_link, actor_url_link, ] - ) + ).to_s end def webfinger_account_link diff --git a/app/controllers/concerns/api/pagination.rb b/app/controllers/concerns/api/pagination.rb index 7f06dc0202..b0b4ae4603 100644 --- a/app/controllers/concerns/api/pagination.rb +++ b/app/controllers/concerns/api/pagination.rb @@ -19,7 +19,7 @@ module Api::Pagination links = [] links << [next_path, [%w(rel next)]] if next_path links << [prev_path, [%w(rel prev)]] if prev_path - response.headers['Link'] = LinkHeader.new(links) unless links.empty? + response.headers['Link'] = LinkHeader.new(links).to_s unless links.empty? end def require_valid_pagination_options! diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index db7eddd78b..a0885b469b 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -56,7 +56,9 @@ class StatusesController < ApplicationController end def set_link_headers - response.headers['Link'] = LinkHeader.new([[ActivityPub::TagManager.instance.uri_for(@status), [%w(rel alternate), %w(type application/activity+json)]]]) + response.headers['Link'] = LinkHeader.new( + [[ActivityPub::TagManager.instance.uri_for(@status), [%w(rel alternate), %w(type application/activity+json)]]] + ).to_s end def set_status diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index 3eee46d7b9..384406a0ea 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -54,17 +54,12 @@ RSpec.describe AccountControllerConcern do it 'Prepares the account, returns success, and sets link headers' do get 'success', params: { account_username: account.username } - expect(response).to have_http_status(200) - expect(response.headers['Link'].to_s).to eq(expected_link_headers) + expect(response) + .to have_http_status(200) + .and have_http_link_header('http://test.host/.well-known/webfinger?resource=acct%3Ausername%40cb6e6126.ngrok.io').for(rel: 'lrdd', type: 'application/jrd+json') + .and have_http_link_header('https://cb6e6126.ngrok.io/users/username').for(rel: 'alternate', type: 'application/activity+json') expect(response.body) .to include(account.username) end - - def expected_link_headers - [ - '; rel="lrdd"; type="application/jrd+json"', - '; rel="alternate"; type="application/activity+json"', - ].join(', ') - end end end diff --git a/spec/requests/accounts_spec.rb b/spec/requests/accounts_spec.rb index d53816eff0..3615868d74 100644 --- a/spec/requests/accounts_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -69,8 +69,7 @@ RSpec.describe 'Accounts show response' do expect(response) .to have_http_status(200) .and render_template(:show) - - expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) + .and have_http_link_header(ActivityPub::TagManager.instance.uri_for(account)).for(rel: 'alternate') end end diff --git a/spec/requests/api/v1/timelines/home_spec.rb b/spec/requests/api/v1/timelines/home_spec.rb index d158e0801c..9dd102fcb6 100644 --- a/spec/requests/api/v1/timelines/home_spec.rb +++ b/spec/requests/api/v1/timelines/home_spec.rb @@ -94,8 +94,9 @@ RSpec.describe 'Home', :inline_jobs do it 'returns http unprocessable entity', :aggregate_failures do subject - expect(response).to have_http_status(422) - expect(response.headers['Link']).to be_nil + expect(response) + .to have_http_status(422) + .and not_have_http_link_header end end end diff --git a/spec/requests/api/v1/timelines/list_spec.rb b/spec/requests/api/v1/timelines/list_spec.rb index 753c784866..eb4395d1f9 100644 --- a/spec/requests/api/v1/timelines/list_spec.rb +++ b/spec/requests/api/v1/timelines/list_spec.rb @@ -47,8 +47,9 @@ RSpec.describe 'API V1 Timelines List' do it 'returns http unprocessable entity' do get "/api/v1/timelines/list/#{list.id}", headers: headers - expect(response).to have_http_status(422) - expect(response.headers['Link']).to be_nil + expect(response) + .to have_http_status(422) + .and not_have_http_link_header end end end diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb index 3116a54d6a..e20f5e79e5 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -6,28 +6,12 @@ RSpec.describe 'Link headers' do describe 'on the account show page' do let(:account) { Fabricate(:account, username: 'test') } - before do + it 'contains webfinger and activitypub urls in link header' do get short_account_path(username: account) - end - it 'contains webfinger url in link header' do - link_header = link_header_with_type('application/jrd+json') - - expect(link_header.href).to eq 'https://cb6e6126.ngrok.io/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io' - expect(link_header.attr_pairs.first).to eq %w(rel lrdd) - end - - it 'contains activitypub url in link header' do - link_header = link_header_with_type('application/activity+json') - - expect(link_header.href).to eq 'https://cb6e6126.ngrok.io/users/test' - expect(link_header.attr_pairs.first).to eq %w(rel alternate) - end - - def link_header_with_type(type) - LinkHeader.parse(response.headers['Link'].to_s).links.find do |link| - link.attr_pairs.any?(['type', type]) - end + expect(response) + .to have_http_link_header('https://cb6e6126.ngrok.io/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io').for(rel: 'lrdd', type: 'application/jrd+json') + .and have_http_link_header('https://cb6e6126.ngrok.io/users/test').for(rel: 'alternate', type: 'application/activity+json') end end end diff --git a/spec/support/matchers/api_pagination.rb b/spec/support/matchers/api_pagination.rb index f7d552b242..1a4f53a84a 100644 --- a/spec/support/matchers/api_pagination.rb +++ b/spec/support/matchers/api_pagination.rb @@ -3,7 +3,7 @@ RSpec::Matchers.define :include_pagination_headers do |links| match do |response| links.map do |key, value| - response.headers['Link'].find_link(['rel', key.to_s]).href == value + expect(response).to have_http_link_header(value).for(rel: key.to_s) end.all? end diff --git a/spec/support/matchers/http_link_header.rb b/spec/support/matchers/http_link_header.rb new file mode 100644 index 0000000000..3e658071c9 --- /dev/null +++ b/spec/support/matchers/http_link_header.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :have_http_link_header do |href| + match do |response| + @response = response + + header_link&.href == href + end + + match_when_negated do |response| + response.headers['Link'].blank? + end + + chain :for do |attributes| + @attributes = attributes + end + + failure_message do |response| + "Expected `#{response.headers['Link']}` to include `href` value of `#{href}` for `#{@attributes}` but it did not." + end + + failure_message_when_negated do + "Expected response not to have a `Link` header but `#{response.headers['Link']}` is present." + end + + def header_link + LinkHeader + .parse(@response.headers['Link']) + .find_link(*@attributes.stringify_keys) + end +end + +RSpec::Matchers.define_negated_matcher :not_have_http_link_header, :have_http_link_header # Allow chaining