Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  hjdivad July 17th, 2010 @ 04:20 AM

  • no changes were found...

  • issue-5137.diff 2.8 KB

• You flagged this item as spam.
  

  hjdivad July 17th, 2010 @ 04:26 AM

  The above test fails in the block because the association proxy is nil, because
  of the following code:

      # 38f1ea8fe267947f4baf832d3b15ee5b83f18f71
      # associations.rb:1364
      if retval.nil? and association_proxy_class == BelongsToAssociation
          association_instance_set(reflection.name, nil)
          return nil
      end
  

  I believe the motivation for the above code was to handle the case exemplified
  by the following test:

      # 38f1ea8fe267947f4baf832d3b15ee5b83f18f71
      # belongs_to_associations_test.rb:305
      def test_forgetting_the_load_when_foreign_key_enters_late
          c = Client.new
          assert_nil c.firm_with_basic_id
  
          c.firm_id = 1
          # sometimes tests on Oracle fail if ORDER BY is not provided therefore add always :order with :first
          assert_equal Firm.find(:first, :order => "id"), c.firm_with_basic_id
      end
  

• You flagged this item as spam.
  

  hjdivad July 17th, 2010 @ 04:27 AM

  In case it's not clear, the patch handles the test_forgetting_the_load_when_foreign_key_enters_late test (as well as the rest of the activerecord test suite, at least in my environment :-).

• 

  Repository July 26th, 2010 @ 04:50 PM

  • State changed from “new” to “resolved”

  (from [0e969bdaf8ff2e3384350687aa0b583f94d6dfbc]) fix escaping id and parameters in path [#5137 state:resolved]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/0e969bdaf8ff2e3384350687aa0b58...

• You flagged this item as spam.
  

  hjdivad July 26th, 2010 @ 06:38 PM

  I'm fairly sure the linked commit actually resolves 5173, and not this bug.

• 

  José Valim July 26th, 2010 @ 06:42 PM

  • State changed from “resolved” to “new”
  • Importance changed from “” to “Low”

  Sorry, my bad. :)

• You flagged this item as spam.
  

  hjdivad July 26th, 2010 @ 06:47 PM

  No worries (^_^)

• You flagged this item as spam.
  

  Zachery Hostens August 31st, 2010 @ 05:02 PM

  created another ticket in regards to querying a has_one :through through a belongs_to. figured id make it separate since this ticket is about creating the records.

  #5512 i also have a test appended.

• 

  Neeraj Singh August 31st, 2010 @ 10:04 PM

  I am not able to reproduce this problem with rails master. This is what I did.

  require 'test/unit'
  require 'test/unit/assertions'
  class Object
    include Test::Unit::Assertions
  end
  
  
  ActiveRecord::Schema.define(:version => 20100511140856) do
  
    create_table "dashboards", :force => true do |t|
      t.string "name"
    end
  
    create_table "minivans", :force => true do |t|
      t.string "name"
    end
  
    create_table "speedometers", :force => true do |t|
      t.string  "name"
      t.integer "minivan_id"
      t.integer "dashboard_id"
    end
  
  
  end
  class Dashboard < ActiveRecord::Base
  end
  
  class Minivan < ActiveRecord::Base
  
    has_one :speedometer
    has_one :dashboard, :through => :speedometer
  
    def self.lab
      minivan = Minivan.new(:name => 'm1')
      dashboard = Dashboard.create(:name => 'd1')
  
      minivan.dashboard = dashboard
  
      assert minivan.speedometer
      assert_equal dashboard, minivan.dashboard
      minivan.save
      assert_equal dashboard, minivan.dashboard
    end
  
  end
  
  class Speedometer < ActiveRecord::Base
    belongs_to :minivan
    belongs_to :dashboard
  end
  
  > Minivan.lab does not fail for me.
  

• 

  Neeraj Singh August 31st, 2010 @ 10:13 PM

  Ok. The test provided in #5512 does fail for me. Looking into it.

