Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid overwriting the methods of AttributeMethods::PrimaryKey #29378

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 3 additions & 7 deletions activerecord/lib/active_record/attribute_methods/primary_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,12 @@ def attribute_method?(attr_name)
end

module ClassMethods
def define_method_attribute(attr_name)
super
ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set

if attr_name == primary_key && attr_name != "id"
generated_attribute_methods.send(:alias_method, :id, primary_key)
end
def instance_method_already_implemented?(method_name)
super || primary_key && ID_ATTRIBUTE_METHODS.include?(method_name)
end

ID_ATTRIBUTE_METHODS = %w(id id= id? id_before_type_cast id_was id_in_database).to_set

def dangerous_attribute_method?(method_name)
super && !ID_ATTRIBUTE_METHODS.include?(method_name)
end
Expand Down
17 changes: 12 additions & 5 deletions activerecord/test/cases/primary_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def test_integer_key
topic = Topic.new
topic.title = "New Topic"
assert_nil topic.id
assert_nothing_raised { topic.save! }
topic.save!
id = topic.id

topicReloaded = Topic.find(id)
Expand All @@ -56,23 +56,30 @@ def test_integer_key
def test_customized_primary_key_auto_assigns_on_save
Keyboard.delete_all
keyboard = Keyboard.new(name: "HHKB")
assert_nothing_raised { keyboard.save! }
keyboard.save!
assert_equal keyboard.id, Keyboard.find_by_name("HHKB").id
end

def test_customized_primary_key_can_be_get_before_saving
keyboard = Keyboard.new
assert_nil keyboard.id
assert_nothing_raised { assert_nil keyboard.key_number }
assert_nil keyboard.key_number
end

def test_customized_string_primary_key_settable_before_save
subscriber = Subscriber.new
assert_nothing_raised { subscriber.id = "webster123" }
subscriber.id = "webster123"
assert_equal "webster123", subscriber.id
assert_equal "webster123", subscriber.nick
end

def test_update_with_non_primary_key_id_column
subscriber = Subscriber.first
subscriber.update(update_count: 1)
subscriber.reload
assert_equal 1, subscriber.update_count
end

def test_string_key
subscriber = Subscriber.find(subscribers(:first).nick)
assert_equal(subscribers(:first).name, subscriber.name)
Expand All @@ -83,7 +90,7 @@ def test_string_key
subscriber.id = "jdoe"
assert_equal("jdoe", subscriber.id)
subscriber.name = "John Doe"
assert_nothing_raised { subscriber.save! }
subscriber.save!
assert_equal("jdoe", subscriber.id)

subscriberReloaded = Subscriber.find("jdoe")
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/schema_dumper_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def test_schema_dump_keeps_id_column_when_id_is_false_and_id_column_added

def test_schema_dump_keeps_id_false_when_id_is_false_and_unique_not_null_column_added
output = standard_dump
assert_match %r{create_table "subscribers", id: false}, output
assert_match %r{create_table "string_key_objects", id: false}, output
end

if ActiveRecord::Base.connection.supports_foreign_keys?
Expand Down
14 changes: 8 additions & 6 deletions activerecord/test/schema/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -807,16 +807,18 @@
t.string :sponsorable_type
end

create_table :string_key_objects, id: false, primary_key: :id, force: true do |t|
t.string :id
t.string :name
t.integer :lock_version, null: false, default: 0
create_table :string_key_objects, id: false, force: true do |t|
t.string :id, null: false
t.string :name
t.integer :lock_version, null: false, default: 0
t.index :id, unique: true
end

create_table :subscribers, force: true, id: false do |t|
create_table :subscribers, force: true do |t|
t.string :nick, null: false
t.string :name
t.column :books_count, :integer, null: false, default: 0
t.integer :books_count, null: false, default: 0
t.integer :update_count, null: false, default: 0
t.index :nick, unique: true
end

Expand Down