Merge pull request #5317 from Matt-Yorkley/caching-timed-shops

[Caching] Timed caching on shops data

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

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

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'

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

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

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

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