From ef879a532f37d110766693a49597257b38a814d3 Mon Sep 17 00:00:00 2001
From: Matt Jankowski <matt@jankowski.online>
Date: Thu, 27 Mar 2025 10:55:13 -0400
Subject: [PATCH] Convert `activitypub/*` controller specs to request specs
 (#33992)

---
 .../activitypub/collections_spec.rb}          |  86 +++++++------
 .../followers_synchronizations_spec.rb}       |  42 ++++---
 .../activitypub/outboxes_spec.rb}             | 118 +++++++++++-------
 .../activitypub/replies_spec.rb}              |  80 +++++++-----
 4 files changed, 197 insertions(+), 129 deletions(-)
 rename spec/{controllers/activitypub/collections_controller_spec.rb => requests/activitypub/collections_spec.rb} (59%)
 rename spec/{controllers/activitypub/followers_synchronizations_controller_spec.rb => requests/activitypub/followers_synchronizations_spec.rb} (68%)
 rename spec/{controllers/activitypub/outboxes_controller_spec.rb => requests/activitypub/outboxes_spec.rb} (63%)
 rename spec/{controllers/activitypub/replies_controller_spec.rb => requests/activitypub/replies_spec.rb} (78%)

diff --git a/spec/controllers/activitypub/collections_controller_spec.rb b/spec/requests/activitypub/collections_spec.rb
similarity index 59%
rename from spec/controllers/activitypub/collections_controller_spec.rb
rename to spec/requests/activitypub/collections_spec.rb
index 408e0dd2f6..d2761f98ea 100644
--- a/spec/controllers/activitypub/collections_controller_spec.rb
+++ b/spec/requests/activitypub/collections_spec.rb
@@ -2,22 +2,19 @@
 
 require 'rails_helper'
 
-RSpec.describe ActivityPub::CollectionsController do
+RSpec.describe 'ActivityPub Collections' do
   let!(:account) { Fabricate(:account) }
   let!(:private_pinned) { Fabricate(:status, account: account, text: 'secret private stuff', visibility: :private) }
   let(:remote_account) { nil }
 
   before do
-    allow(controller).to receive(:signed_request_actor).and_return(remote_account)
-
-    Fabricate(:status_pin, account: account)
-    Fabricate(:status_pin, account: account)
+    Fabricate.times(2, :status_pin, account: account)
     Fabricate(:status_pin, account: account, status: private_pinned)
     Fabricate(:status, account: account, visibility: :private)
   end
 
   describe 'GET #show' do
-    subject(:response) { get :show, params: { id: id, account_username: account.username } }
+    subject { get account_collection_path(id: id, account_username: account.username), headers: nil, sign_with: remote_account }
 
     context 'when id is "featured"' do
       let(:id) { 'featured' }
@@ -26,10 +23,13 @@ RSpec.describe ActivityPub::CollectionsController do
         let(:remote_account) { nil }
 
         it 'returns http success and correct media type and correct items' do
+          subject
+
           expect(response)
             .to have_http_status(200)
             .and have_cacheable_headers
-          expect(response.media_type).to eq 'application/activity+json'
+          expect(response.media_type)
+            .to eq 'application/activity+json'
 
           expect(response.parsed_body[:orderedItems])
             .to be_an(Array)
@@ -45,17 +45,21 @@ RSpec.describe ActivityPub::CollectionsController do
           end
 
           it 'returns http gone' do
-            expect(response).to have_http_status(410)
+            subject
+
+            expect(response)
+              .to have_http_status(410)
           end
         end
 
         context 'when account is temporarily suspended' do
-          before do
-            account.suspend!
-          end
+          before { account.suspend! }
 
           it 'returns http forbidden' do
-            expect(response).to have_http_status(403)
+            subject
+
+            expect(response)
+              .to have_http_status(403)
           end
         end
       end
