From 172f8c92d591ff90158bb6142adc2df82a318fcd Mon Sep 17 00:00:00 2001 From: Roland Moriz Date: Fri, 14 Feb 2020 15:04:29 +0100 Subject: [PATCH] attempt to backport PR #318 to v3.1.0 --- lib/queue_classic.rb | 13 ++-- lib/queue_classic/queue.rb | 2 +- queue_classic.gemspec | 1 + test/activerecord_integration_test.rb | 40 ++++++++++++ test/helper.rb | 24 ++++++- .../queue_classic_rails_connection_test.rb | 65 ++++++++++++++++--- test/worker_test.rb | 9 +-- 7 files changed, 136 insertions(+), 18 deletions(-) create mode 100644 test/activerecord_integration_test.rb diff --git a/lib/queue_classic.rb b/lib/queue_classic.rb index 4276a138..fe38864c 100644 --- a/lib/queue_classic.rb +++ b/lib/queue_classic.rb @@ -58,12 +58,17 @@ def self.has_connection? def self.default_conn_adapter return @conn_adapter if defined?(@conn_adapter) && @conn_adapter + if rails_connection_sharing_enabled? - @conn_adapter = ConnAdapter.new(ActiveRecord::Base.connection.raw_connection) - else - @conn_adapter = ConnAdapter.new + # AR connection should never be memoized, as call to + # ActiveRecord::Base.clear_active_connections! in current thread + # will cause it to return to AR pool, and it will become shared + # between memoized value and pool. And Rails does + # clear_active_connections! after each request + return ConnAdapter.new(ActiveRecord::Base.connection.raw_connection) end - @conn_adapter + + @conn_adapter = ConnAdapter.new end def self.default_conn_adapter=(conn) diff --git a/lib/queue_classic/queue.rb b/lib/queue_classic/queue.rb index 3c90ca94..4ec15fe9 100644 --- a/lib/queue_classic/queue.rb +++ b/lib/queue_classic/queue.rb @@ -17,7 +17,7 @@ def conn_adapter=(a) end def conn_adapter - @adapter ||= QC.default_conn_adapter + (defined?(@adapter) && @adapter) || QC.default_conn_adapter end # enqueue(m,a) inserts a row into the jobs table and trigger a notification. diff --git a/queue_classic.gemspec b/queue_classic.gemspec index 93f1b9a6..7e9f2308 100644 --- a/queue_classic.gemspec +++ b/queue_classic.gemspec @@ -19,4 +19,5 @@ Gem::Specification.new do |s| s.require_paths = %w[lib] s.add_dependency "pg", ">= 0.17", "< 0.19" + s.add_development_dependency "activerecord", "~> 5.2.3" end diff --git a/test/activerecord_integration_test.rb b/test/activerecord_integration_test.rb new file mode 100644 index 00000000..bd7f40ed --- /dev/null +++ b/test/activerecord_integration_test.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +require File.expand_path("../helper.rb", __FILE__) +require 'active_record' + +class QueueClassicActiverecorgdIntegrationTest < QCTest + def setup + super + reset_globals + ActiveRecord::Base.establish_connection "#{ENV["DATABASE_URL"]}?pool=1" + end + + def teardown + super + end + + def test_connection_not_returned_to_pool + take_connection_until_checked = Mutex.new + conn_adapter_ready = Queue.new + connection_from_adapter = nil + take_connection_until_checked.synchronize do + Thread.new do + with_env 'QC_RAILS_DATABASE' => 'true' do + conn_adapter_ready.push QC.default_conn_adapter.connection + end + take_connection_until_checked.synchronize {} + ActiveRecord::Base.clear_active_connections! + end + + connection_from_adapter = conn_adapter_ready.pop + + ActiveRecord::Base.connection_pool.reap + + # Pool now has no free connections available + assert_raises(ActiveRecord::ConnectionTimeoutError) do + ActiveRecord::Base.connection_pool.checkout 0.1 + end + end + end +end diff --git a/test/helper.rb b/test/helper.rb index c96f608f..ad5ee969 100644 --- a/test/helper.rb +++ b/test/helper.rb @@ -2,9 +2,13 @@ $: << File.expand_path("test") require "bundler" -Bundler.setup :default, :test +Bundler.setup :default, :development, :test ENV["DATABASE_URL"] ||= "postgres:///queue_classic_test" +# Rails integration is enabled if ActiveRecord module constant is +# present, and it will be present after tests that use +# ActiveRecord. So forcifully disable Rails by default. +ENV["QC_RAILS_DATABASE"] = "false" require "queue_classic" require "stringio" @@ -18,6 +22,7 @@ def setup def teardown QC.delete_all + reset_globals end def init_db @@ -29,6 +34,11 @@ def init_db c.disconnect end + def reset_globals + QC.default_conn_adapter = nil + QC.default_queue = nil + end + def capture_stderr_output original_stderr = $stderr $stderr = StringIO.new @@ -51,4 +61,16 @@ def capture_debug_output $stdout = original_stdout end + def with_env(temporary_environment) + original_environment = {} + temporary_environment.each do |name, value| + original_environment[name] = ENV[name] + ENV[name] = value + end + yield + ensure + original_environment.each { |name, value| ENV[name] = value } + end + + end diff --git a/test/lib/queue_classic_rails_connection_test.rb b/test/lib/queue_classic_rails_connection_test.rb index 00d9f225..65c7357c 100644 --- a/test/lib/queue_classic_rails_connection_test.rb +++ b/test/lib/queue_classic_rails_connection_test.rb @@ -1,23 +1,72 @@ require File.expand_path("../../helper.rb", __FILE__) class QueueClassicRailsConnectionTest < QCTest - def before_setup - Object.send :const_set, :ActiveRecord, Module.new - ActiveRecord.const_set :Base, Module.new + # Stubs ActiveRecord::Base with empty module. Works if real + # ActiveRecord is loaded too, restoring it when unstubbing. + class StubActiveRecord + def stub + stub_module + stub_base + end + + def unstub + unstub_base + unstub_module + end + + private + + def stub_module + unless Object.const_defined? :ActiveRecord + Object.send :const_set, :ActiveRecord, Module.new + @module_stubbed = true + else + @module_stubbed = false + end + end + + def unstub_module + if @module_stubbed + Object.send :remove_const, :ActiveRecord + end + end + def stub_base + if Object.const_defined? 'ActiveRecord::Base' + @activerecord_orig = ActiveRecord::Base + ActiveRecord.send :remove_const, :Base + else + @activerecord_orig = nil + end + ActiveRecord.const_set :Base, Module.new + end + + def unstub_base + ActiveRecord.send :remove_const, :Base + if @activerecord_orig + ActiveRecord.send :const_set, :Base, @activerecord_orig + end + end + end + + def setup + super + @active_record_stub = StubActiveRecord.new + @active_record_stub.stub @original_conn_adapter = QC.default_conn_adapter QC.default_conn_adapter = nil end - def after_teardown - ActiveRecord.send :remove_const, :Base - Object.send :remove_const, :ActiveRecord - + def teardown + @active_record_stub.unstub QC.default_conn_adapter = @original_conn_adapter + super end def test_uses_active_record_connection_if_exists - connection = get_connection + connection = with_env 'QC_RAILS_DATABASE' => nil do + get_connection + end assert connection.verify end diff --git a/test/worker_test.rb b/test/worker_test.rb index b69a301b..1df77139 100644 --- a/test/worker_test.rb +++ b/test/worker_test.rb @@ -178,7 +178,7 @@ def test_work_with_more_complex_construct end def test_init_worker_with_arg - set_database 'postgres:///invalid' + set_database 'postgres:///queue_classic_test' conn = PG::Connection.connect(dbname: 'queue_classic_test') QC::Worker.new connection: conn @@ -201,9 +201,10 @@ def test_init_worker_with_database_url end def test_init_worker_without_conn - set_database nil - - assert_raises(ArgumentError) { QC::Worker.new } + assert_raises(ArgumentError) do + set_database nil + QC::Worker.new + end ensure reset_database