diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 0a39b00d9b..afde45d3bd 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -18,9 +18,9 @@ class HomeController < BaseController private - # Cache the value of the query count for 24 hours - def cached_count(key, query) - Rails.cache.fetch("home_stats_count_#{key}", expires_in: 1.day, race_condition_ttl: 10) do + # Cache the value of the query count + def cached_count(statistic, query) + CacheService.home_stats(statistic) do query.count end end diff --git a/app/services/cache_service.rb b/app/services/cache_service.rb index 36c33025f5..1963b2f1dc 100644 --- a/app/services/cache_service.rb +++ b/app/services/cache_service.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true class CacheService + HOME_STATS_EXPIRY = 1.day.freeze FILTERS_EXPIRY = 30.seconds.freeze + SHOPS_EXPIRY = 15.seconds.freeze def self.cache(cache_key, options = {}) Rails.cache.fetch cache_key.to_s, options do @@ -23,9 +25,17 @@ class CacheService cached_class.maximum(:updated_at).to_i end + def self.home_stats(statistic) + Rails.cache.fetch("home_stats_count_#{statistic}", + expires_in: HOME_STATS_EXPIRY, + race_condition_ttl: 10) do + yield + end + end + module FragmentCaching # Rails' caching in views is called "Fragment Caching" and uses some slightly different logic. - # Note: supplied keys are actually prepended with "view/" under the hood. + # Note: keys supplied here are actually prepended with "views/" under the hood. def self.ams_all_taxons_key "inject-all-taxons-#{CacheService.latest_timestamp_by_class(Spree::Taxon)}" @@ -34,5 +44,19 @@ class CacheService def self.ams_all_properties_key "inject-all-properties-#{CacheService.latest_timestamp_by_class(Spree::Property)}" end + + def self.ams_shops + [ + "shops/index/inject_enterprises", + { expires_in: SHOPS_EXPIRY } + ] + end + + def self.ams_shop(enterprise) + [ + "enterprises/shop/inject_enterprise_shopfront-#{enterprise.id}", + { expires_in: SHOPS_EXPIRY } + ] + end end end diff --git a/app/views/enterprises/shop.html.haml b/app/views/enterprises/shop.html.haml index 893d43678d..fdca8048c8 100644 --- a/app/views/enterprises/shop.html.haml +++ b/app/views/enterprises/shop.html.haml @@ -6,7 +6,8 @@ = current_distributor.logo.url - content_for :injection_data do - = inject_enterprise_shopfront(@enterprise) + - cache(*CacheService::FragmentCaching.ams_shop(@enterprise)) do + = inject_enterprise_shopfront(@enterprise) %shop.darkswarm - if @shopfront_layout == 'embedded' diff --git a/app/views/shops/index.html.haml b/app/views/shops/index.html.haml index d436b85e5e..032096ebb9 100644 --- a/app/views/shops/index.html.haml +++ b/app/views/shops/index.html.haml @@ -2,7 +2,8 @@ = t :shops_title - content_for :injection_data do - = inject_enterprises(@enterprises) + - cache(*CacheService::FragmentCaching.ams_shops) do + = inject_enterprises(@enterprises) #panes #shops.pane diff --git a/spec/features/consumer/caching/darkwarm_caching_spec.rb b/spec/features/consumer/caching/darkwarm_caching_spec.rb index d58ad22b0a..d32ebb8b9e 100644 --- a/spec/features/consumer/caching/darkwarm_caching_spec.rb +++ b/spec/features/consumer/caching/darkwarm_caching_spec.rb @@ -45,10 +45,11 @@ feature "Darkswarm data caching", js: true, caching: true do expect(page).to have_content property.presentation end - taxon.name = "Changed Taxon" - taxon.save - property.presentation = "Changed Property" - property.save + taxon.update_attributes!(name: "Changed Taxon") + property.update_attributes!(presentation: "Changed Property") + + # Clear timed shops cache so we can test uncached supplied properties + clear_shops_cache visit shops_path @@ -73,4 +74,9 @@ feature "Darkswarm data caching", js: true, caching: true do def expect_cached(key) expect(Rails.cache.exist?(key)).to be true end + + def clear_shops_cache + cache_key = "views/#{CacheService::FragmentCaching.ams_shops[0]}" + Rails.cache.delete cache_key + end end diff --git a/spec/features/consumer/caching/shops_caching_spec.rb b/spec/features/consumer/caching/shops_caching_spec.rb index 7d70796e37..654a2f4524 100644 --- a/spec/features/consumer/caching/shops_caching_spec.rb +++ b/spec/features/consumer/caching/shops_caching_spec.rb @@ -9,6 +9,35 @@ feature "Shops caching", js: true, caching: true do let!(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true, is_primary_producer: true) } let!(:order_cycle) { create(:open_order_cycle, distributors: [distributor], coordinator: distributor) } + describe "caching enterprises AMS data" do + it "caches data for all enterprises, with the provided options" do + visit shops_path + + key, options = CacheService::FragmentCaching.ams_shops + expect_cached "views/#{key}", options + end + + it "keeps data cached for a short time on subsequent requests" do + # Ensure sufficient time for requests to load and timed caches to expire + Timecop.travel(10.minutes.ago) do + visit shops_path + + expect(page).to have_content distributor.name + + distributor.name = "New Name" + distributor.save! + + visit shops_path + + expect(page).to_not have_content "New Name" # Displayed name is unchanged + end + + # A while later... + visit shops_path + expect(page).to have_content "New Name" # Displayed name is now changed + end + end + describe "API action caching on taxons and properties" do let!(:taxon) { create(:taxon, name: "Cached Taxon") } let!(:taxon2) { create(:taxon, name: "New Taxon") } @@ -37,8 +66,8 @@ feature "Shops caching", js: true, caching: true do end it "keeps data cached for a short time on subsequent requests" do - # One minute ago... - Timecop.travel(Time.zone.now - 1.minute) do + # Ensure sufficient time for requests to load and timed caches to expire + Timecop.travel(10.minutes.ago) do visit enterprise_shop_path(distributor) expect(page).to have_content taxon.name diff --git a/spec/services/cache_service_spec.rb b/spec/services/cache_service_spec.rb index 22a6e2b772..68d77b7f1e 100644 --- a/spec/services/cache_service_spec.rb +++ b/spec/services/cache_service_spec.rb @@ -5,7 +5,7 @@ describe CacheService do describe "#cache" do before do - rails_cache.stub(:fetch) + allow(rails_cache).to receive(:fetch) end it "provides a wrapper for basic #fetch calls to Rails.cache" do @@ -21,8 +21,8 @@ describe CacheService do let(:timestamp) { Time.now.to_i } before do - rails_cache.stub(:fetch) - CacheService.stub(:latest_timestamp_by_class) { timestamp } + allow(rails_cache).to receive(:fetch) + allow(Enterprise).to receive(:maximum).with(:updated_at).and_return(timestamp) end it "caches data by timestamp for last record of that class" do @@ -30,7 +30,6 @@ describe CacheService do "TEST" end - expect(CacheService).to have_received(:latest_timestamp_by_class).with(Enterprise) expect(rails_cache).to have_received(:fetch).with("test-cache-key-Enterprise-#{timestamp}") end end