From 83a90f20d78120098b413fdff0e7dc1943598ffc Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 18 Jan 2024 10:31:59 +0100 Subject: [PATCH 01/15] Update dependency async-mutex to v0.4.1 (#28797) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn.lock b/yarn.lock index 953147fe4c..2be132476d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4613,11 +4613,11 @@ __metadata: linkType: hard "async-mutex@npm:^0.4.0": - version: 0.4.0 - resolution: "async-mutex@npm:0.4.0" + version: 0.4.1 + resolution: "async-mutex@npm:0.4.1" dependencies: tslib: "npm:^2.4.0" - checksum: 6541695f80c1d6c5acbf3f7f04e8ff0733b3e029312c48d77bb95243fbe21fc5319f45ac3d72ce08551e6df83dc32440285ce9a3ac17bfc5d385ff0cc8ccd62a + checksum: 3c412736c0bc4a9a2cfd948276a8caab8686aa615866a5bd20986e616f8945320acb310058a17afa1b31b8de6f634a78b7ec2217a33d7559b38f68bb85a95854 languageName: node linkType: hard From 4c23297c04240ae3f4780f1614047be83a909c59 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 18 Jan 2024 09:33:21 +0000 Subject: [PATCH 02/15] Update dependency autoprefixer to v10.4.17 (#28794) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/yarn.lock b/yarn.lock index 2be132476d..6f8381db0e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4670,12 +4670,12 @@ __metadata: linkType: hard "autoprefixer@npm:^10.4.14": - version: 10.4.16 - resolution: "autoprefixer@npm:10.4.16" + version: 10.4.17 + resolution: "autoprefixer@npm:10.4.17" dependencies: - browserslist: "npm:^4.21.10" - caniuse-lite: "npm:^1.0.30001538" - fraction.js: "npm:^4.3.6" + browserslist: "npm:^4.22.2" + caniuse-lite: "npm:^1.0.30001578" + fraction.js: "npm:^4.3.7" normalize-range: "npm:^0.1.2" picocolors: "npm:^1.0.0" postcss-value-parser: "npm:^4.2.0" @@ -4683,7 +4683,7 @@ __metadata: postcss: ^8.1.0 bin: autoprefixer: bin/autoprefixer - checksum: e00256e754d481a026d928bca729b25954074dd142dbec022f0a7db0d3bbc0dc2e2dc7542e94fec22eff81e21fe140e6856448e2d9a002660cb1e2ad434daee0 + checksum: 1d21cc8edb7bf993682094ceed03a32c18f5293f071182a64c2c6defb44bbe91d576ad775d2347469a81997b80cea0bbc4ad3eeb5b12710f9feacf2e6c04bb51 languageName: node linkType: hard @@ -5230,7 +5230,7 @@ __metadata: languageName: node linkType: hard -"browserslist@npm:^4.0.0, browserslist@npm:^4.21.10, browserslist@npm:^4.22.2": +"browserslist@npm:^4.0.0, browserslist@npm:^4.22.2": version: 4.22.2 resolution: "browserslist@npm:4.22.2" dependencies: @@ -5456,10 +5456,10 @@ __metadata: languageName: node linkType: hard -"caniuse-lite@npm:^1.0.0, caniuse-lite@npm:^1.0.30001538, caniuse-lite@npm:^1.0.30001565": - version: 1.0.30001568 - resolution: "caniuse-lite@npm:1.0.30001568" - checksum: 13f01e5a2481134bd61cf565ce9fecbd8e107902927a0dcf534230a92191a81f1715792170f5f39719c767c3a96aa6df9917a8d5601f15bbd5e4041a8cfecc99 +"caniuse-lite@npm:^1.0.0, caniuse-lite@npm:^1.0.30001565, caniuse-lite@npm:^1.0.30001578": + version: 1.0.30001578 + resolution: "caniuse-lite@npm:1.0.30001578" + checksum: c3bd9c08a945cee4f0cc284a217ebe9c2613e04d5aef4b48f1871a779b1875c34286469eb8d7d94bd028b5a354613e676ad503b6bf8db20a2f154574bd5fde48 languageName: node linkType: hard @@ -8274,10 +8274,10 @@ __metadata: languageName: node linkType: hard -"fraction.js@npm:^4.3.6": - version: 4.3.6 - resolution: "fraction.js@npm:4.3.6" - checksum: d224bf62e350c4dbe66c6ac5ad9c4ec6d3c8e64c13323686dbebe7c8cc118491c297dca4961d3c93f847670794cb05e6d8b706f0e870846ab66a9c4491d0e914 +"fraction.js@npm:^4.3.7": + version: 4.3.7 + resolution: "fraction.js@npm:4.3.7" + checksum: df291391beea9ab4c263487ffd9d17fed162dbb736982dee1379b2a8cc94e4e24e46ed508c6d278aded9080ba51872f1bc5f3a5fd8d7c74e5f105b508ac28711 languageName: node linkType: hard From 89c9a4502d2463d2146de3bf5b32f728cdeb3e1c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 04:36:16 -0500 Subject: [PATCH 03/15] Fix `Rails/WhereExists` cop in account/interactions concern (#28789) --- .rubocop_todo.yml | 1 - app/models/concerns/account/interactions.rb | 26 ++++++++++----------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b962fbdddf..73ad0cac05 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -82,7 +82,6 @@ Rails/WhereExists: - 'app/lib/feed_manager.rb' - 'app/lib/status_cache_hydrator.rb' - 'app/lib/suspicious_sign_in_detector.rb' - - 'app/models/concerns/account/interactions.rb' - 'app/models/featured_tag.rb' - 'app/models/poll.rb' - 'app/models/session_activation.rb' diff --git a/app/models/concerns/account/interactions.rb b/app/models/concerns/account/interactions.rb index 351530c2f0..5b05c31e03 100644 --- a/app/models/concerns/account/interactions.rb +++ b/app/models/concerns/account/interactions.rb @@ -183,7 +183,7 @@ module Account::Interactions end def following?(other_account) - active_relationships.where(target_account: other_account).exists? + active_relationships.exists?(target_account: other_account) end def following_anyone? @@ -199,51 +199,51 @@ module Account::Interactions end def blocking?(other_account) - block_relationships.where(target_account: other_account).exists? + block_relationships.exists?(target_account: other_account) end def domain_blocking?(other_domain) - domain_blocks.where(domain: other_domain).exists? + domain_blocks.exists?(domain: other_domain) end def muting?(other_account) - mute_relationships.where(target_account: other_account).exists? + mute_relationships.exists?(target_account: other_account) end def muting_conversation?(conversation) - conversation_mutes.where(conversation: conversation).exists? + conversation_mutes.exists?(conversation: conversation) end def muting_notifications?(other_account) - mute_relationships.where(target_account: other_account, hide_notifications: true).exists? + mute_relationships.exists?(target_account: other_account, hide_notifications: true) end def muting_reblogs?(other_account) - active_relationships.where(target_account: other_account, show_reblogs: false).exists? + active_relationships.exists?(target_account: other_account, show_reblogs: false) end def requested?(other_account) - follow_requests.where(target_account: other_account).exists? + follow_requests.exists?(target_account: other_account) end def favourited?(status) - status.proper.favourites.where(account: self).exists? + status.proper.favourites.exists?(account: self) end def bookmarked?(status) - status.proper.bookmarks.where(account: self).exists? + status.proper.bookmarks.exists?(account: self) end def reblogged?(status) - status.proper.reblogs.where(account: self).exists? + status.proper.reblogs.exists?(account: self) end def pinned?(status) - status_pins.where(status: status).exists? + status_pins.exists?(status: status) end def endorsed?(account) - account_pins.where(target_account: account).exists? + account_pins.exists?(target_account: account) end def status_matches_filters(status) From 07e10e37477bdaa1bea30fbf2bebb05cf9ae793d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 04:36:59 -0500 Subject: [PATCH 04/15] Combine assertions about same setup in `Account#suspend!` spec (#28787) --- spec/models/account_spec.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index ab74579624..d360d934d6 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -9,14 +9,10 @@ RSpec.describe Account do let(:bob) { Fabricate(:account, username: 'bob') } describe '#suspend!' do - it 'marks the account as suspended' do - subject.suspend! - expect(subject.suspended?).to be true - end - - it 'creates a deletion request' do - subject.suspend! - expect(AccountDeletionRequest.where(account: subject).exists?).to be true + it 'marks the account as suspended and creates a deletion request' do + expect { subject.suspend! } + .to change(subject, :suspended?).from(false).to(true) + .and(change { AccountDeletionRequest.exists?(account: subject) }.from(false).to(true)) end context 'when the account is of a local user' do From 6c5a2d51bc30e0a0d46160952295f743c6fa4b2d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 05:07:49 -0500 Subject: [PATCH 05/15] Reduced repeated setup in `PurgeDomainService` spec (#28786) --- spec/services/purge_domain_service_spec.rb | 30 +++++++++++----------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/services/purge_domain_service_spec.rb b/spec/services/purge_domain_service_spec.rb index e96618310b..6d8af14deb 100644 --- a/spec/services/purge_domain_service_spec.rb +++ b/spec/services/purge_domain_service_spec.rb @@ -5,25 +5,25 @@ require 'rails_helper' RSpec.describe PurgeDomainService, type: :service do subject { described_class.new } - let!(:old_account) { Fabricate(:account, domain: 'obsolete.org') } - let!(:old_status_plain) { Fabricate(:status, account: old_account) } - let!(:old_status_with_attachment) { Fabricate(:status, account: old_account) } - let!(:old_attachment) { Fabricate(:media_attachment, account: old_account, status: old_status_with_attachment, file: attachment_fixture('attachment.jpg')) } + let(:domain) { 'obsolete.org' } + let!(:account) { Fabricate(:account, domain: domain) } + let!(:status_plain) { Fabricate(:status, account: account) } + let!(:status_with_attachment) { Fabricate(:status, account: account) } + let!(:attachment) { Fabricate(:media_attachment, account: account, status: status_with_attachment, file: attachment_fixture('attachment.jpg')) } describe 'for a suspension' do - before do - subject.call('obsolete.org') + it 'refreshes instance view and removes associated records' do + expect { subject.call(domain) } + .to change { domain_instance_exists }.from(true).to(false) + + expect { account.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { status_plain.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { status_with_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { attachment.reload }.to raise_exception ActiveRecord::RecordNotFound end - it 'removes the remote accounts\'s statuses and media attachments' do - expect { old_account.reload }.to raise_exception ActiveRecord::RecordNotFound - expect { old_status_plain.reload }.to raise_exception ActiveRecord::RecordNotFound - expect { old_status_with_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound - expect { old_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound - end - - it 'refreshes instances view' do - expect(Instance.where(domain: 'obsolete.org').exists?).to be false + def domain_instance_exists + Instance.exists?(domain: domain) end end end From 3d82040b26846c5431eaff1b997b17a55a6256da Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 05:11:10 -0500 Subject: [PATCH 06/15] Reduced repeated setup in `UnallowDomainService` spec (#28785) --- spec/services/unallow_domain_service_spec.rb | 53 +++++++++----------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/spec/services/unallow_domain_service_spec.rb b/spec/services/unallow_domain_service_spec.rb index 6ac6bc4016..383977d352 100644 --- a/spec/services/unallow_domain_service_spec.rb +++ b/spec/services/unallow_domain_service_spec.rb @@ -5,12 +5,13 @@ require 'rails_helper' RSpec.describe UnallowDomainService, type: :service do subject { described_class.new } - let!(:bad_account) { Fabricate(:account, username: 'badguy666', domain: 'evil.org') } + let(:bad_domain) { 'evil.org' } + let!(:bad_account) { Fabricate(:account, username: 'badguy666', domain: bad_domain) } let!(:bad_status_harassment) { Fabricate(:status, account: bad_account, text: 'You suck') } let!(:bad_status_mean) { Fabricate(:status, account: bad_account, text: 'Hahaha') } let!(:bad_attachment) { Fabricate(:media_attachment, account: bad_account, status: bad_status_mean, file: attachment_fixture('attachment.jpg')) } - let!(:already_banned_account) { Fabricate(:account, username: 'badguy', domain: 'evil.org', suspended: true, silenced: true) } - let!(:domain_allow) { Fabricate(:domain_allow, domain: 'evil.org') } + let!(:already_banned_account) { Fabricate(:account, username: 'badguy', domain: bad_domain, suspended: true, silenced: true) } + let!(:domain_allow) { Fabricate(:domain_allow, domain: bad_domain) } context 'with limited federation mode', :sidekiq_inline do before do @@ -18,23 +19,15 @@ RSpec.describe UnallowDomainService, type: :service do end describe '#call' do - before do - subject.call(domain_allow) - end + it 'makes the domain not allowed and removes accounts from that domain' do + expect { subject.call(domain_allow) } + .to change { bad_domain_allowed }.from(true).to(false) + .and change { bad_domain_account_exists }.from(true).to(false) - it 'removes the allowed domain' do - expect(DomainAllow.allowed?('evil.org')).to be false - end - - it 'removes remote accounts from that domain' do expect { already_banned_account.reload }.to raise_error(ActiveRecord::RecordNotFound) - expect(Account.where(domain: 'evil.org').exists?).to be false - end - - it 'removes the remote accounts\'s statuses and media attachments' do - expect { bad_status_harassment.reload }.to raise_exception ActiveRecord::RecordNotFound - expect { bad_status_mean.reload }.to raise_exception ActiveRecord::RecordNotFound - expect { bad_attachment.reload }.to raise_exception ActiveRecord::RecordNotFound + expect { bad_status_harassment.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { bad_status_mean.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { bad_attachment.reload }.to raise_error(ActiveRecord::RecordNotFound) end end end @@ -45,23 +38,23 @@ RSpec.describe UnallowDomainService, type: :service do end describe '#call' do - before do - subject.call(domain_allow) - end + it 'makes the domain not allowed but preserves accounts from the domain' do + expect { subject.call(domain_allow) } + .to change { bad_domain_allowed }.from(true).to(false) + .and not_change { bad_domain_account_exists }.from(true) - it 'removes the allowed domain' do - expect(DomainAllow.allowed?('evil.org')).to be false - end - - it 'does not remove accounts from that domain' do - expect(Account.where(domain: 'evil.org').exists?).to be true - end - - it 'removes the remote accounts\'s statuses and media attachments' do expect { bad_status_harassment.reload }.to_not raise_error expect { bad_status_mean.reload }.to_not raise_error expect { bad_attachment.reload }.to_not raise_error end end end + + def bad_domain_allowed + DomainAllow.allowed?(bad_domain) + end + + def bad_domain_account_exists + Account.exists?(domain: bad_domain) + end end From da31792ac7768299d32419764bdc118adf7e1ea5 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 07:22:12 -0500 Subject: [PATCH 07/15] Fix `Rails/WhereExists` cop in FeaturedTag model (#28791) --- .rubocop_todo.yml | 1 - app/models/featured_tag.rb | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 73ad0cac05..eaa86ad15a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -82,7 +82,6 @@ Rails/WhereExists: - 'app/lib/feed_manager.rb' - 'app/lib/status_cache_hydrator.rb' - 'app/lib/suspicious_sign_in_detector.rb' - - 'app/models/featured_tag.rb' - 'app/models/poll.rb' - 'app/models/session_activation.rb' - 'app/models/status.rb' diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index 7c36aa8b0b..63cd674765 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -66,6 +66,10 @@ class FeaturedTag < ApplicationRecord end def validate_tag_uniqueness - errors.add(:name, :taken) if FeaturedTag.by_name(name).where(account_id: account_id).exists? + errors.add(:name, :taken) if tag_already_featured_for_account? + end + + def tag_already_featured_for_account? + FeaturedTag.by_name(name).exists?(account_id: account_id) end end From aaa6f2e9302252b70d1e430ed5b4e6689b827d78 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 07:29:41 -0500 Subject: [PATCH 08/15] Group common `class_name` options in associations (#28779) --- app/models/appeal.rb | 7 +++++-- app/models/email_domain_block.rb | 6 ++++-- app/models/poll.rb | 7 +++++-- app/models/report.rb | 9 ++++++--- app/models/status.rb | 6 ++++-- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/app/models/appeal.rb b/app/models/appeal.rb index f1290ad01a..395056b76f 100644 --- a/app/models/appeal.rb +++ b/app/models/appeal.rb @@ -20,8 +20,11 @@ class Appeal < ApplicationRecord belongs_to :account belongs_to :strike, class_name: 'AccountWarning', foreign_key: 'account_warning_id', inverse_of: :appeal - belongs_to :approved_by_account, class_name: 'Account', optional: true - belongs_to :rejected_by_account, class_name: 'Account', optional: true + + with_options class_name: 'Account', optional: true do + belongs_to :approved_by_account + belongs_to :rejected_by_account + end validates :text, presence: true, length: { maximum: 2_000 } validates :account_warning_id, uniqueness: true diff --git a/app/models/email_domain_block.rb b/app/models/email_domain_block.rb index f1b14c8b08..40be59420a 100644 --- a/app/models/email_domain_block.rb +++ b/app/models/email_domain_block.rb @@ -21,8 +21,10 @@ class EmailDomainBlock < ApplicationRecord include DomainNormalizable include Paginable - belongs_to :parent, class_name: 'EmailDomainBlock', optional: true - has_many :children, class_name: 'EmailDomainBlock', foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy + with_options class_name: 'EmailDomainBlock' do + belongs_to :parent, optional: true + has_many :children, foreign_key: :parent_id, inverse_of: :parent, dependent: :destroy + end validates :domain, presence: true, uniqueness: true, domain: true diff --git a/app/models/poll.rb b/app/models/poll.rb index 72f04f00a7..37149c3d86 100644 --- a/app/models/poll.rb +++ b/app/models/poll.rb @@ -27,8 +27,11 @@ class Poll < ApplicationRecord belongs_to :status has_many :votes, class_name: 'PollVote', inverse_of: :poll, dependent: :delete_all - has_many :voters, -> { group('accounts.id') }, through: :votes, class_name: 'Account', source: :account - has_many :local_voters, -> { group('accounts.id').merge(Account.local) }, through: :votes, class_name: 'Account', source: :account + + with_options class_name: 'Account', source: :account, through: :votes do + has_many :voters, -> { group('accounts.id') } + has_many :local_voters, -> { group('accounts.id').merge(Account.local) } + end has_many :notifications, as: :activity, dependent: :destroy diff --git a/app/models/report.rb b/app/models/report.rb index c565362cc6..126701b3d6 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -29,9 +29,12 @@ class Report < ApplicationRecord rate_limit by: :account, family: :reports belongs_to :account - belongs_to :target_account, class_name: 'Account' - belongs_to :action_taken_by_account, class_name: 'Account', optional: true - belongs_to :assigned_account, class_name: 'Account', optional: true + + with_options class_name: 'Account' do + belongs_to :target_account + belongs_to :action_taken_by_account, optional: true + belongs_to :assigned_account, optional: true + end has_many :notes, class_name: 'ReportNote', inverse_of: :report, dependent: :destroy has_many :notifications, as: :activity, dependent: :destroy diff --git a/app/models/status.rb b/app/models/status.rb index a498da288a..9a2169f995 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -59,8 +59,10 @@ class Status < ApplicationRecord belongs_to :conversation, optional: true belongs_to :preloadable_poll, class_name: 'Poll', foreign_key: 'poll_id', optional: true, inverse_of: false - belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies, optional: true - belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, optional: true + with_options class_name: 'Status', optional: true do + belongs_to :thread, foreign_key: 'in_reply_to_id', inverse_of: :replies + belongs_to :reblog, foreign_key: 'reblog_of_id', inverse_of: :reblogs + end has_many :favourites, inverse_of: :status, dependent: :destroy has_many :bookmarks, inverse_of: :status, dependent: :destroy From 81e4e65610932841750bdbef8d96961165b8eb0c Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 07:29:54 -0500 Subject: [PATCH 09/15] Update links to upstream migration helpers, remove unused methods (#28781) --- lib/mastodon/migration_helpers.rb | 161 ++---------------------------- 1 file changed, 9 insertions(+), 152 deletions(-) diff --git a/lib/mastodon/migration_helpers.rb b/lib/mastodon/migration_helpers.rb index 1a2ce6420f..a713f42d41 100644 --- a/lib/mastodon/migration_helpers.rb +++ b/lib/mastodon/migration_helpers.rb @@ -8,15 +8,15 @@ # shorten temporary column names. # Documentation on using these functions (and why one might do so): -# https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/what_requires_downtime.md +# https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/doc/development/database/avoiding_downtime_in_migrations.md -# The file itself: -# https://gitlab.com/gitlab-org/gitlab-ce/blob/master/lib/gitlab/database/migration_helpers.rb +# The original file (since updated): +# https://gitlab.com/gitlab-org/gitlab-foss/-/blob/master/lib/gitlab/database/migration_helpers.rb # It is licensed as follows: -# Copyright (c) 2011-2017 GitLab B.V. - +# Copyright (c) 2011-present GitLab B.V. +# # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal # in the Software without restriction, including without limitation the rights @@ -24,16 +24,16 @@ # copies of the Software, and to permit persons to whom the Software is # furnished to do so, subject to the following conditions: -# The above copyright notice and this permission notice shall be included in -# all copies or substantial portions of the Software. +# The above copyright notice and this permission notice shall be included in all +# copies or substantial portions of the Software. # THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR # IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, # FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE # AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -# THE SOFTWARE. +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. # This is bad form, but there are enough differences that it's impractical to do # otherwise: @@ -77,37 +77,12 @@ module Mastodon end end - BACKGROUND_MIGRATION_BATCH_SIZE = 1000 # Number of rows to process per job - BACKGROUND_MIGRATION_JOB_BUFFER_SIZE = 1000 # Number of jobs to bulk queue at a time - # Gets an estimated number of rows for a table def estimate_rows_in_table(table_name) exec_query('SELECT reltuples FROM pg_class WHERE relname = ' + "'#{table_name}'").to_a.first['reltuples'] end - # Adds `created_at` and `updated_at` columns with timezone information. - # - # This method is an improved version of Rails' built-in method `add_timestamps`. - # - # Available options are: - # default - The default value for the column. - # null - When set to `true` the column will allow NULL values. - # The default is to not allow NULL values. - def add_timestamps_with_timezone(table_name, **options) - options[:null] = false if options[:null].nil? - - [:created_at, :updated_at].each do |column_name| - if options[:default] && transaction_open? - raise '`add_timestamps_with_timezone` with default value cannot be run inside a transaction. ' \ - 'You can disable transactions by calling `disable_ddl_transaction!` ' \ - 'in the body of your migration class' - end - - add_column(table_name, column_name, :datetime_with_timezone, **options) - end - end - # Creates a new index, concurrently when supported # # On PostgreSQL this method creates an index concurrently, on MySQL this @@ -746,39 +721,6 @@ module Mastodon rename_index table_name, "#{index_name}_new", index_name end - # This will replace the first occurrence of a string in a column with - # the replacement - # On postgresql we can use `regexp_replace` for that. - # On mysql we find the location of the pattern, and overwrite it - # with the replacement - def replace_sql(column, pattern, replacement) - quoted_pattern = Arel::Nodes::Quoted.new(pattern.to_s) - quoted_replacement = Arel::Nodes::Quoted.new(replacement.to_s) - - replace = Arel::Nodes::NamedFunction - .new("regexp_replace", [column, quoted_pattern, quoted_replacement]) - Arel::Nodes::SqlLiteral.new(replace.to_sql) - end - - def remove_foreign_key_without_error(*args) - remove_foreign_key(*args) - rescue ArgumentError - end - - def sidekiq_queue_migrate(queue_from, to:) - while sidekiq_queue_length(queue_from) > 0 - Sidekiq.redis do |conn| - conn.rpoplpush "queue:#{queue_from}", "queue:#{to}" - end - end - end - - def sidekiq_queue_length(queue_name) - Sidekiq.redis do |conn| - conn.llen("queue:#{queue_name}") - end - end - def check_trigger_permissions!(table) unless Grant.create_and_execute_trigger?(table) dbname = ActiveRecord::Base.configurations[Rails.env]['database'] @@ -799,91 +741,6 @@ into similar problems in the future (e.g. when new tables are created). end end - # Bulk queues background migration jobs for an entire table, batched by ID range. - # "Bulk" meaning many jobs will be pushed at a time for efficiency. - # If you need a delay interval per job, then use `queue_background_migration_jobs_by_range_at_intervals`. - # - # model_class - The table being iterated over - # job_class_name - The background migration job class as a string - # batch_size - The maximum number of rows per job - # - # Example: - # - # class Route < ActiveRecord::Base - # include EachBatch - # self.table_name = 'routes' - # end - # - # bulk_queue_background_migration_jobs_by_range(Route, 'ProcessRoutes') - # - # Where the model_class includes EachBatch, and the background migration exists: - # - # class Gitlab::BackgroundMigration::ProcessRoutes - # def perform(start_id, end_id) - # # do something - # end - # end - def bulk_queue_background_migration_jobs_by_range(model_class, job_class_name, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) - raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') - - jobs = [] - - model_class.each_batch(of: batch_size) do |relation| - start_id, end_id = relation.pluck('MIN(id), MAX(id)').first - - if jobs.length >= BACKGROUND_MIGRATION_JOB_BUFFER_SIZE - # Note: This code path generally only helps with many millions of rows - # We push multiple jobs at a time to reduce the time spent in - # Sidekiq/Redis operations. We're using this buffer based approach so we - # don't need to run additional queries for every range. - BackgroundMigrationWorker.perform_bulk(jobs) - jobs.clear - end - - jobs << [job_class_name, [start_id, end_id]] - end - - BackgroundMigrationWorker.perform_bulk(jobs) unless jobs.empty? - end - - # Queues background migration jobs for an entire table, batched by ID range. - # Each job is scheduled with a `delay_interval` in between. - # If you use a small interval, then some jobs may run at the same time. - # - # model_class - The table being iterated over - # job_class_name - The background migration job class as a string - # delay_interval - The duration between each job's scheduled time (must respond to `to_f`) - # batch_size - The maximum number of rows per job - # - # Example: - # - # class Route < ActiveRecord::Base - # include EachBatch - # self.table_name = 'routes' - # end - # - # queue_background_migration_jobs_by_range_at_intervals(Route, 'ProcessRoutes', 1.minute) - # - # Where the model_class includes EachBatch, and the background migration exists: - # - # class Gitlab::BackgroundMigration::ProcessRoutes - # def perform(start_id, end_id) - # # do something - # end - # end - def queue_background_migration_jobs_by_range_at_intervals(model_class, job_class_name, delay_interval, batch_size: BACKGROUND_MIGRATION_BATCH_SIZE) - raise "#{model_class} does not have an ID to use for batch ranges" unless model_class.column_names.include?('id') - - model_class.each_batch(of: batch_size) do |relation, index| - start_id, end_id = relation.pluck('MIN(id), MAX(id)').first - - # `BackgroundMigrationWorker.bulk_perform_in` schedules all jobs for - # the same time, which is not helpful in most cases where we wish to - # spread the work over time. - BackgroundMigrationWorker.perform_in(delay_interval * index, job_class_name, [start_id, end_id]) - end - end - private # https://github.com/rails/rails/blob/v5.2.0/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L678-L684 From 9fb9ef418a58dbeeb568050a72e697f17c85afc3 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 08:55:44 -0500 Subject: [PATCH 10/15] Fix `Rails/WhereExists` cop in User model (#28792) --- .rubocop_todo.yml | 1 - app/models/user.rb | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eaa86ad15a..87120daef2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -85,7 +85,6 @@ Rails/WhereExists: - 'app/models/poll.rb' - 'app/models/session_activation.rb' - 'app/models/status.rb' - - 'app/models/user.rb' - 'app/policies/status_policy.rb' - 'app/serializers/rest/announcement_serializer.rb' - 'app/serializers/rest/tag_serializer.rb' diff --git a/app/models/user.rb b/app/models/user.rb index 5c90af56d2..70c24336f3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -434,7 +434,7 @@ class User < ApplicationRecord end def sign_up_from_ip_requires_approval? - !sign_up_ip.nil? && IpBlock.where(severity: :sign_up_requires_approval).where('ip >>= ?', sign_up_ip.to_s).exists? + sign_up_ip.present? && IpBlock.sign_up_requires_approval.exists?(['ip >>= ?', sign_up_ip.to_s]) end def sign_up_email_requires_approval? From 2115bc52e47e66a4a15fe3073df9d39da0bf6e3e Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 09:53:29 -0500 Subject: [PATCH 11/15] Order by sql in `CLI::Maintenance` task (#28289) --- lib/mastodon/cli/maintenance.rb | 44 ++++++++++++++++----------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/mastodon/cli/maintenance.rb b/lib/mastodon/cli/maintenance.rb index f37662aa06..e2ea866152 100644 --- a/lib/mastodon/cli/maintenance.rb +++ b/lib/mastodon/cli/maintenance.rb @@ -223,7 +223,7 @@ module Mastodon::CLI say 'Deduplicating accounts… for local accounts, you will be asked to chose which account to keep unchanged.' find_duplicate_accounts.each do |row| - accounts = Account.where(id: row['ids'].split(',')).to_a + accounts = Account.where(id: row['ids'].split(',')) if accounts.first.local? deduplicate_local_accounts!(accounts) @@ -275,7 +275,7 @@ module Mastodon::CLI def deduplicate_users_process_email ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users GROUP BY email HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse + users = User.where(id: row['ids'].split(',')).order(updated_at: :desc).to_a ref_user = users.shift say "Multiple users registered with e-mail address #{ref_user.email}.", :yellow say "e-mail will be disabled for the following accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow @@ -289,7 +289,7 @@ module Mastodon::CLI def deduplicate_users_process_confirmation_token ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE confirmation_token IS NOT NULL GROUP BY confirmation_token HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:created_at).reverse.drop(1) + users = User.where(id: row['ids'].split(',')).order(created_at: :desc).to_a.drop(1) say "Unsetting confirmation token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| @@ -301,7 +301,7 @@ module Mastodon::CLI def deduplicate_users_process_remember_token if migrator_version < 2022_01_18_183010 ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE remember_token IS NOT NULL GROUP BY remember_token HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) + users = User.where(id: row['ids'].split(',')).order(updated_at: :desc).to_a.drop(1) say "Unsetting remember token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| @@ -313,7 +313,7 @@ module Mastodon::CLI def deduplicate_users_process_password_token ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM users WHERE reset_password_token IS NOT NULL GROUP BY reset_password_token HAVING count(*) > 1").each do |row| - users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse.drop(1) + users = User.where(id: row['ids'].split(',')).order(updated_at: :desc).to_a.drop(1) say "Unsetting password reset token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| @@ -341,7 +341,7 @@ module Mastodon::CLI say 'Removing duplicate account identity proofs…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM account_identity_proofs GROUP BY account_id, provider, provider_username HAVING count(*) > 1").each do |row| - AccountIdentityProof.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + AccountIdentityProof.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring account identity proofs indexes…' @@ -355,7 +355,7 @@ module Mastodon::CLI say 'Removing duplicate announcement reactions…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM announcement_reactions GROUP BY account_id, announcement_id, name HAVING count(*) > 1").each do |row| - AnnouncementReaction.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + AnnouncementReaction.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring announcement_reactions indexes…' @@ -367,7 +367,7 @@ module Mastodon::CLI say 'Deduplicating conversations…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM conversations WHERE uri IS NOT NULL GROUP BY uri HAVING count(*) > 1").each do |row| - conversations = Conversation.where(id: row['ids'].split(',')).sort_by(&:id).reverse + conversations = Conversation.where(id: row['ids'].split(',')).order(id: :desc).to_a ref_conversation = conversations.shift @@ -390,7 +390,7 @@ module Mastodon::CLI say 'Deduplicating custom_emojis…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM custom_emojis GROUP BY shortcode, domain HAVING count(*) > 1").each do |row| - emojis = CustomEmoji.where(id: row['ids'].split(',')).sort_by(&:id).reverse + emojis = CustomEmoji.where(id: row['ids'].split(',')).order(id: :desc).to_a ref_emoji = emojis.shift @@ -409,7 +409,7 @@ module Mastodon::CLI say 'Deduplicating custom_emoji_categories…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM custom_emoji_categories GROUP BY name HAVING count(*) > 1").each do |row| - categories = CustomEmojiCategory.where(id: row['ids'].split(',')).sort_by(&:id).reverse + categories = CustomEmojiCategory.where(id: row['ids'].split(',')).order(id: :desc).to_a ref_category = categories.shift @@ -428,7 +428,7 @@ module Mastodon::CLI say 'Deduplicating domain_allows…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM domain_allows GROUP BY domain HAVING count(*) > 1").each do |row| - DomainAllow.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + DomainAllow.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring domain_allows indexes…' @@ -466,7 +466,7 @@ module Mastodon::CLI say 'Deduplicating unavailable_domains…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM unavailable_domains GROUP BY domain HAVING count(*) > 1").each do |row| - UnavailableDomain.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + UnavailableDomain.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring unavailable_domains indexes…' @@ -478,7 +478,7 @@ module Mastodon::CLI say 'Deduplicating email_domain_blocks…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM email_domain_blocks GROUP BY domain HAVING count(*) > 1").each do |row| - domain_blocks = EmailDomainBlock.where(id: row['ids'].split(',')).sort_by { |b| b.parent.nil? ? 1 : 0 }.to_a + domain_blocks = EmailDomainBlock.where(id: row['ids'].split(',')).order(EmailDomainBlock.arel_table[:parent_id].asc.nulls_first).to_a domain_blocks.drop(1).each(&:destroy) end @@ -507,7 +507,7 @@ module Mastodon::CLI say 'Deduplicating preview_cards…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM preview_cards GROUP BY url HAVING count(*) > 1").each do |row| - PreviewCard.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + PreviewCard.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring preview_cards indexes…' @@ -519,7 +519,7 @@ module Mastodon::CLI say 'Deduplicating statuses…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM statuses WHERE uri IS NOT NULL GROUP BY uri HAVING count(*) > 1").each do |row| - statuses = Status.where(id: row['ids'].split(',')).sort_by(&:id) + statuses = Status.where(id: row['ids'].split(',')).order(id: :asc).to_a ref_status = statuses.shift statuses.each do |status| merge_statuses!(ref_status, status) if status.account_id == ref_status.account_id @@ -541,7 +541,7 @@ module Mastodon::CLI say 'Deduplicating tags…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM tags GROUP BY lower((name)::text) HAVING count(*) > 1").each do |row| - tags = Tag.where(id: row['ids'].split(',')).sort_by { |t| [t.usable?, t.trendable?, t.listable?].count(false) } + tags = Tag.where(id: row['ids'].split(',')).order(Arel.sql('(usable::int + trendable::int + listable::int) desc')).to_a ref_tag = tags.shift tags.each do |tag| merge_tags!(ref_tag, tag) @@ -564,7 +564,7 @@ module Mastodon::CLI say 'Deduplicating webauthn_credentials…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM webauthn_credentials GROUP BY external_id HAVING count(*) > 1").each do |row| - WebauthnCredential.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + WebauthnCredential.where(id: row['ids'].split(',')).order(id: :desc).to_a.drop(1).each(&:destroy) end say 'Restoring webauthn_credentials indexes…' @@ -578,7 +578,7 @@ module Mastodon::CLI say 'Deduplicating webhooks…' ActiveRecord::Base.connection.select_all("SELECT string_agg(id::text, ',') AS ids FROM webhooks GROUP BY url HAVING count(*) > 1").each do |row| - Webhook.where(id: row['ids'].split(',')).sort_by(&:id).reverse.drop(1).each(&:destroy) + Webhook.where(id: row['ids'].split(',')).order(id: :desc).drop(1).each(&:destroy) end say 'Restoring webhooks indexes…' @@ -590,8 +590,8 @@ module Mastodon::CLI SoftwareUpdate.delete_all end - def deduplicate_local_accounts!(accounts) - accounts = accounts.sort_by(&:id).reverse + def deduplicate_local_accounts!(scope) + accounts = scope.order(id: :desc).to_a say "Multiple local accounts were found for username '#{accounts.first.username}'.", :yellow say 'All those accounts are distinct accounts but only the most recently-created one is fully-functional.', :yellow @@ -629,8 +629,8 @@ module Mastodon::CLI end end - def deduplicate_remote_accounts!(accounts) - accounts = accounts.sort_by(&:updated_at).reverse + def deduplicate_remote_accounts!(scope) + accounts = scope.order(updated_at: :desc).to_a reference_account = accounts.shift From 0b853678a45df06f5b9453217a9bf72f23ee322d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 10:57:10 -0500 Subject: [PATCH 12/15] Add coverage for `api/v1/peers/search` endpoint and extract controller query to Instance scope (#28796) --- .../api/v1/peers/search_controller.rb | 15 +++-- app/models/instance.rb | 1 + spec/requests/api/v1/peers/search_spec.rb | 59 +++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 spec/requests/api/v1/peers/search_spec.rb diff --git a/app/controllers/api/v1/peers/search_controller.rb b/app/controllers/api/v1/peers/search_controller.rb index 0c503d9bc5..1780554c5d 100644 --- a/app/controllers/api/v1/peers/search_controller.rb +++ b/app/controllers/api/v1/peers/search_controller.rb @@ -27,7 +27,7 @@ class Api::V1::Peers::SearchController < Api::BaseController @domains = InstancesIndex.query(function_score: { query: { prefix: { - domain: TagManager.instance.normalize_domain(params[:q].strip), + domain: normalized_domain, }, }, @@ -37,11 +37,18 @@ class Api::V1::Peers::SearchController < Api::BaseController }, }).limit(10).pluck(:domain) else - domain = params[:q].strip - domain = TagManager.instance.normalize_domain(domain) - @domains = Instance.searchable.where(Instance.arel_table[:domain].matches("#{Instance.sanitize_sql_like(domain)}%", false, true)).limit(10).pluck(:domain) + domain = normalized_domain + @domains = Instance.searchable.domain_starts_with(domain).limit(10).pluck(:domain) end rescue Addressable::URI::InvalidURIError @domains = [] end + + def normalized_domain + TagManager.instance.normalize_domain(query_value) + end + + def query_value + params[:q].strip + end end diff --git a/app/models/instance.rb b/app/models/instance.rb index 17ee0cbb1e..8f8d87c62a 100644 --- a/app/models/instance.rb +++ b/app/models/instance.rb @@ -23,6 +23,7 @@ class Instance < ApplicationRecord scope :searchable, -> { where.not(domain: DomainBlock.select(:domain)) } scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } + scope :domain_starts_with, ->(value) { where(arel_table[:domain].matches("#{sanitize_sql_like(value)}%", false, true)) } scope :by_domain_and_subdomains, ->(domain) { where("reverse('.' || domain) LIKE reverse(?)", "%.#{domain}") } def self.refresh diff --git a/spec/requests/api/v1/peers/search_spec.rb b/spec/requests/api/v1/peers/search_spec.rb new file mode 100644 index 0000000000..dcdea387a5 --- /dev/null +++ b/spec/requests/api/v1/peers/search_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'API Peers Search' do + describe 'GET /api/v1/peers/search' do + context 'when peers api is disabled' do + before do + Setting.peers_api_enabled = false + end + + it 'returns http not found response' do + get '/api/v1/peers/search' + + expect(response) + .to have_http_status(404) + end + end + + context 'with no search param' do + it 'returns http success and empty response' do + get '/api/v1/peers/search' + + expect(response) + .to have_http_status(200) + expect(body_as_json) + .to be_blank + end + end + + context 'with invalid search param' do + it 'returns http success and empty response' do + get '/api/v1/peers/search', params: { q: 'ftp://Invalid-Host!!.valüe' } + + expect(response) + .to have_http_status(200) + expect(body_as_json) + .to be_blank + end + end + + context 'with search param' do + let!(:account) { Fabricate(:account, domain: 'host.example') } + + before { Instance.refresh } + + it 'returns http success and json with known domains' do + get '/api/v1/peers/search', params: { q: 'host.example' } + + expect(response) + .to have_http_status(200) + expect(body_as_json.size) + .to eq(1) + expect(body_as_json.first) + .to eq(account.domain) + end + end + end +end From d0b3bc23d739e38ca7b7ac9a7f20f1f2a751563b Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 11:11:04 -0500 Subject: [PATCH 13/15] Remove unused `matches_domain` scopes on Account, DomainAllow, DomainBlock (#28803) --- app/models/account.rb | 1 - app/models/domain_allow.rb | 2 -- app/models/domain_block.rb | 1 - spec/models/domain_allow_spec.rb | 20 +++++++++++--------- 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 0fca7ce4bd..c17de682e3 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -125,7 +125,6 @@ class Account < ApplicationRecord scope :alphabetic, -> { order(domain: :asc, username: :asc) } scope :matches_username, ->(value) { where('lower((username)::text) LIKE lower(?)', "#{value}%") } scope :matches_display_name, ->(value) { where(arel_table[:display_name].matches("#{value}%")) } - scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :without_unapproved, -> { left_outer_joins(:user).merge(User.approved.confirmed).or(remote) } scope :searchable, -> { without_unapproved.without_suspended.where(moved_to_account_id: nil) } scope :discoverable, -> { searchable.without_silenced.where(discoverable: true).joins(:account_stat) } diff --git a/app/models/domain_allow.rb b/app/models/domain_allow.rb index ce9597b4d1..47ada7ac23 100644 --- a/app/models/domain_allow.rb +++ b/app/models/domain_allow.rb @@ -17,8 +17,6 @@ class DomainAllow < ApplicationRecord validates :domain, presence: true, uniqueness: true, domain: true - scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } - def to_log_human_identifier domain end diff --git a/app/models/domain_block.rb b/app/models/domain_block.rb index 8da099256a..a05db099a8 100644 --- a/app/models/domain_block.rb +++ b/app/models/domain_block.rb @@ -28,7 +28,6 @@ class DomainBlock < ApplicationRecord has_many :accounts, foreign_key: :domain, primary_key: :domain, inverse_of: false, dependent: nil delegate :count, to: :accounts, prefix: true - scope :matches_domain, ->(value) { where(arel_table[:domain].matches("%#{value}%")) } scope :with_user_facing_limitations, -> { where(severity: [:silence, :suspend]) } scope :with_limitations, -> { where(severity: [:silence, :suspend]).or(where(reject_media: true)) } scope :by_severity, -> { in_order_of(:severity, %w(noop silence suspend)).order(:domain) } diff --git a/spec/models/domain_allow_spec.rb b/spec/models/domain_allow_spec.rb index 49e16376ea..12504211a1 100644 --- a/spec/models/domain_allow_spec.rb +++ b/spec/models/domain_allow_spec.rb @@ -3,16 +3,18 @@ require 'rails_helper' describe DomainAllow do - describe 'scopes' do - describe 'matches_domain' do - let(:domain) { Fabricate(:domain_allow, domain: 'example.com') } - let(:other_domain) { Fabricate(:domain_allow, domain: 'example.biz') } + describe 'Validations' do + it 'is invalid without a domain' do + domain_allow = Fabricate.build(:domain_allow, domain: nil) + domain_allow.valid? + expect(domain_allow).to model_have_error_on_field(:domain) + end - it 'returns the correct records' do - results = described_class.matches_domain('example.com') - - expect(results).to eq([domain]) - end + it 'is invalid if the same normalized domain already exists' do + _domain_allow = Fabricate(:domain_allow, domain: 'にゃん') + domain_allow_with_normalized_value = Fabricate.build(:domain_allow, domain: 'xn--r9j5b5b') + domain_allow_with_normalized_value.valid? + expect(domain_allow_with_normalized_value).to model_have_error_on_field(:domain) end end end From f0b93ab02fdae64fcf3e89508ad3e5919a861264 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 11:11:50 -0500 Subject: [PATCH 14/15] Use AR `database_version` in PG version checks in migrations (#28804) --- db/migrate/20180812173710_copy_status_stats.rb | 3 +-- db/migrate/20181116173541_copy_account_stats.rb | 3 +-- ...0230803082451_add_unique_index_on_preview_cards_statuses.rb | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/db/migrate/20180812173710_copy_status_stats.rb b/db/migrate/20180812173710_copy_status_stats.rb index 52ab43b762..087b1290db 100644 --- a/db/migrate/20180812173710_copy_status_stats.rb +++ b/db/migrate/20180812173710_copy_status_stats.rb @@ -20,8 +20,7 @@ class CopyStatusStats < ActiveRecord::Migration[5.2] private def supports_upsert? - version = select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i - version >= 90_500 + ActiveRecord::Base.connection.database_version >= 90_500 end def up_fast diff --git a/db/migrate/20181116173541_copy_account_stats.rb b/db/migrate/20181116173541_copy_account_stats.rb index 9070200fee..e5faee0cb5 100644 --- a/db/migrate/20181116173541_copy_account_stats.rb +++ b/db/migrate/20181116173541_copy_account_stats.rb @@ -24,8 +24,7 @@ class CopyAccountStats < ActiveRecord::Migration[5.2] private def supports_upsert? - version = select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i - version >= 90_500 + ActiveRecord::Base.connection.database_version >= 90_500 end def up_fast diff --git a/db/post_migrate/20230803082451_add_unique_index_on_preview_cards_statuses.rb b/db/post_migrate/20230803082451_add_unique_index_on_preview_cards_statuses.rb index d29d7847c5..4271f8c08a 100644 --- a/db/post_migrate/20230803082451_add_unique_index_on_preview_cards_statuses.rb +++ b/db/post_migrate/20230803082451_add_unique_index_on_preview_cards_statuses.rb @@ -17,8 +17,7 @@ class AddUniqueIndexOnPreviewCardsStatuses < ActiveRecord::Migration[6.1] def supports_concurrent_reindex? @supports_concurrent_reindex ||= begin - version = select_one("SELECT current_setting('server_version_num') AS v")['v'].to_i - version >= 120_000 + ActiveRecord::Base.connection.database_version >= 120_000 end end From f866413e724c2e7f8329fbc6e96f56f0b186c62a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Thu, 18 Jan 2024 11:14:15 -0500 Subject: [PATCH 15/15] Extract shared tagged statuses method in `FeaturedTag` (#28805) --- app/models/featured_tag.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index 63cd674765..ea8aa4787c 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -45,7 +45,7 @@ class FeaturedTag < ApplicationRecord end def decrement(deleted_status_id) - update(statuses_count: [0, statuses_count - 1].max, last_status_at: account.statuses.where(visibility: %i(public unlisted)).tagged_with(tag).where.not(id: deleted_status_id).select(:created_at).first&.created_at) + update(statuses_count: [0, statuses_count - 1].max, last_status_at: visible_tagged_account_statuses.where.not(id: deleted_status_id).select(:created_at).first&.created_at) end private @@ -55,8 +55,8 @@ class FeaturedTag < ApplicationRecord end def reset_data - self.statuses_count = account.statuses.where(visibility: %i(public unlisted)).tagged_with(tag).count - self.last_status_at = account.statuses.where(visibility: %i(public unlisted)).tagged_with(tag).select(:created_at).first&.created_at + self.statuses_count = visible_tagged_account_statuses.count + self.last_status_at = visible_tagged_account_statuses.select(:created_at).first&.created_at end def validate_featured_tags_limit @@ -72,4 +72,8 @@ class FeaturedTag < ApplicationRecord def tag_already_featured_for_account? FeaturedTag.by_name(name).exists?(account_id: account_id) end + + def visible_tagged_account_statuses + account.statuses.where(visibility: %i(public unlisted)).tagged_with(tag) + end end