@@ -65,11 +69,14 @@ RSpec.describe ActivityPub::CollectionsController do
 
         context 'when getting a featured resource' do
           it 'returns http success and correct media type and expected items' do
+            subject
+
             expect(response)
               .to have_http_status(200)
               .and have_cacheable_headers
 
-            expect(response.media_type).to eq 'application/activity+json'
+            expect(response.media_type)
+              .to eq 'application/activity+json'
 
             expect(response.parsed_body[:orderedItems])
               .to be_an(Array)
@@ -80,39 +87,45 @@ RSpec.describe ActivityPub::CollectionsController do
         end
 
         context 'with authorized fetch mode' do
-          before do
-            allow(controller).to receive(:authorized_fetch_mode?).and_return(true)
-          end
+          before { Setting.authorized_fetch = true }
 
           context 'when signed request account is blocked' do
-            before do
-              account.block!(remote_account)
-            end
+            before { account.block!(remote_account) }
 
             it 'returns http success and correct media type and cache headers and empty items' do
-              expect(response).to have_http_status(200)
-              expect(response.media_type).to eq 'application/activity+json'
-              expect(response.headers['Cache-Control']).to include 'private'
+              subject
 
-              expect(response.parsed_body[:orderedItems])
-                .to be_an(Array)
-                .and be_empty
+              expect(response)
+                .to have_http_status(200)
+              expect(response.media_type)
+                .to eq('application/activity+json')
+              expect(response.headers['Cache-Control'])
+                .to include('private')
+
+              expect(response.parsed_body)
+                .to include(
+                  orderedItems: be_an(Array).and(be_empty)
+                )
             end
           end
 
           context 'when signed request account is domain blocked' do
-            before do
-              account.block_domain!(remote_account.domain)
-            end
+            before { account.block_domain!(remote_account.domain) }
 
             it 'returns http success and correct media type and cache headers and empty items' do
-              expect(response).to have_http_status(200)
-              expect(response.media_type).to eq 'application/activity+json'
-              expect(response.headers['Cache-Control']).to include 'private'
+              subject
 
-              expect(response.parsed_body[:orderedItems])
-                .to be_an(Array)
-                .and be_empty
+              expect(response)
+                .to have_http_status(200)
+              expect(response.media_type)
+                .to eq('application/activity+json')
+              expect(response.headers['Cache-Control'])
+                .to include('private')
+
+              expect(response.parsed_body)
+                .to include(
+                  orderedItems: be_an(Array).and(be_empty)
+                )
             end
           end
         end
@@ -123,7 +136,10 @@ RSpec.describe ActivityPub::CollectionsController do
       let(:id) { 'hoge' }
 
       it 'returns http not found' do
-        expect(response).to have_http_status(404)
+        subject
+
+        expect(response)
+          .to have_http_status(404)
       end
     end
   end
diff --git a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb b/spec/requests/activitypub/followers_synchronizations_spec.rb
similarity index 68%
rename from spec/controllers/activitypub/followers_synchronizations_controller_spec.rb
rename to spec/requests/activitypub/followers_synchronizations_spec.rb
index cbd982f18f..97b8a7908e 100644
--- a/spec/controllers/activitypub/followers_synchronizations_controller_spec.rb
+++ b/spec/requests/activitypub/followers_synchronizations_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe ActivityPub::FollowersSynchronizationsController do
+RSpec.describe 'ActivityPub Follower Synchronizations' do
   let!(:account) { Fabricate(:account) }
   let!(:follower_example_com_user_a) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/users/a') }
   let!(:follower_example_com_user_b) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/users/b') }
@@ -14,32 +14,34 @@ RSpec.describe ActivityPub::FollowersSynchronizationsController do
     follower_example_com_user_b.follow!(account)
     follower_foo_com_user_a.follow!(account)
     follower_example_com_instance_actor.follow!(account)
-
-    allow(controller).to receive(:signed_request_actor).and_return(remote_account)
   end
 
   describe 'GET #show' do
     context 'without signature' do
