Sanitize Base#inspect. Closes #8392.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6761 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

5 files changed, 39 insertions, 35 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index e1f6a91de4..00cac2f0ac 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Sanitize Base#inspect.  #8392 [Nik Wakelin]
+
 * Replace the transaction {|transaction|..} semantics with a new Exception ActiveRecord::Rollback.   [Koz]
 
 * Oracle: extract column length for CHAR also.  #7866 [ymendel]
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index 010e4e396b..96cbd3043a 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -73,7 +73,8 @@ module ActiveRecord
       end
       
       def inspect
-        loaded? ? @target.inspect : "<#{@reflection.name} not loaded yet>"
+        reload unless loaded?
+        @target.inspect
       end
 
       protected
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 31b25e5979..65f808815e 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -36,7 +36,7 @@ module ActiveRecord #:nodoc:
   end
   class Rollback < StandardError #:nodoc:
   end
-  
+
   class AttributeAssignmentError < ActiveRecordError #:nodoc:
     attr_reader :exception, :attribute
     def initialize(message, exception, attribute)
@@ -1812,6 +1812,20 @@ module ActiveRecord #:nodoc:
         clone_attributes :read_attribute_before_type_cast
       end
 
+      # Format attributes nicely for inspect.
+      def attribute_for_inspect(attr_name)
+        raise "Attribute not present #{attr_name}" unless has_attribute?(attr_name)
+        value = read_attribute(attr_name)
+
+        if (value.is_a?(String) && value.length > 50)
+          '"' + value[0..50] + '..."'
+        elsif (value.is_a?(Date) || value.is_a?(Time))
+          '"' + value.to_s(:db) + '"'
+        else
+          value.inspect
+        end
+      end
+
       # Returns true if the specified +attribute+ has been set by the user or by a database load and is neither
       # nil nor empty? (the latter only applies to objects that respond to empty?, most notably Strings).
       def attribute_present?(attribute)
@@ -1892,6 +1906,11 @@ module ActiveRecord #:nodoc:
         @readonly = true
       end
 
+      # Nice pretty inspect.
+      def inspect
+        attributes_as_nice_string = self.class.column_names.collect {|attr_name| "#{attr_name}: #{attribute_for_inspect(attr_name)}"}.join(", ")
+        "#<#{self.class.name} #{attributes_as_nice_string}>"
+      end
 
     private
       def create_or_update
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index aec6a47583..8f60fc95f1 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -93,43 +93,12 @@ class AssociationProxyTest < Test::Unit::TestCase
 
   def test_push_does_not_load_target
     david = authors(:david)
-    not_loaded_string = '<categories not loaded yet>'
-    not_loaded_re = Regexp.new(not_loaded_string)
     
     david.categories << categories(:technology)
-    assert_match not_loaded_re, david.inspect
-    assert_equal not_loaded_string, david.categories.inspect
+    assert !david.categories.loaded?
     assert david.categories.include?(categories(:technology))
   end
   
-  def test_inspect_does_not_load_target
-    david = authors(:david)
-    not_loaded_string = '<posts not loaded yet>'
-    not_loaded_re = Regexp.new(not_loaded_string)
-
-    2.times do
-      assert !david.posts.loaded?, "Posts should not be loaded yet"
-      assert_match not_loaded_re, david.inspect
-      assert_equal not_loaded_string, david.posts.inspect
-
-      assert !david.posts.empty?, "There should be more than one post"
-      assert !david.posts.loaded?, "Posts should still not be loaded yet"
-      assert_match not_loaded_re, david.inspect
-      assert_equal not_loaded_string, david.posts.inspect
-
-      assert !david.posts.find(:all).empty?, "There should be more than one post"
-      assert !david.posts.loaded?, "Posts should still not be loaded yet"
-      assert_match not_loaded_re, david.inspect
-      assert_equal not_loaded_string, david.posts.inspect
-
-      assert !david.posts(true).empty?, "There should be more than one post"
-      assert david.posts.loaded?, "Posts should be loaded now"
-      assert_no_match  not_loaded_re, david.inspect
-      assert_not_equal not_loaded_string, david.posts.inspect
-
-      david.reload
-    end
-  end
 end
 
 class HasOneAssociationsTest < Test::Unit::TestCase
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 7f629a91f4..152355c9dd 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -1623,7 +1623,7 @@ class BasicsTest < Test::Unit::TestCase
   def test_to_param_should_return_string
     assert_kind_of String, Client.find(:first).to_param
   end
-
+  
   # FIXME: this test ought to run, but it needs to run sandboxed so that it
   # doesn't b0rk the current test environment by undefing everything.
   #
@@ -1646,6 +1646,19 @@ class BasicsTest < Test::Unit::TestCase
   #    "expected last count (#{counts.last}) to be <= first count (#{counts.first})"
   #end
   
+  def test_inspect
+    topic = topics(:first)
+    assert_equal topic.inspect, '#<Topic id: 1, title: "The First Topic", author_name: "David", author_email_address: "david@loudthinking.com", written_on: "2003-07-16 07:28:11", bonus_time: "2000-01-01 06:28:00", last_read: "2004-04-15", content: "Have a nice day", approved: false, replies_count: 1, parent_id: nil, type: nil>'
+  end
+  
+  def test_attribute_for_inspect
+    t = topics(:first)
+    t.content = %(This is some really long content, longer than 50 characters, so I can test that text is truncated correctly by the new ActiveRecord::Base#inspect method! Yay! BOOM!)
+    
+    assert_equal t.attribute_for_inspect(:written_on), '"' + t.written_on.to_s(:db) + '"'
+    assert_equal t.attribute_for_inspect(:content),    '"This is some really long content, longer than 50 ch..."'
+  end
+  
   private
     def assert_readers(model, exceptions)
       expected_readers = Set.new(model.column_names - ['id'])