Association builder creates duplicates when associated table is :id => false

[samwgoldman]

If model A has_many B, and B uses a natural key instead of an "id" column, creating a B through A's B-association builder results in A's B-association array containing two duplicate copies.

Steps to reproduce:

1. Create an ActiveRecord object (A) with an ID
2. Create an ActiveRecord object (B) with no ID, referencing A
3. Define has_many association from A to B
4. Create a B through A's B-association build
5. Inspect the content's of A's B-association array

Expected results:
A's B-association array contains exactly the newly created B

Actual results:
A's B-association array contains two identical copies of the newly created B

Note:
"reloading" the A instance removes the duplicate copy.

This bug occurs with ActiveRecord 3.2.11.
I have reproduced the behavior using both PostgreSQL and SQLite.

For a full test case, see https://github.com/samwgoldman/ar_natural_key_bug

[samwgoldman]

I have tested and confirmed the same failure as of 2f0c26b

[samwgoldman]

Sorry for comment spam.

Workaround:

class B < ActiveRecord::Base
  def id
    # the natural key
  end
end

[carlosantoniodasilva]

What's the "natural key" in your credit card example, the payment_method_token attribute? Shouldn't it be set as your "primary key" in that class? Have you tested it? Thanks for reporting.

[samwgoldman]

@carlosantoniodasilva I think you're right, I could have a primary key in my example because two customers will never have the same payment_method_token. I could do that with a secondary index for account_id and things would work for my example.

However, it was just an example. Perhaps I should come up with a better example. I do think the current behavior is wrong regardless of my example.

[senny]

I investigates a bit and on master the problem seems to be fixed. I attached a test-case which passes on master and fails on 3-2-stable. Sadly I was not able to locate the change that fixed it and therefore I'm not sure if we should include the test-case to prevent further regressions. (bisecting over a long timespan feels impossible on master due to endless bundler issues).

@carlosantoniodasilva I'd like to find the commit that fixed the problem to determine if we want to include my test-case but I'm not sure how to find it. What do you suggest?

[carlosantoniodasilva]

I think it's good to have a test case that ensures it is fixed and won't break again in master.

Unfortunately I don't have any idea other than running the bundle on every bisect, which is quite painful to do. But if it cannot be backported, that's fine because at least it's fixed in master and people migrating to rails 4 will have it fixed.