Fix possible race conditions when suspending/unsuspending accounts (#22363) (#23482)

* Fix possible race conditions when suspending/unsuspending accounts

* Fix tests

Tests were assuming SuspensionWorker and UnsuspensionWorker would do the
suspending/unsuspending themselves, but this has changed.
pull/23493/head
Claire 2023-02-09 20:57:06 +01:00 committed by GitHub
parent cc233af129
commit 0ca877f084
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 18 additions and 19 deletions

View File

@ -3,10 +3,13 @@
class SuspendAccountService < BaseService class SuspendAccountService < BaseService
include Payloadable include Payloadable
# Carry out the suspension of a recently-suspended account
# @param [Account] account Account to suspend
def call(account) def call(account)
return unless account.suspended?
@account = account @account = account
suspend!
reject_remote_follows! reject_remote_follows!
distribute_update_actor! distribute_update_actor!
unmerge_from_home_timelines! unmerge_from_home_timelines!
@ -16,10 +19,6 @@ class SuspendAccountService < BaseService
private private
def suspend!
@account.suspend! unless @account.suspended?
end
def reject_remote_follows! def reject_remote_follows!
return if @account.local? || !@account.activitypub? return if @account.local? || !@account.activitypub?

View File

@ -2,10 +2,12 @@
class UnsuspendAccountService < BaseService class UnsuspendAccountService < BaseService
include Payloadable include Payloadable
# Restores a recently-unsuspended account
# @param [Account] account Account to restore
def call(account) def call(account)
@account = account @account = account
unsuspend!
refresh_remote_account! refresh_remote_account!
return if @account.nil? || @account.suspended? return if @account.nil? || @account.suspended?
@ -18,10 +20,6 @@ class UnsuspendAccountService < BaseService
private private
def unsuspend!
@account.unsuspend! if @account.suspended?
end
def refresh_remote_account! def refresh_remote_account!
return if @account.local? return if @account.local?

View File

@ -13,6 +13,8 @@ RSpec.describe SuspendAccountService, type: :service do
local_follower.follow!(account) local_follower.follow!(account)
list.accounts << account list.accounts << account
account.suspend!
end end
it "unmerges from local followers' feeds" do it "unmerges from local followers' feeds" do
@ -21,8 +23,8 @@ RSpec.describe SuspendAccountService, type: :service do
expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list)
end end
it 'marks account as suspended' do it 'does not change the “suspended” flag' do
expect { subject }.to change { account.suspended? }.from(false).to(true) expect { subject }.to_not change { account.suspended? }
end end
end end

View File

@ -14,7 +14,7 @@ RSpec.describe UnsuspendAccountService, type: :service do
local_follower.follow!(account) local_follower.follow!(account)
list.accounts << account list.accounts << account
account.suspend!(origin: :local) account.unsuspend!
end end
end end
@ -30,8 +30,8 @@ RSpec.describe UnsuspendAccountService, type: :service do
stub_request(:post, 'https://bob.com/inbox').to_return(status: 201) stub_request(:post, 'https://bob.com/inbox').to_return(status: 201)
end end
it 'marks account as unsuspended' do it 'does not change the “suspended” flag' do
expect { subject }.to change { account.suspended? }.from(true).to(false) expect { subject }.to_not change { account.suspended? }
end end
include_examples 'common behavior' do include_examples 'common behavior' do
@ -83,8 +83,8 @@ RSpec.describe UnsuspendAccountService, type: :service do
expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list) expect(FeedManager.instance).to have_received(:merge_into_list).with(account, list)
end end
it 'marks account as unsuspended' do it 'does not change the “suspended” flag' do
expect { subject }.to change { account.suspended? }.from(true).to(false) expect { subject }.to_not change { account.suspended? }
end end
end end
@ -107,8 +107,8 @@ RSpec.describe UnsuspendAccountService, type: :service do
expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list) expect(FeedManager.instance).to_not have_received(:merge_into_list).with(account, list)
end end
it 'does not mark the account as unsuspended' do it 'marks account as suspended' do
expect { subject }.not_to change { account.suspended? } expect { subject }.to change { account.suspended? }.from(false).to(true)
end end
end end