From 45770c9306d8dce53ba5b231da6bca73284a3f9a Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 7 Nov 2023 08:01:09 -0500 Subject: [PATCH] Fix `Performance/MapMethodChain` cop (#27744) --- .rubocop_todo.yml | 7 ------- app/models/feed.rb | 4 ++-- lib/mastodon/cli/maintenance.rb | 8 ++++---- spec/services/bulk_import_service_spec.rb | 8 ++++---- spec/services/import_service_spec.rb | 6 +++--- 5 files changed, 13 insertions(+), 20 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index de4dc0b33e..0792194dd4 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -50,13 +50,6 @@ Metrics/CyclomaticComplexity: Metrics/PerceivedComplexity: Max: 27 -Performance/MapMethodChain: - Exclude: - - 'app/models/feed.rb' - - 'lib/mastodon/cli/maintenance.rb' - - 'spec/services/bulk_import_service_spec.rb' - - 'spec/services/import_service_spec.rb' - RSpec/AnyInstance: Exclude: - 'spec/controllers/activitypub/inboxes_controller_spec.rb' diff --git a/app/models/feed.rb b/app/models/feed.rb index f51dcfab1d..d9cab2cd1e 100644 --- a/app/models/feed.rb +++ b/app/models/feed.rb @@ -23,9 +23,9 @@ class Feed max_id = '+inf' if max_id.blank? if min_id.blank? since_id = '-inf' if since_id.blank? - unhydrated = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) + unhydrated = redis.zrevrangebyscore(key, "(#{max_id}", "(#{since_id}", limit: [0, limit], with_scores: true).map { |id| id.first.to_i } else - unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map(&:first).map(&:to_i) + unhydrated = redis.zrangebyscore(key, "(#{min_id}", "(#{max_id}", limit: [0, limit], with_scores: true).map { |id| id.first.to_i } end Status.where(id: unhydrated).cache_ids diff --git a/lib/mastodon/cli/maintenance.rb b/lib/mastodon/cli/maintenance.rb index e73bcbf86a..9cc9a3507a 100644 --- a/lib/mastodon/cli/maintenance.rb +++ b/lib/mastodon/cli/maintenance.rb @@ -224,7 +224,7 @@ module Mastodon::CLI users = User.where(id: row['ids'].split(',')).sort_by(&:updated_at).reverse 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: #{user.map(&:account).map(&:acct).join(', ')}", :yellow + say "e-mail will be disabled for the following accounts: #{user.map { |user| user.account.acct }.join(', ')}", :yellow say 'Please reach out to them and set another address with `tootctl account modify` or delete them.', :yellow users.each_with_index do |user, index| @@ -253,7 +253,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) - say "Unsetting confirmation token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}", :yellow + say "Unsetting confirmation token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| user.update!(confirmation_token: nil) @@ -265,7 +265,7 @@ module Mastodon::CLI if ActiveRecord::Migrator.current_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) - say "Unsetting remember token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}", :yellow + say "Unsetting remember token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| user.update!(remember_token: nil) @@ -277,7 +277,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) - say "Unsetting password reset token for those accounts: #{users.map(&:account).map(&:acct).join(', ')}", :yellow + say "Unsetting password reset token for those accounts: #{users.map { |user| user.account.acct }.join(', ')}", :yellow users.each do |user| user.update!(reset_password_token: nil) diff --git a/spec/services/bulk_import_service_spec.rb b/spec/services/bulk_import_service_spec.rb index 281b642ea4..16d718815a 100644 --- a/spec/services/bulk_import_service_spec.rb +++ b/spec/services/bulk_import_service_spec.rb @@ -54,7 +54,7 @@ RSpec.describe BulkImportService do Import::RowWorker.drain - expect(FollowRequest.includes(:target_account).where(account: account).map(&:target_account).map(&:acct)).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') + expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') end end @@ -102,7 +102,7 @@ RSpec.describe BulkImportService do Import::RowWorker.drain - expect(FollowRequest.includes(:target_account).where(account: account).map(&:target_account).map(&:acct)).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') + expect(FollowRequest.includes(:target_account).where(account: account).map { |follow_request| follow_request.target_account.acct }).to contain_exactly('user@foo.bar', 'unknown@unknown.bar') end end @@ -367,7 +367,7 @@ RSpec.describe BulkImportService do Import::RowWorker.drain - expect(account.bookmarks.map(&:status).map(&:uri)).to contain_exactly(already_bookmarked.uri, status.uri, bookmarked.uri, 'https://domain.unknown/foo') + expect(account.bookmarks.map { |bookmark| bookmark.status.uri }).to contain_exactly(already_bookmarked.uri, status.uri, bookmarked.uri, 'https://domain.unknown/foo') end end @@ -410,7 +410,7 @@ RSpec.describe BulkImportService do Import::RowWorker.drain - expect(account.bookmarks.map(&:status).map(&:uri)).to contain_exactly(status.uri, bookmarked.uri, 'https://domain.unknown/foo') + expect(account.bookmarks.map { |bookmark| bookmark.status.uri }).to contain_exactly(status.uri, bookmarked.uri, 'https://domain.unknown/foo') end end end diff --git a/spec/services/import_service_spec.rb b/spec/services/import_service_spec.rb index e12bbc1a78..3936b2363f 100644 --- a/spec/services/import_service_spec.rb +++ b/spec/services/import_service_spec.rb @@ -232,9 +232,9 @@ RSpec.describe ImportService, type: :service do it 'adds the toots the user has access to to bookmarks' do local_status = Fabricate(:status, account: local_account, uri: 'https://local.com/users/foo/statuses/42', id: 42, local: true) subject.call(import) - expect(account.bookmarks.map(&:status).map(&:id)).to include(local_status.id) - expect(account.bookmarks.map(&:status).map(&:id)).to include(remote_status.id) - expect(account.bookmarks.map(&:status).map(&:id)).to_not include(direct_status.id) + expect(account.bookmarks.map { |bookmark| bookmark.status.id }).to include(local_status.id) + expect(account.bookmarks.map { |bookmark| bookmark.status.id }).to include(remote_status.id) + expect(account.bookmarks.map { |bookmark| bookmark.status.id }).to_not include(direct_status.id) expect(account.bookmarks.count).to eq 3 end end