Added support for associating unsaved objects #402 [Tim Bates]

Added replace to associations, so you can do project.manager.replace(new_manager) or project.milestones.replace(new_milestones) #402 [Tim Bates]
Added build and create methods to has_one and belongs_to associations, so you can now do project.manager.build(attributes) #402 [Tim Bates]
Fixed that Base#== wouldn't work for multiple references to the same unsaved object #402 [Tim Bates]
Added that if a before_* callback returns false, all the later callbacks and the associated action are cancelled. If an after_* callback returns false, all the later callbacks are cancelled. Callbacks are generally run in the order they are defined, with the exception of callbacks defined as methods on the model, which are called last. #402 [Tim Bates]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@417 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

6 files changed, 272 insertions, 101 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index ca87fae5ff..334cdccc3d 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -1,51 +1,34 @@
 module ActiveRecord
   module Associations
-    class AssociationCollection #:nodoc:
-      alias_method :proxy_respond_to?, :respond_to?
-      instance_methods.each { |m| undef_method m unless m =~ /(^__|^nil\?|^proxy_respond_to\?)/ }
-
-      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
-        @owner = owner
-        @options = options
-        @association_name = association_name
-        @association_class = eval(association_class_name)
-        @association_class_primary_key_name = association_class_primary_key_name
-      end
-      
-      def method_missing(symbol, *args, &block)
-        load_collection
-        @collection.send(symbol, *args, &block)
-      end
-  
+    class AssociationCollection < AssociationProxy #:nodoc:
       def to_ary
-        load_collection
-        @collection.to_ary
+        load_target
+        @target.to_ary
       end
   
-      def respond_to?(symbol, include_priv = false)
-        proxy_respond_to?(symbol, include_priv) || [].respond_to?(symbol, include_priv)
+      def reset
+        @target = []
+        @loaded = false
       end
 
-      def loaded?
-        !@collection.nil?
-      end
-      
       def reload
-        @collection = nil
+        reset
       end
 
       # Add +records+ to this association.  Returns +self+ so method calls may be chained.  
       # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
       def <<(*records)
+        result = true
+        load_target
         @owner.transaction do
           flatten_deeper(records).each do |record|
             raise_on_type_mismatch(record)
-            insert_record(record)
-            @collection << record if loaded?
+            result &&= insert_record(record) unless @owner.new_record?
+            @target << record
           end
         end
 
-        self
+        result and self
       end
 
       alias_method :push, :<<
@@ -54,11 +37,13 @@ module ActiveRecord
       # Remove +records+ from this association.  Does not destroy +records+.
       def delete(*records)
         records = flatten_deeper(records)
+        records.each { |record| raise_on_type_mismatch(record) }
+        records.reject! { |record| @target.delete(record) if record.new_record? }
+        return if records.empty?
         
         @owner.transaction do
-          records.each { |record| raise_on_type_mismatch(record) }
           delete_records(records)
-          records.each { |record| @collection.delete(record) } if loaded?
+          records.each { |record| @target.delete(record) }
         end
       end
       
@@ -67,20 +52,27 @@ module ActiveRecord
           each { |record| record.destroy }
         end
 
-        @collection = []
+        @target = []
       end
       
+      def create(attributes = {})
+        # Can't use Base.create since the foreign key may be a protected attribute.
+        record = build(attributes)
+        record.save unless @owner.new_record?
+        record
+      end
+
       # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been loaded and
       # calling collection.size if it has. If it's more likely than not that the collection does have a size larger than zero
       # and you need to fetch that collection afterwards, it'll take one less SELECT query if you use length.
       def size
-        if loaded? then @collection.size else count_records end
+        if loaded? then @target.size else count_records end
       end
       
       # Returns the size of the collection by loading it and calling size on the array. If you want to use this method to check
       # whether the collection is empty, use collection.length.zero? instead of collection.empty?
       def length
-        load_collection.size
+        load_target.size
       end
       
       def empty?
@@ -91,11 +83,14 @@ module ActiveRecord
         collection.inject([]) { |uniq_records, record| uniq_records << record unless uniq_records.include?(record); uniq_records }
       end
 
-      protected
-        def loaded?
-          not @collection.nil?
-        end
+      def replace(other_array)
+        other_array.each{ |val| raise_on_type_mismatch(val) }
+
+        @target = other_array
+        @loaded = true
+      end
 
+      protected
         def quoted_record_ids(records)
           records.map { |record| record.quoted_id }.join(',')
         end
