From 3967d4fc730470a013292395fd9d21b653125896 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sun, 11 Feb 2024 17:46:29 +0100 Subject: [PATCH 1/4] chore: refactor test to use multi database syntax. model.connection should be used instead of ActiveRecord::Base.connection --- test/test_helper.rb | 4 +--- test/test_models.rb | 2 ++ test/with_advisory_lock/parallelism_test.rb | 8 ++++---- test/with_advisory_lock/shared_test.rb | 21 ++++++++++++--------- test/with_advisory_lock/transaction_test.rb | 4 ++-- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 14b0d01..f241feb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -24,10 +24,8 @@ ENV['WITH_ADVISORY_LOCK_PREFIX'] ||= SecureRandom.hex -ActiveRecord::Base.establish_connection - def env_db - @env_db ||= ActiveRecord::Base.connection_db_config.adapter.to_sym + @env_db ||= ActiveRecord::Base.configurations.find_db_config(:default_env).adapter end ActiveRecord::Migration.verbose = false diff --git a/test/test_models.rb b/test/test_models.rb index 8d6fda1..c3cc8fa 100644 --- a/test/test_models.rb +++ b/test/test_models.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +ActiveRecord::Base.establish_connection(:default_env) + ActiveRecord::Schema.define(version: 1) do create_table 'tags', force: true do |t| t.string 'name' diff --git a/test/with_advisory_lock/parallelism_test.rb b/test/with_advisory_lock/parallelism_test.rb index 56f6a20..aa9cd26 100644 --- a/test/with_advisory_lock/parallelism_test.rb +++ b/test/with_advisory_lock/parallelism_test.rb @@ -15,7 +15,7 @@ def initialize(name, use_advisory_lock) def work_later sleep - ApplicationRecord.connection_pool.with_connection do + Tag.connection_pool.with_connection do if @use_advisory_lock Tag.with_advisory_lock(@name) { work } else @@ -46,16 +46,16 @@ def run_workers workers.each(&:join) end # Ensure we're still connected: - ApplicationRecord.connection_pool.connection + Tag.connection_pool.connection end setup do - ApplicationRecord.connection.reconnect! + Tag.connection.reconnect! @workers = 10 end test 'creates multiple duplicate rows without advisory locks' do - skip if %i[sqlite3 jdbcsqlite3].include?(env_db) + skip if is_sqlite3_adapter? @use_advisory_lock = false @iterations = 1 run_workers diff --git a/test/with_advisory_lock/shared_test.rb b/test/with_advisory_lock/shared_test.rb index 7cd23a4..7e22293 100644 --- a/test/with_advisory_lock/shared_test.rb +++ b/test/with_advisory_lock/shared_test.rb @@ -24,7 +24,7 @@ def cleanup! private def work - ApplicationRecord.connection_pool.with_connection do + Tag.connection_pool.with_connection do |connection| Tag.with_advisory_lock('test', timeout_seconds: 0, shared: @shared) do @locked = true sleep 0.01 until @cleanup @@ -36,9 +36,6 @@ def work end class SharedLocksTest < GemTestCase - def supported? - %i[trilogy mysql2 jdbcmysql].exclude?(env_db) - end test 'does not allow two exclusive locks' do one = SharedTestWorker.new(false) @@ -52,12 +49,14 @@ def supported? end end -class NotSupportedEnvironmentTest < SharedLocksTest - setup do - skip if supported? +class NotSupportedEnvironmentTest < GemTestCase + def supported? + !is_mysql_adapter? end test 'raises an error when attempting to use a shared lock' do + skip "Not supported" unless supported? + one = SharedTestWorker.new(true) assert_nil(one.locked?) @@ -69,7 +68,11 @@ class NotSupportedEnvironmentTest < SharedLocksTest end end -class SupportedEnvironmentTest < SharedLocksTest +class SupportedEnvironmentTest < GemTestCase + def supported? + !is_mysql_adapter? + end + setup do skip unless supported? end @@ -113,7 +116,7 @@ class SupportedEnvironmentTest < SharedLocksTest class PostgreSQLTest < SupportedEnvironmentTest setup do - skip unless env_db == :postgresql + skip unless is_postgresql_adapter? end def pg_lock_modes diff --git a/test/with_advisory_lock/transaction_test.rb b/test/with_advisory_lock/transaction_test.rb index 559f55c..0dbedc7 100644 --- a/test/with_advisory_lock/transaction_test.rb +++ b/test/with_advisory_lock/transaction_test.rb @@ -4,7 +4,7 @@ class TransactionScopingTest < GemTestCase def supported? - %i[postgresql jdbcpostgresql].include?(env_db) + is_postgresql_adapter? end test 'raises an error when attempting to use transaction level locks if not supported' do @@ -23,7 +23,7 @@ def supported? class PostgresqlTest < TransactionScopingTest setup do - skip unless env_db == :postgresql + skip unless is_postgresql_adapter? @pg_lock_count = lambda do ApplicationRecord.connection.select_value("SELECT COUNT(*) FROM pg_locks WHERE locktype = 'advisory';").to_i end From 8769c65fc73e01100bb4ca0f28b8b76f8cfb985b Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sun, 11 Feb 2024 19:09:12 +0100 Subject: [PATCH 2/4] add Makefile to test --- Makefile | 14 ++++++++++++++ docker-compose.yml | 20 ++++++++++++++++++++ test/test_helper.rb | 1 + test/with_advisory_lock/shared_test.rb | 2 +- test/with_advisory_lock/thread_test.rb | 4 ++-- 5 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 Makefile create mode 100644 docker-compose.yml diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..84bc7e4 --- /dev/null +++ b/Makefile @@ -0,0 +1,14 @@ +.PHONY: test-pg test-mysql + +test-pg: + docker compose up -d pg + sleep 10 # give some time for the service to start + DATABASE_URL=postgres://with_advisory:with_advisory_pass@localhost/with_advisory_lock_test appraisal activerecord-7.1 rake test + +test-mysql: + docker compose up -d mysql + sleep 10 # give some time for the service to start + DATABASE_URL=mysql2://with_advisory:with_advisory_pass@0.0.0.0:3306/with_advisory_lock_test appraisal activerecord-7.1 rake test + + +test: test-pg test-mysql \ No newline at end of file diff --git a/docker-compose.yml b/docker-compose.yml new file mode 100644 index 0000000..11ca78b --- /dev/null +++ b/docker-compose.yml @@ -0,0 +1,20 @@ +version: "3.9" +services: + pg: + image: postgres:16 + environment: + POSTGRES_USER: with_advisory + POSTGRES_PASSWORD: with_advisory_pass + POSTGRES_DB: with_advisory_lock_test + ports: + - "5432:5432" + mysql: + image: mysql:8 + environment: + MYSQL_USER: with_advisory + MYSQL_PASSWORD: with_advisory_pass + MYSQL_DATABASE: with_advisory_lock_test + MYSQL_RANDOM_ROOT_PASSWORD: "yes" + MYSQL_ROOT_HOST: '%' + ports: + - "3306:3306" \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index f241feb..3b4065f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -18,6 +18,7 @@ ActiveRecord::Base.configurations = { default_env: { url: ENV.fetch('DATABASE_URL', "sqlite3://#{Dir.tmpdir}/with_advisory_lock_test#{RUBY_VERSION}-#{ActiveRecord.gem_version}.sqlite3"), + pool: 11, properties: { allowPublicKeyRetrieval: true } # for JRuby madness } } diff --git a/test/with_advisory_lock/shared_test.rb b/test/with_advisory_lock/shared_test.rb index 7e22293..0478f0d 100644 --- a/test/with_advisory_lock/shared_test.rb +++ b/test/with_advisory_lock/shared_test.rb @@ -24,7 +24,7 @@ def cleanup! private def work - Tag.connection_pool.with_connection do |connection| + Tag.connection_pool.with_connection do Tag.with_advisory_lock('test', timeout_seconds: 0, shared: @shared) do @locked = true sleep 0.01 until @cleanup diff --git a/test/with_advisory_lock/thread_test.rb b/test/with_advisory_lock/thread_test.rb index 2c86be0..4e00f27 100644 --- a/test/with_advisory_lock/thread_test.rb +++ b/test/with_advisory_lock/thread_test.rb @@ -10,7 +10,7 @@ class SeparateThreadTest < GemTestCase @t1_return_value = nil @t1 = Thread.new do - ApplicationRecord.connection_pool.with_connection do + Label.connection_pool.with_connection do @t1_return_value = Label.with_advisory_lock(@lock_name) do @mutex.synchronize { @t1_acquired_lock = true } sleep @@ -21,7 +21,7 @@ class SeparateThreadTest < GemTestCase # Wait for the thread to acquire the lock: sleep(0.1) until @mutex.synchronize { @t1_acquired_lock } - ApplicationRecord.connection.reconnect! + Label.connection.reconnect! end teardown do From 14c40fc275591b5091093e83f3e3672748a62e87 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sun, 11 Feb 2024 21:51:11 +0100 Subject: [PATCH 3/4] chore: add small test to check if adapter support advisory_locks natively (#96) --- test/with_advisory_lock/base_test.rb | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 test/with_advisory_lock/base_test.rb diff --git a/test/with_advisory_lock/base_test.rb b/test/with_advisory_lock/base_test.rb new file mode 100644 index 0000000..691dc88 --- /dev/null +++ b/test/with_advisory_lock/base_test.rb @@ -0,0 +1,9 @@ +require 'test_helper' + +class WithAdvisoryLockBaseTest < GemTestCase + test 'should support advisory_locks_enabled' do + skip if is_sqlite3_adapter? + + assert Tag.connection.advisory_locks_enabled? + end +end From 3b505943a5d492621e6b28156a14ec6093d2d711 Mon Sep 17 00:00:00 2001 From: Abdelkader Boudih Date: Sun, 11 Feb 2024 22:23:54 +0100 Subject: [PATCH 4/4] cleanup mysql implementation --- Makefile | 11 +++++++---- docker-compose.yml | 2 +- lib/with_advisory_lock/database_adapter_support.rb | 5 +++-- lib/with_advisory_lock/mysql.rb | 7 ++++--- test/test_helper.rb | 1 - 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Makefile b/Makefile index 84bc7e4..797788b 100644 --- a/Makefile +++ b/Makefile @@ -1,14 +1,17 @@ -.PHONY: test-pg test-mysql +.PHONY: test-sqlite test-pg test-mysql + +test-sqlite: + appraisal rake test test-pg: docker compose up -d pg sleep 10 # give some time for the service to start - DATABASE_URL=postgres://with_advisory:with_advisory_pass@localhost/with_advisory_lock_test appraisal activerecord-7.1 rake test + DATABASE_URL=postgres://with_advisory:with_advisory_pass@localhost/with_advisory_lock_test appraisal rake test test-mysql: docker compose up -d mysql sleep 10 # give some time for the service to start - DATABASE_URL=mysql2://with_advisory:with_advisory_pass@0.0.0.0:3306/with_advisory_lock_test appraisal activerecord-7.1 rake test + DATABASE_URL=mysql2://with_advisory:with_advisory_pass@0.0.0.0:3306/with_advisory_lock_test appraisal rake test -test: test-pg test-mysql \ No newline at end of file +test: test-sqlite test-pg test-mysql diff --git a/docker-compose.yml b/docker-compose.yml index 11ca78b..2440305 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -17,4 +17,4 @@ services: MYSQL_RANDOM_ROOT_PASSWORD: "yes" MYSQL_ROOT_HOST: '%' ports: - - "3306:3306" \ No newline at end of file + - "3306:3306" diff --git a/lib/with_advisory_lock/database_adapter_support.rb b/lib/with_advisory_lock/database_adapter_support.rb index dafd3ed..680bb68 100644 --- a/lib/with_advisory_lock/database_adapter_support.rb +++ b/lib/with_advisory_lock/database_adapter_support.rb @@ -3,9 +3,10 @@ module WithAdvisoryLock class DatabaseAdapterSupport attr_reader :adapter_name + def initialize(connection) @connection = connection - @adapter_name = connection.adapter_name.downcase.to_sym + @adapter_name = connection.adapter_name.downcase.to_sym end def mysql? @@ -17,7 +18,7 @@ def postgresql? end def sqlite? - [:sqlite3, :sqlite].include? adapter_name + %i[sqlite3 sqlite].include? adapter_name end end end diff --git a/lib/with_advisory_lock/mysql.rb b/lib/with_advisory_lock/mysql.rb index bdb35e7..9f2a810 100644 --- a/lib/with_advisory_lock/mysql.rb +++ b/lib/with_advisory_lock/mysql.rb @@ -5,7 +5,8 @@ class MySQL < Base # Caches nested lock support by MySQL reported version @@mysql_nl_cache = {} @@mysql_nl_cache_mutex = Mutex.new - # See https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock + # See https://dev.mysql.com/doc/refman/5.7/en/locking-functions.html + # See https://dev.mysql.com/doc/refman/8.0/en/locking-functions.html def try_lock raise ArgumentError, 'shared locks are not supported on MySQL' if shared raise ArgumentError, 'transaction level locks are not supported on MySQL' if transaction @@ -18,8 +19,8 @@ def release_lock end def execute_successful?(mysql_function) - sql = "SELECT #{mysql_function} AS #{unique_column_name}" - connection.select_value(sql).to_i.positive? + sql = "SELECT #{mysql_function}" + connection.query_value(sql) == 1 end # MySQL wants a string as the lock key. diff --git a/test/test_helper.rb b/test/test_helper.rb index 3b4065f..f241feb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -18,7 +18,6 @@ ActiveRecord::Base.configurations = { default_env: { url: ENV.fetch('DATABASE_URL', "sqlite3://#{Dir.tmpdir}/with_advisory_lock_test#{RUBY_VERSION}-#{ActiveRecord.gem_version}.sqlite3"), - pool: 11, properties: { allowPublicKeyRetrieval: true } # for JRuby madness } }