-      let(:remote_account) { nil }
-
-      before do
-        get :show, params: { account_username: account.username }
-      end
+      subject { get account_followers_synchronization_path(account_username: account.username) }
 
       it 'returns http not authorized' do
-        expect(response).to have_http_status(401)
+        subject
+
+        expect(response)
+          .to have_http_status(401)
       end
     end
 
     context 'with signature from example.com' do
-      subject(:response) { get :show, params: { account_username: account.username } }
+      subject { get account_followers_synchronization_path(account_username: account.username), headers: nil, sign_with: remote_account }
 
       let(:remote_account) { Fabricate(:account, domain: 'example.com', uri: 'https://example.com/instance') }
 
       it 'returns http success and cache control and activity json types and correct items' do
-        expect(response).to have_http_status(200)
-        expect(response.headers['Cache-Control']).to eq 'max-age=0, private'
-        expect(response.media_type).to eq 'application/activity+json'
+        subject
+
+        expect(response)
+          .to have_http_status(200)
+        expect(response.headers['Cache-Control'])
+          .to eq 'max-age=0, private'
+        expect(response.media_type)
+          .to eq 'application/activity+json'
 
         expect(response.parsed_body[:orderedItems])
           .to be_an(Array)
@@ -57,17 +59,21 @@ RSpec.describe ActivityPub::FollowersSynchronizationsController do
         end
 
         it 'returns http gone' do
-          expect(response).to have_http_status(410)
+          subject
+
+          expect(response)
+            .to have_http_status(410)
         end
       end
 
       context 'when account is temporarily suspended' do
-        before do
-          account.suspend!
-        end
+        before { account.suspend! }
 
         it 'returns http forbidden' do
-          expect(response).to have_http_status(403)
+          subject
+
+          expect(response)
+            .to have_http_status(403)
         end
       end
     end
diff --git a/spec/controllers/activitypub/outboxes_controller_spec.rb b/spec/requests/activitypub/outboxes_spec.rb
similarity index 63%
rename from spec/controllers/activitypub/outboxes_controller_spec.rb
rename to spec/requests/activitypub/outboxes_spec.rb
index ca986dcabb..22b2f97c07 100644
--- a/spec/controllers/activitypub/outboxes_controller_spec.rb
+++ b/spec/requests/activitypub/outboxes_spec.rb
@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe ActivityPub::OutboxesController do
+RSpec.describe 'ActivityPub Outboxes' do
   let!(:account) { Fabricate(:account) }
 
   before do
@@ -11,13 +11,11 @@ RSpec.describe ActivityPub::OutboxesController do
     Fabricate(:status, account: account, visibility: :private)
     Fabricate(:status, account: account, visibility: :direct)
     Fabricate(:status, account: account, visibility: :limited)
-
-    allow(controller).to receive(:signed_request_actor).and_return(remote_account)
   end
 
   describe 'GET #show' do
     context 'without signature' do
-      subject(:response) { get :show, params: { account_username: account.username, page: page } }
+      subject { get account_outbox_path(account_username: account.username, page: page) }
 
       let(:remote_account) { nil }
 
@@ -25,13 +23,18 @@ RSpec.describe ActivityPub::OutboxesController do
         let(:page) { nil }
 
         it 'returns http success and correct media type and headers and items count' do
+          subject
+
           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(response.parsed_body[:totalItems]).to eq 4
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Vary'])
+            .to be_nil
+          expect(response.parsed_body[:totalItems])
+            .to eq 4
         end
 
         context 'when account is permanently suspended' do
@@ -41,17 +44,21 @@ RSpec.describe ActivityPub::OutboxesController do
           end
 
           it 'returns http gone' do
-            expect(response).to have_http_status(410)
+            subject
+
+            expect(response)
+              .to have_http_status(410)
           end
         end
 
         context 'when account is temporarily suspended' do