@@ -117,22 +112,14 @@ module ActiveRecord
         end
 
       private
-        def load_collection
-          if loaded?
-            @collection
-          else
-            begin
-              @collection = find_all_records
-            rescue ActiveRecord::RecordNotFound
-              @collection = []
-            end
-          end
-        end
-
         def raise_on_type_mismatch(record)
           raise ActiveRecord::AssociationTypeMismatch, "#{@association_class} expected, got #{record.class}" unless record.is_a?(@association_class)
         end
 
+        def target_obsolete?
+          false
+        end
+
         # Array#flatten has problems with rescursive arrays. Going one level deeper solves the majority of the problems.
         def flatten_deeper(array)
           array.collect { |element| element.respond_to?(:flatten) ? element.flatten : element }.flatten
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
new file mode 100644
index 0000000000..dcba207e20
--- /dev/null
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -0,0 +1,49 @@
+module ActiveRecord
+  module Associations
+    class AssociationProxy #:nodoc:
+      alias_method :proxy_respond_to?, :respond_to?
+      instance_methods.each { |m| undef_method m unless m =~ /(^__|^nil\?|^proxy_respond_to\?|^send)/ }
+
+      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
+        @owner = owner
+        @options = options
+        @association_name = association_name
+        @association_class = eval(association_class_name)
+        @association_class_primary_key_name = association_class_primary_key_name
+
+        reset
+      end
+      
+      def method_missing(symbol, *args, &block)
+        load_target
+        @target.send(symbol, *args, &block)
+      end
+
+      def respond_to?(symbol, include_priv = false)
+        load_target
+        proxy_respond_to?(symbol, include_priv) || @target.respond_to?(symbol, include_priv)
+      end
+
+      def loaded?
+        @loaded
+      end
+
+      private
+        def load_target
+          unless @owner.new_record?
+            begin
+              @target = find_target if not loaded?
+            rescue ActiveRecord::RecordNotFound
+              reset
+            end
+          end
+          @loaded = true
+          @target
+        end
+
+        def raise_on_type_mismatch(record)
+          raise ActiveRecord::AssociationTypeMismatch, "#{@association_class} expected, got #{record.class}" unless record.is_a?(@association_class)
+        end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
new file mode 100644
index 0000000000..aa627f7495
--- /dev/null
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -0,0 +1,70 @@
+module ActiveRecord
+  module Associations
+    class BelongsToAssociation < AssociationProxy #:nodoc:
+
+      def reset
+        @target = nil
+        @loaded = false
+      end
+
+      def reload
+        reset
+        load_target
+      end
+
+      def create(attributes = {})
+        record = build(attributes)
+        record.save
+        record
+      end
+
+      def build(attributes = {})
+        record = @association_class.new(attributes)
+        replace(record, true)
+        record
+      end
+
+      def replace(obj, dont_save = false)
+        if obj.nil?
+          @target = @owner[@association_class_primary_key_name] = nil
+        else
+          raise_on_type_mismatch(obj) unless obj.nil?
+
+          @target = obj
+          @owner[@association_class_primary_key_name] = obj.id unless obj.new_record?
+        end
+        @loaded = true
+      end
+
+      # Ugly workaround - .nil? is done in C and the method_missing trick doesn't work when we pretend to be nil
+      def nil?
+        load_target
+        @target.nil?
+      end
+
+      private
+        def find_target
+          if @options[:conditions]
+            @association_class.find_on_conditions(@owner[@association_class_primary_key_name], @options[:conditions])
+          else
+            @association_class.find(@owner[@association_class_primary_key_name])
+          end
+        end
+
+        def target_obsolete?
+          @owner[@association_class_primary_key_name] != @target.id
+        end
+
+        def construct_sql
+          # no sql to construct
+        end
+    end
+  end
+end
+
+class NilClass #:nodoc:
+  # Ugly workaround - nil comparison is usually done in C and so a proxy object pretending to be nil doesn't work.
+  def ==(other)
+    other.nil?
+  end
+end
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 1152846df2..83b87547ee 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -1,26 +1,27 @@
 module ActiveRecord
   module Associations
     class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc:
-      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, join_table, options)
-        super(owner, association_name, association_class_name, association_class_primary_key_name, options)
+      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
+        super
 
         @association_foreign_key = options[:association_foreign_key] || Inflector.underscore(Inflector.demodulize(association_class_name)) + "_id"
