From b5ac61b2c5cad94a680527b961def46aea0a1ad4 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 19 Dec 2023 11:59:43 +0100 Subject: [PATCH] Change algorithm of follow recommendations (#28314) Co-authored-by: Claire --- .github/renovate.json5 | 7 +- .../follow_recommendations_controller.rb | 2 +- .../api/v1/suggestions_controller.rb | 13 ++-- .../api/v2/suggestions_controller.rb | 12 +++- app/lib/potential_friendship_tracker.rb | 31 --------- app/models/account_domain_block.rb | 5 ++ app/models/account_suggestions.rb | 48 +++++++++---- .../friends_of_friends_source.rb | 37 ++++++++++ .../account_suggestions/global_source.rb | 34 ++-------- .../past_interactions_source.rb | 36 ---------- .../account_suggestions/setting_source.rb | 34 +++------- .../similar_profiles_source.rb | 67 +++++++++++++++++++ app/models/account_suggestions/source.rb | 30 ++------- app/models/block.rb | 9 ++- app/models/concerns/account/associations.rb | 1 + app/models/concerns/account/interactions.rb | 13 ---- app/models/concerns/account/search.rb | 1 + app/models/follow.rb | 8 ++- app/models/follow_recommendation_filter.rb | 7 +- app/models/follow_recommendation_mute.rb | 26 +++++++ .../follow_recommendation_suppression.rb | 14 ---- app/models/follow_request.rb | 5 ++ app/models/mute.rb | 9 ++- app/models/preview_cards_status.rb | 4 +- app/services/account_search_service.rb | 5 ++ app/services/favourite_service.rb | 7 +- app/services/post_status_service.rb | 3 - app/services/reblog_service.rb | 8 +-- .../follow_recommendations/show.html.haml | 2 + .../follow_recommendations_scheduler.rb | 50 -------------- ...4923_create_follow_recommendation_mutes.rb | 14 ++++ ...dd_languages_index_to_account_summaries.rb | 9 +++ db/schema.rb | 14 +++- spec/requests/api/v1/suggestions_spec.rb | 22 +++--- .../follow_recommendations_scheduler_spec.rb | 2 - 35 files changed, 297 insertions(+), 292 deletions(-) delete mode 100644 app/lib/potential_friendship_tracker.rb create mode 100644 app/models/account_suggestions/friends_of_friends_source.rb delete mode 100644 app/models/account_suggestions/past_interactions_source.rb create mode 100644 app/models/account_suggestions/similar_profiles_source.rb create mode 100644 app/models/follow_recommendation_mute.rb create mode 100644 db/migrate/20231211234923_create_follow_recommendation_mutes.rb create mode 100644 db/migrate/20231212073317_add_languages_index_to_account_summaries.rb diff --git a/.github/renovate.json5 b/.github/renovate.json5 index aca522859..dab99829a 100644 --- a/.github/renovate.json5 +++ b/.github/renovate.json5 @@ -102,9 +102,12 @@ { // Group actions/*-artifact in the same PR matchManagers: ['github-actions'], - matchPackageNames: ['actions/download-artifact', 'actions/upload-artifact'], + matchPackageNames: [ + 'actions/download-artifact', + 'actions/upload-artifact', + ], matchUpdateTypes: ['major'], - groupName: 'artifact actions (major)' + groupName: 'artifact actions (major)', }, { // Update @types/* packages every week, with one grouped PR diff --git a/app/controllers/admin/follow_recommendations_controller.rb b/app/controllers/admin/follow_recommendations_controller.rb index 841e3cc7f..a54e41bd8 100644 --- a/app/controllers/admin/follow_recommendations_controller.rb +++ b/app/controllers/admin/follow_recommendations_controller.rb @@ -8,7 +8,7 @@ module Admin authorize :follow_recommendation, :show? @form = Form::AccountBatch.new - @accounts = filtered_follow_recommendations + @accounts = filtered_follow_recommendations.page(params[:page]) end def update diff --git a/app/controllers/api/v1/suggestions_controller.rb b/app/controllers/api/v1/suggestions_controller.rb index 9737ae5cb..9ba1cef63 100644 --- a/app/controllers/api/v1/suggestions_controller.rb +++ b/app/controllers/api/v1/suggestions_controller.rb @@ -3,22 +3,23 @@ class Api::V1::SuggestionsController < Api::BaseController include Authorization - before_action -> { doorkeeper_authorize! :read } + before_action -> { doorkeeper_authorize! :read, :'read:accounts' }, only: :index + before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, except: :index before_action :require_user! + before_action :set_suggestions def index - suggestions = suggestions_source.get(current_account, limit: limit_param(DEFAULT_ACCOUNTS_LIMIT)) - render json: suggestions.map(&:account), each_serializer: REST::AccountSerializer + render json: @suggestions.get(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:offset].to_i).map(&:account), each_serializer: REST::AccountSerializer end def destroy - suggestions_source.remove(current_account, params[:id]) + @suggestions.remove(params[:id]) render_empty end private - def suggestions_source - AccountSuggestions::PastInteractionsSource.new + def set_suggestions + @suggestions = AccountSuggestions.new(current_account) end end diff --git a/app/controllers/api/v2/suggestions_controller.rb b/app/controllers/api/v2/suggestions_controller.rb index 35eb276c0..8516796e8 100644 --- a/app/controllers/api/v2/suggestions_controller.rb +++ b/app/controllers/api/v2/suggestions_controller.rb @@ -3,17 +3,23 @@ class Api::V2::SuggestionsController < Api::BaseController include Authorization - before_action -> { doorkeeper_authorize! :read } + before_action -> { doorkeeper_authorize! :read, :'read:accounts' }, only: :index + before_action -> { doorkeeper_authorize! :write, :'write:accounts' }, except: :index before_action :require_user! before_action :set_suggestions def index - render json: @suggestions, each_serializer: REST::SuggestionSerializer + render json: @suggestions.get(limit_param(DEFAULT_ACCOUNTS_LIMIT), params[:offset].to_i), each_serializer: REST::SuggestionSerializer + end + + def destroy + @suggestions.remove(params[:id]) + render_empty end private def set_suggestions - @suggestions = AccountSuggestions.get(current_account, limit_param(DEFAULT_ACCOUNTS_LIMIT)) + @suggestions = AccountSuggestions.new(current_account) end end diff --git a/app/lib/potential_friendship_tracker.rb b/app/lib/potential_friendship_tracker.rb deleted file mode 100644 index f5bc20346..000000000 --- a/app/lib/potential_friendship_tracker.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -class PotentialFriendshipTracker - EXPIRE_AFTER = 90.days.seconds - MAX_ITEMS = 80 - - WEIGHTS = { - reply: 1, - favourite: 10, - reblog: 20, - }.freeze - - class << self - include Redisable - - def record(account_id, target_account_id, action) - return if account_id == target_account_id - - key = "interactions:#{account_id}" - weight = WEIGHTS[action] - - redis.zincrby(key, weight, target_account_id) - redis.zremrangebyrank(key, 0, -MAX_ITEMS) - redis.expire(key, EXPIRE_AFTER) - end - - def remove(account_id, target_account_id) - redis.zrem("interactions:#{account_id}", target_account_id) - end - end -end diff --git a/app/models/account_domain_block.rb b/app/models/account_domain_block.rb index db2e37184..753935d6a 100644 --- a/app/models/account_domain_block.rb +++ b/app/models/account_domain_block.rb @@ -19,6 +19,7 @@ class AccountDomainBlock < ApplicationRecord validates :domain, presence: true, uniqueness: { scope: :account_id }, domain: true after_commit :invalidate_domain_blocking_cache + after_commit :invalidate_follow_recommendations_cache private @@ -26,4 +27,8 @@ class AccountDomainBlock < ApplicationRecord Rails.cache.delete("exclude_domains_for:#{account_id}") Rails.cache.delete(['exclude_domains', account_id, domain]) end + + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end end diff --git a/app/models/account_suggestions.rb b/app/models/account_suggestions.rb index d1774e62f..d62176c7c 100644 --- a/app/models/account_suggestions.rb +++ b/app/models/account_suggestions.rb @@ -1,28 +1,48 @@ # frozen_string_literal: true class AccountSuggestions + include DatabaseHelper + SOURCES = [ AccountSuggestions::SettingSource, - AccountSuggestions::PastInteractionsSource, + AccountSuggestions::FriendsOfFriendsSource, + AccountSuggestions::SimilarProfilesSource, AccountSuggestions::GlobalSource, ].freeze - def self.get(account, limit) - SOURCES.each_with_object([]) do |source_class, suggestions| - source_suggestions = source_class.new.get( - account, - skip_account_ids: suggestions.map(&:account_id), - limit: limit - suggestions.size - ) + BATCH_SIZE = 40 - suggestions.concat(source_suggestions) + def initialize(account) + @account = account + end + + def get(limit, offset = 0) + with_read_replica do + account_ids_with_sources = Rails.cache.fetch("follow_recommendations/#{@account.id}", expires_in: 15.minutes) do + SOURCES.flat_map { |klass| klass.new.get(@account, limit: BATCH_SIZE) }.each_with_object({}) do |(account_id, source), h| + (h[account_id] ||= []).concat(Array(source).map(&:to_sym)) + end.to_a.shuffle + end + + # The sources deliver accounts that haven't yet been followed, are not blocked, + # and so on. Since we reset the cache on follows, blocks, and so on, we don't need + # a complicated query on this end. + + account_ids = account_ids_with_sources[offset, limit] + accounts_map = Account.where(id: account_ids.map(&:first)).includes(:account_stat).index_by(&:id) + + account_ids.filter_map do |(account_id, source)| + next unless accounts_map.key?(account_id) + + AccountSuggestions::Suggestion.new( + account: accounts_map[account_id], + source: source + ) + end end end - def self.remove(account, target_account_id) - SOURCES.each do |source_class| - source = source_class.new - source.remove(account, target_account_id) - end + def remove(target_account_id) + FollowRecommendationMute.create(account_id: @account.id, target_account_id: target_account_id) end end diff --git a/app/models/account_suggestions/friends_of_friends_source.rb b/app/models/account_suggestions/friends_of_friends_source.rb new file mode 100644 index 000000000..77d4f635a --- /dev/null +++ b/app/models/account_suggestions/friends_of_friends_source.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class AccountSuggestions::FriendsOfFriendsSource < AccountSuggestions::Source + def get(account, limit: 10) + Account.find_by_sql([<<~SQL.squish, { id: account.id, limit: limit }]).map { |row| [row.id, key] } + WITH first_degree AS ( + SELECT target_account_id + FROM follows + JOIN accounts AS target_accounts ON follows.target_account_id = target_accounts.id + WHERE account_id = :id + AND NOT target_accounts.hide_collections + ) + SELECT accounts.id, COUNT(*) AS frequency + FROM accounts + JOIN follows ON follows.target_account_id = accounts.id + JOIN account_stats ON account_stats.account_id = accounts.id + LEFT OUTER JOIN follow_recommendation_mutes ON follow_recommendation_mutes.target_account_id = accounts.id AND follow_recommendation_mutes.account_id = :id + WHERE follows.account_id IN (SELECT * FROM first_degree) + AND follows.target_account_id NOT IN (SELECT * FROM first_degree) + AND follows.target_account_id <> :id + AND accounts.discoverable + AND accounts.suspended_at IS NULL + AND accounts.silenced_at IS NULL + AND accounts.moved_to_account_id IS NULL + AND follow_recommendation_mutes.target_account_id IS NULL + GROUP BY accounts.id, account_stats.id + ORDER BY frequency DESC, account_stats.followers_count ASC + LIMIT :limit + SQL + end + + private + + def key + :friends_of_friends + end +end diff --git a/app/models/account_suggestions/global_source.rb b/app/models/account_suggestions/global_source.rb index 651041d67..d68f285e4 100644 --- a/app/models/account_suggestions/global_source.rb +++ b/app/models/account_suggestions/global_source.rb @@ -1,39 +1,13 @@ # frozen_string_literal: true class AccountSuggestions::GlobalSource < AccountSuggestions::Source - include Redisable - - def key - :global - end - - def get(account, skip_account_ids: [], limit: 40) - account_ids = account_ids_for_locale(I18n.locale.to_s.split(/[_-]/).first) - [account.id] - skip_account_ids - - as_ordered_suggestions( - scope(account).where(id: account_ids), - account_ids - ).take(limit) - end - - def remove(_account, _target_account_id) - nil + def get(account, limit: 10) + FollowRecommendation.localized(content_locale).joins(:account).merge(base_account_scope(account)).order(rank: :desc).limit(limit).pluck(:account_id, :reason) end private - def scope(account) - Account.searchable - .followable_by(account) - .not_excluded_by_account(account) - .not_domain_blocked_by_account(account) - end - - def account_ids_for_locale(locale) - redis.zrevrange("follow_recommendations:#{locale}", 0, -1).map(&:to_i) - end - - def to_ordered_list_key(account) - account.id + def content_locale + I18n.locale.to_s.split(/[_-]/).first end end diff --git a/app/models/account_suggestions/past_interactions_source.rb b/app/models/account_suggestions/past_interactions_source.rb deleted file mode 100644 index d169394f1..000000000 --- a/app/models/account_suggestions/past_interactions_source.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -class AccountSuggestions::PastInteractionsSource < AccountSuggestions::Source - include Redisable - - def key - :past_interactions - end - - def get(account, skip_account_ids: [], limit: 40) - account_ids = account_ids_for_account(account.id, limit + skip_account_ids.size) - skip_account_ids - - as_ordered_suggestions( - scope.where(id: account_ids), - account_ids - ).take(limit) - end - - def remove(account, target_account_id) - redis.zrem("interactions:#{account.id}", target_account_id) - end - - private - - def scope - Account.searchable - end - - def account_ids_for_account(account_id, limit) - redis.zrevrange("interactions:#{account_id}", 0, limit).map(&:to_i) - end - - def to_ordered_list_key(account) - account.id - end -end diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb index 6185732b4..4b7275bf7 100644 --- a/app/models/account_suggestions/setting_source.rb +++ b/app/models/account_suggestions/setting_source.rb @@ -1,32 +1,18 @@ # frozen_string_literal: true class AccountSuggestions::SettingSource < AccountSuggestions::Source - def key - :staff - end - - def get(account, skip_account_ids: [], limit: 40) - return [] unless setting_enabled? - - as_ordered_suggestions( - scope(account).where(setting_to_where_condition).where.not(id: skip_account_ids), - usernames_and_domains - ).take(limit) - end - - def remove(_account, _target_account_id) - nil + def get(account, limit: 10) + if setting_enabled? + base_account_scope(account).where(setting_to_where_condition).limit(limit).pluck(:id).zip([key].cycle) + else + [] + end end private - def scope(account) - Account.searchable - .followable_by(account) - .not_excluded_by_account(account) - .not_domain_blocked_by_account(account) - .where(locked: false) - .where.not(id: account.id) + def key + :featured end def usernames_and_domains @@ -61,8 +47,4 @@ class AccountSuggestions::SettingSource < AccountSuggestions::Source def setting Setting.bootstrap_timeline_accounts end - - def to_ordered_list_key(account) - [account.username.downcase, account.domain&.downcase] - end end diff --git a/app/models/account_suggestions/similar_profiles_source.rb b/app/models/account_suggestions/similar_profiles_source.rb new file mode 100644 index 000000000..733c5f0bb --- /dev/null +++ b/app/models/account_suggestions/similar_profiles_source.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +class AccountSuggestions::SimilarProfilesSource < AccountSuggestions::Source + class QueryBuilder < AccountSearchService::QueryBuilder + def must_clauses + [ + { + more_like_this: { + fields: %w(text text.stemmed), + like: @query.map { |id| { _index: 'accounts', _id: id } }, + }, + }, + + { + term: { + properties: 'discoverable', + }, + }, + ] + end + + def must_not_clauses + [ + { + terms: { + id: following_ids, + }, + }, + + { + term: { + properties: 'bot', + }, + }, + ] + end + + def should_clauses + { + term: { + properties: { + value: 'verified', + boost: 2, + }, + }, + } + end + end + + def get(account, limit: 10) + recently_followed_account_ids = account.active_relationships.recent.limit(5).pluck(:target_account_id) + + if Chewy.enabled? && !recently_followed_account_ids.empty? + QueryBuilder.new(recently_followed_account_ids, account).build.limit(limit).hits.pluck('_id').map(&:to_i).zip([key].cycle) + else + [] + end + rescue Faraday::ConnectionFailed + [] + end + + private + + def key + :similar_to_recently_followed + end +end diff --git a/app/models/account_suggestions/source.rb b/app/models/account_suggestions/source.rb index 504d26a8b..ee93a1342 100644 --- a/app/models/account_suggestions/source.rb +++ b/app/models/account_suggestions/source.rb @@ -1,34 +1,18 @@ # frozen_string_literal: true class AccountSuggestions::Source - def key - raise NotImplementedError - end - def get(_account, **kwargs) raise NotImplementedError end - def remove(_account, target_account_id) - raise NotImplementedError - end - protected - def as_ordered_suggestions(scope, ordered_list) - return [] if ordered_list.empty? - - map = scope.index_by { |account| to_ordered_list_key(account) } - - ordered_list.filter_map { |ordered_list_key| map[ordered_list_key] }.map do |account| - AccountSuggestions::Suggestion.new( - account: account, - source: key - ) - end - end - - def to_ordered_list_key(_account) - raise NotImplementedError + def base_account_scope(account) + Account.searchable + .followable_by(account) + .not_excluded_by_account(account) + .not_domain_blocked_by_account(account) + .where.not(id: account.id) + .joins("LEFT OUTER JOIN follow_recommendation_mutes ON follow_recommendation_mutes.target_account_id = accounts.id AND follow_recommendation_mutes.account_id = #{account.id}").where(follow_recommendation_mutes: { target_account_id: nil }) end end diff --git a/app/models/block.rb b/app/models/block.rb index 11156ebab..5476542a5 100644 --- a/app/models/block.rb +++ b/app/models/block.rb @@ -26,15 +26,20 @@ class Block < ApplicationRecord end before_validation :set_uri, only: :create - after_commit :remove_blocking_cache + after_commit :invalidate_blocking_cache + after_commit :invalidate_follow_recommendations_cache private - def remove_blocking_cache + def invalidate_blocking_cache Rails.cache.delete("exclude_account_ids_for:#{account_id}") Rails.cache.delete("exclude_account_ids_for:#{target_account_id}") end + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end + def set_uri self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? end diff --git a/app/models/concerns/account/associations.rb b/app/models/concerns/account/associations.rb index 31902ae21..2bb6fed5a 100644 --- a/app/models/concerns/account/associations.rb +++ b/app/models/concerns/account/associations.rb @@ -64,6 +64,7 @@ module Account::Associations has_one :deletion_request, class_name: 'AccountDeletionRequest', inverse_of: :account, dependent: :destroy # Follow recommendations + has_one :follow_recommendation, inverse_of: :account, dependent: nil has_one :follow_recommendation_suppression, inverse_of: :account, dependent: :destroy # Account statuses cleanup policy diff --git a/app/models/concerns/account/interactions.rb b/app/models/concerns/account/interactions.rb index 4ddec9bf4..351530c2f 100644 --- a/app/models/concerns/account/interactions.rb +++ b/app/models/concerns/account/interactions.rb @@ -116,8 +116,6 @@ module Account::Interactions rel.save! if rel.changed? - remove_potential_friendship(other_account) - rel end @@ -131,13 +129,10 @@ module Account::Interactions rel.save! if rel.changed? - remove_potential_friendship(other_account) - rel end def block!(other_account, uri: nil) - remove_potential_friendship(other_account) block_relationships.create_with(uri: uri) .find_or_create_by!(target_account: other_account) end @@ -148,8 +143,6 @@ module Account::Interactions mute.expires_in = duration.zero? ? nil : duration mute.save! - remove_potential_friendship(other_account) - # When toggling a mute between hiding and allowing notifications, the mute will already exist, so the find_or_create_by! call will return the existing Mute without updating the hide_notifications attribute. Therefore, we check that hide_notifications? is what we want and set it if it isn't. mute.update!(hide_notifications: notifications) if mute.hide_notifications? != notifications @@ -307,10 +300,4 @@ module Account::Interactions domain_blocking_by_domain: Account.domain_blocking_map_by_domain(domains, id), }) end - - private - - def remove_potential_friendship(other_account) - PotentialFriendshipTracker.remove(id, other_account.id) - end end diff --git a/app/models/concerns/account/search.rb b/app/models/concerns/account/search.rb index 40d87aaaa..077e5d57b 100644 --- a/app/models/concerns/account/search.rb +++ b/app/models/concerns/account/search.rb @@ -116,6 +116,7 @@ module Account::Search [].tap do |properties| properties << 'bot' if bot? properties << 'verified' if fields.any?(&:verified?) + properties << 'discoverable' if discoverable? end end diff --git a/app/models/follow.rb b/app/models/follow.rb index 108f5c5d5..4d1598dca 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -44,10 +44,10 @@ class Follow < ApplicationRecord before_validation :set_uri, only: :create after_create :increment_cache_counters - after_create :invalidate_hash_cache after_destroy :remove_endorsements after_destroy :decrement_cache_counters - after_destroy :invalidate_hash_cache + after_commit :invalidate_follow_recommendations_cache + after_commit :invalidate_hash_cache private @@ -74,4 +74,8 @@ class Follow < ApplicationRecord Rails.cache.delete("followers_hash:#{target_account_id}:#{account.synchronization_uri_prefix}") end + + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end end diff --git a/app/models/follow_recommendation_filter.rb b/app/models/follow_recommendation_filter.rb index 2fab97569..62a02eba5 100644 --- a/app/models/follow_recommendation_filter.rb +++ b/app/models/follow_recommendation_filter.rb @@ -17,12 +17,9 @@ class FollowRecommendationFilter def results if params['status'] == 'suppressed' - Account.joins(:follow_recommendation_suppression).order(FollowRecommendationSuppression.arel_table[:id].desc).to_a + Account.includes(:account_stat).joins(:follow_recommendation_suppression).order(FollowRecommendationSuppression.arel_table[:id].desc) else - account_ids = redis.zrevrange("follow_recommendations:#{@language}", 0, -1).map(&:to_i) - accounts = Account.where(id: account_ids).index_by(&:id) - - account_ids.filter_map { |id| accounts[id] } + Account.includes(:account_stat).joins(:follow_recommendation).merge(FollowRecommendation.localized(@language).order(rank: :desc)) end end end diff --git a/app/models/follow_recommendation_mute.rb b/app/models/follow_recommendation_mute.rb new file mode 100644 index 000000000..d166d0a62 --- /dev/null +++ b/app/models/follow_recommendation_mute.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +# == Schema Information +# +# Table name: follow_recommendation_mutes +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) not null +# target_account_id :bigint(8) not null +# created_at :datetime not null +# updated_at :datetime not null +# +class FollowRecommendationMute < ApplicationRecord + belongs_to :account + belongs_to :target_account, class_name: 'Account' + + validates :target_account, uniqueness: { scope: :account_id } + + after_commit :invalidate_follow_recommendations_cache + + private + + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end +end diff --git a/app/models/follow_recommendation_suppression.rb b/app/models/follow_recommendation_suppression.rb index e261a2fe3..59e94dc6b 100644 --- a/app/models/follow_recommendation_suppression.rb +++ b/app/models/follow_recommendation_suppression.rb @@ -11,19 +11,5 @@ # class FollowRecommendationSuppression < ApplicationRecord - include Redisable - belongs_to :account - - after_commit :remove_follow_recommendations, on: :create - - private - - def remove_follow_recommendations - redis.pipelined do |pipeline| - I18n.available_locales.each do |locale| - pipeline.zrem("follow_recommendations:#{locale}", account_id) - end - end - end end diff --git a/app/models/follow_request.rb b/app/models/follow_request.rb index a5c23e09d..3c5e8f96f 100644 --- a/app/models/follow_request.rb +++ b/app/models/follow_request.rb @@ -45,10 +45,15 @@ class FollowRequest < ApplicationRecord end before_validation :set_uri, only: :create + after_commit :invalidate_follow_recommendations_cache private def set_uri self.uri = ActivityPub::TagManager.instance.generate_uri_for(self) if uri.nil? end + + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end end diff --git a/app/models/mute.rb b/app/models/mute.rb index 8fc542262..1d18b30ee 100644 --- a/app/models/mute.rb +++ b/app/models/mute.rb @@ -23,11 +23,16 @@ class Mute < ApplicationRecord validates :account_id, uniqueness: { scope: :target_account_id } - after_commit :remove_blocking_cache + after_commit :invalidate_blocking_cache + after_commit :invalidate_follow_recommendations_cache private - def remove_blocking_cache + def invalidate_blocking_cache Rails.cache.delete("exclude_account_ids_for:#{account_id}") end + + def invalidate_follow_recommendations_cache + Rails.cache.delete("follow_recommendations/#{account_id}") + end end diff --git a/app/models/preview_cards_status.rb b/app/models/preview_cards_status.rb index 214eec22e..5ff635205 100644 --- a/app/models/preview_cards_status.rb +++ b/app/models/preview_cards_status.rb @@ -4,8 +4,8 @@ # # Table name: preview_cards_statuses # -# preview_card_id :bigint(8) not null -# status_id :bigint(8) not null +# preview_card_id :bigint(8) not null, primary key +# status_id :bigint(8) not null, primary key # url :string # class PreviewCardsStatus < ApplicationRecord diff --git a/app/services/account_search_service.rb b/app/services/account_search_service.rb index 7b85b09d8..571a0fa57 100644 --- a/app/services/account_search_service.rb +++ b/app/services/account_search_service.rb @@ -23,6 +23,7 @@ class AccountSearchService < BaseService query: { bool: { must: must_clauses, + must_not: must_not_clauses, }, }, @@ -49,6 +50,10 @@ class AccountSearchService < BaseService end end + def must_not_clauses + [] + end + def should_clauses if @account && !@options[:following] [boost_following_query] diff --git a/app/services/favourite_service.rb b/app/services/favourite_service.rb index 6fdc92a17..ded50187f 100644 --- a/app/services/favourite_service.rb +++ b/app/services/favourite_service.rb @@ -20,7 +20,7 @@ class FavouriteService < BaseService Trends.statuses.register(status) create_notification(favourite) - bump_potential_friendship(account, status) + increment_statistics favourite end @@ -37,11 +37,8 @@ class FavouriteService < BaseService end end - def bump_potential_friendship(account, status) + def increment_statistics ActivityTracker.increment('activity:interactions') - return if account.following?(status.account_id) - - PotentialFriendshipTracker.record(account.id, status.account_id, :favourite) end def build_json(favourite) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index ea27f374e..8aa43ab24 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -178,9 +178,6 @@ class PostStatusService < BaseService return if !@status.reply? || @account.id == @status.in_reply_to_account_id ActivityTracker.increment('activity:interactions') - return if @account.following?(@status.in_reply_to_account_id) - - PotentialFriendshipTracker.record(@account.id, @status.in_reply_to_account_id, :reply) end def status_attributes diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 6ec094474..960315ed8 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -33,7 +33,7 @@ class ReblogService < BaseService ActivityPub::DistributionWorker.perform_async(reblog.id) create_notification(reblog) - bump_potential_friendship(account, reblog) + increment_statistics reblog end @@ -50,12 +50,8 @@ class ReblogService < BaseService end end - def bump_potential_friendship(account, reblog) + def increment_statistics ActivityTracker.increment('activity:interactions') - - return if account.following?(reblog.reblog.account_id) - - PotentialFriendshipTracker.record(account.id, reblog.reblog.account_id, :reblog) end def build_json(reblog) diff --git a/app/views/admin/follow_recommendations/show.html.haml b/app/views/admin/follow_recommendations/show.html.haml index dc65a7213..9c2063d3c 100644 --- a/app/views/admin/follow_recommendations/show.html.haml +++ b/app/views/admin/follow_recommendations/show.html.haml @@ -38,3 +38,5 @@ = nothing_here 'nothing-here--under-tabs' - else = render partial: 'account', collection: @accounts, locals: { f: f } + += paginate @accounts diff --git a/app/workers/scheduler/follow_recommendations_scheduler.rb b/app/workers/scheduler/follow_recommendations_scheduler.rb index ea59ad986..ea5aa8b53 100644 --- a/app/workers/scheduler/follow_recommendations_scheduler.rb +++ b/app/workers/scheduler/follow_recommendations_scheduler.rb @@ -2,61 +2,11 @@ class Scheduler::FollowRecommendationsScheduler include Sidekiq::Worker - include Redisable sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i - # The maximum number of accounts that can be requested in one page from the - # API is 80, and the suggestions API does not allow pagination. This number - # leaves some room for accounts being filtered during live access - SET_SIZE = 100 - def perform - # Maintaining a materialized view speeds-up subsequent queries significantly AccountSummary.refresh FollowRecommendation.refresh - - fallback_recommendations = FollowRecommendation.order(rank: :desc).limit(SET_SIZE) - - Trends.available_locales.each do |locale| - recommendations = if AccountSummary.safe.filtered.localized(locale).exists? # We can skip the work if no accounts with that language exist - FollowRecommendation.localized(locale).order(rank: :desc).limit(SET_SIZE).map { |recommendation| [recommendation.rank, recommendation.account_id] } - else - [] - end - - # Use language-agnostic results if there are not enough language-specific ones - missing = SET_SIZE - recommendations.size - - if missing.positive? && fallback_recommendations.size.positive? - max_fallback_rank = fallback_recommendations.first.rank || 0 - - # Language-specific results should be above language-agnostic ones, - # otherwise language-agnostic ones will always overshadow them - recommendations.map! { |(rank, account_id)| [rank + max_fallback_rank, account_id] } - - added = 0 - - fallback_recommendations.each do |recommendation| - next if recommendations.any? { |(_, account_id)| account_id == recommendation.account_id } - - recommendations << [recommendation.rank, recommendation.account_id] - added += 1 - - break if added >= missing - end - end - - redis.multi do |multi| - multi.del(key(locale)) - multi.zadd(key(locale), recommendations) - end - end - end - - private - - def key(locale) - "follow_recommendations:#{locale}" end end diff --git a/db/migrate/20231211234923_create_follow_recommendation_mutes.rb b/db/migrate/20231211234923_create_follow_recommendation_mutes.rb new file mode 100644 index 000000000..92cb16e17 --- /dev/null +++ b/db/migrate/20231211234923_create_follow_recommendation_mutes.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateFollowRecommendationMutes < ActiveRecord::Migration[7.1] + def change + create_table :follow_recommendation_mutes do |t| + t.references :account, null: false, foreign_key: { on_delete: :cascade }, index: false + t.references :target_account, null: false, foreign_key: { to_table: 'accounts', on_delete: :cascade } + + t.timestamps + end + + add_index :follow_recommendation_mutes, [:account_id, :target_account_id], unique: true + end +end diff --git a/db/migrate/20231212073317_add_languages_index_to_account_summaries.rb b/db/migrate/20231212073317_add_languages_index_to_account_summaries.rb new file mode 100644 index 000000000..8adaf792c --- /dev/null +++ b/db/migrate/20231212073317_add_languages_index_to_account_summaries.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddLanguagesIndexToAccountSummaries < ActiveRecord::Migration[7.1] + disable_ddl_transaction! + + def change + add_index :account_summaries, [:account_id, :language, :sensitive], algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index a0062c8ce..126ed8785 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_10_06_183200) do +ActiveRecord::Schema[7.1].define(version: 2023_12_12_073317) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -474,6 +474,15 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_06_183200) do t.index ["tag_id"], name: "index_featured_tags_on_tag_id" end + create_table "follow_recommendation_mutes", force: :cascade do |t| + t.bigint "account_id", null: false + t.bigint "target_account_id", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["account_id", "target_account_id"], name: "idx_on_account_id_target_account_id_a8c8ddf44e", unique: true + t.index ["target_account_id"], name: "index_follow_recommendation_mutes_on_target_account_id" + end + create_table "follow_recommendation_suppressions", force: :cascade do |t| t.bigint "account_id", null: false t.datetime "created_at", null: false @@ -1209,6 +1218,8 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_06_183200) do add_foreign_key "favourites", "statuses", name: "fk_b0e856845e", on_delete: :cascade add_foreign_key "featured_tags", "accounts", on_delete: :cascade add_foreign_key "featured_tags", "tags", on_delete: :cascade + add_foreign_key "follow_recommendation_mutes", "accounts", column: "target_account_id", on_delete: :cascade + add_foreign_key "follow_recommendation_mutes", "accounts", on_delete: :cascade add_foreign_key "follow_recommendation_suppressions", "accounts", on_delete: :cascade add_foreign_key "follow_requests", "accounts", column: "target_account_id", name: "fk_9291ec025d", on_delete: :cascade add_foreign_key "follow_requests", "accounts", name: "fk_76d644b0e7", on_delete: :cascade @@ -1341,6 +1352,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_10_06_183200) do WHERE ((accounts.suspended_at IS NULL) AND (accounts.silenced_at IS NULL) AND (accounts.moved_to_account_id IS NULL) AND (accounts.discoverable = true) AND (accounts.locked = false)) GROUP BY accounts.id; SQL + add_index "account_summaries", ["account_id", "language", "sensitive"], name: "idx_on_account_id_language_sensitive_250461e1eb" add_index "account_summaries", ["account_id"], name: "index_account_summaries_on_account_id", unique: true create_view "global_follow_recommendations", materialized: true, sql_definition: <<-SQL diff --git a/spec/requests/api/v1/suggestions_spec.rb b/spec/requests/api/v1/suggestions_spec.rb index 42b7f8662..dc89613fc 100644 --- a/spec/requests/api/v1/suggestions_spec.rb +++ b/spec/requests/api/v1/suggestions_spec.rb @@ -13,13 +13,12 @@ RSpec.describe 'Suggestions' do get '/api/v1/suggestions', headers: headers, params: params end - let(:bob) { Fabricate(:account) } - let(:jeff) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } + let(:jeff) { Fabricate(:account) } let(:params) { {} } before do - PotentialFriendshipTracker.record(user.account_id, bob.id, :reblog) - PotentialFriendshipTracker.record(user.account_id, jeff.id, :favourite) + Setting.bootstrap_timeline_accounts = [bob, jeff].map(&:acct).join(',') end it_behaves_like 'forbidden for wrong scope', 'write' @@ -65,17 +64,15 @@ RSpec.describe 'Suggestions' do delete "/api/v1/suggestions/#{jeff.id}", headers: headers end - let(:suggestions_source) { instance_double(AccountSuggestions::PastInteractionsSource, remove: nil) } - let(:bob) { Fabricate(:account) } - let(:jeff) { Fabricate(:account) } + let(:bob) { Fabricate(:account) } + let(:jeff) { Fabricate(:account) } + let(:scopes) { 'write' } before do - PotentialFriendshipTracker.record(user.account_id, bob.id, :reblog) - PotentialFriendshipTracker.record(user.account_id, jeff.id, :favourite) - allow(AccountSuggestions::PastInteractionsSource).to receive(:new).and_return(suggestions_source) + Setting.bootstrap_timeline_accounts = [bob, jeff].map(&:acct).join(',') end - it_behaves_like 'forbidden for wrong scope', 'write' + it_behaves_like 'forbidden for wrong scope', 'read' it 'returns http success' do subject @@ -86,8 +83,7 @@ RSpec.describe 'Suggestions' do it 'removes the specified suggestion' do subject - expect(suggestions_source).to have_received(:remove).with(user.account, jeff.id.to_s).once - expect(suggestions_source).to_not have_received(:remove).with(user.account, bob.id.to_s) + expect(FollowRecommendationMute.exists?(account: user.account, target_account: jeff)).to be true end context 'without an authorization header' do diff --git a/spec/workers/scheduler/follow_recommendations_scheduler_spec.rb b/spec/workers/scheduler/follow_recommendations_scheduler_spec.rb index 18d5260e4..246012e12 100644 --- a/spec/workers/scheduler/follow_recommendations_scheduler_spec.rb +++ b/spec/workers/scheduler/follow_recommendations_scheduler_spec.rb @@ -29,14 +29,12 @@ describe Scheduler::FollowRecommendationsScheduler do it 'creates recommendations' do expect { scheduled_run }.to change(FollowRecommendation, :count).from(0).to(target_accounts.size) - expect(redis.zrange('follow_recommendations:en', 0, -1)).to match_array(target_accounts.pluck(:id).map(&:to_s)) end end context 'when there are no accounts to recommend' do it 'does not create follow recommendations' do expect { scheduled_run }.to_not change(FollowRecommendation, :count) - expect(redis.zrange('follow_recommendations:en', 0, -1)).to be_empty end end end