From 2c6c398c6090099005d09600c802358dc1df3f97 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 3 May 2023 23:33:55 -0400 Subject: [PATCH] Fix Performance/CollectionLiteralInLoop cop (#24819) --- .rubocop_todo.yml | 15 --------------- app/models/admin/appeal_filter.rb | 4 +++- app/models/admin/status_filter.rb | 4 +++- app/models/media_attachment.rb | 2 ++ app/models/relationship_filter.rb | 4 +++- app/models/trends/preview_card_filter.rb | 4 +++- app/models/trends/status_filter.rb | 4 +++- app/presenters/status_relationships_presenter.rb | 4 +++- app/services/fetch_resource_service.rb | 3 ++- app/services/suspend_account_service.rb | 2 +- app/services/unsuspend_account_service.rb | 2 +- config/deploy.rb | 7 +++++-- lib/mastodon/media_cli.rb | 10 ++++++---- 13 files changed, 35 insertions(+), 30 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b75d09356e..6a7b7ef83d 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -275,21 +275,6 @@ Naming/VariableNumber: - 'spec/models/user_spec.rb' - 'spec/services/activitypub/fetch_featured_collection_service_spec.rb' -# Configuration parameters: MinSize. -Performance/CollectionLiteralInLoop: - Exclude: - - 'app/models/admin/appeal_filter.rb' - - 'app/models/admin/status_filter.rb' - - 'app/models/relationship_filter.rb' - - 'app/models/trends/preview_card_filter.rb' - - 'app/models/trends/status_filter.rb' - - 'app/presenters/status_relationships_presenter.rb' - - 'app/services/fetch_resource_service.rb' - - 'app/services/suspend_account_service.rb' - - 'app/services/unsuspend_account_service.rb' - - 'config/deploy.rb' - - 'lib/mastodon/media_cli.rb' - # This cop supports unsafe autocorrection (--autocorrect-all). Performance/MapCompact: Exclude: diff --git a/app/models/admin/appeal_filter.rb b/app/models/admin/appeal_filter.rb index f5dcc0f54d..24f8df059c 100644 --- a/app/models/admin/appeal_filter.rb +++ b/app/models/admin/appeal_filter.rb @@ -5,6 +5,8 @@ class Admin::AppealFilter status ).freeze + IGNORED_PARAMS = %w(page).freeze + attr_reader :params def initialize(params) @@ -15,7 +17,7 @@ class Admin::AppealFilter scope = Appeal.order(id: :desc) params.each do |key, value| - next if %w(page).include?(key.to_s) + next if IGNORED_PARAMS.include?(key.to_s) scope.merge!(scope_for(key, value.to_s.strip)) if value.present? end diff --git a/app/models/admin/status_filter.rb b/app/models/admin/status_filter.rb index 4d439e9a1c..645c2e6207 100644 --- a/app/models/admin/status_filter.rb +++ b/app/models/admin/status_filter.rb @@ -6,6 +6,8 @@ class Admin::StatusFilter report_id ).freeze + IGNORED_PARAMS = %w(page report_id).freeze + attr_reader :params def initialize(account, params) @@ -17,7 +19,7 @@ class Admin::StatusFilter scope = @account.statuses.where(visibility: [:public, :unlisted]) params.each do |key, value| - next if %w(page report_id).include?(key.to_s) + next if IGNORED_PARAMS.include?(key.to_s) scope.merge!(scope_for(key, value.to_s.strip)) if value.present? end diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index ee8967a9bb..9a465e5dd5 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -169,6 +169,8 @@ class MediaAttachment < ApplicationRecord original: IMAGE_STYLES[:small].freeze, }.freeze + DEFAULT_STYLES = [:original].freeze + GLOBAL_CONVERT_OPTIONS = { all: '-quality 90 +profile "!icc,*" +set modify-date +set create-date', }.freeze diff --git a/app/models/relationship_filter.rb b/app/models/relationship_filter.rb index 249fe3df8e..3d75dce05e 100644 --- a/app/models/relationship_filter.rb +++ b/app/models/relationship_filter.rb @@ -10,6 +10,8 @@ class RelationshipFilter location ).freeze + IGNORED_PARAMS = %w(relationship page).freeze + attr_reader :params, :account def initialize(account, params) @@ -23,7 +25,7 @@ class RelationshipFilter scope = scope_for('relationship', params['relationship'].to_s.strip) params.each do |key, value| - next if %w(relationship page).include?(key) + next if IGNORED_PARAMS.include?(key) scope.merge!(scope_for(key.to_s, value.to_s.strip)) if value.present? end diff --git a/app/models/trends/preview_card_filter.rb b/app/models/trends/preview_card_filter.rb index f0214c3f0f..ef36ba9878 100644 --- a/app/models/trends/preview_card_filter.rb +++ b/app/models/trends/preview_card_filter.rb @@ -6,6 +6,8 @@ class Trends::PreviewCardFilter locale ).freeze + IGNORED_PARAMS = %w(page).freeze + attr_reader :params def initialize(params) @@ -16,7 +18,7 @@ class Trends::PreviewCardFilter scope = initial_scope params.each do |key, value| - next if %w(page).include?(key.to_s) + next if IGNORED_PARAMS.include?(key.to_s) scope.merge!(scope_for(key, value.to_s.strip)) if value.present? end diff --git a/app/models/trends/status_filter.rb b/app/models/trends/status_filter.rb index de435a0266..da240251fd 100644 --- a/app/models/trends/status_filter.rb +++ b/app/models/trends/status_filter.rb @@ -6,6 +6,8 @@ class Trends::StatusFilter locale ).freeze + IGNORED_PARAMS = %w(page).freeze + attr_reader :params def initialize(params) @@ -16,7 +18,7 @@ class Trends::StatusFilter scope = initial_scope params.each do |key, value| - next if %w(page).include?(key.to_s) + next if IGNORED_PARAMS.include?(key.to_s) scope.merge!(scope_for(key, value.to_s.strip)) if value.present? end diff --git a/app/presenters/status_relationships_presenter.rb b/app/presenters/status_relationships_presenter.rb index be818a2de7..50d1fb31bd 100644 --- a/app/presenters/status_relationships_presenter.rb +++ b/app/presenters/status_relationships_presenter.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class StatusRelationshipsPresenter + PINNABLE_VISIBILITIES = %w(public unlisted private).freeze + attr_reader :reblogs_map, :favourites_map, :mutes_map, :pins_map, :bookmarks_map, :filters_map @@ -16,7 +18,7 @@ class StatusRelationshipsPresenter statuses = statuses.compact status_ids = statuses.flat_map { |s| [s.id, s.reblog_of_id] }.uniq.compact conversation_ids = statuses.filter_map(&:conversation_id).uniq - pinnable_status_ids = statuses.map(&:proper).filter_map { |s| s.id if s.account_id == current_account_id && %w(public unlisted private).include?(s.visibility) } + pinnable_status_ids = statuses.map(&:proper).filter_map { |s| s.id if s.account_id == current_account_id && PINNABLE_VISIBILITIES.include?(s.visibility) } @filters_map = build_filters_map(statuses, current_account_id).merge(options[:filters_map] || {}) @reblogs_map = Status.reblogs_map(status_ids, current_account_id).merge(options[:reblogs_map] || {}) diff --git a/app/services/fetch_resource_service.rb b/app/services/fetch_resource_service.rb index 4470fca010..a2000e5967 100644 --- a/app/services/fetch_resource_service.rb +++ b/app/services/fetch_resource_service.rb @@ -4,6 +4,7 @@ class FetchResourceService < BaseService include JsonLdHelper ACCEPT_HEADER = 'application/activity+json, application/ld+json; profile="https://www.w3.org/ns/activitystreams", text/html;q=0.1' + ACTIVITY_STREAM_LINK_TYPES = ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].freeze attr_reader :response_code @@ -65,7 +66,7 @@ class FetchResourceService < BaseService def process_html(response) page = Nokogiri::HTML(response.body_with_limit) - json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) } + json_link = page.xpath('//link[@rel="alternate"]').find { |link| ACTIVITY_STREAM_LINK_TYPES.include?(link['type']) } process(json_link['href'], terminal: true) unless json_link.nil? end diff --git a/app/services/suspend_account_service.rb b/app/services/suspend_account_service.rb index cfb3eb5831..e9e23a219b 100644 --- a/app/services/suspend_account_service.rb +++ b/app/services/suspend_account_service.rb @@ -68,7 +68,7 @@ class SuspendAccountService < BaseService @account.media_attachments.find_each do |media_attachment| attachment_names.each do |attachment_name| attachment = media_attachment.public_send(attachment_name) - styles = [:original] | attachment.styles.keys + styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys next if attachment.blank? diff --git a/app/services/unsuspend_account_service.rb b/app/services/unsuspend_account_service.rb index d851a0f708..51665eab94 100644 --- a/app/services/unsuspend_account_service.rb +++ b/app/services/unsuspend_account_service.rb @@ -64,7 +64,7 @@ class UnsuspendAccountService < BaseService @account.media_attachments.find_each do |media_attachment| attachment_names.each do |attachment_name| attachment = media_attachment.public_send(attachment_name) - styles = [:original] | attachment.styles.keys + styles = MediaAttachment::DEFAULT_STYLES | attachment.styles.keys next if attachment.blank? diff --git a/config/deploy.rb b/config/deploy.rb index 0907203149..b19567a729 100644 --- a/config/deploy.rb +++ b/config/deploy.rb @@ -13,9 +13,12 @@ set :migration_role, :app append :linked_files, '.env.production', 'public/robots.txt' append :linked_dirs, 'vendor/bundle', 'node_modules', 'public/system' +SYSTEMD_SERVICES = %i[sidekiq streaming web].freeze +SERVICE_ACTIONS = %i[reload restart status].freeze + namespace :systemd do - %i[sidekiq streaming web].each do |service| - %i[reload restart status].each do |action| + SYSTEMD_SERVICES.each do |service| + SERVICE_ACTIONS.each do |action| desc "Perform a #{action} on #{service} service" task "#{service}:#{action}".to_sym do on roles(:app) do diff --git a/lib/mastodon/media_cli.rb b/lib/mastodon/media_cli.rb index b2dfe58d53..7dacd8d3d4 100644 --- a/lib/mastodon/media_cli.rb +++ b/lib/mastodon/media_cli.rb @@ -9,6 +9,8 @@ module Mastodon include ActionView::Helpers::NumberHelper include CLIHelper + VALID_PATH_SEGMENTS_SIZE = [7, 10].freeze + def self.exit_on_failure? true end @@ -133,7 +135,7 @@ module Mastodon path_segments = object.key.split('/') path_segments.delete('cache') - unless [7, 10].include?(path_segments.size) + unless VALID_PATH_SEGMENTS_SIZE.include?(path_segments.size) progress.log(pastel.yellow("Unrecognized file found: #{object.key}")) next end @@ -177,7 +179,7 @@ module Mastodon path_segments = key.split(File::SEPARATOR) path_segments.delete('cache') - unless [7, 10].include?(path_segments.size) + unless VALID_PATH_SEGMENTS_SIZE.include?(path_segments.size) progress.log(pastel.yellow("Unrecognized file found: #{key}")) next end @@ -310,7 +312,7 @@ module Mastodon path_segments = path.split('/')[2..] path_segments.delete('cache') - unless [7, 10].include?(path_segments.size) + unless VALID_PATH_SEGMENTS_SIZE.include?(path_segments.size) say('Not a media URL', :red) exit(1) end @@ -363,7 +365,7 @@ module Mastodon segments = object.key.split('/') segments.delete('cache') - next unless [7, 10].include?(segments.size) + next unless VALID_PATH_SEGMENTS_SIZE.include?(segments.size) model_name = segments.first.classify record_id = segments[2..-2].join.to_i