From ec1b5a7a92e71b4020feabd700f022c74ec12846 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Tue, 12 Nov 2019 19:52:29 +1100
Subject: [PATCH] Test concurrent checkouts

When two people tried to buy the same item at the same time, it was
possible to oversell the item and end up with negative stock.

Parallel checkouts could also lead to other random failures. This spec
is testing that scenario by starting two threads which would run into a
race condition unless they use effective synchronisation. The added spec
fails if the synchronisation is removed from the CheckoutController.
---
 spec/controllers/checkout_controller_spec.rb | 76 ++++++++++++++++++++
 spec/spec_helper.rb                          |  1 +
 2 files changed, 77 insertions(+)

diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb
index b67a23b8ac..b1c01f90f2 100644
--- a/spec/controllers/checkout_controller_spec.rb
+++ b/spec/controllers/checkout_controller_spec.rb
@@ -266,4 +266,80 @@ describe CheckoutController, type: :controller do
       controller.send(:update_failed)
     end
   end
+
+  # This is the first example of testing concurrency in the Open Food Network.
+  # If we want to do this more often, we should look at:
+  #
+  #   https://github.com/forkbreak/fork_break
+  #
+  # The concurrency flag enables multiple threads to see the same database
+  # without isolated transactions.
+  describe "concurrency", concurrency: true do
+    # Re-defining test data because we need full data without doubles.
+    # :order_cycle has suppliers, distributors and products.
+    let(:order_cycle) { create(:order_cycle) }
+    let(:distributor) { order_cycle.distributors.first }
+    let(:order) { create(:order, order_cycle: order_cycle, distributor: distributor) }
+    let(:address) { create(:address) }
+    let(:payment_method) { create(:payment_method, distributors: [distributor]) }
+    let(:breakpoint) { Mutex.new }
+
+    before do
+      # Create a valid order ready for checkout:
+      create(:shipping_method, distributors: [distributor])
+      variant = order_cycle.variants_distributed_by(distributor).first
+      order.line_items << create(:line_item, variant: variant)
+
+      # Set up controller environment:
+      session[:order_id] = order.id
+      allow(controller).to receive(:spree_current_user).and_return(order.user)
+      allow(controller).to receive(:current_distributor).and_return(order.distributor)
+      allow(controller).to receive(:current_order_cycle).and_return(order.order_cycle)
+
+      # New threads start running straight away. The breakpoint is after loading
+      # the order and before advancing the order's state and making payments.
+      breakpoint.lock
+      allow(controller).to receive(:check_order_for_phantom_fees) do
+        breakpoint.synchronize {}
+      end
+    end
+
+    it "waits for concurrent checkouts" do
+      # Basic data the user submits during checkout:
+      address_params = address.attributes.except("id")
+      order_params = {
+        "payments_attributes" => [
+          {
+            "payment_method_id" => payment_method.id,
+            "amount" => order.total
+          }
+        ],
+        "bill_address_attributes" => address_params,
+        "ship_address_attributes" => address_params,
+      }
+
+      # Starting two checkout threads. The first one will wait at the breakpoint.
+      # The second waits for the first one if it uses correct synchronisation.
+      # If the checkout code wouldn't wait, it would load the order, then halt
+      # at the breakpoint and go into a race with the first thread.
+      # The result of that race are (random) errors which make this spec fail.
+      threads = [
+        Thread.new { spree_post :update, format: :json, order: order_params },
+        Thread.new { spree_post :update, format: :json, order: order_params },
+      ]
+      # Let the threads run again. They should not be in a race condition.
+      breakpoint.unlock
+      # Wait for both threads to finish.
+      threads.each(&:join)
+      order.reload
+
+      # When the spec passes, both threads have the same result. The user should
+      # see the order page. This is basically verifying a "double click"
+      # scenario.
+      expect(response.status).to eq(200)
+      expect(response.body).to eq({ path: spree.order_path(order) }.to_json)
+      expect(order.payments.count).to eq 1
+      expect(order.completed?).to be true
+    end
+  end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 983a80d9e6..5eb8cad557 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -100,6 +100,7 @@ RSpec.configure do |config|
   config.before(:suite)          { DatabaseCleaner.clean_with :deletion, except: ['spree_countries', 'spree_states'] }
   config.before(:each)           { DatabaseCleaner.strategy = :transaction }
   config.before(:each, js: true) { DatabaseCleaner.strategy = :deletion, { except: ['spree_countries', 'spree_states'] } }
+  config.before(:each, concurrency: true) { DatabaseCleaner.strategy = :deletion, { except: ['spree_countries', 'spree_states'] } }
   config.before(:each)           { DatabaseCleaner.start }
   config.after(:each)            { DatabaseCleaner.clean }
   config.after(:each, js: true) do