Merge pull request #39599 from kamipo/sg-immutable-strings-by-default

Add a setting to specify that all string columns should be immutable

activemodel/lib/active_model/type/immutable_string.rb

@@ -3,6 +3,12 @@
 module ActiveModel
   module Type
     class ImmutableString < Value # :nodoc:
+      def initialize(**args)
+        @true  = -(args.delete(:true)&.to_s  || "t")
+        @false = -(args.delete(:false)&.to_s || "f")
+        super
+      end
+
       def type
         :string
       end
@@ -10,21 +16,19 @@ module ActiveModel
       def serialize(value)
         case value
         when ::Numeric, ::Symbol, ActiveSupport::Duration then value.to_s
-        when true then "t"
-        when false then "f"
+        when true then @true
+        when false then @false
         else super
         end
       end
 
       private
         def cast_value(value)
-          result = \
           case value
-            when true then "t"
-            when false then "f"
-            else value.to_s
-            end
-          result.freeze
+          when true then @true
+          when false then @false
+          else value.to_s.freeze
+          end
         end
     end
   end

activemodel/lib/active_model/type/registry.rb

@@ -8,6 +8,11 @@ module ActiveModel
         @registrations = []
       end
 
+      def initialize_dup(other)
+        @registrations = @registrations.dup
+        super
+      end
+
       def register(type_name, klass = nil, **options, &block)
         unless block_given?
           block = proc { |_, *args| klass.new(*args) }

activemodel/lib/active_model/type/string.rb

@@ -11,12 +11,22 @@ module ActiveModel
         end
       end
 
+      def to_immutable_string
+        ImmutableString.new(
+          true: @true,
+          false: @false,
+          limit: limit,
+          precision: precision,
+          scale: scale,
+        )
+      end
+
       private
         def cast_value(value)
           case value
           when ::String then ::String.new(value)
-          when true then "t"
-          when false then "f"
+          when true then @true
+          when false then @false
           else value.to_s
           end
         end

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Added the setting `ActiveRecord::Base.immutable_strings_by_default`, which
+    allows you to specify that all string columns should be frozen unless
+    otherwise specified. This will reduce memory pressure for applications which
+    do not generally mutate string properties of Active Record objects.
+
+    *Sean Griffin*
+
 *   Deprecate `map!` and `collect!` on `ActiveRecord::Result`.
 
     *Ryuta Kamizono*

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -576,7 +576,10 @@ module ActiveRecord
         def initialize_type_map(m = type_map)
           super
 
-          register_class_with_limit m, %r(char)i, MysqlString
+          m.register_type(%r(char)i) do |sql_type|
+            limit = extract_limit(sql_type)
+            Type.lookup(:string, adapter: :mysql2, limit: limit)
+          end
 
           m.register_type %r(tinytext)i,   Type::Text.new(limit: 2**8 - 1)
           m.register_type %r(tinyblob)i,   Type::Binary.new(limit: 2**8 - 1)
@@ -599,8 +602,8 @@ module ActiveRecord
           m.alias_type %r(year)i, "integer"
           m.alias_type %r(bit)i,  "binary"
 
-          m.register_type %r(^enum)i, MysqlString.new
-          m.register_type %r(^set)i,  MysqlString.new
+          m.register_type %r(^enum)i, Type.lookup(:string, adapter: :mysql2)
+          m.register_type %r(^set)i,  Type.lookup(:string, adapter: :mysql2)
         end
 
         def register_integer_type(mapping, key, **options)
@@ -841,26 +844,12 @@ module ActiveRecord
           full_version_string.match(/^(?:5\.5\.5-)?(\d+\.\d+\.\d+)/)[1]
         end
 
-        class MysqlString < Type::String # :nodoc:
-          def serialize(value)
-            case value
-            when true then "1"
-            when false then "0"
-            else super
+        ActiveRecord::Type.register(:immutable_string, adapter: :mysql2) do |_, **args|
+          Type::ImmutableString.new(true: "1", false: "0", **args)
         end
+        ActiveRecord::Type.register(:string, adapter: :mysql2) do |_, **args|
+          Type::String.new(true: "1", false: "0", **args)
         end
-
-          private
-            def cast_value(value)
-              case value
-              when true then "1"
-              when false then "0"
-              else super
-              end
-            end
-        end
-
-        ActiveRecord::Type.register(:string, MysqlString, adapter: :mysql2)
         ActiveRecord::Type.register(:unsigned_integer, Type::UnsignedInteger, adapter: :mysql2)
     end
   end

activerecord/lib/active_record/model_schema.rb

@@ -117,6 +117,15 @@ module ActiveRecord
     # during an ordered finder call. Useful when the primary key is not an
     # auto-incrementing integer, for example when it's a UUID. Records are subsorted
     # by the primary key if it exists to ensure deterministic results.
+
+    ##
+    # :singleton-method: immutable_strings_by_default=
+    # :call-seq: immutable_strings_by_default=(bool)
+    #
+    # Determines whether columns should infer their type as `:string` or
+    # `:immutable_string`. This setting does not affect the behavior of
+    # `attribute :foo, :string`. Defaults to false.
+
     included do
       mattr_accessor :primary_key_prefix_type, instance_writer: false
 
@@ -126,6 +135,7 @@ module ActiveRecord
       class_attribute :internal_metadata_table_name, instance_accessor: false, default: "ar_internal_metadata"
       class_attribute :pluralize_table_names, instance_writer: false, default: true
       class_attribute :implicit_order_column, instance_accessor: false
+      class_attribute :immutable_strings_by_default, instance_accessor: false
 
       self.protected_environments = ["production"]
       self.inheritance_column = "type"
