Fixed that the belongs_to and has_one proxy would fail a test like 'if project.manager' -- this unfortunately also means that you can't call methods like project.manager.build unless there already is a manager on the project #492 [Tim Bates]

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

activerecord/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Fixed that the belongs_to and has_one proxy would fail a test like 'if project.manager' -- this unfortunately also means that you can't call methods like project.manager.build unless there already is a manager on the project #492 [Tim Bates]
+
 * Fixed that the Ruby/MySQL adapter wouldn't connect if the password was empty #503 [Pelle]
 
 

activerecord/lib/active_record/associations.rb

@@ -228,7 +228,7 @@ module ActiveRecord
 
         add_multiple_associated_save_callbacks(association_name)
 
-        association_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, HasManyAssociation)
+        collection_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, HasManyAssociation)
         
         # deprecated api
         deprecated_collection_count_method(association_name)
@@ -289,7 +289,8 @@ module ActiveRecord
         module_eval do
           after_save <<-EOF
             association = instance_variable_get("@#{association_name}")
-            if (true or @new_record_before_save) and association.respond_to?(:loaded?) and not association.nil?
+            loaded = instance_variable_get("@#{association_name}_loaded")
+            if loaded and not association.nil?
               association["#{association_class_primary_key_name}"] = id
               association.save(true)
               association.send(:construct_sql)
@@ -364,7 +365,8 @@ module ActiveRecord
         module_eval do
           before_save <<-EOF
             association = instance_variable_get("@#{association_name}")
-            if association.respond_to?(:loaded?) and not association.nil? and association.new_record?
+            loaded = instance_variable_get("@#{association_name}_loaded")
+            if loaded and not association.nil? and association.new_record?
               association.save(true)
               self["#{association_class_primary_key_name}"] = association.id
               association.send(:construct_sql)
@@ -469,7 +471,7 @@ module ActiveRecord
 
         add_multiple_associated_save_callbacks(association_name)
       
-        association_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, HasAndBelongsToManyAssociation)
+        collection_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, HasAndBelongsToManyAssociation)
 
         before_destroy_sql = "DELETE FROM #{options[:join_table]} WHERE #{association_class_primary_key_name} = \\\#{self.quoted_id}"
         module_eval(%{before_destroy "self.connection.delete(%{#{before_destroy_sql}})"}) # "
@@ -512,6 +514,49 @@ module ActiveRecord
         end
         
         def association_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, association_proxy_class)
+          define_method(association_name) do |*params|
+            force_reload = params.first unless params.empty?
+            loaded = instance_variable_get("@#{association_name}_loaded")
+            if loaded and not force_reload
+              association = instance_variable_get("@#{association_name}")
+            else
+              association = association_proxy_class.new(self,
+                association_name, association_class_name,
+                association_class_primary_key_name, options)
+              retval = association.reload
+              unless retval.nil?
+                instance_variable_set("@#{association_name}", association)
+                instance_variable_set("@#{association_name}_loaded", true)
+              else
+                instance_variable_set("@#{association_name}", nil)
+                instance_variable_set("@#{association_name}_loaded", nil)
+                return nil
+              end
+            end
+            association
+          end
+
+          define_method("#{association_name}=") do |new_value|
+            loaded = instance_variable_get("@#{association_name}_loaded")
+            association = instance_variable_get("@#{association_name}")
+            unless loaded and association
+              association = association_proxy_class.new(self,
+                association_name, association_class_name,
+                association_class_primary_key_name, options)
+            end
+            association.replace(new_value)
+            unless new_value.nil?
+              instance_variable_set("@#{association_name}", association)
+              instance_variable_set("@#{association_name}_loaded", true)
+            else
+              instance_variable_set("@#{association_name}", nil)
+              instance_variable_set("@#{association_name}_loaded", nil)
+            end
+            association
+          end
+        end
+
+        def collection_accessor_methods(association_name, association_class_name, association_class_primary_key_name, options, association_proxy_class)
           define_method(association_name) do |*params|
             force_reload = params.first unless params.empty?
             association = instance_variable_get("@#{association_name}")

activerecord/lib/active_record/associations/association_collection.rb

@@ -11,10 +11,6 @@ module ActiveRecord
         @loaded = false
       end
 
-      def reload
-        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)

activerecord/lib/active_record/associations/association_proxy.rb

@@ -14,6 +14,11 @@ module ActiveRecord
         reset
       end
       
+      def reload
+        reset
+        load_target
+      end
+
       def method_missing(symbol, *args, &block)
         load_target
         @target.send(symbol, *args, &block)

activerecord/lib/active_record/associations/belongs_to_association.rb

@@ -7,11 +7,6 @@ module ActiveRecord
         @loaded = false
       end
 
-      def reload
-        reset
-        load_target
-      end
-
       def create(attributes = {})
         record = build(attributes)
         record.save
@@ -34,12 +29,8 @@ module ActiveRecord
           @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?
+        return (@target.nil? ? nil : self)
       end
 
       private
@@ -65,10 +56,3 @@ module ActiveRecord
     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

activerecord/lib/active_record/associations/has_one_association.rb

@@ -25,9 +25,9 @@ module ActiveRecord
 
         @loaded = true
         unless @owner.new_record? or obj.nil? or dont_save
-          return (obj.save ? obj : false)
+          return (obj.save ? self : false)
         else
-          return obj
+          return (obj.nil? ? nil : self)
         end
       end
       

activerecord/test/associations_test.rb

@@ -105,7 +105,7 @@ class HasOneAssociationsTest < Test::Unit::TestCase
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save
 
-    account = firm.account.build("credit_limit" => 1000)
+    firm.account = account = Account.new("credit_limit" => 1000)
     assert_equal account, firm.account
     assert account.save
     assert_equal account, firm.account
@@ -125,7 +125,7 @@ class HasOneAssociationsTest < Test::Unit::TestCase
   def test_build_before_either_saved
     firm = Firm.new("name" => "GlobalMegaCorp")
 
-    account = firm.account.build("credit_limit" => 1000)
+    firm.account = account = Account.new("credit_limit" => 1000)
     assert_equal account, firm.account
     assert account.new_record?
     assert firm.save
@@ -137,7 +137,7 @@ class HasOneAssociationsTest < Test::Unit::TestCase
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save
     
-    account = firm.account.build
+    firm.account = account = Account.new
     assert_equal account, firm.account
     assert !account.save
     assert_equal account, firm.account
@@ -147,12 +147,14 @@ class HasOneAssociationsTest < Test::Unit::TestCase
   def test_create
     firm = Firm.new("name" => "GlobalMegaCorp")
     firm.save
-    assert_equal firm.account.create("credit_limit" => 1000), firm.account
+    firm.account = account = Account.create("credit_limit" => 1000)
+    assert_equal account, firm.account
   end
 
   def test_create_before_save
     firm = Firm.new("name" => "GlobalMegaCorp")
-    assert_equal firm.account.create("credit_limit" => 1000), firm.account
+    firm.account = account = Account.create("credit_limit" => 1000)
+    assert_equal account, firm.account
   end
 
   def test_dependence_with_missing_association
@@ -243,6 +245,21 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_equal 1, Firm.find_first.clients_using_counter_sql.size
     assert_equal 0, Firm.find_first.clients_using_zero_counter_sql.size
   end
+  
+  def test_belongs_to_sanity
+    c = Client.new
+    assert_nil c.firm
+
+    if c.firm
+      assert false, "belongs_to failed if check"
+    end
+
+    unless c.firm
+    else
+      assert false,  "belongs_to failed unless check"
+    end
+        
+  end
 
   def test_find_ids
     firm = Firm.find_first