Merge pull request #26155 from kamipo/refactor_quoted_columns_for_index

Refactor `quoted_columns_for_index` by extracted `add_options_for_index_columns`

2 files changed, 25 insertions, 29 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index da99d00671..cb5d56d4d2 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1161,31 +1161,34 @@ module ActiveRecord
       end
 
       protected
-        def add_index_sort_order(option_strings, column_names, options = {})
-          if options.is_a?(Hash) && order = options[:order]
+
+        def add_index_sort_order(quoted_columns, **options)
+          if order = options[:order]
             case order
             when Hash
-              column_names.each { |name| option_strings[name] += " #{order[name].upcase}" if order.has_key?(name) }
+              quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? }
             when String
-              column_names.each { |name| option_strings[name] += " #{order.upcase}" }
+              quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? }
             end
           end
 
-          return option_strings
+          quoted_columns
         end
 
         # Overridden by the MySQL adapter for supporting index lengths
-        def quoted_columns_for_index(column_names, options = {})
-          return [column_names] if column_names.is_a?(String)
-
-          option_strings = Hash[column_names.map { |name| [name, ""] }]
-
-          # add index sort order if supported
+        def add_options_for_index_columns(quoted_columns, **options)
           if supports_index_sort_order?
-            option_strings = add_index_sort_order(option_strings, column_names, options)
+            quoted_columns = add_index_sort_order(quoted_columns, options)
           end
 
-          column_names.map { |name| quote_column_name(name) + option_strings[name] }
+          quoted_columns
+        end
+
+        def quoted_columns_for_index(column_names, **options)
+          return [column_names] if column_names.is_a?(String)
+
+          quoted_columns = Hash[column_names.map { |name| [name, quote_column_name(name).dup] }]
+          add_options_for_index_columns(quoted_columns, options).values
         end
 
         def index_name_for_remove(table_name, options = {})
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 7fa8355f3d..5900919e88 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -713,32 +713,25 @@ module ActiveRecord
           MySQL::TypeMetadata.new(super(sql_type), extra: extra, strict: strict_mode?)
         end
 
-        def add_index_length(option_strings, column_names, options = {})
-          if options.is_a?(Hash) && length = options[:length]
+        def add_index_length(quoted_columns, **options)
+          if length = options[:length]
             case length
             when Hash
-              column_names.each { |name| option_strings[name] += "(#{length[name]})" if length.has_key?(name) && length[name].present? }
+              quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? }
             when Integer
-              column_names.each { |name| option_strings[name] += "(#{length})" }
+              quoted_columns.each { |name, column| column << "(#{length})" }
             end
           end
 
-          return option_strings
+          quoted_columns
         end
 
-        def quoted_columns_for_index(column_names, options = {})
-          option_strings = Hash[column_names.map { |name| [name, ""] }]
-
-          # add index length
-          option_strings = add_index_length(option_strings, column_names, options)
-
-          # add index sort order
-          option_strings = add_index_sort_order(option_strings, column_names, options)
-
-          column_names.map { |name| quote_column_name(name) + option_strings[name] }
+        def add_options_for_index_columns(quoted_columns, **options)
+          quoted_columns = add_index_length(quoted_columns, options)
+          super
         end
 
-      # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html
+        # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html
         ER_DUP_ENTRY            = 1062
         ER_NO_REFERENCED_ROW_2  = 1452
         ER_DATA_TOO_LONG        = 1406