From c148500d519bb44d6e6c954e4e5a1a174a6103f2 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Mon, 23 Apr 2018 13:32:08 +0800 Subject: [PATCH] FIX: Deadlock when topic with auto close topic timers exceeds `auto_close_topics_post_count`. --- lib/post_creator.rb | 56 +++++++++++------- spec/components/post_creator_spec.rb | 88 +++++++++++++++++++++++----- 2 files changed, 108 insertions(+), 36 deletions(-) diff --git a/lib/post_creator.rb b/lib/post_creator.rb index 1773adecfab..62874b36277 100644 --- a/lib/post_creator.rb +++ b/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 diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 57ab779b7ab..83dc757c88a 100644 --- a/spec/components/post_creator_spec.rb +++ b/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