-        association_table_name = options[:table_name] || @association_class.table_name
-        @join_table = join_table
+        @association_table_name = options[:table_name] || @association_class.table_name
+        @join_table = options[:join_table]
         @order = options[:order] || "t.#{@association_class.primary_key}"
 
-        interpolate_sql_options!(options, :finder_sql, :delete_sql)
-        @finder_sql = options[:finder_sql] ||
-              "SELECT t.*, j.* FROM #{association_table_name} t, #{@join_table} j " +
-              "WHERE t.#{@association_class.primary_key} = j.#{@association_foreign_key} AND " +
-              "j.#{association_class_primary_key_name} = #{@owner.quoted_id} " +
-              (options[:conditions] ? " AND " + interpolate_sql(options[:conditions]) : "") + " " +
-              "ORDER BY #{@order}"
+        construct_sql
       end
  
+      def build(attributes = {})
+        load_target
+        record = @association_class.new(attributes)
+        @target << record
+        record
+      end
+
       # Removes all records from this association.  Returns +self+ so method calls may be chained.
       def clear
-        return self if size == 0 # forces load_collection if hasn't happened already
+        return self if size == 0 # forces load_target if hasn't happened already
 
         if sql = @options[:delete_sql]
           each { |record| @owner.connection.execute(sql) }
@@ -34,12 +35,12 @@ module ActiveRecord
           @owner.connection.execute(sql)
         end
 
-        @collection = []
+        @target = []
         self
       end
 
       def find_first
-        load_collection.first
+        load_target.first
       end
 
       def find(*args)
@@ -56,16 +57,16 @@ module ActiveRecord
         elsif @options[:finder_sql]
           if ids.size == 1
             id = ids.first
-            record = load_collection.detect { |record| id == record.id }
+            record = load_target.detect { |record| id == record.id }
             expects_array? ? [record] : record
           else
-            load_collection.select { |record| ids.include?(record.id) }
+            load_target.select { |record| ids.include?(record.id) }
           end
 
         # Otherwise, construct a query.
         else
           ids_list = ids.map { |id| @owner.send(:quote, id) }.join(',')
