From 0695c8647b0af6acb90563e96d584b5c41b88d09 Mon Sep 17 00:00:00 2001 From: Jacob Morris Date: Fri, 25 Apr 2014 23:26:45 -0600 Subject: [PATCH] Check correct columns are present in join table The `have_and_belong_to_many` matcher could give a false positive if the join table is present but does not contain the correct columns. Check to see if the columns exist in the join table and provide a meaningful failure message if one or more of the columns are not present. --- lib/shoulda/matchers/active_record.rb | 1 + .../active_record/association_matcher.rb | 28 ++++--- .../join_table_matcher.rb | 81 +++++++++++++++++++ .../active_record/association_matcher_spec.rb | 19 ++++- 4 files changed, 116 insertions(+), 13 deletions(-) create mode 100644 lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb diff --git a/lib/shoulda/matchers/active_record.rb b/lib/shoulda/matchers/active_record.rb index 0c27e879..9645499f 100644 --- a/lib/shoulda/matchers/active_record.rb +++ b/lib/shoulda/matchers/active_record.rb @@ -2,6 +2,7 @@ require 'shoulda/matchers/active_record/association_matcher' require 'shoulda/matchers/active_record/association_matchers' require 'shoulda/matchers/active_record/association_matchers/counter_cache_matcher' require 'shoulda/matchers/active_record/association_matchers/inverse_of_matcher' +require 'shoulda/matchers/active_record/association_matchers/join_table_matcher' require 'shoulda/matchers/active_record/association_matchers/order_matcher' require 'shoulda/matchers/active_record/association_matchers/through_matcher' require 'shoulda/matchers/active_record/association_matchers/dependent_matcher' diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index f3e083b8..0aa57e29 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -847,9 +847,9 @@ module Shoulda (polymorphic? || class_exists?) && foreign_key_exists? && class_name_correct? && + join_table_correct? && autosave_correct? && conditions_correct? && - join_table_exists? && validate_correct? && touch_correct? && submatchers_match? @@ -889,7 +889,8 @@ module Shoulda end def missing_options - [missing, failing_submatchers.map(&:missing_option)].flatten.join + missing_options = [missing, failing_submatchers.map(&:missing_option)] + missing_options.flatten.compact.join(', ') end def failing_submatchers @@ -946,6 +947,19 @@ module Shoulda end end + def join_table_correct? + if macro != :has_and_belongs_to_many || join_table_matcher.matches?(@subject) + true + else + @missing = join_table_matcher.failure_message + false + end + end + + def join_table_matcher + @join_table_matcher ||= AssociationMatchers::JoinTableMatcher.new(self) + end + def class_exists? associated_class true @@ -980,16 +994,6 @@ module Shoulda end end - def join_table_exists? - if macro != :has_and_belongs_to_many || - model_class.connection.tables.include?(join_table) - true - else - @missing = "join table #{join_table} doesn't exist" - false - end - end - def validate_correct? if option_verifier.correct_for_boolean?(:validate, options[:validate]) true diff --git a/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb new file mode 100644 index 00000000..1ac63640 --- /dev/null +++ b/lib/shoulda/matchers/active_record/association_matchers/join_table_matcher.rb @@ -0,0 +1,81 @@ +module Shoulda + module Matchers + module ActiveRecord + module AssociationMatchers + # @private + class JoinTableMatcher + attr_reader :association_matcher, :failure_message + alias :missing_option :failure_message + + delegate :model_class, :join_table, :associated_class, + to: :association_matcher + + delegate :connection, to: :model_class + + def initialize(association_matcher) + @association_matcher = association_matcher + end + + def matches?(subject) + join_table_exists? && + join_table_has_correct_columns? + end + + def join_table_exists? + if connection.tables.include?(join_table) + true + else + @failure_message = missing_table_message + false + end + end + + def join_table_has_correct_columns? + if missing_columns.empty? + true + else + @failure_message = missing_columns_message + false + end + end + + private + + def missing_columns + @missing_columns ||= expected_join_table_columns.select do |key| + !actual_join_table_columns.include?(key) + end + end + + def expected_join_table_columns + [ + "#{model_class.name.underscore}_id", + "#{associated_class.name.underscore}_id" + ] + end + + def actual_join_table_columns + connection.columns(join_table).map(&:name) + end + + def missing_table_message + "join table #{join_table} doesn't exist" + end + + def missing_columns_message + missing = missing_columns.join(', ') + "join table #{join_table} missing #{column_label}: #{missing}" + end + + def column_label + if missing_columns.count > 1 + 'columns' + else + 'column' + end + end + end + end + end + end +end diff --git a/spec/shoulda/matchers/active_record/association_matcher_spec.rb b/spec/shoulda/matchers/active_record/association_matcher_spec.rb index 22320d69..292adca5 100644 --- a/spec/shoulda/matchers/active_record/association_matcher_spec.rb +++ b/spec/shoulda/matchers/active_record/association_matcher_spec.rb @@ -712,7 +712,24 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do has_and_belongs_to_many :relatives end - expect(Person.new).not_to have_and_belong_to_many(:relatives) + expected_failure_message = "join table people_relatives doesn't exist" + + expect do + expect(Person.new).to have_and_belong_to_many(:relatives) + end.to fail_with_message_including(expected_failure_message) + end + + it 'rejects an association with a join table with incorrect columns' do + define_model :relative + define_model :person do + has_and_belongs_to_many :relatives + end + + define_model :people_relative, id: false, some_crazy_id: :integer + + expect do + expect(Person.new).to have_and_belong_to_many(:relatives) + end.to fail_with_message_including('missing columns: person_id, relative_id') end it 'rejects an association of the wrong type' do