Merge pull request #19030 from kamipo/extract_short_hand_column_methods

Extract the short-hand column methods into `ColumnMethods`

4 files changed, 173 insertions, 110 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index a2777fcd0a..0acc815d51 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -142,6 +142,41 @@ module ActiveRecord
       end
     end
 
+    module ColumnMethods
+      # Appends a primary key definition to the table definition.
+      # Can be called multiple times, but this is probably not a good idea.
+      def primary_key(name, type = :primary_key, **options)
+        column(name, type, options.merge(primary_key: true))
+      end
+
+      # Appends a column or columns of a specified type.
+      #
+      #  t.string(:goat)
+      #  t.string(:goat, :sheep)
+      #
+      # See TableDefinition#column
+      [
+        :bigint,
+        :binary,
+        :boolean,
+        :date,
+        :datetime,
+        :decimal,
+        :float,
+        :integer,
+        :string,
+        :text,
+        :time,
+        :timestamp,
+      ].each do |column_type|
+        module_eval <<-CODE, __FILE__, __LINE__ + 1
+          def #{column_type}(*args, **options)
+            args.each { |name| column(name, :#{column_type}, options) }
+          end
+        CODE
+      end
+    end
+
     # Represents the schema of an SQL table in an abstract way. This class
     # provides methods for manipulating the schema representation.
     #
@@ -163,6 +198,8 @@ module ActiveRecord
     # The table definitions
     # The Columns are stored as a ColumnDefinition in the +columns+ attribute.
     class TableDefinition
+      include ColumnMethods
+
       # An array of ColumnDefinition objects, representing the column changes
       # that have been defined.
       attr_accessor :indexes
@@ -181,12 +218,6 @@ module ActiveRecord
 
       def columns; @columns_hash.values; end
 
-      # Appends a primary key definition to the table definition.
-      # Can be called multiple times, but this is probably not a good idea.
-      def primary_key(name, type = :primary_key, options = {})
-        column(name, type, options.merge(:primary_key => true))
-      end
-
       # Returns a ColumnDefinition for the column with name +name+.
       def [](name)
         @columns_hash[name.to_s]
@@ -343,14 +374,6 @@ module ActiveRecord
         @columns_hash.delete name.to_s
       end
 
-      [:string, :text, :integer, :bigint, :float, :decimal, :datetime, :timestamp, :time, :date, :binary, :boolean].each do |column_type|
-        define_method column_type do |*args|
-          options = args.extract_options!
-          column_names = args
-          column_names.each { |name| column(name, column_type, options) }
-        end
-      end
-
       # Adds index options to the indexes hash, keyed by column name
       # This is primarily used to track indexes that need to be created after the table
       #
@@ -462,6 +485,7 @@ module ActiveRecord
     # Available transformations are:
     #
     #   change_table :table do |t|
+    #     t.primary_key
     #     t.column
     #     t.index
     #     t.rename_index
@@ -474,6 +498,7 @@ module ActiveRecord
     #     t.string
     #     t.text
     #     t.integer
+    #     t.bigint
     #     t.float
     #     t.decimal
     #     t.datetime
@@ -490,6 +515,8 @@ module ActiveRecord
     #   end
     #
     class Table
+      include ColumnMethods
+
       attr_reader :name
 
       def initialize(table_name, base)
@@ -640,21 +667,6 @@ module ActiveRecord
       end
       alias :remove_belongs_to :remove_references
 
-      # Adds a column or columns of a specified type.
-      #
-      #  t.string(:goat)
-      #  t.string(:goat, :sheep)
-      #
-      # See SchemaStatements#add_column
-      [:string, :text, :integer, :float, :decimal, :datetime, :timestamp, :time, :date, :binary, :boolean].each do |column_type|
-        define_method column_type do |*args|
-          options = args.extract_options!
-          args.each do |column_name|
-            @base.add_column(name, column_name, column_type, options)
-          end
-        end
-      end
-
       def foreign_key(*args) # :nodoc:
         @base.add_foreign_key(name, *args)
       end
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 e11eaabf4b..8f53794be0 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -6,9 +6,9 @@ module ActiveRecord
     class AbstractMysqlAdapter < AbstractAdapter
       include Savepoints
 
-      class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
-        def primary_key(name, type = :primary_key, options = {})
-          options[:auto_increment] ||= type == :bigint
+      module ColumnMethods
+        def primary_key(name, type = :primary_key, **options)
+          options[:auto_increment] = true if type == :bigint
           super
         end
 
@@ -23,6 +23,14 @@ module ActiveRecord
         end
       end
 
+      class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
+        include ColumnMethods
+      end
+
+      class Table < ActiveRecord::ConnectionAdapters::Table
+        include ColumnMethods
+      end
+
       class SchemaCreation < AbstractAdapter::SchemaCreation
         def visit_AddColumn(o)
           add_column_position!(super, column_options(o))
@@ -67,6 +75,10 @@ module ActiveRecord
         end
       end
 
+      def update_table_definition(table_name, base) # :nodoc:
+        Table.new(table_name, base)
+      end
+
       def schema_creation
         SchemaCreation.new self
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
index b9078d4c86..022dbdfa27 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
@@ -2,90 +2,129 @@ module ActiveRecord
   module ConnectionAdapters
     module PostgreSQL
       module ColumnMethods
-        def xml(*args)
-          options = args.extract_options!
-          column(args[0], :xml, options)
+        # Defines the primary key field.
+        # Use of the native PostgreSQL UUID type is supported, and can be used
+        # by defining your tables as such:
+        #
+        #   create_table :stuffs, id: :uuid do |t|
+        #     t.string :content
+        #     t.timestamps
+        #   end
+        #
+        # By default, this will use the +uuid_generate_v4()+ function from the
+        # +uuid-ossp+ extension, which MUST be enabled on your database. To enable
+        # the +uuid-ossp+ extension, you can use the +enable_extension+ method in your
+        # migrations. To use a UUID primary key without +uuid-ossp+ enabled, you can
+        # set the +:default+ option to +nil+:
+        #
+        #   create_table :stuffs, id: false do |t|
+        #     t.primary_key :id, :uuid, default: nil
+        #     t.uuid :foo_id
+        #     t.timestamps
+        #   end
+        #
+        # You may also pass a different UUID generation function from +uuid-ossp+
+        # or another library.
+        #
+        # Note that setting the UUID primary key default value to +nil+ will
+        # require you to assure that you always provide a UUID value before saving
+        # a record (as primary keys cannot be +nil+). This might be done via the
+        # +SecureRandom.uuid+ method and a +before_save+ callback, for instance.
+        def primary_key(name, type = :primary_key, **options)
+          options[:default] = options.fetch(:default, 'uuid_generate_v4()') if type == :uuid
+          super
+        end
+
+        def bigserial(*args, **options)
+          args.each { |name| column(name, :bigserial, options) }
+        end
+
+        def bit(*args, **options)
+          args.each { |name| column(name, :bit, options) }
         end
 
-        def tsvector(*args)
-          options = args.extract_options!
-          column(args[0], :tsvector, options)
+        def bit_varying(*args, **options)
+          args.each { |name| column(name, :bit_varying, options) }
         end
 
-        def int4range(name, options = {})
-          column(name, :int4range, options)
+        def cidr(*args, **options)
+          args.each { |name| column(name, :cidr, options) }
         end
 
-        def int8range(name, options = {})
-          column(name, :int8range, options)
+        def citext(*args, **options)
+          args.each { |name| column(name, :citext, options) }
         end
 
-        def tsrange(name, options = {})
-          column(name, :tsrange, options)
+        def daterange(*args, **options)
+          args.each { |name| column(name, :daterange, options) }
         end
 
-        def tstzrange(name, options = {})
-          column(name, :tstzrange, options)
+        def hstore(*args, **options)
+          args.each { |name| column(name, :hstore, options) }
         end
 
-        def numrange(name, options = {})
-          column(name, :numrange, options)
+        def inet(*args, **options)
+          args.each { |name| column(name, :inet, options) }
         end
 
-        def daterange(name, options = {})
-          column(name, :daterange, options)
+        def int4range(*args, **options)
+          args.each { |name| column(name, :int4range, options) }
         end
 
-        def hstore(name, options = {})
-          column(name, :hstore, options)
+        def int8range(*args, **options)
+          args.each { |name| column(name, :int8range, options) }
         end
 
-        def ltree(name, options = {})
-          column(name, :ltree, options)
+        def json(*args, **options)
+          args.each { |name| column(name, :json, options) }
         end
 
-        def inet(name, options = {})
-          column(name, :inet, options)
+        def jsonb(*args, **options)
+          args.each { |name| column(name, :jsonb, options) }
         end
 
-        def cidr(name, options = {})
-          column(name, :cidr, options)
+        def ltree(*args, **options)
+          args.each { |name| column(name, :ltree, options) }
         end
 
-        def macaddr(name, options = {})
-          column(name, :macaddr, options)
+        def macaddr(*args, **options)
+          args.each { |name| column(name, :macaddr, options) }
         end
 
-        def uuid(name, options = {})
-          column(name, :uuid, options)
+        def money(*args, **options)
+          args.each { |name| column(name, :money, options) }
         end
 
-        def json(name, options = {})
-          column(name, :json, options)
+        def numrange(*args, **options)
+          args.each { |name| column(name, :numrange, options) }
         end
 
-        def jsonb(name, options = {})
-          column(name, :jsonb, options)
+        def point(*args, **options)
+          args.each { |name| column(name, :point, options) }
         end
 
-        def citext(name, options = {})
-          column(name, :citext, options)
+        def serial(*args, **options)
+          args.each { |name| column(name, :serial, options) }
         end
 
-        def point(name, options = {})
-          column(name, :point, options)
+        def tsrange(*args, **options)
+          args.each { |name| column(name, :tsrange, options) }
         end
 
-        def bit(name, options = {})
-          column(name, :bit, options)
+        def tstzrange(*args, **options)
+          args.each { |name| column(name, :tstzrange, options) }
         end
 
-        def bit_varying(name, options = {})
-          column(name, :bit_varying, options)
+        def tsvector(*args, **options)
+          args.each { |name| column(name, :tsvector, options) }
         end
 
-        def money(name, options = {})
-          column(name, :money, options)
+        def uuid(*args, **options)
+          args.each { |name| column(name, :uuid, options) }
+        end
+
+        def xml(*args, **options)
+          args.each { |name| column(name, :xml, options) }
         end
       end
 
@@ -96,39 +135,6 @@ module ActiveRecord
       class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
         include ColumnMethods
 
-        # Defines the primary key field.
-        # Use of the native PostgreSQL UUID type is supported, and can be used
-        # by defining your tables as such:
-        #
-        #   create_table :stuffs, id: :uuid do |t|
-        #     t.string :content
-        #     t.timestamps
-        #   end
-        #
-        # By default, this will use the +uuid_generate_v4()+ function from the
-        # +uuid-ossp+ extension, which MUST be enabled on your database. To enable
-        # the +uuid-ossp+ extension, you can use the +enable_extension+ method in your
-        # migrations. To use a UUID primary key without +uuid-ossp+ enabled, you can
-        # set the +:default+ option to +nil+:
-        #
-        #   create_table :stuffs, id: false do |t|
-        #     t.primary_key :id, :uuid, default: nil
-        #     t.uuid :foo_id
-        #     t.timestamps
-        #   end
-        #
-        # You may also pass a different UUID generation function from +uuid-ossp+
-        # or another library.
-        #
-        # Note that setting the UUID primary key default value to +nil+ will
-        # require you to assure that you always provide a UUID value before saving
-        # a record (as primary keys cannot be +nil+). This might be done via the
-        # +SecureRandom.uuid+ method and a +before_save+ callback, for instance.
-        def primary_key(name, type = :primary_key, options = {})
-          options[:default] = options.fetch(:default, 'uuid_generate_v4()') if type == :uuid
-          super
-        end
-
         def new_column_definition(name, type, options) # :nodoc:
           column = super
           column.array = options[:array]
diff --git a/activerecord/test/cases/migration/change_table_test.rb b/activerecord/test/cases/migration/change_table_test.rb
index 7010af5434..2ffe7a1b0d 100644
--- a/activerecord/test/cases/migration/change_table_test.rb
+++ b/activerecord/test/cases/migration/change_table_test.rb
@@ -13,7 +13,7 @@ module ActiveRecord
       end
 
       def with_change_table
-        yield ConnectionAdapters::Table.new(:delete_me, @connection)
+        yield ActiveRecord::Base.connection.update_table_definition(:delete_me, @connection)
       end
 
       def test_references_column_type_adds_id
@@ -100,6 +100,13 @@ module ActiveRecord
         end
       end
 
+      def test_primary_key_creates_primary_key_column
+        with_change_table do |t|
+          @connection.expect :add_column, nil, [:delete_me, :id, :primary_key, primary_key: true, first: true]
+          t.primary_key :id, first: true
+        end
+      end
+
       def test_integer_creates_integer_column
         with_change_table do |t|
           @connection.expect :add_column, nil, [:delete_me, :foo, :integer, {}]
@@ -108,6 +115,14 @@ module ActiveRecord
         end
       end
 
+      def test_bigint_creates_bigint_column
+        with_change_table do |t|
+          @connection.expect :add_column, nil, [:delete_me, :foo, :bigint, {}]
+          @connection.expect :add_column, nil, [:delete_me, :bar, :bigint, {}]
+          t.bigint :foo, :bar
+        end
+      end
+
       def test_string_creates_string_column
         with_change_table do |t|
           @connection.expect :add_column, nil, [:delete_me, :foo, :string, {}]
@@ -116,6 +131,24 @@ module ActiveRecord
         end
       end
 
+      if current_adapter?(:PostgreSQLAdapter)
+        def test_json_creates_json_column
+          with_change_table do |t|
+            @connection.expect :add_column, nil, [:delete_me, :foo, :json, {}]
+            @connection.expect :add_column, nil, [:delete_me, :bar, :json, {}]
+            t.json :foo, :bar
+          end
+        end
+
+        def test_xml_creates_xml_column
+          with_change_table do |t|
+            @connection.expect :add_column, nil, [:delete_me, :foo, :xml, {}]
+            @connection.expect :add_column, nil, [:delete_me, :bar, :xml, {}]
+            t.xml :foo, :bar
+          end
+        end
+      end
+
       def test_column_creates_column
         with_change_table do |t|
           @connection.expect :add_column, nil, [:delete_me, :bar, :integer, {}]