-          records = find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} IN (#{ids_list}) ORDER BY"))
+          records = find_target(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} IN (#{ids_list}) ORDER BY"))
           if records.size == ids.size
             if ids.size == 1 and !expects_array
               records.first
@@ -82,7 +83,7 @@ module ActiveRecord
         raise_on_type_mismatch(record)
         insert_record_with_join_attributes(record, join_attributes)
         join_attributes.each { |key, value| record.send(:write_attribute, key, value) }
-        @collection << record if loaded?
+        @target << record
         self
       end
       
@@ -93,16 +94,17 @@ module ActiveRecord
       end
       
       protected
-        def find_all_records(sql = @finder_sql)
+        def find_target(sql = @finder_sql)
           records = @association_class.find_by_sql(sql)
           @options[:uniq] ? uniq(records) : records
         end
 
         def count_records
-          load_collection.size
+          load_target.size
         end
 
         def insert_record(record)
+          return false unless record.save
           if @options[:insert_sql]
             @owner.connection.execute(interpolate_sql(@options[:insert_sql], record))
           else
@@ -110,6 +112,7 @@ module ActiveRecord
                   "VALUES (#{@owner.quoted_id},#{record.quoted_id})"
             @owner.connection.execute(sql)
           end
+          true
         end
         
         def insert_record_with_join_attributes(record, join_attributes)
@@ -129,6 +132,16 @@ module ActiveRecord
             @owner.connection.execute(sql)
           end
         end
-      end
+
+        def construct_sql
+          interpolate_sql_options!(@options, :finder_sql, :delete_sql)
+          @finder_sql = @options[:finder_sql] ||
+                "SELECT t.*, j.* FROM #{@association_table_name} t, #{@join_table} j " +
+                "WHERE t.#{@association_class.primary_key} = j.#{@association_foreign_key} AND " +
+                "j.#{@association_class_primary_key_name} = #{@owner.quoted_id} " +
+                (@options[:conditions] ? " AND " + interpolate_sql(@options[:conditions]) : "") + " " +
+                "ORDER BY #{@order}"
+        end
+    end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index f2652f55cc..92f6f4c262 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -2,37 +2,17 @@ module ActiveRecord
   module Associations
     class HasManyAssociation < AssociationCollection #:nodoc:
       def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
-        super(owner, association_name, association_class_name, association_class_primary_key_name, options)
+        super
         @conditions = sanitize_sql(options[:conditions])
 
-        if options[:finder_sql]
-          @finder_sql = interpolate_sql(options[:finder_sql])
-        else
-          @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}"
-          @finder_sql << " AND #{interpolate_sql(@conditions)}" if @conditions
-        end
-
-        if options[:counter_sql]
-          @counter_sql = interpolate_sql(options[:counter_sql])
-        elsif options[:finder_sql]
-          options[:counter_sql] = options[:finder_sql].gsub(/SELECT (.*) FROM/i, "SELECT COUNT(*) FROM")
-          @counter_sql = interpolate_sql(options[:counter_sql])
-        else
-          @counter_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
-        end
-      end
-
-      def create(attributes = {})
-        # Can't use Base.create since the foreign key may be a protected attribute.
-        record = build(attributes)
-        record.save
-        @collection << record if loaded?
-        record
+        construct_sql
       end
 
       def build(attributes = {})
+        load_target
         record = @association_class.new(attributes)
-        record[@association_class_primary_key_name] = @owner.id
+        record[@association_class_primary_key_name] = @owner.id unless @owner.new_record?
+        @target << record
         record
       end
 
@@ -77,10 +57,10 @@ module ActiveRecord
         elsif @options[:finder_sql]
           if ids.size == 1
             id = ids.first
-            record = load_collection.detect { |record| id == record.id }
+            record = load_target.detect { |record| id == record.id }
             expects_array? ? [record] : record
           else
-            load_collection.select { |record| ids.include?(record.id) }
+            load_target.select { |record| ids.include?(record.id) }
           end
 
         # Otherwise, delegate to association class with conditions.
@@ -94,12 +74,12 @@ module ActiveRecord
       # method calls may be chained.
       def clear
         @association_class.update_all("#{@association_class_primary_key_name} = NULL", "#{@association_class_primary_key_name} = #{@owner.quoted_id}")
-        @collection = []
+        @target = []
         self
       end
 
       protected
-        def find_all_records
+        def find_target
           find_all
         end
 
@@ -122,7 +102,8 @@ module ActiveRecord
         end
 
         def insert_record(record)
-          record.update_attribute(@association_class_primary_key_name, @owner.id)
+          record[@association_class_primary_key_name] = @owner.id
+          record.save
         end
 
         def delete_records(records)
@@ -132,6 +113,29 @@ module ActiveRecord
             "#{@association_class_primary_key_name} = #{@owner.quoted_id} AND #{@association_class.primary_key} IN (#{ids})"
           )
         end
+
+        def target_obsolete?
+          false
+        end
+
+        def construct_sql
+          if @options[:finder_sql]
+            @finder_sql = interpolate_sql(@options[:finder_sql])
+          else
+            @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}"
+            @finder_sql << " AND #{interpolate_sql(@conditions)}" if @conditions
+          end
+
+          if @options[:counter_sql]
+            @counter_sql = interpolate_sql(@options[:counter_sql])
+          elsif @options[:finder_sql]
+            @options[:counter_sql] = @options[:finder_sql].gsub(/SELECT (.*) FROM/i, "SELECT COUNT(*) FROM")
+            @counter_sql = interpolate_sql(@options[:counter_sql])
+          else
+            @counter_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}"
+            @counter_sql << " AND #{interpolate_sql(@conditions)}" if @conditions
+          end
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
new file mode 100644
index 0000000000..74e82f146a
--- /dev/null
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -0,0 +1,48 @@
+module ActiveRecord
+  module Associations
+    class HasOneAssociation < BelongsToAssociation #:nodoc:
+      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
+        super
+
+        construct_sql
+      end
+
+      def replace(obj, dont_save = false)
+        load_target
+        unless @target.nil?
+          @target[@association_class_primary_key_name] = nil
+          @target.save unless @owner.new_record?
+        end
+
+        if obj.nil?
+          @target = nil
+        else
+          raise_on_type_mismatch(obj)
+          
+          obj[@association_class_primary_key_name] = @owner.id unless @owner.new_record?
+          @target = obj
+        end
+
+        @loaded = true
+        unless @owner.new_record? or obj.nil? or dont_save
+          return (obj.save ? obj : false)
+        else
+          return obj
+        end
+      end
+      
+      private
+        def find_target
+          @association_class.find_first(@finder_sql, @options[:order])
+        end
+
+        def target_obsolete?
+          false
+        end
+
+        def construct_sql
+          @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@options[:conditions] ? " AND " + @options[:conditions] : ""}"
+        end
+    end
+  end
+end