From e4e07b1c34858dc6b7070f1b5d81f40ec63ebf4c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Fri, 4 Oct 2024 10:11:15 -0400 Subject: [PATCH] Reduce factory usage across `spec/services` area (#32098) --- .../account_statuses_cleanup_service_spec.rb | 75 +++--- .../process_status_update_service_spec.rb | 74 +++-- .../services/authorize_follow_service_spec.rb | 34 ++- .../batched_remove_status_service_spec.rb | 52 ++-- spec/services/block_service_spec.rb | 13 +- spec/services/bulk_import_service_spec.rb | 255 +++++++----------- spec/services/favourite_service_spec.rb | 19 +- spec/services/follow_service_spec.rb | 13 +- .../services/process_mentions_service_spec.rb | 48 ++-- spec/services/reject_follow_service_spec.rb | 36 ++- .../remove_from_followers_service_spec.rb | 23 +- spec/services/remove_status_service_spec.rb | 44 ++- spec/services/report_service_spec.rb | 35 ++- spec/services/resolve_account_service_spec.rb | 128 +++++---- spec/services/resolve_url_service_spec.rb | 44 ++- .../services/translate_status_service_spec.rb | 52 ++-- spec/services/unblock_domain_service_spec.rb | 38 +-- spec/services/unblock_service_spec.rb | 24 +- spec/services/unfollow_service_spec.rb | 38 +-- spec/services/update_account_service_spec.rb | 20 +- spec/services/update_status_service_spec.rb | 143 +++++----- 21 files changed, 567 insertions(+), 641 deletions(-) diff --git a/spec/services/account_statuses_cleanup_service_spec.rb b/spec/services/account_statuses_cleanup_service_spec.rb index 857bd4fda..553d20029 100644 --- a/spec/services/account_statuses_cleanup_service_spec.rb +++ b/spec/services/account_statuses_cleanup_service_spec.rb @@ -27,39 +27,35 @@ RSpec.describe AccountStatusesCleanupService do end context 'when given a normal budget of 10' do - it 'reports 3 deleted statuses' do - expect(subject.call(account_policy, 10)).to eq 3 - end + it 'reports 3 deleted statuses and records last deleted id, deletes statuses, preserves recent unrelated statuses' do + expect(subject.call(account_policy, 10)) + .to eq(3) - it 'records the last deleted id' do - subject.call(account_policy, 10) - expect(account_policy.last_inspected).to eq [old_status.id, another_old_status.id].max - end + expect(account_policy.last_inspected) + .to eq [old_status.id, another_old_status.id].max - it 'actually deletes the statuses' do - subject.call(account_policy, 10) - expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])).to be_nil - expect { recent_status.reload }.to_not raise_error - end - - it 'preserves recent and unrelated statuses' do - subject.call(account_policy, 10) - expect { unrelated_status.reload }.to_not raise_error - expect { recent_status.reload }.to_not raise_error + expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])) + .to be_nil + expect { recent_status.reload } + .to_not raise_error + expect { unrelated_status.reload } + .to_not raise_error + expect { recent_status.reload } + .to_not raise_error end end context 'when called repeatedly with a budget of 2' do - it 'reports 2 then 1 deleted statuses' do - expect(subject.call(account_policy, 2)).to eq 2 - expect(subject.call(account_policy, 2)).to eq 1 - end + it 'reports 2 then 1 deleted statuses and deletes in expected order' do + expect(subject.call(account_policy, 2)) + .to eq(2) + expect(Status.find_by(id: very_old_status.id)) + .to be_nil - it 'actually deletes the statuses in the expected order' do - subject.call(account_policy, 2) - expect(Status.find_by(id: very_old_status.id)).to be_nil - subject.call(account_policy, 2) - expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])).to be_nil + expect(subject.call(account_policy, 2)) + .to eq(1) + expect(Status.find_by(id: [very_old_status.id, old_status.id, another_old_status.id])) + .to be_nil end end @@ -90,19 +86,24 @@ RSpec.describe AccountStatusesCleanupService do end end - it 'reports 0 deleted statuses then 0 then 3 then 0 again' do - expect(subject.call(account_policy, 10)).to eq 0 - expect(subject.call(account_policy, 10)).to eq 0 - expect(subject.call(account_policy, 10)).to eq 3 - expect(subject.call(account_policy, 10)).to eq 0 + it 'reports 0 deleted statuses then 0 then 3 then 0 again, and keeps id under oldest deletable record' do + expect(subject.call(account_policy, 10)) + .to eq(0) + expect(subject.call(account_policy, 10)) + .to eq(0) + expect(subject.call(account_policy, 10)) + .to eq(3) + expect(subject.call(account_policy, 10)) + .to eq(0) + expect(account_policy.last_inspected) + .to be < oldest_deletable_record_id end - it 'never causes the recorded id to get higher than oldest deletable toot' do - subject.call(account_policy, 10) - subject.call(account_policy, 10) - subject.call(account_policy, 10) - subject.call(account_policy, 10) - expect(account_policy.last_inspected).to be < Mastodon::Snowflake.id_at(account_policy.min_status_age.seconds.ago, with_random: false) + def oldest_deletable_record_id + Mastodon::Snowflake.id_at( + account_policy.min_status_age.seconds.ago, + with_random: false + ) end end end diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index a97e84080..b6ceba374 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -2,10 +2,6 @@ require 'rails_helper' -def poll_option_json(name, votes) - { type: 'Note', name: name, replies: { type: 'Collection', totalItems: votes } } -end - RSpec.describe ActivityPub::ProcessStatusUpdateService do subject { described_class.new } @@ -294,7 +290,6 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do context 'when originally without media attachments' do before do stub_request(:get, 'https://example.com/foo.png').to_return(body: attachment_fixture('emojo.png')) - subject.call(status, json, json) end let(:payload) do @@ -310,19 +305,18 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do } end - it 'updates media attachments' do - media_attachment = status.reload.ordered_media_attachments.first + it 'updates media attachments, fetches attachment, records media change in edit' do + subject.call(status, json, json) - expect(media_attachment).to_not be_nil - expect(media_attachment.remote_url).to eq 'https://example.com/foo.png' - end + expect(status.reload.ordered_media_attachments.first) + .to be_present + .and(have_attributes(remote_url: 'https://example.com/foo.png')) - it 'fetches the attachment' do - expect(a_request(:get, 'https://example.com/foo.png')).to have_been_made - end + expect(a_request(:get, 'https://example.com/foo.png')) + .to have_been_made - it 'records media change in edit' do - expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty + expect(status.edits.reload.last.ordered_media_attachment_ids) + .to_not be_empty end end @@ -344,27 +338,26 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do before do allow(RedownloadMediaWorker).to receive(:perform_async) + end + + it 'updates the existing media attachment in-place, does not queue redownload, updates media, records media change' do subject.call(status, json, json) - end - it 'updates the existing media attachment in-place' do - media_attachment = status.media_attachments.ordered.reload.first + expect(status.media_attachments.ordered.reload.first) + .to be_present + .and have_attributes( + remote_url: 'https://example.com/foo.png', + description: 'A picture' + ) - expect(media_attachment).to_not be_nil - expect(media_attachment.remote_url).to eq 'https://example.com/foo.png' - expect(media_attachment.description).to eq 'A picture' - end + expect(RedownloadMediaWorker) + .to_not have_received(:perform_async) - it 'does not queue redownload for the existing media attachment' do - expect(RedownloadMediaWorker).to_not have_received(:perform_async) - end + expect(status.ordered_media_attachments.map(&:remote_url)) + .to eq %w(https://example.com/foo.png) - it 'updates media attachments' do - expect(status.ordered_media_attachments.map(&:remote_url)).to eq %w(https://example.com/foo.png) - end - - it 'records media change in edit' do - expect(status.edits.reload.last.ordered_media_attachment_ids).to_not be_empty + expect(status.edits.reload.last.ordered_media_attachment_ids) + .to_not be_empty end end @@ -372,10 +365,11 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do before do poll = Fabricate(:poll, status: status) status.update(preloadable_poll: poll) - subject.call(status, json, json) end it 'removes poll and records media change in edit' do + subject.call(status, json, json) + expect(status.reload.poll).to be_nil expect(status.edits.reload.last.poll_options).to be_nil end @@ -398,15 +392,13 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do } end - before do - subject.call(status, json, json) - end - it 'creates a poll and records media change in edit' do - poll = status.reload.poll + subject.call(status, json, json) + + expect(status.reload.poll) + .to be_present + .and have_attributes(options: %w(Foo Bar Baz)) - expect(poll).to_not be_nil - expect(poll.options).to eq %w(Foo Bar Baz) expect(status.edits.reload.last.poll_options).to eq %w(Foo Bar Baz) end end @@ -419,4 +411,8 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do .to eq '2021-09-08 22:39:25 UTC' end end + + def poll_option_json(name, votes) + { type: 'Note', name: name, replies: { type: 'Collection', totalItems: votes } } + end end diff --git a/spec/services/authorize_follow_service_spec.rb b/spec/services/authorize_follow_service_spec.rb index 533b791fb..de2857280 100644 --- a/spec/services/authorize_follow_service_spec.rb +++ b/spec/services/authorize_follow_service_spec.rb @@ -12,15 +12,15 @@ RSpec.describe AuthorizeFollowService do before do FollowRequest.create(account: bob, target_account: sender) + end + + it 'removes follow request and creates follow relation' do subject.call(bob, sender) - end - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'creates follow relation' do - expect(bob.following?(sender)).to be true + expect(bob) + .to_not be_requested(sender) + expect(bob) + .to be_following(sender) end end @@ -30,19 +30,17 @@ RSpec.describe AuthorizeFollowService do before do FollowRequest.create(account: bob, target_account: sender) stub_request(:post, bob.inbox_url).to_return(status: 200) + end + + it 'removes follow request, creates follow relation, send accept activity', :inline_jobs do subject.call(bob, sender) - end - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'creates follow relation' do - expect(bob.following?(sender)).to be true - end - - it 'sends an accept activity', :inline_jobs do - expect(a_request(:post, bob.inbox_url)).to have_been_made.once + expect(bob) + .to_not be_requested(sender) + expect(bob) + .to be_following(sender) + expect(a_request(:post, bob.inbox_url)) + .to have_been_made.once end end end diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index 628bb198e..1ff73a633 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -24,32 +24,38 @@ RSpec.describe BatchedRemoveStatusService, :inline_jobs do status_alice_hello status_alice_other + end + it 'removes status records, removes from author and local follower feeds, notifies stream, sends delete' do subject.call([status_alice_hello, status_alice_other]) + + expect { Status.find(status_alice_hello.id) } + .to raise_error ActiveRecord::RecordNotFound + expect { Status.find(status_alice_other.id) } + .to raise_error ActiveRecord::RecordNotFound + + expect(feed_ids_for(alice)) + .to_not include(status_alice_hello.id, status_alice_other.id) + + expect(feed_ids_for(jeff)) + .to_not include(status_alice_hello.id, status_alice_other.id) + + expect(redis) + .to have_received(:publish) + .with("timeline:#{jeff.id}", any_args).at_least(:once) + + expect(redis) + .to have_received(:publish) + .with('timeline:public', any_args).at_least(:once) + + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.at_least_once end - it 'removes statuses' do - expect { Status.find(status_alice_hello.id) }.to raise_error ActiveRecord::RecordNotFound - expect { Status.find(status_alice_other.id) }.to raise_error ActiveRecord::RecordNotFound - end - - it 'removes statuses from author\'s home feed' do - expect(HomeFeed.new(alice).get(10).pluck(:id)).to_not include(status_alice_hello.id, status_alice_other.id) - end - - it 'removes statuses from local follower\'s home feed' do - expect(HomeFeed.new(jeff).get(10).pluck(:id)).to_not include(status_alice_hello.id, status_alice_other.id) - end - - it 'notifies streaming API of followers' do - expect(redis).to have_received(:publish).with("timeline:#{jeff.id}", any_args).at_least(:once) - end - - it 'notifies streaming API of public timeline' do - expect(redis).to have_received(:publish).with('timeline:public', any_args).at_least(:once) - end - - it 'sends delete activity to followers' do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.at_least_once + def feed_ids_for(account) + HomeFeed + .new(account) + .get(10) + .pluck(:id) end end diff --git a/spec/services/block_service_spec.rb b/spec/services/block_service_spec.rb index 46dd69198..d9687a540 100644 --- a/spec/services/block_service_spec.rb +++ b/spec/services/block_service_spec.rb @@ -26,15 +26,16 @@ RSpec.describe BlockService do before do stub_request(:post, 'http://example.com/inbox').to_return(status: 200) + end + + it 'creates a blocking relation and send block activity', :inline_jobs do subject.call(sender, bob) - end - it 'creates a blocking relation' do - expect(sender.blocking?(bob)).to be true - end + expect(sender) + .to be_blocking(bob) - it 'sends a block activity', :inline_jobs do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end end diff --git a/spec/services/bulk_import_service_spec.rb b/spec/services/bulk_import_service_spec.rb index 0197f81a4..f52fc4d7d 100644 --- a/spec/services/bulk_import_service_spec.rb +++ b/spec/services/bulk_import_service_spec.rb @@ -24,30 +24,19 @@ RSpec.describe BulkImportService do ].map { |data| import.rows.create!(data: data) } end - before do - account.follow!(Fabricate(:account)) - end + before { account.follow!(Fabricate(:account)) } - it 'does not immediately change who the account follows' do - expect { subject.call(import) }.to_not(change { account.reload.active_relationships.to_a }) - end + it 'does not immediately change who the account follows, enqueues workers, sends follow requests after worker run' do + expect { subject.call(import) } + .to_not(change { account.reload.active_relationships.to_a }) - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows.map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows.map(&:id)) - it 'requests to follow all the listed users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') + expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }) + .to contain_exactly('user@foo.bar', 'unknown@unknown.bar') end end @@ -71,31 +60,20 @@ RSpec.describe BulkImportService do account.follow!(to_be_unfollowed) end - it 'unfollows user not present on list' do - subject.call(import) - expect(account.following?(to_be_unfollowed)).to be false - end + it 'updates the existing follow relationship as expected and unfollows user not on list, enqueues workers, sends follow reqs after worker run' do + expect { subject.call(import) } + .to change { Follow.where(account: account, target_account: followed).pick(:show_reblogs, :notify, :languages) }.from([true, false, nil]).to([false, true, ['en']]) - it 'updates the existing follow relationship as expected' do - expect { subject.call(import) }.to change { Follow.where(account: account, target_account: followed).pick(:show_reblogs, :notify, :languages) }.from([true, false, nil]).to([false, true, ['en']]) - end + expect(account) + .to_not be_following(to_be_unfollowed) - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows[1..].map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows[1..].map(&:id)) - it 'requests to follow all the expected users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') + expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }) + .to contain_exactly('user@foo.bar', 'unknown@unknown.bar') end end @@ -110,30 +88,19 @@ RSpec.describe BulkImportService do ].map { |data| import.rows.create!(data: data) } end - before do - account.block!(Fabricate(:account, username: 'already_blocked', domain: 'remote.org')) - end + before { account.block!(Fabricate(:account, username: 'already_blocked', domain: 'remote.org')) } - it 'does not immediately change who the account blocks' do - expect { subject.call(import) }.to_not(change { account.reload.blocking.to_a }) - end + it 'does not immediately change who the account blocks, enqueues worker, blocks after run' do + expect { subject.call(import) } + .to_not(change { account.reload.blocking.to_a }) - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows.map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows.map(&:id)) - it 'blocks all the listed users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(account.blocking.map(&:acct)).to contain_exactly('already_blocked@remote.org', 'user@foo.bar', 'unknown@unknown.bar') + expect(account.reload.blocking.map(&:acct)) + .to contain_exactly('already_blocked@remote.org', 'user@foo.bar', 'unknown@unknown.bar') end end @@ -157,27 +124,18 @@ RSpec.describe BulkImportService do account.block!(to_be_unblocked) end - it 'unblocks user not present on list' do + it 'unblocks user not present on list, enqueues worker, requests follow after run' do subject.call(import) + expect(account.blocking?(to_be_unblocked)).to be false - end - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows[1..].map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows[1..].map(&:id)) - it 'requests to follow all the expected users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(account.blocking.map(&:acct)).to contain_exactly('blocked@foo.bar', 'user@foo.bar', 'unknown@unknown.bar') + expect(account.blocking.map(&:acct)) + .to contain_exactly('blocked@foo.bar', 'user@foo.bar', 'unknown@unknown.bar') end end @@ -192,30 +150,19 @@ RSpec.describe BulkImportService do ].map { |data| import.rows.create!(data: data) } end - before do - account.mute!(Fabricate(:account, username: 'already_muted', domain: 'remote.org')) - end + before { account.mute!(Fabricate(:account, username: 'already_muted', domain: 'remote.org')) } - it 'does not immediately change who the account blocks' do - expect { subject.call(import) }.to_not(change { account.reload.muting.to_a }) - end + it 'does not immediately change who the account blocks, enqueures worker, mutes users after worker run' do + expect { subject.call(import) } + .to_not(change { account.reload.muting.to_a }) - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows.map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows.map(&:id)) - it 'mutes all the listed users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(account.muting.map(&:acct)).to contain_exactly('already_muted@remote.org', 'user@foo.bar', 'unknown@unknown.bar') + expect(account.reload.muting.map(&:acct)) + .to contain_exactly('already_muted@remote.org', 'user@foo.bar', 'unknown@unknown.bar') end end @@ -239,31 +186,19 @@ RSpec.describe BulkImportService do account.mute!(to_be_unmuted) end - it 'updates the existing mute as expected' do - expect { subject.call(import) }.to change { Mute.where(account: account, target_account: muted).pick(:hide_notifications) }.from(false).to(true) - end + it 'updates the existing mute as expected and unblocks user not on list, and enqueues worker, and requests follow after worker run' do + expect { subject.call(import) } + .to change { Mute.where(account: account, target_account: muted).pick(:hide_notifications) }.from(false).to(true) - it 'unblocks user not present on list' do - subject.call(import) expect(account.muting?(to_be_unmuted)).to be false - end - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows[1..].map(&:id)) - end + expect(row_worker_job_args) + .to match_array(rows[1..].map(&:id)) - it 'requests to follow all the expected users once the workers have run' do - subject.call(import) + stub_resolve_account_and_drain_workers - resolve_account_service_double = instance_double(ResolveAccountService) - allow(ResolveAccountService).to receive(:new).and_return(resolve_account_service_double) - allow(resolve_account_service_double).to receive(:call).with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } - allow(resolve_account_service_double).to receive(:call).with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } - - Import::RowWorker.drain - - expect(account.muting.map(&:acct)).to contain_exactly('muted@foo.bar', 'user@foo.bar', 'unknown@unknown.bar') + expect(account.muting.map(&:acct)) + .to contain_exactly('muted@foo.bar', 'user@foo.bar', 'unknown@unknown.bar') end end @@ -284,13 +219,11 @@ RSpec.describe BulkImportService do account.block_domain!('blocked.com') end - it 'blocks all the new domains' do + it 'blocks all the new domains and marks import finished' do subject.call(import) - expect(account.domain_blocks.pluck(:domain)).to contain_exactly('alreadyblocked.com', 'blocked.com', 'to-block.com') - end - it 'marks the import as finished' do - subject.call(import) + expect(account.domain_blocks.pluck(:domain)) + .to contain_exactly('alreadyblocked.com', 'blocked.com', 'to-block.com') expect(import.reload.state_finished?).to be true end end @@ -312,14 +245,13 @@ RSpec.describe BulkImportService do account.block_domain!('blocked.com') end - it 'blocks all the new domains' do + it 'blocks all the new domains and marks import finished' do subject.call(import) - expect(account.domain_blocks.pluck(:domain)).to contain_exactly('blocked.com', 'to-block.com') - end - it 'marks the import as finished' do - subject.call(import) - expect(import.reload.state_finished?).to be true + expect(account.domain_blocks.pluck(:domain)) + .to contain_exactly('blocked.com', 'to-block.com') + expect(import.reload.state_finished?) + .to be true end end @@ -347,22 +279,16 @@ RSpec.describe BulkImportService do account.bookmarks.create!(status: bookmarked) end - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows.map(&:id)) - end - - it 'updates the bookmarks as expected once the workers have run' do + it 'enqueues workers for the expected rows and updates bookmarks after worker run' do subject.call(import) - service_double = instance_double(ActivityPub::FetchRemoteStatusService) - allow(ActivityPub::FetchRemoteStatusService).to receive(:new).and_return(service_double) - allow(service_double).to receive(:call).with('https://domain.unknown/foo') { Fabricate(:status, uri: 'https://domain.unknown/foo') } - allow(service_double).to receive(:call).with('https://domain.unknown/private') { Fabricate(:status, uri: 'https://domain.unknown/private', visibility: :direct) } + expect(row_worker_job_args) + .to match_array(rows.map(&:id)) - Import::RowWorker.drain + stub_fetch_remote_and_drain_workers - expect(account.bookmarks.map { |bookmark| bookmark.status.uri }).to contain_exactly(already_bookmarked.uri, status.uri, bookmarked.uri, 'https://domain.unknown/foo') + expect(account.bookmarks.map { |bookmark| bookmark.status.uri }) + .to contain_exactly(already_bookmarked.uri, status.uri, bookmarked.uri, 'https://domain.unknown/foo') end end @@ -390,23 +316,48 @@ RSpec.describe BulkImportService do account.bookmarks.create!(status: bookmarked) end - it 'enqueues workers for the expected rows' do - subject.call(import) - expect(Import::RowWorker.jobs.pluck('args').flatten).to match_array(rows.map(&:id)) - end - - it 'updates the bookmarks as expected once the workers have run' do + it 'enqueues workers for the expected rows and updates bookmarks' do subject.call(import) - service_double = instance_double(ActivityPub::FetchRemoteStatusService) - allow(ActivityPub::FetchRemoteStatusService).to receive(:new).and_return(service_double) - allow(service_double).to receive(:call).with('https://domain.unknown/foo') { Fabricate(:status, uri: 'https://domain.unknown/foo') } - allow(service_double).to receive(:call).with('https://domain.unknown/private') { Fabricate(:status, uri: 'https://domain.unknown/private', visibility: :direct) } + expect(row_worker_job_args) + .to match_array(rows.map(&:id)) - Import::RowWorker.drain + stub_fetch_remote_and_drain_workers - expect(account.bookmarks.map { |bookmark| bookmark.status.uri }).to contain_exactly(status.uri, bookmarked.uri, 'https://domain.unknown/foo') + expect(account.bookmarks.map { |bookmark| bookmark.status.uri }) + .to contain_exactly(status.uri, bookmarked.uri, 'https://domain.unknown/foo') end end + + def row_worker_job_args + Import::RowWorker + .jobs + .pluck('args') + .flatten + end + + def stub_resolve_account_and_drain_workers + resolve_account_service_double = instance_double(ResolveAccountService) + allow(ResolveAccountService) + .to receive(:new) + .and_return(resolve_account_service_double) + allow(resolve_account_service_double) + .to receive(:call) + .with('user@foo.bar', any_args) { Fabricate(:account, username: 'user', domain: 'foo.bar', protocol: :activitypub) } + allow(resolve_account_service_double) + .to receive(:call) + .with('unknown@unknown.bar', any_args) { Fabricate(:account, username: 'unknown', domain: 'unknown.bar', protocol: :activitypub) } + + Import::RowWorker.drain + end + + def stub_fetch_remote_and_drain_workers + service_double = instance_double(ActivityPub::FetchRemoteStatusService) + allow(ActivityPub::FetchRemoteStatusService).to receive(:new).and_return(service_double) + allow(service_double).to receive(:call).with('https://domain.unknown/foo') { Fabricate(:status, uri: 'https://domain.unknown/foo') } + allow(service_double).to receive(:call).with('https://domain.unknown/private') { Fabricate(:status, uri: 'https://domain.unknown/private', visibility: :direct) } + + Import::RowWorker.drain + end end end diff --git a/spec/services/favourite_service_spec.rb b/spec/services/favourite_service_spec.rb index c39362def..123e8699c 100644 --- a/spec/services/favourite_service_spec.rb +++ b/spec/services/favourite_service_spec.rb @@ -11,11 +11,9 @@ RSpec.describe FavouriteService do let(:bob) { Fabricate(:account) } let(:status) { Fabricate(:status, account: bob) } - before do - subject.call(sender, status) - end - it 'creates a favourite' do + subject.call(sender, status) + expect(status.favourites.first).to_not be_nil end end @@ -26,15 +24,16 @@ RSpec.describe FavouriteService do before do stub_request(:post, 'http://example.com/inbox').to_return(status: 200, body: '', headers: {}) + end + + it 'creates a favourite and sends like activity', :inline_jobs do subject.call(sender, status) - end - it 'creates a favourite' do - expect(status.favourites.first).to_not be_nil - end + expect(status.favourites.first) + .to_not be_nil - it 'sends a like activity', :inline_jobs do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end end diff --git a/spec/services/follow_service_spec.rb b/spec/services/follow_service_spec.rb index 0c4cd6004..bbd8a6f99 100644 --- a/spec/services/follow_service_spec.rb +++ b/spec/services/follow_service_spec.rb @@ -143,15 +143,16 @@ RSpec.describe FollowService do before do stub_request(:post, 'http://example.com/inbox').to_return(status: 200, body: '', headers: {}) + end + + it 'creates follow request and sends an activity to inbox', :inline_jobs do subject.call(sender, bob) - end - it 'creates follow request' do - expect(FollowRequest.find_by(account: sender, target_account: bob)).to_not be_nil - end + expect(FollowRequest.find_by(account: sender, target_account: bob)) + .to_not be_nil - it 'sends a follow activity to the inbox', :inline_jobs do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end end diff --git a/spec/services/process_mentions_service_spec.rb b/spec/services/process_mentions_service_spec.rb index 2c202d3e5..3cc83d82f 100644 --- a/spec/services/process_mentions_service_spec.rb +++ b/spec/services/process_mentions_service_spec.rb @@ -16,20 +16,25 @@ RSpec.describe ProcessMentionsService do before do account.block!(individually_blocked_account) account.domain_blocks.create!(domain: domain_blocked_account.domain) - - subject.call(status) end - it 'creates a mention to the non-blocked account' do - expect(non_blocked_account.mentions.where(status: status).count).to eq 1 + it 'creates a mention to the non-blocked account but not the individually or domain blocked accounts' do + expect { subject.call(status) } + .to create_mention_for_non_blocked + .and skip_mention_for_individual + .and skip_mention_for_domain_blocked end - it 'does not create a mention to the individually blocked account' do - expect(individually_blocked_account.mentions.where(status: status).count).to eq 0 + def create_mention_for_non_blocked + change { non_blocked_account.mentions.where(status: status).count }.to(1) end - it 'does not create a mention to the domain-blocked account' do - expect(domain_blocked_account.mentions.where(status: status).count).to eq 0 + def skip_mention_for_individual + not_change { individually_blocked_account.mentions.where(status: status).count }.from(0) + end + + def skip_mention_for_domain_blocked + not_change { domain_blocked_account.mentions.where(status: status).count }.from(0) end end @@ -40,11 +45,9 @@ RSpec.describe ProcessMentionsService do context 'with a valid remote user' do let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } - before do - subject.call(status) - end - it 'creates a mention' do + subject.call(status) + expect(remote_user.mentions.where(status: status).count).to eq 1 end end @@ -53,11 +56,9 @@ RSpec.describe ProcessMentionsService do let!(:remote_user) { Fabricate(:account, username: 'remote_user', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') } let(:status) { Fabricate(:status, account: account, text: "Hello @#{remote_user.acct} @#{remote_user.acct} @#{remote_user.acct}", visibility: :public) } - before do - subject.call(status, save_records: false) - end - it 'creates exactly one mention' do + subject.call(status, save_records: false) + expect(status.mentions.size).to eq 1 end end @@ -66,11 +67,9 @@ RSpec.describe ProcessMentionsService do let!(:remote_user) { Fabricate(:account, username: 'sneak', protocol: :activitypub, domain: 'xn--hresiar-mxa.ch', inbox_url: 'http://example.com/inbox') } let!(:status) { Fabricate(:status, account: account, text: 'Hello @sneak@hæresiar.ch') } - before do - subject.call(status) - end - it 'creates a mention' do + subject.call(status) + expect(remote_user.mentions.where(status: status).count).to eq 1 end end @@ -79,11 +78,9 @@ RSpec.describe ProcessMentionsService do let!(:remote_user) { Fabricate(:account, username: 'foo', protocol: :activitypub, domain: 'xn--y9a3aq.xn--y9a3aq', inbox_url: 'http://example.com/inbox') } let!(:status) { Fabricate(:status, account: account, text: 'Hello @foo@հայ.հայ') } - before do - subject.call(status) - end - it 'creates a mention' do + subject.call(status) + expect(remote_user.mentions.where(status: status).count).to eq 1 end end @@ -95,10 +92,11 @@ RSpec.describe ProcessMentionsService do before do stub_request(:get, 'https://example.com/.well-known/host-meta').to_return(status: 404) stub_request(:get, 'https://example.com/.well-known/webfinger?resource=acct:remote_user@example.com').to_return(status: 500) - subject.call(status) end it 'creates a mention' do + subject.call(status) + expect(remote_user.mentions.where(status: status).count).to eq 1 end end diff --git a/spec/services/reject_follow_service_spec.rb b/spec/services/reject_follow_service_spec.rb index d2c7a0020..eec0d6c1e 100644 --- a/spec/services/reject_follow_service_spec.rb +++ b/spec/services/reject_follow_service_spec.rb @@ -10,17 +10,15 @@ RSpec.describe RejectFollowService do describe 'local' do let(:bob) { Fabricate(:account) } - before do - FollowRequest.create(account: bob, target_account: sender) + before { FollowRequest.create(account: bob, target_account: sender) } + + it 'removes follow request and does not create relation' do subject.call(bob, sender) - end - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'does not create follow relation' do - expect(bob.following?(sender)).to be false + expect(bob) + .to_not be_requested(sender) + expect(bob) + .to_not be_following(sender) end end @@ -30,19 +28,17 @@ RSpec.describe RejectFollowService do before do FollowRequest.create(account: bob, target_account: sender) stub_request(:post, bob.inbox_url).to_return(status: 200) + end + + it 'removes follow request, does not create relation, sends reject activity', :inline_jobs do subject.call(bob, sender) - end - it 'removes follow request' do - expect(bob.requested?(sender)).to be false - end - - it 'does not create follow relation' do - expect(bob.following?(sender)).to be false - end - - it 'sends a reject activity', :inline_jobs do - expect(a_request(:post, bob.inbox_url)).to have_been_made.once + expect(bob) + .to_not be_requested(sender) + expect(bob) + .to_not be_following(sender) + expect(a_request(:post, bob.inbox_url)) + .to have_been_made.once end end end diff --git a/spec/services/remove_from_followers_service_spec.rb b/spec/services/remove_from_followers_service_spec.rb index 515600096..381daf1a5 100644 --- a/spec/services/remove_from_followers_service_spec.rb +++ b/spec/services/remove_from_followers_service_spec.rb @@ -10,13 +10,13 @@ RSpec.describe RemoveFromFollowersService do describe 'local' do let(:sender) { Fabricate(:account, username: 'alice') } - before do - Follow.create(account: sender, target_account: bob) - subject.call(bob, sender) - end + before { Follow.create(account: sender, target_account: bob) } it 'does not create follow relation' do - expect(bob.followed_by?(sender)).to be false + subject.call(bob, sender) + + expect(bob) + .to_not be_followed_by(sender) end end @@ -26,15 +26,16 @@ RSpec.describe RemoveFromFollowersService do before do Follow.create(account: sender, target_account: bob) stub_request(:post, sender.inbox_url).to_return(status: 200) + end + + it 'does not create follow relation and sends reject activity', :inline_jobs do subject.call(bob, sender) - end - it 'does not create follow relation' do - expect(bob.followed_by?(sender)).to be false - end + expect(bob) + .to_not be_followed_by(sender) - it 'sends a reject activity', :inline_jobs do - expect(a_request(:post, sender.inbox_url)).to have_been_made.once + expect(a_request(:post, sender.inbox_url)) + .to have_been_made.once end end end diff --git a/spec/services/remove_status_service_spec.rb b/spec/services/remove_status_service_spec.rb index 08f519b53..f2b46f05b 100644 --- a/spec/services/remove_status_service_spec.rb +++ b/spec/services/remove_status_service_spec.rb @@ -28,42 +28,38 @@ RSpec.describe RemoveStatusService, :inline_jobs do Fabricate(:status, account: bill, reblog: status, uri: 'hoge') end - it 'removes status from author\'s home feed' do - subject.call(status) - expect(HomeFeed.new(alice).get(10).pluck(:id)).to_not include(status.id) - end - - it 'removes status from local follower\'s home feed' do - subject.call(status) - expect(HomeFeed.new(jeff).get(10).pluck(:id)).to_not include(status.id) - end - - it 'publishes to public media timeline' do + it 'removes status from notifications and from author and local follower home feeds, publishes to media timeline, sends delete activities' do allow(redis).to receive(:publish).with(any_args) - subject.call(status) + expect { subject.call(status) } + .to remove_status_from_notifications - expect(redis).to have_received(:publish).with('timeline:public:media', Oj.dump(event: :delete, payload: status.id.to_s)) - end + expect(home_feed_ids(alice)) + .to_not include(status.id) + expect(home_feed_ids(jeff)) + .to_not include(status.id) - it 'sends Delete activity to followers' do - subject.call(status) + expect(redis) + .to have_received(:publish).with('timeline:public:media', Oj.dump(event: :delete, payload: status.id.to_s)) expect(delete_delivery(hank, status)) .to have_been_made.once - end - - it 'sends Delete activity to rebloggers' do - subject.call(status) expect(delete_delivery(bill, status)) .to have_been_made.once end - it 'remove status from notifications' do - expect { subject.call(status) }.to change { - Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count - }.from(1).to(0) + def home_feed_ids(personage) + HomeFeed + .new(personage) + .get(10) + .pluck(:id) + end + + def remove_status_from_notifications + change { Notification.where(activity_type: 'Favourite', from_account: jeff, account: alice).count } + .from(1) + .to(0) end def delete_delivery(target, status) diff --git a/spec/services/report_service_spec.rb b/spec/services/report_service_spec.rb index 6518c5c27..4659e1c7a 100644 --- a/spec/services/report_service_spec.rb +++ b/spec/services/report_service_spec.rb @@ -31,14 +31,13 @@ RSpec.describe ReportService do context 'when forward is true', :inline_jobs do let(:forward) { true } - it 'sends ActivityPub payload when forward is true' do - subject.call(source_account, remote_account, forward: forward) - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made - end - - it 'has an uri' do + it 'has a URI and sends ActivityPub payload' do report = subject.call(source_account, remote_account, forward: forward) - expect(report.uri).to_not be_nil + + expect(report.uri) + .to_not be_nil + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made end context 'when reporting a reply on a different remote server' do @@ -122,13 +121,12 @@ RSpec.describe ReportService do status.mentions.create(account: source_account) end - it 'creates a report' do - expect { subject.call }.to change { target_account.targeted_reports.count }.from(0).to(1) - end + it 'creates a report and attaches the DM to the report' do + expect { subject.call } + .to change { target_account.targeted_reports.count }.from(0).to(1) - it 'attaches the DM to the report' do - subject.call - expect(target_account.targeted_reports.pluck(:status_ids)).to eq [[status.id]] + expect(target_account.targeted_reports.pluck(:status_ids)) + .to eq [[status.id]] end end @@ -146,13 +144,12 @@ RSpec.describe ReportService do status.mentions.create(account: source_account) end - it 'creates a report' do - expect { subject.call }.to change { target_account.targeted_reports.count }.from(0).to(1) - end + it 'creates a report and attaches DM to report' do + expect { subject.call } + .to change { target_account.targeted_reports.count }.from(0).to(1) - it 'attaches the DM to the report' do - subject.call - expect(target_account.targeted_reports.pluck(:status_ids)).to eq [[status.id]] + expect(target_account.targeted_reports.pluck(:status_ids)) + .to eq [[status.id]] end end diff --git a/spec/services/resolve_account_service_spec.rb b/spec/services/resolve_account_service_spec.rb index a856e019a..1bd4e9a8e 100644 --- a/spec/services/resolve_account_service_spec.rb +++ b/spec/services/resolve_account_service_spec.rb @@ -22,37 +22,38 @@ RSpec.describe ResolveAccountService do context 'when domain is banned' do before { Fabricate(:domain_block, domain: 'ap.example.com', severity: :suspend) } - it 'does not return an account' do - expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to be_nil - end - - it 'does not make a webfinger query' do - subject.call('foo@ap.example.com', skip_webfinger: true) - expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made + it 'does not return an account or make a webfinger query' do + expect(subject.call('foo@ap.example.com', skip_webfinger: true)) + .to be_nil + expect(webfinger_discovery_request) + .to_not have_been_made end end context 'when domain is not banned' do - it 'returns the expected account' do - expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to eq remote_account - end - - it 'does not make a webfinger query' do - subject.call('foo@ap.example.com', skip_webfinger: true) - expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made + it 'returns the expected account and does not make a webfinger query' do + expect(subject.call('foo@ap.example.com', skip_webfinger: true)) + .to eq remote_account + expect(webfinger_discovery_request) + .to_not have_been_made end end end context 'when account is not known' do - it 'does not return an account' do - expect(subject.call('foo@ap.example.com', skip_webfinger: true)).to be_nil + it 'does not return an account and does not make webfinger query' do + expect(subject.call('foo@ap.example.com', skip_webfinger: true)) + .to be_nil + expect(webfinger_discovery_request) + .to_not have_been_made end + end - it 'does not make a webfinger query' do - subject.call('foo@ap.example.com', skip_webfinger: true) - expect(a_request(:get, 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com')).to_not have_been_made - end + def webfinger_discovery_request + a_request( + :get, + 'https://ap.example.com/.well-known/webfinger?resource=acct:foo@ap.example.com' + ) end end @@ -84,13 +85,11 @@ RSpec.describe ResolveAccountService do allow(AccountDeletionWorker).to receive(:perform_async) end - it 'returns nil' do - expect(subject.call('hoge@example.com')).to be_nil - end - - it 'queues account deletion worker' do - subject.call('hoge@example.com') - expect(AccountDeletionWorker).to have_received(:perform_async) + it 'returns nil and queues deletion worker' do + expect(subject.call('hoge@example.com')) + .to be_nil + expect(AccountDeletionWorker) + .to have_received(:perform_async) end end @@ -110,9 +109,12 @@ RSpec.describe ResolveAccountService do it 'returns new remote account' do account = subject.call('Foo@redirected.example.com') - expect(account.activitypub?).to be true - expect(account.acct).to eq 'foo@ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' + expect(account) + .to have_attributes( + activitypub?: true, + acct: 'foo@ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox' + ) end end @@ -125,9 +127,12 @@ RSpec.describe ResolveAccountService do it 'returns new remote account' do account = subject.call('Foo@redirected.example.com') - expect(account.activitypub?).to be true - expect(account.acct).to eq 'foo@ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' + expect(account) + .to have_attributes( + activitypub?: true, + acct: 'foo@ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox' + ) end end @@ -161,9 +166,12 @@ RSpec.describe ResolveAccountService do it 'returns new remote account' do account = subject.call('foo@ap.example.com') - expect(account.activitypub?).to be true - expect(account.domain).to eq 'ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' + expect(account) + .to have_attributes( + activitypub?: true, + domain: 'ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox' + ) end context 'with multiple types' do @@ -174,10 +182,13 @@ RSpec.describe ResolveAccountService do it 'returns new remote account' do account = subject.call('foo@ap.example.com') - expect(account.activitypub?).to be true - expect(account.domain).to eq 'ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' - expect(account.actor_type).to eq 'Person' + expect(account) + .to have_attributes( + activitypub?: true, + domain: 'ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox', + actor_type: 'Person' + ) end end end @@ -186,20 +197,21 @@ RSpec.describe ResolveAccountService do let!(:duplicate) { Fabricate(:account, username: 'foo', domain: 'old.example.com', uri: 'https://ap.example.com/users/foo') } let!(:status) { Fabricate(:status, account: duplicate, text: 'foo') } - it 'returns new remote account' do + it 'returns new remote account and merges accounts', :inline_jobs do account = subject.call('foo@ap.example.com') - expect(account.activitypub?).to be true - expect(account.domain).to eq 'ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' - expect(account.uri).to eq 'https://ap.example.com/users/foo' - end + expect(account) + .to have_attributes( + activitypub?: true, + domain: 'ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox', + uri: 'https://ap.example.com/users/foo' + ) - it 'merges accounts', :inline_jobs do - account = subject.call('foo@ap.example.com') - - expect(status.reload.account_id).to eq account.id - expect(Account.where(uri: account.uri).count).to eq 1 + expect(status.reload.account_id) + .to eq account.id + expect(Account.where(uri: account.uri).count) + .to eq 1 end end @@ -210,11 +222,15 @@ RSpec.describe ResolveAccountService do it 'returns new remote account' do account = subject.call('foo@ap.example.com') - expect(account.activitypub?).to be true - expect(account.domain).to eq 'ap.example.com' - expect(account.inbox_url).to eq 'https://ap.example.com/users/foo/inbox' - expect(account.uri).to eq 'https://ap.example.com/users/foo' - expect(status.reload.account).to eq(account) + expect(account) + .to have_attributes( + activitypub?: true, + domain: 'ap.example.com', + inbox_url: 'https://ap.example.com/users/foo/inbox', + uri: 'https://ap.example.com/users/foo' + ) + expect(status.reload.account) + .to eq(account) end end diff --git a/spec/services/resolve_url_service_spec.rb b/spec/services/resolve_url_service_spec.rb index 80f2a5a4b..eaf00c1ed 100644 --- a/spec/services/resolve_url_service_spec.rb +++ b/spec/services/resolve_url_service_spec.rb @@ -51,12 +51,11 @@ RSpec.describe ResolveURLService do let(:url) { 'https://example.com/@foo/42' } let(:uri) { 'https://example.com/users/foo/statuses/42' } - it 'returns status by url' do - expect(subject.call(url, on_behalf_of: account)).to eq(status) - end - - it 'returns status by uri' do - expect(subject.call(uri, on_behalf_of: account)).to eq(status) + it 'returns status by URL or URI' do + expect(subject.call(url, on_behalf_of: account)) + .to eq(status) + expect(subject.call(uri, on_behalf_of: account)) + .to eq(status) end end @@ -75,12 +74,11 @@ RSpec.describe ResolveURLService do let(:url) { 'https://example.com/@foo/42' } let(:uri) { 'https://example.com/users/foo/statuses/42' } - it 'does not return the status by url' do - expect(subject.call(url, on_behalf_of: account)).to be_nil - end - - it 'does not return the status by uri' do - expect(subject.call(uri, on_behalf_of: account)).to be_nil + it 'does not return the status by URL or URI' do + expect(subject.call(url, on_behalf_of: account)) + .to be_nil + expect(subject.call(uri, on_behalf_of: account)) + .to be_nil end end @@ -107,22 +105,20 @@ RSpec.describe ResolveURLService do account.follow!(poster) end - it 'returns status by url' do - expect(subject.call(url, on_behalf_of: account)).to eq(status) - end - - it 'returns status by uri' do - expect(subject.call(uri, on_behalf_of: account)).to eq(status) + it 'returns status by URL or URI' do + expect(subject.call(url, on_behalf_of: account)) + .to eq(status) + expect(subject.call(uri, on_behalf_of: account)) + .to eq(status) end end context 'when the account does not follow the poster' do - it 'does not return the status by url' do - expect(subject.call(url, on_behalf_of: account)).to be_nil - end - - it 'does not return the status by uri' do - expect(subject.call(uri, on_behalf_of: account)).to be_nil + it 'does not return the status by URL or URI' do + expect(subject.call(url, on_behalf_of: account)) + .to be_nil + expect(subject.call(uri, on_behalf_of: account)) + .to be_nil end end end diff --git a/spec/services/translate_status_service_spec.rb b/spec/services/translate_status_service_spec.rb index 0779fbbe6..cd92fb8d1 100644 --- a/spec/services/translate_status_service_spec.rb +++ b/spec/services/translate_status_service_spec.rb @@ -32,20 +32,14 @@ RSpec.describe TranslateStatusService do allow(TranslationService).to receive_messages(configured?: true, configured: translation_service) end - it 'returns translated status content' do - expect(service.call(status, 'es').content).to eq '

Hola

' - end - - it 'returns source language' do - expect(service.call(status, 'es').detected_source_language).to eq 'en' - end - - it 'returns translation provider' do - expect(service.call(status, 'es').provider).to eq 'Dummy' - end - - it 'returns original status' do - expect(service.call(status, 'es').status).to eq status + it 'returns translated status content and source language and provider and original status' do + expect(service.call(status, 'es')) + .to have_attributes( + content: '

Hola

', + detected_source_language: 'en', + provider: 'Dummy', + status: status + ) end describe 'status has content with custom emoji' do @@ -155,26 +149,16 @@ RSpec.describe TranslateStatusService do let!(:source_texts) { service.send(:source_texts) } it 'returns formatted poll options' do - expect(source_texts.size).to eq 3 - expect(source_texts.values).to eq %w(

Hello

Blue Green) - end - - it 'has a first key with content' do - expect(source_texts.keys.first).to eq :content - end - - it 'has the first option in the second key with correct options' do - option1 = source_texts.keys.second - expect(option1).to be_a Poll::Option - expect(option1.id).to eq '0' - expect(option1.title).to eq 'Blue' - end - - it 'has the second option in the third key with correct options' do - option2 = source_texts.keys.third - expect(option2).to be_a Poll::Option - expect(option2.id).to eq '1' - expect(option2.title).to eq 'Green' + expect(source_texts) + .to have_attributes( + size: 3, + values: %w(

Hello

Blue Green), + keys: contain_exactly( + eq(:content), + be_a(Poll::Option).and(have_attributes(id: '0', title: 'Blue')), + be_a(Poll::Option).and(have_attributes(id: '1', title: 'Green')) + ) + ) end end end diff --git a/spec/services/unblock_domain_service_spec.rb b/spec/services/unblock_domain_service_spec.rb index 405fe1cfd..daa1d480a 100644 --- a/spec/services/unblock_domain_service_spec.rb +++ b/spec/services/unblock_domain_service_spec.rb @@ -12,26 +12,32 @@ RSpec.describe UnblockDomainService do let!(:silenced) { Fabricate(:account, domain: 'example.com', silenced_at: domain_block.created_at) } let!(:suspended) { Fabricate(:account, domain: 'example.com', suspended_at: domain_block.created_at) } - it 'unsilences accounts and removes block' do - domain_block.update(severity: :silence) + context 'with severity of silence' do + before { domain_block.update(severity: :silence) } - subject.call(domain_block) - expect_deleted_domain_block - expect(silenced.reload.silenced?).to be false - expect(suspended.reload.suspended?).to be true - expect(independently_suspended.reload.suspended?).to be true - expect(independently_silenced.reload.silenced?).to be true + it 'unsilences accounts and removes block' do + subject.call(domain_block) + + expect_deleted_domain_block + expect(silenced.reload.silenced?).to be false + expect(suspended.reload.suspended?).to be true + expect(independently_suspended.reload.suspended?).to be true + expect(independently_silenced.reload.silenced?).to be true + end end - it 'unsuspends accounts and removes block' do - domain_block.update(severity: :suspend) + context 'with severity of suspend' do + before { domain_block.update(severity: :suspend) } - subject.call(domain_block) - expect_deleted_domain_block - expect(suspended.reload.suspended?).to be false - expect(silenced.reload.silenced?).to be false - expect(independently_suspended.reload.suspended?).to be true - expect(independently_silenced.reload.silenced?).to be true + it 'unsuspends accounts and removes block' do + subject.call(domain_block) + + expect_deleted_domain_block + expect(suspended.reload.suspended?).to be false + expect(silenced.reload.silenced?).to be false + expect(independently_suspended.reload.suspended?).to be true + expect(independently_silenced.reload.silenced?).to be true + end end end diff --git a/spec/services/unblock_service_spec.rb b/spec/services/unblock_service_spec.rb index 6132e7441..a2c5188f0 100644 --- a/spec/services/unblock_service_spec.rb +++ b/spec/services/unblock_service_spec.rb @@ -10,13 +10,13 @@ RSpec.describe UnblockService do describe 'local' do let(:bob) { Fabricate(:account) } - before do - sender.block!(bob) - subject.call(sender, bob) - end + before { sender.block!(bob) } it 'destroys the blocking relation' do - expect(sender.blocking?(bob)).to be false + subject.call(sender, bob) + + expect(sender) + .to_not be_blocking(bob) end end @@ -26,15 +26,15 @@ RSpec.describe UnblockService do before do sender.block!(bob) stub_request(:post, 'http://example.com/inbox').to_return(status: 200) + end + + it 'destroys the blocking relation and sends unblock activity', :inline_jobs do subject.call(sender, bob) - end - it 'destroys the blocking relation' do - expect(sender.blocking?(bob)).to be false - end - - it 'sends an unblock activity', :inline_jobs do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(sender) + .to_not be_blocking(bob) + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end end diff --git a/spec/services/unfollow_service_spec.rb b/spec/services/unfollow_service_spec.rb index 0c206c4b9..6cf24ca5e 100644 --- a/spec/services/unfollow_service_spec.rb +++ b/spec/services/unfollow_service_spec.rb @@ -10,13 +10,13 @@ RSpec.describe UnfollowService do describe 'local' do let(:bob) { Fabricate(:account, username: 'bob') } - before do - sender.follow!(bob) - subject.call(sender, bob) - end + before { sender.follow!(bob) } it 'destroys the following relation' do - expect(sender.following?(bob)).to be false + subject.call(sender, bob) + + expect(sender) + .to_not be_following(bob) end end @@ -26,15 +26,15 @@ RSpec.describe UnfollowService do before do sender.follow!(bob) stub_request(:post, 'http://example.com/inbox').to_return(status: 200) + end + + it 'destroys the following relation and sends unfollow activity' do subject.call(sender, bob) - end - it 'destroys the following relation' do - expect(sender.following?(bob)).to be false - end - - it 'sends an unfollow activity' do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(sender) + .to_not be_following(bob) + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end @@ -44,15 +44,15 @@ RSpec.describe UnfollowService do before do bob.follow!(sender) stub_request(:post, 'http://example.com/inbox').to_return(status: 200) + end + + it 'destroys the following relation and sends a reject activity' do subject.call(bob, sender) - end - it 'destroys the following relation' do - expect(bob.following?(sender)).to be false - end - - it 'sends a reject activity' do - expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once + expect(sender) + .to_not be_following(bob) + expect(a_request(:post, 'http://example.com/inbox')) + .to have_been_made.once end end end diff --git a/spec/services/update_account_service_spec.rb b/spec/services/update_account_service_spec.rb index d066db481..f9059af07 100644 --- a/spec/services/update_account_service_spec.rb +++ b/spec/services/update_account_service_spec.rb @@ -18,23 +18,19 @@ RSpec.describe UpdateAccountService do FollowService.new.call(alice, account) FollowService.new.call(bob, account) FollowService.new.call(eve, account) + end + it 'auto accepts pending follow requests from appropriate accounts' do subject.call(account, { locked: false }) - end - it 'auto-accepts pending follow requests' do - expect(alice.following?(account)).to be true - expect(alice.requested?(account)).to be false - end + expect(alice).to be_following(account) + expect(alice).to_not be_requested(account) - it 'does not auto-accept pending follow requests from silenced users' do - expect(bob.following?(account)).to be false - expect(bob.requested?(account)).to be true - end + expect(bob).to_not be_following(account) + expect(bob).to be_requested(account) - it 'auto-accepts pending follow requests from muted users so as to not leak mute' do - expect(eve.following?(account)).to be true - expect(eve.requested?(account)).to be false + expect(eve).to be_following(account) + expect(eve).to_not be_requested(account) end end end diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index de06fb13c..7c92adeff 100644 --- a/spec/services/update_status_service_spec.rb +++ b/spec/services/update_status_service_spec.rb @@ -10,15 +10,15 @@ RSpec.describe UpdateStatusService do before do allow(ActivityPub::DistributionWorker).to receive(:perform_async) + end + + it 'does not create an edit or notify anyone' do subject.call(status, status.account_id, text: 'Foo') - end - it 'does not create an edit' do - expect(status.reload.edits).to be_empty - end - - it 'does not notify anyone' do - expect(ActivityPub::DistributionWorker).to_not have_received(:perform_async) + expect(status.reload.edits) + .to be_empty + expect(ActivityPub::DistributionWorker) + .to_not have_received(:perform_async) end end @@ -28,18 +28,16 @@ RSpec.describe UpdateStatusService do before do PreviewCardsStatus.create(status: status, preview_card: preview_card) + end + + it 'updates text, resets card, saves edit history' do subject.call(status, status.account_id, text: 'Bar') - end - it 'updates text' do - expect(status.reload.text).to eq 'Bar' - end - - it 'resets preview card' do - expect(status.reload.preview_card).to be_nil - end - - it 'saves edit history' do + expect(status.reload) + .to have_attributes( + text: 'Bar', + preview_card: be_nil + ) expect(status.edits.ordered.pluck(:text)).to eq %w(Foo Bar) end end @@ -50,15 +48,15 @@ RSpec.describe UpdateStatusService do before do PreviewCardsStatus.create(status: status, preview_card: preview_card) + end + + it 'updates content warning and saves history' do subject.call(status, status.account_id, text: 'Foo', spoiler_text: 'Bar') - end - it 'updates content warning' do - expect(status.reload.spoiler_text).to eq 'Bar' - end - - it 'saves edit history' do - expect(status.edits.ordered.pluck(:text, :spoiler_text)).to eq [['Foo', ''], ['Foo', 'Bar']] + expect(status.reload.spoiler_text) + .to eq 'Bar' + expect(status.edits.ordered.pluck(:text, :spoiler_text)) + .to eq [['Foo', ''], ['Foo', 'Bar']] end end @@ -69,23 +67,19 @@ RSpec.describe UpdateStatusService do before do status.media_attachments << detached_media_attachment + end + + it 'updates media attachments, handles attachments, saves history' do subject.call(status, status.account_id, text: 'Foo', media_ids: [attached_media_attachment.id.to_s]) - end - it 'updates media attachments' do - expect(status.ordered_media_attachments).to eq [attached_media_attachment] - end - - it 'does not detach detached media attachments' do - expect(detached_media_attachment.reload.status_id).to eq status.id - end - - it 'attaches attached media attachments' do - expect(attached_media_attachment.reload.status_id).to eq status.id - end - - it 'saves edit history' do - expect(status.edits.ordered.pluck(:ordered_media_attachment_ids)).to eq [[detached_media_attachment.id], [attached_media_attachment.id]] + expect(status.ordered_media_attachments) + .to eq [attached_media_attachment] + expect(detached_media_attachment.reload.status_id) + .to eq status.id + expect(attached_media_attachment.reload.status_id) + .to eq status.id + expect(status.edits.ordered.pluck(:ordered_media_attachment_ids)) + .to eq [[detached_media_attachment.id], [attached_media_attachment.id]] end end @@ -95,19 +89,18 @@ RSpec.describe UpdateStatusService do before do status.media_attachments << media_attachment + end + + it 'does not detach media attachment, updates description, and saves history' do subject.call(status, status.account_id, text: 'Foo', media_ids: [media_attachment.id.to_s], media_attributes: [{ id: media_attachment.id, description: 'New description' }]) - end - it 'does not detach media attachment' do - expect(media_attachment.reload.status_id).to eq status.id - end - - it 'updates the media attachment description' do - expect(media_attachment.reload.description).to eq 'New description' - end - - it 'saves edit history' do - expect(status.edits.ordered.map { |edit| edit.ordered_media_attachments.map(&:description) }).to eq [['Old description'], ['New description']] + expect(media_attachment.reload) + .to have_attributes( + status_id: status.id, + description: 'New description' + ) + expect(status.edits.ordered.map { |edit| edit.ordered_media_attachments.map(&:description) }) + .to eq [['Old description'], ['New description']] end end @@ -120,28 +113,27 @@ RSpec.describe UpdateStatusService do before do status.update(poll: poll) VoteService.new.call(voter, poll, [0]) + end + + it 'updates poll, resets votes, saves history, requeues notifications' do subject.call(status, status.account_id, text: 'Foo', poll: { options: %w(Bar Baz Foo), expires_in: 5.days.to_i }) - end - it 'updates poll' do poll = status.poll.reload - expect(poll.options).to eq %w(Bar Baz Foo) - end - it 'resets votes' do - poll = status.poll.reload - expect(poll.votes_count).to eq 0 - expect(poll.votes.count).to eq 0 - expect(poll.cached_tallies).to eq [0, 0, 0] - end + expect(poll) + .to have_attributes( + options: %w(Bar Baz Foo), + votes_count: 0, + cached_tallies: [0, 0, 0] + ) + expect(poll.votes.count) + .to eq(0) - it 'saves edit history' do - expect(status.edits.ordered.pluck(:poll_options)).to eq [%w(Foo Bar), %w(Bar Baz Foo)] - end + expect(status.edits.ordered.pluck(:poll_options)) + .to eq [%w(Foo Bar), %w(Bar Baz Foo)] - it 'requeues expiration notification' do - poll = status.poll.reload - expect(PollExpirationNotifyWorker).to have_enqueued_sidekiq_job(poll.id).at(poll.expires_at + 5.minutes) + expect(PollExpirationNotifyWorker) + .to have_enqueued_sidekiq_job(poll.id).at(poll.expires_at + 5.minutes) end end @@ -151,16 +143,13 @@ RSpec.describe UpdateStatusService do let!(:bob) { Fabricate(:account, username: 'bob') } let!(:status) { PostStatusService.new.call(account, text: 'Hello @alice') } - before do + it 'changes mentions and keeps old as silent' do subject.call(status, status.account_id, text: 'Hello @bob') - end - it 'changes mentions' do - expect(status.active_mentions.pluck(:account_id)).to eq [bob.id] - end - - it 'keeps old mentions as silent mentions' do - expect(status.mentions.pluck(:account_id)).to contain_exactly(alice.id, bob.id) + expect(status.active_mentions.pluck(:account_id)) + .to eq [bob.id] + expect(status.mentions.pluck(:account_id)) + .to contain_exactly(alice.id, bob.id) end end @@ -168,11 +157,9 @@ RSpec.describe UpdateStatusService do let!(:account) { Fabricate(:account) } let!(:status) { PostStatusService.new.call(account, text: 'Hello #foo') } - before do - subject.call(status, status.account_id, text: 'Hello #bar') - end - it 'changes tags' do + subject.call(status, status.account_id, text: 'Hello #bar') + expect(status.tags.pluck(:name)).to eq %w(bar) end end