Merge pull request #60 from bitcrowd/fix/advisory-xact-lock-should-accept-atoms

Fix Repo.advisory_xact_lock/1 to handle atom names

Author: andreasknoepfle

lib/bitcrowd_ecto/repo.ex

@@ -137,7 +137,7 @@ defmodule BitcrowdEcto.Repo do
   "application" key and one specific lock key), in which case PostgreSQL concatenates them
   into a 64-bit value. In any case you need to pass integers.
 
-  We decided that we wanted to have atom or string keys for better readability.  Hence, in=
+  We decided that we wanted to have atom or string keys for better readability.  Hence, in
   order to make PostgreSQL happy, we hash these strings into 64 bits signed ints.
 
   64 bits make a pretty big number already, so it is quite unlikely that two of our keys (or
@@ -254,8 +254,8 @@ defmodule BitcrowdEcto.Repo do
   end
 
   @doc false
-  def advisory_xact_lock(repo, name) do
-    <<advisory_lock_key::signed-integer-64, _rest::binary>> = :crypto.hash(:sha, name)
+  def advisory_xact_lock(repo, name) when is_atom(name) or is_binary(name) do
+    <<advisory_lock_key::signed-integer-64, _rest::binary>> = :crypto.hash(:sha, to_string(name))
     SQL.query!(repo, "SELECT pg_advisory_xact_lock($1);", [advisory_lock_key])
     :ok
   end

test/bitcrowd_ecto/repo_test.exs

@@ -102,13 +102,16 @@ defmodule BitcrowdEcto.RepoTest do
       assert TestRepo.fetch_by(query, []) == {:ok, resource}
     end
 
-    # The actual lock is non-trivial to test, I tried.
     test "can lock for :update", %{resource: %{id: id} = resource} do
-      assert TestRepo.fetch_by(TestSchema, [id: id], lock: :update) == {:ok, resource}
+      assert_lock_granted("relation = 'test_schema_pkey'::regclass::oid", fn ->
+        assert TestRepo.fetch_by(TestSchema, [id: id], lock: :update) == {:ok, resource}
+      end)
     end
 
     test "can lock for :no_key_update", %{resource: %{id: id} = resource} do
-      assert TestRepo.fetch_by(TestSchema, [id: id], lock: :no_key_update) == {:ok, resource}
+      assert_lock_granted("relation = 'test_schema_pkey'::regclass::oid", fn ->
+        assert TestRepo.fetch_by(TestSchema, [id: id], lock: :no_key_update) == {:ok, resource}
+      end)
     end
 
     test "converts CastErrors for binary_id columns to not_found errors" do
@@ -161,4 +164,41 @@ defmodule BitcrowdEcto.RepoTest do
       assert TestRepo.fetch_by(TestSchema, [id: resource.id], prefix: prefix) == {:ok, resource}
     end
   end
+
+  describe "advisory_xact_lock/1" do
+    test "acquires an advisory lock" do
+      assert_lock_granted("locktype = 'advisory'", fn ->
+        TestRepo.advisory_xact_lock("foo")
+      end)
+    end
+
+    test "accepts atoms" do
+      assert_lock_granted("locktype = 'advisory'", fn ->
+        TestRepo.advisory_xact_lock(:foo)
+      end)
+    end
+  end
+
+  defp assert_lock_granted(where, fun) do
+    assert locks_granted(where) == 0
+    result = fun.()
+    assert locks_granted(where) == 1
+    result
+  end
+
+  defp locks_granted(where) do
+    %{rows: [[vxid]]} =
+      TestRepo.query!("""
+      SELECT virtualtransaction FROM pg_locks
+      WHERE transactionid::text = (txid_current() % (2^32)::bigint)::text;
+      """)
+
+    %{rows: [[count]]} =
+      TestRepo.query!("""
+      SELECT COUNT(*) FROM pg_locks
+      WHERE virtualtransaction = '#{vxid}' AND granted IS TRUE AND #{where};
+      """)
+
+    count
+  end
 end