From 4a77e477ee50f69160cecec25a68ee88b53dfcf8 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 28 May 2024 10:11:31 -0400 Subject: [PATCH] Consolidate account scopes for `LOWER` (index using) username/domain queries (#30451) --- app/models/account.rb | 2 + .../account_suggestions/setting_source.rb | 10 ++--- app/models/concerns/account/finder_concern.rb | 41 +++---------------- .../concerns/user/ldap_authenticable.rb | 2 +- app/services/report_service.rb | 2 +- app/validators/unique_username_validator.rb | 5 +-- 6 files changed, 14 insertions(+), 48 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 3c533822fd..8a990bb831 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -142,6 +142,8 @@ class Account < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { where(arel_table[:domain].eq(nil).or(arel_table[:domain].not_in(account.excluded_from_timeline_domains))) } scope :dormant, -> { joins(:account_stat).merge(AccountStat.without_recent_activity) } + scope :with_username, ->(value) { where arel_table[:username].lower.eq(value.to_s.downcase) } + scope :with_domain, ->(value) { where arel_table[:domain].lower.eq(value&.to_s&.downcase) } after_update_commit :trigger_update_webhooks diff --git a/app/models/account_suggestions/setting_source.rb b/app/models/account_suggestions/setting_source.rb index 9f3cd7bd3d..6143481723 100644 --- a/app/models/account_suggestions/setting_source.rb +++ b/app/models/account_suggestions/setting_source.rb @@ -3,7 +3,7 @@ class AccountSuggestions::SettingSource < AccountSuggestions::Source def get(account, limit: DEFAULT_LIMIT) if setting_enabled? - base_account_scope(account).where(setting_to_where_condition).limit(limit).pluck(:id).zip([key].cycle) + base_account_scope(account).merge(setting_to_where_condition).limit(limit).pluck(:id).zip([key].cycle) else [] end @@ -25,11 +25,9 @@ class AccountSuggestions::SettingSource < AccountSuggestions::Source def setting_to_where_condition usernames_and_domains.map do |(username, domain)| - Arel::Nodes::Grouping.new( - Account.arel_table[:username].lower.eq(username.downcase).and( - Account.arel_table[:domain].lower.eq(domain&.downcase) - ) - ) + Account + .with_username(username) + .with_domain(domain) end.reduce(:or) end diff --git a/app/models/concerns/account/finder_concern.rb b/app/models/concerns/account/finder_concern.rb index a7acff1cbb..249a7b5fd1 100644 --- a/app/models/concerns/account/finder_concern.rb +++ b/app/models/concerns/account/finder_concern.rb @@ -25,42 +25,11 @@ module Account::FinderConcern end def find_remote(username, domain) - AccountFinder.new(username, domain).account - end - end - - class AccountFinder - attr_reader :username, :domain - - def initialize(username, domain) - @username = username - @domain = domain - end - - def account - scoped_accounts.order(id: :asc).take - end - - private - - def scoped_accounts - Account.unscoped.tap do |scope| - scope.merge! with_usernames - scope.merge! matching_username - scope.merge! matching_domain - end - end - - def with_usernames - Account.where.not(Account.arel_table[:username].lower.eq '') - end - - def matching_username - Account.where(Account.arel_table[:username].lower.eq username.to_s.downcase) - end - - def matching_domain - Account.where(Account.arel_table[:domain].lower.eq(domain.nil? ? nil : domain.to_s.downcase)) + Account + .with_username(username) + .with_domain(domain) + .order(id: :asc) + .take end end end diff --git a/app/models/concerns/user/ldap_authenticable.rb b/app/models/concerns/user/ldap_authenticable.rb index c8e9fa9348..fc1ee78d09 100644 --- a/app/models/concerns/user/ldap_authenticable.rb +++ b/app/models/concerns/user/ldap_authenticable.rb @@ -22,7 +22,7 @@ module User::LdapAuthenticable safe_username = safe_username.gsub(keys, replacement) end - resource = joins(:account).merge(Account.where(Account.arel_table[:username].lower.eq safe_username.downcase)).take + resource = joins(:account).merge(Account.with_username(safe_username)).take if resource.blank? resource = new( diff --git a/app/services/report_service.rb b/app/services/report_service.rb index fe546c383e..dea6df7b0a 100644 --- a/app/services/report_service.rb +++ b/app/services/report_service.rb @@ -81,7 +81,7 @@ class ReportService < BaseService # If the account making reports is remote, it is likely anonymized so we have to relax the requirements for attaching statuses. domain = @source_account.domain.to_s.downcase - has_followers = @target_account.followers.where(Account.arel_table[:domain].lower.eq(domain)).exists? + has_followers = @target_account.followers.with_domain(domain).exists? visibility = has_followers ? %i(public unlisted private) : %i(public unlisted) scope = @target_account.statuses.with_discarded scope.merge!(scope.where(visibility: visibility).or(scope.where('EXISTS (SELECT 1 FROM mentions m JOIN accounts a ON m.account_id = a.id WHERE lower(a.domain) = ?)', domain))) diff --git a/app/validators/unique_username_validator.rb b/app/validators/unique_username_validator.rb index 09c8fadb55..c417e2f696 100644 --- a/app/validators/unique_username_validator.rb +++ b/app/validators/unique_username_validator.rb @@ -6,10 +6,7 @@ class UniqueUsernameValidator < ActiveModel::Validator def validate(account) return if account.username.blank? - normalized_username = account.username.downcase - normalized_domain = account.domain&.downcase - - scope = Account.where(Account.arel_table[:username].lower.eq normalized_username).where(Account.arel_table[:domain].lower.eq normalized_domain) + scope = Account.with_username(account.username).with_domain(account.domain) scope = scope.where.not(id: account.id) if account.persisted? account.errors.add(:username, :taken) if scope.exists?