diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 17b0a33a0a0c747cf7efed6142a7fbcb29cb4ef3..5e2e90892771627733e739147d8caf89b53ed196 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1410,13 +1410,6 @@ Rails/SkipsModelValidations: - 'spec/services/follow_service_spec.rb' - 'spec/services/update_account_service_spec.rb' -Rails/TransactionExitStatement: - Exclude: - - 'app/lib/activitypub/activity/announce.rb' - - 'app/lib/activitypub/activity/create.rb' - - 'app/lib/activitypub/activity/delete.rb' - - 'app/services/activitypub/process_account_service.rb' - # Configuration parameters: Include. # Include: app/models/**/*.rb Rails/UniqueValidationWithoutIndex: diff --git a/app/controllers/media_proxy_controller.rb b/app/controllers/media_proxy_controller.rb index 1b5486c122650c72cb5387fd8d585d757fafd810..8d480d704e22fcc0997bd2f6edf2c225abc4655a 100644 --- a/app/controllers/media_proxy_controller.rb +++ b/app/controllers/media_proxy_controller.rb @@ -16,7 +16,7 @@ class MediaProxyController < ApplicationController rescue_from HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError, with: :internal_server_error def show - with_lock("media_download:#{params[:id]}") do + with_redis_lock("media_download:#{params[:id]}") do @media_attachment = MediaAttachment.remote.attached.find(params[:id]) authorize @media_attachment.status, :show? redownload! if @media_attachment.needs_redownload? && !reject_media? diff --git a/app/controllers/settings/exports_controller.rb b/app/controllers/settings/exports_controller.rb index deaa7940eb7db5b89233bcc2eb556571d2083d6c..46a340aeb3dfd4ae08ff23ccef74af63df8524af 100644 --- a/app/controllers/settings/exports_controller.rb +++ b/app/controllers/settings/exports_controller.rb @@ -15,7 +15,7 @@ class Settings::ExportsController < Settings::BaseController def create backup = nil - with_lock("backup:#{current_user.id}") do + with_redis_lock("backup:#{current_user.id}") do authorize :backup, :create? backup = current_user.backups.create! end diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index e6674be8ae0d8a51a6895ae1c4420cf2768ced98..9dcafff3abc71a208dd8177ba6ff7cf22bd53b79 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -4,7 +4,7 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def perform return reject_payload! if delete_arrived_first?(@json['id']) || !related_to_local_activity? - with_lock("announce:#{value_or_id(@object)}") do + with_redis_lock("announce:#{value_or_id(@object)}") do original_status = status_from_object return reject_payload! if original_status.nil? || !announceable?(original_status) diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 23fbabf4cc1d13bc1e1fbf048090b7e7f25a582c..28cea8ec04a64af4f57de92717373b16a4047f2a 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -47,7 +47,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def create_status return reject_payload! if unsupported_object_type? || non_matching_uri_hosts?(@account.uri, object_uri) || tombstone_exists? || !related_to_local_activity? - with_lock("create:#{object_uri}") do + with_redis_lock("create:#{object_uri}") do return if delete_arrived_first?(object_uri) || poll_vote? @status = find_existing_status @@ -313,7 +313,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity poll = replied_to_status.preloadable_poll already_voted = true - with_lock("vote:#{replied_to_status.poll_id}:#{@account.id}") do + with_redis_lock("vote:#{replied_to_status.poll_id}:#{@account.id}") do already_voted = poll.votes.where(account: @account).exists? poll.votes.create!(account: @account, choice: poll.options.index(@object['name']), uri: object_uri) end diff --git a/app/lib/activitypub/activity/delete.rb b/app/lib/activitypub/activity/delete.rb index 3af500f2b195088199ed5562cdccde373109f927..61f6ca699775c2edfab3325af03c09e9b14024ba 100644 --- a/app/lib/activitypub/activity/delete.rb +++ b/app/lib/activitypub/activity/delete.rb @@ -12,7 +12,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity private def delete_person - with_lock("delete_in_progress:#{@account.id}", autorelease: 2.hours, raise_on_failure: false) do + with_redis_lock("delete_in_progress:#{@account.id}", autorelease: 2.hours, raise_on_failure: false) do DeleteAccountService.new.call(@account, reserve_username: false, skip_activitypub: true) end end @@ -20,14 +20,14 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity def delete_note return if object_uri.nil? - with_lock("delete_status_in_progress:#{object_uri}", raise_on_failure: false) do + with_redis_lock("delete_status_in_progress:#{object_uri}", raise_on_failure: false) do unless non_matching_uri_hosts?(@account.uri, object_uri) # This lock ensures a concurrent `ActivityPub::Activity::Create` either # does not create a status at all, or has finished saving it to the # database before we try to load it. # Without the lock, `delete_later!` could be called after `delete_arrived_first?` # and `Status.find` before `Status.create!` - with_lock("create:#{object_uri}") { delete_later!(object_uri) } + with_redis_lock("create:#{object_uri}") { delete_later!(object_uri) } Tombstone.find_or_create_by(uri: object_uri, account: @account) end diff --git a/app/models/account_migration.rb b/app/models/account_migration.rb index fa8cb6013ccaa7ee112a774ed3b129e5f3c334c0..b9da596172b5e1b291327b19682aeec085cb50e9 100644 --- a/app/models/account_migration.rb +++ b/app/models/account_migration.rb @@ -42,7 +42,7 @@ class AccountMigration < ApplicationRecord return false unless errors.empty? - with_lock("account_migration:#{account.id}") do + with_redis_lock("account_migration:#{account.id}") do save end end diff --git a/app/models/concerns/lockable.rb b/app/models/concerns/lockable.rb index 55a9714ca84f37cd4efb568f5050b1e1601e3df9..3354ce1a99acc3d48f5a98e5c63e4410e4d9585f 100644 --- a/app/models/concerns/lockable.rb +++ b/app/models/concerns/lockable.rb @@ -5,7 +5,7 @@ module Lockable # @param [ActiveSupport::Duration] autorelease Automatically release the lock after this time # @param [Boolean] raise_on_failure Raise an error if a lock cannot be acquired, or fail silently # @raise [Mastodon::RaceConditionError] - def with_lock(lock_name, autorelease: 15.minutes, raise_on_failure: true) + def with_redis_lock(lock_name, autorelease: 15.minutes, raise_on_failure: true) with_redis do |redis| RedisLock.acquire(redis: redis, key: "lock:#{lock_name}", autorelease: autorelease.seconds) do |lock| if lock.acquired? diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index 603e4cf48b3d13710d8259e977e87ecd20f54eeb..ca0083b167dc8b09c390320fca09883c9c596e43 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -24,7 +24,7 @@ class ActivityPub::ProcessAccountService < BaseService # The key does not need to be unguessable, it just needs to be somewhat unique @options[:request_id] ||= "#{Time.now.utc.to_i}-#{username}@#{domain}" - with_lock("process_account:#{@uri}") do + with_redis_lock("process_account:#{@uri}") do @account = Account.remote.find_by(uri: @uri) if @options[:only_key] @account ||= Account.find_remote(@username, @domain) @old_public_key = @account&.public_key diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index ac7372f745e56a6be75ccd6a3e6348880213fe32..38f6bf25149398909527f4d4305a64310ea60e47 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -35,7 +35,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService last_edit_date = @status.edited_at.presence || @status.created_at # Only allow processing one create/update per status at a time - with_lock("create:#{@uri}") do + with_redis_lock("create:#{@uri}") do Status.transaction do record_previous_edit! update_media_attachments! @@ -58,7 +58,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def handle_implicit_update! - with_lock("create:#{@uri}") do + with_redis_lock("create:#{@uri}") do update_poll!(allow_significant_changes: false) queue_poll_notifications! end diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 8d07958b7335df7ed3dfd0c489288f4fe15b72bc..9c56c862ecebf9aee26546401fb5be78d0699dfd 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -23,7 +23,7 @@ class FetchLinkCardService < BaseService @url = @original_url.to_s - with_lock("fetch:#{@original_url}") do + with_redis_lock("fetch:#{@original_url}") do @card = PreviewCard.find_by(url: @url) process_url if @card.nil? || @card.updated_at <= 2.weeks.ago || @card.missing_image? end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index ea799db57f2b671594839afe30783cc8a2ea10f0..25da2c6eb46f0f5a0b5171c7a48fcc269ff90209 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -18,7 +18,7 @@ class RemoveStatusService < BaseService @account = status.account @options = options - with_lock("distribute:#{@status.id}") do + with_redis_lock("distribute:#{@status.id}") do @status.discard_with_reblogs StatusPin.find_by(status: @status)&.destroy diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb index abe1534a55140d2a736a491d7cb226630391e1d2..870d67ec8daced1c5721b8899a227416da33232e 100644 --- a/app/services/resolve_account_service.rb +++ b/app/services/resolve_account_service.rb @@ -106,7 +106,7 @@ class ResolveAccountService < BaseService def fetch_account! return unless activitypub_ready? - with_lock("resolve:#{@username}@#{@domain}") do + with_redis_lock("resolve:#{@username}@#{@domain}") do @account = ActivityPub::FetchRemoteAccountService.new.call(actor_url, suppress_errors: @options[:suppress_errors]) end diff --git a/app/services/unfollow_service.rb b/app/services/unfollow_service.rb index d83a60e4e72a8f7d42904e7748e8755de8ac0f95..fe9a7f0d87ccd39db453e7b3891000af0b7cf702 100644 --- a/app/services/unfollow_service.rb +++ b/app/services/unfollow_service.rb @@ -15,7 +15,7 @@ class UnfollowService < BaseService @target_account = target_account @options = options - with_lock("relationship:#{[source_account.id, target_account.id].sort.join(':')}") do + with_redis_lock("relationship:#{[source_account.id, target_account.id].sort.join(':')}") do unfollow! || undo_follow_request! end end diff --git a/app/services/vote_service.rb b/app/services/vote_service.rb index 9ebf5a98d968b0d021a36d693a7c0b4a084e4e87..3e92a1690a63deeae6362c1485dbc933cde87c47 100644 --- a/app/services/vote_service.rb +++ b/app/services/vote_service.rb @@ -18,7 +18,7 @@ class VoteService < BaseService already_voted = true - with_lock("vote:#{@poll.id}:#{@account.id}") do + with_redis_lock("vote:#{@poll.id}:#{@account.id}") do already_voted = @poll.votes.where(account: @account).exists? ApplicationRecord.transaction do diff --git a/app/workers/distribution_worker.rb b/app/workers/distribution_worker.rb index 59cdbc7b25c6d0c84d97b1845d946e1d81592d13..1d58e53e941325557310b037ac22c9e42fd29850 100644 --- a/app/workers/distribution_worker.rb +++ b/app/workers/distribution_worker.rb @@ -6,7 +6,7 @@ class DistributionWorker include Lockable def perform(status_id, options = {}) - with_lock("distribute:#{status_id}") do + with_redis_lock("distribute:#{status_id}") do FanOutOnWriteService.new.call(Status.find(status_id), **options.symbolize_keys) end rescue ActiveRecord::RecordNotFound