Merge pull request #22330 from samphilipd/samphilipd/rename_advisory_lock_key

Rename 'key' to 'lock_id' or 'lock_name' for advisory locking

8 files changed, 65 insertions, 65 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 55910865e5..4d4dc07b04 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -289,14 +289,14 @@ module ActiveRecord
       # locks
       #
       # Return true if we got the lock, otherwise false
-      def get_advisory_lock(key) # :nodoc:
+      def get_advisory_lock(lock_id) # :nodoc:
       end
 
       # This is meant to be implemented by the adapters that support advisory
       # locks.
       #
       # Return true if we released the lock, otherwise false
-      def release_advisory_lock(key) # :nodoc:
+      def release_advisory_lock(lock_id) # :nodoc:
       end
 
       # A list of extensions, to be filled in by adapters that support them.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index c75c9ab3b5..deef246c37 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -224,12 +224,12 @@ module ActiveRecord
         version >= '5.0.0'
       end
 
-      def get_advisory_lock(key, timeout = 0) # :nodoc:
-        select_value("SELECT GET_LOCK('#{key}', #{timeout});").to_s == '1'
+      def get_advisory_lock(lock_name, timeout = 0) # :nodoc:
+        select_value("SELECT GET_LOCK('#{lock_name}', #{timeout});").to_s == '1'
       end
 
-      def release_advisory_lock(key) # :nodoc:
-        select_value("SELECT RELEASE_LOCK('#{key}')").to_s == '1'
+      def release_advisory_lock(lock_name) # :nodoc:
+        select_value("SELECT RELEASE_LOCK('#{lock_name}')").to_s == '1'
       end
 
       def native_database_types
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 3eefab8f44..719592349b 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -304,18 +304,18 @@ module ActiveRecord
         postgresql_version >= 90300
       end
 
-      def get_advisory_lock(key) # :nodoc:
-        unless key.is_a?(Integer) && key.bit_length <= 63
-          raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer")
+      def get_advisory_lock(lock_id) # :nodoc:
+        unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
+          raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer")
         end
-        select_value("SELECT pg_try_advisory_lock(#{key});")
+        select_value("SELECT pg_try_advisory_lock(#{lock_id});")
       end
 
-      def release_advisory_lock(key) # :nodoc:
-        unless key.is_a?(Integer) && key.bit_length <= 63
-          raise(ArgumentError, "Postgres requires advisory lock keys to be a signed 64 bit integer")
+      def release_advisory_lock(lock_id) # :nodoc:
+        unless lock_id.is_a?(Integer) && lock_id.bit_length <= 63
+          raise(ArgumentError, "Postgres requires advisory lock ids to be a signed 64 bit integer")
         end
-        select_value("SELECT pg_advisory_unlock(#{key})")
+        select_value("SELECT pg_advisory_unlock(#{lock_id})")
       end
 
       def enable_extension(name)
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index ffd243a517..ca2537cdc3 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -1202,17 +1202,17 @@ module ActiveRecord
     end
 
     def with_advisory_lock
-      key = generate_migrator_advisory_lock_key
-      got_lock = Base.connection.get_advisory_lock(key)
+      lock_id = generate_migrator_advisory_lock_id
+      got_lock = Base.connection.get_advisory_lock(lock_id)
       raise ConcurrentMigrationError unless got_lock
       load_migrated # reload schema_migrations to be sure it wasn't changed by another process before we got the lock
       yield
     ensure
-      Base.connection.release_advisory_lock(key) if got_lock
+      Base.connection.release_advisory_lock(lock_id) if got_lock
     end
 
     MIGRATOR_SALT = 2053462845
-    def generate_migrator_advisory_lock_key
+    def generate_migrator_advisory_lock_id
       db_name_hash = Zlib.crc32(Base.connection.current_database)
       MIGRATOR_SALT * db_name_hash
     end
diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb
index 75653ee9af..390dd15b92 100644
--- a/activerecord/test/cases/adapters/mysql/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql/connection_test.rb
@@ -171,31 +171,31 @@ class MysqlConnectionTest < ActiveRecord::MysqlTestCase
   end
 
   def test_get_and_release_advisory_lock
-    key = "test_key"
+    lock_name = "test_lock_name"
 
-    got_lock = @connection.get_advisory_lock(key)
+    got_lock = @connection.get_advisory_lock(lock_name)
     assert got_lock, "get_advisory_lock should have returned true but it didn't"
 
-    assert_equal test_lock_free(key), false,
+    assert_equal test_lock_free(lock_name), false,
       "expected the test advisory lock to be held but it wasn't"
 
