Extract submatchers from AssociationMatcher

Refactored AssociationMatcher so that `#order`, `#through`, and `#dependent`
would be their own submatchers. This reduces some of the clutter in the main class,
especially as we continue expanding it. In addition, a few related tests were
modified so that they would check failure messages also.

NEWS.md

@@ -1,5 +1,8 @@
 # HEAD
 
+* Extracted `#order`, `#through`, and `#dependent` from AssociationMatcher as
+their own submatchers.
+
 # v 2.2.0
 
 * Fix `have_and_belong_to_many` matcher issue for Rails 4.

lib/shoulda/matchers/active_record.rb

@@ -1,4 +1,8 @@
 require 'shoulda/matchers/active_record/association_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'
+require 'shoulda/matchers/active_record/association_matchers/model_reflector'
 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'

lib/shoulda/matchers/active_record/association_matcher.rb

@@ -76,23 +76,46 @@ module Shoulda # :nodoc:
           @macro = macro
           @name = name
           @options = {}
+          @submatchers = []
+          @missing = ''
         end
 
         def through(through)
-          @options[:through] = through
+          through_matcher = AssociationMatchers::ThroughMatcher.new(through, @name)
+          add_submatcher(through_matcher)
           self
         end
 
         def dependent(dependent)
-          @options[:dependent] = dependent
+          dependent_matcher = AssociationMatchers::DependentMatcher.new(dependent, @name)
+          add_submatcher(dependent_matcher)
           self
         end
 
         def order(order)
-          @options[:order] = order
+          order_matcher = AssociationMatchers::OrderMatcher.new(order, @name)
+          add_submatcher(order_matcher)
           self
         end
 
+        def add_submatcher(matcher)
+          @submatchers << matcher
+        end
+
+        def submatchers_match?
+          failing_submatchers.empty?
+        end
+
+        def submatcher_failure_messages
+          failing_submatchers.map(&:failure_message_for_should)
+        end
+
+        def failing_submatchers
+          @failing_submatchers ||= @submatchers.select do |matcher|
+            !matcher.matches?(@subject)
+          end
+        end
+
         def conditions(conditions)
           @options[:conditions] = conditions
           self
@@ -123,18 +146,20 @@ module Shoulda # :nodoc:
           association_exists? &&
             macro_correct? &&
             foreign_key_exists? &&
-            through_association_valid? &&
-            dependent_correct? &&
             class_name_correct? &&
-            order_correct? &&
             conditions_correct? &&
             join_table_exists? &&
             validate_correct? &&
-            touch_correct?
+            touch_correct? &&
+            submatchers_match?
         end
 
         def failure_message_for_should
-          "Expected #{expectation} (#{@missing})"
+          "Expected #{expectation} (#{missing})"
+        end
+
+        def missing
+          [[@missing] + failing_submatchers.map(&:missing_option)].compact.join
         end
 
         def failure_message_for_should_not
@@ -143,11 +168,8 @@ module Shoulda # :nodoc:
 
         def description
           description = "#{macro_description} #{@name}"
-          description += " through #{@options[:through]}"          if @options.key?(:through)
-          description += " dependent => #{@options[:dependent]}"   if @options.key?(:dependent)
           description += " class_name => #{@options[:class_name]}" if @options.key?(:class_name)
-          description += " order => #{@options[:order]}"           if @options.key?(:order)
-          description
+          [description, @submatchers.map(&:description)].flatten.join(' ')
         end
 
         protected
@@ -184,38 +206,6 @@ module Shoulda # :nodoc:
             !class_has_foreign_key?(associated_class)
         end
 
-        def through_association_valid?
-          @options[:through].nil? || (through_association_exists? && through_association_correct?)
-        end
-
-        def through_association_exists?
-          if through_reflection.nil?
-            @missing = "#{model_class.name} does not have any relationship to #{@options[:through]}"
-            false
-          else
-            true
-          end
-        end
-
-        def through_association_correct?
-          if @options[:through] == reflection.options[:through]
-            true
-          else
-            @missing = "Expected #{model_class.name} to have #{@name} through #{@options[:through]}, " +
-              "but got it through #{reflection.options[:through]}"
-            false
-          end
-        end
-
-        def dependent_correct?
-          if @options[:dependent].nil? || @options[:dependent].to_s == reflection.options[:dependent].to_s
-            true
-          else
-            @missing = "#{@name} should have #{@options[:dependent]} dependency"
-            false
-          end
-        end
-
         def class_name_correct?
           if @options.key?(:class_name)
             if @options[:class_name].to_s == reflection.klass.to_s
@@ -229,19 +219,6 @@ module Shoulda # :nodoc:
           end
         end
 
-        def order_correct?
-          if @options.key?(:order)
-            if @options[:order].to_s == reflection.options[:order].to_s
-              true
-            else
-              @missing = "#{@name} should be ordered by #{@options[:order]}"
-              false
-            end
-          else
-            true
-          end
-        end
-
         def conditions_correct?
           if @options.key?(:conditions)
             if @options[:conditions].to_s == reflection.options[:conditions].to_s
@@ -346,10 +323,6 @@ module Shoulda # :nodoc:
           end
         end
 
-        def through_reflection
-          @through_reflection ||= model_class.reflect_on_association(@options[:through])
-        end
-
         def expectation
           "#{model_class.name} to have a #{@macro} association called #{@name}"
         end

lib/shoulda/matchers/active_record/association_matchers/dependent_matcher.rb

