Merge branch 'blacklist-destroy-all' into 'master'

Blacklist the use of destroy_all

See merge request gitlab-org/gitlab-ce!21250

app/controllers/projects/pages_controller.rb

@@ -11,7 +11,7 @@ def show
 
   def destroy
     project.remove_pages
-    project.pages_domains.destroy_all
+    project.pages_domains.destroy_all # rubocop: disable DestroyAll
 
     respond_to do |format|
       format.html  do

app/models/concerns/awardable.rb

@@ -101,7 +101,7 @@ def create_award_emoji(name, current_user)
   end
 
   def remove_award_emoji(name, current_user)
-    award_emoji.where(name: name, user: current_user).destroy_all
+    award_emoji.where(name: name, user: current_user).destroy_all # rubocop: disable DestroyAll
   end
 
   def toggle_award_emoji(emoji_name, current_user)

app/models/concerns/fast_destroy_all.rb

@@ -34,7 +34,7 @@ module FastDestroyAll
 
   included do
     before_destroy do
-      raise ForbiddenActionError, '`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`'
+      raise ForbiddenActionError, '`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`'
     end
   end
 

app/models/lfs_object.rb

@@ -29,11 +29,13 @@ def local_store?
     [nil, LfsObjectUploader::Store::LOCAL].include?(self.file_store)
   end
 
+  # rubocop: disable DestroyAll
   def self.destroy_unreferenced
     joins("LEFT JOIN lfs_objects_projects ON lfs_objects_projects.lfs_object_id = #{table_name}.id")
         .where(lfs_objects_projects: { id: nil })
         .destroy_all
   end
+  # rubocop: enable DestroyAll
 
   def self.calculate_oid(path)
     Digest::SHA256.file(path).hexdigest

app/models/user.rb

@@ -516,7 +516,7 @@ def disable_two_factor!
         otp_grace_period_started_at: nil,
         otp_backup_codes:            nil
       )
-      self.u2f_registrations.destroy_all
+      self.u2f_registrations.destroy_all # rubocop: disable DestroyAll
     end
   end
 

app/services/labels/promote_service.rb

@@ -65,7 +65,7 @@ def update_priorities(new_label, label_ids)
     end
 
     def update_project_labels(label_ids)
-      Label.where(id: label_ids).destroy_all
+      Label.where(id: label_ids).destroy_all # rubocop: disable DestroyAll
     end
 
     def clone_label_to_group_label(label)

app/services/milestones/promote_service.rb

@@ -73,7 +73,7 @@ def group
     end
 
     def destroy_old_milestones(milestone)
-      Milestone.where(id: milestone_ids_for_merge(milestone)).destroy_all
+      Milestone.where(id: milestone_ids_for_merge(milestone)).destroy_all # rubocop: disable DestroyAll
     end
 
     def group_project_ids

app/services/projects/move_deploy_keys_projects_service.rb

@@ -27,7 +27,7 @@ def non_existent_deploy_keys_projects
     end
 
     def remove_remaining_deploy_keys_projects
-      source_project.deploy_keys_projects.destroy_all
+      source_project.deploy_keys_projects.destroy_all # rubocop: disable DestroyAll
     end
   end
 end

app/services/projects/move_lfs_objects_projects_service.rb

@@ -21,7 +21,7 @@ def move_lfs_objects_projects
     end
 
     def remove_remaining_lfs_objects_project
-      source_project.lfs_objects_projects.destroy_all
+      source_project.lfs_objects_projects.destroy_all # rubocop: disable DestroyAll
     end
 
     def non_existent_lfs_objects_projects

app/services/projects/move_notification_settings_service.rb

@@ -22,7 +22,7 @@ def move_notification_settings
 
     # Remove remaining notification settings from source_project
     def remove_remaining_notification_settings
-      source_project.notification_settings.destroy_all
+      source_project.notification_settings.destroy_all # rubocop: disable DestroyAll
     end
 
     # Get users of current notification_settings

app/services/projects/move_project_group_links_service.rb

@@ -26,7 +26,7 @@ def move_group_links
 
     # Remove remaining project group links from source_project
     def remove_remaining_project_group_links
