From bd3e97cc438a6b66ab3d431a7c078c16f0a1127a Mon Sep 17 00:00:00 2001 From: Marko Kajzer Date: Mon, 2 Dec 2024 01:02:40 +0100 Subject: [PATCH] fix(ActiveRecord): correctly connect to database in Rails 7.2+ (#175) * fix(ActiveRecord): correctly connect to the database in Rails 7.2+ ci: add active_record 7.2 to ci matrix deps: pin pagy_cursor to prevent incompatible version deps: pin activerecord 7.2+ compatible version of database_cleaner ran into https://github.com/DatabaseCleaner/database_cleaner-active_record/issues/83 * chore: skip activerecord specs on mongoid * deps: use correct database_cleaner adapter per database * deps: pin mongoid-scroll to v1 --- .github/workflows/test-postgresql.yml | 1 + CHANGELOG.md | 1 + Gemfile | 13 +++++-- .../config/database_adapters/activerecord.rb | 7 +++- .../activerecord}/database_cleaner.rb | 2 +- .../mongoid/database_cleaner.rb | 14 +++++++ spec/slack-ruby-bot-server/app_spec.rb | 37 +++++++++++++++++++ 7 files changed, 70 insertions(+), 5 deletions(-) rename spec/{support => database_adapters/activerecord}/database_cleaner.rb (85%) create mode 100644 spec/database_adapters/mongoid/database_cleaner.rb diff --git a/.github/workflows/test-postgresql.yml b/.github/workflows/test-postgresql.yml index f395bec..7c30962 100644 --- a/.github/workflows/test-postgresql.yml +++ b/.github/workflows/test-postgresql.yml @@ -10,6 +10,7 @@ jobs: - { ruby: 2.6.2, postgresql: 11, active_record: '~> 6.0.0' } - { ruby: 3.1.1, postgresql: 11, active_record: '~> 6.1.0' } - { ruby: 3.1.1, postgresql: 14, active_record: '~> 7.0.0' } + - { ruby: 3.1.1, postgresql: 14, active_record: '~> 7.2.0' } name: test (ruby=${{ matrix.entry.ruby }}, postgresql=${{ matrix.entry.postgresql }}, active_record=${{ matrix.entry.active_record }}) steps: - uses: actions/checkout@v2 diff --git a/CHANGELOG.md b/CHANGELOG.md index c40f8d7..4be8fc1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 2.1.2 (Next) * Your contribution here. +* [#175](https://github.com/slack-ruby/slack-ruby-bot-server/pull/175): Fix(activerecord): correctly check for database in rails 7.2+ - [@markokajzer](https://github.com/markokajzer). ### 2.1.1 (2023/07/25) diff --git a/Gemfile b/Gemfile index 1401663..5962aab 100644 --- a/Gemfile +++ b/Gemfile @@ -4,13 +4,21 @@ case ENV.fetch('DATABASE_ADAPTER', nil) when 'mongoid' then gem 'kaminari-mongoid' gem 'mongoid', ENV['MONGOID_VERSION'] || '~> 7.3.0' - gem 'mongoid-scroll' + gem 'mongoid-scroll', '~> 1.0.1' gem 'mongoid-shell' + + group :development, :test do + gem 'database_cleaner-mongoid', '~> 2.0.1' + end when 'activerecord' then gem 'activerecord', ENV['ACTIVERECORD_VERSION'] || '~> 6.0.0' gem 'otr-activerecord' - gem 'pagy_cursor' + gem 'pagy_cursor', '~> 0.6.1' gem 'pg' + + group :development, :test do + gem 'database_cleaner-active_record', '~> 2.2.0' + end when nil warn "Missing ENV['DATABASE_ADAPTER']." else @@ -23,7 +31,6 @@ group :development, :test do gem 'bundler' gem 'byebug' gem 'capybara', '~> 3.36.0' - gem 'database_cleaner', '~> 1.8.5' gem 'fabrication' gem 'faker' gem 'faraday', '0.17.5' diff --git a/lib/slack-ruby-bot-server/config/database_adapters/activerecord.rb b/lib/slack-ruby-bot-server/config/database_adapters/activerecord.rb index d7ea4ad..c3e8e0f 100644 --- a/lib/slack-ruby-bot-server/config/database_adapters/activerecord.rb +++ b/lib/slack-ruby-bot-server/config/database_adapters/activerecord.rb @@ -6,7 +6,12 @@ module SlackRubyBotServer module DatabaseAdapter def self.check! - ActiveRecord::Base.connection_pool.with_connection(&:active?) + if ActiveRecord.version >= Gem::Version.new('7.2') + ActiveRecord::Base.connection.database_exists? + else + ActiveRecord::Base.connection_pool.with_connection(&:active?) + end + raise 'Unexpected error.' unless ActiveRecord::Base.connected? rescue StandardError => e warn "Error connecting to PostgreSQL: #{e.message}" diff --git a/spec/support/database_cleaner.rb b/spec/database_adapters/activerecord/database_cleaner.rb similarity index 85% rename from spec/support/database_cleaner.rb rename to spec/database_adapters/activerecord/database_cleaner.rb index 312b7b3..e9dc858 100644 --- a/spec/support/database_cleaner.rb +++ b/spec/database_adapters/activerecord/database_cleaner.rb @@ -1,4 +1,4 @@ -require 'database_cleaner' +require 'database_cleaner/active_record' RSpec.configure do |config| config.before :suite do diff --git a/spec/database_adapters/mongoid/database_cleaner.rb b/spec/database_adapters/mongoid/database_cleaner.rb new file mode 100644 index 0000000..51e01da --- /dev/null +++ b/spec/database_adapters/mongoid/database_cleaner.rb @@ -0,0 +1,14 @@ +require 'database_cleaner/mongoid' + +RSpec.configure do |config| + config.before :suite do + DatabaseCleaner.strategy = :deletion + DatabaseCleaner.clean_with :deletion + end + + config.around :each do |example| + DatabaseCleaner.cleaning do + example.run + end + end +end diff --git a/spec/slack-ruby-bot-server/app_spec.rb b/spec/slack-ruby-bot-server/app_spec.rb index c66cb5d..901b890 100644 --- a/spec/slack-ruby-bot-server/app_spec.rb +++ b/spec/slack-ruby-bot-server/app_spec.rb @@ -13,6 +13,43 @@ expect(app.instance).to be_an_instance_of(app) end end + context '#prepare!' do + context 'when connection cannot be established' do + context 'with ActiveRecord >= 7.2' do + before do + skip 'incorrect database adapter' unless ENV['DATABASE_ADAPTER'] == 'activerecord' + skip 'incorrect ActiveRecord version' if ActiveRecord.version < Gem::Version.new('7.2') + + # Make sure ActiveRecord is not connected in any way before the spec starts + ActiveRecord::Base.connection_pool.disconnect! + end + + it 'raises' do + expect(ActiveRecord::Base).not_to be_connected + + # Simulate that #database_exists? does not connect to the database + allow(ActiveRecord::Base).to receive_message_chain(:connection, :database_exists?) + expect { subject.prepare! } + .to raise_error('Unexpected error.') + end + end + + context 'with ActiveRecord < 7.2' do + before do + skip 'incorrect database adapter' unless ENV['DATABASE_ADAPTER'] == 'activerecord' + skip 'incorrect ActiveRecord version' if ActiveRecord.version >= Gem::Version.new('7.2') + end + + it 'raises' do + # In ActiveRecord < 7.1, `disconnect!` closes the connection that has already been leased by + # DatabaseCleaner, so we cannot do that trick here to simulate a not established connection. + allow(ActiveRecord::Base).to receive(:connected?).and_return(false) + expect { subject.prepare! } + .to raise_error('Unexpected error.') + end + end + end + end context '#purge_inactive_teams!' do it 'purges teams' do expect(Team).to receive(:purge!)