-          before do
-            account.suspend!
-          end
+          before { account.suspend! }
 
           it 'returns http forbidden' do
-            expect(response).to have_http_status(403)
+            subject
+
+            expect(response)
+              .to have_http_status(403)
           end
         end
       end
@@ -60,12 +67,16 @@ RSpec.describe ActivityPub::OutboxesController do
         let(:page) { 'true' }
 
         it 'returns http success and correct media type and vary header and items' do
+          subject
+
           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'
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Vary'])
+            .to include 'Signature'
 
           expect(response.parsed_body)
             .to include(
@@ -82,35 +93,42 @@ RSpec.describe ActivityPub::OutboxesController do
           end
 
           it 'returns http gone' do
-            expect(response).to have_http_status(410)
+            subject
+
+            expect(response)
+              .to have_http_status(410)
           end
         end
 
         context 'when account is temporarily suspended' do
-          before do
-            account.suspend!
-          end
+          before { account.suspend! }
 
           it 'returns http forbidden' do
-            expect(response).to have_http_status(403)
+            subject
+
+            expect(response)
+              .to have_http_status(403)
           end
         end
       end
     end
 
     context 'with signature' do
+      subject { get account_outbox_path(account_username: account.username, page: page), headers: nil, sign_with: remote_account }
+
       let(:remote_account) { Fabricate(:account, domain: 'example.com') }
       let(:page) { 'true' }
 
       context 'when signed request account does not follow account' do
-        before do
-          get :show, params: { account_username: account.username, page: page }
-        end
-
         it 'returns http success and correct media type and headers and items' do
-          expect(response).to have_http_status(200)
-          expect(response.media_type).to eq 'application/activity+json'
-          expect(response.headers['Cache-Control']).to eq 'max-age=60, private'
+          subject
+
+          expect(response)
+            .to have_http_status(200)
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Cache-Control'])
+            .to eq 'private, no-store'
 
           expect(response.parsed_body)
             .to include(
@@ -122,15 +140,17 @@ RSpec.describe ActivityPub::OutboxesController do
       end
 
       context 'when signed request account follows account' do
-        before do
-          remote_account.follow!(account)
-          get :show, params: { account_username: account.username, page: page }
-        end
+        before { remote_account.follow!(account) }
 
         it 'returns http success and correct media type and headers and items' do
-          expect(response).to have_http_status(200)
-          expect(response.media_type).to eq 'application/activity+json'
-          expect(response.headers['Cache-Control']).to eq 'max-age=60, private'
+          subject
+
+          expect(response)
+            .to have_http_status(200)
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Cache-Control'])
+            .to eq 'private, no-store'
 
           expect(response.parsed_body)
             .to include(
@@ -142,15 +162,17 @@ RSpec.describe ActivityPub::OutboxesController do
       end
 
       context 'when signed request account is blocked' do
-        before do
-          account.block!(remote_account)
-          get :show, params: { account_username: account.username, page: page }
-        end
+        before { account.block!(remote_account) }
 
         it 'returns http success and correct media type and headers and items' do
-          expect(response).to have_http_status(200)
-          expect(response.media_type).to eq 'application/activity+json'
-          expect(response.headers['Cache-Control']).to eq 'max-age=60, private'
+          subject
+
+          expect(response)
+            .to have_http_status(200)
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Cache-Control'])
+            .to eq 'private, no-store'
 
           expect(response.parsed_body)
             .to include(
@@ -160,15 +182,17 @@ RSpec.describe ActivityPub::OutboxesController do
       end
 
       context 'when signed request account is domain blocked' do
-        before do
-          account.block_domain!(remote_account.domain)
-          get :show, params: { account_username: account.username, page: page }
-        end
+        before { account.block_domain!(remote_account.domain) }
 
         it 'returns http success and correct media type and headers and items' do
-          expect(response).to have_http_status(200)
-          expect(response.media_type).to eq 'application/activity+json'
-          expect(response.headers['Cache-Control']).to eq 'max-age=60, private'
+          subject
+
+          expect(response)
+            .to have_http_status(200)
+          expect(response.media_type)
+            .to eq 'application/activity+json'
+          expect(response.headers['Cache-Control'])
+            .to eq 'private, no-store'
 
           expect(response.parsed_body)
             .to include(
diff --git a/spec/controllers/activitypub/replies_controller_spec.rb b/spec/requests/activitypub/replies_spec.rb
similarity index 78%
rename from spec/controllers/activitypub/replies_controller_spec.rb
rename to spec/requests/activitypub/replies_spec.rb
index d7c2c2d3b0..313cab2a44 100644
--- a/spec/controllers/activitypub/replies_controller_spec.rb
+++ b/spec/requests/activitypub/replies_spec.rb
@@ -2,9 +2,9 @@
 
 require 'rails_helper'
 
-RSpec.describe ActivityPub::RepliesController do
+RSpec.describe 'ActivityPub Replies' do
   let(:status) { Fabricate(:status, visibility: parent_visibility) }
-  let(:remote_account)  { Fabricate(:account, domain: 'foobar.com') }
+  let(:remote_account) { Fabricate(:account, domain: 'foobar.com') }
   let(:remote_reply_id) { 'https://foobar.com/statuses/1234' }
   let(:remote_querier) { nil }
 
@@ -13,7 +13,10 @@ RSpec.describe ActivityPub::RepliesController do
       let(:parent_visibility) { :private }
 
       it 'returns http not found' do
-        expect(response).to have_http_status(404)
+        subject
+
+        expect(response)
+          .to have_http_status(404)
       end
     end
 
@@ -21,7 +24,10 @@ RSpec.describe ActivityPub::RepliesController do
       let(:parent_visibility) { :direct }
 
       it 'returns http not found' do
-        expect(response).to have_http_status(404)
+        subject
+
+        expect(response)
+          .to have_http_status(404)
       end
     end
   end
@@ -31,7 +37,10 @@ RSpec.describe ActivityPub::RepliesController do
       let(:parent_visibility) { :public }
 
       it 'returns http not found' do
-        expect(response).to have_http_status(404)
+        subject
+
+        expect(response)
+          .to have_http_status(404)
       end
     end
 
@@ -48,19 +57,23 @@ RSpec.describe ActivityPub::RepliesController do
       end
 
       it 'returns http gone' do
-        expect(response).to have_http_status(410)
+        subject
+
+        expect(response)
+          .to have_http_status(410)
       end
     end
 
     context 'when account is temporarily suspended' do
       let(:parent_visibility) { :public }
 
-      before do
-        status.account.suspend!
-      end
+      before { status.account.suspend! }
 
       it 'returns http forbidden' do
-        expect(response).to have_http_status(403)
+        subject
+
+        expect(response)
+          .to have_http_status(403)
       end
     end
 
@@ -68,15 +81,20 @@ RSpec.describe ActivityPub::RepliesController do
       let(:parent_visibility) { :public }
 
       it 'returns http success and correct media type' do
+        subject
+
         expect(response)
           .to have_http_status(200)
           .and have_cacheable_headers
 
-        expect(response.media_type).to eq 'application/activity+json'
+        expect(response.media_type)
+          .to eq 'application/activity+json'
       end
 
-      context 'without only_other_accounts' do
+      context 'without `only_other_accounts` param' do
         it "returns items with thread author's replies" do
+          subject
+
           expect(response.parsed_body)
             .to include(
               first: be_a(Hash).and(
@@ -91,6 +109,8 @@ RSpec.describe ActivityPub::RepliesController do
 
         context 'when there are few self-replies' do
           it 'points next to replies from other people' do
+            subject
+
             expect(response.parsed_body)
               .to include(
                 first: be_a(Hash).and(
@@ -108,6 +128,8 @@ RSpec.describe ActivityPub::RepliesController do
           end
 
           it 'points next to other self-replies' do
+            subject
+
             expect(response.parsed_body)
               .to include(
                 first: be_a(Hash).and(
@@ -120,31 +142,33 @@ RSpec.describe ActivityPub::RepliesController do
         end
       end
 
-      context 'with only_other_accounts' do
+      context 'with `only_other_accounts` param' do
         let(:only_other_accounts) { 'true' }
 
-        it 'returns items with other public or unlisted replies' do
+        it 'returns items with other public or unlisted replies and correctly inlines replies and uses IDs' do
+          subject
+
           expect(response.parsed_body)
             .to include(
               first: be_a(Hash).and(
                 include(items: be_an(Array).and(have_attributes(size: 3)))
               )
             )
-        end
 
-        it 'only inlines items that are local and public or unlisted replies' do
+          # Only inline replies that are local and public, or unlisted
           expect(inlined_replies)
             .to all(satisfy { |item| targets_public_collection?(item) })
             .and all(satisfy { |item| ActivityPub::TagManager.instance.local_uri?(item[:id]) })
-        end
 
-        it 'uses ids for remote toots' do
+          # Use ids for remote replies
           expect(remote_replies)
             .to all(satisfy { |item| item.is_a?(String) && !ActivityPub::TagManager.instance.local_uri?(item) })
         end
 
         context 'when there are few replies' do
           it 'does not have a next page' do
+            subject
+
             expect(response.parsed_body)
               .to include(
                 first: be_a(Hash).and(not_include(next: be_present))
@@ -158,6 +182,8 @@ RSpec.describe ActivityPub::RepliesController do
           end
 
           it 'points next to other replies' do
+            subject
+
             expect(response.parsed_body)
               .to include(
                 first: be_a(Hash).and(
@@ -176,10 +202,8 @@ RSpec.describe ActivityPub::RepliesController do
 
   before do
     stub_const 'ActivityPub::RepliesController::DESCENDANTS_LIMIT', 5
-    allow(controller).to receive(:signed_request_actor).and_return(remote_querier)
 
-    Fabricate(:status, thread: status, visibility: :public)
-    Fabricate(:status, thread: status, visibility: :public)
+    Fabricate.times(2, :status, thread: status, visibility: :public)
     Fabricate(:status, thread: status, visibility: :private)
     Fabricate(:status, account: status.account, thread: status, visibility: :public)
     Fabricate(:status, account: status.account, thread: status, visibility: :private)
@@ -188,31 +212,29 @@ RSpec.describe ActivityPub::RepliesController do
   end
 
   describe 'GET #index' do
-    subject(:response) { get :index, params: { account_username: status.account.username, status_id: status.id, only_other_accounts: only_other_accounts } }
-
     let(:only_other_accounts) { nil }
 
     context 'with no signature' do
+      subject { get account_status_replies_path(account_username: status.account.username, status_id: status.id, only_other_accounts: only_other_accounts) }
+
       it_behaves_like 'allowed access'
     end
 
     context 'with signature' do
+      subject { get account_status_replies_path(account_username: status.account.username, status_id: status.id, only_other_accounts: only_other_accounts), headers: nil, sign_with: remote_querier }
+
       let(:remote_querier) { Fabricate(:account, domain: 'example.com') }
 
       it_behaves_like 'allowed access'
 
       context 'when signed request account is blocked' do
-        before do
-          status.account.block!(remote_querier)
-        end
+        before { status.account.block!(remote_querier) }
 
         it_behaves_like 'disallowed access'
       end
 
       context 'when signed request account is domain blocked' do
-        before do
-          status.account.block_domain!(remote_querier.domain)
-        end
+        before { status.account.block_domain!(remote_querier.domain) }
 
         it_behaves_like 'disallowed access'
       end