Begin refactoring ThroughReflection

This allows us to decouple AssociationReflection and
ThroughReflection making ThroughReflection it's own Reflection
bucket in a way. The benefit of this is to be able to remove
checks against the macro's symbol for exmaple `macro == :belongs_to`.

Get all tests passing again

Some of the methods that used to be inherited from MacroReflection
through AssociationReflection were no longer getting passed through.
They needed to be duplicated into the ThroughReflection. I will
extract these out into a separate class.

Refactor shared methods into strategy object

Now that we've separated ThroughReflection and AssociationReflection
we can combine shared methods into one class to avoid duplication.

Break out class for each type of reflection

This creates a class for each reflection type (has_many, has_one,
belongs_to and habtm). We then can remove the need to set the macro
symbol in each initialization.

Tests were updated to reflect these changes because creation of
these reflections is now different.

Remove need for @collection instance var

We now define `collection?` as `false` by default and set it to
`true` in `has_and_belongs_to_many` and `has_many` reflections.
This removes the need for the `@collection` instance variable.

Raise exception on unknown macro types

We shouldn't accept just any macro when creating reflections. An
unrecongnized AssociationReflection raises an error. Tests in
`reflection_test` were updated to reflect these new changes.
`:has_and_belongs_to_many` macro tests were removed because we no
longer internally return HABTM.

1 files changed, 127 insertions, 66 deletions

diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index 28c39bdd5c..12208aecf7 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -13,14 +13,21 @@ module ActiveRecord
     end
 
     def self.create(macro, name, scope, options, ar)
-      case macro
-      when :has_many, :belongs_to, :has_one
-        klass = options[:through] ? ThroughReflection : AssociationReflection
-      when :composed_of
-        klass = AggregateReflection
-      end
-
-      klass.new(macro, name, scope, options, ar)
+      klass = case macro
+              when :composed_of
+                AggregateReflection
+              when :has_many
+                HasManyReflection
+              when :has_one
+                HasOneReflection
+              when :belongs_to
+                BelongsToReflection
+              else
+                raise "Unsupported Macro: #{macro}"
+              end
+
+      reflection = klass.new(name, scope, options, ar)
+      options[:through] ? ThroughReflection.new(reflection) : reflection
     end
 
     def self.add_reflection(ar, name, reflection)
@@ -110,6 +117,52 @@ module ActiveRecord
       end
     end
 
+    # Holds all the methods that are shared between MacroReflection, AssociationReflection
+    # and ThroughReflection
+    class AbstractReflection # :nodoc:
+      def table_name
+        klass.table_name
+      end
+
+      # Returns a new, unsaved instance of the associated class. +attributes+ will
+      # be passed to the class's constructor.
+      def build_association(attributes, &block)
+        klass.new(attributes, &block)
+      end
+
+      def quoted_table_name
+        klass.quoted_table_name
+      end
+
+      def primary_key_type
+        klass.type_for_attribute(klass.primary_key)
+      end
+
+      # Returns the class name for the macro.
+      #
+      # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>'Money'</tt>
+      # <tt>has_many :clients</tt> returns <tt>'Client'</tt>
+      def class_name
+        @class_name ||= (options[:class_name] || derive_class_name).to_s
+      end
+
+      JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
+
+      def join_keys(assoc_klass)
+        if source_macro == :belongs_to
+          if polymorphic?
+            reflection_key = association_primary_key(assoc_klass)
+          else
+            reflection_key = association_primary_key
+          end
+          reflection_foreign_key = foreign_key
+        else
+          reflection_foreign_key = active_record_primary_key
+          reflection_key = foreign_key
+        end
+        JoinKeys.new(reflection_key, reflection_foreign_key)
+      end
+    end
     # Base class for AggregateReflection and AssociationReflection. Objects of
     # AggregateReflection and AssociationReflection are returned by the Reflection::ClassMethods.
     #
@@ -117,7 +170,7 @@ module ActiveRecord
     #     AggregateReflection
     #     AssociationReflection
     #       ThroughReflection
-    class MacroReflection
+    class MacroReflection < AbstractReflection
       # Returns the name of the macro.
       #
       # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>:balance</tt>
@@ -142,8 +195,7 @@ module ActiveRecord
 
       attr_reader :plural_name # :nodoc:
 
-      def initialize(macro, name, scope, options, active_record)
-        @macro         = macro
+      def initialize(name, scope, options, active_record)
         @name          = name
         @scope         = scope
         @options       = options
@@ -167,15 +219,11 @@ module ActiveRecord
       # <tt>composed_of :balance, class_name: 'Money'</tt> returns the Money class
       # <tt>has_many :clients</tt> returns the Client class
       def klass
-        @klass ||= class_name.constantize
+        @klass ||= compute_class(class_name)
       end
 
-      # Returns the class name for the macro.
-      #
-      # <tt>composed_of :balance, class_name: 'Money'</tt> returns <tt>'Money'</tt>
-      # <tt>has_many :clients</tt> returns <tt>'Client'</tt>
-      def class_name
-        @class_name ||= (options[:class_name] || derive_class_name).to_s
+      def compute_class(name)
+        name.constantize
       end
 
       # Returns +true+ if +self+ and +other_aggregation+ have the same +name+ attribute, +active_record+ attribute,