-      source_project.reload.project_group_links.destroy_all
+      source_project.reload.project_group_links.destroy_all # rubocop: disable DestroyAll
     end
 
     def group_links_in_target_project

app/services/projects/move_project_members_service.rb

@@ -25,7 +25,7 @@ def move_project_members
 
     def remove_remaining_members
       # Remove remaining members and authorizations from source_project
-      source_project.project_members.destroy_all
+      source_project.project_members.destroy_all # rubocop: disable DestroyAll
     end
 
     def project_members_in_target_project

app/services/protected_branches/legacy_api_update_service.rb

@@ -38,11 +38,11 @@ def execute(protected_branch)
 
     def delete_redundant_access_levels
       unless @developers_can_merge.nil?
-        @protected_branch.merge_access_levels.destroy_all
+        @protected_branch.merge_access_levels.destroy_all # rubocop: disable DestroyAll
       end
 
       unless @developers_can_push.nil?
-        @protected_branch.push_access_levels.destroy_all
+        @protected_branch.push_access_levels.destroy_all # rubocop: disable DestroyAll
       end
     end
   end

app/workers/remove_expired_group_links_worker.rb

@@ -5,6 +5,6 @@ class RemoveExpiredGroupLinksWorker
   include CronjobQueue
 
   def perform
-    ProjectGroupLink.expired.destroy_all
+    ProjectGroupLink.expired.destroy_all # rubocop: disable DestroyAll
   end
 end

app/workers/remove_old_web_hook_logs_worker.rb

@@ -6,7 +6,9 @@ class RemoveOldWebHookLogsWorker
 
   WEB_HOOK_LOG_LIFETIME = 2.days
 
+  # rubocop: disable DestroyAll
   def perform
     WebHookLog.destroy_all(['created_at < ?', Time.now - WEB_HOOK_LOG_LIFETIME])
   end
+  # rubocop: enable DestroyAll
 end

db/migrate/20160712171823_remove_award_emojis_with_no_user.rb

@@ -16,6 +16,6 @@ class RemoveAwardEmojisWithNoUser < ActiveRecord::Migration
   # disable_ddl_transaction!
 
   def up
-    AwardEmoji.joins('LEFT JOIN users ON users.id = user_id').where('users.id IS NULL').destroy_all
+    AwardEmoji.joins('LEFT JOIN users ON users.id = user_id').where('users.id IS NULL').destroy_all # rubocop: disable DestroyAll
   end
 end

lib/gitlab/import_export/members_mapper.rb

@@ -45,7 +45,7 @@ def missing_keys_tracking_hash
       end
 
       def ensure_default_member!
-        @project.project_members.destroy_all
+        @project.project_members.destroy_all # rubocop: disable DestroyAll
 
         ProjectMember.create!(user: @user, access_level: ProjectMember::MAINTAINER, source_id: @project.id, importing: true)
       end

rubocop/cop/destroy_all.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Cop that blacklists the use of `destroy_all`.
+    class DestroyAll < RuboCop::Cop::Cop
+      MSG = 'Use `delete_all` instead of `destroy_all`. ' \
+        '`destroy_all` will load the rows into memory, then execute a ' \
+        '`DELETE` for every individual row.'
+
+      def_node_matcher :destroy_all?, <<~PATTERN
+        (send {send ivar lvar const} :destroy_all ...)
+      PATTERN
+
+      def on_send(node)
+        return unless destroy_all?(node)
+
+        add_offense(node, location: :expression)
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -27,3 +27,4 @@
 require_relative 'cop/rspec/env_assignment'
 require_relative 'cop/rspec/factories_in_migration_specs'
 require_relative 'cop/sidekiq_options_queue'
+require_relative 'cop/destroy_all'

spec/controllers/omniauth_callbacks_controller_spec.rb

@@ -95,7 +95,7 @@
           end
 
           it 'allows linking the disabled provider' do
-            user.identities.destroy_all
+            user.identities.destroy_all # rubocop: disable DestroyAll
             sign_in(user)
 
             expect { post provider }.to change { user.reload.identities.count }.by(1)