Add ViewPrimaryKeyChecker to enforce explicit primary_key on view-backed models

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### [Unreleased]
 
+- Add `ViewPrimaryKeyChecker` to enforce setting `primary_key` for models pointing to database views.
 - Fix `UniqueIndexChecker` to skip partial indexes (indexes with a `WHERE` clause).
 - Fix `MissingUniqueIndexChecker` to skip uniqueness validators with a `conditions` option.
 

README.md

@@ -39,6 +39,7 @@ Currently, the tool can:
 - [find models that have missing tables](https://github.com/djezzzl/database_consistency/wiki/missingtablechecker)
 - [find models with UUID primary keys without specified ordering column](https://github.com/djezzzl/database_consistency/wiki/implicitorderingchecker)
 - [find belongs_to associations with foreign keys without dependent/on_delete options](https://github.com/djezzzl/database_consistency/wiki/missingdependentdestroychecker)
+- [find models pointing to views without a primary_key set](https://github.com/djezzzl/database_consistency/wiki/viewprimarykeychecker)
 
 Besides that, the tool provides:
 - [auto-correction](https://github.com/djezzzl/database_consistency/wiki/auto-correction)
@@ -95,6 +96,7 @@ MissingAssociationClassChecker fail Company anything refers to undefined model "
 MissingTableChecker fail LegacyModel should have a table in the database
 ImplicitOrderingChecker fail Secondary::User id implicit_order_column is recommended when using uuid column type for primary 
 MissingDependentDestroyChecker fail Organization company should have a corresponding has_one/has_many association with dependent option (destroy, delete, delete_all, nullify) or a foreign key with on_delete (cascade, nullify)
+ViewPrimaryKeyChecker fail ReportView self model pointing to a view should have primary_key set
 ```
 
 ## Funding

lib/database_consistency.rb

@@ -44,6 +44,8 @@
 require 'database_consistency/writers/simple/missing_table'
 require 'database_consistency/writers/simple/implicit_order_column_missing'
 require 'database_consistency/writers/simple/missing_dependent_destroy'
+require 'database_consistency/writers/simple/view_missing_primary_key'
+require 'database_consistency/writers/simple/view_primary_key_column_missing'
 require 'database_consistency/writers/simple_writer'
 
 require 'database_consistency/writers/autofix/helpers/migration'
@@ -77,6 +79,7 @@
 
 require 'database_consistency/checkers/model_checkers/model_checker'
 require 'database_consistency/checkers/model_checkers/missing_table_checker'
+require 'database_consistency/checkers/model_checkers/view_primary_key_checker'
 
 require 'database_consistency/checkers/association_checkers/association_checker'
 require 'database_consistency/checkers/association_checkers/missing_index_checker'

lib/database_consistency/checkers/model_checkers/view_primary_key_checker.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Checkers
+    # This class checks that a model pointing to a view has a primary_key set and that column exists
+    class ViewPrimaryKeyChecker < ModelChecker
+      private
+
+      def preconditions
+        ActiveRecord::VERSION::MAJOR >= 5 &&
+          !model.abstract_class? &&
+          model.connection.view_exists?(model.table_name)
+      end
+
+      # Table of possible statuses
+      # | primary_key set | column exists | status |
+      # | --------------- | ------------- | ------ |
+      # | no              | -             | fail   |
+      # | yes             | no            | fail   |
+      # | yes             | yes           | ok     |
+      def check
+        if model.primary_key.blank?
+          report_template(:fail, error_slug: :view_missing_primary_key)
+        elsif !primary_key_column_exists?
+          report_template(:fail, error_slug: :view_primary_key_column_missing)
+        else
+          report_template(:ok)
+        end
+      end
+
+      def primary_key_column_exists?
+        Array(model.primary_key).all? { |key| model.column_names.include?(key.to_s) }
+      end
+    end
+  end
+end

lib/database_consistency/writers/simple/view_missing_primary_key.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Writers
+    module Simple
+      class ViewMissingPrimaryKey < Base # :nodoc:
+        private
+
+        def template
+          'model pointing to a view should have primary_key set'
+        end
+
+        def unique_attributes
+          {
+            table_or_model_name: report.table_or_model_name
+          }
+        end
+      end
+    end
+  end
+end

lib/database_consistency/writers/simple/view_primary_key_column_missing.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+module DatabaseConsistency
+  module Writers
+    module Simple
+      class ViewPrimaryKeyColumnMissing < Base # :nodoc:
+        private
+
+        def template
+          'model pointing to a view has a non-existent primary_key column set'
+        end
+
+        def unique_attributes
+          {
+            table_or_model_name: report.table_or_model_name
+          }
+        end
+      end
+    end
+  end
+end

spec/checkers/view_primary_key_checker_spec.rb

@@ -0,0 +1,152 @@
+# frozen_string_literal: true
+
+RSpec.describe DatabaseConsistency::Checkers::ViewPrimaryKeyChecker, :sqlite, :mysql, :postgresql do
+  subject(:checker) { described_class.new(model) }
+
+  let(:model) { view_klass }
+  let(:view_klass) { define_class('EntityView', :entity_views) }
+
+  context 'when table is not a view' do
+    let(:model) { define_class }
+
+    before do
+      define_database do
+        create_table :entities
+      end
+    end
+
+    specify do
+      expect(checker.report).to be_nil
+    end
+  end
+
+  context 'when model is abstract' do
+    let(:model) { define_class { |klass| klass.abstract_class = true } }
+
+    before do
+      define_database do
+        create_table :entities
+      end
+    end
+
+    specify do
+      expect(checker.report).to be_nil
+    end
+  end
+
+  context 'when table is a view' do
+    before do
+      skip('view_exists? not supported before ActiveRecord 5') if ActiveRecord::VERSION::MAJOR < 5
+
+      define_database do
+        create_table :entities do |t|
+          t.string :name
+        end
+      end
+
+      model.connection.execute(<<~SQL)
+        DROP VIEW IF EXISTS #{view_klass.table_name};
+      SQL
+      model.connection.execute(<<~SQL)
+        CREATE VIEW #{view_klass.table_name} AS SELECT * FROM entities;
+      SQL
+    end
+
+    context 'without primary_key set' do
+      specify do
+        expect(checker.report).to have_attributes(
+          checker_name: 'ViewPrimaryKeyChecker',
+          table_or_model_name: view_klass.name,
+          column_or_attribute_name: 'self',
+          status: :fail,
+          error_slug: :view_missing_primary_key,
+          error_message: nil
+        )
+      end
+    end
+
+    context 'with primary_key set to a non-existent column' do
+      let(:view_klass) do
+        define_class('EntityView', :entity_views) do |klass|
+          klass.primary_key = :nonexistent
+        end
+      end
+
+      specify do
+        expect(checker.report).to have_attributes(
+          checker_name: 'ViewPrimaryKeyChecker',
+          table_or_model_name: view_klass.name,
+          column_or_attribute_name: 'self',
+          status: :fail,
+          error_slug: :view_primary_key_column_missing,
+          error_message: nil
+        )
+      end
+    end
+
+    context 'with primary_key set to an existing column' do
+      let(:view_klass) do
+        define_class('EntityView', :entity_views) do |klass|
+          klass.primary_key = :id
+        end
+      end
+
+      specify do
+        expect(checker.report).to have_attributes(
+          checker_name: 'ViewPrimaryKeyChecker',
+          table_or_model_name: view_klass.name,
+          column_or_attribute_name: 'self',
+          status: :ok,
+          error_slug: nil,
+          error_message: nil
+        )
+      end
+    end
+
+    context 'with composite primary_key set to existing columns' do
+      let(:view_klass) do
+        define_class('EntityView', :entity_views) do |klass|
+          klass.primary_key = %i[id name]
+        end
+      end
+
+      before do
+        skip('Composite primary keys are supported only in Rails 7.1+') unless compound_primary_keys_supported?
+      end
+
+      specify do
+        expect(checker.report).to have_attributes(
+          checker_name: 'ViewPrimaryKeyChecker',
+          table_or_model_name: view_klass.name,
+          column_or_attribute_name: 'self',
+          status: :ok,
+          error_slug: nil,
+          error_message: nil
+        )
+      end
+    end
+
+    context 'with composite primary_key where one column is missing' do
+      let(:view_klass) do
+        define_class('EntityView', :entity_views) do |klass|
+          klass.primary_key = %i[id nonexistent]
+        end
+      end
+
+      before do
+        skip('Composite primary keys are supported only in Rails 7.1+') unless compound_primary_keys_supported?
+      end
+
+      specify do
+        expect(checker.report).to have_attributes(
+          checker_name: 'ViewPrimaryKeyChecker',
+          table_or_model_name: view_klass.name,
+          column_or_attribute_name: 'self',
+          status: :fail,
+          error_slug: :view_primary_key_column_missing,
+          error_message: nil
+        )
+      end
+    end
+  end
+end