@@ -0,0 +1,35 @@
+module Shoulda # :nodoc:
+  module Matchers
+    module ActiveRecord # :nodoc:
+      module AssociationMatchers
+        class DependentMatcher
+          attr_accessor :missing_option
+
+          def initialize(dependent, name)
+            @dependent = dependent
+            @name = name
+            @missing_option = ''
+          end
+
+          def description
+            "dependent => #{dependent}"
+          end
+
+          def matches?(subject)
+            subject = ModelReflector.new(subject, name)
+
+            if dependent.nil? || subject.option_set_properly?(dependent, :dependent)
+              true
+            else
+              self.missing_option = "#{name} should have #{dependent} dependency"
+              false
+            end
+          end
+
+          private
+          attr_accessor :dependent, :name
+        end
+      end
+    end
+  end
+end

lib/shoulda/matchers/active_record/association_matchers/model_reflector.rb

@@ -0,0 +1,37 @@
+module Shoulda # :nodoc:
+  module Matchers
+    module ActiveRecord # :nodoc:
+      module AssociationMatchers
+        class ModelReflector
+          def initialize(subject, name)
+            @subject = subject
+            @name = name
+          end
+
+          def reflection
+            @reflection ||= reflect_on_association(name)
+          end
+
+          def reflect_on_association(name)
+            model_class.reflect_on_association(name)
+          end
+
+          def model_class
+            subject.class
+          end
+
+          def option_string(key)
+            reflection.options[key].to_s
+          end
+
+          def option_set_properly?(option, option_key)
+            option.to_s == option_string(option_key)
+          end
+
+          private
+          attr_reader :subject, :name
+        end
+      end
+    end
+  end
+end

lib/shoulda/matchers/active_record/association_matchers/order_matcher.rb

@@ -0,0 +1,35 @@
+module Shoulda # :nodoc:
+  module Matchers
+    module ActiveRecord # :nodoc:
+      module AssociationMatchers
+        class OrderMatcher
+          attr_accessor :missing_option
+
+          def initialize(order, name)
+            @order = order
+            @name = name
+            @missing_option = ''
+          end
+
+          def description
+            "order => #{order}"
+          end
+
+          def matches?(subject)
+            subject = ModelReflector.new(subject, name)
+
+            if subject.option_set_properly?(order, :order)
+              true
+            else
+              self.missing_option = "#{name} should be ordered by #{order}"
+              false
+            end
+          end
+
+          private
+          attr_accessor :order, :name
+        end
+      end
+    end
+  end
+end

lib/shoulda/matchers/active_record/association_matchers/through_matcher.rb

@@ -0,0 +1,57 @@
+module Shoulda # :nodoc:
+  module Matchers
+    module ActiveRecord # :nodoc:
+      module AssociationMatchers
+        class ThroughMatcher
+          attr_accessor :missing_option
+
+          def initialize(through, name)
+            @through = through
+            @name = name
+            @missing_option = ''
+          end
+
+          def description
+            "through #{through}"
+          end
+
+          def matches?(subject)
+            self.subject = ModelReflector.new(subject, name)
+            through.nil? || association_set_properly?
+          end
+
+          def association_set_properly?
+            through_association_exists? && through_association_correct?
+          end
+
+          def through_association_exists?
+            if through_reflection.present?
+              true
+            else
+              self.missing_option = "#{name} does not have any relationship to #{through}"
+              false
+            end
+          end
+
+          def through_reflection
+            @through_reflection ||= subject.reflect_on_association(through)
+          end
+
+          def through_association_correct?
+            if subject.option_set_properly?(through, :through)
+              true
+            else
+              self.missing_option =
+                "Expected #{name} to have #{name} through #{through}, " +
+                "but got it through #{subject.option_string(:through)}"
+              false
+            end
+          end
+
+          private
+          attr_accessor :through, :name, :subject
+        end
+      end
+    end
+  end
+end

spec/shoulda/matchers/active_record/association_matcher_spec.rb

@@ -256,7 +256,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do
     end
 
     it 'rejects an association with a bad :dependent option' do
-      having_many_children.should_not have_many(:children).dependent(:destroy)
+      matcher = have_many(:children).dependent(:destroy)
+
+      having_many_children.should_not matcher
+
+      matcher.failure_message_for_should.should =~ /children should have destroy dependency/
     end
 
     it 'accepts an association with a valid :order option' do
@@ -265,7 +269,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do
     end
 
     it 'rejects an association with a bad :order option' do
-      having_many_children.should_not have_many(:children).order(:id)
+      matcher = have_many(:children).order(:id)
+
+      having_many_children.should_not matcher
+
+      matcher.failure_message_for_should.should =~ /children should be ordered by id/
     end
 
     it 'accepts an association with a valid :conditions option' do
@@ -401,7 +409,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do
     end
 
     it 'rejects an association with a bad :dependent option' do
-      having_one_detail.should_not have_one(:detail).dependent(:destroy)
+      matcher = have_one(:detail).dependent(:destroy)
+
+      having_one_detail.should_not matcher
+
+      matcher.failure_message_for_should.should =~ /detail should have destroy dependency/
     end
 
     it 'accepts an association with a valid :order option' do
@@ -409,7 +421,11 @@ describe Shoulda::Matchers::ActiveRecord::AssociationMatcher do
     end
 
     it 'rejects an association with a bad :order option' do
-      having_one_detail.should_not have_one(:detail).order(:id)
+      matcher = have_one(:detail).order(:id)
+
+      having_one_detail.should_not matcher
+
+      matcher.failure_message_for_should.should =~ /detail should be ordered by id/
     end
 
     it 'accepts an association with a valid :conditions option' do