FIX: Deadlock when topic with auto close topic timers exceeds `auto_close_topics_post_count`.

lib/post_creator.rb

@@ -291,22 +291,34 @@ class PostCreator
   end
 
   def auto_close
-    if @post.topic.private_message? &&
-        !@post.topic.closed &&
+    topic = @post.topic
+    is_private_message = topic.private_message?
+    topic_posts_count = @post.topic.posts_count
+
+    if is_private_message &&
+        !topic.closed &&
         SiteSetting.auto_close_messages_post_count > 0 &&
-        SiteSetting.auto_close_messages_post_count <= @post.topic.posts_count
+        SiteSetting.auto_close_messages_post_count <= topic_posts_count
 
-      @post.topic.update_status(:closed, true, Discourse.system_user,
-          message: I18n.t('topic_statuses.autoclosed_message_max_posts', count: SiteSetting.auto_close_messages_post_count))
-
-    elsif !@post.topic.private_message? &&
-        !@post.topic.closed &&
+      @post.topic.update_status(
+        :closed, true, Discourse.system_user,
+        message: I18n.t(
+          'topic_statuses.autoclosed_message_max_posts',
+          count: SiteSetting.auto_close_messages_post_count
+        )
+      )
+    elsif !is_private_message &&
+        !topic.closed &&
         SiteSetting.auto_close_topics_post_count > 0 &&
-        SiteSetting.auto_close_topics_post_count <= @post.topic.posts_count
-
-      @post.topic.update_status(:closed, true, Discourse.system_user,
-          message: I18n.t('topic_statuses.autoclosed_topic_max_posts', count: SiteSetting.auto_close_topics_post_count))
+        SiteSetting.auto_close_topics_post_count <= topic_posts_count
 
+      topic.update_status(
+        :closed, true, Discourse.system_user,
+        message: I18n.t(
+          'topic_statuses.autoclosed_topic_max_posts',
+          count: SiteSetting.auto_close_topics_post_count
+        )
+      )
     end
   end
 
@@ -405,16 +417,20 @@ class PostCreator
   end
 
   def update_topic_auto_close
-    topic_timer = @topic.public_topic_timer
+    if @topic.closed?
+      @topic.delete_topic_timer(TopicTimer.types[:close])
+    else
+      topic_timer = @topic.public_topic_timer
 
-    if topic_timer &&
-       topic_timer.based_on_last_post &&
-       topic_timer.duration > 0
+      if topic_timer &&
+         topic_timer.based_on_last_post &&
+         topic_timer.duration > 0
 
-      @topic.set_or_create_timer(TopicTimer.types[:close],
-        topic_timer.duration,
-        based_on_last_post: topic_timer.based_on_last_post
-      )
+        @topic.set_or_create_timer(TopicTimer.types[:close],
+          topic_timer.duration,
+          based_on_last_post: topic_timer.based_on_last_post
+        )
+      end
     end
   end
 

spec/components/post_creator_spec.rb

@@ -306,23 +306,64 @@ describe PostCreator do
           expect(topic_status_update.created_at).to be_within(1.second).of(Time.zone.now)
         end
 
-        it "updates topic's auto close date when it's based on last post" do
-          freeze_time
-          topic = Fabricate(:topic_timer,
-            based_on_last_post: true,
-            execute_at: Time.zone.now - 12.hours,
-            created_at: Time.zone.now - 24.hours
-          ).topic
+        describe "topic's auto close based on last post" do
+          let(:topic_timer) do
+            Fabricate(:topic_timer,
+              based_on_last_post: true,
+              execute_at: Time.zone.now - 12.hours,
+              created_at: Time.zone.now - 24.hours
+            )
+          end
 
-          Fabricate(:post, topic: topic)
+          let(:topic) { topic_timer.topic }
 
-          PostCreator.new(topic.user, topic_id: topic.id, raw: "this is a second post").create
+          let(:post) do
+            Fabricate(:post, topic: topic)
+          end
 
-          topic_status_update = TopicTimer.last
-          expect(topic_status_update.execute_at).to be_within(1.second).of(Time.zone.now + 12.hours)
-          expect(topic_status_update.created_at).to be_within(1.second).of(Time.zone.now)
+          it "updates topic's auto close date" do
+            freeze_time
+            post
+
+            PostCreator.new(
+              topic.user,
+              topic_id: topic.id,
+              raw: "this is a second post"
+            ).create
+
+            topic_timer.reload
+
+            expect(topic_timer.execute_at).to eq(Time.zone.now + 12.hours)
+            expect(topic_timer.created_at).to eq(Time.zone.now)
+          end
+
+          describe "when auto_close_topics_post_count has been reached" do
+            before do
+              SiteSetting.auto_close_topics_post_count = 2
+            end
+
+            it "closes the topic and deletes the topic timer" do
+              freeze_time
+              post
+
+              PostCreator.new(
+                topic.user,
+                topic_id: topic.id,
+                raw: "this is a second post"
+              ).create
+
+              topic.reload
+
+              expect(topic.posts.last.raw).to eq(I18n.t(
+                'topic_statuses.autoclosed_topic_max_posts',
+                count: SiteSetting.auto_close_topics_post_count
+              ))
+
+              expect(topic.closed).to eq(true)
+              expect(topic_timer.reload.deleted_at).to eq(Time.zone.now)
+            end
+          end
         end
-
       end
 
       context "tags" do
@@ -742,9 +783,17 @@ describe PostCreator do
       post1 = create_post(archetype: Archetype.private_message,
                           target_usernames: [admin.username])
 
-      _post2 = create_post(user: post1.user, topic_id: post1.topic_id)
+      expect do
+        create_post(user: post1.user, topic_id: post1.topic_id)
+      end.to change { Post.count }.by(2)
 
       post1.topic.reload
+
+      expect(post1.topic.posts.last.raw).to eq(I18n.t(
+        'topic_statuses.autoclosed_message_max_posts',
+        count: SiteSetting.auto_close_messages_post_count
+      ))
+
       expect(post1.topic.closed).to eq(true)
     end
 
@@ -752,11 +801,18 @@ describe PostCreator do
       SiteSetting.auto_close_topics_post_count = 2
 
       post1 = create_post
-      _post2 = create_post(user: post1.user, topic_id: post1.topic_id)
+
+      expect do
+        create_post(user: post1.user, topic_id: post1.topic_id)
+      end.to change { Post.count }.by(2)
 
       post1.topic.reload
 
-      expect(post1.topic.posts_count).to eq(3)
+      expect(post1.topic.posts.last.raw).to eq(I18n.t(
+        'topic_statuses.autoclosed_topic_max_posts',
+        count: SiteSetting.auto_close_topics_post_count
+      ))
+
       expect(post1.topic.closed).to eq(true)
     end
   end