Merge pull request #35 from Invoca/TECH-4971_declareschema_treat_integer_limit_8_and_bigint_types_identically

Tech 4971 declareschema treat integer limit 8 and bigint types identically

Author: yinonrousso

CHANGELOG.md

@@ -4,6 +4,10 @@ Inspired by [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 Note: this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.6.0] - Unreleased
+### Added
+- Fields may now be declared with `:bigint` type which is identical to `:integer, limit 8`
+
 ## [0.5.0] - 2020-12-21
 ### Added
 - Added support for configuring the character set and collation for MySQL databases
@@ -73,6 +77,7 @@ using the appropriate Rails configuration attributes.
 ### Added
 - Initial version from https://github.com/Invoca/hobo_fields v4.1.0.
 
+[0.6.0]: https://github.com/Invoca/declare_schema/compare/v0.5.0...v0.6.0
 [0.5.0]: https://github.com/Invoca/declare_schema/compare/v0.4.2...v0.5.0
 [0.4.2]: https://github.com/Invoca/declare_schema/compare/v0.4.1...v0.4.2
 [0.4.1]: https://github.com/Invoca/declare_schema/compare/v0.4.0...v0.4.1

lib/declare_schema/model.rb

@@ -88,7 +88,7 @@ def declare_field(name, type, *args)
         add_formatting_for_field(name, type)
         add_validations_for_field(name, type, args, options)
         add_index_for_field(name, args, options)
-        field_specs[name] = ::DeclareSchema::Model::FieldSpec.new(self, name, type, options)
+        field_specs[name] = ::DeclareSchema::Model::FieldSpec.new(self, name, type, position: field_specs.size, **options)
         attr_order << name unless attr_order.include?(name)
       end
 

lib/declare_schema/model/field_spec.rb

@@ -33,7 +33,8 @@ def round_up_mysql_text_limit(limit)
 
       attr_reader :model, :name, :type, :position, :options
 
-      def initialize(model, name, type, options = {})
+      def initialize(model, name, type, position: 0, **options)
+        # TODO: TECH-5116
         # Invoca change - searching for the primary key was causing an additional database read on every model load.  Assume
         # "id" which works for invoca.
         # raise ArgumentError, "you cannot provide a field spec for the primary key" if name == model.primary_key
@@ -43,9 +44,8 @@ def initialize(model, name, type, options = {})
         @name = name.to_sym
         type.is_a?(Symbol) or raise ArgumentError, "type must be a Symbol; got #{type.inspect}"
         @type = type
-        position_option = options.delete(:position)
+        @position = position
         @options = options
-
         case type
         when :text
           @options[:default] and raise "default may not be given for :text field #{model}##{@name}"
@@ -54,11 +54,15 @@ def initialize(model, name, type, options = {})
           end
         when :string
           @options[:limit] or raise "limit must be given for :string field #{model}##{@name}: #{@options.inspect}; do you want `limit: 255`?"
-        else
+        when :bigint
+          @type = :integer
+          @options = options.merge(limit: 8)
+        end
+
+        unless type.in?([:text, :string])
           @options[:collation] and raise "collation may only given for :string and :text fields"
           @options[:charset]   and raise "charset may only given for :string and :text fields"
         end
-        @position = position_option || model.field_specs.length
       end
 
       TYPE_SYNONYMS = { timestamp: :datetime }.freeze

lib/generators/declare_schema/migration/migrator.rb

@@ -37,11 +37,9 @@ def table_exists?
 
         def field_specs
           i = 0
-          foreign_keys.reduce({}) do |h, v|
-            # some trickery to avoid an infinite loop when FieldSpec#initialize tries to call model.field_specs
-            h[v] = ::DeclareSchema::Model::FieldSpec.new(self, v, :integer, position: i, null: false)
+          foreign_keys.each_with_object({}) do |v, result|
+            result[v] = ::DeclareSchema::Model::FieldSpec.new(self, v, :integer, position: i, null: false)
             i += 1
-            h
           end
         end
 