• You flagged this item as spam.
  

  hjdivad September 1st, 2010 @ 12:08 AM

  Neeraj,

  Thank you for looking into this.

  Your code above does not test the problem: there, you have a has_one (dashboard)
  through a has_one(speedometer).

  If you change Minivan so it belongs_to(speedometer), instead of has_one, then
  your test will exhibit the same failure.

      [davidjh@nyx ((no branch); ruby-1.9.2-p0)] ~/devel/foss/rails/activerecord
      $ ruby --version
      ruby 1.9.2p0 (2010-08-18 revision 29036) [x86_64-linux]
      [davidjh@nyx ((no branch); ruby-1.9.2-p0)] ~/devel/foss/rails/activerecord
      $ git rev-parse HEAD
      8b2e4fddbf7e9d204579cd87ac72f14bc592d5a5
      [davidjh@nyx ((no branch); ruby-1.9.2-p0)] ~/devel/foss/rails/activerecord
  

      # cat test/cases/associations/issue_5137.rb
      require 'test/unit'
      require 'test/unit/assertions'
      require 'test/cases/helper'
      class Object
        include Test::Unit::Assertions
      end
  
  
      ActiveRecord::Schema.define(:version => 20100511140856) do
  
        create_table "dashboards", :force => true do |t|
          t.string "name"
        end
  
        create_table "minivans", :force => true do |t|
          t.string "name"
        end
  
        create_table "speedometers", :force => true do |t|
          t.string  "name"
          t.integer "minivan_id"
          t.integer "dashboard_id"
        end
  
  
      end
      class Dashboard < ActiveRecord::Base
      end
  
      class Minivan < ActiveRecord::Base
  
        set_primary_key :minivan_id
  
  
        belongs_to :speedometer
        has_one :dashboard, :through => :speedometer
  
        def self.lab
          minivan = Minivan.new(:name => 'm1')
          dashboard = Dashboard.create(:name => 'd1')
  
          minivan.dashboard = dashboard
  
          assert minivan.speedometer
          assert_equal dashboard, minivan.dashboard
          minivan.save
          assert_equal dashboard, minivan.dashboard
        end
  
      end
  
      class Speedometer < ActiveRecord::Base
        belongs_to :minivan
        belongs_to :dashboard
      end
  
  
      class MinivanTest < ActiveRecord::TestCase
        def test_minivan_lab
          Minivan.lab
        end
      end
  

      [davidjh@nyx ((no branch); ruby-1.9.2-p0)] ~/devel/foss/rails/activerecord
      $ ruby -I'test' -I'test/connections/native_sqlite3/' -I'.' test/cases/associations/issue_5137.rb
      Using native SQLite3
      -- create_table("dashboards", {:force=>true})
         -> 0.0052s
      -- create_table("minivans", {:force=>true})
         -> 0.0273s
      -- create_table("speedometers", {:force=>true})
         -> 0.0055s
      -- initialize_schema_migrations_table()
         -> 0.0014s
      -- assume_migrated_upto_version(20100511140856, "db/migrate")
         -> 0.0003s
      Loaded suite test/cases/associations/issue_5137
      Started
      E
      Finished in 0.057919 seconds.
  
        1) Error:
      test_minivan_lab(MinivanTest):
      NoMethodError: undefined method `build' for nil:NilClass
          /home/davidjh/devel/foss/rails/activerecord/lib/active_record/associations/has_one_through_association.rb:27:in `create_through_record'
          /home/davidjh/devel/foss/rails/activerecord/lib/active_record/associations/has_one_through_association.rb:10:in `replace'
          /home/davidjh/devel/foss/rails/activerecord/lib/active_record/associations.rb:1474:in `block in association_accessor_methods'
          test/cases/associations/issue_5137.rb:42:in `lab'
          test/cases/associations/issue_5137.rb:60:in `test_minivan_lab'
          /home/davidjh/.rvm/gems/ruby-1.9.2-p0/gems/mocha-0.9.8/lib/mocha/integration/mini_test/version_131_and_above.rb:26:in `run'
          /home/davidjh/devel/foss/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:35:in `block in run'
          /home/davidjh/devel/foss/rails/activesupport/lib/active_support/callbacks.rb:413:in `_run_setup_callbacks'
          /home/davidjh/devel/foss/rails/activesupport/lib/active_support/testing/setup_and_teardown.rb:34:in `run'
  
      1 tests, 0 assertions, 0 failures, 1 errors, 0 skips
  
      Test run options: --seed 37085
  

  I have rebased http://github.com/hjdivad/rails/tree/issue-5137 to the current
  master, which as of now is 8b2e4fddbf7e9d204579cd87ac72f14bc592d5a5. The diff
  remains basically the same, but I have regenerated it again for your
  convenience.

  • issue-5137.diff 2.3 KB
• 

  Neeraj Singh September 1st, 2010 @ 09:43 PM

  • Milestone set to “3.x”
  • State changed from “new” to “open”

  I am unable to follow the logic why key_changed is needed.

    key_changed = send("#{reflection.primary_key_name}_changed?"
  

• You flagged this item as spam.
  

  hjdivad September 2nd, 2010 @ 07:31 PM

  Neeraj,

  The test for invoking association.reload was to handle the following case,
  exhibited in a test in test/cases/associations/belongs_to_association_test.rb.

   def test_forgetting_the_load_when_foreign_key_enters_late
     c = Client.new
     assert_nil c.firm_with_basic_id
  
     c.firm_id = 1
     # sometimes tests on Oracle fail if ORDER BY is not provided
  therefore add always :order with :first
     assert_equal Firm.find(:first, :order => "id"), c.firm_with_basic_id
   end
  

  i.e. if the key changes, reload the association. However, the original logic I
  had did not properly handle all cases. I have updated the patch branch with a
  better patch that is also hopefully more clear. Attached is the new patch.

  • issue-5137.diff 4.1 KB
• 

  Neeraj Singh September 3rd, 2010 @ 03:46 PM

  I finally got the minivan example failing. However the attached fix is not passing. I am leaving for the long weekend. Will take a look when I come back.

• 

  Jon Leighton December 21st, 2010 @ 12:12 PM

  This bug seems to be related to (or possibly a duplicate of) #142.

• 

  Repository January 4th, 2011 @ 12:28 AM

  • State changed from “open” to “resolved”

  (from [a0be389d39b790e0625339251d2674b8250b16b1]) Allow assignment on has_one :through where the owner is a new record [#5137 state:resolved]

  This required changing the code to keep the association proxy for a belongs_to around, despite its target being nil. Which in turn required various changes to the way that stale target checking is handled, in order to support various edge cases (loaded target is nil then foreign key added, foreign key is changed and then changed back, etc). A side effect is that the code is nicer and more succinct.

  Note that I am removing test_no_unexpected_aliasing since that is basically checking that the proxy for a belongs_to does change, which is the exact opposite of the intention of this commit. Also adding various tests for various edge cases and related things.

  Phew, long commit message!
  https://github.com/rails/rails/commit/a0be389d39b790e0625339251d267...