Skip to content

Commit

Permalink
Improve Spree::Order::NumberGenerator speed
Browse files Browse the repository at this point in the history
When generating a number, if it already exists, the length of numbers
will be increased if over half of the possible options are already
taken. This query is very expensive when having millions of records.
Upon multiple number collisions, this SQL becomes a bottleneck.

Co-authored-by: Sung <[email protected]>
  • Loading branch information
RyanofWoods and nohmar committed Nov 17, 2022
1 parent 1fe96cc commit 4cce19a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 1 deletion.
8 changes: 7 additions & 1 deletion core/app/models/spree/order/number_generator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,17 @@ def generate
# Use the random number if no other order exists with it.
if Spree::Order.exists?(number: random)
# If over half of all possible options are taken add another digit.
@length += 1 if Spree::Order.count > (10**@length / 2)
@length += 1 if order_count > (10**@length / 2)
else
break random
end
end
end

private

def order_count
@order_count ||= Spree::Order.count
end
end
end
22 changes: 22 additions & 0 deletions core/spec/models/spree/order/number_generator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,28 @@
expect(subject.length).to eq option_length
end
end

context "when the first generated number already exists" do
before do
allow(Spree::Order).to receive(:exists?).and_return(true, false)
end

it "regenerates a new number" do
expect(subject).to be_a(String)
expect(subject.length).to eq(default_length)
end

context "when over half the possible order numbers already exist" do
before do
allow(Spree::Order).to receive(:count).and_return(10 ** Spree::Order::ORDER_NUMBER_LENGTH / 2 + 1)
end

it "regenerates a new number with an increased length" do
expect(subject).to be_a(String)
expect(subject.length).to eq(default_length + 1)
end
end
end
end

context "when letters option is true" do
Expand Down

0 comments on commit 4cce19a

Please sign in to comment.