From 711901d85a54fce31f370233eb519e5976faa333 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 11 Jul 2024 12:18:05 -0400 Subject: [PATCH 1/5] Add `have_http_link_header` matcher --- spec/support/matchers/http_link_header.rb | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 spec/support/matchers/http_link_header.rb diff --git a/spec/support/matchers/http_link_header.rb b/spec/support/matchers/http_link_header.rb new file mode 100644 index 00000000000..0676c8e08a4 --- /dev/null +++ b/spec/support/matchers/http_link_header.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :have_http_link_header do |rel, href| + chain :with_type do |type| + @type = type + end + + match do |response| + header_link = link_for(response, rel) + + header_link.href == href && + (@type.nil? || header_link.attrs['type'] == @type) + end + + match_when_negated do |response| + response.headers['Link'].blank? + end + + failure_message do |response| + (+'').tap do |string| + string << "Expected `#{response.headers['Link']}` to include `href` value of `#{href}` " + string << "with `type` of `#{@type}` " if @type.present? + string << "for `rel=#{rel}` but it did not." + end + end + + def link_for(response, rel) + LinkHeader + .parse(response.headers['Link']) + .find_link(['rel', rel.to_s]) + end +end + +RSpec::Matchers.define_negated_matcher :not_have_http_link_header, :have_http_link_header # Allow chaining From b4bb27d9754599d5775b133e209fce78d4bb8dcd Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 11 Jul 2024 12:18:23 -0400 Subject: [PATCH 2/5] Update pagination matcher to use link header matcher --- spec/support/matchers/api_pagination.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/support/matchers/api_pagination.rb b/spec/support/matchers/api_pagination.rb index f7d552b242a..10d3d0b835c 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(key, value) end.all? end From 71fa939640699e27d0f6c3ca579f792b47e5a997 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 11 Jul 2024 12:18:43 -0400 Subject: [PATCH 3/5] Assign the http link header string, not object --- app/controllers/concerns/account_controller_concern.rb | 2 +- app/controllers/concerns/api/pagination.rb | 2 +- app/controllers/statuses_controller.rb | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/concerns/account_controller_concern.rb b/app/controllers/concerns/account_controller_concern.rb index d63bcc85c95..b75f3e35819 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 7f06dc02023..b0b4ae4603f 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 db7eddd78b3..a0885b469b2 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 From f44e0a722d53f332bbeeca27087b0da4762c7e4d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 11 Jul 2024 12:21:15 -0400 Subject: [PATCH 4/5] Use http link header matcher --- .../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 ++++--------------- 5 files changed, 15 insertions(+), 35 deletions(-) diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index 6eb970dedbb..a6e8d162a6b 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -55,15 +55,10 @@ describe AccountControllerConcern do get 'success', params: { account_username: account.username } expect(assigns(:account)).to eq account - expect(response).to have_http_status(200) - expect(response.headers['Link'].to_s).to eq(expected_link_headers) - end - - def expected_link_headers - [ - '; rel="lrdd"; type="application/jrd+json"', - '; rel="alternate"; type="application/activity+json"', - ].join(', ') + expect(response) + .to have_http_status(200) + .and have_http_link_header(:lrdd, 'http://test.host/.well-known/webfinger?resource=acct%3Ausername%40cb6e6126.ngrok.io').with_type('application/jrd+json') + .and have_http_link_header(:alternate, 'https://cb6e6126.ngrok.io/users/username').with_type('application/activity+json') end end end diff --git a/spec/requests/accounts_spec.rb b/spec/requests/accounts_spec.rb index bf067cdc38a..31d913ee91c 100644 --- a/spec/requests/accounts_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -69,8 +69,7 @@ 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(:alternate, ActivityPub::TagManager.instance.uri_for(account)) end end diff --git a/spec/requests/api/v1/timelines/home_spec.rb b/spec/requests/api/v1/timelines/home_spec.rb index 96bd153affe..29de79057ee 100644 --- a/spec/requests/api/v1/timelines/home_spec.rb +++ b/spec/requests/api/v1/timelines/home_spec.rb @@ -94,8 +94,9 @@ 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 98d24567459..8987d6dbf25 100644 --- a/spec/requests/api/v1/timelines/list_spec.rb +++ b/spec/requests/api/v1/timelines/list_spec.rb @@ -47,8 +47,9 @@ 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 b822adbfb85..304955e9e08 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -6,28 +6,12 @@ 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 'http://www.example.com/.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(:lrdd, 'http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io').with_type('application/jrd+json') + .and have_http_link_header(:alternate, 'https://cb6e6126.ngrok.io/users/test').with_type('application/activity+json') end end end From 25baa364dea8ffeaa05a76b31d4019dd8e986a2c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 11 Jul 2024 15:41:39 -0400 Subject: [PATCH 5/5] Simplify matcher --- .../account_controller_concern_spec.rb | 4 +-- spec/requests/accounts_spec.rb | 2 +- spec/requests/link_headers_spec.rb | 4 +-- spec/support/matchers/api_pagination.rb | 2 +- spec/support/matchers/http_link_header.rb | 25 +++++++------------ 5 files changed, 15 insertions(+), 22 deletions(-) diff --git a/spec/controllers/concerns/account_controller_concern_spec.rb b/spec/controllers/concerns/account_controller_concern_spec.rb index a6e8d162a6b..e8e81f6b74f 100644 --- a/spec/controllers/concerns/account_controller_concern_spec.rb +++ b/spec/controllers/concerns/account_controller_concern_spec.rb @@ -57,8 +57,8 @@ describe AccountControllerConcern do expect(assigns(:account)).to eq account expect(response) .to have_http_status(200) - .and have_http_link_header(:lrdd, 'http://test.host/.well-known/webfinger?resource=acct%3Ausername%40cb6e6126.ngrok.io').with_type('application/jrd+json') - .and have_http_link_header(:alternate, 'https://cb6e6126.ngrok.io/users/username').with_type('application/activity+json') + .and have_http_link_header('http://test.host/.well-known/webfinger?resource=acct%3Ausername%40cb6e6126.ngrok.io', rel: 'lrdd', type: 'application/jrd+json') + .and have_http_link_header('https://cb6e6126.ngrok.io/users/username', rel: 'alternate', type: 'application/activity+json') end end end diff --git a/spec/requests/accounts_spec.rb b/spec/requests/accounts_spec.rb index 31d913ee91c..f734e21f738 100644 --- a/spec/requests/accounts_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -69,7 +69,7 @@ describe 'Accounts show response' do expect(response) .to have_http_status(200) .and render_template(:show) - .and have_http_link_header(:alternate, ActivityPub::TagManager.instance.uri_for(account)) + .and have_http_link_header(ActivityPub::TagManager.instance.uri_for(account), rel: 'alternate') end end diff --git a/spec/requests/link_headers_spec.rb b/spec/requests/link_headers_spec.rb index 304955e9e08..b8b5890a1de 100644 --- a/spec/requests/link_headers_spec.rb +++ b/spec/requests/link_headers_spec.rb @@ -10,8 +10,8 @@ describe 'Link headers' do get short_account_path(username: account) expect(response) - .to have_http_link_header(:lrdd, 'http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io').with_type('application/jrd+json') - .and have_http_link_header(:alternate, 'https://cb6e6126.ngrok.io/users/test').with_type('application/activity+json') + .to have_http_link_header('http://www.example.com/.well-known/webfinger?resource=acct%3Atest%40cb6e6126.ngrok.io', rel: 'lrdd', type: 'application/jrd+json') + .and have_http_link_header('https://cb6e6126.ngrok.io/users/test', 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 10d3d0b835c..4fc508db340 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| - expect(response).to have_http_link_header(key, value) + expect(response).to have_http_link_header(value, 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 index 0676c8e08a4..b48f38a7cd0 100644 --- a/spec/support/matchers/http_link_header.rb +++ b/spec/support/matchers/http_link_header.rb @@ -1,15 +1,8 @@ # frozen_string_literal: true -RSpec::Matchers.define :have_http_link_header do |rel, href| - chain :with_type do |type| - @type = type - end - +RSpec::Matchers.define :have_http_link_header do |href, **attrs| match do |response| - header_link = link_for(response, rel) - - header_link.href == href && - (@type.nil? || header_link.attrs['type'] == @type) + link_for(response, attrs)&.href == href end match_when_negated do |response| @@ -17,17 +10,17 @@ RSpec::Matchers.define :have_http_link_header do |rel, href| end failure_message do |response| - (+'').tap do |string| - string << "Expected `#{response.headers['Link']}` to include `href` value of `#{href}` " - string << "with `type` of `#{@type}` " if @type.present? - string << "for `rel=#{rel}` but it did not." - end + "Expected `#{response.headers['Link']}` to include `href` value of `#{href}` for `#{attrs}` but it did not." end - def link_for(response, rel) + failure_message_when_negated do + "Expected response not to have a `Link` header but `#{response.headers['Link']}` is present." + end + + def link_for(response, attrs) LinkHeader .parse(response.headers['Link']) - .find_link(['rel', rel.to_s]) + .find_link(*attrs.stringify_keys) end end