-    released_lock = @connection.release_advisory_lock(key)
+    released_lock = @connection.release_advisory_lock(lock_name)
     assert released_lock, "expected release_advisory_lock to return true but it didn't"
 
-    assert test_lock_free(key), 'expected the test key to be available after releasing'
+    assert test_lock_free(lock_name), 'expected the test lock to be available after releasing'
   end
 
   def test_release_non_existent_advisory_lock
-    fake_key = "fake_key"
-    released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+    lock_name = "fake_lock_name"
+    released_non_existent_lock = @connection.release_advisory_lock(lock_name)
     assert_equal released_non_existent_lock, false,
       'expected release_advisory_lock to return false when there was no lock to release'
   end
 
   protected
 
-  def test_lock_free(key)
-    @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == '1'
+  def test_lock_free(lock_name)
+    @connection.select_value("SELECT IS_FREE_LOCK('#{lock_name}');") == '1'
   end
 
   private
diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb
index 71c4028675..507d024bb6 100644
--- a/activerecord/test/cases/adapters/mysql2/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb
@@ -133,30 +133,30 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase
   end
 
   def test_get_and_release_advisory_lock
-    key = "test_key"
+    lock_name = "test_lock_name"
 
-    got_lock = @connection.get_advisory_lock(key)
+    got_lock = @connection.get_advisory_lock(lock_name)
     assert got_lock, "get_advisory_lock should have returned true but it didn't"
 
-    assert_equal test_lock_free(key), false,
+    assert_equal test_lock_free(lock_name), false,
       "expected the test advisory lock to be held but it wasn't"
 
-    released_lock = @connection.release_advisory_lock(key)
+    released_lock = @connection.release_advisory_lock(lock_name)
     assert released_lock, "expected release_advisory_lock to return true but it didn't"
 
-    assert test_lock_free(key), 'expected the test key to be available after releasing'
+    assert test_lock_free(lock_name), 'expected the test lock to be available after releasing'
   end
 
   def test_release_non_existent_advisory_lock
-    fake_key = "fake_key"
-    released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+    lock_name = "fake_lock_name"
+    released_non_existent_lock = @connection.release_advisory_lock(lock_name)
     assert_equal released_non_existent_lock, false,
       'expected release_advisory_lock to return false when there was no lock to release'
   end
 
   protected
 
-  def test_lock_free(key)
-    @connection.select_value("SELECT IS_FREE_LOCK('#{key}');") == 1
+  def test_lock_free(lock_name)
+    @connection.select_value("SELECT IS_FREE_LOCK('#{lock_name}');") == 1
   end
 end
diff --git a/activerecord/test/cases/adapters/postgresql/connection_test.rb b/activerecord/test/cases/adapters/postgresql/connection_test.rb
index 0ecac2cfa3..d559de3e28 100644
--- a/activerecord/test/cases/adapters/postgresql/connection_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/connection_test.rb
@@ -211,33 +211,33 @@ module ActiveRecord
     end
 
     def test_get_and_release_advisory_lock
-      key = 5295901941911233559
+      lock_id = 5295901941911233559
       list_advisory_locks = <<-SQL
         SELECT locktype,
-              (classid::bigint << 32) | objid::bigint AS lock_key
+              (classid::bigint << 32) | objid::bigint AS lock_id
         FROM pg_locks
         WHERE locktype = 'advisory'
       SQL
 
-      got_lock = @connection.get_advisory_lock(key)
+      got_lock = @connection.get_advisory_lock(lock_id)
       assert got_lock, "get_advisory_lock should have returned true but it didn't"
 
-      advisory_lock = @connection.query(list_advisory_locks).find {|l| l[1] == key}
+      advisory_lock = @connection.query(list_advisory_locks).find {|l| l[1] == lock_id}
       assert advisory_lock,
-        "expected to find an advisory lock with key #{key} but there wasn't one"
+        "expected to find an advisory lock with lock_id #{lock_id} but there wasn't one"
 
-      released_lock = @connection.release_advisory_lock(key)
+      released_lock = @connection.release_advisory_lock(lock_id)
       assert released_lock, "expected release_advisory_lock to return true but it didn't"
 
-      advisory_locks = @connection.query(list_advisory_locks).select {|l| l[1] == key}
+      advisory_locks = @connection.query(list_advisory_locks).select {|l| l[1] == lock_id}
       assert_empty advisory_locks,
-        "expected to have released advisory lock with key #{key} but it was still held"
+        "expected to have released advisory lock with lock_id #{lock_id} but it was still held"
     end
 
     def test_release_non_existent_advisory_lock
