mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-01 21:47:16 +00:00
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.
346 lines
13 KiB
Ruby
346 lines
13 KiB
Ruby
require 'spec_helper'
|
|
|
|
describe CheckoutController, type: :controller do
|
|
let(:distributor) { double(:distributor) }
|
|
let(:order_cycle) { create(:simple_order_cycle) }
|
|
let(:order) { create(:order) }
|
|
let(:reset_order_service) { double(ResetOrderService) }
|
|
|
|
before do
|
|
allow(order).to receive(:checkout_allowed?).and_return true
|
|
allow(controller).to receive(:check_authorization).and_return true
|
|
end
|
|
|
|
it "redirects home when no distributor is selected" do
|
|
get :edit
|
|
expect(response).to redirect_to root_path
|
|
end
|
|
|
|
it "redirects to the shop when no order cycle is selected" do
|
|
allow(controller).to receive(:current_distributor).and_return(distributor)
|
|
get :edit
|
|
expect(response).to redirect_to shop_path
|
|
end
|
|
|
|
it "redirects home with message if hub is not ready for checkout" do
|
|
allow(distributor).to receive(:ready_for_checkout?) { false }
|
|
allow(order).to receive_messages(distributor: distributor, order_cycle: order_cycle)
|
|
allow(controller).to receive(:current_order).and_return(order)
|
|
|
|
expect(order).to receive(:empty!)
|
|
expect(order).to receive(:set_distribution!).with(nil, nil)
|
|
|
|
get :edit
|
|
|
|
expect(response).to redirect_to root_url
|
|
expect(flash[:info]).to eq("The hub you have selected is temporarily closed for orders. Please try again later.")
|
|
end
|
|
|
|
describe "redirection to the cart" do
|
|
let(:order_cycle_distributed_variants) { double(:order_cycle_distributed_variants) }
|
|
|
|
before do
|
|
allow(controller).to receive(:current_order).and_return(order)
|
|
allow(order).to receive(:distributor).and_return(distributor)
|
|
order.order_cycle = order_cycle
|
|
|
|
allow(OrderCycleDistributedVariants).to receive(:new).with(order_cycle, distributor).and_return(order_cycle_distributed_variants)
|
|
end
|
|
|
|
it "redirects when some items are out of stock" do
|
|
allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false
|
|
|
|
get :edit
|
|
expect(response).to redirect_to cart_path
|
|
end
|
|
|
|
it "redirects when some items are not available" do
|
|
allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true
|
|
expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(false)
|
|
|
|
get :edit
|
|
expect(response).to redirect_to cart_path
|
|
end
|
|
|
|
it "does not redirect when items are available and in stock" do
|
|
allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true
|
|
expect(order_cycle_distributed_variants).to receive(:distributes_order_variants?).with(order).and_return(true)
|
|
|
|
get :edit
|
|
expect(response).to be_success
|
|
end
|
|
end
|
|
|
|
describe "building the order" do
|
|
before do
|
|
allow(controller).to receive(:current_distributor).and_return(distributor)
|
|
allow(controller).to receive(:current_order_cycle).and_return(order_cycle)
|
|
allow(controller).to receive(:current_order).and_return(order)
|
|
end
|
|
|
|
it "set shipping_address_from_distributor when re-rendering edit" do
|
|
expect(order.updater).to receive(:shipping_address_from_distributor)
|
|
allow(order).to receive(:update_attributes).and_return false
|
|
spree_post :update, format: :json, order: {}
|
|
end
|
|
|
|
it "set shipping_address_from_distributor when the order state cannot be advanced" do
|
|
expect(order.updater).to receive(:shipping_address_from_distributor)
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(order).to receive(:next).and_return false
|
|
spree_post :update, format: :json, order: {}
|
|
end
|
|
|
|
context "#update with shipping_method_id" do
|
|
let(:test_shipping_method_id) { "111" }
|
|
|
|
before do
|
|
# stub order and resetorderservice
|
|
allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
|
|
allow(reset_order_service).to receive(:call)
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(controller).to receive(:current_order).and_return order
|
|
|
|
# make order workflow pass through delivery
|
|
allow(order).to receive(:next).twice do
|
|
if order.state == 'cart'
|
|
order.update_column :state, 'delivery'
|
|
else
|
|
order.update_column :state, 'complete'
|
|
end
|
|
end
|
|
end
|
|
|
|
it "does not fail to update" do
|
|
expect(controller).to_not receive(:clear_ship_address)
|
|
spree_post :update, order: { shipping_method_id: test_shipping_method_id }
|
|
end
|
|
|
|
it "does not send shipping_method_id to the order model as an attribute" do
|
|
expect(order).to receive(:update_attributes).with({})
|
|
spree_post :update, order: { shipping_method_id: test_shipping_method_id }
|
|
end
|
|
|
|
it "selects the shipping_method in the order" do
|
|
expect(order).to receive(:select_shipping_method).with(test_shipping_method_id)
|
|
spree_post :update, order: { shipping_method_id: test_shipping_method_id }
|
|
end
|
|
end
|
|
|
|
context 'when completing the order' do
|
|
before do
|
|
order.state = 'complete'
|
|
allow(order).to receive(:update_attributes).and_return(true)
|
|
allow(order).to receive(:next).and_return(true)
|
|
allow(order).to receive(:set_distributor!).and_return(true)
|
|
end
|
|
|
|
it "sets the new order's token to the same as the old order" do
|
|
order = controller.current_order(true)
|
|
spree_post :update, order: {}
|
|
expect(controller.current_order.token).to eq order.token
|
|
end
|
|
|
|
it 'expires the current order' do
|
|
allow(controller).to receive(:expire_current_order)
|
|
put :update, order: {}
|
|
expect(controller).to have_received(:expire_current_order)
|
|
end
|
|
|
|
it 'sets the access_token of the session' do
|
|
put :update, order: {}
|
|
expect(session[:access_token]).to eq(controller.current_order.token)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe '#expire_current_order' do
|
|
it 'empties the order_id of the session' do
|
|
expect(session).to receive(:[]=).with(:order_id, nil)
|
|
controller.expire_current_order
|
|
end
|
|
|
|
it 'resets the @current_order ivar' do
|
|
controller.expire_current_order
|
|
expect(controller.instance_variable_get(:@current_order)).to be_nil
|
|
end
|
|
end
|
|
|
|
context "via xhr" do
|
|
before do
|
|
allow(controller).to receive(:current_distributor).and_return(distributor)
|
|
|
|
allow(controller).to receive(:current_order_cycle).and_return(order_cycle)
|
|
allow(controller).to receive(:current_order).and_return(order)
|
|
end
|
|
|
|
it "returns errors" do
|
|
spree_post :update, format: :json, order: {}
|
|
expect(response.status).to eq(400)
|
|
expect(response.body).to eq({ errors: assigns[:order].errors, flash: {} }.to_json)
|
|
end
|
|
|
|
it "returns flash" do
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(order).to receive(:next).and_return false
|
|
spree_post :update, format: :json, order: {}
|
|
expect(response.body).to eq({ errors: assigns[:order].errors, flash: { error: "Payment could not be processed, please check the details you entered" } }.to_json)
|
|
end
|
|
|
|
it "returns order confirmation url on success" do
|
|
allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
|
|
expect(reset_order_service).to receive(:call)
|
|
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(order).to receive(:state).and_return "complete"
|
|
|
|
spree_post :update, format: :json, order: {}
|
|
expect(response.status).to eq(200)
|
|
expect(response.body).to eq({ path: spree.order_path(order) }.to_json)
|
|
end
|
|
|
|
describe "stale object handling" do
|
|
it "retries when a stale object error is encountered" do
|
|
allow(ResetOrderService).to receive(:new).with(controller, order) { reset_order_service }
|
|
expect(reset_order_service).to receive(:call)
|
|
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(controller).to receive(:state_callback)
|
|
|
|
# The first time, raise a StaleObjectError. The second time, succeed.
|
|
allow(order).to receive(:next).once.
|
|
and_raise(ActiveRecord::StaleObjectError.new(Spree::Variant.new, 'update'))
|
|
allow(order).to receive(:next).once do
|
|
order.update_column :state, 'complete'
|
|
true
|
|
end
|
|
|
|
spree_post :update, format: :json, order: {}
|
|
expect(response.status).to eq(200)
|
|
end
|
|
|
|
it "tries a maximum of 3 times before giving up and returning an error" do
|
|
allow(order).to receive(:update_attributes).and_return true
|
|
allow(order).to receive(:next) { raise ActiveRecord::StaleObjectError.new(Spree::Variant.new, 'update') }
|
|
|
|
spree_post :update, format: :json, order: {}
|
|
expect(response.status).to eq(400)
|
|
end
|
|
end
|
|
end
|
|
|
|
describe "Paypal routing" do
|
|
let(:payment_method) { create(:payment_method, type: "Spree::Gateway::PayPalExpress") }
|
|
let(:restart_checkout) { instance_double(RestartCheckout, call: true) }
|
|
|
|
before do
|
|
allow(controller).to receive(:current_distributor) { distributor }
|
|
allow(controller).to receive(:current_order_cycle) { order_cycle }
|
|
allow(controller).to receive(:current_order) { order }
|
|
|
|
allow(RestartCheckout).to receive(:new) { restart_checkout }
|
|
end
|
|
|
|
it "should check the payment method for Paypalness if we've selected one" do
|
|
expect(Spree::PaymentMethod).to receive(:find).with(payment_method.id.to_s) { payment_method }
|
|
allow(order).to receive(:update_attributes) { true }
|
|
allow(order).to receive(:state) { "payment" }
|
|
spree_post :update, order: { payments_attributes: [{ payment_method_id: payment_method.id }] }
|
|
end
|
|
end
|
|
|
|
describe "#update_failed" do
|
|
let(:restart_checkout) { instance_double(RestartCheckout, call: true) }
|
|
|
|
before do
|
|
controller.instance_variable_set(:@order, order)
|
|
allow(RestartCheckout).to receive(:new) { restart_checkout }
|
|
allow(controller).to receive(:current_order) { order }
|
|
end
|
|
|
|
it "set shipping_address_from_distributor and restarts the checkout" do
|
|
expect(order.updater).to receive(:shipping_address_from_distributor)
|
|
expect(restart_checkout).to receive(:call)
|
|
expect(controller).to receive(:respond_to)
|
|
|
|
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
|