Commit b5b6c9b5 authored by Robert Speicher's avatar Robert Speicher

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

EE: blacklist the use of destroy_all

See merge request gitlab-org/gitlab-ee!6914
parents e4ce8918 e35a8709
......@@ -11,7 +11,7 @@ class Projects::PagesController < Projects::ApplicationController
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
......
......@@ -101,7 +101,7 @@ module Awardable
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)
......
......@@ -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
......
......@@ -30,11 +30,13 @@ class LfsObject < ActiveRecord::Base
[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
......
......@@ -97,8 +97,8 @@ class ProjectMember < Member
def delete_member_branch_protection
if user.present? && project.present?
project.protected_branches.merge_access_by_user(user).destroy_all
project.protected_branches.push_access_by_user(user).destroy_all
project.protected_branches.merge_access_by_user(user).destroy_all # rubocop: disable DestroyAll
project.protected_branches.push_access_by_user(user).destroy_all # rubocop: disable DestroyAll
end
end
......
......@@ -51,8 +51,8 @@ class ProjectGroupLink < ActiveRecord::Base
def delete_branch_protection
if group.present? && project.present?
project.protected_branches.merge_access_by_group(group).destroy_all
project.protected_branches.push_access_by_group(group).destroy_all
project.protected_branches.merge_access_by_group(group).destroy_all # rubocop: disable DestroyAll
project.protected_branches.push_access_by_group(group).destroy_all # rubocop: disable DestroyAll
end
end
......
......@@ -534,7 +534,7 @@ class User < ActiveRecord::Base
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
......
......@@ -65,7 +65,7 @@ module Labels
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)
......
......@@ -75,7 +75,7 @@ module Milestones
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
......
......@@ -27,7 +27,7 @@ module 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
......@@ -21,7 +21,7 @@ module 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
......
......@@ -22,7 +22,7 @@ module Projects
# 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
......
......@@ -26,7 +26,7 @@ module Projects
# 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
......
......@@ -25,7 +25,7 @@ module Projects
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
......
......@@ -38,11 +38,11 @@ module ProtectedBranches
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 # 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 # rubocop: disable DestroyAll
end
end
......@@ -52,18 +52,18 @@ module ProtectedBranches
def delete_redundant_ee_access_levels
case @developers_can_merge
when true
@protected_branch.merge_access_levels.developer.destroy_all
@protected_branch.merge_access_levels.developer.destroy_all # rubocop: disable DestroyAll
when false
@protected_branch.merge_access_levels.developer.destroy_all
@protected_branch.merge_access_levels.maintainer.destroy_all
@protected_branch.merge_access_levels.developer.destroy_all # rubocop: disable DestroyAll
@protected_branch.merge_access_levels.maintainer.destroy_all # rubocop: disable DestroyAll
end
case @developers_can_push
when true
@protected_branch.push_access_levels.developer.destroy_all
@protected_branch.push_access_levels.developer.destroy_all # rubocop: disable DestroyAll
when false
@protected_branch.push_access_levels.developer.destroy_all
@protected_branch.push_access_levels.maintainer.destroy_all
@protected_branch.push_access_levels.developer.destroy_all # rubocop: disable DestroyAll
@protected_branch.push_access_levels.maintainer.destroy_all # rubocop: disable DestroyAll
end
end
end
......
......@@ -5,6 +5,6 @@ class RemoveExpiredGroupLinksWorker
include CronjobQueue
def perform
ProjectGroupLink.expired.destroy_all
ProjectGroupLink.expired.destroy_all # rubocop: disable DestroyAll
end
end
......@@ -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
......@@ -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
......@@ -26,7 +26,7 @@ class Groups::LdapGroupLinksController < Groups::ApplicationController
end
def destroy
@group.ldap_group_links.where(id: params[:id]).destroy_all
@group.ldap_group_links.where(id: params[:id]).destroy_all # rubocop: disable DestroyAll
redirect_back_or_default(default: { action: 'index' }, options: { notice: 'LDAP link removed' })
end
......
......@@ -7,8 +7,8 @@ module CleanupApprovers
private
def cleanup_approvers(target, reload: false)
target.approvers.where.not(user_id: params[:approver_ids]).destroy_all
target.approver_groups.where.not(group_id: params[:approver_group_ids]).destroy_all
target.approvers.where.not(user_id: params[:approver_ids]).destroy_all # rubocop: disable DestroyAll
target.approver_groups.where.not(group_id: params[:approver_group_ids]).destroy_all # rubocop: disable DestroyAll
# If the target already has `approvers` and/or `approver_groups` loaded then we need to
# force a reload in order to not return stale information after the destroys above
......
......@@ -8,7 +8,7 @@ module MergeRequests
currently_approved = merge_request.approved?
if approval.destroy_all
if approval.destroy_all # rubocop: disable DestroyAll
merge_request.reset_approval_cache!
create_note(merge_request)
......
......@@ -48,7 +48,7 @@ describe Projects::Settings::IntegrationsController do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it_behaves_like 'endpoint with some disabled services'
......
......@@ -11,7 +11,7 @@ describe "Admin uploads license" do
set(:license) { build(:license, data: build(:gitlab_license, restrictions: { active_user_count: 2000 }).export) }
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
visit(admin_license_path(trial_key: license.data))
end
......
......@@ -156,7 +156,7 @@ describe Gitlab::Database::LoadBalancing do
context 'without a license' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it 'is disabled' do
......
......@@ -53,7 +53,7 @@ describe Burndown do
context "when all closed issues does not have closed events" do
before do
Event.where(target: milestone.issues, action: Event::CLOSED).destroy_all
Event.where(target: milestone.issues, action: Event::CLOSED).destroy_all # rubocop: disable DestroyAll
end
it "considers closed_at as milestone start date" do
......@@ -73,7 +73,7 @@ describe Burndown do
context "when one or more closed issues does not have a closed event" do
before do
Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all
Event.where(target: milestone.issues.closed.first, action: Event::CLOSED).destroy_all # rubocop: disable DestroyAll
end
it "sets attribute accurate to false" do
......
......@@ -560,7 +560,7 @@ describe License do
let(:license) { create(:license, trial: true, expired: true) }
before(:all) do
described_class.destroy_all
described_class.destroy_all # rubocop: disable DestroyAll
end
::License::EES_FEATURES.each do |feature|
......
......@@ -134,7 +134,7 @@ ICON_STATUS_HTML
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it_behaves_like 'a disabled jenkins deprecated service'
......
......@@ -164,7 +164,7 @@ describe JenkinsService do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it_behaves_like 'project with disabled Jenkins service'
......
......@@ -774,7 +774,7 @@ describe Project do
context 'without a License' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it 'is disabled' do
......@@ -1123,7 +1123,7 @@ describe Project do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it_behaves_like 'project with disabled services'
......
......@@ -7,7 +7,7 @@ RSpec.configure do |config|
config.include EE::LicenseHelpers
config.before(:all) do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
TestLicense.init
end
......
......@@ -30,7 +30,7 @@ describe HistoricalDataWorker do
context 'when there is not a license key' do
it 'does not track historical data' do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
expect(HistoricalData).not_to receive(:track!)
......
......@@ -19,7 +19,7 @@ describe LdapAllGroupsSyncWorker do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it 'does not sync all groups' do
......
......@@ -35,7 +35,7 @@ describe LdapGroupSyncWorker do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it 'does not sync groups' do
......
......@@ -21,7 +21,7 @@ describe LdapSyncWorker do
context 'without a license key' do
before do
License.destroy_all
License.destroy_all # rubocop: disable DestroyAll
end
it 'does not sync LDAP users' do
......
......@@ -45,7 +45,7 @@ module Gitlab
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
......
# 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
......@@ -27,3 +27,4 @@ require_relative 'cop/project_path_helper'
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'
......@@ -95,7 +95,7 @@ describe OmniauthCallbacksController, type: :controller do
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)
......
......@@ -14,7 +14,7 @@ describe Projects::ReleasesController do
describe 'GET #edit' do
it 'initializes a new release' do
tag_id = release.tag
project.releases.destroy_all
project.releases.destroy_all # rubocop: disable DestroyAll
get :edit, namespace_id: project.namespace, project_id: project, tag_id: tag_id
......
......@@ -5,7 +5,7 @@ describe Gitlab::BackgroundMigration::CreateGpgKeySubkeysFromGpgKeys, :migration
let!(:gpg_key) { create(:gpg_key, key: GpgHelpers::User3.public_key) }
before do
GpgKeySubkey.destroy_all
GpgKeySubkey.destroy_all # rubocop: disable DestroyAll
end
it 'generate the subkeys' do
......
......@@ -6,7 +6,7 @@ describe ScheduleCreateGpgKeySubkeysFromGpgKeys, :migration, :sidekiq do
create(:gpg_key, id: 1, key: GpgHelpers::User1.public_key) # rubocop:disable RSpec/FactoriesInMigrationSpecs
create(:gpg_key, id: 2, key: GpgHelpers::User3.public_key) # rubocop:disable RSpec/FactoriesInMigrationSpecs
# Delete all subkeys so they can be recreated
GpgKeySubkey.destroy_all
GpgKeySubkey.destroy_all # rubocop: disable DestroyAll
end
it 'correctly schedules background migrations' do
......
......@@ -11,7 +11,7 @@ describe ForkNetworkMember do
let(:fork_network) { fork_network_member.fork_network }
it 'removes the fork network if it was the last member' do
fork_network.fork_network_members.destroy_all
fork_network.fork_network_members.destroy_all # rubocop: disable DestroyAll
expect(ForkNetwork.count).to eq(0)
end
......
......@@ -73,7 +73,7 @@ describe SystemHook do
it "project_destroy hook" do
project.add_maintainer(user)
project.project_members.destroy_all
project.project_members.destroy_all # rubocop: disable DestroyAll
expect(WebMock).to have_requested(:post, system_hook.url).with(
body: /user_remove_from_team/,
......@@ -110,7 +110,7 @@ describe SystemHook do
it 'group member destroy hook' do
group.add_maintainer(user)
group.group_members.destroy_all
group.group_members.destroy_all # rubocop: disable DestroyAll
expect(WebMock).to have_requested(:post, system_hook.url).with(
body: /user_remove_from_group/,
......
......@@ -2000,7 +2000,7 @@ describe MergeRequest do
context 'with no discussions' do
before do
merge_request.notes.destroy_all
merge_request.notes.destroy_all # rubocop: disable DestroyAll
end
it 'returns true' do
......
......@@ -41,7 +41,7 @@ describe ProjectGroupLink do
project.project_group_links.create(group: group)
group_users.each { |user| expect(user.authorized_projects).to include(project) }
project.project_group_links.destroy_all
project.project_group_links.destroy_all # rubocop: disable DestroyAll
group_users.each { |user| expect(user.authorized_projects).not_to include(project) }
end
end
......
......@@ -206,7 +206,7 @@ describe GroupPolicy do
nested_group.add_guest(developer)
nested_group.add_guest(maintainer)
group.owners.destroy_all
group.owners.destroy_all # rubocop: disable DestroyAll
group.add_guest(owner)
nested_group.add_owner(owner)
......
require 'spec_helper'
require 'rubocop'
require 'rubocop/rspec/support'
require_relative '../../../rubocop/cop/destroy_all'
describe RuboCop::Cop::DestroyAll do
include CopHelper
subject(:cop) { described_class.new }
it 'flags the use of destroy_all with a send receiver' do
inspect_source('foo.destroy_all # rubocop: disable DestroyAll')
expect(cop.offenses.size).to eq(1)
end
it 'flags the use of destroy_all with a constant receiver' do
inspect_source('User.destroy_all # rubocop: disable DestroyAll')
expect(cop.offenses.size).to eq(1)
end
it 'flags the use of destroy_all when passing arguments' do
inspect_source('User.destroy_all([])')
expect(cop.offenses.size).to eq(1)
end
it 'flags the use of destroy_all with a local variable receiver' do
inspect_source(<<~RUBY)
users = User.all
users.destroy_all # rubocop: disable DestroyAll
RUBY
expect(cop.offenses.size).to eq(1)
end
it 'does not flag the use of delete_all' do
inspect_source('foo.delete_all')
expect(cop.offenses).to be_empty
end
end
......@@ -134,9 +134,11 @@ describe MergeRequests::CreateService do
let!(:pipeline_3) { create(:ci_pipeline, project: project, ref: "other_branch", project_id: project.id) }
before do
# rubocop: disable DestroyAll
project.merge_requests
.where(source_branch: opts[:source_branch], target_branch: opts[:target_branch])
.destroy_all
# rubocop: enable DestroyAll
end
it 'sets head pipeline' do
......
......@@ -46,10 +46,12 @@ describe MergeRequests::DeleteNonLatestDiffsService, :clean_gitlab_redis_shared_
end
it 'schedules no removal if there is no non-latest diffs' do
# rubocop: disable DestroyAll
merge_request
.merge_request_diffs
.where.not(id: merge_request.latest_merge_request_diff_id)
.destroy_all
# rubocop: enable DestroyAll
expect(DeleteDiffFilesWorker).not_to receive(:bulk_perform_in)
......
......@@ -448,7 +448,7 @@ describe MergeRequests::UpdateService, :mailer do
update_merge_request(approver_ids: [existing_approver, removed_approver].map(&:id).join(','))
end
Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all
Todo.where(action: Todo::APPROVAL_REQUIRED).destroy_all # rubocop: disable DestroyAll
ActionMailer::Base.deliveries.clear
end
......
......@@ -280,7 +280,7 @@ describe TodoService do
end
it 'does not create a todo if unassigned' do
issue.assignees.destroy_all
issue.assignees.destroy_all # rubocop: disable DestroyAll
should_not_create_any_todo { service.reassigned_issue(issue, author) }
end
......
......@@ -4,8 +4,8 @@ shared_examples_for 'fast destroyable' do
expect(external_data_counter).to be > 0
expect(subjects.count).to be > 0
expect { subjects.first.destroy }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`')
expect { subjects.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`')
expect { subjects.first.destroy }.to raise_error('`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`')
expect { subjects.destroy_all }.to raise_error('`destroy` and `destroy_all` are forbidden. Please use `fast_destroy_all`') # rubocop: disable DestroyAll
expect(subjects.count).to be > 0
expect(external_data_counter).to be > 0
......
......@@ -6,7 +6,7 @@ describe RepositoryCheck::SingleRepositoryWorker do
it 'skips when the project has no push events' do
project = create(:project, :repository, :wiki_disabled)
project.events.destroy_all
project.events.destroy_all # rubocop: disable DestroyAll
break_project(project)
expect(worker).not_to receive(:git_fsck)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment