diff --git a/.gitignore b/.gitignore index dcf7be9..a1871cc 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ Gemfile.lock pkg tmp +/coverage/ \ No newline at end of file diff --git a/Gemfile b/Gemfile index 3c887d1..c5c403e 100644 --- a/Gemfile +++ b/Gemfile @@ -4,9 +4,13 @@ source 'https://rubygems.org' gemspec gem 'rake' +gem 'simplecov', require: false +gem 'railties' unless defined?(Appraisal) gem 'appraisal', require: false + gem 'sqlite3', '~>1.4' + gem 'pry-byebug' group :lint do gem 'rubocop' diff --git a/gemfiles/rails_61.gemfile b/gemfiles/rails_61.gemfile index c47e654..cac9102 100644 --- a/gemfiles/rails_61.gemfile +++ b/gemfiles/rails_61.gemfile @@ -3,6 +3,8 @@ source "https://rubygems.org" gem "rake" +gem "simplecov", require: false +gem "railties" gem "activerecord", "~>6.1.5" gem "sqlite3", "~>1.4" diff --git a/gemfiles/rails_61.gemfile.lock b/gemfiles/rails_61.gemfile.lock index fcb3c44..b325fc1 100644 --- a/gemfiles/rails_61.gemfile.lock +++ b/gemfiles/rails_61.gemfile.lock @@ -36,12 +36,14 @@ GEM concurrent-ruby (1.2.3) crass (1.0.6) diff-lcs (1.5.1) + docile (1.4.0) erubi (1.12.0) i18n (1.14.4) concurrent-ruby (~> 1.0) loofah (2.22.0) crass (~> 1.0.2) nokogiri (>= 1.12.0) + method_source (1.1.0) minitest (5.22.3) mysql2 (0.5.6) nokogiri (1.16.4-arm64-darwin) @@ -60,6 +62,12 @@ GEM rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) + railties (6.1.7.7) + actionpack (= 6.1.7.7) + activesupport (= 6.1.7.7) + method_source + rake (>= 12.2) + thor (~> 1.0) rake (13.2.1) rspec (3.13.0) rspec-core (~> 3.13.0) @@ -74,7 +82,14 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.12.3) + simplecov_json_formatter (0.1.4) sqlite3 (1.7.3-arm64-darwin) + thor (1.3.1) tzinfo (2.0.6) concurrent-ruby (~> 1.0) zeitwerk (2.6.13) @@ -88,9 +103,11 @@ DEPENDENCIES activerecord (~> 6.1.5) mysql2 pg + railties rake rspec (>= 2.0) seamless_database_pool! + simplecov sqlite3 (~> 1.4) BUNDLED WITH diff --git a/gemfiles/rails_70.gemfile b/gemfiles/rails_70.gemfile index f89b9d0..d971e1e 100644 --- a/gemfiles/rails_70.gemfile +++ b/gemfiles/rails_70.gemfile @@ -3,6 +3,8 @@ source "https://rubygems.org" gem "rake" +gem "simplecov", require: false +gem "railties" gem "activerecord", "~>7.0.4" gem "sqlite3", "~>1.4" diff --git a/gemfiles/rails_70.gemfile.lock b/gemfiles/rails_70.gemfile.lock index 8be57c8..ba3f072 100644 --- a/gemfiles/rails_70.gemfile.lock +++ b/gemfiles/rails_70.gemfile.lock @@ -35,12 +35,14 @@ GEM concurrent-ruby (1.2.3) crass (1.0.6) diff-lcs (1.5.1) + docile (1.4.0) erubi (1.12.0) i18n (1.14.4) concurrent-ruby (~> 1.0) loofah (2.22.0) crass (~> 1.0.2) nokogiri (>= 1.12.0) + method_source (1.1.0) minitest (5.22.3) mysql2 (0.5.6) nokogiri (1.16.4-arm64-darwin) @@ -59,6 +61,13 @@ GEM rails-html-sanitizer (1.6.0) loofah (~> 2.21) nokogiri (~> 1.14) + railties (7.0.8.1) + actionpack (= 7.0.8.1) + activesupport (= 7.0.8.1) + method_source + rake (>= 12.2) + thor (~> 1.0) + zeitwerk (~> 2.5) rake (13.2.1) rspec (3.13.0) rspec-core (~> 3.13.0) @@ -73,9 +82,17 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-support (3.13.1) + simplecov (0.22.0) + docile (~> 1.1) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.12.3) + simplecov_json_formatter (0.1.4) sqlite3 (1.7.3-arm64-darwin) + thor (1.3.1) tzinfo (2.0.6) concurrent-ruby (~> 1.0) + zeitwerk (2.6.13) PLATFORMS arm64-darwin-22 @@ -86,9 +103,11 @@ DEPENDENCIES activerecord (~> 7.0.4) mysql2 pg + railties rake rspec (>= 2.0) seamless_database_pool! + simplecov sqlite3 (~> 1.4) BUNDLED WITH diff --git a/lib/active_record/connection_adapters/seamless_database_pool_adapter.rb b/lib/active_record/connection_adapters/seamless_database_pool_adapter.rb index d4cd989..5f55449 100755 --- a/lib/active_record/connection_adapters/seamless_database_pool_adapter.rb +++ b/lib/active_record/connection_adapters/seamless_database_pool_adapter.rb @@ -12,6 +12,9 @@ def seamless_database_pool_connection(config) default_config.delete(:pool_adapter) master_config = default_config.merge(config[:master]).with_indifferent_access + if (url = master_config.delete(:url)) + master_config.merge!(ActiveRecord::DatabaseConfigurations::ConnectionUrlResolver.new(url).to_hash) + end establish_adapter(master_config[:adapter]) master_connection = send("#{master_config[:adapter]}_connection".to_sym, master_config) pool_weights[master_connection] = master_config[:pool_weight].to_i if master_config[:pool_weight].to_i > 0 @@ -21,6 +24,9 @@ def seamless_database_pool_connection(config) read_connections = [] config[:read_pool].each_with_index do |read_config, i| read_config = default_config.merge(read_config).with_indifferent_access + if (url = read_config.delete(:url)) + master_config.merge!(ActiveRecord::DatabaseConfigurations::ConnectionUrlResolver.new(url).to_hash) + end read_config[:pool_weight] = read_config[:pool_weight].to_i if read_config[:pool_weight] > 0 begin @@ -146,6 +152,7 @@ def adapter_class(master_connection) end # Set the arel visitor on the connections. + # unused in modern rails (was used in around rails 3)? looks being replaced by `connection.arel_visitor` def visitor_for(pool) # This is ugly, but then again, so is the code in ActiveRecord for setting the arel # visitor. There is a note in the code indicating the method signatures should be updated. diff --git a/lib/seamless_database_pool.rb b/lib/seamless_database_pool.rb index ddd782d..686c1dc 100644 --- a/lib/seamless_database_pool.rb +++ b/lib/seamless_database_pool.rb @@ -150,8 +150,8 @@ def master_database_configuration(database_configs) { adapter: new_conf[:pool_adapter] } ) - if conf.respond_to?(:url) - ActiveRecord::DatabaseConfigurations::UrlConfig.new(conf.env_name, conf.name, conf.url, new_conf) + if conf.respond_to?(:url) && (url = conf.url) || url = new_conf.delete(:url) + ActiveRecord::DatabaseConfigurations::UrlConfig.new(conf.env_name, conf.name, url, new_conf) else ActiveRecord::DatabaseConfigurations::HashConfig.new(conf.env_name, conf.name, new_conf) end diff --git a/lib/seamless_database_pool/arel_compiler.rb b/lib/seamless_database_pool/arel_compiler.rb index 11654ab..e27b8a2 100644 --- a/lib/seamless_database_pool/arel_compiler.rb +++ b/lib/seamless_database_pool/arel_compiler.rb @@ -1,3 +1,4 @@ +# is it used? module Arel module SqlCompiler # Hook into arel to use the compiler used by the master connection. diff --git a/spec/controller_filter_spec.rb b/spec/controller_filter_spec.rb index 9440c55..c5514e0 100644 --- a/spec/controller_filter_spec.rb +++ b/spec/controller_filter_spec.rb @@ -86,7 +86,9 @@ def read RSpec.describe SeamlessDatabasePool::ControllerFilter do let(:session){Hash.new} - let(:controller){SeamlessDatabasePool::TestOtherController.new(session)} + let(:base_controller_class) { SeamlessDatabasePool::TestBaseController } + let(:controller_class) { SeamlessDatabasePool::TestOtherController } + let(:controller) { controller_class.new(session) } it "should work with nothing set" do controller = SeamlessDatabasePool::TestApplicationController.new(session) @@ -96,6 +98,7 @@ def read it "should allow setting a connection type for a single action" do controller = SeamlessDatabasePool::TestBaseController.new(session) expect(controller.process('read')).to eq :persistent + expect(controller.process('other')).to eq :master end it "should allow setting a connection type for actions" do @@ -123,6 +126,19 @@ def read expect(controller.process('read')).to eq :persistent end + context "when set unknown method" do + let(:controller_class) do + Class.new(base_controller_class) do + use_database_pool({ base_action: :lala_foo }) + end + end + + it "raises" do + # expect(controller.send(:process_action, 'base_action')).to eq :persistent + expect { controller.send(:process_action, 'base_action') }.to raise_error(/Invalid read pool method/) + end + end + it "should not break trying to force the master connection if sessions are not enabled" do expect(controller.process('read')).to eq :persistent controller.use_master_db_connection_on_next_request diff --git a/spec/railtie_spec.rb b/spec/railtie_spec.rb new file mode 100644 index 0000000..c801f9e --- /dev/null +++ b/spec/railtie_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require 'rails' +require 'active_record/railtie' +require 'seamless_database_pool/railtie' + +class DummyApp < Rails::Application +end + +RSpec.describe SeamlessDatabasePool::Railtie do + it "installs log subscriber" do + described_class.initializers.each(&:run) + # expect ? + end + + + it "installs rake task" do + require 'rake' + require 'rake/testtask' + + Rails.application.load_tasks + expect(Rake.application["db:load_config"].actions.map(&:source_location).map(&:first)).to include(/seamless_database_pool/) + + ActiveRecord::Tasks::DatabaseTasks.database_configuration = {} + Rake.application["db:load_config"].invoke + ActiveRecord::Base.configurations + end +end diff --git a/spec/seamless_database_pool_adapter_spec.rb b/spec/seamless_database_pool_adapter_spec.rb index 5505a20..b92a43e 100755 --- a/spec/seamless_database_pool_adapter_spec.rb +++ b/spec/seamless_database_pool_adapter_spec.rb @@ -74,6 +74,43 @@ def columns(table_name, name = nil); end expect(conn).to eq pool_connection end + it "should support urls in config" do + options = { + adapter: 'seamless_database_pool', + pool_adapter: 'reader', + username: 'user', + master: { + url: 'writer://master-host' + }, + read_pool: [ + {'host' => 'read_host_1'}, + {'host' => 'read_host_2', 'pool_weight' => '2'}, + { 'url' => 'reader://read-host-3', 'pool_weight' => '0' } + ] + } + + pool_connection = double(:connection) + master_connection = SeamlessDatabasePool::MockConnection.new("master") + read_connection_1 = SeamlessDatabasePool::MockConnection.new("read_1") + read_connection_2 = SeamlessDatabasePool::MockConnection.new("read_2") + logger = ActiveRecord::Base.logger + weights = {master_connection => 1, read_connection_1 => 1, read_connection_2 => 2} + + expect(ActiveRecord::Base).to receive(:writer_connection).with({'adapter' => 'writer', 'host' => 'master-host', 'username' => 'user', 'pool_weight' => 1}).and_return(master_connection) + expect(ActiveRecord::Base).to receive(:reader_connection).with({'adapter' => 'reader', 'host' => 'read_host_1', 'username' => 'user', 'pool_weight' => 1}).and_return(read_connection_1) + expect(ActiveRecord::Base).to receive(:reader_connection).with({'adapter' => 'reader', 'host' => 'read_host_2', 'username' => 'user', 'pool_weight' => 2}).and_return(read_connection_2) + + klass = double(:class) + expect(ActiveRecord::ConnectionAdapters::SeamlessDatabasePoolAdapter).to receive(:adapter_class).with(master_connection).and_return(klass) + expect(klass).to receive(:new).with(nil, logger, master_connection, [read_connection_1, read_connection_2], weights, options).and_return(pool_connection) + + expect(ActiveRecord::Base).to receive(:establish_adapter).with('writer') + expect(ActiveRecord::Base).to receive(:establish_adapter).with('reader').twice + + conn = ActiveRecord::Base.seamless_database_pool_connection(options) + expect(conn).to eq pool_connection + end + it "should raise an error if the adapter would be recursive" do expect do ActiveRecord::Base.seamless_database_pool_connection({ master: { adapter: 'seamless_database_pool' } }) diff --git a/spec/seamless_database_pool_spec.rb b/spec/seamless_database_pool_spec.rb index 96d8995..9eee702 100644 --- a/spec/seamless_database_pool_spec.rb +++ b/spec/seamless_database_pool_spec.rb @@ -191,7 +191,12 @@ shard_replica: pool_adapter: mysql2 database: dev_db_shard - replica: true + replica: true + with_url: + adapter: seamless_database_pool + pool_adapter: mysql2 + master: + url: mysql2://localhost:1234 test: adapter: mysql2 database: test_db @@ -200,7 +205,7 @@ it "should pull out the master configurations for compatibility with rake db:* tasks" do expect(master_database_configuration).to be_a(ActiveRecord::DatabaseConfigurations) - expect(master_database_configuration.configurations.size).to eq(3) # except replica + expect(master_database_configuration.configurations.size).to eq(4) # except replica expect(master_database_configuration.configs_for(env_name: 'development').map(&:configuration_hash)).to eq([ { @@ -215,6 +220,11 @@ migrations_paths: 'db/migrate_shards', database_tasks: true, schema_dump: false + }, + { + adapter: 'mysql2', + host: 'localhost', + port: 1234 } ]) expect(master_database_configuration.configs_for(env_name: 'test').map(&:configuration_hash)).to eq([{ diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index eb60b87..39f60e2 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,12 @@ require 'rubygems' +if ENV['COVERAGE'] + require 'simplecov' + SimpleCov.start do + add_filter 'spec' + end +end + require 'active_record' puts "Testing Against ActiveRecord #{ActiveRecord::VERSION::STRING} on Ruby #{RUBY_VERSION}" if defined?(ActiveRecord::VERSION)