From 55f45d9549485b8f7f4b28e55af5ee6fbecc2904 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 16 Aug 2013 15:38:10 -0600 Subject: [PATCH] Refactor AssociationMatcher to use new OptionVerifier When using an association matcher you may have qualifiers on that matcher which let you make assertions on options passed to the association method that you are testing. For instance, has_many has a :dependent option and so in order to test this you say something like it { should have_many(:people).dependent(:destroy) } In order to test such an option we have to compare the option you passed with what the actual value of that option is. This is usually obtained by looking at the reflection object of the association in question, although it can be obtained by other means too. Anyway, the code that does this comparison isn't terrible, but there are two problems with it. First, it involves typecasting both expected and actual values. For instance, this: has_many :people, dependent: :destroy it { should have_many(:people).dependent(:destroy) } should be equivalent to: has_many :people, dependent: :destroy it { should have_many(:people).dependent('destroy') } should be equivalent to: has_many :people, dependent: 'destroy' it { should have_many(:people).dependent(:destroy) } Second, we are a little screwed if the method of obtaining the actual value of the option changes depending on which Rails version you're using. So, OptionVerifier attempts to address both of these issues. It's a little crazy, but it works. I also moved some methods from AssociationMatcher to ModelReflector where they really belong. --- lib/shoulda/matchers/active_record.rb | 1 + .../active_record/association_matcher.rb | 55 +++++---------- .../counter_cache_matcher.rb | 11 ++- .../association_matchers/dependent_matcher.rb | 11 ++- .../association_matchers/model_reflector.rb | 17 +++-- .../association_matchers/option_verifier.rb | 70 +++++++++++++++++++ .../association_matchers/order_matcher.rb | 11 ++- .../association_matchers/through_matcher.rb | 9 ++- 8 files changed, 133 insertions(+), 52 deletions(-) create mode 100644 lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb diff --git a/lib/shoulda/matchers/active_record.rb b/lib/shoulda/matchers/active_record.rb index d60a8157..51fb003f 100644 --- a/lib/shoulda/matchers/active_record.rb +++ b/lib/shoulda/matchers/active_record.rb @@ -4,6 +4,7 @@ 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' require 'shoulda/matchers/active_record/association_matchers/model_reflector' +require 'shoulda/matchers/active_record/association_matchers/option_verifier' require 'shoulda/matchers/active_record/have_db_column_matcher' require 'shoulda/matchers/active_record/have_db_index_matcher' require 'shoulda/matchers/active_record/have_readonly_attribute_matcher' diff --git a/lib/shoulda/matchers/active_record/association_matcher.rb b/lib/shoulda/matchers/active_record/association_matcher.rb index 1e4e106c..9c7d0a79 100644 --- a/lib/shoulda/matchers/active_record/association_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matcher.rb @@ -1,3 +1,5 @@ +require 'forwardable' + module Shoulda # :nodoc: module Matchers module ActiveRecord # :nodoc: @@ -72,6 +74,9 @@ module Shoulda # :nodoc: end class AssociationMatcher # :nodoc: + delegate :reflection, :model_class, :associated_class, :through?, + :join_table, to: :reflector + def initialize(macro, name) @macro = macro @name = name @@ -160,6 +165,14 @@ module Shoulda # :nodoc: attr_reader :submatchers, :missing, :subject, :macro, :name, :options + def reflector + @reflector ||= AssociationMatchers::ModelReflector.new(subject, name) + end + + def option_verifier + @option_verifier ||= AssociationMatchers::OptionVerifier.new(reflector) + end + def add_submatcher(matcher) @submatchers << matcher end @@ -200,10 +213,6 @@ module Shoulda # :nodoc: end end - def reflection - @reflection ||= model_class.reflect_on_association(name) - end - def macro_correct? if reflection.macro == macro true @@ -221,27 +230,15 @@ module Shoulda # :nodoc: macro == :belongs_to && !class_has_foreign_key?(model_class) end - def model_class - subject.class - end - def has_foreign_key_missing? [:has_many, :has_one].include?(macro) && !through? && !class_has_foreign_key?(associated_class) end - def through? - reflection.options[:through] - end - - def associated_class - reflection.klass - end - def class_name_correct? if options.key?(:class_name) - if options[:class_name].to_s == reflection.klass.to_s + if option_verifier.correct_for_string?(:class_name, options[:class_name]) true else @missing = "#{name} should resolve to #{options[:class_name]} for class_name" @@ -254,7 +251,7 @@ module Shoulda # :nodoc: def conditions_correct? if options.key?(:conditions) - if options[:conditions].to_s == reflection.options[:conditions].to_s + if option_verifier.correct_for_string?(:conditions, options[:conditions]) true else @missing = "#{name} should have the following conditions: #{options[:conditions]}" @@ -276,7 +273,7 @@ module Shoulda # :nodoc: end def validate_correct? - if option_correct?(:validate) + if option_verifier.correct_for_boolean?(:validate, options[:validate]) true else @missing = "#{name} should have :validate => #{options[:validate]}" @@ -285,7 +282,7 @@ module Shoulda # :nodoc: end def touch_correct? - if option_correct?(:touch) + if option_verifier.correct_for_boolean?(:touch, options[:touch]) true else @missing = "#{name} should have :touch => #{options[:touch]}" @@ -293,17 +290,9 @@ module Shoulda # :nodoc: end end - def option_correct?(key) - !options.key?(key) || reflection_set_properly_for?(key) - end - - def reflection_set_properly_for?(key) - options[key] == !!reflection.options[key] - end - def class_has_foreign_key?(klass) if options.key?(:foreign_key) - reflection.options[:foreign_key] == options[:foreign_key] + option_verifier.correct_for?(:foreign_key, options[:foreign_key]) else if klass.column_names.include?(foreign_key) true @@ -314,14 +303,6 @@ module Shoulda # :nodoc: end end - def join_table - if reflection.respond_to? :join_table - reflection.join_table.to_s - else - reflection.options[:join_table].to_s - end - end - def foreign_key if foreign_key_reflection if foreign_key_reflection.respond_to?(:foreign_key) diff --git a/lib/shoulda/matchers/active_record/association_matchers/counter_cache_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/counter_cache_matcher.rb index b04049ad..761e97af 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/counter_cache_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/counter_cache_matcher.rb @@ -16,9 +16,9 @@ module Shoulda # :nodoc: end def matches?(subject) - subject = ModelReflector.new(subject, name) + self.subject = ModelReflector.new(subject, name) - if subject.option_set_properly?(counter_cache, :counter_cache) + if option_verifier.correct_for_string?(:counter_cache, counter_cache) true else self.missing_option = "#{name} should have #{description}" @@ -27,7 +27,12 @@ module Shoulda # :nodoc: end private - attr_accessor :counter_cache, :name + + attr_accessor :subject, :counter_cache, :name + + def option_verifier + @option_verifier ||= OptionVerifier.new(subject) + end end end end diff --git a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb index 36146db4..ac5adecd 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb @@ -16,9 +16,9 @@ module Shoulda # :nodoc: end def matches?(subject) - subject = ModelReflector.new(subject, name) + self.subject = ModelReflector.new(subject, name) - if dependent.nil? || subject.option_set_properly?(dependent, :dependent) + if option_verifier.correct_for_string?(:dependent, dependent) true else self.missing_option = "#{name} should have #{dependent} dependency" @@ -27,7 +27,12 @@ module Shoulda # :nodoc: end private - attr_accessor :dependent, :name + + attr_accessor :subject, :dependent, :name + + def option_verifier + @option_verifier ||= OptionVerifier.new(subject) + end end end end diff --git a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb index bec28ccc..a4dd58dc 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb @@ -20,15 +20,24 @@ module Shoulda # :nodoc: subject.class end - def option_string(key) - reflection.options[key].to_s + def associated_class + reflection.klass end - def option_set_properly?(option, option_key) - option.to_s == option_string(option_key) + def through? + reflection.options[:through] + end + + def join_table + if reflection.respond_to? :join_table + reflection.join_table.to_s + else + reflection.options[:join_table].to_s + end end private + attr_reader :subject, :name end end diff --git a/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb b/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb new file mode 100644 index 00000000..24ae78de --- /dev/null +++ b/lib/shoulda/matchers/active_record/association_matchers/option_verifier.rb @@ -0,0 +1,70 @@ +module Shoulda # :nodoc: + module Matchers + module ActiveRecord # :nodoc: + module AssociationMatchers + class OptionVerifier + delegate :reflection, to: :reflector + + attr_reader :reflector + + def initialize(reflector) + @reflector = reflector + end + + def correct_for_string?(name, expected_value) + correct_for?(:string, name, expected_value) + end + + def correct_for_boolean?(name, expected_value) + correct_for?(:boolean, name, expected_value) + end + + def correct_for_hash?(name, expected_value) + correct_for?(:hash, name, expected_value) + end + + def actual_value_for(name) + method_name = "actual_value_for_#{name}" + if respond_to?(method_name, true) + __send__(method_name) + else + reflection.options[name] + end + end + + private + + attr_reader :reflector + + def correct_for?(*args) + expected_value, name, type = args.reverse + if expected_value.nil? + true + else + expected_value = type_cast(type, expected_value_for(name, expected_value)) + actual_value = type_cast(type, actual_value_for(name)) + expected_value == actual_value + end + end + + def type_cast(type, value) + case type + when :string then value.to_s + when :boolean then !!value + when :hash then Hash(value).stringify_keys + else value + end + end + + def expected_value_for(name, value) + value + end + + def actual_value_for_class_name + reflector.associated_class + end + end + end + end + end +end diff --git a/lib/shoulda/matchers/active_record/association_matchers/order_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/order_matcher.rb index f3c0bc81..fde78a49 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/order_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/order_matcher.rb @@ -16,9 +16,9 @@ module Shoulda # :nodoc: end def matches?(subject) - subject = ModelReflector.new(subject, name) + self.subject = ModelReflector.new(subject, name) - if subject.option_set_properly?(order, :order) + if option_verifier.correct_for_string?(:order, order) true else self.missing_option = "#{name} should be ordered by #{order}" @@ -27,7 +27,12 @@ module Shoulda # :nodoc: end private - attr_accessor :order, :name + + attr_accessor :subject, :order, :name + + def option_verifier + @option_verifier ||= OptionVerifier.new(subject) + end end end end diff --git a/lib/shoulda/matchers/active_record/association_matchers/through_matcher.rb b/lib/shoulda/matchers/active_record/association_matchers/through_matcher.rb index 59e9a79a..c92dea4f 100644 --- a/lib/shoulda/matchers/active_record/association_matchers/through_matcher.rb +++ b/lib/shoulda/matchers/active_record/association_matchers/through_matcher.rb @@ -38,18 +38,23 @@ module Shoulda # :nodoc: end def through_association_correct? - if subject.option_set_properly?(through, :through) + if option_verifier.correct_for_string?(:through, through) true else self.missing_option = "Expected #{name} to have #{name} through #{through}, " + - "but got it through #{subject.option_string(:through)}" + "but got it through #{option_verifier.actual_value_for(:through)}" false end end private + attr_accessor :through, :name, :subject + + def option_verifier + @option_verifier ||= OptionVerifier.new(subject) + end end end end