From 6ef345c5d89a0fd9b3dc0cbb65ba432e2c997c0a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 21 Apr 2020 10:45:57 +0200 Subject: [PATCH 01/11] Touch taxon when a taxon is applied to a product --- app/models/spree/classification_decorator.rb | 1 + spec/models/spree/taxon_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/app/models/spree/classification_decorator.rb b/app/models/spree/classification_decorator.rb index fec270bc14..20608bad94 100644 --- a/app/models/spree/classification_decorator.rb +++ b/app/models/spree/classification_decorator.rb @@ -1,5 +1,6 @@ Spree::Classification.class_eval do belongs_to :product, class_name: "Spree::Product", touch: true + belongs_to :taxon, class_name: "Spree::Taxon", touch: true before_destroy :dont_destroy_if_primary_taxon diff --git a/spec/models/spree/taxon_spec.rb b/spec/models/spree/taxon_spec.rb index 5a565c5146..b19f979d99 100644 --- a/spec/models/spree/taxon_spec.rb +++ b/spec/models/spree/taxon_spec.rb @@ -28,5 +28,14 @@ module Spree expect(Taxon.distributed_taxons(:current)).to eq(e.id => Set.new([t1.id])) end end + + describe "touches" do + let!(:taxon) { create(:taxon) } + let!(:product) { create(:simple_product) } + + it "is touched when applied to a product" do + expect{ product.taxons << taxon }.to change { taxon.reload.updated_at } + end + end end end From 1b18808d2164b0163ade47273849c9508d13d82c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 22 Apr 2020 22:44:02 +0200 Subject: [PATCH 02/11] Touch ShippingMethod when it's assigned to a new distributor --- app/models/distributor_shipping_method.rb | 2 +- spec/models/spree/shipping_method_spec.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/distributor_shipping_method.rb b/app/models/distributor_shipping_method.rb index 5c69d18084..a2c81506dd 100644 --- a/app/models/distributor_shipping_method.rb +++ b/app/models/distributor_shipping_method.rb @@ -1,5 +1,5 @@ class DistributorShippingMethod < ActiveRecord::Base self.table_name = "distributors_shipping_methods" - belongs_to :shipping_method, class_name: Spree::ShippingMethod + belongs_to :shipping_method, class_name: Spree::ShippingMethod, touch: true belongs_to :distributor, class_name: Enterprise, touch: true end diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index 7bbc506a0f..94501eaa4e 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -109,5 +109,15 @@ module Spree expect(shipping_method.include?(address)).to be true end end + + describe "touches" do + let!(:distributor) { create(:distributor_enterprise) } + let!(:shipping_method) { create(:shipping_method) } + let(:add_distributor) { shipping_method.distributors << distributor } + + it "is touched when applied to a distributor" do + expect{ add_distributor }.to change { shipping_method.reload.updated_at} + end + end end end From eb5f8b85ff6f9ee6bd1c094df0d73ba91ead8c51 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 24 Apr 2020 12:46:55 +0200 Subject: [PATCH 03/11] Touch primary taxon when a product's primary_taxon is changed --- app/models/spree/product_decorator.rb | 2 +- spec/models/spree/taxon_spec.rb | 17 +++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index d136c2b681..fe20fe5194 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -12,7 +12,7 @@ Spree::Product.class_eval do has_many :option_types, through: :product_option_types, dependent: :destroy belongs_to :supplier, class_name: 'Enterprise', touch: true - belongs_to :primary_taxon, class_name: 'Spree::Taxon' + belongs_to :primary_taxon, class_name: 'Spree::Taxon', touch: true delegate_belongs_to :master, :unit_value, :unit_description delegate :images_attributes=, :display_as=, to: :master diff --git a/spec/models/spree/taxon_spec.rb b/spec/models/spree/taxon_spec.rb index b19f979d99..54a0069da9 100644 --- a/spec/models/spree/taxon_spec.rb +++ b/spec/models/spree/taxon_spec.rb @@ -30,11 +30,20 @@ module Spree end describe "touches" do - let!(:taxon) { create(:taxon) } - let!(:product) { create(:simple_product) } + let!(:taxon1) { create(:taxon) } + let!(:taxon2) { create(:taxon) } + let!(:taxon3) { create(:taxon) } + let!(:product) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1, taxon2]) } - it "is touched when applied to a product" do - expect{ product.taxons << taxon }.to change { taxon.reload.updated_at } + it "is touched when a taxon is applied to a product" do + expect{ product.taxons << taxon3 }.to change { taxon3.reload.updated_at } + end + + it "is touched when assignment of primary_taxon on a product changes" do + expect do + product.primary_taxon = taxon2 + product.save + end.to change { taxon2.reload.updated_at } end end end From 53ebe10483c568cac37ab6ae64887a7ef2ca456c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 22 Apr 2020 00:07:05 +0200 Subject: [PATCH 04/11] Fix issue with generic primary taxon in product factory changing which taxons are correctly counted as being in open order cycles. --- spec/features/consumer/producers_spec.rb | 4 ++-- spec/features/consumer/shops_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/features/consumer/producers_spec.rb b/spec/features/consumer/producers_spec.rb index f8d7e3bb57..d58ebe1397 100644 --- a/spec/features/consumer/producers_spec.rb +++ b/spec/features/consumer/producers_spec.rb @@ -16,8 +16,8 @@ feature ' let(:taxon_fruit) { create(:taxon, name: 'Fruit') } let(:taxon_veg) { create(:taxon, name: 'Vegetables') } - let!(:product1) { create(:simple_product, supplier: producer1, taxons: [taxon_fruit]) } - let!(:product2) { create(:simple_product, supplier: producer2, taxons: [taxon_veg]) } + let!(:product1) { create(:simple_product, supplier: producer1, primary_taxon: taxon_fruit, taxons: [taxon_fruit]) } + let!(:product2) { create(:simple_product, supplier: producer2, primary_taxon: taxon_veg, taxons: [taxon_veg]) } let(:shop) { create(:distributor_enterprise) } let!(:er) { create(:enterprise_relationship, parent: shop, child: producer1) } diff --git a/spec/features/consumer/shops_spec.rb b/spec/features/consumer/shops_spec.rb index 87db26883c..236e556b79 100644 --- a/spec/features/consumer/shops_spec.rb +++ b/spec/features/consumer/shops_spec.rb @@ -110,13 +110,13 @@ feature 'Shops', js: true do describe "taxon badges" do let!(:closed_oc) { create(:closed_order_cycle, distributors: [shop], variants: [p_closed.variants.first]) } - let!(:p_closed) { create(:simple_product, taxons: [taxon_closed]) } + let!(:p_closed) { create(:simple_product, primary_taxon: taxon_closed, taxons: [taxon_closed]) } let(:shop) { create(:distributor_enterprise, with_payment_and_shipping: true) } let(:taxon_closed) { create(:taxon, name: 'Closed') } describe "open shops" do let!(:open_oc) { create(:open_order_cycle, distributors: [shop], variants: [p_open.variants.first]) } - let!(:p_open) { create(:simple_product, taxons: [taxon_open]) } + let!(:p_open) { create(:simple_product, primary_taxon: taxon_open, taxons: [taxon_open]) } let(:taxon_open) { create(:taxon, name: 'Open') } it "shows taxons for open order cycles only" do From 4d098448f5f1997f106ca7d98c86e6b64c5537e1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Mon, 20 Apr 2020 19:14:55 +0200 Subject: [PATCH 05/11] Cache rendered AMS arrays in darkswarm layout based on latest timestamps of rendered object classes --- app/views/layouts/darkswarm.html.haml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index fa42b8f745..d080be45dd 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -48,8 +48,10 @@ = inject_current_hub = inject_current_user = inject_rails_flash - = inject_taxons - = inject_properties + - cache "inject-all-taxons-#{CacheService.latest_timestamp_by_class(Spree::Taxon)}" do + = inject_taxons + - cache "inject-all-properties-#{CacheService.latest_timestamp_by_class(Spree::Property)}" do + = inject_properties = inject_current_order = inject_currency_config = yield :injection_data From ec581dccb83c627265f1be72ece768d1563ab36f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 28 Apr 2020 14:32:51 +0200 Subject: [PATCH 06/11] Use class-based caching for queries in EnterpriseInjectionData --- .../enterprise_injection_data.rb | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/open_food_network/enterprise_injection_data.rb b/lib/open_food_network/enterprise_injection_data.rb index 8bc386b2a6..dd6db131af 100644 --- a/lib/open_food_network/enterprise_injection_data.rb +++ b/lib/open_food_network/enterprise_injection_data.rb @@ -10,11 +10,25 @@ module OpenFoodNetwork end def shipping_method_services - @shipping_method_services ||= Spree::ShippingMethod.services + @shipping_method_services ||= begin + CacheService.cached_data_by_class("shipping_method_services", Spree::ShippingMethod) do + # This result relies on a simple join with DistributorShippingMethod. + # Updated DistributorShippingMethod records touch their associated Spree::ShippingMethod. + Spree::ShippingMethod.services + end + end end def supplied_taxons - @supplied_taxons ||= Spree::Taxon.supplied_taxons + @supplied_taxons ||= begin + CacheService.cached_data_by_class("supplied_taxons", Spree::Taxon) do + # This result relies on a join with associated supplied products, through the + # class Classification which maps the relationship. Classification records touch + # their associated Spree::Taxon when updated. A Spree::Product's primary_taxon + # is also touched when changed. + Spree::Taxon.supplied_taxons + end + end end def all_distributed_taxons From 210029aaff2a3aa2a88a3aafa7d2709aa8315349 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 07/11] 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 a35f8cdb02575b6d63b010de23f37c35b1023ffb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 30 Apr 2020 09:45:02 +0200 Subject: [PATCH 08/11] Move cache keys used in views into the cache service --- app/services/cache_service.rb | 13 +++++++++++++ app/views/layouts/darkswarm.html.haml | 4 ++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/app/services/cache_service.rb b/app/services/cache_service.rb index 4a87dcea9a..ba19497529 100644 --- a/app/services/cache_service.rb +++ b/app/services/cache_service.rb @@ -20,4 +20,17 @@ class CacheService def self.latest_timestamp_by_class(cached_class) cached_class.maximum(:updated_at).to_i 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. + + def self.ams_all_taxons_key + "inject-all-taxons-#{CacheService.latest_timestamp_by_class(Spree::Taxon)}" + end + + def self.ams_all_properties_key + "inject-all-properties-#{CacheService.latest_timestamp_by_class(Spree::Property)}" + end + end end diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index d080be45dd..50b2d3f812 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -48,9 +48,9 @@ = inject_current_hub = inject_current_user = inject_rails_flash - - cache "inject-all-taxons-#{CacheService.latest_timestamp_by_class(Spree::Taxon)}" do + - cache CacheService::FragmentCaching.ams_all_taxons_key do = inject_taxons - - cache "inject-all-properties-#{CacheService.latest_timestamp_by_class(Spree::Property)}" do + - cache CacheService::FragmentCaching.ams_all_properties_key do = inject_properties = inject_current_order = inject_currency_config From f724d1b57270be24331d860140c5543d842a05d2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Wed, 29 Apr 2020 21:47:20 +0200 Subject: [PATCH 09/11] Add feature spec for caching properties and taxons AMS --- spec/features/consumer/caching_spec.rb | 64 ++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 spec/features/consumer/caching_spec.rb diff --git a/spec/features/consumer/caching_spec.rb b/spec/features/consumer/caching_spec.rb new file mode 100644 index 0000000000..92e6464142 --- /dev/null +++ b/spec/features/consumer/caching_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require "spec_helper" + +feature "Caching", js: true, caching: true do + let!(:taxon) { create(:taxon, name: "Cached Taxon") } + let!(:property) { create(:property, presentation: "Cached Property") } + + let!(:producer) { create(:supplier_enterprise) } + let!(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true, is_primary_producer: true) } + let!(:product) { create(:simple_product, supplier: producer, primary_taxon: taxon, taxons: [taxon], properties: [property]) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor], coordinator: distributor) } + let(:exchange) { order_cycle.exchanges.outgoing.where(receiver_id: distributor.id).first } + + before do + exchange.variants << product.variants.first + end + + describe "caching injected taxons and properties" do + it "caches taxons and properties" do + visit shops_path + + taxon_timestamp1 = CacheService.latest_timestamp_by_class(Spree::Taxon) + expect_cached "views/#{CacheService::FragmentCaching.ams_all_taxons_key}" + + property_timestamp1 = CacheService.latest_timestamp_by_class(Spree::Property) + expect_cached "views/#{CacheService::FragmentCaching.ams_all_properties_key}" + + toggle_filters + + within "#hubs .filter-row" do + expect(page).to have_content taxon.name + expect(page).to have_content property.presentation + end + + taxon.name = "Changed Taxon" + taxon.save + property.presentation = "Changed Property" + property.save + + visit shops_path + + taxon_timestamp2 = CacheService.latest_timestamp_by_class(Spree::Taxon) + expect_cached "views/#{CacheService::FragmentCaching.ams_all_taxons_key}" + + property_timestamp2 = CacheService.latest_timestamp_by_class(Spree::Property) + expect_cached "views/#{CacheService::FragmentCaching.ams_all_properties_key}" + + expect(taxon_timestamp1).to_not eq taxon_timestamp2 + expect(property_timestamp1).to_not eq property_timestamp2 + + toggle_filters + + within "#hubs .filter-row" do + expect(page).to have_content "Changed Taxon" + expect(page).to have_content "Changed Property" + end + end + end + + def expect_cached(key) + expect(Rails.cache.exist?(key)).to be true + end +end From 7a22f7f783edcf334a227908be020a5aca780df1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 30 Apr 2020 15:44:53 +0200 Subject: [PATCH 10/11] Move and rename caching spec --- .../{caching_spec.rb => caching/darkwarm_caching_spec.rb} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename spec/features/consumer/{caching_spec.rb => caching/darkwarm_caching_spec.rb} (97%) diff --git a/spec/features/consumer/caching_spec.rb b/spec/features/consumer/caching/darkwarm_caching_spec.rb similarity index 97% rename from spec/features/consumer/caching_spec.rb rename to spec/features/consumer/caching/darkwarm_caching_spec.rb index 92e6464142..57cc6ede5d 100644 --- a/spec/features/consumer/caching_spec.rb +++ b/spec/features/consumer/caching/darkwarm_caching_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -feature "Caching", js: true, caching: true do +feature "Darkswarm data caching", js: true, caching: true do let!(:taxon) { create(:taxon, name: "Cached Taxon") } let!(:property) { create(:property, presentation: "Cached Property") } From 1990417b72be8b79b9d2ef6899860a32c3925300 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 30 Apr 2020 15:59:26 +0200 Subject: [PATCH 11/11] Test values are not fetched from database when cache exists --- .../consumer/caching/darkwarm_caching_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/features/consumer/caching/darkwarm_caching_spec.rb b/spec/features/consumer/caching/darkwarm_caching_spec.rb index 57cc6ede5d..6dcbc18ac5 100644 --- a/spec/features/consumer/caching/darkwarm_caching_spec.rb +++ b/spec/features/consumer/caching/darkwarm_caching_spec.rb @@ -18,6 +18,18 @@ feature "Darkswarm data caching", js: true, caching: true do describe "caching injected taxons and properties" do it "caches taxons and properties" do + expect(Spree::Taxon).to receive(:all) { [taxon] } + expect(Spree::Property).to receive(:all) { [property] } + + visit shops_path + + expect(Spree::Taxon).to_not receive(:all) + expect(Spree::Property).to_not receive(:all) + + visit shops_path + end + + it "invalidates caches for taxons and properties" do visit shops_path taxon_timestamp1 = CacheService.latest_timestamp_by_class(Spree::Taxon)