From 71729f86f9cabab7cceacc80f96ea323cec9be6b Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 14 Sep 2023 21:40:03 +0900 Subject: [PATCH 1/6] Improve N+1 problem on timeline statuses (except for emoji_reaction N+1) --- app/models/status.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/status.rb b/app/models/status.rb index c6c05e95e7..d679bde788 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -173,6 +173,8 @@ class Status < ApplicationRecord :tags, :preview_cards, :preloadable_poll, + :reference_objects, + :scheduled_expiration_status, account: [:account_stat, user: :role], active_mentions: { account: :account_stat }, reblog: [ @@ -183,6 +185,8 @@ class Status < ApplicationRecord :conversation, :status_stat, :preloadable_poll, + :reference_objects, + :scheduled_expiration_status, account: [:account_stat, user: :role], active_mentions: { account: :account_stat }, ], From 1fbbac76f604cc95dfd3e20c42d442a21c77b4b6 Mon Sep 17 00:00:00 2001 From: KMY Date: Thu, 14 Sep 2023 21:41:53 +0900 Subject: [PATCH 2/6] Bump version to 3.1 --- lib/mastodon/version.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index c00ed740a2..fbe320776e 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -9,7 +9,7 @@ module Mastodon end def kmyblue_minor - 0 + 1 end def major From 2f57815fb098dcb8036df4e3a2a4c093dc53016d Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 15 Sep 2023 08:09:34 +0900 Subject: [PATCH 3/6] Remove status cache when status reference set --- app/models/status_reference.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/status_reference.rb b/app/models/status_reference.rb index 2ed5b2aee9..62be6df3b1 100644 --- a/app/models/status_reference.rb +++ b/app/models/status_reference.rb @@ -18,8 +18,16 @@ class StatusReference < ApplicationRecord has_one :notification, as: :activity, dependent: :destroy validate :validate_status_visibilities + after_commit :reset_parent_cache + + private def validate_status_visibilities raise Mastodon::ValidationError, I18n.t('status_references.errors.invalid_status_visibilities') if [:public, :public_unlisted, :unlisted, :login].exclude?(target_status.visibility.to_sym) end + + def reset_parent_cache + Rails.cache.delete("statuses/#{status_id}") + Rails.cache.delete("statuses/#{target_status_id}") + end end From cf96e7eb39444ddb4b469b1cebce73ce3c8f1480 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 15 Sep 2023 08:17:13 +0900 Subject: [PATCH 4/6] Set status references maximium value --- app/services/process_references_service.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/services/process_references_service.rb b/app/services/process_references_service.rb index 0dbdc43560..9c8b3813f5 100644 --- a/app/services/process_references_service.rb +++ b/app/services/process_references_service.rb @@ -5,13 +5,14 @@ class ProcessReferencesService < BaseService DOMAIN = ENV['WEB_DOMAIN'] || ENV.fetch('LOCAL_DOMAIN', nil) REFURL_EXP = /(RT|QT|BT|RN|RE)((:|;)?\s+|:|;)(#{URI::DEFAULT_PARSER.make_regexp(%w(http https))})/ + MAX_REFERENCES = 5 def call(status, reference_parameters, urls: nil) @status = status @reference_parameters = reference_parameters || [] @urls = urls || [] - old_references + @references_count = old_references.size return unless added_references.size.positive? || removed_references.size.positive? @@ -63,6 +64,9 @@ class ProcessReferencesService < BaseService statuses.each do |status| @added_objects << @status.reference_objects.new(target_status: status) status.increment_count!(:status_referred_by_count) + @references_count += 1 + + break if @references_count >= MAX_REFERENCES end end @@ -85,6 +89,7 @@ class ProcessReferencesService < BaseService @status.reference_objects.where(target_status: statuses).destroy_all statuses.each do |status| status.decrement_count!(:status_referred_by_count) + @references_count -= 1 end end end From 7c387becb61e02798de55429c98729249c2c4a32 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 15 Sep 2023 08:19:58 +0900 Subject: [PATCH 5/6] Remove some error of fetch_instance_info_worker --- app/workers/activitypub/fetch_instance_info_worker.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/workers/activitypub/fetch_instance_info_worker.rb b/app/workers/activitypub/fetch_instance_info_worker.rb index faf7401fc9..57cbd97d10 100644 --- a/app/workers/activitypub/fetch_instance_info_worker.rb +++ b/app/workers/activitypub/fetch_instance_info_worker.rb @@ -9,7 +9,6 @@ class ActivityPub::FetchInstanceInfoWorker sidekiq_options queue: 'push', retry: 2 class Error < StandardError; end - class GoneError < Error; end class RequestError < Error; end class DeadError < Error; end @@ -68,9 +67,7 @@ class ActivityPub::FetchInstanceInfoWorker raise Mastodon::UnexpectedResponseError, response unless response_successful?(response) || response_error_unsalvageable?(response) body_to_json(response.body_with_limit) - elsif response.code == 410 - raise ActivityPub::FetchInstanceInfoWorker::GoneError, "#{@instance.domain} is gone from the server" - elsif response.code == 404 + elsif [400, 401, 403, 404, 410].include?(response.code) raise ActivityPub::FetchInstanceInfoWorker::DeadError, "Request for #{@instance.domain} returned HTTP #{response.code}" else raise ActivityPub::FetchInstanceInfoWorker::RequestError, "Request for #{@instance.domain} returned HTTP #{response.code}" From c0ff0754a3e23c90c50525593dcc9d34c2d0ad97 Mon Sep 17 00:00:00 2001 From: KMY Date: Fri, 15 Sep 2023 09:31:12 +0900 Subject: [PATCH 6/6] Fix timeline emoji_reactions N+1 problem --- .../api/v1/accounts/statuses_controller.rb | 4 ++- .../api/v1/bookmarks_controller.rb | 4 ++- .../api/v1/emoji_reactions_controller.rb | 4 ++- .../api/v1/favourites_controller.rb | 4 ++- .../referred_by_statuses_controller.rb | 4 ++- .../api/v1/timelines/home_controller.rb | 2 ++ .../api/v1/timelines/public_controller.rb | 4 ++- .../api/v1/timelines/tag_controller.rb | 4 ++- app/models/status.rb | 26 ++++++++++++++----- .../emoji_reaction_accounts_presenter.rb | 22 ++++++++++++++++ .../status_relationships_presenter.rb | 4 ++- app/serializers/rest/status_serializer.rb | 18 +++++++++++-- 12 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 app/presenters/emoji_reaction_accounts_presenter.rb diff --git a/app/controllers/api/v1/accounts/statuses_controller.rb b/app/controllers/api/v1/accounts/statuses_controller.rb index 51f541bd23..7004dbb530 100644 --- a/app/controllers/api/v1/accounts/statuses_controller.rb +++ b/app/controllers/api/v1/accounts/statuses_controller.rb @@ -9,7 +9,9 @@ class Api::V1::Accounts::StatusesController < Api::BaseController def index cache_if_unauthenticated! @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/bookmarks_controller.rb b/app/controllers/api/v1/bookmarks_controller.rb index 498eb16f44..0985366964 100644 --- a/app/controllers/api/v1/bookmarks_controller.rb +++ b/app/controllers/api/v1/bookmarks_controller.rb @@ -7,7 +7,9 @@ class Api::V1::BookmarksController < Api::BaseController def index @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/emoji_reactions_controller.rb b/app/controllers/api/v1/emoji_reactions_controller.rb index db32962266..43b39921db 100644 --- a/app/controllers/api/v1/emoji_reactions_controller.rb +++ b/app/controllers/api/v1/emoji_reactions_controller.rb @@ -7,7 +7,9 @@ class Api::V1::EmojiReactionsController < Api::BaseController def index @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/favourites_controller.rb b/app/controllers/api/v1/favourites_controller.rb index faf1bda96a..984f3d3051 100644 --- a/app/controllers/api/v1/favourites_controller.rb +++ b/app/controllers/api/v1/favourites_controller.rb @@ -7,7 +7,9 @@ class Api::V1::FavouritesController < Api::BaseController def index @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/statuses/referred_by_statuses_controller.rb b/app/controllers/api/v1/statuses/referred_by_statuses_controller.rb index 70f04df209..b0ad6754ea 100644 --- a/app/controllers/api/v1/statuses/referred_by_statuses_controller.rb +++ b/app/controllers/api/v1/statuses/referred_by_statuses_controller.rb @@ -9,7 +9,9 @@ class Api::V1::Statuses::ReferredByStatusesController < Api::BaseController def index @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/timelines/home_controller.rb b/app/controllers/api/v1/timelines/home_controller.rb index 83b8cb4c66..b7c78e8734 100644 --- a/app/controllers/api/v1/timelines/home_controller.rb +++ b/app/controllers/api/v1/timelines/home_controller.rb @@ -9,11 +9,13 @@ class Api::V1::Timelines::HomeController < Api::BaseController with_read_replica do @statuses = load_statuses @relationships = StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + @emoji_reactions = EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end render json: @statuses, each_serializer: REST::StatusSerializer, relationships: @relationships, + emoji_reaction_permitted_account_ids: @emoji_reactions, status: account_home_feed.regenerating? ? 206 : 200 end diff --git a/app/controllers/api/v1/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb index 5bbd92b9ee..9c320e688e 100644 --- a/app/controllers/api/v1/timelines/public_controller.rb +++ b/app/controllers/api/v1/timelines/public_controller.rb @@ -7,7 +7,9 @@ class Api::V1::Timelines::PublicController < Api::BaseController def show cache_if_unauthenticated! @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb index a79d65c124..1de00f50c8 100644 --- a/app/controllers/api/v1/timelines/tag_controller.rb +++ b/app/controllers/api/v1/timelines/tag_controller.rb @@ -8,7 +8,9 @@ class Api::V1::Timelines::TagController < Api::BaseController def show cache_if_unauthenticated! @statuses = load_statuses - render json: @statuses, each_serializer: REST::StatusSerializer, relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id) + render json: @statuses, each_serializer: REST::StatusSerializer, + relationships: StatusRelationshipsPresenter.new(@statuses, current_user&.account_id), + emoji_reaction_permitted_account_ids: EmojiReactionAccountsPresenter.new(@statuses, current_user&.account_id) end private diff --git a/app/models/status.rb b/app/models/status.rb index d679bde788..d4a28220d1 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -357,21 +357,30 @@ class Status < ApplicationRecord return [] if account.present? && !self.account.show_emoji_reaction?(account) return [] if account.nil? && !options[:force] && self.account.emoji_reaction_policy != :allow + permitted_account_ids = options[:permitted_account_ids] + (Oj.load(status_stat&.emoji_reactions || '', mode: :strict) || []).tap do |emoji_reactions| if account.present? - remove_emoji_reactions = [] + public_emoji_reactions = [] + emoji_reactions.each do |emoji_reaction| emoji_reaction['me'] = emoji_reaction['account_ids'].include?(account.id.to_s) emoji_reaction['account_ids'] -= account.excluded_from_timeline_account_ids.map(&:to_s) - accounts = Account.where(id: emoji_reaction['account_ids'], silenced_at: nil, suspended_at: nil) - accounts = accounts.where('domain IS NULL OR domain NOT IN (?)', account.excluded_from_timeline_domains) if account.excluded_from_timeline_domains.size.positive? - emoji_reaction['account_ids'] = accounts.pluck(:id).map(&:to_s) + accounts = [] + if permitted_account_ids + emoji_reaction['account_ids'] = emoji_reaction['account_ids'] & permitted_account_ids.map(&:to_s) + else + accounts = Account.where(id: emoji_reaction['account_ids'], silenced_at: nil, suspended_at: nil) + accounts = accounts.where('domain IS NULL OR domain NOT IN (?)', account.excluded_from_timeline_domains) if account.excluded_from_timeline_domains.size.positive? + emoji_reaction['account_ids'] = accounts.pluck(:id).map(&:to_s) + end emoji_reaction['count'] = emoji_reaction['account_ids'].size - remove_emoji_reactions << emoji_reaction if emoji_reaction['count'] <= 0 + public_emoji_reactions << emoji_reaction if (emoji_reaction['count']).positive? end - emoji_reactions - remove_emoji_reactions + + public_emoji_reactions end end end @@ -463,6 +472,11 @@ class Status < ApplicationRecord EmojiReaction.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |e, h| h[e.status_id] = true } end + def emoji_reaction_allows_map(status_ids, account_id) + my_account = Account.find_by(id: account_id) + Status.where(id: status_ids).pluck(:account_id).uniq.index_with { |a| Account.find_by(id: a).show_emoji_reaction?(my_account) } + end + def reload_stale_associations!(cached_items) account_ids = [] diff --git a/app/presenters/emoji_reaction_accounts_presenter.rb b/app/presenters/emoji_reaction_accounts_presenter.rb new file mode 100644 index 0000000000..1b683f3366 --- /dev/null +++ b/app/presenters/emoji_reaction_accounts_presenter.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class EmojiReactionAccountsPresenter + attr_reader :permitted_account_ids + + def initialize(statuses, current_account_id = nil, **_options) + @current_account_id = current_account_id + + statuses = statuses.compact + status_ids = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.uniq.compact + emoji_reactions = EmojiReaction.where(status_id: status_ids) + account_ids = emoji_reactions.pluck(:account_id).uniq + + permitted_accounts = Account.where(id: account_ids, silenced_at: nil, suspended_at: nil) + if current_account_id.present? + account = Account.find(current_account_id) + permitted_accounts = permitted_accounts.where('domain IS NULL OR domain NOT IN (?)', account.excluded_from_timeline_domains) if account.present? && account.excluded_from_timeline_domains.size.positive? + end + + @permitted_account_ids = permitted_accounts.pluck(:id) + end +end diff --git a/app/presenters/status_relationships_presenter.rb b/app/presenters/status_relationships_presenter.rb index 3a5f0e1589..9e55742403 100644 --- a/app/presenters/status_relationships_presenter.rb +++ b/app/presenters/status_relationships_presenter.rb @@ -4,7 +4,7 @@ class StatusRelationshipsPresenter PINNABLE_VISIBILITIES = %w(public public_unlisted unlisted login private).freeze attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map, - :bookmarks_map, :filters_map, :emoji_reactions_map, :attributes_map + :bookmarks_map, :filters_map, :emoji_reactions_map, :attributes_map, :emoji_reaction_allows_map def initialize(statuses, current_account_id = nil, **options) @current_account_id = current_account_id @@ -17,6 +17,7 @@ class StatusRelationshipsPresenter @pins_map = {} @filters_map = {} @emoji_reactions_map = {} + @emoji_reaction_allows_map = nil else statuses = statuses.compact status_ids = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.uniq.compact @@ -30,6 +31,7 @@ class StatusRelationshipsPresenter @mutes_map = Status.mutes_map(conversation_ids, current_account_id).merge(options[:mutes_map] || {}) @pins_map = Status.pins_map(pinnable_status_ids, current_account_id).merge(options[:pins_map] || {}) @emoji_reactions_map = Status.emoji_reactions_map(status_ids, current_account_id).merge(options[:emoji_reactions_map] || {}) + @emoji_reaction_allows_map = Status.emoji_reaction_allows_map(status_ids, current_account_id).merge(options[:emoji_reaction_allows_map] || {}) @attributes_map = options[:attributes_map] || {} end end diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index e53fb535bc..68b3485169 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -122,7 +122,7 @@ class REST::StatusSerializer < ActiveModel::Serializer end def emoji_reactions - object.emoji_reactions_grouped_by_name(current_user&.account) + show_emoji_reaction? ? object.emoji_reactions_grouped_by_name(current_user&.account, permitted_account_ids: emoji_reaction_permitted_account_ids) : [] end def emoji_reactions_count @@ -131,7 +131,17 @@ class REST::StatusSerializer < ActiveModel::Serializer object.account.emoji_reaction_policy == :allow ? object.emoji_reactions_count : 0 else - object.account.show_emoji_reaction?(current_user.account) ? object.emoji_reactions_count : 0 + show_emoji_reaction? ? object.emoji_reactions_count : 0 + end + end + + def show_emoji_reaction? + if relationships + return true if relationships.emoji_reaction_allows_map.nil? + + relationships.emoji_reaction_allows_map[object.account_id] || false + else + object.account.show_emoji_reaction?(current_user.account) end end @@ -211,6 +221,10 @@ class REST::StatusSerializer < ActiveModel::Serializer instance_options && instance_options[:relationships] end + def emoji_reaction_permitted_account_ids + current_user.present? && instance_options && instance_options[:emoji_reaction_permitted_account_ids]&.permitted_account_ids + end + class ApplicationSerializer < ActiveModel::Serializer attributes :name, :website