Fix/remove calling private method with send in model (#22951)

* fix(status): remove send usage for private unlink_from_conversations

- make unlink_from_conversations public method
- rename unlink_from_conversations to unlink_from_conversations!
- fix send call on private method in statuses_vacuum and batched_remove_status_service

* fix(feeds_vacuum): replace find_in_batches with in_batches

because active record query results should be a little more efficient than
itterating with map and each. Postgres can grasp such lists of ids much quicker
than ruby can.
Will probably make allmost no difference, but cannot hurt either.
This commit is contained in:
Kaspar V 2023-01-11 21:57:24 +01:00 committed by GitHub
parent a65f86ae55
commit ae62e5fa53
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 20 additions and 25 deletions

View File

@ -9,14 +9,14 @@ class Vacuum::FeedsVacuum
private private
def vacuum_inactive_home_feeds! def vacuum_inactive_home_feeds!
inactive_users.select(:id, :account_id).find_in_batches do |users| inactive_users.select(:id, :account_id).in_batches do |users|
feed_manager.clean_feeds!(:home, users.map(&:account_id)) feed_manager.clean_feeds!(:home, users.pluck(:account_id))
end end
end end
def vacuum_inactive_list_feeds! def vacuum_inactive_list_feeds!
inactive_users_lists.select(:id).find_in_batches do |lists| inactive_users_lists.select(:id).in_batches do |lists|
feed_manager.clean_feeds!(:list, lists.map(&:id)) feed_manager.clean_feeds!(:list, lists.ids)
end end
end end

View File

@ -19,10 +19,7 @@ class Vacuum::StatusesVacuum
# as the search index, must be handled first. # as the search index, must be handled first.
statuses.direct_visibility statuses.direct_visibility
.includes(mentions: :account) .includes(mentions: :account)
.find_each do |status| .find_each(&:unlink_from_conversations!)
# TODO: replace temporary solution - call of private model method
status.send(:unlink_from_conversations)
end
remove_from_search_index(statuses.ids) if Chewy.enabled? remove_from_search_index(statuses.ids) if Chewy.enabled?
# Foreign keys take care of most associated records for us. # Foreign keys take care of most associated records for us.

View File

@ -29,7 +29,7 @@
# #
class Status < ApplicationRecord class Status < ApplicationRecord
before_destroy :unlink_from_conversations before_destroy :unlink_from_conversations!
include Discard::Model include Discard::Model
include Paginable include Paginable
@ -309,14 +309,14 @@ class Status < ApplicationRecord
after_create_commit :store_uri, if: :local? after_create_commit :store_uri, if: :local?
after_create_commit :update_statistics, if: :local? after_create_commit :update_statistics, if: :local?
around_create Mastodon::Snowflake::Callbacks
before_validation :prepare_contents, if: :local? before_validation :prepare_contents, if: :local?
before_validation :set_reblog before_validation :set_reblog
before_validation :set_visibility before_validation :set_visibility
before_validation :set_conversation before_validation :set_conversation
before_validation :set_local before_validation :set_local
around_create Mastodon::Snowflake::Callbacks
after_create :set_poll_id after_create :set_poll_id
class << self class << self
@ -447,6 +447,17 @@ class Status < ApplicationRecord
update_attribute(:deleted_at, discard_time) update_attribute(:deleted_at, discard_time)
end end
def unlink_from_conversations!
return unless direct_visibility?
inbox_owners = mentioned_accounts.local
inbox_owners += [account] if account.local?
inbox_owners.each do |inbox_owner|
AccountConversation.remove_status(inbox_owner, self)
end
end
private private
def update_status_stat!(attrs) def update_status_stat!(attrs)
@ -524,15 +535,4 @@ class Status < ApplicationRecord
reblog&.decrement_count!(:reblogs_count) if reblog? reblog&.decrement_count!(:reblogs_count) if reblog?
thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && distributable? thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && distributable?
end end
def unlink_from_conversations
return unless direct_visibility?
inbox_owners = mentioned_accounts.local
inbox_owners += [account] if account.local?
inbox_owners.each do |inbox_owner|
AccountConversation.remove_status(inbox_owner, self)
end
end
end end

View File

@ -19,9 +19,7 @@ class BatchedRemoveStatusService < BaseService
ActiveRecord::Associations::Preloader.new.preload(statuses_with_account_conversations, [mentions: :account]) ActiveRecord::Associations::Preloader.new.preload(statuses_with_account_conversations, [mentions: :account])
statuses_with_account_conversations.each do |status| statuses_with_account_conversations.each(&:unlink_from_conversations!)
status.send(:unlink_from_conversations)
end
# We do not batch all deletes into one to avoid having a long-running # We do not batch all deletes into one to avoid having a long-running
# transaction lock the database, but we use the delete method instead # transaction lock the database, but we use the delete method instead