Revert the whole refactoring in the association builder classes.

This is to get activerecord-deprecated_finders work again

8 files changed, 64 insertions, 47 deletions

diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index f86d975592..3973570da4 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -15,23 +15,26 @@ module ActiveRecord::Associations::Builder
   class Association #:nodoc:
     class << self
       attr_accessor :extensions
+      # TODO: This class accessor is needed to make activerecord-deprecated_finders work.
+      # We can move it to a constant in 5.0.
+      attr_accessor :valid_options
     end
     self.extensions = []
 
-    # TODO: This class accessor is needed to make activerecord-deprecated_finders work.
-    # We can move it to a constant in 5.0.
-    cattr_accessor :valid_options, instance_accessor: false
     self.valid_options = [:class_name, :class, :foreign_key, :validate]
 
+    attr_reader :name, :scope, :options
+
     def self.build(model, name, scope, options, &block)
-      extension = define_extensions model, name, &block
-      reflection = create_reflection model, name, scope, options, extension
+      builder = create_builder model, name, scope, options, &block
+      reflection = builder.build(model)
       define_accessors model, reflection
       define_callbacks model, reflection
+      builder.define_extensions model
       reflection
     end
 
-    def self.create_reflection(model, name, scope, options, extension = nil)
+    def self.create_builder(model, name, scope, options, &block)
       raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol)
 
       if scope.is_a?(Hash)
@@ -39,44 +42,39 @@ module ActiveRecord::Associations::Builder
         scope   = nil
       end
 
-      validate_options(options)
-
-      scope = build_scope(scope, extension)
-
-      ActiveRecord::Reflection.create(macro, name, scope, options, model)
+      new(model, name, scope, options, &block)
     end
 
-    def self.build_scope(scope, extension)
-      new_scope = scope
+    def initialize(model, name, scope, options)
+      # TODO: Remove this model argument as soon we drop support to activerecord-deprecated_finders.
+      @name    = name
+      @scope   = scope
+      @options = options
 
-      if scope && scope.arity == 0
-        new_scope = proc { instance_exec(&scope) }
-      end
+      validate_options
 
-      if extension
-        new_scope = wrap_scope new_scope, extension
+      if scope && scope.arity == 0
+        @scope = proc { instance_exec(&scope) }
       end
-
-      new_scope
     end
 
-    def self.wrap_scope(scope, extension)
-      scope
+    def build(model)
+      ActiveRecord::Reflection.create(macro, name, scope, options, model)
     end
 
-    def self.macro
+    def macro
       raise NotImplementedError
     end
 
-    def self.build_valid_options(options)
-      self.valid_options + Association.extensions.flat_map(&:valid_options)
+    def valid_options
+      Association.valid_options + Association.extensions.flat_map(&:valid_options)
     end
 
-    def self.validate_options(options)
-      options.assert_valid_keys(build_valid_options(options))
+    def validate_options
+      options.assert_valid_keys(valid_options)
     end
 
-    def self.define_extensions(model, name)
+    def define_extensions(model)
     end
 
     def self.define_callbacks(model, reflection)
@@ -119,6 +117,8 @@ module ActiveRecord::Associations::Builder
       raise NotImplementedError
     end
 
+    private
+
     def self.add_before_destroy_callbacks(model, reflection)
       unless valid_dependent_options.include? reflection.options[:dependent]
         raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}"
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index 7dd8a7deb1..62cc1e3a8d 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class BelongsTo < SingularAssociation #:nodoc:
-    def self.macro
+    def macro
       :belongs_to
     end
 
-    def self.build_valid_options(options)
+    def valid_options
       super + [:foreign_type, :polymorphic, :touch, :counter_cache]
     end
 
@@ -23,6 +23,8 @@ module ActiveRecord::Associations::Builder
       add_counter_cache_methods mixin
     end
 
+    private
+
     def self.add_counter_cache_methods(mixin)
       return if mixin.method_defined? :belongs_to_counter_cache_after_create
 
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index b696914883..bc15a49996 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -7,11 +7,22 @@ module ActiveRecord::Associations::Builder
 
     CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove]
 
-    def self.build_valid_options(options)
+    def valid_options
       super + [:table_name, :before_add,
                :after_add, :before_remove, :after_remove, :extend]
     end
 
+    attr_reader :block_extension
+
+    def initialize(model, name, scope, options)
+      super
+      @mod = nil
+      if block_given?
+        @mod = Module.new(&Proc.new)
+        @scope = wrap_scope @scope, @mod
+      end
+    end
+
     def self.define_callbacks(model, reflection)
       super
       name    = reflection.name
@@ -21,11 +32,10 @@ module ActiveRecord::Associations::Builder
       }
     end
 
-    def self.define_extensions(model, name)
-      if block_given?
+    def define_extensions(model)
+      if @mod
         extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension"
-        extension = Module.new(&Proc.new)
-        model.parent.const_set(extension_module_name, extension)
+        model.parent.const_set(extension_module_name, @mod)
       end
     end
 
@@ -68,7 +78,9 @@ module ActiveRecord::Associations::Builder
       CODE
     end
 
-    def self.wrap_scope(scope, mod)
+    private
+
+    def wrap_scope(scope, mod)
       if scope
         proc { |owner| instance_exec(owner, &scope).extending(mod) }
       else
diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index 1c9c04b044..af596a3a64 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -84,11 +84,11 @@ module ActiveRecord::Associations::Builder
       middle_name = [lhs_model.name.downcase.pluralize,
                      association_name].join('_').gsub(/::/, '_').to_sym
       middle_options = middle_options join_model
-
-      HasMany.create_reflection(lhs_model,
-                                middle_name,
-                                nil,
-                                middle_options)
+      hm_builder = HasMany.create_builder(lhs_model,
+                                          middle_name,
+                                          nil,
+                                          middle_options)
+      hm_builder.build lhs_model
     end
 
     private
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index 4ec808bc77..7909b93622 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class HasMany < CollectionAssociation #:nodoc:
-    def self.macro
+    def macro
       :has_many
     end
 
-    def self.build_valid_options(options)
+    def valid_options
       super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache]
     end
 
diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb
index 216d6abec1..f359efd496 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -1,10 +1,10 @@
 module ActiveRecord::Associations::Builder
   class HasOne < SingularAssociation #:nodoc:
-    def self.macro
+    def macro
       :has_one
     end
 
-    def self.build_valid_options(options)
+    def valid_options
       valid = super + [:order, :as]
       valid += [:through, :source, :source_type] if options[:through]
       valid
@@ -14,6 +14,8 @@ module ActiveRecord::Associations::Builder
       [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception]
     end
 
+    private
+
     def self.add_before_destroy_callbacks(model, reflection)
       super unless reflection.options[:through]
     end
diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb
index 6a1a0073e6..e655c389a6 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -2,7 +2,7 @@
 
 module ActiveRecord::Associations::Builder
   class SingularAssociation < Association #:nodoc:
-    def self.build_valid_options(options)
+    def valid_options
       super + [:remote, :dependent, :primary_key, :inverse_of]
     end
 
diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb
index f8f2832ab1..4c1fdfdd9a 100644
--- a/activerecord/test/cases/associations/extension_test.rb
+++ b/activerecord/test/cases/associations/extension_test.rb
@@ -75,6 +75,7 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase
   private
 
     def extend!(model)
-      ActiveRecord::Associations::Builder::HasMany.define_extensions(model, :association_name) { }
+      builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, nil, {}) { }
+      builder.define_extensions(model)
     end
 end