From d6e93d44aee30f3721d5830f008fdad7d0a38ae2 Mon Sep 17 00:00:00 2001 From: rostikkkk2 Date: Mon, 23 Dec 2019 16:36:37 +0200 Subject: [PATCH 1/3] Bump ARD to 1.7.1 & fix something --- inquisition.gemspec | 2 +- lib/inquisition/active_record_doctor/runner.rb | 2 ++ spec/fixtures/files/active_record_doctor.yml | 4 +++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/inquisition.gemspec b/inquisition.gemspec index a4f867ab..beda1b34 100644 --- a/inquisition.gemspec +++ b/inquisition.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'simplecov', '~> 0.17.0' spec.add_development_dependency 'timecop', '~> 0.9.1' - spec.add_dependency 'active_record_doctor', '~> 1.6.0' + spec.add_dependency 'active_record_doctor', '~> 1.7', '>= 1.7.1' spec.add_dependency 'brakeman', '~> 4.6' spec.add_dependency 'bundler-audit', '~> 0.6' spec.add_dependency 'bundler-leak', '~> 0.1' diff --git a/lib/inquisition/active_record_doctor/runner.rb b/lib/inquisition/active_record_doctor/runner.rb index c5552007..7ae18a90 100644 --- a/lib/inquisition/active_record_doctor/runner.rb +++ b/lib/inquisition/active_record_doctor/runner.rb @@ -24,6 +24,8 @@ class Runner < ::Inquisition::Runner def call TASKS.each do |ard_task| ard_task.run.first.each do |table, column| + next unless table && column + @issues << Inquisition::Issue.new(Issue.new(ard_task, table, column).to_h.merge(runner: self)) end end diff --git a/spec/fixtures/files/active_record_doctor.yml b/spec/fixtures/files/active_record_doctor.yml index 0cbc063b..150ead79 100644 --- a/spec/fixtures/files/active_record_doctor.yml +++ b/spec/fixtures/files/active_record_doctor.yml @@ -3,7 +3,9 @@ - severity: :low message: 'projects has missing foreign keys, details: user_id' - severity: :low - message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type, record_id, blob_id' + message: 'projects has missing non null constraint, details: name, user_id' +- severity: :low + message: 'ActiveStorage::Attachment has missing presence validation, details: name, record_type' - severity: :low message: 'ActiveStorage::Blob has missing presence validation, details: key, filename, byte_size, checksum' - severity: :low From f9a725b06a44f1f2f259edbb45d33e007adc85e1 Mon Sep 17 00:00:00 2001 From: DELL Date: Fri, 10 Apr 2020 23:06:17 +0300 Subject: [PATCH 2/3] Split ARD tasks into separate runners --- .../extraneous_indexes_runner.rb | 17 +++++++++ lib/inquisition/active_record_doctor/issue.rb | 4 +- .../missing_foreign_keys_runner.rb | 17 +++++++++ .../missing_non_null_constraint_runner.rb | 17 +++++++++ .../missing_presence_validation_runner.rb | 17 +++++++++ .../missing_unique_indexes_runner.rb | 17 +++++++++ .../active_record_doctor/runner.rb | 37 +++++++------------ .../undefined_table_references_runner.rb | 17 +++++++++ .../unindexed_deleted_at_runner.rb | 17 +++++++++ .../unindexed_foreign_keys_runner.rb | 17 +++++++++ 10 files changed, 151 insertions(+), 26 deletions(-) create mode 100644 lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb create mode 100644 lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb create mode 100644 lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb create mode 100644 lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb create mode 100644 lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb create mode 100644 lib/inquisition/active_record_doctor/undefined_table_references_runner.rb create mode 100644 lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb create mode 100644 lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb diff --git a/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb b/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb new file mode 100644 index 00000000..57c0c636 --- /dev/null +++ b/lib/inquisition/active_record_doctor/extraneous_indexes_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/extraneous_indexes' + +module Inquisition + module ActiveRecordDoctor + class ExtraneousIndexesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::ExtraneousIndexes + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/issue.rb b/lib/inquisition/active_record_doctor/issue.rb index 23a3272b..194090e1 100644 --- a/lib/inquisition/active_record_doctor/issue.rb +++ b/lib/inquisition/active_record_doctor/issue.rb @@ -11,7 +11,7 @@ def to_h { severity: Severity::LOW, message: create_message, - context: task.to_s.split('::').last + context: task.demodulize } end @@ -20,7 +20,7 @@ def to_h attr_reader :task, :table, :column def create_message - issue_text = task.to_s.split('::').last.split(/(?=[A-Z])/).map(&:downcase).join(' ') + issue_text = task.demodulize.gsub(/([a-z]+)([A-Z])/, '\1 \2').downcase "#{table} has #{issue_text}, details: #{column ? column.join(', ') : 'n/a'}" end end diff --git a/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb b/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb new file mode 100644 index 00000000..5e43f939 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_foreign_keys_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_foreign_keys' + +module Inquisition + module ActiveRecordDoctor + class MissingForeignKeysRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingForeignKeys + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb b/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb new file mode 100644 index 00000000..1565ea39 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_non_null_constraint_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_non_null_constraint' + +module Inquisition + module ActiveRecordDoctor + class MissingNonNullConstraintRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb b/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb new file mode 100644 index 00000000..7a33f694 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_presence_validation_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_presence_validation' + +module Inquisition + module ActiveRecordDoctor + class MissingPresenceValidationRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingPresenceValidation + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb b/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb new file mode 100644 index 00000000..8c077747 --- /dev/null +++ b/lib/inquisition/active_record_doctor/missing_unique_indexes_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/missing_unique_indexes' + +module Inquisition + module ActiveRecordDoctor + class MissingUniqueIndexesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/runner.rb b/lib/inquisition/active_record_doctor/runner.rb index 7ae18a90..1f69813f 100644 --- a/lib/inquisition/active_record_doctor/runner.rb +++ b/lib/inquisition/active_record_doctor/runner.rb @@ -1,33 +1,22 @@ -require 'active_record_doctor/tasks/extraneous_indexes' -require 'active_record_doctor/tasks/missing_foreign_keys' -require 'active_record_doctor/tasks/unindexed_deleted_at' -require 'active_record_doctor/tasks/unindexed_foreign_keys' -require 'active_record_doctor/tasks/undefined_table_references' -require 'active_record_doctor/tasks/missing_unique_indexes' -require 'active_record_doctor/tasks/missing_presence_validation' -require 'active_record_doctor/tasks/missing_non_null_constraint' - module Inquisition module ActiveRecordDoctor class Runner < ::Inquisition::Runner - TASKS = [ - ::ActiveRecordDoctor::Tasks::ExtraneousIndexes, - ::ActiveRecordDoctor::Tasks::MissingForeignKeys, - ::ActiveRecordDoctor::Tasks::MissingNonNullConstraint, - ::ActiveRecordDoctor::Tasks::MissingPresenceValidation, - ::ActiveRecordDoctor::Tasks::MissingUniqueIndexes, - ::ActiveRecordDoctor::Tasks::UndefinedTableReferences, - ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt, - ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys - ].freeze + RUNNERS = %w[ExtraneousIndexesRunner + MissingForeignKeysRunner + MissingNonNullConstraintRunner + MissingPresenceValidationRunner + MissingUniqueIndexesRunner + UndefinedTableReferencesRunner + UnindexedDeletedAtRunner + UnindexedForeignKeysRunner].freeze def call - TASKS.each do |ard_task| - ard_task.run.first.each do |table, column| - next unless table && column + RUNNERS.each do |runner| + runner = "Inquisition::ActiveRecordDoctor::#{runner}".constantize.new + runner.run + issues = runner.issues - @issues << Inquisition::Issue.new(Issue.new(ard_task, table, column).to_h.merge(runner: self)) - end + issues.each { |issue| @issues << Inquisition::Issue.new(issue) } end @issues end diff --git a/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb b/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb new file mode 100644 index 00000000..27d7b45d --- /dev/null +++ b/lib/inquisition/active_record_doctor/undefined_table_references_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/undefined_table_references' + +module Inquisition + module ActiveRecordDoctor + class UndefinedTableReferencesRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UndefinedTableReferences + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb b/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb new file mode 100644 index 00000000..3f72d5b0 --- /dev/null +++ b/lib/inquisition/active_record_doctor/unindexed_deleted_at_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/unindexed_deleted_at' + +module Inquisition + module ActiveRecordDoctor + class UnindexedDeletedAtRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UnindexedDeletedAt + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end diff --git a/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb b/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb new file mode 100644 index 00000000..9ac00362 --- /dev/null +++ b/lib/inquisition/active_record_doctor/unindexed_foreign_keys_runner.rb @@ -0,0 +1,17 @@ +require_relative 'runner' +require 'active_record_doctor/tasks/unindexed_foreign_keys' + +module Inquisition + module ActiveRecordDoctor + class UnindexedForeignKeysRunner < ActiveRecordDoctor::Runner + attr_reader :issues + TASK = ::ActiveRecordDoctor::Tasks::UnindexedForeignKeys + + def run + TASK.run.first.each do |table, column| + @issues << Issue.new(TASK.name, table, column).to_h.merge(runner: TASK) + end + end + end + end +end From c1a87f9ae1adf5d02529bbca4b62e59191e6a90f Mon Sep 17 00:00:00 2001 From: DELL Date: Mon, 13 Apr 2020 11:28:48 +0300 Subject: [PATCH 3/3] Fixed ARD tests after modification --- lib/inquisition.rb | 8 ++++++++ spec/inquisition/active_record_doctor/issue_spec.rb | 2 +- spec/inquisition/active_record_doctor/runner_spec.rb | 3 ++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/inquisition.rb b/lib/inquisition.rb index 5074a097..c14eba55 100644 --- a/lib/inquisition.rb +++ b/lib/inquisition.rb @@ -11,6 +11,14 @@ require 'inquisition/active_record_doctor/runner' require 'inquisition/active_record_doctor/issue' +require_relative 'inquisition/active_record_doctor/unindexed_foreign_keys_runner' +require_relative 'inquisition/active_record_doctor/extraneous_indexes_runner' +require_relative 'inquisition/active_record_doctor/missing_foreign_keys_runner' +require_relative 'inquisition/active_record_doctor/missing_non_null_constraint_runner' +require_relative 'inquisition/active_record_doctor/missing_presence_validation_runner' +require_relative 'inquisition/active_record_doctor/missing_unique_indexes_runner' +require_relative 'inquisition/active_record_doctor/undefined_table_references_runner' +require_relative 'inquisition/active_record_doctor/unindexed_deleted_at_runner' require 'inquisition/brakeman/vulnerability' require 'inquisition/brakeman/runner' require 'inquisition/rubocop' diff --git a/spec/inquisition/active_record_doctor/issue_spec.rb b/spec/inquisition/active_record_doctor/issue_spec.rb index 540099f8..b006f78e 100644 --- a/spec/inquisition/active_record_doctor/issue_spec.rb +++ b/spec/inquisition/active_record_doctor/issue_spec.rb @@ -2,7 +2,7 @@ describe '#to_h' do subject(:vulnerability) { described_class.new(task, 'table', ['column#1', 'column#2']) } - let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys } + let(:task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys.name } let(:message) { 'table has unindexed foreign keys, details: column#1, column#2' } let(:options) do diff --git a/spec/inquisition/active_record_doctor/runner_spec.rb b/spec/inquisition/active_record_doctor/runner_spec.rb index e7d2704e..bdbb0923 100644 --- a/spec/inquisition/active_record_doctor/runner_spec.rb +++ b/spec/inquisition/active_record_doctor/runner_spec.rb @@ -3,6 +3,7 @@ describe '#call' do let(:runner) { described_class.new } + let(:ard_runner) { 'UnindexedForeignKeysRunner' } let(:ard_task) { ActiveRecordDoctor::Tasks::UnindexedForeignKeys } let(:warning) { { 'unindexed_table' => %w[unindexed_column_1 unindexed_column_1] } } let(:message) do @@ -12,7 +13,7 @@ before { allow(ard_task).to receive(:run).and_return([warning, true]) } it 'returns issues array with specific message' do - stub_const('Inquisition::ActiveRecordDoctor::Runner::TASKS', [ard_task]) + stub_const('Inquisition::ActiveRecordDoctor::Runner::RUNNERS', [ard_runner]) expect(runner.call.first.message).to eq(message) end end