From c05b6170671030dca5a5295064c8cfdb6950b924 Mon Sep 17 00:00:00 2001
From: Sam Saffron <sam.saffron@gmail.com>
Date: Fri, 7 Jun 2019 11:19:57 +1000
Subject: [PATCH] FIX: ensure_consistency was able to create corrupt category
 topics

- Correct create_category_definition to skip validations and use a
transaction, no longer able to create corrupt topics

- ensure_consistency now clears topic_id if pointing at deleted or missing
topic_id

- Stop creating category definition topics for uncategorized
---
 app/models/category.rb       | 44 ++++++++++++++++++++++++++++++------
 spec/models/category_spec.rb | 12 ++++++++++
 2 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/app/models/category.rb b/app/models/category.rb
index b149ddd7dbf..f2cc2512f2a 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -222,13 +222,18 @@ class Category < ActiveRecord::Base
   def create_category_definition
     return if skip_category_definition
 
-    t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id)
-    t.skip_callbacks = true
-    t.ignore_category_auto_close = true
-    t.delete_topic_timer(TopicTimer.types[:close])
-    t.save!(validate: false)
-    update_column(:topic_id, t.id)
-    t.posts.create(raw: description || post_template, user: user)
+    Topic.transaction do
+      t = Topic.new(title: I18n.t("category.topic_prefix", category: name), user: user, pinned_at: Time.now, category_id: id)
+      t.skip_callbacks = true
+      t.ignore_category_auto_close = true
+      t.delete_topic_timer(TopicTimer.types[:close])
+      t.save!(validate: false)
+      update_column(:topic_id, t.id)
+      post = t.posts.build(raw: description || post_template, user: user)
+      post.save!(validate: false)
+
+      t
+    end
   end
 
   def topic_url
@@ -665,8 +670,33 @@ class Category < ActiveRecord::Base
   end
 
   def self.ensure_consistency!
+
+    sql = <<~SQL
+      SELECT t.id FROM topics t
+      JOIN categories c ON c.topic_id = t.id
+      LEFT JOIN posts p ON p.topic_id = t.id AND p.post_number = 1
+      WHERE p.id IS NULL
+    SQL
+
+    DB.query_single(sql).each do |id|
+      Topic.find(id).destroy!
+    end
+
+    sql = <<~SQL
+      UPDATE categories c
+      SET topic_id = NULL
+      WHERE c.id IN (
+        SELECT c2.id FROM categories c2
+        LEFT JOIN topics t ON t.id = c2.topic_id AND t.deleted_at IS NULL
+        WHERE t.id IS NULL AND c2.topic_id IS NOT NULL
+      )
+    SQL
+
+    DB.exec(sql)
+
     Category
       .joins('LEFT JOIN topics ON categories.topic_id = topics.id AND topics.deleted_at IS NULL')
+      .where('categories.id <> ?', SiteSetting.uncategorized_category_id)
       .where(topics: { id: nil })
       .find_each do |category|
       category.create_category_definition
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index 5746d396311..7f175b22e24 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -892,6 +892,12 @@ describe Category do
 
   describe "#ensure_consistency!" do
     it "creates category topic" do
+
+      # corrupt a category topic
+      uncategorized = Category.find(SiteSetting.uncategorized_category_id)
+      uncategorized.create_category_definition
+      uncategorized.topic.posts.first.destroy!
+
       category = Fabricate(:category)
       category_destroyed = Fabricate(:category)
       category_trashed = Fabricate(:category)
@@ -901,6 +907,12 @@ describe Category do
       category_trashed.topic.trash!
 
       Category.ensure_consistency!
+      # step one fix corruption
+      expect(uncategorized.reload.topic_id).to eq(nil)
+
+      Category.ensure_consistency!
+      # step two don't create a category definition for uncategorized
+      expect(uncategorized.reload.topic_id).to eq(nil)
 
       expect(category.reload.topic_id).to eq(category_topic_id)
       expect(category_destroyed.reload.topic).to_not eq(nil)