Fix Sidekiq warnings about JSON serialization (#17381)
* Fix Sidekiq warnings about JSON serialization This occurs on every symbol argument we pass, and every symbol key in hashes, because Sidekiq expects strings instead. See https://github.com/mperham/sidekiq/pull/5071 We do not need to change how workers parse their arguments because this has not changed and we were already converting to symbols adequately or using `with_indifferent_access`. * Set Sidekiq to raise on unsafe arguments in test mode In order to more easily catch issues that would produce warnings in production code.
This commit is contained in:
parent
14c69a535b
commit
03d59340da
16 changed files with 25 additions and 22 deletions
|
@ -56,7 +56,7 @@ class Api::V1::StatusesController < Api::BaseController
|
||||||
authorize @status, :destroy?
|
authorize @status, :destroy?
|
||||||
|
|
||||||
@status.discard
|
@status.discard
|
||||||
RemovalWorker.perform_async(@status.id, redraft: true)
|
RemovalWorker.perform_async(@status.id, { 'redraft' => true })
|
||||||
@status.account.statuses_count = @status.account.statuses_count - 1
|
@status.account.statuses_count = @status.account.statuses_count - 1
|
||||||
|
|
||||||
render json: @status, serializer: REST::StatusSerializer, source_requested: true
|
render json: @status, serializer: REST::StatusSerializer, source_requested: true
|
||||||
|
|
|
@ -94,7 +94,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
|
||||||
LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id)
|
LinkCrawlWorker.perform_in(rand(1..59).seconds, @status.id)
|
||||||
|
|
||||||
# Distribute into home and list feeds and notify mentioned accounts
|
# Distribute into home and list feeds and notify mentioned accounts
|
||||||
::DistributionWorker.perform_async(@status.id, silenced_account_ids: @silenced_account_ids) if @options[:override_timestamps] || @status.within_realtime_window?
|
::DistributionWorker.perform_async(@status.id, { 'silenced_account_ids' => @silenced_account_ids }) if @options[:override_timestamps] || @status.within_realtime_window?
|
||||||
end
|
end
|
||||||
|
|
||||||
def find_existing_status
|
def find_existing_status
|
||||||
|
@ -168,7 +168,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
|
||||||
|
|
||||||
return unless delivered_to_account.following?(@account)
|
return unless delivered_to_account.following?(@account)
|
||||||
|
|
||||||
FeedInsertWorker.perform_async(@status.id, delivered_to_account.id, :home)
|
FeedInsertWorker.perform_async(@status.id, delivered_to_account.id, 'home')
|
||||||
end
|
end
|
||||||
|
|
||||||
def delivered_to_account
|
def delivered_to_account
|
||||||
|
|
|
@ -59,7 +59,7 @@ class FeedManager
|
||||||
return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?)
|
return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?)
|
||||||
|
|
||||||
trim(:home, account.id)
|
trim(:home, account.id)
|
||||||
PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", update: update) if push_update_required?("timeline:#{account.id}")
|
PushUpdateWorker.perform_async(account.id, status.id, "timeline:#{account.id}", { 'update' => update }) if push_update_required?("timeline:#{account.id}")
|
||||||
true
|
true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -84,7 +84,7 @@ class FeedManager
|
||||||
return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?)
|
return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?)
|
||||||
|
|
||||||
trim(:list, list.id)
|
trim(:list, list.id)
|
||||||
PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", update: update) if push_update_required?("timeline:list:#{list.id}")
|
PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}", { 'update' => update }) if push_update_required?("timeline:list:#{list.id}")
|
||||||
true
|
true
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -56,7 +56,7 @@ class Admin::StatusBatchAction
|
||||||
end
|
end
|
||||||
|
|
||||||
UserMailer.warning(target_account.user, @warning).deliver_later! if target_account.local?
|
UserMailer.warning(target_account.user, @warning).deliver_later! if target_account.local?
|
||||||
RemovalWorker.push_bulk(status_ids) { |status_id| [status_id, preserve: target_account.local?, immediate: !target_account.local?] }
|
RemovalWorker.push_bulk(status_ids) { |status_id| [status_id, { 'preserve' => target_account.local?, 'immediate' => !target_account.local? }] }
|
||||||
end
|
end
|
||||||
|
|
||||||
def handle_report!
|
def handle_report!
|
||||||
|
|
|
@ -103,7 +103,7 @@ class Form::AccountBatch
|
||||||
authorize(account.user, :reject?)
|
authorize(account.user, :reject?)
|
||||||
log_action(:reject, account.user, username: account.username)
|
log_action(:reject, account.user, username: account.username)
|
||||||
account.suspend!(origin: :local)
|
account.suspend!(origin: :local)
|
||||||
AccountDeletionWorker.perform_async(account.id, reserve_username: false)
|
AccountDeletionWorker.perform_async(account.id, { 'reserve_username' => false })
|
||||||
end
|
end
|
||||||
|
|
||||||
def suspend_account(account)
|
def suspend_account(account)
|
||||||
|
|
|
@ -15,7 +15,7 @@ class AccountStatusesCleanupService < BaseService
|
||||||
|
|
||||||
account_policy.statuses_to_delete(budget, cutoff_id, account_policy.last_inspected).reorder(nil).find_each(order: :asc) do |status|
|
account_policy.statuses_to_delete(budget, cutoff_id, account_policy.last_inspected).reorder(nil).find_each(order: :asc) do |status|
|
||||||
status.discard
|
status.discard
|
||||||
RemovalWorker.perform_async(status.id, redraft: false)
|
RemovalWorker.perform_async(status.id, { 'redraft' => false })
|
||||||
num_deleted += 1
|
num_deleted += 1
|
||||||
last_deleted = status.id
|
last_deleted = status.id
|
||||||
end
|
end
|
||||||
|
|
|
@ -266,7 +266,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService
|
||||||
end
|
end
|
||||||
|
|
||||||
def broadcast_updates!
|
def broadcast_updates!
|
||||||
::DistributionWorker.perform_async(@status.id, update: true)
|
::DistributionWorker.perform_async(@status.id, { 'update' => true })
|
||||||
end
|
end
|
||||||
|
|
||||||
def queue_poll_notifications!
|
def queue_poll_notifications!
|
||||||
|
|
|
@ -59,7 +59,7 @@ class FanOutOnWriteService < BaseService
|
||||||
def notify_mentioned_accounts!
|
def notify_mentioned_accounts!
|
||||||
@status.active_mentions.where.not(id: @options[:silenced_account_ids] || []).joins(:account).merge(Account.local).select(:id, :account_id).reorder(nil).find_in_batches do |mentions|
|
@status.active_mentions.where.not(id: @options[:silenced_account_ids] || []).joins(:account).merge(Account.local).select(:id, :account_id).reorder(nil).find_in_batches do |mentions|
|
||||||
LocalNotificationWorker.push_bulk(mentions) do |mention|
|
LocalNotificationWorker.push_bulk(mentions) do |mention|
|
||||||
[mention.account_id, mention.id, 'Mention', :mention]
|
[mention.account_id, mention.id, 'Mention', 'mention']
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -67,7 +67,7 @@ class FanOutOnWriteService < BaseService
|
||||||
def deliver_to_all_followers!
|
def deliver_to_all_followers!
|
||||||
@account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers|
|
@account.followers_for_local_distribution.select(:id).reorder(nil).find_in_batches do |followers|
|
||||||
FeedInsertWorker.push_bulk(followers) do |follower|
|
FeedInsertWorker.push_bulk(followers) do |follower|
|
||||||
[@status.id, follower.id, :home, update: update?]
|
[@status.id, follower.id, 'home', { 'update' => update? }]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -75,7 +75,7 @@ class FanOutOnWriteService < BaseService
|
||||||
def deliver_to_lists!
|
def deliver_to_lists!
|
||||||
@account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists|
|
@account.lists_for_local_distribution.select(:id).reorder(nil).find_in_batches do |lists|
|
||||||
FeedInsertWorker.push_bulk(lists) do |list|
|
FeedInsertWorker.push_bulk(lists) do |list|
|
||||||
[@status.id, list.id, :list, update: update?]
|
[@status.id, list.id, 'list', { 'update' => update? }]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -83,7 +83,7 @@ class FanOutOnWriteService < BaseService
|
||||||
def deliver_to_mentioned_followers!
|
def deliver_to_mentioned_followers!
|
||||||
@status.mentions.joins(:account).merge(@account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions|
|
@status.mentions.joins(:account).merge(@account.followers_for_local_distribution).select(:id, :account_id).reorder(nil).find_in_batches do |mentions|
|
||||||
FeedInsertWorker.push_bulk(mentions) do |mention|
|
FeedInsertWorker.push_bulk(mentions) do |mention|
|
||||||
[@status.id, mention.account_id, :home, update: update?]
|
[@status.id, mention.account_id, 'home', { 'update' => update? }]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -68,7 +68,7 @@ class FollowService < BaseService
|
||||||
follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
|
follow_request = @source_account.request_follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
|
||||||
|
|
||||||
if @target_account.local?
|
if @target_account.local?
|
||||||
LocalNotificationWorker.perform_async(@target_account.id, follow_request.id, follow_request.class.name, :follow_request)
|
LocalNotificationWorker.perform_async(@target_account.id, follow_request.id, follow_request.class.name, 'follow_request')
|
||||||
elsif @target_account.activitypub?
|
elsif @target_account.activitypub?
|
||||||
ActivityPub::DeliveryWorker.perform_async(build_json(follow_request), @source_account.id, @target_account.inbox_url)
|
ActivityPub::DeliveryWorker.perform_async(build_json(follow_request), @source_account.id, @target_account.inbox_url)
|
||||||
end
|
end
|
||||||
|
@ -79,7 +79,7 @@ class FollowService < BaseService
|
||||||
def direct_follow!
|
def direct_follow!
|
||||||
follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
|
follow = @source_account.follow!(@target_account, reblogs: @options[:reblogs], notify: @options[:notify], rate_limit: @options[:with_rate_limit], bypass_limit: @options[:bypass_limit])
|
||||||
|
|
||||||
LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name, :follow)
|
LocalNotificationWorker.perform_async(@target_account.id, follow.id, follow.class.name, 'follow')
|
||||||
MergeWorker.perform_async(@target_account.id, @source_account.id)
|
MergeWorker.perform_async(@target_account.id, @source_account.id)
|
||||||
|
|
||||||
follow
|
follow
|
||||||
|
|
|
@ -76,7 +76,7 @@ class ImportService < BaseService
|
||||||
if presence_hash[target_account.acct]
|
if presence_hash[target_account.acct]
|
||||||
items.delete(target_account.acct)
|
items.delete(target_account.acct)
|
||||||
extra = presence_hash[target_account.acct][1]
|
extra = presence_hash[target_account.acct][1]
|
||||||
Import::RelationshipWorker.perform_async(@account.id, target_account.acct, action, extra)
|
Import::RelationshipWorker.perform_async(@account.id, target_account.acct, action, extra.stringify_keys)
|
||||||
else
|
else
|
||||||
Import::RelationshipWorker.perform_async(@account.id, target_account.acct, undo_action)
|
Import::RelationshipWorker.perform_async(@account.id, target_account.acct, undo_action)
|
||||||
end
|
end
|
||||||
|
@ -87,7 +87,7 @@ class ImportService < BaseService
|
||||||
tail_items = items - head_items
|
tail_items = items - head_items
|
||||||
|
|
||||||
Import::RelationshipWorker.push_bulk(head_items + tail_items) do |acct, extra|
|
Import::RelationshipWorker.push_bulk(head_items + tail_items) do |acct, extra|
|
||||||
[@account.id, acct, action, extra]
|
[@account.id, acct, action, extra.stringify_keys]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -47,7 +47,7 @@ class ReblogService < BaseService
|
||||||
reblogged_status = reblog.reblog
|
reblogged_status = reblog.reblog
|
||||||
|
|
||||||
if reblogged_status.account.local?
|
if reblogged_status.account.local?
|
||||||
LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name, :reblog)
|
LocalNotificationWorker.perform_async(reblogged_status.account_id, reblog.id, reblog.class.name, 'reblog')
|
||||||
elsif reblogged_status.account.activitypub? && !reblogged_status.account.following?(reblog.account)
|
elsif reblogged_status.account.activitypub? && !reblogged_status.account.following?(reblog.account)
|
||||||
ActivityPub::DeliveryWorker.perform_async(build_json(reblog), reblog.account_id, reblogged_status.account.inbox_url)
|
ActivityPub::DeliveryWorker.perform_async(build_json(reblog), reblog.account_id, reblogged_status.account.inbox_url)
|
||||||
end
|
end
|
||||||
|
|
|
@ -143,7 +143,7 @@ class ResolveAccountService < BaseService
|
||||||
|
|
||||||
def queue_deletion!
|
def queue_deletion!
|
||||||
@account.suspend!(origin: :remote)
|
@account.suspend!(origin: :remote)
|
||||||
AccountDeletionWorker.perform_async(@account.id, reserve_username: false, skip_activitypub: true)
|
AccountDeletionWorker.perform_async(@account.id, { 'reserve_username' => false, 'skip_activitypub' => true })
|
||||||
end
|
end
|
||||||
|
|
||||||
def lock_options
|
def lock_options
|
||||||
|
|
|
@ -27,6 +27,6 @@ class ActivityPub::DistributionWorker < ActivityPub::RawDistributionWorker
|
||||||
end
|
end
|
||||||
|
|
||||||
def options
|
def options
|
||||||
{ synchronize_followers: @status.private_visibility? }
|
{ 'synchronize_followers' => @status.private_visibility? }
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -3,7 +3,7 @@
|
||||||
class FeedInsertWorker
|
class FeedInsertWorker
|
||||||
include Sidekiq::Worker
|
include Sidekiq::Worker
|
||||||
|
|
||||||
def perform(status_id, id, type = :home, options = {})
|
def perform(status_id, id, type = 'home', options = {})
|
||||||
@type = type.to_sym
|
@type = type.to_sym
|
||||||
@status = Status.find(status_id)
|
@status = Status.find(status_id)
|
||||||
@options = options.symbolize_keys
|
@options = options.symbolize_keys
|
||||||
|
|
|
@ -29,7 +29,7 @@ class Scheduler::UserCleanupScheduler
|
||||||
def clean_discarded_statuses!
|
def clean_discarded_statuses!
|
||||||
Status.discarded.where('deleted_at <= ?', 30.days.ago).find_in_batches do |statuses|
|
Status.discarded.where('deleted_at <= ?', 30.days.ago).find_in_batches do |statuses|
|
||||||
RemovalWorker.push_bulk(statuses) do |status|
|
RemovalWorker.push_bulk(statuses) do |status|
|
||||||
[status.id, { immediate: true }]
|
[status.id, { 'immediate' => true }]
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -70,3 +70,6 @@ if ENV['PAM_ENABLED'] == 'true'
|
||||||
env: { email: 'pam@example.com' }
|
env: { email: 'pam@example.com' }
|
||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# Catch serialization warnings early
|
||||||
|
Sidekiq.strict_args!
|
||||||
|
|
Loading…
Reference in a new issue