Browse Source

Fix Performance/CollectionLiteralInLoop cop (#24819)

Matt Jankowski 1 year ago
parent
commit
2c6c398c60

+ 0 - 15
.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:

+ 3 - 1
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

+ 3 - 1
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

+ 2 - 0
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

+ 3 - 1
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

+ 3 - 1
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

+ 3 - 1
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

+ 3 - 1
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] || {})

+ 2 - 1
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

+ 1 - 1
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?
 

+ 1 - 1
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?
 

+ 5 - 2
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

+ 6 - 4
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