From 7a48deb42f00c4e4caa07f636bb968ea2bee5eaa Mon Sep 17 00:00:00 2001 From: "Javier G. Montoya S" Date: Thu, 19 Sep 2024 12:41:13 -0300 Subject: [PATCH] fix(database_task): check schema_migration table check to connection pool --- Changelog.md | 2 + README.md | 1 + lib/data_migrate/database_tasks.rb | 52 +++++++-- lib/data_migrate/test.rb | 14 --- spec/data_migrate/database_tasks_spec.rb | 137 ++++++++++++++++++++--- 5 files changed, 167 insertions(+), 39 deletions(-) delete mode 100644 lib/data_migrate/test.rb diff --git a/Changelog.md b/Changelog.md index c84b563..07b4ed8 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,7 @@ # Changelog +- Fix db:prepare:with_data task on [Rails 7.2](https://github.com/ilyakatz/data-migrate/pull/339) + # 11.0.0 - [Update rexml to 3.3.6](https://github.com/ilyakatz/data-migrate/pull/329) diff --git a/README.md b/README.md index 19f2280..e30fb66 100644 --- a/README.md +++ b/README.md @@ -86,6 +86,7 @@ You can generate a data migration as you would a schema migration: rake db:migrate:status:with_data # Display status of data and schema migrations rake db:migrate:up:with_data # Runs the "up" for a given migration VERSION rake db:migrate:with_data # Migrate the database data and schema (options: VERSION=x, VERBOSE=false) + rake db:prepare:with_data # Runs setup if database does not exist, or runs data and schema migrations if it does rake db:rollback:with_data # Rolls the schema back to the previous version (specify steps w/ STEP=n) rake db:schema:load:with_data # Load both schema.rb and data_schema.rb file into the database rake db:structure:load:with_data # Load both structure.sql and data_schema.rb file into the database diff --git a/lib/data_migrate/database_tasks.rb b/lib/data_migrate/database_tasks.rb index 90f9a27..63d0b7b 100644 --- a/lib/data_migrate/database_tasks.rb +++ b/lib/data_migrate/database_tasks.rb @@ -214,19 +214,13 @@ def self.prepare_all_with_data seed = false each_current_configuration(env) do |db_config| - next unless db_config.primary? + next unless primary?(db_config) - with_temporary_pool(db_config) do - begin - database_initialized = migration_connection.schema_migration.table_exists? - rescue ActiveRecord::NoDatabaseError + with_temporary_pool(db_config) do |pool| + unless database_exists?(pool.connection) create(db_config) - retry - end - - unless database_initialized if File.exist?(schema_dump_path(db_config)) - load_schema(db_config, ActiveRecord.schema_format, nil) + load_schema(db_config, schema_format, nil) load_schema_current( :ruby, ENV["DATA_SCHEMA"] @@ -237,7 +231,7 @@ def self.prepare_all_with_data end migrate_with_data - if ActiveRecord.dump_schema_after_migration + if dump_schema_after_migration? dump_schema(db_config) DataMigrate::Tasks::DataMigrateTasks.dump end @@ -246,5 +240,41 @@ def self.prepare_all_with_data load_seed if seed end + + private + + def database_exists?(connection) + if connection.respond_to?(:database_exists?) # Rails 7.1+ + connection.database_exists? + else + connection.table_exists?(ActiveRecord::SchemaMigration.table_name) + end + rescue ActiveRecord::NoDatabaseError + false + end + + def primary?(db_config) + if db_config.respond_to?(:primary?) # Rails 7.0+ + db_config.primary? + else + db_config.name == "primary" + end + end + + def dump_schema_after_migration? + if ActiveRecord.respond_to?(:dump_schema_after_migration) + ActiveRecord.dump_schema_after_migration + else + ActiveRecord::Base.dump_schema_after_migration + end + end + + def schema_format + if ActiveRecord.respond_to?(:schema_format) + ActiveRecord.schema_format + else + ActiveRecord::Base.schema_format + end + end end end diff --git a/lib/data_migrate/test.rb b/lib/data_migrate/test.rb deleted file mode 100644 index e0e8010..0000000 --- a/lib/data_migrate/test.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Base - extend self - - def foo - puts "Base#foo called" - end -end - -module Child - extend Base - extend self - - puts "foo: #{respond_to?(:foo)}" -end diff --git a/spec/data_migrate/database_tasks_spec.rb b/spec/data_migrate/database_tasks_spec.rb index 32d955a..9361659 100644 --- a/spec/data_migrate/database_tasks_spec.rb +++ b/spec/data_migrate/database_tasks_spec.rb @@ -5,9 +5,7 @@ describe DataMigrate::DatabaseTasks do let(:subject) { DataMigrate::DatabaseTasks } let(:migration_path) { "spec/db/migrate" } - let(:data_migrations_path) { - DataMigrate.config.data_migrations_path - } + let(:data_migrations_path) { DataMigrate.config.data_migrations_path } before do # In a normal Rails installation, db_dir would defer to @@ -18,11 +16,13 @@ end before do - allow(DataMigrate::Tasks::DataMigrateTasks).to receive(:migrations_paths) { + allow(DataMigrate::Tasks::DataMigrateTasks).to receive(:migrations_paths) do data_migrations_path - } - ActiveRecord::Base.establish_connection({ adapter: "sqlite3", database: "spec/db/test.db" }) - hash_config = ActiveRecord::DatabaseConfigurations::HashConfig.new('test', 'test', { adapter: "sqlite3", database: "spec/db/test.db" }) + end + ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: "spec/db/test.db") + hash_config = ActiveRecord::DatabaseConfigurations::HashConfig.new( + 'test', 'test', adapter: "sqlite3", database: "spec/db/test.db" + ) config_obj = ActiveRecord::DatabaseConfigurations.new([hash_config]) allow(ActiveRecord::Base).to receive(:configurations).and_return(config_obj) end @@ -36,12 +36,10 @@ before do DataMigrate::RailsHelper.schema_migration.create_table - allow(DataMigrate::SchemaMigration).to receive(:migrations_paths) { - migration_path - } - allow(DataMigrate::DatabaseTasks).to receive(:data_migrations_path) { + allow(DataMigrate::SchemaMigration).to receive(:migrations_paths) { migration_path } + allow(DataMigrate::DatabaseTasks).to receive(:data_migrations_path) do data_migrations_path - }.at_least(:once) + end.at_least(:once) end describe :past_migrations do @@ -79,11 +77,14 @@ if DataMigrate::RailsHelper.rails_version_equal_to_or_higher_than_7_0 describe :schema_dump_path do before do - allow(ActiveRecord::Base).to receive(:configurations).and_return(ActiveRecord::DatabaseConfigurations.new([db_config])) + allow(ActiveRecord::Base).to receive(:configurations) + .and_return(ActiveRecord::DatabaseConfigurations.new([db_config])) end context "for primary database" do - let(:db_config) { ActiveRecord::DatabaseConfigurations::HashConfig.new("development", "primary", {} ) } + let(:db_config) do + ActiveRecord::DatabaseConfigurations::HashConfig.new("development", "primary", {}) + end context "for :ruby db format" do it 'returns the data schema path' do @@ -101,5 +102,113 @@ end end end + + describe :prepare_all_with_data do + let(:db_config) do + ActiveRecord::DatabaseConfigurations::HashConfig.new( + 'test', + 'primary', + adapter: "sqlite3", + database: "spec/db/test.db" + ) + end + + let(:pool) { double("ConnectionPool") } + let(:connection) { double("Connection") } + + before do + allow(subject).to receive(:each_current_configuration).and_yield(db_config) + allow(subject).to receive(:with_temporary_pool).with(db_config).and_yield(pool) + allow(pool).to receive(:connection).and_return(connection) + allow(subject).to receive(:schema_dump_path).and_return("db/data_schema.rb") + allow(File).to receive(:exist?).and_return(true) + allow(subject).to receive(:load_schema) + allow(subject).to receive(:load_schema_current) + allow(subject).to receive(:migrate_with_data) + allow(subject).to receive(:dump_schema) + allow(DataMigrate::Tasks::DataMigrateTasks).to receive(:dump) + allow(subject).to receive(:load_seed) + + configurations = ActiveRecord::DatabaseConfigurations.new([db_config]) + allow(ActiveRecord::Base).to receive(:configurations).and_return(configurations) + end + + context "when the database does not exist" do + before do + allow(subject).to receive(:database_exists?).with(connection).and_return(false) + allow_any_instance_of(ActiveRecord::Tasks::DatabaseTasks).to receive(:create) + .and_return(true) + end + + it "creates the database" do + expect_any_instance_of(ActiveRecord::Tasks::DatabaseTasks).to receive(:create) + .with(db_config) + subject.prepare_all_with_data + end + + it "loads the schema" do + expect(subject).to receive(:load_schema).with( + db_config, + subject.send(:schema_format), + nil + ) + subject.prepare_all_with_data + end + + it "loads the current data schema" do + expect(subject).to receive(:load_schema_current).with(:ruby, ENV["DATA_SCHEMA"]) + subject.prepare_all_with_data + end + + it "runs migrations with data" do + expect(subject).to receive(:migrate_with_data) + subject.prepare_all_with_data + end + + it "dumps the schema after migration" do + expect(subject).to receive(:dump_schema).with(db_config) + expect(DataMigrate::Tasks::DataMigrateTasks).to receive(:dump) + subject.prepare_all_with_data + end + + it "loads seed data" do + expect(subject).to receive(:load_seed) + subject.prepare_all_with_data + end + end + + context "when the database exists" do + before do + allow(subject).to receive(:database_exists?).with(connection).and_return(true) + end + + it "does not create the database" do + expect(ActiveRecord::Tasks::DatabaseTasks).not_to receive(:create) + subject.prepare_all_with_data + end + + it "does not load the schema" do + expect(subject).not_to receive(:load_schema) + expect(subject).not_to receive(:load_schema_current) + subject.prepare_all_with_data + end + + it "runs migrations with data" do + expect(subject).to receive(:migrate_with_data) + subject.prepare_all_with_data + end + + it "dumps the schema after migration" do + expect(subject).to receive(:dump_schema).with(db_config) + expect(DataMigrate::Tasks::DataMigrateTasks).to receive(:dump) + subject.prepare_all_with_data + end + + it "does not load seed data" do + expect(subject).not_to receive(:load_seed) + subject.prepare_all_with_data + end + end + end end end