Check foreign keys for has many associations

Author: dsalahutdinov

lib/dbcop.rb

@@ -12,6 +12,7 @@
 require 'dbcop/cop/belongs_to/foreign_key'
 require 'dbcop/cop/validation/presence'
 require 'dbcop/cop/validation/inclusion'
+require 'dbcop/cop/has_many/foreign_key'
 
 # Base module for the gem
 module Dbcop

lib/dbcop/cop/accordance/primary_key.rb

@@ -15,6 +15,8 @@ def call
       private
 
       def valid?
+        return true unless connection.table_exists?(model.table_name)
+
         @valid ||= !connection.primary_keys(model.table_name).map! do |pk|
           column_valid?(pk)
         end.include?(false)

lib/dbcop/cop/has_many/foreign_key.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+require_relative '../cop'
+
+module Dbcop
+  module HasMany
+    # Checks foreign key presence to the parent table of belongs_to association
+    class ForeignKey < Cop
+      def call
+        results = associations.map do |association|
+          valid?(association)
+        end
+
+        results.none?(&:!)
+      end
+
+      private
+
+      def valid?(association)
+        success = foreign_key?(association)
+        unless success
+          from_table = association.class_name.constantize.table_name
+          log("has_many #{association.name} but has no foreign key from #{from_table}.id")
+        end
+        progress(success, 'F')
+
+        success
+      rescue NameError
+        log("Error processing #{model.name}.#{association.name}")
+      end
+
+      def associations
+        model
+          ._reflections
+          .values
+          .select { |association| association.is_a?(ActiveRecord::Reflection::HasManyReflection) }
+          .reject(&:polymorphic?)
+      end
+
+      def foreign_key?(association)
+        to_table = model.table_name
+        connection.foreign_keys(association.class_name.constantize.table_name).any? do |foreign_key|
+          foreign_key.to_table == to_table && foreign_key.options.fetch(:primary_key) == 'id'
+        end
+      end
+    end
+  end
+end

lib/dbcop/cop/validation/inclusion.rb

@@ -35,6 +35,8 @@ def validators
       end
 
       def find_column(attribute)
+        return nil unless connection.table_exists?(model.table_name)
+
         connection
           .columns(model.table_name)
           .find { |col| col.name == attribute.to_s }

spec/internal/app/models/has_many.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module HasMany
+  def self.table_name_prefix
+    'has_many_'
+  end
+end

spec/internal/app/models/has_many/address.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module HasMany
+  class Address < ApplicationRecord
+    has_many :phones, class_name: 'HasMany::Phone'
+  end
+end

spec/internal/app/models/has_many/phone.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+module HasMany
+  class Phone < ApplicationRecord
+  end
+end

spec/internal/app/models/has_many/user.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module HasMany
+  class User < ApplicationRecord
+    has_many :addresses, class_name: 'HasMany::Address'
+  end
+end

spec/internal/db/migrate/20180105000000_create_has_many_tables.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+class CreateHasManyTables < ActiveRecord::Migration[5.2]
+  def change
+    create_table :has_many_users, &:timestamps
+
+    create_table :has_many_addresses do |t|
+      t.references :user, foreign_key: { to_table: :has_many_users }
+      t.text :data
+
+      t.timestamps
+    end
+
+    create_table :has_many_phone do |t|
+      t.references :address, foreign_key: false
+    end
+  end
+end

spec/lib/dbcop/has_many/foreign_key_spec.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Dbcop::HasMany::ForeignKey do
+  describe '#call' do
+    context 'when there is fereign key to the parent table' do
+      let(:result) do
+        described_class.new(HasMany::User).call
+      end
+
+      it { expect(result).to be_truthy }
+    end
+
+    context 'when there is no foreign key to the parent table' do
+      let(:result) { described_class.new(HasMany::Address).call }
+
+      it { expect(result).to be_falsey }
+    end
+  end
+end