From 8061d50391d705ffc12a8713393014baea4746b8 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 15:15:19 -0500 Subject: [PATCH 01/15] fix(percona_adapter): add methods required by Rails 7.1 `internal_exec_query`: https://github.com/rails/rails/pull/48188/files `raw_execute`: https://github.com/rails/rails/commit/63c0d6b31bcd0fc33745ec6fd278b2d1aab9be54 Implementations for these are just copied from the MySQL2 adapter. For `reconnect`, only a stub is present because retries make less sense when offloading to PTOSC --- .../connection_adapters/percona_adapter.rb | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index 07fd6ec3..934babee 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -86,19 +86,20 @@ def write_query?(sql) # :nodoc: def exec_delete(sql, name, binds) execute(to_sql(sql, binds), name) - @connection.affected_rows + mysql_adapter.raw_connection.affected_rows end alias exec_update exec_delete - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) # rubocop:disable Lint/UnusedMethodArgument, Metrics/LineLength + def exec_insert(sql, name, binds, pk = nil, sequence_name = nil, returning: nil) # rubocop:disable Lint/UnusedMethodArgument, Metrics/LineLength execute(to_sql(sql, binds), name) end - def exec_query(sql, name = 'SQL', _binds = [], **_kwargs) + def internal_exec_query(sql, name = 'SQL', _binds = [], **_kwargs) # :nodoc: result = execute(sql, name) fields = result.fields if defined?(result.fields) ActiveRecord::Result.new(fields, result.to_a) end + alias :exec_query :internal_exec_query # Executes a SELECT query and returns an array of rows. Each row is an # array of field values. @@ -196,6 +197,21 @@ def last_inserted_id(result) private attr_reader :mysql_adapter + + def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true) + log(sql, name, async: async) do + with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn| + sync_timezone_changes(conn) + result = conn.query(sql) + verified! + handle_warnings(sql) + result + end + end + end + + def reconnect + end end end end From fd8e1e6e336379032059433f1c0f6640b7af0128 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 15:16:56 -0500 Subject: [PATCH 02/15] fix(tests): fix deprecation warning for MigrationContext https://github.com/rails/rails/blob/f481353150c100dbd944d5aa84f2480264cd723c/activerecord/lib/active_record/migration.rb#L1206 --- spec/spec_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 984c8206..ac4250e4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -79,3 +79,7 @@ def initialize(migrations_paths, schema_migration = nil) ActiveRecord::Migrator.send :prepend, Rails5Compatibility::Migrator ActiveRecord::MigrationContext.send :prepend, Rails5Compatibility::MigrationContext end + +if ActiveRecord::VERSION::STRING >= '7.1' + ActiveRecord::MigrationContext.send :prepend, Rails5Compatibility::MigrationContext +end From 2eb4db1c4e4c4dd9c0ba0a61b8da3485f4d83b83 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 16:18:48 -0500 Subject: [PATCH 03/15] tests(integration): use MigrationContext everywhere The signature of Migrator has changed again, and does not seem friendly to external usage. Standardization on MigrationContext fixes the missing method issue. --- spec/integration/change_table_spec.rb | 11 ++- spec/integration/columns_spec.rb | 69 +++++++----------- spec/integration/data_migrations_spec.rb | 65 +++++------------ spec/integration/indexes_spec.rb | 91 +++++++----------------- spec/integration/tables_spec.rb | 26 ++++--- spec/integration_spec.rb | 20 +++--- 6 files changed, 99 insertions(+), 183 deletions(-) diff --git a/spec/integration/change_table_spec.rb b/spec/integration/change_table_spec.rb index 9752195a..d5645904 100644 --- a/spec/integration/change_table_spec.rb +++ b/spec/integration/change_table_spec.rb @@ -3,11 +3,10 @@ describe Departure, integration: true do class Comment < ActiveRecord::Base; end - let(:migration_fixtures) do - ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration).migrations.select do |m| - m.version == version - end + let(:migration_context) do + ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration) end + let(:direction) { :up } context 'change_table' do @@ -19,7 +18,7 @@ def column_metadata(table, name) context 'creating column' do before(:each) do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, version).migrate + migration_context.run(direction, version) end it 'adds the column in the DB table' do @@ -38,7 +37,7 @@ def column_metadata(table, name) end it 'marks the migration as up' do - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end it 'changes column' do diff --git a/spec/integration/columns_spec.rb b/spec/integration/columns_spec.rb index a86e0136..d0dc891d 100644 --- a/spec/integration/columns_spec.rb +++ b/spec/integration/columns_spec.rb @@ -3,10 +3,9 @@ describe Departure, integration: true do class Comment < ActiveRecord::Base; end - let(:migration_fixtures) do - ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration).migrations + let(:migration_context) do + ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration) end - let(:migration_paths) { [MIGRATION_FIXTURES] } let(:direction) { :up } @@ -17,25 +16,15 @@ class Comment < ActiveRecord::Base; end let(:direction) { :up } it 'adds the column in the DB table' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - ).migrate + migration_context.run(direction, version) expect(:comments).to have_column('some_id_field') end it 'marks the migration as up' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - ).migrate - - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + + expect(migration_context.current_version).to eq(version) end end @@ -43,29 +32,19 @@ class Comment < ActiveRecord::Base; end let(:direction) { :down } before do - ActiveRecord::Migrator.new(:up, migration_fixtures, ActiveRecord::SchemaMigration, version).migrate + migration_context.up(version) end it 'drops the column from the DB table' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - 1 - ).migrate + migration_context.run(direction, version) expect(:comments).not_to have_column('some_id_field') end it 'marks the migration as down' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - 1 - ).migrate - - expect(ActiveRecord::Migrator.current_version).to eq(version - 1) + migration_context.run(direction, version) + + expect(migration_context.current_version).to eq(version - 1) end end @@ -82,12 +61,12 @@ class Comment < ActiveRecord::Base; end end it 'changes the column name' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).to have_column('new_id_field') end it 'does not keep the old column' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).not_to have_column('some_id_field') end end @@ -100,20 +79,20 @@ class Comment < ActiveRecord::Base; end end before do - ActiveRecord::Migrator.new(:up, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.up(1) end context 'when null is true' do let(:version) { 14 } it 'sets the column to allow nulls' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(column.null).to be_truthy end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(version) end end @@ -121,13 +100,13 @@ class Comment < ActiveRecord::Base; end let(:version) { 15 } it 'sets the column not to allow nulls' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(column.null).to be_falsey end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(version) end end end @@ -136,12 +115,12 @@ class Comment < ActiveRecord::Base; end let(:version) { 22 } it 'adds a created_at column' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).to have_column('created_at') end it 'adds a updated_at column' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).to have_column('updated_at') end end @@ -158,12 +137,12 @@ class Comment < ActiveRecord::Base; end end it 'removes the created_at column' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).not_to have_column('created_at') end it 'removes the updated_at column' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).not_to have_column('updated_at') end end diff --git a/spec/integration/data_migrations_spec.rb b/spec/integration/data_migrations_spec.rb index 99d6dbda..9f99d7e0 100644 --- a/spec/integration/data_migrations_spec.rb +++ b/spec/integration/data_migrations_spec.rb @@ -3,7 +3,10 @@ describe Departure, integration: true do class Comment < ActiveRecord::Base; end - let(:migration_fixtures) { [MIGRATION_FIXTURES] } + let(:migration_context) do + ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration) + end + let(:direction) { :up } before do @@ -25,21 +28,15 @@ class Comment < ActiveRecord::Base; end let(:version) { 9 } it 'updates all the required data' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) expect(Comment.pluck(:read)).to match_array([true, true]) end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end end @@ -47,10 +44,7 @@ class Comment < ActiveRecord::Base; end let(:version) { 30 } it 'updates all the required data' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) expect(Comment.pluck(:author, :read)).to match_array([ [nil, false], @@ -61,12 +55,9 @@ class Comment < ActiveRecord::Base; end end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end end @@ -74,21 +65,15 @@ class Comment < ActiveRecord::Base; end let(:version) { 10 } it 'updates all the required data' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) expect(Comment.pluck(:read)).to match_array([true, true]) end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end end @@ -96,21 +81,15 @@ class Comment < ActiveRecord::Base; end let(:version) { 11 } it 'updates all the required data' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) expect(Comment.pluck(:read)).to match_array([true, true]) end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end end @@ -118,21 +97,15 @@ class Comment < ActiveRecord::Base; end let(:version) { 12 } it 'updates all the required data' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) expect(Comment.pluck(:read)).to match_array([true, true]) end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_fixtures, ActiveRecord::SchemaMigration).run( - direction, - version - ) + migration_context.run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + expect(migration_context.current_version).to eq(version) end end end diff --git a/spec/integration/indexes_spec.rb b/spec/integration/indexes_spec.rb index 785ac8be..3140b547 100644 --- a/spec/integration/indexes_spec.rb +++ b/spec/integration/indexes_spec.rb @@ -3,12 +3,10 @@ describe Departure, integration: true do class Comment < ActiveRecord::Base; end - let(:migration_fixtures) do - ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration).migrations + let(:migration_context) do + ActiveRecord::MigrationContext.new([MIGRATION_FIXTURES], ActiveRecord::SchemaMigration) end - let(:migration_paths) { [MIGRATION_FIXTURES] } - let(:direction) { :up } context 'managing indexes' do @@ -17,38 +15,22 @@ class Comment < ActiveRecord::Base; end context 'adding indexes' do let(:direction) { :up } - # TODO: Create it directly like this? before do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - 1 - ).migrate + migration_context.run(direction, 1) end it 'executes the percona command' do expect_percona_command('ADD INDEX `index_comments_on_some_id_field` (`some_id_field`)') - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - ).migrate + migration_context.run(direction, version) expect(:comments).to have_index('index_comments_on_some_id_field') end it 'marks the migration as up' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - ).migrate - - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + + expect(migration_context.current_version).to eq(version) end end @@ -56,43 +38,22 @@ class Comment < ActiveRecord::Base; end let(:direction) { :down } before do - ActiveRecord::Migrator.new( - :up, - migration_fixtures, - ActiveRecord::SchemaMigration, - 1 - ).migrate - - ActiveRecord::Migrator.new( - :up, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - ).migrate + migration_context.up(1) + migration_context.up(version) end it 'executes the percona command' do expect_percona_command('DROP INDEX `index_comments_on_some_id_field`') - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - 1 - ).migrate + migration_context.run(direction, version) expect(:comments).not_to have_index('index_comments_on_some_id_field') end it 'marks the migration as down' do - ActiveRecord::Migrator.new( - direction, - migration_fixtures, - ActiveRecord::SchemaMigration, - version - 1 - ).migrate - - expect(ActiveRecord::Migrator.current_version).to eq(1) + migration_context.run(direction, version) + + expect(migration_context.current_version).to eq(1) end end @@ -101,7 +62,7 @@ class Comment < ActiveRecord::Base; end let(:version) { 13 } before do - ActiveRecord::Migrator.new(:up, migration_fixtures, ActiveRecord::SchemaMigration, 2).migrate + migration_context.up(2) end it 'executes the percona command' do @@ -112,13 +73,13 @@ class Comment < ActiveRecord::Base; end expect_percona_command('DROP INDEX `index_comments_on_some_id_field`') end - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:comments).to have_index('new_index_comments_on_some_id_field') end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(version) end end end @@ -130,21 +91,21 @@ class Comment < ActiveRecord::Base; end let(:direction) { :up } before do - ActiveRecord::Migrator.new(:up, migration_fixtures, ActiveRecord::SchemaMigration,1).migrate + migration_context.migrate(1) end it 'executes the percona command' do expect_percona_command('ADD UNIQUE INDEX `index_comments_on_some_id_field` (`some_id_field`)') - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(unique_indexes_from(:comments)) .to match_array(['index_comments_on_some_id_field']) end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(version) end end @@ -152,22 +113,22 @@ class Comment < ActiveRecord::Base; end let(:direction) { :down } before do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(:up, 1) - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(:up, version) + migration_context.run(:up, 1) + migration_context.run(:up, version) end it 'executes the percona command' do expect_percona_command('DROP INDEX `index_comments_on_some_id_field`') - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(unique_indexes_from(:comments)) .not_to match_array(['index_comments_on_some_id_field']) end it 'marks the migration as down' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(1) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(1) end end end diff --git a/spec/integration/tables_spec.rb b/spec/integration/tables_spec.rb index 52ea2643..841a50aa 100644 --- a/spec/integration/tables_spec.rb +++ b/spec/integration/tables_spec.rb @@ -3,6 +3,10 @@ describe Departure, integration: true do class Comment < ActiveRecord::Base; end + let(:migration_context) do + ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration) + end + let(:migration_paths) { [MIGRATION_FIXTURES] } let(:direction) { :up } @@ -10,13 +14,13 @@ class Comment < ActiveRecord::Base; end let(:version) { 8 } it 'creates the table' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(tables).to include('things') end it 'marks the migration as up' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(version) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(version) end end @@ -25,13 +29,13 @@ class Comment < ActiveRecord::Base; end let(:direction) { :down } it 'drops the table' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(tables).not_to include('things') end it 'updates the schema_migrations' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) - expect(ActiveRecord::Migrator.current_version).to eq(0) + migration_context.run(direction, version) + expect(migration_context.current_version).to eq(0) end end @@ -39,22 +43,22 @@ class Comment < ActiveRecord::Base; end let(:version) { 24 } before do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, 1) - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, 2) + migration_context.run(direction, 1) + migration_context.run(direction, 2) end it 'changes the table name' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(tables).to include('new_comments') end it 'does not keep the old name' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(tables).not_to include('comments') end it 'changes the index names in the new table' do - ActiveRecord::MigrationContext.new(migration_paths, ActiveRecord::SchemaMigration).run(direction, version) + migration_context.run(direction, version) expect(:new_comments).to have_index('index_new_comments_on_some_id_field') end end diff --git a/spec/integration_spec.rb b/spec/integration_spec.rb index 655201c4..c7792986 100644 --- a/spec/integration_spec.rb +++ b/spec/integration_spec.rb @@ -34,7 +34,7 @@ class Comment < ActiveRecord::Base; end it "doesn't send the output to stdout" do expect do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end.to_not output.to_stdout end end @@ -49,7 +49,7 @@ class Comment < ActiveRecord::Base; end it 'sends the output to stdout' do expect do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end.to output.to_stdout end end @@ -59,7 +59,7 @@ class Comment < ActiveRecord::Base; end let(:db_config) { Configuration.new } it 'reconnects to the database using PerconaAdapter' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) expect(spec_config[:adapter]).to eq('percona') end @@ -75,7 +75,7 @@ class Comment < ActiveRecord::Base; end end it 'uses the provided username' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) expect(spec_config[:username]).to eq('root') end end @@ -91,7 +91,7 @@ class Comment < ActiveRecord::Base; end end it 'uses root' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) expect(spec_config[:username]).to eq('root') end end @@ -101,14 +101,14 @@ class Comment < ActiveRecord::Base; end xit 'patches it to use regular Rails migration methods' do expect(Departure::Lhm::Fake::Adapter) .to receive(:new).and_return(true) - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end end context 'when there is no LHM' do xit 'does not patch it' do expect(Departure::Lhm::Fake).not_to receive(:patching_lhm) - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end end end @@ -174,7 +174,7 @@ class Comment < ActiveRecord::Base; end .and_return(command) ClimateControl.modify PERCONA_ARGS: '--chunk-time=1' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end end end @@ -187,7 +187,7 @@ class Comment < ActiveRecord::Base; end .and_return(command) ClimateControl.modify PERCONA_ARGS: '--chunk-time=1 --max-lag=2' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end end end @@ -200,7 +200,7 @@ class Comment < ActiveRecord::Base; end .and_return(command) ClimateControl.modify PERCONA_ARGS: '--alter-foreign-keys-method=drop_swap' do - ActiveRecord::Migrator.new(direction, migration_fixtures, ActiveRecord::SchemaMigration, 1).migrate + migration_context.run(direction, 1) end end end From 8caefb6e064f860e950572419e74bca8dcbf40f7 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 16:33:24 -0500 Subject: [PATCH 04/15] tests(percona_adapter): fix impacted tests --- .../connection_adapters/percona_adapter_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/active_record/connection_adapters/percona_adapter_spec.rb b/spec/active_record/connection_adapters/percona_adapter_spec.rb index 14f9b4eb..c0ee6b8c 100644 --- a/spec/active_record/connection_adapters/percona_adapter_spec.rb +++ b/spec/active_record/connection_adapters/percona_adapter_spec.rb @@ -181,12 +181,13 @@ describe '#exec_delete' do let(:sql) { 'DELETE FROM comments WHERE id = 1' } + let(:affected_rows) { 1 } let(:name) { nil } let(:binds) { nil } before do - allow(runner).to receive(:query).with(sql) - allow(runner).to receive(:affected_rows).and_return(1) + allow(runner).to receive(:query).with(anything) + allow(mysql_client).to receive(:affected_rows).and_return(affected_rows) end it 'executes the sql' do @@ -195,7 +196,7 @@ end it 'returns the number of affected rows' do - expect(adapter.exec_delete(sql, name, binds)).to eq(1) + expect(adapter.exec_delete(sql, name, binds)).to eq(affected_rows) end end From 4f1aad00f68fba341af393efc1c32bcdd1dac358 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 16:40:38 -0500 Subject: [PATCH 05/15] ci: bump Rails version Ref: https://github.com/departurerb/departure/pull/92 Co-authored-by: Katie Edgar <38359249+web-kat@users.noreply.github.com> --- .github/workflows/test.yml | 8 ++++++++ departure.gemspec | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7e1e5975..d247b7ec 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,6 +15,7 @@ jobs: - 5.2.0 - 6.0.0 - 6.1.0 + - 7.1.2 include: - ruby: 2.4 rails: 5.2.0 @@ -22,6 +23,13 @@ jobs: rails: 7.0.1 - ruby: 3.0 rails: 6.1.0 + - ruby: 3.0 + rails: 7.1.2 + exclude: + - ruby: 2.5 + rails: 7.1.2 + - ruby: 2.6 + rails: 7.1.2 env: PERCONA_DB_USER: root PERCONA_DB_PASSWORD: root diff --git a/departure.gemspec b/departure.gemspec index 49ddeddb..9ca20992 100644 --- a/departure.gemspec +++ b/departure.gemspec @@ -7,7 +7,7 @@ require 'departure/version' # This environment variable is set on CI to facilitate testing with multiple # versions of Rails. -RAILS_DEPENDENCY_VERSION = ENV.fetch('RAILS_VERSION', ['>= 5.2.0', '!= 7.0.0', '< 7.1']) +RAILS_DEPENDENCY_VERSION = ENV.fetch('RAILS_VERSION', ['>= 5.2.0', '!= 7.0.0', '<= 7.1.2']) Gem::Specification.new do |spec| spec.name = 'departure' From fdc128192788ad69d486e98b8e147178e812befd Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 17:51:04 -0500 Subject: [PATCH 06/15] fix(percona_adapter): use MySQL type map --- lib/active_record/connection_adapters/percona_adapter.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index 934babee..e3468c52 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -43,6 +43,8 @@ def percona_connection(config) module ConnectionAdapters class DepartureAdapter < AbstractMysqlAdapter + TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } + class Column < ActiveRecord::ConnectionAdapters::MySQL::Column def adapter DepartureAdapter From 2c74e6de854a6374951ab268516b5e7b34155a23 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 17:57:06 -0500 Subject: [PATCH 07/15] style: fix various nitpicks --- lib/active_record/connection_adapters/percona_adapter.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index e3468c52..7a2c567d 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -92,7 +92,7 @@ def exec_delete(sql, name, binds) end alias exec_update exec_delete - def exec_insert(sql, name, binds, pk = nil, sequence_name = nil, returning: nil) # rubocop:disable Lint/UnusedMethodArgument, Metrics/LineLength + def exec_insert(sql, name, binds, pk = nil, sequence_name = nil, returning: nil) # rubocop:disable Lint/UnusedMethodArgument, Metrics/LineLength, Metrics/ParameterLists execute(to_sql(sql, binds), name) end @@ -101,7 +101,7 @@ def internal_exec_query(sql, name = 'SQL', _binds = [], **_kwargs) # :nodoc: fields = result.fields if defined?(result.fields) ActiveRecord::Result.new(fields, result.to_a) end - alias :exec_query :internal_exec_query + alias exec_query internal_exec_query # Executes a SELECT query and returns an array of rows. Each row is an # array of field values. @@ -212,8 +212,7 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac end end - def reconnect - end + def reconnect; end end end end From 99c1dd8192329d86b2d3e76f1130f11e7b849e99 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 18:07:33 -0500 Subject: [PATCH 08/15] Try to fix older versions' type map --- lib/active_record/connection_adapters/percona_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index 7a2c567d..52d4526a 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -43,7 +43,7 @@ def percona_connection(config) module ConnectionAdapters class DepartureAdapter < AbstractMysqlAdapter - TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } + TYPE_MAP = Type::TypeMap.new.tap { |m| initialize_type_map(m) } if defined?(initialize_type_map) class Column < ActiveRecord::ConnectionAdapters::MySQL::Column def adapter From 07b821732e73b542faea74409b8e2733989f51f6 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 19:51:50 -0500 Subject: [PATCH 09/15] fix(column_with_sql): regex update for non-null --- lib/lhm/column_with_sql.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/lhm/column_with_sql.rb b/lib/lhm/column_with_sql.rb index 95b7c5f9..29118609 100644 --- a/lib/lhm/column_with_sql.rb +++ b/lib/lhm/column_with_sql.rb @@ -87,10 +87,10 @@ def default_value # # @return [Boolean] def null_value - match = /((\w*) NULL)/i.match(definition) + match = /((NOT)? NULL)/i.match(definition) return true unless match - match[2].downcase == 'not' ? false : true + match[2]&.downcase == 'not' ? false : true end end end From 3a11c6cafa9ba2eb0a617ad5edf08f6c682c2c5b Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 20:13:25 -0500 Subject: [PATCH 10/15] fix(percona_adapter): only override for 7.1+ --- Dockerfile | 2 +- lib/active_record/connection_adapters/percona_adapter.rb | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index cf6096ec..2236e49d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM ruby:2.3.4 +FROM ruby:3.0 MAINTAINER muffinista@gmail.com # Install apt based dependencies required to run Rails as diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index 52d4526a..da4e3831 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -200,6 +200,10 @@ def last_inserted_id(result) attr_reader :mysql_adapter + def self.less_than_active_record_7_1? + ActiveRecord.version < Gem::Version.create("7.1.0") + end + def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true) log(sql, name, async: async) do with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn| @@ -210,7 +214,7 @@ def raw_execute(sql, name, async: false, allow_retry: false, materialize_transac result end end - end + end unless less_than_active_record_7_1? def reconnect; end end From b16e117be5acdaba56b38d08f530e1bcb9632076 Mon Sep 17 00:00:00 2001 From: Brian Austin Date: Fri, 29 Dec 2023 20:19:41 -0500 Subject: [PATCH 11/15] style: fixes in percona adapter --- .rubocop.yml | 3 +++ .../connection_adapters/percona_adapter.rb | 24 +++++++++---------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ba5f8e47..83ebba91 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,4 +1,7 @@ --- +AllCops: + TargetRubyVersion: 2.4 + Metrics/AbcSize: Enabled: false diff --git a/lib/active_record/connection_adapters/percona_adapter.rb b/lib/active_record/connection_adapters/percona_adapter.rb index da4e3831..f2076ea7 100644 --- a/lib/active_record/connection_adapters/percona_adapter.rb +++ b/lib/active_record/connection_adapters/percona_adapter.rb @@ -200,21 +200,19 @@ def last_inserted_id(result) attr_reader :mysql_adapter - def self.less_than_active_record_7_1? - ActiveRecord.version < Gem::Version.create("7.1.0") - end - - def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true) - log(sql, name, async: async) do - with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn| - sync_timezone_changes(conn) - result = conn.query(sql) - verified! - handle_warnings(sql) - result + if ActiveRecord.version >= Gem::Version.create('7.1.0') + def raw_execute(sql, name, async: false, allow_retry: false, materialize_transactions: true) + log(sql, name, async: async) do + with_raw_connection(allow_retry: allow_retry, materialize_transactions: materialize_transactions) do |conn| + sync_timezone_changes(conn) + result = conn.query(sql) + verified! + handle_warnings(sql) + result + end end end - end unless less_than_active_record_7_1? + end def reconnect; end end From e04f5da37f6c9280d3386d2129b8c28163bcdcec Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Mon, 1 Jan 2024 12:18:25 -0300 Subject: [PATCH 12/15] Ignore new frozen string obligation --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 83ebba91..df8a0c9a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -47,6 +47,9 @@ Style/CommandLiteral: Style/Documentation: Enabled: false +Style/FrozenStringLiteralComment: + Enabled: false # We never ended up being forced to do this for Ruby 3.x + Style/MultilineBlockChain: Exclude: - 'spec/integration_spec.rb' From 05db8c84049987a897b6681d35e642005ef62da8 Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Mon, 1 Jan 2024 12:20:13 -0300 Subject: [PATCH 13/15] Satisfy Rubocop --- lib/lhm/column_with_sql.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/lhm/column_with_sql.rb b/lib/lhm/column_with_sql.rb index 29118609..476015d0 100644 --- a/lib/lhm/column_with_sql.rb +++ b/lib/lhm/column_with_sql.rb @@ -72,7 +72,7 @@ def column # # @return [String, NilClass] def default_value - match = if definition =~ /timestamp|datetime/i + match = if definition.match?(/timestamp|datetime/i) /default '?(.+[^'])'?/i.match(definition) else /default '?(\w+)'?/i.match(definition) From 3ffefac94a7f10c727be55ad190ac9ca32c2f566 Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Tue, 2 Jan 2024 10:42:44 -0300 Subject: [PATCH 14/15] Allow all 7.1.x Co-authored-by: Brian Austin <13002992+brianjaustin@users.noreply.github.com> --- departure.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/departure.gemspec b/departure.gemspec index 9ca20992..46cbb1d5 100644 --- a/departure.gemspec +++ b/departure.gemspec @@ -7,7 +7,7 @@ require 'departure/version' # This environment variable is set on CI to facilitate testing with multiple # versions of Rails. -RAILS_DEPENDENCY_VERSION = ENV.fetch('RAILS_VERSION', ['>= 5.2.0', '!= 7.0.0', '<= 7.1.2']) +RAILS_DEPENDENCY_VERSION = ENV.fetch('RAILS_VERSION', ['>= 5.2.0', '!= 7.0.0', '< 7.2.0']) Gem::Specification.new do |spec| spec.name = 'departure' From a08c1024f32a91c286919e963ff5af7cad5b3bae Mon Sep 17 00:00:00 2001 From: Ben Langfeld Date: Tue, 2 Jan 2024 11:26:08 -0300 Subject: [PATCH 15/15] Changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 543d6111..93894107 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Please follow the format in [Keep a Changelog](http://keepachangelog.com/) ## [Unreleased] - Fix support for Rails 6.0 and ForAlter `remove_index` . +- Support Rails 7.1.2 ## [6.5.0] - 2023-01-24