From c3c25b894a5dc755e42c78e234aea1b057141a01 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Wed, 20 Mar 2013 13:54:32 -0400
Subject: [PATCH] Cache dashboard data in the controller, not the report model

---
 .../admin/models/admin_dashboard.js           |   1 +
 app/controllers/admin/dashboard_controller.rb |   3 +-
 app/models/report.rb                          |  72 ++----
 spec/models/report_spec.rb                    | 218 ++++++------------
 4 files changed, 92 insertions(+), 202 deletions(-)

diff --git a/app/assets/javascripts/admin/models/admin_dashboard.js b/app/assets/javascripts/admin/models/admin_dashboard.js
index f98ad76dfef..7d134c2fc33 100644
--- a/app/assets/javascripts/admin/models/admin_dashboard.js
+++ b/app/assets/javascripts/admin/models/admin_dashboard.js
@@ -5,6 +5,7 @@ Discourse.AdminDashboard.reopenClass({
     var model = Discourse.AdminDashboard.create();
     return $.ajax("/admin/dashboard", {
       type: 'GET',
+      dataType: 'json',
       success: function(json) {
         model.mergeAttributes(json);
         model.set('loaded', true);
diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index 032d7255235..95f524779ce 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -1,6 +1,7 @@
-
 class Admin::DashboardController < Admin::AdminController
 
+  caches_action :index, expires_in: 1.hour
+
   def index
     render_json_dump(AdminDashboardData.fetch)
   end
diff --git a/app/models/report.rb b/app/models/report.rb
index 7c2b2c4e3d0..309e4c653a0 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -35,98 +35,60 @@ class Report
 
   def self.report_visits(report)
     report.data = []
-    fetch report do
-      UserVisit.by_day(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    UserVisit.by_day(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
   def self.report_signups(report)
     report.data = []
-    fetch report do
-      User.count_by_signup_date(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    User.count_by_signup_date(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
   def self.report_topics(report)
     report.data = []
-    fetch report do
-      Topic.count_per_day(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    Topic.count_per_day(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
   def self.report_posts(report)
     report.data = []
-    fetch report do
-      Post.count_per_day(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    Post.count_per_day(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
   def self.report_flags(report)
     report.data = []
-    fetch report do
-      (0..30).to_a.reverse.each do |i|
-        if (count = PostAction.where('date(created_at) = ?', i.days.ago.to_date).where(post_action_type_id: PostActionType.flag_types.values).count) > 0
-          report.data << {x: i.days.ago.to_date.to_s, y: count}
-        end
+    (0..30).to_a.reverse.each do |i|
+      if (count = PostAction.where('date(created_at) = ?', i.days.ago.to_date).where(post_action_type_id: PostActionType.flag_types.values).count) > 0
+        report.data << {x: i.days.ago.to_date.to_s, y: count}
       end
     end
   end
 
   def self.report_users_by_trust_level(report)
     report.data = []
-    fetch report do
-      User.counts_by_trust_level.each do |level, count|
-        report.data << {x: level.to_i, y: count}
-      end
+    User.counts_by_trust_level.each do |level, count|
+      report.data << {x: level.to_i, y: count}
     end
   end
 
   def self.report_likes(report)
     report.data = []
-    fetch report do
-      PostAction.count_likes_per_day(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    PostAction.count_likes_per_day(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
   def self.report_emails(report)
     report.data = []
-    fetch report do
-      EmailLog.count_per_day(30.days.ago).each do |date, count|
-        report.data << {x: date, y: count}
-      end
+    EmailLog.count_per_day(30.days.ago).each do |date, count|
+      report.data << {x: date, y: count}
     end
   end
 
-  private
-
-    def self.fetch(report)
-      unless report.cache and $redis
-        yield
-        return
-      end
-
-      data_set = "#{report.type}:data"
-      if $redis.exists(data_set)
-        $redis.get(data_set).split('|').each do |pair|
-          date, count = pair.split(',')
-          report.data << {x: date, y: count.to_i}
-        end
-      else
-        yield
-        $redis.setex data_set, cache_expiry, report.data.map { |item| "#{item[:x]},#{item[:y]}" }.join('|')
-      end
-    rescue Redis::BaseConnectionError
-      yield
-    end
-
 end
diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb
index 0c73735be4f..9353fae42da 100644
--- a/spec/models/report_spec.rb
+++ b/spec/models/report_spec.rb
@@ -1,162 +1,88 @@
-# require 'spec_helper'
+require 'spec_helper'
 
-# describe Report do
+describe Report do
 
+  # describe 'visits report' do
+  #   let(:report) { Report.find('visits', cache: false) }
 
-#   describe 'visits report' do
-#     let(:report) { Report.find('visits', cache: false) }
+  #   context "no visits" do
+  #     it "returns an empty report" do
+  #       report.data.should be_blank
+  #     end
+  #   end
 
-#     context "no visits" do
-#       it "returns an empty report" do
-#         report.data.should be_blank
-#       end
-#     end
+  #   context "with visits" do
+  #     let(:user) { Fabricate(:user) }
 
-#     context "with visits" do
-#       let(:user) { Fabricate(:user) }
+  #     before do
+  #       user.user_visits.create(visited_at: 1.day.ago)
+  #       user.user_visits.create(visited_at: 2.days.ago)
+  #     end
 
-#       before do
-#         user.user_visits.create(visited_at: 1.day.ago)
-#         user.user_visits.create(visited_at: 2.days.ago)
-#       end
+  #     it "returns a report with data" do
+  #       report.data.should be_present
+  #     end
+  #   end
+  # end
 
-#       it "returns a report with data" do
-#         report.data.should be_present
-#       end
-#     end
-#   end
+  # [:signup, :topic, :post, :flag, :like, :email].each do |arg|
+  #   describe "#{arg} report" do
+  #     pluralized = arg.to_s.pluralize
 
-#   [:signup, :topic, :post, :flag, :like, :email].each do |arg|
-#     describe "#{arg} report" do
-#       pluralized = arg.to_s.pluralize
+  #     let(:report) { Report.find(pluralized, cache: false) }
 
-#       let(:report) { Report.find(pluralized, cache: false) }
+  #     context "no #{pluralized}" do
+  #       it 'returns an empty report' do
+  #         report.data.should be_blank
+  #       end
+  #     end
 
-#       context "no #{pluralized}" do
-#         it 'returns an empty report' do
-#           report.data.should be_blank
-#         end
-#       end
+  #     context "with #{pluralized}" do
+  #       before do
+  #         fabricator = case arg
+  #         when :signup
+  #           :user
+  #         when :email
+  #           :email_log
+  #         else
+  #           arg
+  #         end
+  #         Fabricate(fabricator, created_at: 25.hours.ago)
+  #         Fabricate(fabricator, created_at: 1.hours.ago)
+  #         Fabricate(fabricator, created_at: 1.hours.ago)
+  #       end
 
-#       context "with #{pluralized}" do
-#         before do
-#           fabricator = case arg
-#           when :signup
-#             :user
-#           when :email
-#             :email_log
-#           else
-#             arg
-#           end
-#           Fabricate(fabricator, created_at: 25.hours.ago)
-#           Fabricate(fabricator, created_at: 1.hours.ago)
-#           Fabricate(fabricator, created_at: 1.hours.ago)
-#         end
+  #       it 'returns correct data' do
+  #         report.data[0][:y].should == 1
+  #         report.data[1][:y].should == 2
+  #       end
+  #     end
+  #   end
+  # end
 
-#         it 'returns correct data' do
-#           report.data[0][:y].should == 1
-#           report.data[1][:y].should == 2
-#         end
-#       end
-#     end
-#   end
+  describe 'users by trust level report' do
+    let(:report) { Report.find('users_by_trust_level', cache: false) }
 
-#   describe 'users by trust level report' do
-#     let(:report) { Report.find('users_by_trust_level', cache: false) }
+    context "no users" do
+      it "returns an empty report" do
+        report.data.should be_blank
+      end
+    end
 
-#     context "no users" do
-#       it "returns an empty report" do
-#         report.data.should be_blank
-#       end
-#     end
+    context "with users at different trust levels" do
+      before do
+        3.times { Fabricate(:user, trust_level: TrustLevel.levels[:visitor]) }
+        2.times { Fabricate(:user, trust_level: TrustLevel.levels[:regular]) }
+        Fabricate(:user, trust_level: TrustLevel.levels[:elder])
+      end
 
-#     context "with users at different trust levels" do
-#       before do
-#         3.times { Fabricate(:user, trust_level: TrustLevel.levels[:new]) }
-#         2.times { Fabricate(:user, trust_level: TrustLevel.levels[:regular]) }
-#         Fabricate(:user, trust_level: TrustLevel.levels[:moderator])
-#       end
+      it "returns a report with data" do
+        report.data.should be_present
+        report.data.find {|d| d[:x] == TrustLevel.levels[:visitor]} [:y].should == 3
+        report.data.find {|d| d[:x] == TrustLevel.levels[:regular]}[:y].should == 2
+        report.data.find {|d| d[:x] == TrustLevel.levels[:elder]}[:y].should == 1
+      end
+    end
+  end
 
-#       it "returns a report with data" do
-#         report.data.should be_present
-#         report.data.find {|d| d[:x] == TrustLevel.levels[:new]} [:y].should == 3
-#         report.data.find {|d| d[:x] == TrustLevel.levels[:regular]}[:y].should == 2
-#         report.data.find {|d| d[:x] == TrustLevel.levels[:moderator]}[:y].should == 1
-#       end
-#     end
-#   end
-
-#   describe '#fetch' do
-#     context 'signups' do
-#       let(:report) { Report.find('signups', cache: true) }
-
-#       context 'no data' do
-#         context 'cache miss' do
-#           before do
-#             $redis.expects(:exists).with('signups:data').returns(false)
-#           end
-
-#           it 'should cache an empty data set' do
-#             $redis.expects(:setex).with('signups:data', Report.cache_expiry, "")
-#             report.data.should be_blank
-#           end
-#         end
-
-#         context 'cache hit' do
-#           before do
-#             $redis.expects(:exists).with('signups:data').returns(true)
-#           end
-
-#           it 'returns the cached empty report' do
-#             User.expects(:count_by_signup_date).never
-#             $redis.expects(:setex).never
-#             $redis.expects(:get).with('signups:data').returns('')
-#             report.data.should be_blank
-#           end
-#         end
-#       end
-
-#       context 'with data' do
-#         before do
-#           Fabricate(:user, created_at: 25.hours.ago)
-#           Fabricate(:user, created_at: 1.hour.ago)
-#           Fabricate(:user, created_at: 1.hour.ago)
-#         end
-
-#         context 'cache miss' do
-#           before do
-#             $redis.expects(:exists).with('signups:data').returns(false)
-#           end
-
-#           it 'should cache the data set' do
-#             $redis.expects(:setex).with do |key, expiry, string|
-#               string =~ /(\d)+-(\d)+-(\d)+,1/ and string =~ /(\d)+-(\d)+-(\d)+,2/
-#             end
-#             report()
-#           end
-
-#           it 'should return correct data' do
-#             report.data[0][:y].should == 1
-#             report.data[1][:y].should == 2
-#           end
-#         end
-
-#         context 'cache hit' do
-#           before do
-#             $redis.expects(:exists).with('signups:data').returns(true)
-#           end
-
-#           it 'returns the cached data' do
-#             User.expects(:count_by_signup_date).never
-#             $redis.expects(:setex).never
-#             $redis.expects(:get).with('signups:data').returns("#{2.days.ago.to_date.to_s},1|#{1.day.ago.to_date.to_s},2")
-#             report.data[0][:y].should == 1
-#             report.data[1][:y].should == 2
-#           end
-#         end
-#       end
-#     end
-#   end
-
-
-# end
+end
\ No newline at end of file