From 9b7b4d748fd0bbfb537f8e2024c4566d71fb0122 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Wed, 8 Nov 2023 07:37:29 -0500
Subject: [PATCH] Simplify request cache spec shared examples (#27673)

---
 spec/requests/cache_spec.rb | 108 +++++++++++-------------------------
 1 file changed, 32 insertions(+), 76 deletions(-)

diff --git a/spec/requests/cache_spec.rb b/spec/requests/cache_spec.rb
index 19232fce6..ce73d3af7 100644
--- a/spec/requests/cache_spec.rb
+++ b/spec/requests/cache_spec.rb
@@ -119,24 +119,26 @@ module TestEndpoints
 end
 
 describe 'Caching behavior' do
-  shared_examples 'cachable response' do
-    it 'does not set cookies' do
+  shared_examples 'cachable response' do |http_success: false|
+    it 'does not set cookies or set public cache control', :aggregate_failures do
       expect(response.cookies).to be_empty
-    end
 
-    it 'sets public cache control', :aggregate_failures do
       # expect(response.cache_control[:max_age]&.to_i).to be_positive
       expect(response.cache_control[:public]).to be_truthy
       expect(response.cache_control[:private]).to be_falsy
       expect(response.cache_control[:no_store]).to be_falsy
       expect(response.cache_control[:no_cache]).to be_falsy
+
+      expect(response).to have_http_status(200) if http_success
     end
   end
 
-  shared_examples 'non-cacheable response' do
+  shared_examples 'non-cacheable response' do |http_success: false|
     it 'sets private cache control' do
       expect(response.cache_control[:private]).to be_truthy
       expect(response.cache_control[:no_store]).to be_truthy
+
+      expect(response).to have_http_status(200) if http_success
     end
   end
 
@@ -149,7 +151,7 @@ describe 'Caching behavior' do
 
   shared_examples 'language-dependent' do
     it 'has a Vary on Accept-Language' do
-      expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('accept-language')
+      expect(response_vary_headers).to include('accept-language')
     end
   end
 
@@ -202,7 +204,7 @@ describe 'Caching behavior' do
         it_behaves_like 'cachable response'
 
         it 'has a Vary on Cookie' do
-          expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('cookie')
+          expect(response_vary_headers).to include('cookie')
         end
 
         it_behaves_like 'language-dependent' if TestEndpoints::LANGUAGE_DEPENDENT.include?(endpoint)
@@ -216,7 +218,7 @@ describe 'Caching behavior' do
         it_behaves_like 'cachable response'
 
         it 'has a Vary on Authorization' do
-          expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('authorization')
+          expect(response_vary_headers).to include('authorization')
         end
 
         it_behaves_like 'language-dependent' if TestEndpoints::LANGUAGE_DEPENDENT.include?(endpoint)
@@ -302,7 +304,7 @@ describe 'Caching behavior' do
         it_behaves_like 'non-cacheable response'
 
         it 'has a Vary on Cookie' do
-          expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('cookie')
+          expect(response_vary_headers).to include('cookie')
         end
       end
     end
@@ -311,11 +313,7 @@ describe 'Caching behavior' do
       describe endpoint do
         before { get endpoint }
 
-        it_behaves_like 'non-cacheable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'non-cacheable response', http_success: true
       end
     end
 
@@ -351,7 +349,7 @@ describe 'Caching behavior' do
         it_behaves_like 'non-cacheable response'
 
         it 'has a Vary on Authorization' do
-          expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('authorization')
+          expect(response_vary_headers).to include('authorization')
         end
       end
     end
@@ -362,11 +360,7 @@ describe 'Caching behavior' do
           get endpoint, headers: { 'Authorization' => "Bearer #{token.token}" }
         end
 
-        it_behaves_like 'non-cacheable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'non-cacheable response', http_success: true
       end
     end
 
@@ -393,11 +387,7 @@ describe 'Caching behavior' do
       context 'when allowed for local users only' do
         let(:show_domain_blocks) { 'users' }
 
-        it_behaves_like 'non-cacheable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'non-cacheable response', http_success: true
       end
 
       context 'when disabled' do
@@ -421,11 +411,7 @@ describe 'Caching behavior' do
         get '/actor', sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
       end
 
-      it_behaves_like 'cachable response'
-
-      it 'returns HTTP success' do
-        expect(response).to have_http_status(200)
-      end
+      it_behaves_like 'cachable response', http_success: true
     end
 
     TestEndpoints::REQUIRE_SIGNATURE.each do |endpoint|
@@ -434,11 +420,7 @@ describe 'Caching behavior' do
           get endpoint, sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'non-cacheable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'non-cacheable response', http_success: true
       end
     end
   end
@@ -456,11 +438,7 @@ describe 'Caching behavior' do
           get '/actor', headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'cachable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'cachable response', http_success: true
       end
 
       (TestEndpoints::REQUIRE_SIGNATURE + TestEndpoints::AuthorizedFetch::REQUIRE_SIGNATURE).each do |endpoint|
@@ -487,11 +465,7 @@ describe 'Caching behavior' do
           get '/actor', sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'cachable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'cachable response', http_success: true
       end
 
       (TestEndpoints::REQUIRE_SIGNATURE + TestEndpoints::AuthorizedFetch::REQUIRE_SIGNATURE).each do |endpoint|
@@ -500,11 +474,7 @@ describe 'Caching behavior' do
             get endpoint, sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
           end
 
-          it_behaves_like 'non-cacheable response'
-
-          it 'returns HTTP success' do
-            expect(response).to have_http_status(200)
-          end
+          it_behaves_like 'non-cacheable response', http_success: true
         end
       end
     end
@@ -528,11 +498,7 @@ describe 'Caching behavior' do
           get '/actor', headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'cachable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'cachable response', http_success: true
       end
 
       (TestEndpoints::REQUIRE_SIGNATURE + TestEndpoints::AuthorizedFetch::REQUIRE_SIGNATURE).each do |endpoint|
@@ -560,11 +526,7 @@ describe 'Caching behavior' do
           get '/actor', sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'cachable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'cachable response', http_success: true
       end
 
       (TestEndpoints::REQUIRE_SIGNATURE + TestEndpoints::AuthorizedFetch::REQUIRE_SIGNATURE).each do |endpoint|
@@ -573,11 +535,7 @@ describe 'Caching behavior' do
             get endpoint, sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
           end
 
-          it_behaves_like 'non-cacheable response'
-
-          it 'returns HTTP success' do
-            expect(response).to have_http_status(200)
-          end
+          it_behaves_like 'non-cacheable response', http_success: true
         end
       end
     end
@@ -591,11 +549,7 @@ describe 'Caching behavior' do
           get '/actor', sign_with: remote_actor, headers: { 'Accept' => 'application/activity+json' }
         end
 
-        it_behaves_like 'cachable response'
-
-        it 'returns HTTP success' do
-          expect(response).to have_http_status(200)
-        end
+        it_behaves_like 'cachable response', http_success: true
       end
 
       (TestEndpoints::REQUIRE_SIGNATURE + TestEndpoints::AuthorizedFetch::REQUIRE_SIGNATURE).each do |endpoint|
@@ -667,7 +621,7 @@ describe 'Caching behavior' do
           it_behaves_like 'non-cacheable response'
 
           it 'has a Vary on Authorization' do
-            expect(response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }).to include('authorization')
+            expect(response_vary_headers).to include('authorization')
           end
         end
       end
@@ -678,13 +632,15 @@ describe 'Caching behavior' do
             get endpoint, headers: { 'Authorization' => "Bearer #{token.token}" }
           end
 
-          it_behaves_like 'non-cacheable response'
-
-          it 'returns HTTP success' do
-            expect(response).to have_http_status(200)
-          end
+          it_behaves_like 'non-cacheable response', http_success: true
         end
       end
     end
   end
+
+  private
+
+  def response_vary_headers
+    response.headers['Vary']&.split(',')&.map { |x| x.strip.downcase }
+  end
 end