@@ -495,9 +505,11 @@ module ActiveRecord
           columns_hash = columns_hash.except(*ignored_columns) unless ignored_columns.empty?
           @columns_hash = columns_hash.freeze
           @columns_hash.each do |name, column|
+            type = connection.lookup_cast_type_from_column(column)
+            type = _convert_type_from_options(type)
             define_attribute(
               name,
-              connection.lookup_cast_type_from_column(column),
+              type,
               default: column.default,
               user_provided_default: false
             )
@@ -546,6 +558,14 @@ module ActiveRecord
             base_class.table_name
           end
         end
+
+        def _convert_type_from_options(type)
+          if immutable_strings_by_default && type.respond_to?(:to_immutable_string)
+            type.to_immutable_string
+          else
+            type
+          end
+        end
     end
   end
 end

activerecord/lib/active_record/type.rb

@@ -63,6 +63,7 @@ module ActiveRecord
     Decimal = ActiveModel::Type::Decimal
     Float = ActiveModel::Type::Float
     Integer = ActiveModel::Type::Integer
+    ImmutableString = ActiveModel::Type::ImmutableString
     String = ActiveModel::Type::String
     Value = ActiveModel::Type::Value
 
@@ -74,6 +75,7 @@ module ActiveRecord
     register(:decimal, Type::Decimal, override: false)
     register(:float, Type::Float, override: false)
     register(:integer, Type::Integer, override: false)
+    register(:immutable_string, Type::ImmutableString, override: false)
     register(:json, Type::Json, override: false)
     register(:string, Type::String, override: false)
     register(:text, Type::Text, override: false)

activerecord/test/cases/attributes_test.rb

@@ -111,22 +111,24 @@ module ActiveRecord
 
     test "overloading properties does not attribute method order" do
       attribute_names = OverloadedType.attribute_names
-      assert_equal %w(id overloaded_float unoverloaded_float overloaded_string_with_limit string_with_default non_existent_decimal), attribute_names
+      expected = OverloadedType.column_names + ["non_existent_decimal"]
+      assert_equal expected, attribute_names
     end
 
     test "caches are cleared" do
       klass = Class.new(OverloadedType)
+      column_count = klass.columns.length
 
-      assert_equal 6, klass.attribute_types.length
-      assert_equal 6, klass.column_defaults.length
-      assert_equal 6, klass.attribute_names.length
+      assert_equal column_count + 1, klass.attribute_types.length
+      assert_equal column_count + 1, klass.column_defaults.length
+      assert_equal column_count + 1, klass.attribute_names.length
       assert_not klass.attribute_types.include?("wibble")
 
       klass.attribute :wibble, Type::Value.new
 
-      assert_equal 7, klass.attribute_types.length
-      assert_equal 7, klass.column_defaults.length
-      assert_equal 7, klass.attribute_names.length
+      assert_equal column_count + 2, klass.attribute_types.length
+      assert_equal column_count + 2, klass.column_defaults.length
+      assert_equal column_count + 2, klass.attribute_names.length
       assert_includes klass.attribute_types, "wibble"
     end
 
@@ -292,5 +294,44 @@ module ActiveRecord
       assert_equal 1, klass.new(no_type: 1).no_type
       assert_equal "foo", klass.new(no_type: "foo").no_type
     end
+
+    test "immutable_strings_by_default changes schema inference for string columns" do
+      with_immutable_strings do
+        OverloadedType.reset_column_information
+        immutable_string_type = Type.lookup(:immutable_string).class
+        assert_instance_of immutable_string_type, OverloadedType.type_for_attribute("inferred_string")
+      end
+    end
+
+    test "immutable_strings_by_default retains limit information" do
+      with_immutable_strings do
+        OverloadedType.reset_column_information
+        assert_equal 255, OverloadedType.type_for_attribute("inferred_string").limit
+      end
+    end
+
+    test "immutable_strings_by_default does not affect `attribute :foo, :string`" do
+      with_immutable_strings do
+        OverloadedType.reset_column_information
+        default_string_type = Type.lookup(:string).class
+        assert_instance_of default_string_type, OverloadedType.type_for_attribute("string_with_default")
+      end
+    end
+
+    test "serialize boolean for both string types" do
+      default_string_type = Type.lookup(:string)
+      immutable_string_type = Type.lookup(:immutable_string)
+      assert_equal default_string_type.serialize(true), immutable_string_type.serialize(true)
+      assert_equal default_string_type.serialize(false), immutable_string_type.serialize(false)
+    end
+
+    private
+      def with_immutable_strings
+        old_value = ActiveRecord::Base.immutable_strings_by_default
+        ActiveRecord::Base.immutable_strings_by_default = true
+        yield
+      ensure
+        ActiveRecord::Base.immutable_strings_by_default = old_value
+      end
   end
 end

activerecord/test/cases/inheritance_test.rb

@@ -588,7 +588,7 @@ end
 class InheritanceAttributeMappingTest < ActiveRecord::TestCase
   setup do
     @old_registry = ActiveRecord::Type.registry
-    ActiveRecord::Type.registry = ActiveRecord::Type::AdapterSpecificRegistry.new
+    ActiveRecord::Type.registry = ActiveRecord::Type.registry.dup
     ActiveRecord::Type.register :omg_sti, InheritanceAttributeMappingTest::OmgStiType
     Company.delete_all
     Sponsor.delete_all

activerecord/test/schema/schema.rb

@@ -1125,6 +1125,7 @@ ActiveRecord::Schema.define do
     t.float :unoverloaded_float
     t.string :overloaded_string_with_limit, limit: 255
     t.string :string_with_default, default: "the original default"
+    t.string :inferred_string, limit: 255
   end
 
   create_table :users, force: true do |t|