From 753633abdfc72743419ba86ef4981679098365e5 Mon Sep 17 00:00:00 2001 From: fatkodima Date: Thu, 3 Jun 2021 20:30:35 +0300 Subject: [PATCH] Respect Active Record's primary_key_type in Active Storage migrations --- activestorage/CHANGELOG.md | 4 ++ ...0806125915_create_active_storage_tables.rb | 24 +++++-- ...1_create_active_storage_variant_records.rb | 17 ++++- activestorage/test/migrations_test.rb | 67 +++++++++++++++++++ 4 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 activestorage/test/migrations_test.rb diff --git a/activestorage/CHANGELOG.md b/activestorage/CHANGELOG.md index a8b7ac10b9..60aed8b15d 100644 --- a/activestorage/CHANGELOG.md +++ b/activestorage/CHANGELOG.md @@ -9,6 +9,10 @@ *Breno Gazzola* +* Respect Active Record's primary_key_type in Active Storage migrations + + *fatkodima* + * Allow `expires_in` for ActiveStorage signed ids. *aki77* diff --git a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb index 6dd94c3874..abebb74f73 100644 --- a/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb +++ b/activestorage/db/migrate/20170806125915_create_active_storage_tables.rb @@ -1,6 +1,9 @@ class CreateActiveStorageTables < ActiveRecord::Migration[5.2] def change - create_table :active_storage_blobs do |t| + # Use Active Record's configured type for primary and foreign keys + primary_key_type, foreign_key_type = primary_and_foreign_key_types + + create_table :active_storage_blobs, id: primary_key_type do |t| t.string :key, null: false t.string :filename, null: false t.string :content_type @@ -18,10 +21,10 @@ def change t.index [ :key ], unique: true end - create_table :active_storage_attachments do |t| + create_table :active_storage_attachments, id: primary_key_type do |t| t.string :name, null: false - t.references :record, null: false, polymorphic: true, index: false - t.references :blob, null: false + t.references :record, null: false, polymorphic: true, index: false, type: foreign_key_type + t.references :blob, null: false, type: foreign_key_type if connection.supports_datetime_with_precision? t.datetime :created_at, precision: 6, null: false @@ -33,12 +36,21 @@ def change t.foreign_key :active_storage_blobs, column: :blob_id end - create_table :active_storage_variant_records do |t| - t.belongs_to :blob, null: false, index: false + create_table :active_storage_variant_records, id: primary_key_type do |t| + t.belongs_to :blob, null: false, index: false, type: foreign_key_type t.string :variation_digest, null: false t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true t.foreign_key :active_storage_blobs, column: :blob_id end end + + private + def primary_and_foreign_key_types + config = Rails.configuration.generators + setting = config.options[config.orm][:primary_key_type] + primary_key_type = setting || :primary_key + foreign_key_type = setting || :bigint + [primary_key_type, foreign_key_type] + end end diff --git a/activestorage/db/update_migrate/20191206030411_create_active_storage_variant_records.rb b/activestorage/db/update_migrate/20191206030411_create_active_storage_variant_records.rb index 80862f66f2..804ae76f6f 100644 --- a/activestorage/db/update_migrate/20191206030411_create_active_storage_variant_records.rb +++ b/activestorage/db/update_migrate/20191206030411_create_active_storage_variant_records.rb @@ -1,11 +1,24 @@ class CreateActiveStorageVariantRecords < ActiveRecord::Migration[6.0] def change - create_table :active_storage_variant_records do |t| - t.belongs_to :blob, null: false, index: false + # Use Active Record's configured type for primary key + create_table :active_storage_variant_records, id: primary_key_type do |t| + t.belongs_to :blob, null: false, index: false, type: blobs_primary_key_type t.string :variation_digest, null: false t.index %i[ blob_id variation_digest ], name: "index_active_storage_variant_records_uniqueness", unique: true t.foreign_key :active_storage_blobs, column: :blob_id end end + + private + def primary_key_type + config = Rails.configuration.generators + config.options[config.orm][:primary_key_type] || :primary_key + end + + def blobs_primary_key_type + pkey_name = connection.primary_key(:active_storage_blobs) + pkey_column = connection.columns(:active_storage_blobs).find { |c| c.name == pkey_name } + pkey_column.bigint? ? :bigint : pkey_column.type + end end diff --git a/activestorage/test/migrations_test.rb b/activestorage/test/migrations_test.rb new file mode 100644 index 0000000000..fbc801ad44 --- /dev/null +++ b/activestorage/test/migrations_test.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require "test_helper" +require "database/setup" + +class ActiveStorage::MigrationsTest < ActiveSupport::TestCase + setup do + @original_verbose = ActiveRecord::Migration.verbose + ActiveRecord::Migration.verbose = false + + @connection = ActiveRecord::Base.connection + @original_options = Rails.configuration.generators.options.deep_dup + end + + teardown do + Rails.configuration.generators.options = @original_options + rerun_migration + ActiveRecord::Migration.verbose = @original_verbose + end + + test "migration creates tables with default primary and foreign key types" do + rerun_migration + + active_storage_tables.each do |table| + assert_equal :integer, primary_key(table).type + + foreign_keys(table).each do |foreign_key| + assert_equal :integer, foreign_key.type + end + end + end + + test "migration creates tables with configured primary and foreign key types" do + Rails.configuration.generators do |g| + g.orm :active_record, primary_key_type: :string + end + + rerun_migration + + active_storage_tables.each do |table| + assert_equal :string, primary_key(table).type + + foreign_keys(table).each do |foreign_key| + assert_equal :string, foreign_key.type + end + end + end + + private + def rerun_migration + CreateActiveStorageTables.migrate(:down) + CreateActiveStorageTables.migrate(:up) + end + + def active_storage_tables + [:active_storage_blobs, :active_storage_attachments, :active_storage_variant_records] + end + + def primary_key(table) + @connection.columns(table).find { |c| c.name == "id" } + end + + def foreign_keys(table) + columns = @connection.foreign_keys(table).map(&:column) + @connection.columns(table).select { |c| columns.include?(c.name) } + end +end