Merge pull request #149 from Invoca/TECH-7766_fix_habtm_primary_key_index_generation_bug

TECH-7766: [SPIKE] get change working for deploybot branch; go back a…

Author: yinonrousso

CHANGELOG.md

@@ -4,6 +4,11 @@ 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).
 
+## [1.1.0] - Unreleased
+### Changed
+- Fixed a bug where `DeclareSchema::Model::HabtmModelShim` `indexes` and `integer limits` were not being generated properly. Use `limit 8` for ids and primary composite key for habtm model.
+- `index_definitions_with_primary_key`
+
 ## [1.0.2] - 2022-07-18
 ### Fixed
 - Fixed a bug where `SchemaChange::ColumnRemove` was not loaded and causing an exception to raise
@@ -224,6 +229,7 @@ using the appropriate Rails configuration attributes.
 ### Added
 - Initial version from https://github.com/Invoca/hobo_fields v4.1.0.
 
+[1.1.0]: https://github.com/Invoca/declare_schema/compare/v1.0.2...v1.1.0
 [1.0.2]: https://github.com/Invoca/declare_schema/compare/v1.0.1...v1.0.2
 [1.0.1]: https://github.com/Invoca/declare_schema/compare/v1.0.0...v1.0.1
 [1.0.0]: https://github.com/Invoca/declare_schema/compare/v0.14.3...v1.0.0

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    declare_schema (1.0.2)
+    declare_schema (1.1.0)
       rails (>= 5.0)
 
 GEM

lib/declare_schema/model/foreign_key_definition.rb

@@ -36,7 +36,7 @@ def for_model(model, old_table_name)
               parent_table:    parent_table,
               foreign_key:     foreign_key
             }
-            options[:dependent] = :delete if fkc['ON DELETE CASCADE']
+            options[:dependent] = :delete if fkc['ON DELETE CASCADE'] || model.is_a?(DeclareSchema::Model::HabtmModelShim)
 
             new(model, foreign_key, **options)
           end

lib/declare_schema/model/habtm_model_shim.rb

@@ -37,9 +37,13 @@ def table_name
         join_table
       end
 
+      def index_name
+        "index_#{table_name}_on_#{foreign_keys.first}_#{foreign_keys.last}"
+      end
+
       def field_specs
         foreign_keys.each_with_index.each_with_object({}) do |(v, position), result|
-          result[v] = ::DeclareSchema::Model::FieldSpec.new(self, v, :integer, position: position, null: false)
+          result[v] = ::DeclareSchema::Model::FieldSpec.new(self, v, :bigint, position: position, null: false)
         end
       end
 
@@ -53,7 +57,7 @@ def _declared_primary_key
 
       def index_definitions_with_primary_key
         [
-          IndexDefinition.new(self, foreign_keys, unique: true, name: ::DeclareSchema::Model::IndexDefinition::PRIMARY_KEY_NAME),
+          IndexDefinition.new(self, foreign_keys, unique: true, name: index_name), # creates a primary composite key on both foriegn keys
           IndexDefinition.new(self, foreign_keys.last) # not unique by itself; combines with primary key to be unique
         ]
       end

lib/declare_schema/model/index_definition.rb

@@ -33,11 +33,14 @@ def initialize(model, fields, **options)
 
       class << self
         # extract IndexSpecs from an existing table
-        # always includes the PRIMARY KEY index
+        # always includes the PRIMARY KEY index for Model unless it is a HABTM Model.
         def for_model(model, old_table_name = nil)
           t = old_table_name || model.table_name
 
-          primary_key_columns = Array(model.connection.primary_key(t)).presence or
+          habtm_model = model.is_a?(DeclareSchema::Model::HabtmModelShim)
+
+          primary_key_columns = Array(model.connection.primary_key(t)).presence
+          primary_key_columns || habtm_model or
             raise "could not find primary key for table #{t} in #{model.connection.columns(t).inspect}"
 
           primary_key_found = false
@@ -47,14 +50,14 @@ def for_model(model, old_table_name = nil)
               i.columns == primary_key_columns && i.unique or
                 raise "primary key on #{t} was not unique on #{primary_key_columns} (was unique=#{i.unique} on #{i.columns})"
               primary_key_found = true
-            elsif i.columns == primary_key_columns && i.unique
+            elsif i.columns == primary_key_columns && i.unique && !habtm_model
               # skip this primary key index since we'll create it below, with PRIMARY_KEY_NAME
               next
             end
             new(model, i.columns, name: i.name, unique: i.unique, where: i.where, table_name: old_table_name)
           end.compact
 
-          if !primary_key_found
+          if !primary_key_found && !habtm_model
             index_definitions << new(model, primary_key_columns, name: PRIMARY_KEY_NAME, unique: true, where: nil, table_name: old_table_name)
           end
           index_definitions

lib/declare_schema/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module DeclareSchema
-  VERSION = "1.0.2"
+  VERSION = "1.1.0"
 end

spec/lib/declare_schema/model/habtm_model_shim_spec.rb

@@ -102,14 +102,9 @@ class Parent2 < ActiveRecord::Base
         expect(result.size).to eq(2), result.inspect
 
         expect(result.first).to be_a(::DeclareSchema::Model::IndexDefinition)
-        expect(result.first.name).to eq('PRIMARY')
+        expect(result.first.name).to eq('index_parent_1_parent_2_on_parent_1_id_parent_2_id')
         expect(result.first.fields).to eq(['parent_1_id', 'parent_2_id'])
         expect(result.first.unique).to be_truthy
-
-        expect(result.last).to be_a(::DeclareSchema::Model::IndexDefinition)
-        expect(result.last.name).to eq('on_parent_2_id')
-        expect(result.last.unique).to be_falsey
-        expect(result.last.fields).to eq(['parent_2_id'])
       end
     end