spec/lib/declare_schema/field_spec_spec.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+RSpec.describe 'DeclareSchema Model FieldSpec' do
+  before do
+    load File.expand_path('prepare_testapp.rb', __dir__)
+  end
+
+  context 'There are no model columns to change' do
+    it '#different_to should return false for int8 == int8' do
+      subject = DeclareSchema::Model::FieldSpec.new(Object, :price, :integer, limit: 8, null: false, position: 0)
+
+      case Rails::VERSION::MAJOR
+      when 4
+        cast_type = ActiveRecord::Type::Integer.new(limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, cast_type, "integer(8)", false)
+      else
+        sql_type_metadata = ActiveRecord::ConnectionAdapters::SqlTypeMetadata.new(sql_type: "integer(8)", type: :integer, limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, sql_type_metadata, false, "adverts")
+      end
+
+      expect(subject.different_to?(subject.name, col)).to eq(false)
+    end
+
+    it '#different_to should return false for bigint == bigint' do
+      subject = DeclareSchema::Model::FieldSpec.new(Object, :price, :bigint, null: false, position: 0)
+
+      case Rails::VERSION::MAJOR
+      when 4
+        cast_type = ActiveRecord::Type::BigInteger.new(limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, cast_type, "bigint(20)", false)
+      else
+        sql_type_metadata = ActiveRecord::ConnectionAdapters::SqlTypeMetadata.new(sql_type: "bigint(20)", type: :integer, limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, sql_type_metadata, false, "adverts")
+      end
+
+      expect(subject.different_to?(subject.name, col)).to eq(false)
+    end
+
+    it '#different_to should return false for int8 == bigint' do
+      subject = DeclareSchema::Model::FieldSpec.new(Object, :price, :integer, limit: 8, null: false, position: 0)
+
+      case Rails::VERSION::MAJOR
+      when 4
+        cast_type = ActiveRecord::Type::BigInteger.new(limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, cast_type, "bigint(20)", false)
+      else
+        sql_type_metadata = ActiveRecord::ConnectionAdapters::SqlTypeMetadata.new(sql_type: "bigint(20)", type: :integer, limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, sql_type_metadata, false, "adverts")
+      end
+
+      expect(subject.different_to?(subject.name, col)).to eq(false)
+    end
+
+    it '#different_to should return false for bigint == int8' do
+      subject = DeclareSchema::Model::FieldSpec.new(Object, :price, :bigint, null: false, position: 0)
+
+      case Rails::VERSION::MAJOR
+      when 4
+        cast_type = ActiveRecord::Type::Integer.new(limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, cast_type, "integer(8)", false)
+      else
+        sql_type_metadata = ActiveRecord::ConnectionAdapters::SqlTypeMetadata.new(sql_type: "integer(8)", type: :integer, limit: 8)
+        col = ActiveRecord::ConnectionAdapters::Column.new("price", nil, sql_type_metadata, false, "adverts")
+      end
+
+      expect(subject.different_to?(subject.name, col)).to eq(false)
+    end
+  end
+end

spec/lib/declare_schema/migration_generator_spec.rb

@@ -1093,4 +1093,29 @@ class Advert < active_record_base_class.constantize
       expect(base_class).to eq("(Rails::VERSION::MAJOR >= 5 ? ActiveRecord::Migration[4.2] : ActiveRecord::Migration)")
     end
   end
+
+  context 'Does not generate migrations' do
+    it 'for aliased fields bigint -> integer limit 8' do
+      if Rails::VERSION::MAJOR >= 5 || !ActiveRecord::Base.connection.class.name.match?(/SQLite3Adapter/)
+        class Advert < active_record_base_class.constantize
+          fields do
+            price :bigint
+          end
+        end
+
+        generate_migrations '-n', '-m'
+
+        migrations = Dir.glob('db/migrate/*declare_schema_migration*.rb')
+        expect(migrations.size).to eq(1), migrations.inspect
+
+        class Advert < active_record_base_class.constantize
+          fields do
+            price :integer, limit: 8
+          end
+        end
+
+        expect { generate_migrations '-n', '-g' }.to output("Database and models match -- nothing to change\n").to_stdout
+      end
+    end
+  end
 end