From 65cd0fda2572ac9c78d8582496a9009d0c48df08 Mon Sep 17 00:00:00 2001
From: Eugene Kenny <elkenny@gmail.com>
Date: Tue, 6 Nov 2018 01:51:52 +0000
Subject: Fix inspect with non-primary key id attribute

The `read_attribute` method always returns the primary key when asked to
read the `id` attribute, even if the primary key isn't named `id`, and
even if another attribute named `id` exists.

For the `inspect`, `attribute_for_inspect` and `pretty_print` methods,
this behaviour is undesirable, as they're used to examine the internal
state of the record. By using `_read_attribute` instead, we'll get the
real value of the `id` attribute.
---
 activerecord/lib/active_record/attribute_methods.rb |  2 +-
 activerecord/lib/active_record/core.rb              |  4 ++--
 activerecord/test/cases/attribute_methods_test.rb   |  7 +++++++
 activerecord/test/cases/core_test.rb                | 12 ++++++++++++
 activerecord/test/models/topic.rb                   |  4 ++++
 5 files changed, 26 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 1e92ee3b96..fd8c1da842 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -328,7 +328,7 @@ module ActiveRecord
     #   person.attribute_for_inspect(:tag_ids)
     #   # => "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]"
     def attribute_for_inspect(attr_name)
-      value = read_attribute(attr_name)
+      value = _read_attribute(attr_name)
       format_for_inspect(value)
     end
 
diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb
index da3e2549a2..50f3087c51 100644
--- a/activerecord/lib/active_record/core.rb
+++ b/activerecord/lib/active_record/core.rb
@@ -498,7 +498,7 @@ module ActiveRecord
       inspection = if defined?(@attributes) && @attributes
         self.class.attribute_names.collect do |name|
           if has_attribute?(name)
-            attr = read_attribute(name)
+            attr = _read_attribute(name)
             value = if attr.nil?
               attr.inspect
             else
@@ -528,7 +528,7 @@ module ActiveRecord
               pp.text attr_name
               pp.text ":"
               pp.breakable
-              value = read_attribute(attr_name)
+              value = _read_attribute(attr_name)
               value = inspection_filter.filter_param(attr_name, value) unless value.nil?
               pp.pp value
             end
diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb
index 6c2e256447..0dbdd56ae6 100644
--- a/activerecord/test/cases/attribute_methods_test.rb
+++ b/activerecord/test/cases/attribute_methods_test.rb
@@ -56,6 +56,13 @@ class AttributeMethodsTest < ActiveRecord::TestCase
     assert_equal "[1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11]", t.attribute_for_inspect(:content)
   end
 
+  test "attribute_for_inspect with a non-primary key id attribute" do
+    t = topics(:first).becomes(TitlePrimaryKeyTopic)
+    t.title = "The First Topic Now Has A Title With\nNewlines And More Than 50 Characters"
+
+    assert_equal "1", t.attribute_for_inspect(:id)
+  end
+
   test "attribute_present" do
     t = Topic.new
     t.title = "hello there!"
diff --git a/activerecord/test/cases/core_test.rb b/activerecord/test/cases/core_test.rb
index f7fbf3ee8a..36e3d543cd 100644
--- a/activerecord/test/cases/core_test.rb
+++ b/activerecord/test/cases/core_test.rb
@@ -30,6 +30,11 @@ class CoreTest < ActiveRecord::TestCase
     assert_equal %(#<Topic id: 1, title: "The First Topic">), Topic.all.merge!(select: "id, title", where: "id = 1").first.inspect
   end
 
+  def test_inspect_instance_with_non_primary_key_id_attribute
+    topic = topics(:first).becomes(TitlePrimaryKeyTopic)
+    assert_match(/id: 1/, topic.inspect)
+  end
+
   def test_inspect_class_without_table
     assert_equal "NonExistentTable(Table doesn't exist)", NonExistentTable.inspect
   end
@@ -110,4 +115,11 @@ class CoreTest < ActiveRecord::TestCase
     PP.pp(subtopic.new, StringIO.new(actual))
     assert_equal "inspecting topic\n", actual
   end
+
+  def test_pretty_print_with_non_primary_key_id_attribute
+    topic = topics(:first).becomes(TitlePrimaryKeyTopic)
+    actual = +""
+    PP.pp(topic, StringIO.new(actual))
+    assert_match(/id: 1/, actual)
+  end
 end
diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb
index 4aad6a4498..03430154db 100644
--- a/activerecord/test/models/topic.rb
+++ b/activerecord/test/models/topic.rb
@@ -138,6 +138,10 @@ class BlankTopic < Topic
   end
 end
 
+class TitlePrimaryKeyTopic < Topic
+  self.primary_key = :title
+end
+
 module Web
   class Topic < ActiveRecord::Base
     has_many :replies, dependent: :destroy, foreign_key: "parent_id", class_name: "Web::Reply"
-- 
cgit v1.2.3