Add DBLock.with_lock (#159)

* Skip mysql create db

* Cleanup mysql adapter

* Add DBLock.with_lock

* Update README

* Rubocop

.github/workflows/main.yml

@@ -41,6 +41,7 @@ jobs:
         image: mysql:5.7
         env:
           MYSQL_ALLOW_EMPTY_PASSWORD: yes
+          MYSQL_DATABASE: test
         ports:
           - 3306:3306
         options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
@@ -82,8 +83,6 @@ jobs:
           done
       - name: Create SQLServer Test DB
         run: echo -e "CREATE DATABASE test\nGO" | tsql -H 127.0.0.1 -p 1433 -U sa -P Password1234
-      - name: Create MYSQL test DB
-        run: mysql --host 127.0.0.1 --port 3306 -uroot -e "CREATE DATABASE IF NOT EXISTS test;"
       - uses: actions/checkout@v3
       - uses: ruby/setup-ruby@v1
         with:

.rubocop.yml

@@ -1,5 +1,3 @@
-inherit_from: .rubocop_todo.yml
-
 AllCops:
   NewCops: enable
   TargetRubyVersion: 2.7

README.md

@@ -20,22 +20,22 @@ then run `bundle`
 ## Usage
 
 ```ruby
-DBLock::Lock.get('name_of_lock', 5) do
+DBLock.with_lock('name_of_lock', 5) do
   # code here
 end
 ```
 
 Before the code block is executed, it will attempt to acquire a db lock for X seconds (5 in this example). If this fails it will raise an `DBLock::AlreadyLocked` error. The lock is released after the block is executed, even if the block raised an error itself.
 
-The current implementation uses a class variable to store lock state so it is not thread-safe when using multiple threads to acquire/release locks.
+The locking will already fail with an error if the current thread already holds a lock via this gem.
 
 Locks are achieved on the database via:
 
-| Database  | Locking method   |
-|-----------|------------------|
-| MySQL     | GET_LOCK         |
-| Postgres  | pg_advisory_lock |
-| SQLServer | sp_getapplock    |
+| Database  | Locking method     |
+|-----------|--------------------|
+| MySQL     | `GET_LOCK`         |
+| Postgres  | `pg_advisory_lock` |
+| SQLServer | `sp_getapplock`    |
 
 ## Dynamic lock name
 

lib/db_lock.rb

@@ -1,17 +1,25 @@
+require 'active_support'
 require 'digest/md5'
 
 module DBLock
-  extend self
-
   autoload :Adapter, 'db_lock/adapter'
   autoload :Lock, 'db_lock/lock'
+  autoload :Locking, 'db_lock/locking'
+
+  extend Locking
 
   class AlreadyLocked < StandardError; end
 
   attr_writer :db_handler
 
-  def db_handler
+  def self.db_handler
     # this must be an active record base object or subclass
     @db_handler || ActiveRecord::Base
   end
+
+  custom_deprecator = ActiveSupport::Deprecation.new('1.0.0', 'DBLock')
+  ActiveSupport::Deprecation.deprecate_methods(DBLock::Lock, get: 'use DBLock.with_lock instead',
+                                                             deprecator: custom_deprecator)
+  ActiveSupport::Deprecation.deprecate_methods(DBLock::Lock, locked?: 'will be removed without replacement',
+                                                             deprecator: custom_deprecator)
 end

lib/db_lock/adapter/mysql.rb

@@ -2,15 +2,13 @@ module DBLock
   module Adapter
     class MYSQL < Base
       def lock(name, timeout = 0)
-        sql = sanitize_sql_array 'SELECT GET_LOCK(?, ?)', name, timeout
-        res = connection.select_one sql
-        (res && res.values.first == 1)
+        res = select_value 'SELECT GET_LOCK(?, ?)', name, timeout
+        res == 1
       end
 
       def release(name)
-        sql = sanitize_sql_array 'SELECT RELEASE_LOCK(?)', name
-        res = connection.select_one sql
-        (res && res.values.first == 1)
+        res = select_value 'SELECT RELEASE_LOCK(?)', name
+        res == 1
       end
     end
   end

lib/db_lock/lock.rb

@@ -4,46 +4,12 @@ module DBLock
   module Lock
     extend self
 
-    def get(name, timeout = 0)
-      timeout = timeout.to_f # catches nil
-      timeout = 0 if timeout.negative?
-
-      raise "Invalid lock name: #{name.inspect}" if name.empty?
-      raise AlreadyLocked, 'Already lock in progress' if locked?
-
-      name = generate_lock_name(name)
-
-      if Adapter.lock(name, timeout)
-        @locked = true
-        yield
-      else
-        raise AlreadyLocked, "Unable to obtain lock '#{name}' within #{timeout} seconds" unless locked?
-      end
-    ensure
-      Adapter.release(name) if locked?
-      @locked = false
+    def get(name, timeout = 0, &block)
+      DBLock.with_lock(name, timeout, &block)
     end
 
     def locked?
-      @locked ||= false
-    end
-
-    private
-
-    def generate_lock_name(name)
-      name = "#{rails_app_name}.#{Rails.env}#{name}" if name[0] == '.' && defined? Rails
-      # reduce lock names of > 64 chars in size
-      # MySQL 5.7 only supports 64 chars max, there might be similar limitations elsewhere
-      name = "#{name.chars.first(15).join}-#{Digest::MD5.hexdigest(name)}-#{name.chars.last(15).join}" if name.length > 64
-      name
-    end
-
-    def rails_app_name
-      if Gem::Version.new(Rails.version) >= Gem::Version.new('6.0.0')
-        Rails.application.class.module_parent_name
-      else
-        Rails.application.class.parent_name
-      end
+      DBLock.send(:locked?)
     end
   end
 end

lib/db_lock/locking.rb

@@ -0,0 +1,47 @@
+require 'digest/md5'
+
+module DBLock
+  module Locking
+    def with_lock(name, timeout = 0)
+      timeout = timeout.to_f # catches nil
+      timeout = 0 if timeout.negative?
+
+      raise ArgumentError, "Invalid lock name: #{name.inspect}" if name.empty?
+      raise AlreadyLocked, 'Already lock in progress' if locked?
+
+      name = generate_lock_name(name)
+
+      if Adapter.lock(name, timeout)
+        @locked = true
+        yield
+      else
+        raise AlreadyLocked, "Unable to obtain lock '#{name}' within #{timeout} seconds" unless locked?
+      end
+    ensure
+      Adapter.release(name) if locked?
+      @locked = false
+    end
+
+    private
+
+    def locked?
+      @locked ||= false
+    end
+
+    def generate_lock_name(name)
+      name = "#{rails_app_name}.#{Rails.env}#{name}" if name[0] == '.' && defined? Rails
+      # reduce lock names of > 64 chars in size
+      # MySQL 5.7 only supports 64 chars max, there might be similar limitations elsewhere
+      name = "#{name.chars.first(15).join}-#{Digest::MD5.hexdigest(name)}-#{name.chars.last(15).join}" if name.length > 64
+      name
+    end
+
+    def rails_app_name
+      if Gem::Version.new(Rails.version) >= Gem::Version.new('6.0.0')
+        Rails.application.class.module_parent_name
+      else
+        Rails.application.class.parent_name
+      end
+    end
+  end
+end

spec/db_lock/adapter/mysql_spec.rb

@@ -9,34 +9,37 @@ module Adapter
       let(:timeout) { 5 }
 
       before do
-        allow(DBLock).to receive(:db_handler).and_return(MysqlA)
+        allow(DBLock).to receive(:db_handler).and_return(ModelMysql)
+      end
+
+      def is_free_lock(name)
+        subject.select_value 'SELECT IS_FREE_LOCK(?)', name
       end
 
       describe '#lock' do
         it 'obtains a mysql lock with the right name' do
-          expect(subject.lock(name, timeout)).to be true
-          res = MysqlA.connection.select_one "SELECT IS_FREE_LOCK('#{name}')"
-          expect(res.first.last).to eq(0)
+          expect {
+            expect(subject.lock(name, timeout)).to be true
+          }.to change { is_free_lock(name) }.from(1).to(0)
         end
 
         it 'waits for timeout seconds' do
-          MysqlB.connection.execute "SELECT GET_LOCK('#{name}', 0)"
-          time1 = Time.now
-          expect(subject.lock(name, 1)).to be false
-          time2 = Time.now
-          expect((time2 - time1).round(2)).to be_between(1.0, 1.1).inclusive
+          in_other_thread { subject.lock(name) }
+
+          time = Benchmark.realtime do
+            expect(subject.lock(name, 1)).to be false
+          end
+          expect(time.round(2)).to be_between(1.0, 1.1).inclusive
         end
       end
 
       describe '#release' do
-        # rubocop:disable RSpec/ExpectInHook
-        before { expect(subject.lock(name)).to be true }
-        # rubocop:enable RSpec/ExpectInHook
+        before { subject.lock(name) }
 
         it 'releases a lock' do
-          expect(subject.release(name)).to be true
-          res = MysqlA.connection.select_one "SELECT IS_FREE_LOCK('#{name}')"
-          expect(res.first.last).to eq(1)
+          expect {
+            expect(subject.release(name)).to be true
+          }.to change { is_free_lock(name) }.from(0).to(1)
         end
       end
     end

spec/db_lock/lock_spec.rb

@@ -28,10 +28,12 @@ module DBLock
       end
 
       context 'when using dynamic lock names based on Rails app name' do
+        # rubocop:disable RSpec/MessageChain
         before do
           allow(Rails).to receive_message_chain(:application, :class, :parent_name).and_return('Dummy')
           allow(Rails).to receive_message_chain(:application, :class, :module_parent_name).and_return('Dummy')
         end
+        # rubocop:enable RSpec/MessageChain
 
         it 'supports lock names from rails app name' do
           subject.get(".custom_lock", timeout) { sleep 0 }

spec/db_lock_spec.rb

@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+module DBLock
+  RSpec.describe '.with_lock' do
+    let(:name) { "custom_lock:db_lock:#{(0...8).map { rand(65..90).chr }.join}" }
+    let(:timeout) { 5 }
+
+    before do
+      allow(Adapter).to receive(:lock).and_return(true)
+      allow(Adapter).to receive(:release).and_return(true)
+    end
+
+    it 'uses the Adapter to receive and release the lock' do
+      DBLock.with_lock(name, timeout) { sleep 0 }
+      expect(Adapter).to have_received(:lock).with(name, timeout)
+      expect(Adapter).to have_received(:release).with(name)
+    end
+
+    it 'limits lock names to 64 characters' do
+      DBLock.with_lock("lock.name.exceeding.#{'asdf' * 10}.sixtyfour.characters", timeout) { sleep 0 }
+      short_name = 'lock.name.excee-9782cc3fe0258bd32022ddfd0a24c8d4-four.characters'
+      expect(Adapter).to have_received(:lock).with(short_name, timeout)
+      expect(Adapter).to have_received(:release).with(short_name)
+    end
+
+    context 'when using dynamic lock names based on Rails app name' do
+      # rubocop:disable RSpec/MessageChain
+      before do
+        allow(Rails).to receive_message_chain(:application, :class, :parent_name).and_return('Dummy')
+        allow(Rails).to receive_message_chain(:application, :class, :module_parent_name).and_return('Dummy')
+      end
+      # rubocop:enable RSpec/MessageChain
+
+      it 'supports lock names from rails app name' do
+        DBLock.with_lock('.custom_lock', timeout) { sleep 0 }
+        expect(Adapter).to have_received(:lock).with('Dummy.test.custom_lock', timeout)
+        expect(Adapter).to have_received(:release).with('Dummy.test.custom_lock')
+      end
+    end
+
+    context 'when the lock can be achieved' do
+      before do
+        allow(Adapter).to receive(:lock).and_return(true)
+      end
+
+      it 'executes the block' do
+        x = 0
+        DBLock.with_lock(name) { x += 1 }
+        expect(x).to eq(1)
+      end
+
+      it 'passes through errors but still frees the lock' do
+        expect do
+          DBLock.with_lock(name, timeout) { raise 'something happened' }
+        end.to raise_error(RuntimeError)
+        expect(Adapter).to have_received(:release)
+      end
+    end
+
+    context 'when the lock can not be achieved' do
+      before do
+        allow(Adapter).to receive(:lock).and_return(false)
+      end
+
+      it 'raises an error and does not execute the block' do
+        x = 0
+        expect { DBLock.with_lock(name, 0) { x += 1 } }.to raise_error(DBLock::AlreadyLocked)
+        expect(x).to eq(0), 'the block was executed'
+      end
+    end
+  end
+end

spec/spec_helper.rb

@@ -35,8 +35,7 @@ def skip_unless(adapter)
 
   config.before(:suite) do
     ENV['MYSQL_URL']&.then do |url|
-      MysqlA.establish_connection url
-      MysqlB.establish_connection url
+      ModelMysql.establish_connection url
     end
     ENV['POSTGRES_URL']&.then do |url|
       ModelPostgres.establish_connection url

spec/support/connections.rb

@@ -1,5 +1,4 @@
 MssqlA = Class.new(ActiveRecord::Base)
 MssqlB = Class.new(ActiveRecord::Base)
-MysqlA = Class.new(ActiveRecord::Base)
-MysqlB = Class.new(ActiveRecord::Base)
+ModelMysql = Class.new(ActiveRecord::Base)
 ModelPostgres = Class.new(ActiveRecord::Base)