From bce0e1493024ccfe78efc93da14740a8d503cb7c Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 18 Jan 2005 11:07:03 +0000 Subject: [PATCH] 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 | 2 + .../lib/active_record/associations.rb | 53 +++++++++++++++++-- .../associations/association_collection.rb | 4 -- .../associations/association_proxy.rb | 5 ++ .../associations/belongs_to_association.rb | 18 +------ .../associations/has_one_association.rb | 4 +- activerecord/test/associations_test.rb | 27 ++++++++-- 7 files changed, 81 insertions(+), 32 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 2a5c4cd878..a3bc6f4806 100644 --- a/activerecord/CHANGELOG +++ b/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] diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 882baffbe4..166d985eee 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/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}") diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index f4d8be8b0f..9507822f40 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/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) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index e1dcffe537..009e2ec4c2 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/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) diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 8de48cd880..521aff4058 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/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 diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 74e82f146a..30704b6f7e 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/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 diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb index 6ed550a730..0e992526bc 100755 --- a/activerecord/test/associations_test.rb +++ b/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