-      fake_key = 2940075057017742022
+      fake_lock_id = 2940075057017742022
       with_warning_suppression do
-        released_non_existent_lock = @connection.release_advisory_lock(fake_key)
+        released_non_existent_lock = @connection.release_advisory_lock(fake_lock_id)
         assert_equal released_non_existent_lock, false,
           'expected release_advisory_lock to return false when there was no lock to release'
       end
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 15a0e0516d..c3c204cf9f 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -524,33 +524,33 @@ class MigrationTest < ActiveRecord::TestCase
   end
 
   if ActiveRecord::Base.connection.supports_advisory_locks?
-    def test_migrator_generates_valid_lock_key
+    def test_migrator_generates_valid_lock_id
       migration = Class.new(ActiveRecord::Migration).new
       migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
 
-      lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+      lock_id = migrator.send(:generate_migrator_advisory_lock_id)
 
-      assert ActiveRecord::Base.connection.get_advisory_lock(lock_key),
-        "the Migrator should have generated a valid lock key, but it didn't"
-      assert ActiveRecord::Base.connection.release_advisory_lock(lock_key),
-        "the Migrator should have generated a valid lock key, but it didn't"
+      assert ActiveRecord::Base.connection.get_advisory_lock(lock_id),
+        "the Migrator should have generated a valid lock id, but it didn't"
+      assert ActiveRecord::Base.connection.release_advisory_lock(lock_id),
+        "the Migrator should have generated a valid lock id, but it didn't"
     end
 
-    def test_generate_migrator_advisory_lock_key
+    def test_generate_migrator_advisory_lock_id
       # It is important we are consistent with how we generate this so that
       # exclusive locking works across migrator versions
       migration = Class.new(ActiveRecord::Migration).new
       migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
 
-      lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+      lock_id = migrator.send(:generate_migrator_advisory_lock_id)
 
       current_database = ActiveRecord::Base.connection.current_database
       salt = ActiveRecord::Migrator::MIGRATOR_SALT
-      expected_key = Zlib.crc32(current_database) * salt
+      expected_id = Zlib.crc32(current_database) * salt
 
-      assert lock_key == expected_key, "expected lock key generated by the migrator to be #{expected_key}, but it was #{lock_key} instead"
-      assert lock_key.is_a?(Fixnum), "expected lock key to be a Fixnum, but it wasn't"
-      assert lock_key.bit_length <= 63, "lock key must be a signed integer of max 63 bits magnitude"
+      assert lock_id == expected_id, "expected lock id generated by the migrator to be #{expected_id}, but it was #{lock_id} instead"
+      assert lock_id.is_a?(Fixnum), "expected lock id to be a Fixnum, but it wasn't"
+      assert lock_id.bit_length <= 63, "lock id must be a signed integer of max 63 bits magnitude"
     end
 
     def test_migrator_one_up_with_unavailable_lock
@@ -564,9 +564,9 @@ class MigrationTest < ActiveRecord::TestCase
       }.new
 
       migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
-      lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+      lock_id = migrator.send(:generate_migrator_advisory_lock_id)
 
-      with_another_process_holding_lock(lock_key) do
+      with_another_process_holding_lock(lock_id) do
         assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.migrate }
       end
 
@@ -585,9 +585,9 @@ class MigrationTest < ActiveRecord::TestCase
       }.new
 
       migrator = ActiveRecord::Migrator.new(:up, [migration], 100)
-      lock_key = migrator.send(:generate_migrator_advisory_lock_key)
+      lock_id = migrator.send(:generate_migrator_advisory_lock_id)
 
-      with_another_process_holding_lock(lock_key) do
+      with_another_process_holding_lock(lock_id) do
         assert_raise(ActiveRecord::ConcurrentMigrationError) { migrator.run }
       end
 
@@ -606,18 +606,18 @@ class MigrationTest < ActiveRecord::TestCase
       }
     end
 
-    def with_another_process_holding_lock(lock_key)
+    def with_another_process_holding_lock(lock_id)
       thread_lock = Concurrent::CountDownLatch.new
       test_terminated = Concurrent::CountDownLatch.new
 
       other_process = Thread.new do
         begin
           conn = ActiveRecord::Base.connection_pool.checkout
-          conn.get_advisory_lock(lock_key)
+          conn.get_advisory_lock(lock_id)
           thread_lock.count_down
           test_terminated.wait # hold the lock open until we tested everything
         ensure
-          conn.release_advisory_lock(lock_key)
+          conn.release_advisory_lock(lock_id)
           ActiveRecord::Base.connection_pool.checkin(conn)
         end
       end