From 36395855f7fa6670bd0d63b12cc238b531ea21d9 Mon Sep 17 00:00:00 2001
From: John Hawthorn <john@stembolt.com>
Date: Thu, 7 Sep 2017 12:21:09 -0700
Subject: [PATCH] Speed up taxation of shipping rates

Instead of loading the tax rates for the order for each rate which needs
taxing, we can perform the SQL queries only once.

This is done by building a single instance of the
shipping_rate_tax_calculator_class for a shipment, and calling
`calculate` on that object for each rate, allowing it to cache the order
rates.
---
 core/app/models/spree/stock/estimator.rb        | 10 +++++++++-
 .../app/models/spree/tax/shipping_rate_taxer.rb |  2 +-
 .../spree/tax_calculator/shipping_rate.rb       | 15 ++++++++++++---
 core/lib/spree/app_configuration.rb             |  2 --
 core/spec/models/spree/stock/estimator_spec.rb  | 17 ++++++++---------
 5 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/core/app/models/spree/stock/estimator.rb b/core/app/models/spree/stock/estimator.rb
index 5ba5b88417e..06428cc43ff 100644
--- a/core/app/models/spree/stock/estimator.rb
+++ b/core/app/models/spree/stock/estimator.rb
@@ -31,6 +31,8 @@ def choose_default_shipping_rate(shipping_rates)
       end
 
       def calculate_shipping_rates(package)
+        tax_calculator_class = Spree::Config.shipping_rate_tax_calculator_class
+        tax_calculator = tax_calculator_class.new(package.shipment.order)
         shipping_methods(package).map do |shipping_method|
           cost = shipping_method.calculator.compute(package)
           if cost
@@ -38,7 +40,13 @@ def calculate_shipping_rates(package)
               cost: cost,
               shipment: package.shipment
             )
-            Spree::Config.shipping_rate_taxer_class.new.tax(rate)
+            tax_calculator.calculate(rate).each do |tax|
+              rate.taxes.new(
+                amount: tax.amount,
+                tax_rate: tax.tax_rate
+              )
+            end
+            rate
           end
         end.compact
       end
diff --git a/core/app/models/spree/tax/shipping_rate_taxer.rb b/core/app/models/spree/tax/shipping_rate_taxer.rb
index 94be4ae40c1..033494dc39e 100644
--- a/core/app/models/spree/tax/shipping_rate_taxer.rb
+++ b/core/app/models/spree/tax/shipping_rate_taxer.rb
@@ -8,7 +8,7 @@ class ShippingRateTaxer
       #   This parameter will be modified.
       # @return [Spree::ShippingRate] The shipping rate with associated tax objects
       def tax(shipping_rate)
-        taxes = Spree::Config.shipping_rate_tax_calculator_class.new(shipping_rate).calculate
+        taxes = Spree::Config.shipping_rate_tax_calculator_class.new(shipping_rate.order).calculate(shipping_rate)
         taxes.each do |tax|
           shipping_rate.taxes.build(
             amount: tax.amount,
diff --git a/core/app/models/spree/tax_calculator/shipping_rate.rb b/core/app/models/spree/tax_calculator/shipping_rate.rb
index 6d8e857e4fd..5fedb3e9772 100644
--- a/core/app/models/spree/tax_calculator/shipping_rate.rb
+++ b/core/app/models/spree/tax_calculator/shipping_rate.rb
@@ -21,15 +21,24 @@ class ShippingRate
       # @param [Spree::ShippingRate] shipping_rate the shipping rate to
       #   calculate taxes on
       # @return [Spree::TaxCalculator::ShippingRate]
-      def initialize(shipping_rate)
-        @shipping_rate = shipping_rate
+      def initialize(order)
+        if order.is_a?(::Spree::ShippingRate)
+          Spree::Deprecation.warn "passing a single shipping rate to Spree::TaxCalculator::ShippingRate is DEPRECATED. It now expects an order"
+          shipping_rate = order
+          @order = shipping_rate.order
+          @shipping_rate = shipping_rate
+        else
+          @order = order
+          @shipping_rate = nil
+        end
       end
 
       # Calculate taxes for a shipping rate.
       #
       # @return [Array<Spree::Tax::ItemTax>] the calculated taxes for the
       #   shipping rate
-      def calculate
+      def calculate(shipping_rate)
+        shipping_rate ||= @shipping_rate
         rates_for_item(shipping_rate).map do |rate|
           amount = rate.compute_amount(shipping_rate)
 
diff --git a/core/lib/spree/app_configuration.rb b/core/lib/spree/app_configuration.rb
index d47dce3529e..06573661924 100644
--- a/core/lib/spree/app_configuration.rb
+++ b/core/lib/spree/app_configuration.rb
@@ -287,8 +287,6 @@ def default_pricing_options
 
     class_name_attribute :shipping_rate_selector_class, default: 'Spree::Stock::ShippingRateSelector'
 
-    class_name_attribute :shipping_rate_taxer_class, default: 'Spree::Tax::ShippingRateTaxer'
-
     # Allows providing your own class for calculating taxes on a shipping rate.
     #
     # @!attribute [rw] shipping_rate_tax_calculator_class
diff --git a/core/spec/models/spree/stock/estimator_spec.rb b/core/spec/models/spree/stock/estimator_spec.rb
index 64f867fb1a8..7186ea3d3cd 100644
--- a/core/spec/models/spree/stock/estimator_spec.rb
+++ b/core/spec/models/spree/stock/estimator_spec.rb
@@ -189,20 +189,19 @@ class Spree::Stock::TestSorter; end;
         end
 
         it 'uses the configured shipping rate taxer' do
-          class Spree::Tax::TestTaxer
-            def initialize
+          class Spree::Tax::TestTaxCalculator
+            def initialize(_order)
             end
 
-            def tax(_)
-              Spree::ShippingRate.new
+            def calculate(_shipping_rate)
+              [
+                Spree::Tax::ItemTax.new(label: "TAX", amount: 5)
+              ]
             end
           end
-          Spree::Config.shipping_rate_taxer_class = Spree::Tax::TestTaxer
-
-          shipping_rate = Spree::ShippingRate.new
-          allow(Spree::ShippingRate).to receive(:new).and_return(shipping_rate)
+          Spree::Config.shipping_rate_tax_calculator_class = Spree::Tax::TestTaxCalculator
 
-          expect(Spree::Tax::TestTaxer).to receive(:new).and_call_original
+          expect(Spree::Tax::TestTaxCalculator).to receive(:new).and_call_original
           subject.shipping_rates(package)
         end
       end