FIX: ensure_consistency was able to create corrupt category topics

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

diff --git a/app/models/category.rb b/app/models/category.rb
index b149ddd..f2cc251 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 5746d39..7f175b2 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)

GitHub sha: c05b6170

FIX: ensure consistency should handle cases where a topic trashed