Merge pull request #15788 from sgrif/sg-mutable-strings

Detect in-place modifications on Strings

3 files changed, 39 insertions, 12 deletions

diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 5d6601a881..ea73c561e9 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -309,16 +309,14 @@ class DirtyTest < ActiveRecord::TestCase
   def test_attribute_will_change!
     pirate = Pirate.create!(:catchphrase => 'arr')
 
-    pirate.catchphrase << ' matey'
     assert !pirate.catchphrase_changed?
-
     assert pirate.catchphrase_will_change!
     assert pirate.catchphrase_changed?
-    assert_equal ['arr matey', 'arr matey'], pirate.catchphrase_change
+    assert_equal ['arr', 'arr'], pirate.catchphrase_change
 
-    pirate.catchphrase << '!'
+    pirate.catchphrase << ' matey!'
     assert pirate.catchphrase_changed?
-    assert_equal ['arr matey', 'arr matey!'], pirate.catchphrase_change
+    assert_equal ['arr', 'arr matey!'], pirate.catchphrase_change
   end
 
   def test_association_assignment_changes_foreign_key
diff --git a/activerecord/test/cases/type/string_test.rb b/activerecord/test/cases/type/string_test.rb
new file mode 100644
index 0000000000..420177ed49
--- /dev/null
+++ b/activerecord/test/cases/type/string_test.rb
@@ -0,0 +1,36 @@
+require 'cases/helper'
+
+module ActiveRecord
+  class StringTypeTest < ActiveRecord::TestCase
+    test "type casting" do
+      type = Type::String.new
+      assert_equal "1", type.type_cast_from_user(true)
+      assert_equal "0", type.type_cast_from_user(false)
+      assert_equal "123", type.type_cast_from_user(123)
+    end
+
+    test "values are duped coming out" do
+      s = "foo"
+      type = Type::String.new
+      assert_not_same s, type.type_cast_from_user(s)
+      assert_not_same s, type.type_cast_from_database(s)
+    end
+
+    test "string mutations are detected" do
+      klass = Class.new(Base)
+      klass.table_name = 'authors'
+
+      author = klass.create!(name: 'Sean')
+      assert_not author.changed?
+
+      author.name << ' Griffin'
+      assert author.name_changed?
+
+      author.save!
+      author.reload
+
+      assert_equal 'Sean Griffin', author.name
+      assert_not author.changed?
+    end
+  end
+end
diff --git a/activerecord/test/cases/types_test.rb b/activerecord/test/cases/types_test.rb
index 961aae88cb..47cf775cb6 100644
--- a/activerecord/test/cases/types_test.rb
+++ b/activerecord/test/cases/types_test.rb
@@ -35,13 +35,6 @@ module ActiveRecord
         assert_equal false, type.type_cast_from_user('SOMETHING RANDOM')
       end
 
-      def test_type_cast_string
-        type = Type::String.new
-        assert_equal "1", type.type_cast_from_user(true)
-        assert_equal "0", type.type_cast_from_user(false)
-        assert_equal "123", type.type_cast_from_user(123)
-      end
-
       def test_type_cast_integer
         type = Type::Integer.new
         assert_equal 1, type.type_cast_from_user(1)