From 0b7a6cda7ab5e6008f2edf9f2f6fa5a7bb2beec8 Mon Sep 17 00:00:00 2001
From: Ben Sheldon <bensheldon@gmail.com>
Date: Tue, 29 Dec 2020 22:22:18 -0800
Subject: [PATCH] Have Lockable#advisory_locked? directly query pg_locks table

Addresses a flaky test that simply counted `pg_locks` table rows.
---
 lib/good_job/lockable.rb           | 10 +++++++++-
 spec/lib/good_job/lockable_spec.rb | 21 ++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/lib/good_job/lockable.rb b/lib/good_job/lockable.rb
index 20fc22691..d6449d8bd 100644
--- a/lib/good_job/lockable.rb
+++ b/lib/good_job/lockable.rb
@@ -205,7 +205,15 @@ def with_advisory_lock
     # Tests whether this record has an advisory lock on it.
     # @return [Boolean]
     def advisory_locked?
-      self.class.unscoped.advisory_locked.exists?(id: send(self.class.primary_key))
+      query = <<~SQL.squish
+        SELECT 1 AS one
+        FROM pg_locks
+        WHERE pg_locks.locktype = 'advisory'
+          AND pg_locks.objsubid = 1
+          AND pg_locks.classid = ('x' || substr(md5(:table_name || :id::text), 1, 16))::bit(32)::int
+          AND pg_locks.objid = (('x' || substr(md5(:table_name || :id::text), 1, 16))::bit(64) << 32)::bit(32)::int
+      SQL
+      self.class.connection.execute(sanitize_sql_for_conditions([query, { table_name: self.class.table_name, id: send(self.class.primary_key) }])).ntuples.positive?
     end
 
     # Tests whether this record is locked by the current database session.
diff --git a/spec/lib/good_job/lockable_spec.rb b/spec/lib/good_job/lockable_spec.rb
index 7fb232be1..78ac9492d 100644
--- a/spec/lib/good_job/lockable_spec.rb
+++ b/spec/lib/good_job/lockable_spec.rb
@@ -109,7 +109,26 @@
 
       expect do
         job.advisory_unlock
-      end.to change(PgLock, :count).by(-1)
+      end.to change(job, :advisory_locked?).from(true).to(false)
+    end
+  end
+
+  describe '#advisory_locked?' do
+    it 'reflects whether the record is locked' do
+      expect(job.advisory_locked?).to eq false
+      job.advisory_lock
+      expect(job.advisory_locked?).to eq true
+      job.advisory_unlock
+      expect(job.advisory_locked?).to eq false
+    end
+
+    it 'is accurate even if the job has been destroyed' do
+      job.advisory_lock
+      expect(job.advisory_locked?).to eq true
+      job.destroy!
+      expect(job.advisory_locked?).to eq true
+      job.advisory_unlock
+      expect(job.advisory_locked?).to eq false
     end
   end