From ac166f359054c2d03b0cd5dee047fe794600e7c2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 28 Apr 2020 14:15:16 +0200 Subject: [PATCH 1/8] Add CacheService --- app/services/cache_service.rb | 23 +++++++++++++ spec/services/cache_service_spec.rb | 50 +++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 app/services/cache_service.rb create mode 100644 spec/services/cache_service_spec.rb diff --git a/app/services/cache_service.rb b/app/services/cache_service.rb new file mode 100644 index 0000000000..4a87dcea9a --- /dev/null +++ b/app/services/cache_service.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CacheService + def self.cache(cache_key, options = {}) + Rails.cache.fetch cache_key.to_s, options do + yield + end + end + + # Yields a cached query, expired by the most recently updated record for a given class. + # E.g: if *any* Spree::Taxon record is updated, all keys based on Spree::Taxon will auto-expire. + def self.cached_data_by_class(cache_key, cached_class) + Rails.cache.fetch "#{cache_key}-#{cached_class}-#{latest_timestamp_by_class(cached_class)}" do + yield + end + end + + # Gets the :updated_at value of the most recently updated record for a given class, and returns + # it as a timestamp, eg: `1583836069`. + def self.latest_timestamp_by_class(cached_class) + cached_class.maximum(:updated_at).to_i + end +end diff --git a/spec/services/cache_service_spec.rb b/spec/services/cache_service_spec.rb new file mode 100644 index 0000000000..22a6e2b772 --- /dev/null +++ b/spec/services/cache_service_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +describe CacheService do + let(:rails_cache) { Rails.cache } + + describe "#cache" do + before do + rails_cache.stub(:fetch) + end + + it "provides a wrapper for basic #fetch calls to Rails.cache" do + CacheService.cache("test-cache-key", expires_in: 10.seconds) do + "TEST" + end + + expect(rails_cache).to have_received(:fetch).with("test-cache-key", expires_in: 10.seconds) + end + end + + describe "#cached_data_by_class" do + let(:timestamp) { Time.now.to_i } + + before do + rails_cache.stub(:fetch) + CacheService.stub(:latest_timestamp_by_class) { timestamp } + end + + it "caches data by timestamp for last record of that class" do + CacheService.cached_data_by_class("test-cache-key", Enterprise) 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 + + describe "#latest_timestamp_by_class" do + let!(:taxon1) { create(:taxon) } + let!(:taxon2) { create(:taxon) } + + it "gets the :updated_at value of the last record for a given class and returns a timestamp" do + taxon1.touch + expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).to eq taxon1.updated_at.to_i + + taxon2.touch + expect(CacheService.latest_timestamp_by_class(Spree::Taxon)).to eq taxon2.updated_at.to_i + end + end +end From 975afb3152b8976c62cda2c447696414eb13a571 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 22 Apr 2020 12:35:39 +0200 Subject: [PATCH 2/8] Enable use of Action Caching in the API :tada: See: https://guides.rubyonrails.org/api_app.html#adding-other-modules --- app/models/concerns/api_action_caching.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 app/models/concerns/api_action_caching.rb diff --git a/app/models/concerns/api_action_caching.rb b/app/models/concerns/api_action_caching.rb new file mode 100644 index 0000000000..ef0eba604d --- /dev/null +++ b/app/models/concerns/api_action_caching.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +# API controllers inherit from ActionController::Metal to keep them slim and fast. +# This concern adds the minimum requirements needed to use Action Caching in the API. + +module ApiActionCaching + extend ActiveSupport::Concern + + included do + include ActionController::Caching + include ActionController::Caching::Actions + include AbstractController::Layouts + + # This config is not passed to the controller automatically with ActionController::Metal + self.cache_store = Rails.configuration.cache_store + + # ActionController::Caching asks for a controller's layout, but they're not used in the API + layout false + end +end From ea1ec1a1c67df84173a7fb8b83a218e50e141b17 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 28 Apr 2020 14:20:44 +0200 Subject: [PATCH 3/8] Use ActionCaching in OrderCyclesController (taxons and properties) --- app/controllers/api/order_cycles_controller.rb | 4 +++- app/services/cache_service.rb | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/order_cycles_controller.rb b/app/controllers/api/order_cycles_controller.rb index 0ac5223ea1..e2bca5a626 100644 --- a/app/controllers/api/order_cycles_controller.rb +++ b/app/controllers/api/order_cycles_controller.rb @@ -1,10 +1,12 @@ module Api class OrderCyclesController < Api::BaseController include EnterprisesHelper - respond_to :json + include ApiActionCaching skip_authorization_check + caches_action :taxons, :properties, expires_in: CacheService::FILTERS_EXPIRY + def products products = ProductsRenderer.new( distributor, diff --git a/app/services/cache_service.rb b/app/services/cache_service.rb index 4a87dcea9a..610b4ca6b4 100644 --- a/app/services/cache_service.rb +++ b/app/services/cache_service.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class CacheService + FILTERS_EXPIRY = 30.seconds.freeze + def self.cache(cache_key, options = {}) Rails.cache.fetch cache_key.to_s, options do yield From a25a75bbe892276171945e3452cf78458d52cdc3 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 22 Apr 2020 19:40:24 +0200 Subject: [PATCH 4/8] Ensure action caching includes GET params By default the auto-generated action cache keys looks like this: `views/0.0.0.0:3000/api/order_cycles/1/properties` With this change the cache keys now look like this: `views/0.0.0.0:3000/api/order_cycles/1/properties?distributor=3` --- app/controllers/api/order_cycles_controller.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/order_cycles_controller.rb b/app/controllers/api/order_cycles_controller.rb index e2bca5a626..f92927b0fe 100644 --- a/app/controllers/api/order_cycles_controller.rb +++ b/app/controllers/api/order_cycles_controller.rb @@ -5,7 +5,9 @@ module Api skip_authorization_check - caches_action :taxons, :properties, expires_in: CacheService::FILTERS_EXPIRY + caches_action :taxons, :properties, + expires_in: CacheService::FILTERS_EXPIRY, + cache_path: proc { |controller| controller.request.url } def products products = ProductsRenderer.new( From 61d7adaf74a1276445e634599ed6bf06e82af4a1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 29 Apr 2020 20:04:37 +0200 Subject: [PATCH 5/8] Enable optional `caching` tag in test metadata --- spec/spec_helper.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5eb8cad557..ec5311f1eb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -114,6 +114,15 @@ RSpec.configure do |config| .each { |s| s.driver.reset! } end + # Enable caching in any specs tagged with `caching: true`. Usage is exactly the same as the + # well-known `js: true` tag used to enable javascript in feature specs. + config.around(:each, :caching) do |example| + caching = ActionController::Base.perform_caching + ActionController::Base.perform_caching = example.metadata[:caching] + example.run + ActionController::Base.perform_caching = caching + end + config.before(:all) { restart_phantomjs } # Geocoding From 7e4c00ba3f97b80e9df947f08ecee92f893d8d3b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 May 2020 18:44:48 +0200 Subject: [PATCH 6/8] Ensure caching works in cache tests for API controllers --- app/models/concerns/api_action_caching.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/api_action_caching.rb b/app/models/concerns/api_action_caching.rb index ef0eba604d..05dbc9f0d4 100644 --- a/app/models/concerns/api_action_caching.rb +++ b/app/models/concerns/api_action_caching.rb @@ -11,8 +11,9 @@ module ApiActionCaching include ActionController::Caching::Actions include AbstractController::Layouts - # This config is not passed to the controller automatically with ActionController::Metal + # These configs are not assigned to the controller automatically with ActionController::Metal self.cache_store = Rails.configuration.cache_store + self.perform_caching = ActionController::Base.perform_caching # ActionController::Caching asks for a controller's layout, but they're not used in the API layout false From 4aac97c9855fc633730ac1ee16c70f32ad51ebee Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 5 May 2020 20:34:26 +0200 Subject: [PATCH 7/8] Add feature spec for action caching of taxon and properties endpoints --- .../consumer/caching/shops_caching_spec.rb | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 spec/features/consumer/caching/shops_caching_spec.rb diff --git a/spec/features/consumer/caching/shops_caching_spec.rb b/spec/features/consumer/caching/shops_caching_spec.rb new file mode 100644 index 0000000000..7d70796e37 --- /dev/null +++ b/spec/features/consumer/caching/shops_caching_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "spec_helper" + +feature "Shops caching", js: true, caching: true do + include WebHelper + include UIComponentHelper + + 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 "API action caching on taxons and properties" do + let!(:taxon) { create(:taxon, name: "Cached Taxon") } + let!(:taxon2) { create(:taxon, name: "New Taxon") } + let!(:property) { create(:property, presentation: "Cached Property") } + let!(:property2) { create(:property, presentation: "New Property") } + let!(:product) { create(:product, taxons: [taxon], primary_taxon: taxon, properties: [property]) } + let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } + + let(:test_domain) { "#{Capybara.current_session.server.host}:#{Capybara.current_session.server.port}" } + let(:taxons_key) { "views/#{test_domain}/api/order_cycles/#{order_cycle.id}/taxons?distributor=#{distributor.id}" } + let(:properties_key) { "views/#{test_domain}/api/order_cycles/#{order_cycle.id}/properties?distributor=#{distributor.id}" } + let(:options) { { expires_in: CacheService::FILTERS_EXPIRY } } + + before do + exchange.variants << product.variants.first + end + + it "caches rendered response for taxons and properties, with the provided options" do + visit enterprise_shop_path(distributor) + + expect(page).to have_content "Cached Taxon" + expect(page).to have_content "Cached Property" + + expect_cached taxons_key, options + expect_cached properties_key, options + end + + it "keeps data cached for a short time on subsequent requests" do + # One minute ago... + Timecop.travel(Time.zone.now - 1.minute) do + visit enterprise_shop_path(distributor) + + expect(page).to have_content taxon.name + expect(page).to have_content property.presentation + + product.update_attribute(:taxons, [taxon2]) + product.update_attribute(:primary_taxon, taxon2) + product.update_attribute(:properties, [property2]) + + visit enterprise_shop_path(distributor) + + expect(page).to have_content taxon.name # Taxon list is unchanged + expect(page).to have_content property.presentation # Property list is unchanged + end + + # A while later... + visit enterprise_shop_path(distributor) + + expect(page).to have_content taxon2.name + expect(page).to have_content property2.presentation + end + end + + def expect_cached(key, options = {}) + expect(Rails.cache.exist?(key, options)).to be true + end +end From 9c24c1cd05a1e71d8c17600da2c1cc73fbf7d941 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 6 May 2020 12:24:33 +0200 Subject: [PATCH 8/8] Ensure #caches_action works in test suite --- app/models/concerns/api_action_caching.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/api_action_caching.rb b/app/models/concerns/api_action_caching.rb index 05dbc9f0d4..b3e030e408 100644 --- a/app/models/concerns/api_action_caching.rb +++ b/app/models/concerns/api_action_caching.rb @@ -13,7 +13,7 @@ module ApiActionCaching # These configs are not assigned to the controller automatically with ActionController::Metal self.cache_store = Rails.configuration.cache_store - self.perform_caching = ActionController::Base.perform_caching + self.perform_caching = true # ActionController::Caching asks for a controller's layout, but they're not used in the API layout false