FEATURE: Improve backup stats on admin dashboard

* Dashboard doesn't timeout anymore when Amazon S3 is used for backups
* Storage stats are now a proper report with the same caching rules
* Changing the backup_location, s3_backup_bucket or creating and deleting backups removes the report from the cache
* It shows the number of backups and the backup location
* It shows the used space for the correct backup location instead of always showing used space on local storage
* It shows the date of the last backup as relative date

app/assets/javascripts/admin/components/admin-report-storage-stats.js.es6

@@ -0,0 +1,40 @@
+import { setting } from "discourse/lib/computed";
+import computed from "ember-addons/ember-computed-decorators";
+
+export default Ember.Component.extend({
+  classNames: ["admin-report-storage-stats"],
+
+  backupLocation: setting("backup_location"),
+  backupStats: Ember.computed.alias("model.data.backups"),
+  uploadStats: Ember.computed.alias("model.data.uploads"),
+
+  @computed("backupStats")
+  showBackupStats(stats) {
+    return stats && this.currentUser.admin;
+  },
+
+  @computed("backupLocation")
+  backupLocationName(backupLocation) {
+    return I18n.t(`admin.backups.location.${backupLocation}`);
+  },
+
+  @computed("backupStats.used_bytes")
+  usedBackupSpace(bytes) {
+    return I18n.toHumanSize(bytes);
+  },
+
+  @computed("backupStats.free_bytes")
+  freeBackupSpace(bytes) {
+    return I18n.toHumanSize(bytes);
+  },
+
+  @computed("uploadStats.used_bytes")
+  usedUploadSpace(bytes) {
+    return I18n.toHumanSize(bytes);
+  },
+
+  @computed("uploadStats.free_bytes")
+  freeUploadSpace(bytes) {
+    return I18n.toHumanSize(bytes);
+  }
+});

app/assets/javascripts/admin/controllers/admin-dashboard-next-general.js.es6

