From 69c23fe09887fc5d97ac7b0194825c21efe244a5 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sun, 29 Oct 2023 17:24:47 +0100 Subject: [PATCH] feat: Add testing for activerecord 7.1 and support for trilogy adapter (#77) --------- Co-authored-by: Hartley McGuire --- .github/workflows/ci.yml | 17 +- .gitignore | 3 +- Appraisals | 36 ++-- gemfiles/activerecord_6.0.gemfile | 19 -- gemfiles/activerecord_6.1.gemfile | 2 + gemfiles/activerecord_7.0.gemfile | 8 + ..._edge.gemfile => activerecord_7.1.gemfile} | 3 +- lib/with_advisory_lock.rb | 4 +- lib/with_advisory_lock/concern.rb | 21 +-- .../database_adapter_support.rb | 41 +--- lib/with_advisory_lock/mysql.rb | 1 - lib/with_advisory_lock/mysql_no_nesting.rb | 22 --- .../nested_advisory_lock_error.rb | 16 -- test/concern_test.rb | 18 +- test/lock_test.rb | 115 +++++------- test/nesting_test.rb | 66 +------ test/options_test.rb | 40 ++-- test/parallelism_test.rb | 50 +++-- test/shared_test.rb | 175 +++++++++--------- test/test_helper.rb | 39 ++-- test/thread_test.rb | 27 ++- test/transaction_test.rb | 57 +++--- with_advisory_lock.gemspec | 7 +- 23 files changed, 297 insertions(+), 490 deletions(-) delete mode 100644 gemfiles/activerecord_6.0.gemfile rename gemfiles/{activerecord_edge.gemfile => activerecord_7.1.gemfile} (73%) delete mode 100644 lib/with_advisory_lock/mysql_no_nesting.rb delete mode 100644 lib/with_advisory_lock/nested_advisory_lock_error.rb diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9bbdba2..69336b5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,7 +11,7 @@ jobs: runs-on: ubuntu-latest services: mysql: - image: mysql/mysql-server:8.0.30 + image: mysql/mysql-server:5.7 ports: - "3306:3306" env: @@ -41,24 +41,15 @@ jobs: - '2.7' - 'truffleruby' rails: + - activerecord_7.1 - activerecord_7.0 - activerecord_6.1 - - activerecord_6.0 - - activerecord_edge adapter: - sqlite3:///tmp/test.sqlite3 - mysql2://root:root@0/with_advisory_lock_test + - trilogy://root:root@0/with_advisory_lock_test - postgres://closure_tree:closure_tree@0/with_advisory_lock_test include: - - ruby: jruby - rails: activerecord_6.0 - adapter: jdbcmysql://root:root@0/with_advisory_lock_test - - ruby: jruby - rails: activerecord_6.0 - adapter: jdbcsqlite3:///tmp/test.sqlite3 - - ruby: jruby - rails: activerecord_6.0 - adapter: jdbcpostgresql://closure_tree:closure_tree@0/with_advisory_lock_test - ruby: jruby rails: activerecord_6.1 adapter: jdbcmysql://root:root@0/with_advisory_lock_test @@ -70,7 +61,7 @@ jobs: adapter: jdbcpostgresql://closure_tree:closure_tree@0/with_advisory_lock_test steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Setup Ruby uses: ruby/setup-ruby@v1 diff --git a/.gitignore b/.gitignore index ae8fb37..d4f7e62 100644 --- a/.gitignore +++ b/.gitignore @@ -18,4 +18,5 @@ test/tmp test/version_tmp tmp *.iml -test/sqlite.db \ No newline at end of file +test/sqlite.db +.env \ No newline at end of file diff --git a/Appraisals b/Appraisals index d15c3f6..5e51862 100644 --- a/Appraisals +++ b/Appraisals @@ -1,24 +1,22 @@ # frozen_string_literal: true -appraise 'activerecord-6.0' do - gem 'activerecord', '~> 6.0.0' +appraise 'activerecord-7.1' do + gem 'activerecord', '~> 7.1.0' platforms :ruby do gem 'sqlite3' gem 'mysql2' + gem 'trilogy' gem 'pg' end - platforms :jruby do - gem "activerecord-jdbcmysql-adapter" - gem "activerecord-jdbcpostgresql-adapter" - gem "activerecord-jdbcsqlite3-adapter" - end end -appraise 'activerecord-6.1' do - gem 'activerecord', '~> 6.1.0' +appraise 'activerecord-7.0' do + gem 'activerecord', '~> 7.0.0' platforms :ruby do gem 'sqlite3' gem 'mysql2' + gem 'trilogy' + gem "activerecord-trilogy-adapter" gem 'pg' end platforms :jruby do @@ -28,20 +26,20 @@ appraise 'activerecord-6.1' do end end -appraise 'activerecord-7.0' do - gem 'activerecord', '~> 7.0.0' - platforms :ruby do - gem 'sqlite3' - gem 'mysql2' - gem 'pg' - end -end +appraise 'activerecord-6.1' do + gem 'activerecord', '~> 6.1.0' -appraise 'activerecord-edge' do - gem 'activerecord', github: 'rails/rails', branch: 'main' platforms :ruby do gem 'sqlite3' gem 'mysql2' + gem 'trilogy' + gem "activerecord-trilogy-adapter" gem 'pg' end + platforms :jruby do + gem "activerecord-jdbcmysql-adapter" + gem "activerecord-jdbcpostgresql-adapter" + gem "activerecord-jdbcsqlite3-adapter" + end end + diff --git a/gemfiles/activerecord_6.0.gemfile b/gemfiles/activerecord_6.0.gemfile deleted file mode 100644 index 8d80c12..0000000 --- a/gemfiles/activerecord_6.0.gemfile +++ /dev/null @@ -1,19 +0,0 @@ -# This file was generated by Appraisal - -source "https://rubygems.org" - -gem "activerecord", "~> 6.0.0" - -platforms :ruby do - gem "sqlite3" - gem "mysql2" - gem "pg" -end - -platforms :jruby do - gem "activerecord-jdbcmysql-adapter" - gem "activerecord-jdbcpostgresql-adapter" - gem "activerecord-jdbcsqlite3-adapter" -end - -gemspec path: "../" diff --git a/gemfiles/activerecord_6.1.gemfile b/gemfiles/activerecord_6.1.gemfile index f7a143f..b87ebba 100644 --- a/gemfiles/activerecord_6.1.gemfile +++ b/gemfiles/activerecord_6.1.gemfile @@ -7,6 +7,8 @@ gem "activerecord", "~> 6.1.0" platforms :ruby do gem "sqlite3" gem "mysql2" + gem "trilogy" + gem "activerecord-trilogy-adapter" gem "pg" end diff --git a/gemfiles/activerecord_7.0.gemfile b/gemfiles/activerecord_7.0.gemfile index 3e360db..3ef6b18 100644 --- a/gemfiles/activerecord_7.0.gemfile +++ b/gemfiles/activerecord_7.0.gemfile @@ -7,7 +7,15 @@ gem "activerecord", "~> 7.0.0" platforms :ruby do gem "sqlite3" gem "mysql2" + gem "trilogy" + gem "activerecord-trilogy-adapter" gem "pg" end +platforms :jruby do + gem "activerecord-jdbcmysql-adapter" + gem "activerecord-jdbcpostgresql-adapter" + gem "activerecord-jdbcsqlite3-adapter" +end + gemspec path: "../" diff --git a/gemfiles/activerecord_edge.gemfile b/gemfiles/activerecord_7.1.gemfile similarity index 73% rename from gemfiles/activerecord_edge.gemfile rename to gemfiles/activerecord_7.1.gemfile index d370fd2..a9455ae 100644 --- a/gemfiles/activerecord_edge.gemfile +++ b/gemfiles/activerecord_7.1.gemfile @@ -2,11 +2,12 @@ source "https://rubygems.org" -gem "activerecord", github: "rails/rails", branch: "main" +gem "activerecord", "~> 7.1.0" platforms :ruby do gem "sqlite3" gem "mysql2" + gem "trilogy" gem "pg" end diff --git a/lib/with_advisory_lock.rb b/lib/with_advisory_lock.rb index a7387aa..a3cf4b8 100644 --- a/lib/with_advisory_lock.rb +++ b/lib/with_advisory_lock.rb @@ -1,5 +1,6 @@ require 'with_advisory_lock/version' require 'active_support' +require_relative 'with_advisory_lock/failed_to_acquire_lock' module WithAdvisoryLock extend ActiveSupport::Autoload @@ -9,9 +10,6 @@ module WithAdvisoryLock autoload :DatabaseAdapterSupport autoload :Flock autoload :MySQL, 'with_advisory_lock/mysql' - autoload :MySQLNoNesting, 'with_advisory_lock/mysql_no_nesting' - autoload :NestedAdvisoryLockError - autoload :FailedToAcquireLock autoload :PostgreSQL, 'with_advisory_lock/postgresql' end diff --git a/lib/with_advisory_lock/concern.rb b/lib/with_advisory_lock/concern.rb index f787268..42d566d 100644 --- a/lib/with_advisory_lock/concern.rb +++ b/lib/with_advisory_lock/concern.rb @@ -7,7 +7,7 @@ module Concern extend ActiveSupport::Concern delegate :with_advisory_lock, :with_advisory_lock!, :advisory_lock_exists?, to: 'self.class' - module ClassMethods + class_methods do def with_advisory_lock(lock_name, options = {}, &block) result = with_advisory_lock_result(lock_name, options, &block) result.lock_was_acquired? ? result.result : false @@ -23,8 +23,7 @@ def with_advisory_lock!(lock_name, options = {}, &block) end def with_advisory_lock_result(lock_name, options = {}, &block) - class_options = options.extract!(:force_nested_lock_support) if options.respond_to?(:fetch) - impl = impl_class(class_options).new(connection, lock_name, options) + impl = impl_class.new(connection, lock_name, options) impl.with_advisory_lock_if_needed(&block) end @@ -40,24 +39,12 @@ def current_advisory_lock private - def impl_class(options = nil) + def impl_class adapter = WithAdvisoryLock::DatabaseAdapterSupport.new(connection) if adapter.postgresql? WithAdvisoryLock::PostgreSQL elsif adapter.mysql? - nested_lock = if options.respond_to?(:fetch) && [true, - false].include?(options.fetch(:force_nested_lock_support, - nil)) - options.fetch(:force_nested_lock_support) - else - adapter.mysql_nested_lock_support? - end - - if nested_lock - WithAdvisoryLock::MySQL - else - WithAdvisoryLock::MySQLNoNesting - end + WithAdvisoryLock::MySQL else WithAdvisoryLock::Flock end diff --git a/lib/with_advisory_lock/database_adapter_support.rb b/lib/with_advisory_lock/database_adapter_support.rb index 2b0d6d3..640c9bc 100644 --- a/lib/with_advisory_lock/database_adapter_support.rb +++ b/lib/with_advisory_lock/database_adapter_support.rb @@ -12,46 +12,7 @@ def initialize(connection) end def mysql? - @sym_name == :mysql2 - end - - # Nested lock support for MySQL was introduced in 5.7.5 - # Checking by version number is complicated by MySQL compatible DBs (like MariaDB) having their own versioning schemes - # Therefore, we check for nested lock support by simply trying a nested lock, then testing and caching the outcome - def mysql_nested_lock_support? - return false unless mysql? - - # We select the MySQL version this way and cache on it, as MySQL will report versions like "5.7.5", and MariaDB will - # report versions like "10.3.8-MariaDB", which allow us to cache on features without introducing problems. - version = @connection.select_value('SELECT version()') - - @@mysql_nl_cache_mutex.synchronize do - return @@mysql_nl_cache[version] if @@mysql_nl_cache.keys.include?(version) - - lock_1 = "\"nested-test-1-#{SecureRandom.hex}\"" - lock_2 = "\"nested-test-2-#{SecureRandom.hex}\"" - - get_1 = @connection.select_value("SELECT GET_LOCK(#{lock_1}, 0) AS t#{SecureRandom.hex}") - get_2 = @connection.select_value("SELECT GET_LOCK(#{lock_2}, 0) AS t#{SecureRandom.hex}") - - # Both locks should succeed in old and new MySQL versions with "1" - raise "Unexpected nested lock acquire result #{get_1}, #{get_2}" unless [get_1, get_2] == [1, 1] - - release_1 = @connection.select_value("SELECT RELEASE_LOCK(#{lock_1}) AS t#{SecureRandom.hex}") - release_2 = @connection.select_value("SELECT RELEASE_LOCK(#{lock_2}) AS t#{SecureRandom.hex}") - - # In MySQL < 5.7.5 release_1 will return nil (not currently locked) and release_2 will return 1 (successfully unlocked) - # In MySQL >= 5.7.5 release_1 and release_2 will return 1 (both successfully unlocked) - # See https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock for more - @@mysql_nl_cache[version] = case [release_1, release_2] - when [1, 1] - true - when [nil, 1] - false - else - raise "Unexpected nested lock release result #{release_1}, #{release_2}" - end - end + %i[mysql2 trilogy].include? @sym_name end def postgresql? diff --git a/lib/with_advisory_lock/mysql.rb b/lib/with_advisory_lock/mysql.rb index b3101f6..a437dec 100644 --- a/lib/with_advisory_lock/mysql.rb +++ b/lib/with_advisory_lock/mysql.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module WithAdvisoryLock - # MySQL > 5.7.5 supports nested locks class MySQL < Base # See https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock def try_lock diff --git a/lib/with_advisory_lock/mysql_no_nesting.rb b/lib/with_advisory_lock/mysql_no_nesting.rb deleted file mode 100644 index 68ef78f..0000000 --- a/lib/with_advisory_lock/mysql_no_nesting.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -module WithAdvisoryLock - # For MySQL < 5.7.5 that does not support nested locks - class MySQLNoNesting < MySQL - # See http://dev.mysql.com/doc/refman/5.0/en/miscellaneous-functions.html#function_get-lock - def try_lock - unless lock_stack.empty? - raise NestedAdvisoryLockError.new( - "MySQL < 5.7.5 doesn't support nested Advisory Locks", - lock_stack.dup - ) - end - super - end - - # MySQL doesn't support nested locks: - def already_locked? - lock_stack.last == lock_stack_item - end - end -end diff --git a/lib/with_advisory_lock/nested_advisory_lock_error.rb b/lib/with_advisory_lock/nested_advisory_lock_error.rb deleted file mode 100644 index 7c4dd15..0000000 --- a/lib/with_advisory_lock/nested_advisory_lock_error.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module WithAdvisoryLock - class NestedAdvisoryLockError < StandardError - attr_accessor :lock_stack - - def initialize(msg = nil, lock_stack = nil) - super(msg) - @lock_stack = lock_stack - end - - def to_s - super + (lock_stack ? ": lock stack = #{lock_stack}" : '') - end - end -end diff --git a/test/concern_test.rb b/test/concern_test.rb index 5944207..1299b3a 100644 --- a/test/concern_test.rb +++ b/test/concern_test.rb @@ -2,34 +2,32 @@ require 'test_helper' -describe 'with_advisory_lock.concern' do - it 'adds with_advisory_lock to ActiveRecord classes' do +class WithAdvisoryLockConcernTest < GemTestCase + test 'adds with_advisory_lock to ActiveRecord classes' do assert_respond_to(Tag, :with_advisory_lock) end - it 'adds with_advisory_lock to ActiveRecord instances' do + test 'adds with_advisory_lock to ActiveRecord instances' do assert_respond_to(Label.new, :with_advisory_lock) end - it 'adds advisory_lock_exists? to ActiveRecord classes' do + test 'adds advisory_lock_exists? to ActiveRecord classes' do assert_respond_to(Tag, :advisory_lock_exists?) end - it 'adds advisory_lock_exists? to ActiveRecord classes' do + test 'adds advisory_lock_exists? to ActiveRecord instances' do assert_respond_to(Label.new, :advisory_lock_exists?) end end -describe 'ActiveRecord query cache' do - it 'does not disable quary cache by default' do +class ActiveRecordQueryCacheTest < GemTestCase + test 'does not disable quary cache by default' do ActiveRecord::Base.expects(:uncached).never - Tag.with_advisory_lock('lock') { Tag.first } end - it 'can disable ActiveRecord query cache' do + test 'can disable ActiveRecord query cache' do ActiveRecord::Base.expects(:uncached).once - Tag.with_advisory_lock('a-lock', disable_query_cache: true) { Tag.first } end end diff --git a/test/lock_test.rb b/test/lock_test.rb index 1a6e583..e43dfcd 100644 --- a/test/lock_test.rb +++ b/test/lock_test.rb @@ -2,92 +2,79 @@ require 'test_helper' -describe 'class methods' do - let(:lock_name) { 'test lock' } - let(:return_val) { 1900 } +class LockTest < GemTestCase + setup do + @lock_name = 'test lock' + @return_val = 1900 + end - describe '.current_advisory_lock' do - it 'returns nil outside an advisory lock request' do - assert_nil(Tag.current_advisory_lock) - end + test 'returns nil outside an advisory lock request' do + assert_nil(Tag.current_advisory_lock) + end - it 'returns the name of the last lock acquired' do - Tag.with_advisory_lock(lock_name) do - # The lock name may have a prefix if WITH_ADVISORY_LOCK_PREFIX env is set - assert_match(/#{lock_name}/, Tag.current_advisory_lock) - end + test 'returns the name of the last lock acquired' do + Tag.with_advisory_lock(@lock_name) do + assert_match(/#{@lock_name}/, Tag.current_advisory_lock) end + end - it 'can obtain a lock with a name that attempts to disrupt a SQL comment' do - dangerous_lock_name = 'test */ lock /*' - Tag.with_advisory_lock(dangerous_lock_name) do - assert_match(/#{Regexp.escape(dangerous_lock_name)}/, Tag.current_advisory_lock) - end + test 'can obtain a lock with a name that attempts to disrupt a SQL comment' do + dangerous_lock_name = 'test */ lock /*' + Tag.with_advisory_lock(dangerous_lock_name) do + assert_match(/#{Regexp.escape(dangerous_lock_name)}/, Tag.current_advisory_lock) end end - describe '.advisory_lock_exists?' do - it 'returns false for an unacquired lock' do - refute(Tag.advisory_lock_exists?(lock_name)) - end + test 'returns false for an unacquired lock' do + refute(Tag.advisory_lock_exists?(@lock_name)) + end - it 'returns the name of the last lock acquired' do - Tag.with_advisory_lock(lock_name) do - assert(Tag.advisory_lock_exists?(lock_name)) - end + test 'returns true for an acquired lock' do + Tag.with_advisory_lock(@lock_name) do + assert(Tag.advisory_lock_exists?(@lock_name)) end end - describe '.with_advisory_lock' do - it 'returns block return value if lock successful' do - assert_equal(return_val, Tag.with_advisory_lock!(lock_name) { return_val }) - end + test 'returns block return value if lock successful' do + assert_equal(@return_val, Tag.with_advisory_lock!(@lock_name) { @return_val }) + end - it 'returns false on lock acquisition failure' do - thread_with_lock = Thread.new do - Tag.with_advisory_lock(lock_name, timeout_seconds: 0) do - @locked_elsewhere = true - sleep 0.01 while true - end + test 'returns false on lock acquisition failure' do + thread_with_lock = Thread.new do + Tag.with_advisory_lock(@lock_name, timeout_seconds: 0) do + @locked_elsewhere = true + loop { sleep 0.01 } end - - sleep 0.01 until @locked_elsewhere - assert_false(Tag.with_advisory_lock(lock_name, timeout_seconds: 0) { return_val }) - - thread_with_lock.kill end - end - describe '.with_advisory_lock!' do - it 'returns block return value if lock successful' do - assert_equal(return_val, Tag.with_advisory_lock!(lock_name) { return_val }) - end + sleep 0.01 until @locked_elsewhere + assert_not(Tag.with_advisory_lock(@lock_name, timeout_seconds: 0) { @return_val }) - it 'raises an error on lock acquisition failure' do - thread_with_lock = Thread.new do - Tag.with_advisory_lock(lock_name, timeout_seconds: 0) do - @locked_elsewhere = true - sleep 0.01 while true - end - end + thread_with_lock.kill + end - sleep 0.01 until @locked_elsewhere - assert_raises(WithAdvisoryLock::FailedToAcquireLock) do - Tag.with_advisory_lock!(lock_name, timeout_seconds: 0) { return_val } + test 'raises an error on lock acquisition failure' do + thread_with_lock = Thread.new do + Tag.with_advisory_lock(@lock_name, timeout_seconds: 0) do + @locked_elsewhere = true + loop { sleep 0.01 } end + end - thread_with_lock.kill + sleep 0.01 until @locked_elsewhere + assert_raises(WithAdvisoryLock::FailedToAcquireLock) do + Tag.with_advisory_lock!(@lock_name, timeout_seconds: 0) { @return_val } end - end - describe 'zero timeout_seconds' do - it 'attempts the lock exactly once with no timeout' do - expected = SecureRandom.base64 - actual = Tag.with_advisory_lock(lock_name, 0) do - expected - end + thread_with_lock.kill + end - assert_equal(expected, actual) + test 'attempts the lock exactly once with no timeout' do + expected = SecureRandom.base64 + actual = Tag.with_advisory_lock(@lock_name, 0) do + expected end + + assert_equal(expected, actual) end end diff --git a/test/nesting_test.rb b/test/nesting_test.rb index d729357..4c2c7a3 100644 --- a/test/nesting_test.rb +++ b/test/nesting_test.rb @@ -2,18 +2,17 @@ require 'test_helper' -describe 'lock nesting' do - # This simplifies what we expect from the lock name: - before :each do +class LockNestingTest < GemTestCase + setup do @prior_prefix = ENV['WITH_ADVISORY_LOCK_PREFIX'] ENV['WITH_ADVISORY_LOCK_PREFIX'] = nil end - after :each do + teardown do ENV['WITH_ADVISORY_LOCK_PREFIX'] = @prior_prefix end - it "doesn't request the same lock twice" do + test "doesn't request the same lock twice" do impl = WithAdvisoryLock::Base.new(nil, nil, nil) assert_empty(impl.lock_stack) Tag.with_advisory_lock('first') do @@ -26,61 +25,4 @@ end assert_empty(impl.lock_stack) end - - it 'does not raise errors with MySQL < 5.7.5 when acquiring nested error force enabled' do - skip unless [:mysql2].include?(env_db) - impl = WithAdvisoryLock::Base.new(nil, nil, nil) - assert_empty(impl.lock_stack) - Tag.with_advisory_lock('first', force_nested_lock_support: true) do - assert_equal(%w[first], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('second', force_nested_lock_support: true) do - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('first', force_nested_lock_support: true) do - # Shouldn't ask for another lock: - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('second', force_nested_lock_support: true) do - # Shouldn't ask for another lock: - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - end - end - end - assert_equal(%w[first], impl.lock_stack.map(&:name)) - end - assert_empty(impl.lock_stack) - end - - it 'supports nested advisory locks with !MySQL 5.6' do - skip if [:mysql2].include? env_db - impl = WithAdvisoryLock::Base.new(nil, nil, nil) - assert_empty(impl.lock_stack) - Tag.with_advisory_lock('first') do - assert_equal(%w[first], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('second') do - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('first') do - # Shouldn't ask for another lock: - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - Tag.with_advisory_lock('second') do - # Shouldn't ask for another lock: - assert_equal(%w[first second], impl.lock_stack.map(&:name)) - end - end - end - assert_equal(%w[first], impl.lock_stack.map(&:name)) - end - assert_empty(impl.lock_stack) - end - - it 'raises with !MySQL 5.6 and nested error force disabled' do - skip unless [:mysql2].include?(env_db) - - exc = assert_raises(WithAdvisoryLock::NestedAdvisoryLockError) do - Tag.with_advisory_lock('first', force_nested_lock_support: false) do - Tag.with_advisory_lock('second', force_nested_lock_support: false) do - end - end - end - - assert_equal(%w[first], exc.lock_stack.map(&:name)) - end end diff --git a/test/options_test.rb b/test/options_test.rb index 74fa346..d987417 100644 --- a/test/options_test.rb +++ b/test/options_test.rb @@ -2,65 +2,65 @@ require 'test_helper' -describe 'options parsing' do +class OptionsParsingTest < GemTestCase def parse_options(options) WithAdvisoryLock::Base.new(mock, mock, options) end - specify 'defaults (empty hash)' do + test 'defaults (empty hash)' do impl = parse_options({}) assert_nil(impl.timeout_seconds) - refute(impl.shared) - refute(impl.transaction) + assert_not(impl.shared) + assert_not(impl.transaction) end - specify 'nil sets timeout to nil' do + test 'nil sets timeout to nil' do impl = parse_options(nil) assert_nil(impl.timeout_seconds) - refute(impl.shared) - refute(impl.transaction) + assert_not(impl.shared) + assert_not(impl.transaction) end - specify 'integer sets timeout to value' do + test 'integer sets timeout to value' do impl = parse_options(42) assert_equal(42, impl.timeout_seconds) - refute(impl.shared) - refute(impl.transaction) + assert_not(impl.shared) + assert_not(impl.transaction) end - specify 'hash with invalid key errors' do + test 'hash with invalid key errors' do assert_raises(ArgumentError) do parse_options(foo: 42) end end - specify 'hash with timeout_seconds sets timeout to value' do + test 'hash with timeout_seconds sets timeout to value' do impl = parse_options(timeout_seconds: 123) assert_equal(123, impl.timeout_seconds) - refute(impl.shared) - refute(impl.transaction) + assert_not(impl.shared) + assert_not(impl.transaction) end - specify 'hash with shared option sets shared to true' do + test 'hash with shared option sets shared to true' do impl = parse_options(shared: true) assert_nil(impl.timeout_seconds) assert(impl.shared) - refute(impl.transaction) + assert_not(impl.transaction) end - specify 'hash with transaction option set transaction to true' do + test 'hash with transaction option set transaction to true' do impl = parse_options(transaction: true) assert_nil(impl.timeout_seconds) - refute(impl.shared) + assert_not(impl.shared) assert(impl.transaction) end - specify 'hash with multiple keys sets options' do + test 'hash with multiple keys sets options' do foo = mock bar = mock impl = parse_options(timeout_seconds: foo, shared: bar) assert_equal(foo, impl.timeout_seconds) assert_equal(bar, impl.shared) - refute(impl.transaction) + assert_not(impl.transaction) end end diff --git a/test/parallelism_test.rb b/test/parallelism_test.rb index 4b1ed86..d8eb688 100644 --- a/test/parallelism_test.rb +++ b/test/parallelism_test.rb @@ -3,35 +3,35 @@ require 'test_helper' require 'forwardable' -describe 'parallelism' do - class FindOrCreateWorker - extend Forwardable - def_delegators :@thread, :join, :wakeup, :status, :to_s +class FindOrCreateWorker + extend Forwardable + def_delegators :@thread, :join, :wakeup, :status, :to_s - def initialize(name, use_advisory_lock) - @name = name - @use_advisory_lock = use_advisory_lock - @thread = Thread.new { work_later } - end + def initialize(name, use_advisory_lock) + @name = name + @use_advisory_lock = use_advisory_lock + @thread = Thread.new { work_later } + end - def work_later - sleep - ActiveRecord::Base.connection_pool.with_connection do - if @use_advisory_lock - Tag.with_advisory_lock(@name) { work } - else - work - end + def work_later + sleep + ActiveRecord::Base.connection_pool.with_connection do + if @use_advisory_lock + Tag.with_advisory_lock(@name) { work } + else + work end end + end - def work - Tag.transaction do - Tag.where(name: @name).first_or_create - end + def work + Tag.transaction do + Tag.where(name: @name).first_or_create end end +end +class ParallelismTest < GemTestCase def run_workers @names = @iterations.times.map { |iter| "iteration ##{iter}" } @names.each do |name| @@ -49,14 +49,12 @@ def run_workers ActiveRecord::Base.connection_pool.connection end - before :each do + setup do ActiveRecord::Base.connection.reconnect! @workers = 10 end - # < SQLite, understandably, throws "The database file is locked (database is locked)" - - it 'creates multiple duplicate rows without advisory locks' do + test 'creates multiple duplicate rows without advisory locks' do skip if %i[sqlite3 jdbcsqlite3].include?(env_db) @use_advisory_lock = false @iterations = 1 @@ -66,7 +64,7 @@ def run_workers assert_operator(Label.all.size, :>, @iterations) # <- any duplicated rows will make me happy. end - it "doesn't create multiple duplicate rows with advisory locks" do + test "doesn't create multiple duplicate rows with advisory locks" do @use_advisory_lock = true @iterations = 10 run_workers diff --git a/test/shared_test.rb b/test/shared_test.rb index 32b3269..83a0c1d 100644 --- a/test/shared_test.rb +++ b/test/shared_test.rb @@ -1,47 +1,46 @@ # frozen_string_literal: true require 'test_helper' +class SharedTestWorker + def initialize(shared) + @shared = shared -describe 'shared locks' do - def supported? - %i[mysql2 jdbcmysql].exclude?(env_db) + @locked = nil + @cleanup = false + @thread = Thread.new { work } end - class SharedTestWorker - def initialize(shared) - @shared = shared - - @locked = nil - @cleanup = false - @thread = Thread.new { work } - end - - def locked? - sleep 0.01 while @locked.nil? && @thread.alive? - @locked - end + def locked? + sleep 0.01 while @locked.nil? && @thread.alive? + @locked + end - def cleanup! - @cleanup = true - @thread.join - raise if @thread.status.nil? - end + def cleanup! + @cleanup = true + @thread.join + raise if @thread.status.nil? + end - private + private - def work - ActiveRecord::Base.connection_pool.with_connection do - Tag.with_advisory_lock('test', timeout_seconds: 0, shared: @shared) do - @locked = true - sleep 0.01 until @cleanup - end - @locked = false + def work + ActiveRecord::Base.connection_pool.with_connection do + Tag.with_advisory_lock('test', timeout_seconds: 0, shared: @shared) do + @locked = true sleep 0.01 until @cleanup end + @locked = false + sleep 0.01 until @cleanup end end +end + +class SharedLocksTest < GemTestCase + def supported? + %i[trilogy mysql2 jdbcmysql].exclude?(env_db) + end - it 'does not allow two exclusive locks' do + test 'does not allow two exclusive locks' do one = SharedTestWorker.new(false) assert_predicate(one, :locked?) @@ -51,85 +50,85 @@ def work one.cleanup! two.cleanup! end +end - describe 'not supported' do - before do - skip if supported? - end - - it 'raises an error when attempting to use a shared lock' do - one = SharedTestWorker.new(true) - assert_nil(one.locked?) +class NotSupportedEnvironmentTest < SharedLocksTest + setup do + skip if supported? + end - exception = assert_raises(ArgumentError) do - one.cleanup! - end + test 'raises an error when attempting to use a shared lock' do + one = SharedTestWorker.new(true) + assert_nil(one.locked?) - assert_match(/#{Regexp.escape('not supported')}/, exception.message) + exception = assert_raises(ArgumentError) do + one.cleanup! end + + assert_match(/#{Regexp.escape('not supported')}/, exception.message) end +end - describe 'supported' do - before do - skip unless supported? - end +class SupportedEnvironmentTest < SharedLocksTest + setup do + skip unless supported? + end - it 'does allow two shared locks' do - one = SharedTestWorker.new(true) - assert_predicate(one, :locked?) + test 'does allow two shared locks' do + one = SharedTestWorker.new(true) + assert_predicate(one, :locked?) - two = SharedTestWorker.new(true) - assert_predicate(two, :locked?) + two = SharedTestWorker.new(true) + assert_predicate(two, :locked?) - one.cleanup! - two.cleanup! - end + one.cleanup! + two.cleanup! + end - it 'does not allow exclusive lock with shared lock' do - one = SharedTestWorker.new(true) - assert_predicate(one, :locked?) + test 'does not allow exclusive lock with shared lock' do + one = SharedTestWorker.new(true) + assert_predicate(one, :locked?) - two = SharedTestWorker.new(false) - refute(two.locked?) + two = SharedTestWorker.new(false) + refute(two.locked?) - three = SharedTestWorker.new(true) - assert_predicate(three, :locked?) + three = SharedTestWorker.new(true) + assert_predicate(three, :locked?) - one.cleanup! - two.cleanup! - three.cleanup! - end + one.cleanup! + two.cleanup! + three.cleanup! + end - it 'does not allow shared lock with exclusive lock' do - one = SharedTestWorker.new(false) - assert_predicate(one, :locked?) + test 'does not allow shared lock with exclusive lock' do + one = SharedTestWorker.new(false) + assert_predicate(one, :locked?) - two = SharedTestWorker.new(true) - refute(two.locked?) + two = SharedTestWorker.new(true) + refute(two.locked?) - one.cleanup! - two.cleanup! - end + one.cleanup! + two.cleanup! + end - describe 'PostgreSQL' do - before do - skip unless env_db == :postgresql - end + class PostgreSQLTest < SupportedEnvironmentTest + setup do + skip unless env_db == :postgresql + end - def pg_lock_modes - ActiveRecord::Base.connection.select_values("SELECT mode FROM pg_locks WHERE locktype = 'advisory';") - end + def pg_lock_modes + ActiveRecord::Base.connection.select_values("SELECT mode FROM pg_locks WHERE locktype = 'advisory';") + end - it 'allows shared lock to be upgraded to an exclusive lock' do - assert_empty(pg_lock_modes) - Tag.with_advisory_lock 'test', shared: true do - assert_equal(%w[ShareLock], pg_lock_modes) - Tag.with_advisory_lock 'test', shared: false do - assert_equal(%w[ShareLock ExclusiveLock], pg_lock_modes) - end + test 'allows shared lock to be upgraded to an exclusive lock' do + assert_empty(pg_lock_modes) + Tag.with_advisory_lock 'test', shared: true do + assert_equal(%w[ShareLock], pg_lock_modes) + Tag.with_advisory_lock 'test', shared: false do + assert_equal(%w[ShareLock ExclusiveLock], pg_lock_modes) end - assert_empty(pg_lock_modes) end + assert_empty(pg_lock_modes) end end end diff --git a/test/test_helper.rb b/test/test_helper.rb index f28bb1b..2a8c43f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -5,6 +5,15 @@ require 'with_advisory_lock' require 'tmpdir' require 'securerandom' +begin + require 'activerecord-trilogy-adapter' + ActiveSupport.on_load(:active_record) do + require "trilogy_adapter/connection" + ActiveRecord::Base.public_send :extend, TrilogyAdapter::Connection + end +rescue LoadError + # do nothing +end ActiveRecord::Base.configurations = { default_env: { @@ -18,11 +27,7 @@ ActiveRecord::Base.establish_connection def env_db - @env_db ||= if ActiveRecord::Base.respond_to?(:connection_db_config) - ActiveRecord::Base.connection_db_config.adapter - else - ActiveRecord::Base.connection_config[:adapter] - end.to_sym + @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym end ActiveRecord::Migration.verbose = false @@ -30,20 +35,18 @@ def env_db require 'test_models' require 'minitest' require 'maxitest/autorun' -require 'minitest/great_expectations' require 'mocha/minitest' -puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" -module MiniTest - class Spec - before do - ENV['FLOCK_DIR'] = Dir.mktmpdir - Tag.delete_all - TagAudit.delete_all - Label.delete_all - end - after do - FileUtils.remove_entry_secure ENV['FLOCK_DIR'] - end +class GemTestCase < ActiveSupport::TestCase + setup do + ENV['FLOCK_DIR'] = Dir.mktmpdir + Tag.delete_all + TagAudit.delete_all + Label.delete_all + end + teardown do + FileUtils.remove_entry_secure ENV['FLOCK_DIR'] end end + +puts "Testing with #{env_db} database, ActiveRecord #{ActiveRecord.gem_version} and #{RUBY_ENGINE} #{RUBY_ENGINE_VERSION} as #{RUBY_VERSION}" diff --git a/test/thread_test.rb b/test/thread_test.rb index 0c486e1..e4156bf 100644 --- a/test/thread_test.rb +++ b/test/thread_test.rb @@ -2,17 +2,16 @@ require 'test_helper' -describe 'separate thread tests' do - let(:lock_name) { 'testing 1,2,3' } # OMG COMMAS - - before do +class SeparateThreadTest < GemTestCase + setup do + @lock_name = 'testing 1,2,3' # OMG COMMAS @mutex = Mutex.new @t1_acquired_lock = false @t1_return_value = nil @t1 = Thread.new do ActiveRecord::Base.connection_pool.with_connection do - @t1_return_value = Label.with_advisory_lock(lock_name) do + @t1_return_value = Label.with_advisory_lock(@lock_name) do @mutex.synchronize { @t1_acquired_lock = true } sleep 't1 finished' @@ -25,34 +24,34 @@ ActiveRecord::Base.connection.reconnect! end - after do + teardown do @t1.wakeup if @t1.status == 'sleep' @t1.join end - it '#with_advisory_lock with a 0 timeout returns false immediately' do - response = Label.with_advisory_lock(lock_name, 0) do + test '#with_advisory_lock with a 0 timeout returns false immediately' do + response = Label.with_advisory_lock(@lock_name, 0) do raise 'should not be yielded to' end - refute(response) + assert_not(response) end - it '#with_advisory_lock yields to the provided block' do + test '#with_advisory_lock yields to the provided block' do assert(@t1_acquired_lock) end - it '#advisory_lock_exists? returns true when another thread has the lock' do - assert(Tag.advisory_lock_exists?(lock_name)) + test '#advisory_lock_exists? returns true when another thread has the lock' do + assert(Tag.advisory_lock_exists?(@lock_name)) end - it 'can re-establish the lock after the other thread releases it' do + test 'can re-establish the lock after the other thread releases it' do @t1.wakeup @t1.join assert_equal('t1 finished', @t1_return_value) # We should now be able to acquire the lock immediately: reacquired = false - lock_result = Label.with_advisory_lock(lock_name, 0) do + lock_result = Label.with_advisory_lock(@lock_name, 0) do reacquired = true end diff --git a/test/transaction_test.rb b/test/transaction_test.rb index e0e224f..e7d951b 100644 --- a/test/transaction_test.rb +++ b/test/transaction_test.rb @@ -2,51 +2,46 @@ require 'test_helper' -describe 'transaction scoping' do +class TransactionScopingTest < GemTestCase def supported? %i[postgresql jdbcpostgresql].include?(env_db) end - describe 'not supported' do - before do - skip if supported? - end + test 'raises an error when attempting to use transaction level locks if not supported' do + skip if supported? - it 'raises an error when attempting to use transaction level locks' do - Tag.transaction do - exception = assert_raises(ArgumentError) do - Tag.with_advisory_lock 'test', transaction: true do - raise 'should not get here' - end + Tag.transaction do + exception = assert_raises(ArgumentError) do + Tag.with_advisory_lock 'test', transaction: true do + raise 'should not get here' end - - assert_match(/#{Regexp.escape('not supported')}/, exception.message) end + + assert_match(/#{Regexp.escape('not supported')}/, exception.message) end end - describe 'supported' do - before do + class PostgresqlTest < TransactionScopingTest + setup do skip unless env_db == :postgresql + @pg_lock_count = lambda do + ActiveRecord::Base.connection.select_value("SELECT COUNT(*) FROM pg_locks WHERE locktype = 'advisory';").to_i + end end - def pg_lock_count - ActiveRecord::Base.connection.select_value("SELECT COUNT(*) FROM pg_locks WHERE locktype = 'advisory';").to_i - end - - specify 'session locks release after the block executes' do + test 'session locks release after the block executes' do Tag.transaction do - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) Tag.with_advisory_lock 'test' do - assert_equal(1, pg_lock_count) + assert_equal(1, @pg_lock_count.call) end - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) end end - specify 'session locks release when transaction fails inside block' do + test 'session locks release when transaction fails inside block' do Tag.transaction do - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) exception = assert_raises(ActiveRecord::StatementInvalid) do Tag.with_advisory_lock 'test' do @@ -55,19 +50,19 @@ def pg_lock_count end assert_match(/#{Regexp.escape('division by zero')}/, exception.message) - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) end end - specify 'transaction level locks hold until the transaction completes' do + test 'transaction level locks hold until the transaction completes' do Tag.transaction do - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) Tag.with_advisory_lock 'test', transaction: true do - assert_equal(1, pg_lock_count) + assert_equal(1, @pg_lock_count.call) end - assert_equal(1, pg_lock_count) + assert_equal(1, @pg_lock_count.call) end - assert_equal(0, pg_lock_count) + assert_equal(0, @pg_lock_count.call) end end end diff --git a/with_advisory_lock.gemspec b/with_advisory_lock.gemspec index 055b182..502e191 100644 --- a/with_advisory_lock.gemspec +++ b/with_advisory_lock.gemspec @@ -1,8 +1,6 @@ # frozen_string_literal: true require 'English' -lib = File.expand_path('lib', __dir__) -$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require_relative 'lib/with_advisory_lock/version' Gem::Specification.new do |spec| @@ -19,14 +17,13 @@ Gem::Specification.new do |spec| spec.test_files = spec.files.grep(%r{^test/}) spec.require_paths = %w[lib] spec.metadata = { "rubyspecs_mfa_required" => "true" } - spec.required_ruby_version = '>= 2.6.8' + spec.required_ruby_version = '>= 2.7.0' spec.metadata["yard.run"] = "yri" - spec.add_runtime_dependency 'activerecord', '>= 6.0' + spec.add_runtime_dependency 'activerecord', '>= 6.1' spec.add_development_dependency 'appraisal' spec.add_development_dependency 'maxitest' - spec.add_development_dependency 'minitest-great_expectations' spec.add_development_dependency 'minitest-reporters' spec.add_development_dependency 'mocha' spec.add_development_dependency 'yard'