Merge pull request #3403 from evazion/fix-favgroup-race

Fix race condition when adding posts to favgroups

app/models/favorite_group.rb

@@ -173,11 +173,13 @@ class FavoriteGroup < ApplicationRecord
   end
 
   def add!(post_id)
-    post_id = post_id.id if post_id.is_a?(Post)
-    return if contains?(post_id)
+    with_lock do
+      post_id = post_id.id if post_id.is_a?(Post)
+      return if contains?(post_id)
 
-    clear_post_id_array
-    update_attributes(:post_ids => add_number_to_string(post_id, post_ids))
+      clear_post_id_array
+      update_attributes(:post_ids => add_number_to_string(post_id, post_ids))
+    end
   end
 
   def self.purge_post(post_id)
@@ -188,11 +190,13 @@ class FavoriteGroup < ApplicationRecord
   end
 
   def remove!(post_id)
-    post_id = post_id.id if post_id.is_a?(Post)
-    return unless contains?(post_id)
+    with_lock do
+      post_id = post_id.id if post_id.is_a?(Post)
+      return unless contains?(post_id)
 
-    clear_post_id_array
-    update_attributes(:post_ids => remove_number_from_string(post_id, post_ids))
+      clear_post_id_array
+      update_attributes(:post_ids => remove_number_from_string(post_id, post_ids))
+    end
   end
 
   def add_number_to_string(number, string)

test/unit/post_test.rb

@@ -835,6 +835,25 @@ class PostTest < ActiveSupport::TestCase
           end
         end
 
+        context "for a favgroup" do
+          setup do
+            @favgroup = FactoryGirl.create(:favorite_group, creator: @user)
+            @post = FactoryGirl.create(:post, :tag_string => "aaa favgroup:#{@favgroup.id}")
+          end
+
+          should "add the post to the favgroup" do
+            assert_equal(1, @favgroup.reload.post_count)
+            assert_equal(true, !!@favgroup.contains?(@post.id))
+          end
+
+          should "remove the post from the favgroup" do
+            @post.update(:tag_string => "-favgroup:#{@favgroup.id}")
+
+            assert_equal(0, @favgroup.reload.post_count)
+            assert_equal(false, !!@favgroup.contains?(@post.id))
+          end
+        end
+
         context "for a pool" do
           setup do
             mock_pool_archive_service!