From 12bb7be8b558122e44a66a4337e7db999c11e0b3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 17 Oct 2023 07:32:10 -0400 Subject: [PATCH] Spec speed ups on `AccountsController` spec (#25391) --- spec/controllers/accounts_controller_spec.rb | 638 ++++++++----------- 1 file changed, 254 insertions(+), 384 deletions(-) diff --git a/spec/controllers/accounts_controller_spec.rb b/spec/controllers/accounts_controller_spec.rb index d46fdb108d..cc9e3198b6 100644 --- a/spec/controllers/accounts_controller_spec.rb +++ b/spec/controllers/accounts_controller_spec.rb @@ -7,449 +7,319 @@ RSpec.describe AccountsController do let(:account) { Fabricate(:account) } - describe 'GET #show' do - let(:format) { 'html' } + shared_examples 'unapproved account check' do + before { account.user.update(approved: false) } - let!(:status) { Fabricate(:status, account: account) } - let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) } - let!(:status_self_reply) { Fabricate(:status, account: account, thread: status) } - let!(:status_media) { Fabricate(:status, account: account) } - let!(:status_pinned) { Fabricate(:status, account: account) } - let!(:status_private) { Fabricate(:status, account: account, visibility: :private) } - let!(:status_direct) { Fabricate(:status, account: account, visibility: :direct) } - let!(:status_reblog) { Fabricate(:status, account: account, reblog: Fabricate(:status)) } + it 'returns http not found' do + get :show, params: { username: account.username, format: format } + expect(response).to have_http_status(404) + end + end + + shared_examples 'permanently suspended account check' do before do - status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image) - account.pinned_statuses << status_pinned - account.pinned_statuses << status_private + account.suspend! + account.deletion_request.destroy end - shared_examples 'preliminary checks' do - context 'when account is not approved' do - before do - account.user.update(approved: false) - end + it 'returns http gone' do + get :show, params: { username: account.username, format: format } - it 'returns http not found' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(404) - end + expect(response).to have_http_status(410) + end + end + + shared_examples 'temporarily suspended account check' do |code: 403| + before { account.suspend! } + + it 'returns appropriate http response code' do + get :show, params: { username: account.username, format: format } + + expect(response).to have_http_status(code) + end + end + + describe 'GET #show' do + context 'with basic account status checks' do + context 'with HTML' do + let(:format) { 'html' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check' + end + + context 'with JSON' do + let(:format) { 'json' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check', code: 200 + end + + context 'with RSS' do + let(:format) { 'rss' } + + it_behaves_like 'unapproved account check' + it_behaves_like 'permanently suspended account check' + it_behaves_like 'temporarily suspended account check' end end - context 'with HTML' do - let(:format) { 'html' } - - it_behaves_like 'preliminary checks' - - context 'when account is permanently suspended' do - before do - account.suspend! - account.deletion_request.destroy - end - - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) - end - end - - context 'when account is temporarily suspended' do - before do - account.suspend! - end - - it 'returns http forbidden' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(403) - end - end - - shared_examples 'common response characteristics' do - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns Link header' do - expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) - end - - it 'renders show template' do - expect(response).to render_template(:show) - end - end - - context 'with a normal account in an HTML request' do - before do - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with replies' do - before do - allow(controller).to receive(:replies_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with media' do - before do - allow(controller).to receive(:media_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - end - - context 'with tag' do - let(:tag) { Fabricate(:tag) } - - let!(:status_tag) { Fabricate(:status, account: account) } - - before do - allow(controller).to receive(:tag_requested?).and_return(true) - status_tag.tags << tag - get :show, params: { username: account.username, format: format, tag: tag.to_param } - end - - it_behaves_like 'common response characteristics' - end - end - - context 'with JSON' do - let(:authorized_fetch_mode) { false } - let(:format) { 'json' } + context 'with existing statuses' do + let!(:status) { Fabricate(:status, account: account) } + let!(:status_reply) { Fabricate(:status, account: account, thread: Fabricate(:status)) } + let!(:status_self_reply) { Fabricate(:status, account: account, thread: status) } + let!(:status_media) { Fabricate(:status, account: account) } + let!(:status_pinned) { Fabricate(:status, account: account) } + let!(:status_private) { Fabricate(:status, account: account, visibility: :private) } + let!(:status_direct) { Fabricate(:status, account: account, visibility: :direct) } + let!(:status_reblog) { Fabricate(:status, account: account, reblog: Fabricate(:status)) } before do - allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode) + status_media.media_attachments << Fabricate(:media_attachment, account: account, type: :image) + account.pinned_statuses << status_pinned + account.pinned_statuses << status_private end - it_behaves_like 'preliminary checks' + context 'with HTML' do + let(:format) { 'html' } - context 'when account is suspended permanently' do - before do - account.suspend! - account.deletion_request.destroy + shared_examples 'common HTML response' do + it 'returns a standard HTML response', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.headers['Link'].to_s).to include ActivityPub::TagManager.instance.uri_for(account) + + expect(response).to render_template(:show) + end end - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) + context 'with a normal account in an HTML request' do + before do + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with replies' do + before do + allow(controller).to receive(:replies_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with media' do + before do + allow(controller).to receive(:media_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end + + it_behaves_like 'common HTML response' + end + + context 'with tag' do + let(:tag) { Fabricate(:tag) } + + let!(:status_tag) { Fabricate(:status, account: account) } + + before do + allow(controller).to receive(:tag_requested?).and_return(true) + status_tag.tags << tag + get :show, params: { username: account.username, format: format, tag: tag.to_param } + end + + it_behaves_like 'common HTML response' end end - context 'when account is suspended temporarily' do + context 'with JSON' do + let(:authorized_fetch_mode) { false } + let(:format) { 'json' } + before do - account.suspend! + allow(controller).to receive(:authorized_fetch_mode?).and_return(authorized_fetch_mode) end - it 'returns http success' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(200) - end - end + context 'with a normal account in a JSON request' do + before do + get :show, params: { username: account.username, format: format } + end - context 'with a normal account in a JSON request' do - before do - get :show, params: { username: account.username, format: format } + it 'returns a JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) + + expect(response.media_type).to eq 'application/activity+json' + + 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 } + + it 'returns http unauthorized' do + expect(response).to have_http_status(401) + end + end end - it 'returns http success' do - expect(response).to have_http_status(200) + context 'when signed in' do + let(:user) { Fabricate(:user) } + + before do + sign_in(user) + get :show, params: { username: account.username, format: format } + end + + it 'returns a private JSON version of the account', :aggregate_failures 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' + + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end end - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end + context 'with signature' do + let(:remote_account) { Fabricate(:account, domain: 'example.com') } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + before do + allow(controller).to receive(:signed_request_actor).and_return(remote_account) + get :show, params: { username: account.username, format: format } + end - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end + it 'returns a JSON version of the account', :aggregate_failures do + expect(response).to have_http_status(200) - context 'with authorized fetch mode' do - let(:authorized_fetch_mode) { true } + expect(response.media_type).to eq 'application/activity+json' - it 'returns http unauthorized' do - expect(response).to have_http_status(401) + 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 } + + it 'returns a private signature JSON version of the account', :aggregate_failures 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' + + expect(response.headers['Vary']).to include 'Signature' + + expect(body_as_json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + end end end end - context 'when signed in' do - let(:user) { Fabricate(:user) } - - before do - sign_in(user) - get :show, params: { username: account.username, format: format } - end - - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end - - it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'private' - end - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end - end - - context 'with signature' do - let(:remote_account) { Fabricate(:account, domain: 'example.com') } - - before do - allow(controller).to receive(:signed_request_actor).and_return(remote_account) - get :show, params: { username: account.username, format: format } - end - - it 'returns http success' do - expect(response).to have_http_status(200) - end - - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' - end - - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) - end - - context 'with authorized fetch mode' do - let(:authorized_fetch_mode) { true } + context 'with RSS' do + let(:format) { 'rss' } + shared_examples 'common RSS response' do it 'returns http success' do expect(response).to have_http_status(200) end - it 'returns application/activity+json' do - expect(response.media_type).to eq 'application/activity+json' + it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' + end + + context 'with a normal account in an RSS request' do + before do + get :show, params: { username: account.username, format: format } end - it 'returns private Cache-Control header' do - expect(response.headers['Cache-Control']).to include 'private' - end + it_behaves_like 'common RSS response' - it 'returns Vary header with Signature' do - expect(response.headers['Vary']).to include 'Signature' - end - - it 'renders account' do - json = body_as_json - expect(json).to include(:id, :type, :preferredUsername, :inbox, :publicKey, :name, :summary) + it 'responds with correct statuses', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to include_status_tag(status_self_reply) + expect(response.body).to include_status_tag(status) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) end end - end - end - context 'with RSS' do - let(:format) { 'rss' } + context 'with replies' do + before do + allow(controller).to receive(:replies_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end - it_behaves_like 'preliminary checks' + it_behaves_like 'common RSS response' - context 'when account is permanently suspended' do - before do - account.suspend! - account.deletion_request.destroy + it 'responds with correct statuses with replies', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to include_status_tag(status_reply) + expect(response.body).to include_status_tag(status_self_reply) + expect(response.body).to include_status_tag(status) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + end end - it 'returns http gone' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(410) - end - end + context 'with media' do + before do + allow(controller).to receive(:media_requested?).and_return(true) + get :show, params: { username: account.username, format: format } + end - context 'when account is temporarily suspended' do - before do - account.suspend! + it_behaves_like 'common RSS response' + + it 'responds with correct statuses with media', :aggregate_failures do + expect(response.body).to include_status_tag(status_media) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) + expect(response.body).to_not include_status_tag(status_self_reply) + expect(response.body).to_not include_status_tag(status) + end end - it 'returns http forbidden' do - get :show, params: { username: account.username, format: format } - expect(response).to have_http_status(403) - end - end + context 'with tag' do + let(:tag) { Fabricate(:tag) } - shared_examples 'common response characteristics' do - it 'returns http success' do - expect(response).to have_http_status(200) - end + let!(:status_tag) { Fabricate(:status, account: account) } - it_behaves_like 'cacheable response', expects_vary: 'Accept, Accept-Language, Cookie' - end + before do + allow(controller).to receive(:tag_requested?).and_return(true) + status_tag.tags << tag + get :show, params: { username: account.username, format: format, tag: tag.to_param } + end - context 'with a normal account in an RSS request' do - before do - get :show, params: { username: account.username, format: format } - end + it_behaves_like 'common RSS response' - it_behaves_like 'common response characteristics' - - it 'renders public status' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'renders self-reply' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with replies' do - before do - allow(controller).to receive(:replies_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - - it 'renders public status' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'renders self-reply' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'renders reply to someone else' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with media' do - before do - allow(controller).to receive(:media_requested?).and_return(true) - get :show, params: { username: account.username, format: format } - end - - it_behaves_like 'common response characteristics' - - it 'does not render public status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'does not render self-reply' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'renders status with media' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - end - - context 'with tag' do - let(:tag) { Fabricate(:tag) } - - let!(:status_tag) { Fabricate(:status, account: account) } - - before do - allow(controller).to receive(:tag_requested?).and_return(true) - status_tag.tags << tag - get :show, params: { username: account.username, format: format, tag: tag.to_param } - end - - it_behaves_like 'common response characteristics' - - it 'does not render public status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status)) - end - - it 'does not render self-reply' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_self_reply)) - end - - it 'does not render status with media' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_media)) - end - - it 'does not render reblog' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reblog.reblog)) - end - - it 'does not render private status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_private)) - end - - it 'does not render direct status' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_direct)) - end - - it 'does not render reply to someone else' do - expect(response.body).to_not include(ActivityPub::TagManager.instance.url_for(status_reply)) - end - - it 'renders status with tag' do - expect(response.body).to include(ActivityPub::TagManager.instance.url_for(status_tag)) + it 'responds with correct statuses with a tag', :aggregate_failures do + expect(response.body).to include_status_tag(status_tag) + expect(response.body).to_not include_status_tag(status_direct) + expect(response.body).to_not include_status_tag(status_media) + expect(response.body).to_not include_status_tag(status_private) + expect(response.body).to_not include_status_tag(status_reblog.reblog) + expect(response.body).to_not include_status_tag(status_reply) + expect(response.body).to_not include_status_tag(status_self_reply) + expect(response.body).to_not include_status_tag(status) + end end end end end + + def include_status_tag(status) + include ActivityPub::TagManager.instance.url_for(status) + end end