From b4e0c5afe03394d6ce5cbde6d7218a2713ca16d4 Mon Sep 17 00:00:00 2001 From: Guo Xiang Tan Date: Thu, 21 Apr 2016 14:45:16 +0800 Subject: [PATCH] FIX: Fetch stats if it has not been cached. --- app/jobs/scheduled/about_stats.rb | 6 +---- app/jobs/scheduled/dashboard_stats.rb | 6 +---- app/jobs/stats.rb | 9 ------- app/models/concerns/stats_cacheable.rb | 17 ++++++++++++- spec/jobs/about_stats_spec.rb | 12 ++++++--- spec/jobs/dashboard_stats_spec.rb | 16 +++++++++--- .../shared_examples_for_stats_cacheable.rb | 25 +++++++++++-------- 7 files changed, 53 insertions(+), 38 deletions(-) delete mode 100644 app/jobs/stats.rb diff --git a/app/jobs/scheduled/about_stats.rb b/app/jobs/scheduled/about_stats.rb index d745f8a0d..1ae2a0866 100644 --- a/app/jobs/scheduled/about_stats.rb +++ b/app/jobs/scheduled/about_stats.rb @@ -1,13 +1,9 @@ module Jobs class AboutStats < Jobs::Scheduled - include Jobs::Stats - every 30.minutes def execute(args) - stats = About.new.stats - set_cache(About, stats) - stats + About.refresh_stats end end end diff --git a/app/jobs/scheduled/dashboard_stats.rb b/app/jobs/scheduled/dashboard_stats.rb index b891b182a..2b35fc438 100644 --- a/app/jobs/scheduled/dashboard_stats.rb +++ b/app/jobs/scheduled/dashboard_stats.rb @@ -1,7 +1,5 @@ module Jobs class DashboardStats < Jobs::Scheduled - include Jobs::Stats - every 30.minutes def execute(args) @@ -12,9 +10,7 @@ module Jobs GroupMessage.create(Group[:admins].name, :dashboard_problems, {limit_once_per: 7.days.to_i}) end - stats = AdminDashboardData.fetch_stats - set_cache(AdminDashboardData, stats) - stats + AdminDashboardData.refresh_stats end end end diff --git a/app/jobs/stats.rb b/app/jobs/stats.rb deleted file mode 100644 index 637091cc6..000000000 --- a/app/jobs/stats.rb +++ /dev/null @@ -1,9 +0,0 @@ -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 diff --git a/app/models/concerns/stats_cacheable.rb b/app/models/concerns/stats_cacheable.rb index d7364111f..150c2a7b2 100644 --- a/app/models/concerns/stats_cacheable.rb +++ b/app/models/concerns/stats_cacheable.rb @@ -18,7 +18,22 @@ module StatsCacheable 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 + stats = refresh_stats if !stats + JSON.parse(stats).with_indifferent_access + end + + def refresh_stats + stats = fetch_stats.to_json + set_cache(stats) + stats + end + + private + + def set_cache(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 stats_cache_key, (recalculate_stats_interval + 5).minutes, stats end end end diff --git a/spec/jobs/about_stats_spec.rb b/spec/jobs/about_stats_spec.rb index 25ae8291f..85a1020e6 100644 --- a/spec/jobs/about_stats_spec.rb +++ b/spec/jobs/about_stats_spec.rb @@ -1,10 +1,16 @@ require 'rails_helper' describe Jobs::AboutStats do + after do + $redis.flushall + end + 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) + stats = About.fetch_stats.to_json + cache_key = About.stats_cache_key + + expect($redis.get(cache_key)).to eq(nil) expect(described_class.new.execute({})).to eq(stats) + expect($redis.get(cache_key)).to eq(stats) end end diff --git a/spec/jobs/dashboard_stats_spec.rb b/spec/jobs/dashboard_stats_spec.rb index b00130d02..82ff12edc 100644 --- a/spec/jobs/dashboard_stats_spec.rb +++ b/spec/jobs/dashboard_stats_spec.rb @@ -1,10 +1,18 @@ require 'rails_helper' describe Jobs::DashboardStats do + after do + $redis.flushall + end + 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) + Timecop.freeze do + stats = AdminDashboardData.fetch_stats.to_json + cache_key = AdminDashboardData.stats_cache_key + + expect($redis.get(cache_key)).to eq(nil) + expect(described_class.new.execute({})).to eq(stats) + expect($redis.get(cache_key)).to eq(stats) + end end end diff --git a/spec/support/shared_examples_for_stats_cacheable.rb b/spec/support/shared_examples_for_stats_cacheable.rb index d47877c58..b6a2b4dfe 100644 --- a/spec/support/shared_examples_for_stats_cacheable.rb +++ b/spec/support/shared_examples_for_stats_cacheable.rb @@ -1,22 +1,25 @@ 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 + after do + $redis.flushall end - it 'returns nil if no stats has been cached' do - expect(described_class.fetch_cached_stats).to eq(nil) + it 'returns the cached stats' do + stats = described_class.fetch_stats.to_json + $redis.set(described_class.stats_cache_key, stats) + expect(described_class.fetch_cached_stats).to eq(JSON.parse(stats)) + end + + it 'returns fetches the stats if stats has not been cached' do + Timecop.freeze do + $redis.del(described_class.stats_cache_key) + expect(described_class.fetch_cached_stats).to eq(JSON.parse(described_class.fetch_stats.to_json)) + end end end describe 'fetch_stats' do - it 'has been implemented' do + it 'has not been implemented' do expect{ described_class.fetch_stats }.to_not raise_error end end