From e1fa456c7cfa5dc41cec21a94573bc6cb1ec60cc Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 3 Sep 2024 11:35:19 -0400 Subject: [PATCH] Add `have_cacheable_headers` matcher for responses (#31727) --- .../collections_controller_spec.rb | 13 ++--- .../activitypub/outboxes_controller_spec.rb | 14 +++--- .../activitypub/replies_controller_spec.rb | 7 +-- spec/controllers/statuses_controller_spec.rb | 9 ++-- spec/requests/accounts_spec.rb | 32 ++++++------ spec/requests/custom_stylesheets_spec.rb | 3 +- spec/requests/instance_actor_spec.rb | 3 +- spec/requests/manifest_spec.rb | 3 +- spec/requests/tags_spec.rb | 13 ++--- spec/support/examples/cache.rb | 14 ------ spec/support/matchers/cacheable_response.rb | 50 +++++++++++++++++++ 11 files changed, 96 insertions(+), 65 deletions(-) delete mode 100644 spec/support/examples/cache.rb create mode 100644 spec/support/matchers/cacheable_response.rb diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/controllers/activitypub/collections_controller_spec.rb index a5718fbd7d..0880273853 100644 --- a/spec/controllers/activitypub/collections_controller_spec.rb +++ b/spec/controllers/activitypub/collections_controller_spec.rb @@ -25,10 +25,10 @@ RSpec.describe ActivityPub::CollectionsController do context 'without signature' do let(:remote_account) { nil } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and correct items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers expect(response.media_type).to eq 'application/activity+json' expect(body_as_json[:orderedItems]) @@ -64,10 +64,11 @@ RSpec.describe ActivityPub::CollectionsController do let(:remote_account) { Fabricate(:account, domain: 'example.com') } context 'when getting a featured resource' do - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and expected items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(body_as_json[:orderedItems]) diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/controllers/activitypub/outboxes_controller_spec.rb index 3c8e8e399f..26a52bad93 100644 --- a/spec/controllers/activitypub/outboxes_controller_spec.rb +++ b/spec/controllers/activitypub/outboxes_controller_spec.rb @@ -25,10 +25,11 @@ RSpec.describe ActivityPub::OutboxesController do context 'with page not requested' do let(:page) { nil } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and headers and items count' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(response.headers['Vary']).to be_nil expect(body[:totalItems]).to eq 4 @@ -59,10 +60,11 @@ RSpec.describe ActivityPub::OutboxesController do context 'with page requested' do let(:page) { 'true' } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type and vary header and items' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' expect(response.headers['Vary']).to include 'Signature' diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/controllers/activitypub/replies_controller_spec.rb index c556e07270..c10c782c9a 100644 --- a/spec/controllers/activitypub/replies_controller_spec.rb +++ b/spec/controllers/activitypub/replies_controller_spec.rb @@ -68,10 +68,11 @@ RSpec.describe ActivityPub::RepliesController do let(:parent_visibility) { :public } let(:page_json) { body_as_json[:first] } - it_behaves_like 'cacheable response' - it 'returns http success and correct media type' do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers + expect(response.media_type).to eq 'application/activity+json' end diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index fe40ee6de1..084dcfaa75 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -72,13 +72,12 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'renders ActivityPub Note object successfully', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + expect(response.headers).to include( - 'Vary' => 'Accept, Accept-Language, Cookie', 'Content-Type' => include('application/activity+json'), 'Link' => satisfy { |header| header.to_s.include?('activity+json') } ) @@ -380,13 +379,11 @@ describe StatusesController do context 'with JSON' do let(:format) { 'json' } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'renders ActivityPub Note object successfully', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.headers).to include( - 'Vary' => 'Accept, Accept-Language, Cookie', 'Content-Type' => include('application/activity+json'), 'Link' => satisfy { |header| header.to_s.include?('activity+json') } ) diff --git a/spec/requests/accounts_spec.rb b/spec/requests/accounts_spec.rb index bf067cdc38..238524c75c 100644 --- a/spec/requests/accounts_spec.rb +++ b/spec/requests/accounts_spec.rb @@ -130,6 +130,7 @@ describe 'Accounts show response' do it 'returns a JSON version of the account', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') .and have_attributes( media_type: eq('application/activity+json') ) @@ -137,8 +138,6 @@ describe 'Accounts show response' do expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - context 'with authorized fetch mode' do let(:authorized_fetch_mode) { true } @@ -179,6 +178,7 @@ describe 'Accounts show response' do it 'returns a JSON version of the account', :aggregate_failures do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') .and have_attributes( media_type: eq('application/activity+json') ) @@ -186,8 +186,6 @@ describe 'Accounts show response' do expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - context 'with authorized fetch mode' do let(:authorized_fetch_mode) { true } @@ -215,10 +213,10 @@ describe 'Accounts show response' do get short_account_path(username: account.username, format: format) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'responds with correct statuses', :aggregate_failures do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.body).to include(status_tag_for(status_media)) expect(response.body).to include(status_tag_for(status_self_reply)) expect(response.body).to include(status_tag_for(status)) @@ -234,10 +232,11 @@ describe 'Accounts show response' do get short_account_with_replies_path(username: account.username, format: format) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'responds with correct statuses with replies', :aggregate_failures do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + expect(response.body).to include(status_tag_for(status_media)) expect(response.body).to include(status_tag_for(status_reply)) expect(response.body).to include(status_tag_for(status_self_reply)) @@ -253,10 +252,10 @@ describe 'Accounts show response' do get short_account_media_path(username: account.username, format: format) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'responds with correct statuses with media', :aggregate_failures do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.body).to include(status_tag_for(status_media)) expect(response.body).to_not include(status_tag_for(status_direct)) expect(response.body).to_not include(status_tag_for(status_private)) @@ -277,10 +276,11 @@ describe 'Accounts show response' do get short_account_tag_path(username: account.username, tag: tag, format: format) end - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - it 'responds with correct statuses with a tag', :aggregate_failures do - expect(response).to have_http_status(200) + expect(response) + .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') + expect(response.body).to include(status_tag_for(status_tag)) expect(response.body).to_not include(status_tag_for(status_direct)) expect(response.body).to_not include(status_tag_for(status_media)) diff --git a/spec/requests/custom_stylesheets_spec.rb b/spec/requests/custom_stylesheets_spec.rb index 982511297b..128d173f3a 100644 --- a/spec/requests/custom_stylesheets_spec.rb +++ b/spec/requests/custom_stylesheets_spec.rb @@ -9,11 +9,10 @@ describe 'Custom stylesheets' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers .and have_attributes( content_type: match('text/css') ) end - - it_behaves_like 'cacheable response' end end diff --git a/spec/requests/instance_actor_spec.rb b/spec/requests/instance_actor_spec.rb index 9c7ee9ff90..7e4784203f 100644 --- a/spec/requests/instance_actor_spec.rb +++ b/spec/requests/instance_actor_spec.rb @@ -17,6 +17,7 @@ RSpec.describe 'Instance actor endpoint' do it 'returns http success with correct media type and body' do expect(response) .to have_http_status(200) + .and have_cacheable_headers expect(response.content_type) .to start_with('application/activity+json') expect(body_as_json) @@ -32,8 +33,6 @@ RSpec.describe 'Instance actor endpoint' do url: about_more_url(instance_actor: true) ) end - - it_behaves_like 'cacheable response' end context 'with limited federation mode disabled' do diff --git a/spec/requests/manifest_spec.rb b/spec/requests/manifest_spec.rb index 749c4038df..55b8147d7e 100644 --- a/spec/requests/manifest_spec.rb +++ b/spec/requests/manifest_spec.rb @@ -9,6 +9,7 @@ describe 'Manifest' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers .and have_attributes( content_type: match('application/json') ) @@ -18,7 +19,5 @@ describe 'Manifest' do name: 'Mastodon' ) end - - it_behaves_like 'cacheable response' end end diff --git a/spec/requests/tags_spec.rb b/spec/requests/tags_spec.rb index c037bbef5a..7974a6b152 100644 --- a/spec/requests/tags_spec.rb +++ b/spec/requests/tags_spec.rb @@ -8,16 +8,15 @@ RSpec.describe 'Tags' do let(:tag) { Fabricate :tag } context 'with HTML format' do - # TODO: Convert the cacheable response shared example into a matcher, - # remove this example, rely on system spec (which should use matcher) + # TODO: Update the have_cacheable_headers matcher to operate on capybara sessions + # Remove this example, rely on system spec (which should use matcher) before { get tag_path(tag) } it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end context 'with JSON format' do @@ -26,11 +25,10 @@ RSpec.describe 'Tags' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.content_type) .to start_with('application/activity+json') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end context 'with RSS format' do @@ -39,11 +37,10 @@ RSpec.describe 'Tags' do it 'returns http success' do expect(response) .to have_http_status(200) + .and have_cacheable_headers.with_vary('Accept, Accept-Language, Cookie') expect(response.content_type) .to start_with('application/rss+xml') end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' end end diff --git a/spec/support/examples/cache.rb b/spec/support/examples/cache.rb deleted file mode 100644 index 60e522f426..0000000000 --- a/spec/support/examples/cache.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -shared_examples 'cacheable response' do |expects_vary: false| - it 'sets correct cache and vary headers and does not set cookies or session', :aggregate_failures do - expect(response.cookies).to be_empty - expect(response.headers['Set-Cookies']).to be_nil - - expect(session).to be_empty - - expect(response.headers['Vary']).to include(expects_vary) if expects_vary - - expect(response.headers['Cache-Control']).to include('public') - end -end diff --git a/spec/support/matchers/cacheable_response.rb b/spec/support/matchers/cacheable_response.rb new file mode 100644 index 0000000000..da8570c8c5 --- /dev/null +++ b/spec/support/matchers/cacheable_response.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +RSpec::Matchers.define :have_cacheable_headers do + match do |response| + @response = response + + @errors = [].tap do |errors| + errors << check_cookies + errors << check_cookie_headers + errors << check_session + errors << check_cache_control + errors << check_vary if @expected_vary.present? + end + + @errors.compact.empty? + end + + chain :with_vary do |string| + @expected_vary = string + end + + failure_message do + <<~ERROR + Expected that the response would be cacheable but it was not: + - #{@errors.compact.join("\n - ")} + ERROR + end + + def check_vary + puts @expected_vary + pp @response.headers + "Response `Vary` header does not contain `#{@expected_vary}`" unless @response.headers['Vary'].include?(@expected_vary) + end + + def check_cookies + 'Reponse cookies are present' unless @response.cookies.empty? + end + + def check_cookie_headers + 'Response `Set-Cookies` headers are present' if @response.headers['Set-Cookies'].present? + end + + def check_session + 'The session is not empty' unless session.empty? + end + + def check_cache_control + 'The `Cache-Control` header does not contain `public`' unless @response.headers['Cache-Control'].include?('public') + end +end