From bd6f1b55c306fb70fffc5e34055efa1ad3fbd46f Mon Sep 17 00:00:00 2001
From: Steve Abrams <sabrams@gitlab.com>
Date: Tue, 12 Apr 2022 15:39:26 -0600
Subject: [PATCH] Add a new cop Database/DisableReferentialIntegrity
Add a cop to prevent usage of disable_referential_integrity
as this method can lead to data consistency problems and
create unsafe situations where triggers on all tables are
disabled.
---
.../disable_referential_integrity.yml | 4 +++
.../database/disable_referential_integrity.rb | 32 +++++++++++++++++
.../disable_referential_integrity_spec.rb | 36 +++++++++++++++++++
3 files changed, 72 insertions(+)
create mode 100644 .rubocop_todo/database/disable_referential_integrity.yml
create mode 100644 rubocop/cop/database/disable_referential_integrity.rb
create mode 100644 spec/rubocop/cop/database/disable_referential_integrity_spec.rb
diff --git a/.rubocop_todo/database/disable_referential_integrity.yml b/.rubocop_todo/database/disable_referential_integrity.yml
new file mode 100644
index 0000000000000..95cfc5920d461
--- /dev/null
+++ b/.rubocop_todo/database/disable_referential_integrity.yml
@@ -0,0 +1,4 @@
+---
+Database/DisableReferentialIntegrity:
+ Exclude:
+ - 'spec/lib/gitlab/background_migration/cleanup_orphaned_lfs_objects_projects_spec.rb'
diff --git a/rubocop/cop/database/disable_referential_integrity.rb b/rubocop/cop/database/disable_referential_integrity.rb
new file mode 100644
index 0000000000000..80d52678011ef
--- /dev/null
+++ b/rubocop/cop/database/disable_referential_integrity.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module RuboCop
+ module Cop
+ module Database
+ # Cop that checks if 'disable_referential_integrity' method is called.
+ class DisableReferentialIntegrity < RuboCop::Cop::Cop
+ MSG = <<~TEXT
+ Do not use `disable_referential_integrity`, disable triggers in a safe
+ transaction instead. Follow the format:
+ BEGIN;
+ ALTER TABLE my_table DISABLE TRIGGER ALL;
+ -- execute query that requires disabled triggers
+ ALTER TABLE my_table ENABLE TRIGGER ALL;
+ COMMIT;
+ TEXT
+
+ def_node_matcher :disable_referential_integrity?, <<~PATTERN
+ (send _ :disable_referential_integrity)
+ PATTERN
+
+ RESTRICT_ON_SEND = %i[disable_referential_integrity].freeze
+
+ def on_send(node)
+ return unless disable_referential_integrity?(node)
+
+ add_offense(node)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/rubocop/cop/database/disable_referential_integrity_spec.rb b/spec/rubocop/cop/database/disable_referential_integrity_spec.rb
new file mode 100644
index 0000000000000..9ac67363cb685
--- /dev/null
+++ b/spec/rubocop/cop/database/disable_referential_integrity_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require_relative '../../../../rubocop/cop/database/disable_referential_integrity'
+
+RSpec.describe RuboCop::Cop::Database::DisableReferentialIntegrity do
+ subject(:cop) { described_class.new }
+
+ it 'does not flag the use of disable_referential_integrity with a send receiver' do
+ expect_offense(<<~SOURCE)
+ foo.disable_referential_integrity
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `disable_referential_integrity`, [...]
+ SOURCE
+ end
+
+ it 'flags the use of disable_referential_integrity with a full definition' do
+ expect_offense(<<~SOURCE)
+ ActiveRecord::Base.connection.disable_referential_integrity
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `disable_referential_integrity`, [...]
+ SOURCE
+ end
+
+ it 'flags the use of disable_referential_integrity with a nil receiver' do
+ expect_offense(<<~SOURCE)
+ class Foo ; disable_referential_integrity ; end
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `disable_referential_integrity`, [...]
+ SOURCE
+ end
+
+ it 'flags the use of disable_referential_integrity when passing a block' do
+ expect_offense(<<~SOURCE)
+ class Foo ; disable_referential_integrity { :foo } ; end
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not use `disable_referential_integrity`, [...]
+ SOURCE
+ end
+end
--
GitLab