@@ -188,23 +236,6 @@ module ActiveRecord
           active_record == other_aggregation.active_record
       end
 
-      JoinKeys = Struct.new(:key, :foreign_key) # :nodoc:
-
-      def join_keys(assoc_klass)
-        if source_macro == :belongs_to
-          if polymorphic?
-            reflection_key = association_primary_key(assoc_klass)
-          else
-            reflection_key = association_primary_key
-          end
-          reflection_foreign_key = foreign_key
-        else
-          reflection_key = foreign_key
-          reflection_foreign_key = active_record_primary_key
-        end
-        JoinKeys.new(reflection_key, reflection_foreign_key)
-      end
-
       private
         def derive_class_name
           name.to_s.camelize
@@ -237,15 +268,18 @@ module ActiveRecord
       # a new association object. Use +build_association+ or +create_association+
       # instead. This allows plugins to hook into association object creation.
       def klass
-        @klass ||= active_record.send(:compute_type, class_name)
+        @klass ||= compute_class(class_name)
+      end
+
+      def compute_class(name)
+        active_record.send(:compute_type, name)
       end
 
       attr_reader :type, :foreign_type
       attr_accessor :parent_reflection # [:name, Reflection]
 
-      def initialize(macro, name, scope, options, active_record)
+      def initialize(name, scope, options, active_record)
         super
-        @collection = macro == :has_many
         @automatic_inverse_of = nil
         @type         = options[:as] && "#{options[:as]}_type"
         @foreign_type = options[:foreign_type] || "#{name}_type"
@@ -264,24 +298,10 @@ module ActiveRecord
         }
       end
 
-      # Returns a new, unsaved instance of the associated class. +attributes+ will
-      # be passed to the class's constructor.
-      def build_association(attributes, &block)
-        klass.new(attributes, &block)
-      end
-
       def constructable? # :nodoc:
         @constructable
       end
 
-      def table_name
-        klass.table_name
-      end
-
-      def quoted_table_name
-        klass.quoted_table_name
-      end
-
       def join_table
         @join_table ||= options[:join_table] || derive_join_table
       end
@@ -290,10 +310,6 @@ module ActiveRecord
         @foreign_key ||= options[:foreign_key] || derive_foreign_key
       end
 
-      def primary_key_type
-        klass.type_for_attribute(klass.primary_key)
-      end
-
       def association_foreign_key
         @association_foreign_key ||= options[:association_foreign_key] || class_name.foreign_key
       end
@@ -396,7 +412,7 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
       # association. Returns +true+ if the +macro+ is either +has_many+ or
       # +has_and_belongs_to_many+, +false+ otherwise.
       def collection?
-        @collection
+        false
       end
 
       # Returns whether or not the association should be validated as part of
@@ -560,22 +576,57 @@ Joining, Preloading and eager loading of these associations is deprecated and wi
         end
     end
 
+    class HasManyReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :has_many
+        super(name, scope, options, active_record)
+      end
+
+      def collection?
+        true
+      end
+    end
+
+    class HasOneReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :has_one
+        super(name, scope, options, active_record)
+      end
+    end
+
+    class BelongsToReflection < AssociationReflection #:nodoc:
+      def initialize(name, scope, options, active_record)
+        @macro = :belongs_to
+        super(name, scope, options, active_record)
+      end
+    end
+
     class HasAndBelongsToManyReflection < AssociationReflection #:nodoc:
-      def initialize(macro, name, scope, options, active_record)
+      def initialize(name, scope, options, active_record)
+        @macro = :has_and_belongs_to_many
         super
-        @collection = true
+      end
+
+      def collection?
+        true
       end
     end
 
     # Holds all the meta-data about a :through association as it was specified
     # in the Active Record class.
-    class ThroughReflection < AssociationReflection #:nodoc:
+    class ThroughReflection < AbstractReflection #:nodoc:
+      attr_reader :delegate_reflection
       delegate :foreign_key, :foreign_type, :association_foreign_key,
                :active_record_primary_key, :type, :to => :source_reflection
 
-      def initialize(macro, name, scope, options, active_record)
-        super
-        @source_reflection_name = options[:source]
+      def initialize(delegate_reflection)
+        @delegate_reflection = delegate_reflection
+        @klass         = delegate_reflection.options[:class]
+        @source_reflection_name = delegate_reflection.options[:source]
+      end
+
+      def klass
+        @klass ||= delegate_reflection.compute_class(class_name)
       end
 
       # Returns the source of the through reflection. It checks both a singularized
@@ -777,15 +828,25 @@ directive on your declaration like:
 
       protected
 
-      def actual_source_reflection # FIXME: this is a horrible name
-        source_reflection.actual_source_reflection
-      end
+        def actual_source_reflection # FIXME: this is a horrible name
+          source_reflection.send(:actual_source_reflection)
+        end
+
+        def primary_key(klass)
+          klass.primary_key || raise(UnknownPrimaryKey.new(klass))
+        end
 
       private
         def derive_class_name
           # get the class_name of the belongs_to association of the through reflection
           options[:source_type] || source_reflection.class_name
         end
+
+        delegate_methods = AssociationReflection.public_instance_methods -
+          public_instance_methods
+
+        delegate(*delegate_methods, to: :delegate_reflection)
+
     end
   end
 end