From f57e8513d564926ae87a3b0d034fbf7ba506388f Mon Sep 17 00:00:00 2001 From: Will Marshall Date: Thu, 10 Jul 2014 12:46:25 +1000 Subject: [PATCH 1/2] Spiking out products serializers and caching --- app/controllers/shop_controller.rb | 4 ++++ .../api/master_variant_serializer.rb | 4 ++++ app/serializers/api/product_serializer.rb | 22 +++++++++++++++++++ app/serializers/api/property_serializer.rb | 3 +++ app/serializers/api/taxon_image_serializer.rb | 11 ++++++++++ app/serializers/api/variant_serializer.rb | 8 +++++++ 6 files changed, 52 insertions(+) create mode 100644 app/serializers/api/master_variant_serializer.rb create mode 100644 app/serializers/api/product_serializer.rb create mode 100644 app/serializers/api/property_serializer.rb create mode 100644 app/serializers/api/taxon_image_serializer.rb create mode 100644 app/serializers/api/variant_serializer.rb diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index b3e338e645..2398a50840 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -8,12 +8,16 @@ class ShopController < BaseController end def products + # Can we make this query less slow? unless @products = current_order_cycle.andand .valid_products_distributed_by(current_distributor).andand .select { |p| !p.deleted? && p.has_stock_for_distribution?(current_order_cycle, current_distributor) }.andand .sort_by {|p| p.name } render json: "", status: 404 end + render status: 200, + json: ActiveModel::ArraySerializer.new(@products, each_serializer: Api::ProductSerializer, + current_order_cycle: current_order_cycle, current_distributor: current_distributor).to_json end def order_cycle diff --git a/app/serializers/api/master_variant_serializer.rb b/app/serializers/api/master_variant_serializer.rb new file mode 100644 index 0000000000..8012e8f055 --- /dev/null +++ b/app/serializers/api/master_variant_serializer.rb @@ -0,0 +1,4 @@ +class Api::MasterVariantSerializer < ActiveModel::Serializer + attributes :id, :is_master, :count_on_hand, :name_to_display, :unit_to_display, :count_on_hand, :on_demand + has_many :images, serializer: Api::TaxonImageSerializer +end diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb new file mode 100644 index 0000000000..f079480fff --- /dev/null +++ b/app/serializers/api/product_serializer.rb @@ -0,0 +1,22 @@ +class Api::ProductSerializer < ActiveModel::Serializer + # TODO + # Prices can't be cached? How? + + cached + delegate :cache_key, to: :object + + attributes :id, :name, :permalink, :count_on_hand, :on_demand, :group_buy, + :notes, :description, :price + + has_many :variants, serializer: Api::VariantSerializer + has_many :taxons, serializer: Api::IdSerializer + has_many :properties, serializer: Api::PropertySerializer + + has_one :supplier, serializer: Api::IdSerializer + has_one :primary_taxon, serializer: Api::TaxonSerializer + has_one :master, serializer: Api::MasterVariantSerializer + + def price + object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle]) + end +end diff --git a/app/serializers/api/property_serializer.rb b/app/serializers/api/property_serializer.rb new file mode 100644 index 0000000000..ce04480d30 --- /dev/null +++ b/app/serializers/api/property_serializer.rb @@ -0,0 +1,3 @@ +class Api::PropertySerializer < ActiveModel::Serializer + +end diff --git a/app/serializers/api/taxon_image_serializer.rb b/app/serializers/api/taxon_image_serializer.rb new file mode 100644 index 0000000000..bc920637cd --- /dev/null +++ b/app/serializers/api/taxon_image_serializer.rb @@ -0,0 +1,11 @@ +class Api::TaxonImageSerializer < ActiveModel::Serializer + attributes :id, :alt, :small_url, :large_url + + def small_url + object.attachment.url(:small, false) + end + + def large_url + object.attachment.url(:large, false) + end +end diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb new file mode 100644 index 0000000000..20728fb935 --- /dev/null +++ b/app/serializers/api/variant_serializer.rb @@ -0,0 +1,8 @@ +class Api::VariantSerializer < ActiveModel::Serializer + attributes :id, :is_master, :count_on_hand, :name_to_display, :unit_to_display, :on_demand, :price + has_many :images, serializer: Api::TaxonImageSerializer + + def price + 0.0 + end +end From 26e8a1fd915bf2559e4ebedd5628fa21c43ae562 Mon Sep 17 00:00:00 2001 From: Will Marshall Date: Thu, 10 Jul 2014 14:49:21 +1000 Subject: [PATCH 2/2] Uncaching some parts and adding specs --- app/controllers/shop_controller.rb | 10 +++++--- app/serializers/api/product_serializer.rb | 29 +++++++++++++++++---- app/serializers/api/variant_serializer.rb | 31 ++++++++++++++++++++--- spec/controllers/shop_controller_spec.rb | 5 ++-- 4 files changed, 59 insertions(+), 16 deletions(-) diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index 2398a50840..2c1b4cd6d5 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -9,15 +9,17 @@ class ShopController < BaseController def products # Can we make this query less slow? - unless @products = current_order_cycle.andand + # + if @products = current_order_cycle.andand .valid_products_distributed_by(current_distributor).andand .select { |p| !p.deleted? && p.has_stock_for_distribution?(current_order_cycle, current_distributor) }.andand .sort_by {|p| p.name } + render status: 200, + json: ActiveModel::ArraySerializer.new(@products, each_serializer: Api::ProductSerializer, + current_order_cycle: current_order_cycle, current_distributor: current_distributor).to_json + else render json: "", status: 404 end - render status: 200, - json: ActiveModel::ArraySerializer.new(@products, each_serializer: Api::ProductSerializer, - current_order_cycle: current_order_cycle, current_distributor: current_distributor).to_json end def order_cycle diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index f079480fff..08aba2b6ba 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -1,12 +1,35 @@ class Api::ProductSerializer < ActiveModel::Serializer # TODO # Prices can't be cached? How? + def serializable_hash + cached_serializer_hash.merge uncached_serializer_hash + end + + private + def cached_serializer_hash + Api::CachedProductSerializer.new(object, @options).serializable_hash + end + + def uncached_serializer_hash + Api::UncachedProductSerializer.new(object, @options).serializable_hash + end +end + +class Api::UncachedProductSerializer < ActiveModel::Serializer + attributes :price + + def price + object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle]) + end +end + +class Api::CachedProductSerializer < ActiveModel::Serializer cached delegate :cache_key, to: :object attributes :id, :name, :permalink, :count_on_hand, :on_demand, :group_buy, - :notes, :description, :price + :notes, :description has_many :variants, serializer: Api::VariantSerializer has_many :taxons, serializer: Api::IdSerializer @@ -15,8 +38,4 @@ class Api::ProductSerializer < ActiveModel::Serializer has_one :supplier, serializer: Api::IdSerializer has_one :primary_taxon, serializer: Api::TaxonSerializer has_one :master, serializer: Api::MasterVariantSerializer - - def price - object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle]) - end end diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb index 20728fb935..7f5a37e8e1 100644 --- a/app/serializers/api/variant_serializer.rb +++ b/app/serializers/api/variant_serializer.rb @@ -1,8 +1,31 @@ class Api::VariantSerializer < ActiveModel::Serializer - attributes :id, :is_master, :count_on_hand, :name_to_display, :unit_to_display, :on_demand, :price - has_many :images, serializer: Api::TaxonImageSerializer + def serializable_hash + cached_serializer_hash.merge uncached_serializer_hash + end - def price - 0.0 + private + + def cached_serializer_hash + Api::CachedVariantSerializer.new(object, @options).serializable_hash + end + + def uncached_serializer_hash + Api::UncachedVariantSerializer.new(object, @options).serializable_hash end end + +class Api::UncachedVariantSerializer < ActiveModel::Serializer + attributes :price + + def price + object.price_with_fees(options[:current_distributor], options[:current_order_cycle]) + end +end + +class Api::CachedVariantSerializer < ActiveModel::Serializer + cached + delegate :cache_key, to: :object + + attributes :id, :is_master, :count_on_hand, :name_to_display, :unit_to_display, :on_demand + has_many :images, serializer: Api::TaxonImageSerializer +end diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb index f0857f54bc..fc68f86e4d 100644 --- a/spec/controllers/shop_controller_spec.rb +++ b/spec/controllers/shop_controller_spec.rb @@ -142,11 +142,10 @@ describe ShopController do end it "includes the primary taxon" do - taxon = mock_model(Spree::Taxon, name: "fruitbat") + taxon = create(:taxon) Spree::Product.any_instance.stub(:primary_taxon).and_return taxon - taxon.stub_chain(:icon, :url).and_return "" xhr :get, :products - response.body.should have_content "fruitbat" + response.body.should have_content taxon.name end end end