@@ -16,12 +16,7 @@ export default Ember.Controller.extend(PeriodComputationMixin, {
   isLoading: false,
   dashboardFetchedAt: null,
   exceptionController: Ember.inject.controller("exception"),
-  diskSpace: Ember.computed.alias("model.attributes.disk_space"),
   logSearchQueriesEnabled: setting("log_search_queries"),
-  lastBackupTakenAt: Ember.computed.alias(
-    "model.attributes.last_backup_taken_at"
-  ),
-  shouldDisplayDurability: Ember.computed.and("diskSpace"),
   basePath: Discourse.BaseUri,
 
   @computed
@@ -87,6 +82,7 @@ export default Ember.Controller.extend(PeriodComputationMixin, {
 
   usersByTypeReport: staticReport("users_by_type"),
   usersByTrustLevelReport: staticReport("users_by_trust_level"),
+  storageReport: staticReport("storage_report"),
 
   fetchDashboard() {
     if (this.get("isLoading")) return;
@@ -129,13 +125,6 @@ export default Ember.Controller.extend(PeriodComputationMixin, {
       .format("LLL");
   },
 
-  @computed("lastBackupTakenAt")
-  backupTimestamp(lastBackupTakenAt) {
-    return moment(lastBackupTakenAt)
-      .tz(moment.tz.guess())
-      .format("LLL");
-  },
-
   _reportsForPeriodURL(period) {
     return Discourse.getURL(`/admin?period=${period}`);
   }

app/assets/javascripts/admin/controllers/admin-dashboard.js.es6

@@ -4,7 +4,6 @@ import AdminUser from "admin/models/admin-user";
 import computed from "ember-addons/ember-computed-decorators";
 
 const ATTRIBUTES = [
-  "disk_space",
   "admins",
   "moderators",
   "silenced",

app/assets/javascripts/admin/models/admin-dashboard-next.js.es6

@@ -1,6 +1,6 @@
 import { ajax } from "discourse/lib/ajax";
 
-const GENERAL_ATTRIBUTES = ["disk_space", "updated_at", "last_backup_taken_at"];
+const GENERAL_ATTRIBUTES = ["updated_at"];
 
 const AdminDashboardNext = Discourse.Model.extend({});
 

app/assets/javascripts/admin/templates/components/admin-report-storage-stats.hbs

@@ -0,0 +1,33 @@
+{{#if showBackupStats}}
+  <div class="backups">
+    <h3 class="storage-stats-title">
+      <a href="{{get-url '/admin/backups'}}">{{d-icon "archive"}} {{i18n "admin.dashboard.backups"}}</a>
+    </h3>
+    <p>
+      {{#if backupStats.free_bytes}}
+        {{i18n "admin.dashboard.space_used_and_free" usedSize=usedBackupSpace freeSize=freeBackupSpace}}
+      {{else}}
+        {{i18n "admin.dashboard.space_used" usedSize=usedBackupSpace}}
+      {{/if}}
+
+      <br>
+      {{i18n "admin.dashboard.backup_count" count=backupStats.count location=backupLocationName}}
+
+      {{#if backupStats.last_backup_taken_at}}
+        <br>
+        {{{i18n "admin.dashboard.lastest_backup" date=(format-date backupStats.last_backup_taken_at leaveAgo="true")}}}
+      {{/if}}
+    </p>
+  </div>
+{{/if}}
+
+<div class="uploads">
+  <h3 class="storage-stats-title">{{d-icon "upload"}} {{i18n "admin.dashboard.uploads"}}</h3>
+  <p>
+    {{#if uploadStats.free_bytes}}
+      {{i18n "admin.dashboard.space_used_and_free" usedSize=usedUploadSpace freeSize=freeUploadSpace}}
+    {{else}}
+      {{i18n "admin.dashboard.space_used" usedSize=usedUploadSpace}}
+    {{/if}}
+  </p>
+</div>

app/assets/javascripts/admin/templates/dashboard_next_general.hbs

@@ -103,51 +103,26 @@
         {{/conditional-loading-section}}
       </div>
 
-      {{#conditional-loading-section isLoading=isLoading title=(i18n "admin.dashboard.backups")}}
+      <div class="misc">
-        <div class="misc">
+        {{admin-report
+          forcedModes="storage-stats"
+          dataSourceName="storage_stats"
+          showHeader=false}}
 
-          {{#if shouldDisplayDurability}}
+        <div class="last-dashboard-update">
-            <div class="durability">
+          <div>
-              {{#if currentUser.admin}}
-                <div class="backups">
-                  <h3 class="durability-title">
-                    <a href="{{get-url '/admin/backups'}}">{{d-icon "archive"}} {{i18n "admin.dashboard.backups"}}</a>
-                  </h3>
-                  <p>
-                    {{diskSpace.backups_used}} ({{i18n "admin.dashboard.space_free" size=diskSpace.backups_free}})
-
-                    {{#if lastBackupTakenAt}}
-                      <br />
-                      {{{i18n "admin.dashboard.lastest_backup" date=backupTimestamp}}}
-                    {{/if}}
-                  </p>
-                </div>
-              {{/if}}
-
-              <div class="uploads">
-                <h3 class="durability-title">{{d-icon "upload"}} {{i18n "admin.dashboard.uploads"}}</h3>
-                <p>
-                  {{diskSpace.uploads_used}} ({{i18n "admin.dashboard.space_free" size=diskSpace.uploads_free}})
-                </p>
-              </div>
-            </div>
-          {{/if}}
-
-          <div class="last-dashboard-update">
-            <div>
             <h4>{{i18n "admin.dashboard.last_updated"}} </h4>
             <p>{{updatedTimestamp}}</p>
-               <a rel="noopener" target="_blank" href="https://meta.discourse.org/tags/release-notes" class="btn btn-default">
+            <a rel="noopener" target="_blank" href="https://meta.discourse.org/tags/release-notes" class="btn btn-default">
-                 {{i18n "admin.dashboard.whats_new_in_discourse"}}
+              {{i18n "admin.dashboard.whats_new_in_discourse"}}
-                </a>
+            </a>
-            </div>
           </div>
         </div>
+      </div>
 
-        <p>
+      <p>
-          {{i18n 'admin.dashboard.find_old'}} {{#link-to 'admin.dashboard'}}{{i18n "admin.dashboard.old_link"}}{{/link-to}}
+        {{i18n 'admin.dashboard.find_old'}} {{#link-to 'admin.dashboard'}}{{i18n "admin.dashboard.old_link"}}{{/link-to}}
-        </p>
+      </p>
-      {{/conditional-loading-section}}
     </div>
 
     <div class="section-column">

app/assets/stylesheets/common/admin/dashboard_next.scss

@@ -191,7 +191,7 @@
     display: flex;
     border: 1px solid $primary-low;
 
-    .durability,
+    .storage-stats,
     .last-dashboard-update {
       flex: 1 1 50%;
       box-sizing: border-box;
@@ -199,7 +199,7 @@
       padding: 0 1em;
     }
 
-    .durability {
+    .storage-stats {
       display: flex;
       flex-wrap: wrap;
       justify-content: space-between;
@@ -213,15 +213,11 @@
       .uploads p:last-of-type {
         margin-bottom: 0;
       }
-
-      .durability-title {
-        text-transform: capitalize;
-      }
     }
 
     @media screen and (max-width: 400px) {
       flex-wrap: wrap;
-      .durability,
+      .storage-stats,
       .last-dashboard-update {
         flex: 1 1 100%;
         text-align: left;

app/controllers/admin/dashboard_controller.rb

@@ -1,9 +1,7 @@
-require 'disk_space'
 class Admin::DashboardController < Admin::AdminController
   def index
     dashboard_data = AdminDashboardData.fetch_cached_stats || Jobs::DashboardStats.new.execute({})
     dashboard_data.merge!(version_check: DiscourseUpdates.check_version.as_json) if SiteSetting.version_checks?
-    dashboard_data[:disk_space] = DiskSpace.cached_stats
     render json: dashboard_data
   end
 

app/controllers/admin/dashboard_next_controller.rb

@@ -1,5 +1,3 @@
-require 'disk_space'
-
 class Admin::DashboardNextController < Admin::AdminController
   def index
     data = AdminDashboardNextIndexData.fetch_cached_stats
@@ -15,25 +13,6 @@ class Admin::DashboardNextController < Admin::AdminController
   def security; end
 
   def general
-    data = AdminDashboardNextGeneralData.fetch_cached_stats
+    render json: AdminDashboardNextGeneralData.fetch_cached_stats
-
-    if SiteSetting.enable_backups
-      data[:last_backup_taken_at] = last_backup_taken_at
-      data[:disk_space] = DiskSpace.cached_stats
-    end
-
-    render json: data
-  end
-
-  private
-
-  def last_backup_taken_at
-    store = BackupRestore::BackupStore.create
-
-    begin
-      store.latest_file&.last_modified
-    rescue BackupRestore::BackupStore::StorageError
-      nil
-    end
   end
 end

app/jobs/regular/update_disk_space.rb

@@ -1,12 +0,0 @@
-require 'disk_space'
-
-module Jobs
-  class UpdateDiskSpace < Jobs::Base
-    sidekiq_options retry: false
-
-    def execute(args)
-      Discourse.cache.write(DiskSpace::DISK_SPACE_STATS_CACHE_KEY, DiskSpace.stats.to_json)
-      Discourse.cache.write(DiskSpace::DISK_SPACE_STATS_UPDATED_CACHE_KEY, Time.now.to_i)
-    end
-  end
-end

app/models/report.rb

@@ -49,8 +49,10 @@ class Report
     ].compact.map(&:to_s).join(':')
   end
 
-  def self.clear_cache
+  def self.clear_cache(type = nil)
-    Discourse.cache.keys("reports:*").each do |key|
+    pattern = type ? "reports:#{type}:*" : "reports:*"
+
+    Discourse.cache.keys(pattern).each do |key|
       Discourse.cache.redis.del(key)
     end
   end
@@ -76,9 +78,9 @@ class Report
 
     {
       type: type,
-      title: I18n.t("reports.#{type}.title"),
+      title: I18n.t("reports.#{type}.title", default: nil),
-      xaxis: I18n.t("reports.#{type}.xaxis"),
+      xaxis: I18n.t("reports.#{type}.xaxis", default: nil),
-      yaxis: I18n.t("reports.#{type}.yaxis"),
+      yaxis: I18n.t("reports.#{type}.yaxis", default: nil),
       description: description.presence ? description : nil,
       data: data,
       start_date: start_date&.iso8601,
@@ -1407,6 +1409,28 @@ class Report
     end
   end
 
+  def self.report_storage_stats(report)
+    backup_stats = begin
+      BackupRestore::BackupStore.create.stats
+    rescue BackupRestore::BackupStore::StorageError
+      nil
+    end
+
+    report.data = {
+      backups: backup_stats,
+      uploads: {
+        used_bytes: DiskSpace.uploads_used_bytes,
+        free_bytes: DiskSpace.uploads_free_bytes
+      }
+    }
+  end
+
+  DiscourseEvent.on(:site_setting_saved) do |site_setting|
+    if ["backup_location", "s3_backup_bucket"].include?(site_setting.name.to_s)
+      clear_cache(:storage_stats)
+    end
+  end
+
   private
 
   def hex_to_rgbs(hex_color)

config/locales/client.en.yml

@@ -2839,9 +2839,13 @@ en:
         private_messages_short: "Msgs"
         private_messages_title: "Messages"
         mobile_title: "Mobile"
-        space_free: "{{size}} free"
+        space_used: "%{usedSize} used"
-        uploads: "uploads"
+        space_used_and_free: "%{usedSize} (%{freeSize} free)"
-        backups: "backups"
+        uploads: "Uploads"
+        backups: "Backups"
+        backup_count:
+          one: "%{count} backup on %{location}"
+          other: "%{count} backups on %{location}"
         lastest_backup: "Latest: %{date}"
         traffic_short: "Traffic"
         traffic: "Application web requests"
@@ -3216,7 +3220,7 @@ en:
             title: "Rollback the database to previous working state"
             confirm: "Are you sure you want to rollback the database to the previous working state?"
         location:
-          local: "Local"
+          local: "Local Storage"
           s3: "Amazon S3"
 
       export_csv:

lib/backup_restore/backup_store.rb

@@ -18,7 +18,7 @@ module BackupRestore
 
     # @return [Array<BackupFile>]
     def files
-      unsorted_files.sort_by { |file| -file.last_modified.to_i }
+      @files ||= unsorted_files.sort_by { |file| -file.last_modified.to_i }
     end
 
     # @return [BackupFile]
@@ -26,6 +26,11 @@ module BackupRestore
       files.first
     end
 
+    def reset_cache
+      @files = nil
+      Report.clear_cache(:storage_stats)
+    end
+
     def delete_old
       return unless cleanup_allowed?
       return if (backup_files = files).size <= SiteSetting.maximum_backups
@@ -33,6 +38,8 @@ module BackupRestore
       backup_files[SiteSetting.maximum_backups..-1].each do |file|
         delete_file(file.filename)
       end
+
+      reset_cache
     end
 
     def remote?
@@ -60,6 +67,15 @@ module BackupRestore
       fail NotImplementedError
     end
 
+    def stats
+      {
+        used_bytes: used_bytes,
+        free_bytes: free_bytes,
+        count: files.size,
+        last_backup_taken_at: latest_file&.last_modified
+      }
+    end
+
     private
 
     # @return [Array<BackupFile>]
@@ -70,5 +86,13 @@ module BackupRestore
     def cleanup_allowed?
       true
     end
+
+    def used_bytes
+      files.sum { |file| file.size }
+    end
+
+    def free_bytes
+      fail NotImplementedError
+    end
   end
 end

lib/backup_restore/backuper.rb

@@ -1,4 +1,3 @@
-require "disk_space"
 require "mini_mime"
 
 module BackupRestore
@@ -304,7 +303,7 @@ module BackupRestore
 
     def refresh_disk_space
       log "Refreshing disk stats..."
-      DiskSpace.reset_cached_stats
+      @store.reset_cache
     rescue => ex
       log "Something went wrong while refreshing disk stats.", ex
     end

lib/backup_restore/local_backup_store.rb

@@ -34,7 +34,7 @@ module BackupRestore
 
       if File.exists?(path)
         FileUtils.remove_file(path, force: true)
-        DiskSpace.reset_cached_stats
+        reset_cache
       end
     end
 
@@ -63,5 +63,9 @@ module BackupRestore
         source: include_download_source ? path : nil
       )
     end
+
+    def free_bytes
+      DiskSpace.free(@base_directory)
+    end
   end
 end

lib/backup_restore/s3_backup_store.rb

@@ -24,7 +24,11 @@ module BackupRestore
 
     def delete_file(filename)
       obj = @s3_helper.object(filename)
-      obj.delete if obj.exists?
+
+      if obj.exists?
+        obj.delete
+        reset_cache
+      end
     end
 
     def download_file(filename, destination_path, failure_message = nil)
@@ -38,6 +42,7 @@ module BackupRestore
       raise BackupFileExists.new if obj.exists?
 
       obj.upload_file(source_path, content_type: content_type)
+      reset_cache
     end
 
     def generate_upload_url(filename)
@@ -100,5 +105,9 @@ module BackupRestore
         SiteSetting.s3_backup_bucket
       end
     end
+
+    def free_bytes
+      nil
+    end
   end
 end

lib/disk_space.rb

@@ -1,10 +1,4 @@
 class DiskSpace
-
-  extend ActionView::Helpers::NumberHelper
-
-  DISK_SPACE_STATS_CACHE_KEY ||= 'disk_space_stats'.freeze
-  DISK_SPACE_STATS_UPDATED_CACHE_KEY ||= 'disk_space_stats_updated'.freeze
-
   def self.uploads_used_bytes
     # used(uploads_path)
     # temporary (on our internal setup its just too slow to iterate)
@@ -15,51 +9,6 @@ class DiskSpace
     free(uploads_path)
   end
 
-  def self.backups_used_bytes
-    used(backups_path)
-  end
-
-  def self.backups_free_bytes
-    free(backups_path)
-  end
-
-  def self.backups_path
-    BackupRestore::LocalBackupStore.base_directory
-  end
-
-  def self.uploads_path
-    "#{Rails.root}/public/uploads/#{RailsMultisite::ConnectionManagement.current_db}"
-  end
-
-  def self.stats
-    {
-      uploads_used: number_to_human_size(uploads_used_bytes),
-      uploads_free: number_to_human_size(uploads_free_bytes),
-      backups_used: number_to_human_size(backups_used_bytes),
-      backups_free: number_to_human_size(backups_free_bytes)
-    }
-  end
-
-  def self.reset_cached_stats
-    Discourse.cache.delete(DISK_SPACE_STATS_UPDATED_CACHE_KEY)
-    Discourse.cache.delete(DISK_SPACE_STATS_CACHE_KEY)
-  end
-
-  def self.cached_stats
-    stats = Discourse.cache.read(DISK_SPACE_STATS_CACHE_KEY)
-    updated_at = Discourse.cache.read(DISK_SPACE_STATS_UPDATED_CACHE_KEY)
-
-    unless updated_at && (Time.now.to_i - updated_at.to_i) < 30.minutes
-      Jobs.enqueue(:update_disk_space)
-    end
-
-    if stats
-      JSON.parse(stats)
-    end
-  end
-
-  protected
-
   def self.free(path)
     `df -Pk #{path} | awk 'NR==2 {print $4;}'`.to_i * 1024
   end
@@ -67,4 +16,9 @@ class DiskSpace
   def self.used(path)
     `du -s #{path}`.to_i * 1024
   end
+
+  def self.uploads_path
+    "#{Rails.root}/public/uploads/#{RailsMultisite::ConnectionManagement.current_db}"
+  end
+  private_class_method :uploads_path
 end

spec/lib/backup_restore/s3_backup_store_spec.rb

@@ -81,11 +81,19 @@ describe BackupRestore::S3BackupStore do
     before { create_backups }
     after(:all) { remove_backups }
 
-    it "doesn't delete files when cleanup is disabled" do
+    describe "#delete_old" do
-      SiteSetting.maximum_backups = 1
+      it "doesn't delete files when cleanup is disabled" do
-      SiteSetting.s3_disable_cleanup = true
+        SiteSetting.maximum_backups = 1
+        SiteSetting.s3_disable_cleanup = true
 
-      expect { store.delete_old }.to_not change { store.files }
+        expect { store.delete_old }.to_not change { store.files }
+      end
+    end
+
+    describe "#stats" do
+      it "returns nil for 'free_bytes'" do
+        expect(store.stats[:free_bytes]).to be_nil
+      end
     end
   end
 

spec/lib/backup_restore/shared_examples_for_backup_store.rb

@@ -29,6 +29,17 @@ shared_examples "backup store" do
         expect(store.latest_file).to be_nil
       end
     end
+
+    describe "#stats" do
+      it "works when there are no files" do
+        stats = store.stats
+
+        expect(stats[:used_bytes]).to eq(0)
+        expect(stats).to have_key(:free_bytes)
+        expect(stats[:count]).to eq(0)
+        expect(stats[:last_backup_taken_at]).to be_nil
+      end
+    end
   end
 
   context "with backup files" do
@@ -69,6 +80,18 @@ shared_examples "backup store" do
       end
     end
 
+    describe "#reset_cache" do
+      it "resets the storage stats report" do
+        report_type = "storage_stats"
+        report = Report.find(report_type)
+        Report.cache(report, 35.minutes)
+        expect(Report.find_cached(report_type)).to be_present
+
+        store.reset_cache
+        expect(Report.find_cached(report_type)).to be_nil
+      end
+    end
+
     describe "#delete_old" do
       it "does nothing if the number of files is <= maximum_backups" do
         SiteSetting.maximum_backups = 3
@@ -166,6 +189,17 @@ shared_examples "backup store" do
         end
       end
     end
+
+    describe "#stats" do
+      it "returns the correct stats" do
+        stats = store.stats
+
+        expect(stats[:used_bytes]).to eq(57)
+        expect(stats).to have_key(:free_bytes)
+        expect(stats[:count]).to eq(3)
+        expect(stats[:last_backup_taken_at]).to eq(Time.parse("2018-09-13T15:10:00Z"))
+      end
+    end
   end
 end
 

test/javascripts/fixtures/dashboard-next-general.js.es6

@@ -1,13 +1,6 @@
 export default {
   "/admin/dashboard/general.json": {
     reports: [],
-    last_backup_taken_at: "2018-04-13T12:51:19.926Z",
+    updated_at: "2018-04-25T08:06:11.292Z"
-    updated_at: "2018-04-25T08:06:11.292Z",
-    disk_space: {
-      uploads_used: "74.5 KB",
-      uploads_free: "117 GB",
-      backups_used: "4.24 GB",
-      backups_free: "117 GB"
-    }
   }
 };