From 23034a55d47decd29587fc21cb7af7d7451bf043 Mon Sep 17 00:00:00 2001 From: Will Marshall Date: Wed, 2 Jul 2014 16:04:14 +1000 Subject: [PATCH] Adding tests and full touch behaviour for caching --- app/models/distributor_shipping_method.rb | 5 ++ app/models/enterprise.rb | 3 +- app/models/spree/product_decorator.rb | 6 +++ app/models/spree/shipping_method_decorator.rb | 11 +++- app/serializers/api/enterprise_serializer.rb | 20 +------- ...fields_to_distributors_shipping_methods.rb | 7 +++ db/schema.rb | 10 ++-- .../distributor_shipping_method_spec.rb | 5 ++ spec/models/enterprise_caching_spec.rb | 51 +++++++++++++++++++ 9 files changed, 94 insertions(+), 24 deletions(-) create mode 100644 app/models/distributor_shipping_method.rb create mode 100644 db/migrate/20140702053145_add_fields_to_distributors_shipping_methods.rb create mode 100644 spec/models/distributor_shipping_method_spec.rb create mode 100644 spec/models/enterprise_caching_spec.rb diff --git a/app/models/distributor_shipping_method.rb b/app/models/distributor_shipping_method.rb new file mode 100644 index 0000000000..e765a23e50 --- /dev/null +++ b/app/models/distributor_shipping_method.rb @@ -0,0 +1,5 @@ +class DistributorShippingMethod < ActiveRecord::Base + self.table_name = "distributors_shipping_methods" + belongs_to :shipping_method, class_name: Spree::ShippingMethod + belongs_to :distributor, class_name: Enterprise, touch: true +end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 5b922b15e4..e5250309b4 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -14,7 +14,8 @@ class Enterprise < ActiveRecord::Base has_many :enterprise_roles, :dependent => :destroy has_many :users, through: :enterprise_roles has_and_belongs_to_many :payment_methods, join_table: 'distributors_payment_methods', class_name: 'Spree::PaymentMethod', foreign_key: 'distributor_id' - has_and_belongs_to_many :shipping_methods, join_table: 'distributors_shipping_methods', class_name: 'Spree::ShippingMethod', foreign_key: 'distributor_id' + has_many :distributor_shipping_methods, foreign_key: :distributor_id + has_many :shipping_methods, through: :distributor_shipping_methods delegate :latitude, :longitude, :city, :state_name, :to => :address diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 6de297cba9..e0cc8cc436 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -5,6 +5,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' has_many :product_distributions, :dependent => :destroy has_many :distributors, :through => :product_distributions @@ -27,6 +28,7 @@ Spree::Product.class_eval do after_initialize :set_available_on_to_now, :if => :new_record? after_save :update_units + after_touch :touch_distributors before_save :add_primary_taxon_to_taxons @@ -186,6 +188,10 @@ Spree::Product.class_eval do end end + def touch_distributors + Enterprise.distributing_product(self).each(&:touch) + end + def add_primary_taxon_to_taxons taxons << primary_taxon unless taxons.find_by_id(primary_taxon) end diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb index 5d7bb89a8f..4a2cf75b47 100644 --- a/app/models/spree/shipping_method_decorator.rb +++ b/app/models/spree/shipping_method_decorator.rb @@ -1,5 +1,8 @@ Spree::ShippingMethod.class_eval do - has_and_belongs_to_many :distributors, join_table: 'distributors_shipping_methods', :class_name => 'Enterprise', association_foreign_key: 'distributor_id' + has_many :distributor_shipping_methods + has_many :distributors, through: :distributor_shipping_methods, class_name: 'Enterprise', foreign_key: 'distributor_id' + + after_save :touch_distributors attr_accessible :distributor_ids, :description attr_accessible :require_ship_address @@ -43,4 +46,10 @@ Spree::ShippingMethod.class_eval do def adjustment_label 'Shipping' end + + private + + def touch_distributors + distributors.each(&:touch) + end end diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index c929bf2c55..3ef46f1724 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -1,20 +1,5 @@ class Api::EnterpriseSerializer < ActiveModel::Serializer # To improve this: http://hawkins.io/2013/06/caching_object_graphs_with_active_model_serializers/ - # - # Taxons: - # classifications touch products - # products touch suppliers - # - # Relatives: - # Enterprise_relationships touches parent, child - # Otherwise dereferencing makes easy - # - # Address: - # Fine - # - # Shipping Methods: - # Create distributors_shipping_methods class - # Set up touches def serializable_hash cached_serializer_hash.merge uncached_serializer_hash @@ -33,6 +18,7 @@ end class Api::UncachedEnterpriseSerializer < ActiveModel::Serializer attributes :orders_close_at, :active + has_one :address, serializer: Api::AddressSerializer def orders_close_at OrderCycle.first_closing_for(object).andand.orders_close_at @@ -53,13 +39,11 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer :email, :hash, :logo, :promo_image, :icon, :path, :pickup, :delivery - has_many :distributed_taxons, key: :taxons, serializer: Api::TaxonSerializer + has_many :distributed_taxons, key: :taxons, serializer: Api::IdSerializer has_many :supplied_taxons, serializer: Api::IdSerializer has_many :distributors, key: :hubs, serializer: Api::IdSerializer has_many :suppliers, key: :producers, serializer: Api::IdSerializer - has_one :address, serializer: Api::AddressSerializer - def pickup object.shipping_methods.where(:require_ship_address => false).present? end diff --git a/db/migrate/20140702053145_add_fields_to_distributors_shipping_methods.rb b/db/migrate/20140702053145_add_fields_to_distributors_shipping_methods.rb new file mode 100644 index 0000000000..ed54707cb1 --- /dev/null +++ b/db/migrate/20140702053145_add_fields_to_distributors_shipping_methods.rb @@ -0,0 +1,7 @@ +class AddFieldsToDistributorsShippingMethods < ActiveRecord::Migration + def change + add_column :distributors_shipping_methods, :id, :primary_key + add_column :distributors_shipping_methods, :created_at, :datetime + add_column :distributors_shipping_methods, :updated_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 5932749993..608cdb299b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20140613004344) do +ActiveRecord::Schema.define(:version => 20140702053145) do create_table "adjustment_metadata", :force => true do |t| t.integer "adjustment_id" @@ -163,9 +163,11 @@ ActiveRecord::Schema.define(:version => 20140613004344) do add_index "distributors_payment_methods", ["distributor_id"], :name => "index_distributors_payment_methods_on_distributor_id" add_index "distributors_payment_methods", ["payment_method_id"], :name => "index_distributors_payment_methods_on_payment_method_id" - create_table "distributors_shipping_methods", :id => false, :force => true do |t| - t.integer "distributor_id" - t.integer "shipping_method_id" + create_table "distributors_shipping_methods", :force => true do |t| + t.integer "distributor_id" + t.integer "shipping_method_id" + t.datetime "created_at" + t.datetime "updated_at" end add_index "distributors_shipping_methods", ["distributor_id"], :name => "index_distributors_shipping_methods_on_distributor_id" diff --git a/spec/models/distributor_shipping_method_spec.rb b/spec/models/distributor_shipping_method_spec.rb new file mode 100644 index 0000000000..bd8d7d2973 --- /dev/null +++ b/spec/models/distributor_shipping_method_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe DistributorShippingMethod do + pending "add some examples to (or delete) #{__FILE__}" +end diff --git a/spec/models/enterprise_caching_spec.rb b/spec/models/enterprise_caching_spec.rb new file mode 100644 index 0000000000..af05897a6e --- /dev/null +++ b/spec/models/enterprise_caching_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +describe Enterprise do + context "key-based caching invalidation" do + describe "is touched when a(n)" do + let(:enterprise) { create(:distributor_enterprise, updated_at: 1.week.ago) } + let(:taxon) { create(:taxon) } + + describe "with supplied taxon" do + let(:product) { create(:simple_product, supplier: enterprise) } + let!(:classification) { create(:classification, taxon: taxon, product: product) } + it "supplied taxon is updated" do + expect{classification.save!}.to change{enterprise.updated_at} + end + end + + describe "with distributed taxon" do + let(:product) { create(:simple_product) } + let!(:oc) { create(:simple_order_cycle, distributors: [enterprise], variants: [product.master]) } + let!(:classification) { create(:classification, taxon: taxon, product: product) } + it "distributed taxon is updated" do + expect{classification.save!}.to change{enterprise.reload.updated_at} + end + end + + describe "with relatives" do + let(:child_enterprise) { create(:supplier_enterprise) } + let!(:er) { create(:enterprise_relationship, parent: enterprise, child: child_enterprise) } + it "enterprise relationship is updated" do + expect{er.save!}.to change {enterprise.reload.updated_at } + end + end + + describe "with shipping methods" do + let(:sm) { create(:shipping_method) } + before do + enterprise.shipping_methods << sm + end + it "distributor_shipping_method is updated" do + expect { + enterprise.distributor_shipping_methods.first.save! + }.to change {enterprise.reload.updated_at} + end + + it "shipping method is updated" do + expect{sm.save!}.to change {enterprise.reload.updated_at } + end + end + end + end +end