Merge pull request #3593 from tgxworld/cache_results_on_about_page

Cache results on about page

app/jobs/scheduled/about_stats.rb

@@ -0,0 +1,13 @@
+module Jobs
+  class AboutStats < Jobs::Scheduled
+    include Jobs::Stats
+
+    every 30.minutes
+
+    def execute(args)
+      stats = About.new.stats
+      set_cache(About, stats)
+      stats
+    end
+  end
+end

app/jobs/scheduled/dashboard_stats.rb

@@ -1,16 +1,13 @@
 module Jobs
   class DashboardStats < Jobs::Scheduled
+    include Jobs::Stats
+
     every 30.minutes
 
     def execute(args)
-      stats = AdminDashboardData.fetch_stats.as_json
-
-      # Add some extra time to the expiry so that the next job run has plenty of time to
-      # finish before previous cached value expires.
-      $redis.setex AdminDashboardData.stats_cache_key, (AdminDashboardData.recalculate_interval + 5).minutes, stats.to_json
-
+      stats = AdminDashboardData.new.as_json
+      set_cache(AdminDashboardData, stats)
       stats
     end
-
   end
 end

app/jobs/stats.rb

@@ -0,0 +1,9 @@
+module Jobs
+  module Stats
+    def set_cache(klass, stats)
+      # Add some extra time to the expiry so that the next job run has plenty of time to
+      # finish before previous cached value expires.
+      $redis.setex klass.stats_cache_key, (klass.recalculate_stats_interval + 5).minutes, stats.to_json
+    end
+  end
+end

app/models/about.rb

@@ -1,9 +1,18 @@
 class About
   include ActiveModel::Serialization
+  include StatsCacheable
 
   attr_accessor :moderators,
                 :admins
 
+  def self.stats_cache_key
+    'about-stats'
+  end
+
+  def self.fetch_stats
+    About.new.stats
+  end
+
   def version
     Discourse::VERSION::STRING
   end

app/models/admin_dashboard_data.rb

@@ -1,6 +1,7 @@
 require_dependency 'mem_info'
 
 class AdminDashboardData
+  include StatsCacheable
 
   GLOBAL_REPORTS ||= [
     'visits',
@@ -57,13 +58,7 @@ class AdminDashboardData
 
 
   def self.fetch_stats
-    AdminDashboardData.new
-  end
-
-  def self.fetch_cached_stats
-    # The DashboardStats job is responsible for generating and caching this.
-    stats = $redis.get(stats_cache_key)
-    stats ? JSON.parse(stats) : nil
+    AdminDashboardData.new.as_json
   end
 
   def self.stats_cache_key
@@ -97,11 +92,6 @@ class AdminDashboardData
     source.map { |type| Report.find(type).as_json }
   end
 
-  # Could be configurable, multisite need to support it.
-  def self.recalculate_interval
-    30 # minutes
-  end
-
   def rails_env_check
     I18n.t("dashboard.rails_env_warning", env: Rails.env) unless Rails.env.production?
   end

app/models/concerns/stats_cacheable.rb

@@ -0,0 +1,24 @@
+module StatsCacheable
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def stats_cache_key
+      raise 'Stats cache key has not been set.'
+    end
+
+    def fetch_stats
+      raise 'Not implemented.'
+    end
+
+    # Could be configurable, multisite need to support it.
+    def recalculate_stats_interval
+      30 # minutes
+    end
+
+    def fetch_cached_stats
+      # The scheduled Stats job is responsible for generating and caching this.
+      stats = $redis.get(stats_cache_key)
+      stats ? JSON.parse(stats) : nil
+    end
+  end
+end

app/serializers/about_serializer.rb

@@ -8,4 +8,8 @@ class AboutSerializer < ApplicationSerializer
              :locale,
              :version,
              :https
+
+  def stats
+    object.class.fetch_cached_stats || Jobs::AboutStats.new.execute({})
+  end
 end

spec/jobs/about_stats_spec.rb

@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe Jobs::AboutStats do
+  it 'caches the stats' do
+    stats = { "visited" => 10 }
+    About.any_instance.expects(:stats).returns(stats)
+    $redis.expects(:setex).with(About.stats_cache_key, 35.minutes, stats.to_json)
+    expect(described_class.new.execute({})).to eq(stats)
+  end
+end

spec/jobs/dashboard_stats_spec.rb

@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe Jobs::DashboardStats do
+  it 'caches the stats' do
+    json = { "visited" => 10 }
+    AdminDashboardData.any_instance.expects(:as_json).returns(json)
+    $redis.expects(:setex).with(AdminDashboardData.stats_cache_key, 35.minutes, json.to_json)
+    expect(described_class.new.execute({})).to eq(json)
+  end
+end

spec/models/about_spec.rb

@@ -0,0 +1,9 @@
+require 'spec_helper'
+
+describe About do
+
+  describe 'stats cache' do
+    include_examples 'stats cachable'
+  end
+
+end

spec/models/admin_dashboard_data_spec.rb

@@ -244,4 +244,8 @@ describe AdminDashboardData do
     end
   end
 
+  describe 'stats cache' do
+    include_examples 'stats cachable'
+  end
+
 end

spec/support/shared_examples_for_stats_cacheable.rb

@@ -0,0 +1,23 @@
+shared_examples_for 'stats cachable' do
+  describe 'fetch_cached_stats' do
+    it 'returns the cached stats' do
+      begin
+        stats = { "visits" => 10 }
+        $redis.set(described_class.stats_cache_key, stats.to_json)
+        expect(described_class.fetch_cached_stats).to eq(stats)
+      ensure
+        $redis.del(described_class.stats_cache_key)
+      end
+    end
+
+    it 'returns nil if no stats has been cached' do
+      expect(described_class.fetch_cached_stats).to eq(nil)
+    end
+  end
+
+  describe 'fetch_stats' do
+    it 'has been implemented' do
+      expect{ described_class.fetch_stats }.to_not raise_error
+    end
+  end
+end