Merge pull request #29378 from kamipo/avoid_overwriting_id_attribute_methods

Avoid overwriting the methods of `AttributeMethods::PrimaryKey`

3 files changed, 21 insertions, 12 deletions

diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index 5ded619716..200d9e6434 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -46,7 +46,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase
     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)
@@ -56,23 +56,30 @@ class PrimaryKeysTest < ActiveRecord::TestCase
   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)
@@ -83,7 +90,7 @@ class PrimaryKeysTest < ActiveRecord::TestCase
     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")
diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb
index 417c6f4832..b5386ba801 100644
--- a/activerecord/test/cases/schema_dumper_test.rb
+++ b/activerecord/test/cases/schema_dumper_test.rb
@@ -320,7 +320,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
 
   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?
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 50f1d9bfe7..8863736943 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -807,16 +807,18 @@ ActiveRecord::Schema.define do
     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