From 2e4fe3a4ada95d08a77ff4df5cbf49ada0a10f6d Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Thu, 15 Jun 2017 13:00:20 +0100 Subject: [PATCH] Don't map id to primary key in raw_write_attribute The `raw_write_attribute` method is used to update a record's attributes to reflect the new state of the database in `update_columns`. The hash provided to `update_columns` is turned into an UPDATE query directly, which means passing an `id` key results in an update to the `id` column, even if the model uses a different attribute as its primary key. When updating the record, we don't want to apply the `id` column change to the primary key attribute, since that's not what happened in the query. Without the code to handle this case, `write_attribute_with_type_cast` no longer contains any logic shared between `raw_write_attribute` and `write_attribute`, so we can inline the code into those two methods. --- .../active_record/attribute_methods/write.rb | 21 ++++++------------- activerecord/test/cases/primary_keys_test.rb | 6 ++++++ 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index fe0e01db28..75c5a1a600 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -35,11 +35,15 @@ module ActiveRecord attr_name.to_s end - write_attribute_with_type_cast(name, value, true) + name = self.class.primary_key if name == "id".freeze && self.class.primary_key + @attributes.write_from_user(name, value) + value end def raw_write_attribute(attr_name, value) # :nodoc: - write_attribute_with_type_cast(attr_name, value, false) + name = attr_name.to_s + @attributes.write_cast_value(name, value) + value end private @@ -47,19 +51,6 @@ module ActiveRecord def attribute=(attribute_name, value) write_attribute(attribute_name, value) end - - def write_attribute_with_type_cast(attr_name, value, should_type_cast) - attr_name = attr_name.to_s - attr_name = self.class.primary_key if attr_name == "id" && self.class.primary_key - - if should_type_cast - @attributes.write_from_user(attr_name, value) - else - @attributes.write_cast_value(attr_name, value) - end - - value - end end end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 200d9e6434..56229b70bc 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -80,6 +80,12 @@ class PrimaryKeysTest < ActiveRecord::TestCase assert_equal 1, subscriber.update_count end + def test_update_columns_with_non_primary_key_id_column + subscriber = Subscriber.first + subscriber.update_columns(id: 1) + assert_not_equal 1, subscriber.nick + end + def test_string_key subscriber = Subscriber.find(subscribers(:first).nick) assert_equal(subscribers(:first).name, subscriber.name)