Use an advisory lock to prevent concurrent saves

Author: lukearndt

Gemfile

@@ -23,6 +23,7 @@ gem "rack-cors", require: "rack/cors"
 gem "rails", "~> 6.0"
 gem "sass-rails", "~> 6.0"
 gem "secure_headers", ">= 3.0"
+gem "with_advisory_lock"
 
 group :production, :staging do
   gem "net-pop"

Gemfile.lock

@@ -446,6 +446,9 @@ GEM
     websocket-driver (0.7.6)
       websocket-extensions (>= 0.1.0)
     websocket-extensions (0.1.5)
+    with_advisory_lock (5.1.0)
+      activerecord (>= 6.1)
+      zeitwerk (>= 2.6)
     xpath (3.2.0)
       nokogiri (~> 1.8)
     zeitwerk (2.6.13)
@@ -502,6 +505,7 @@ DEPENDENCIES
   standard
   timecop
   web-console (~> 4.1)
+  with_advisory_lock
 
 RUBY VERSION
    ruby 3.3.1p55

app/models/measure_category.rb

@@ -14,16 +14,18 @@ class MeasureCategory < VersionedRecord
   # replacing "save": before creating the record we need to remove all
   # previous relationships (there should only be one) between the measure
   # and other categories of the same taxonomy
-  # also we need to lock the measure to ensure that no other category is associated
+  # also we need an advisory lock to ensure that no other category is associated during the transaction
   def save_with_cleanup
-    with_locked_measure do
+    if multiple_categories_allowed_for_taxonomy?
+      return save!
+    end
+
+    self.class.with_advisory_lock("MeasureCategory-measure_id_#{measure_id}-taxonomy_id_#{category.taxonomy_id}") do
       transaction do
-        if category && category.taxonomy&.allow_multiple == false
-          self.class.where(
-            category_id: category.taxonomy.categories.where.not(id: category.id).pluck(:id),
-            measure_id: measure_id
-          ).destroy_all
-        end
+        self.class.where(
+          category_id: category.taxonomy.categories.where.not(id: category.id).pluck(:id),
+          measure_id: measure_id
+        ).destroy_all
 
         save!
       end
@@ -32,6 +34,10 @@ def save_with_cleanup
 
   private
 
+  def multiple_categories_allowed_for_taxonomy?
+    !category&.taxonomy || category&.taxonomy&.allow_multiple
+  end
+
   def set_relationship_updated
     if measure && !measure.destroyed?
       measure.update_column(:relationship_updated_at, Time.zone.now)
@@ -40,25 +46,15 @@ def set_relationship_updated
   end
 
   def single_category_per_taxonomy
-    if category&.taxonomy && !category.taxonomy.allow_multiple
-      existing_categories = self.class.where(
-        category_id: category.taxonomy.categories.pluck(:id),  # Ensure you're using IDs here
-        measure_id: measure_id
-      )
+    return if multiple_categories_allowed_for_taxonomy?
 
-      if existing_categories.count >= 1
-        errors.add(:category, "This measure already has a category in the same taxonomy. Multiple categories are not allowed for the taxonomy.")
-      end
-    end
-  end
+    existing_categories = self.class.where(
+      category_id: category.taxonomy.categories.pluck(:id),  # Ensure you're using IDs here
+      measure_id: measure_id
+    )
 
-  def with_locked_measure
-    if measure
-      measure.with_lock do
-        yield
-      end
-    else
-      yield
+    if existing_categories.count >= 1
+      errors.add(:category, "This measure already has a category in the same taxonomy. Multiple categories are not allowed for the taxonomy.")
     end
   end
 end

app/models/recommendation_category.rb

@@ -16,14 +16,16 @@ class RecommendationCategory < VersionedRecord
   # and other categories of the same taxonomy
   # also we need to lock the recommendation to ensure that no other category is associated
   def save_with_cleanup
-    with_locked_recommendation do
+    if multiple_categories_allowed_for_taxonomy?
+      return save!
+    end
+
+    self.class.with_advisory_lock("RecommendationCategory-recommendation_id_#{recommendation_id}-taxonomy_id_#{category.taxonomy_id}") do
       transaction do
-        if category && category.taxonomy&.allow_multiple == false
-          self.class.where(
-            category_id: category.taxonomy.categories.where.not(id: category.id).pluck(:id),
-            recommendation_id: recommendation_id
-          ).destroy_all
-        end
+        self.class.where(
+          category_id: category.taxonomy.categories.where.not(id: category.id).pluck(:id),
+          recommendation_id: recommendation_id
+        ).destroy_all
 
         save!
       end
@@ -32,6 +34,10 @@ def save_with_cleanup
 
   private
 
+  def multiple_categories_allowed_for_taxonomy?
+    !category&.taxonomy || category&.taxonomy&.allow_multiple
+  end
+
   def set_relationship_updated
     if recommendation && !recommendation.destroyed?
       recommendation.update_column(:relationship_updated_at, Time.zone.now)
@@ -51,14 +57,4 @@ def single_category_per_taxonomy
       end
     end
   end
-
-  def with_locked_recommendation
-    if recommendation
-      recommendation.with_lock do
-        yield
-      end
-    else
-      yield
-    end
-  end
 end

app/models/user_category.rb

@@ -12,9 +12,13 @@ class UserCategory < ApplicationRecord
   # replacing "save": before creating the record we need to remove all
   # previous relationships (there should only be one) between the user
   # and other categories of the same taxonomy
-  # also we need to lock the user to ensure that no other category is associated
+  # also we need an advisory lock to ensure that no other category is associated during the transaction
   def save_with_cleanup
-    with_locked_user do
+    if multiple_categories_allowed_for_taxonomy?
+      return save!
+    end
+
+    self.class.with_advisory_lock("UserCategory-user_id_#{user_id}-taxonomy_id_#{category.taxonomy_id}") do
       transaction do
         if category && category.taxonomy&.allow_multiple == false
           self.class.where(
@@ -30,6 +34,10 @@ def save_with_cleanup
 
   private
 
+  def multiple_categories_allowed_for_taxonomy?
+    !category&.taxonomy || category&.taxonomy&.allow_multiple
+  end
+
   def set_relationship_updated
     if user && !user.destroyed?
       user.update_column(:relationship_updated_at, Time.zone.now)
@@ -38,25 +46,15 @@ def set_relationship_updated
   end
 
   def single_category_per_taxonomy
-    if category&.taxonomy && !category.taxonomy.allow_multiple
-      existing_categories = self.class.where(
-        category_id: category.taxonomy.categories.pluck(:id),  # Ensure you're using IDs here
-        user_id: user_id
-      )
-
-      if existing_categories.count >= 1
-        errors.add(:category, "This user already has a category in the same taxonomy. Multiple categories are not allowed for the taxonomy.")
-      end
-    end
-  end
+    return if multiple_categories_allowed_for_taxonomy?
 
-  def with_locked_user
-    if user
-      user.with_lock do
-        yield
-      end
-    else
-      yield
+    existing_categories = self.class.where(
+      category_id: category.taxonomy.categories.pluck(:id),  # Ensure you're using IDs here
+      user_id: user_id
+    )
+
+    if existing_categories.count >= 1
+      errors.add(:category, "This user already has a category in the same taxonomy. Multiple categories are not allowed for the taxonomy.")
     end
   end
 end