From 521934f1638bf059bc4dc8b8522fbc86ce4cec6f Mon Sep 17 00:00:00 2001
From: Mark VanLandingham <mark.vanlandingham@discourse.org>
Date: Wed, 9 Dec 2020 10:54:41 -0600
Subject: [PATCH] FIX: Only cache reports with exceptions for 1 minute (#11447)

---
 app/controllers/admin/reports_controller.rb   |  4 +--
 app/models/report.rb                          |  3 ++-
 .../shared_examples_for_backup_store.rb       |  2 +-
 spec/models/report_spec.rb                    | 27 +++++++++++++++++++
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb
index 5fc2544b48..e9be087108 100644
--- a/app/controllers/admin/reports_controller.rb
+++ b/app/controllers/admin/reports_controller.rb
@@ -42,7 +42,7 @@ class Admin::ReportsController < Admin::AdminController
           report = Report.find(report_type, args)
 
           if (report_params[:cache]) && report
-            Report.cache(report, 35.minutes)
+            Report.cache(report)
           end
 
           if report.blank?
@@ -80,7 +80,7 @@ class Admin::ReportsController < Admin::AdminController
       raise Discourse::NotFound if report.blank?
 
       if (params[:cache])
-        Report.cache(report, 35.minutes)
+        Report.cache(report)
       end
 
       render_json_dump(report: report)
diff --git a/app/models/report.rb b/app/models/report.rb
index 9a62774d9e..b0f97d7ae5 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -223,7 +223,8 @@ class Report
     Discourse.cache.read(cache_key(report))
   end
 
-  def self.cache(report, duration)
+  def self.cache(report)
+    duration = report.error == :exception ? 1.minute : 35.minutes
     Discourse.cache.write(cache_key(report), report.as_json, expires_in: duration)
   end
 
diff --git a/spec/lib/backup_restore/shared_examples_for_backup_store.rb b/spec/lib/backup_restore/shared_examples_for_backup_store.rb
index 679bc9aa79..f6d3fbab3d 100644
--- a/spec/lib/backup_restore/shared_examples_for_backup_store.rb
+++ b/spec/lib/backup_restore/shared_examples_for_backup_store.rb
@@ -88,7 +88,7 @@ shared_examples "backup store" do
         report_type = "storage_stats"
         report = Report.find(report_type)
 
-        Report.cache(report, 35.minutes)
+        Report.cache(report)
         expect(Report.find_cached(report_type)).to be_present
 
         store.reset_cache
diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb
index 576c713125..0b4c330af8 100644
--- a/spec/models/report_spec.rb
+++ b/spec/models/report_spec.rb
@@ -1237,6 +1237,33 @@ describe Report do
     end
   end
 
+  describe ".cache" do
+    let(:exception_report) { Report.find("exception_test", wrap_exceptions_in_test: true) }
+    let(:valid_report) { Report.find("valid_test", wrap_exceptions_in_test: true) }
+
+    before(:each) do
+      class Report
+        def self.report_exception_test(report)
+          report.data = x
+        end
+
+        def self.report_valid_test(report)
+          report.data = "success!"
+        end
+      end
+    end
+
+    it "caches exception reports for 1 minute" do
+      Discourse.cache.expects(:write).with(Report.cache_key(exception_report), exception_report.as_json, { expires_in: 1.minute })
+      Report.cache(exception_report)
+    end
+
+    it "caches valid reports for 35 minutes" do
+      Discourse.cache.expects(:write).with(Report.cache_key(valid_report), valid_report.as_json, { expires_in: 35.minutes })
+      Report.cache(valid_report)
+    end
+  end
+
   describe "top_uploads" do
     context "with no data" do
       it "works" do