Add have_cacheable_headers matcher for responses (#31727)

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])

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'
 

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
 

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') }
             )

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))

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

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

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

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
 

spec/support/examples/cache.rb

@@ -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

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