diff --git a/lib/bill_plan_command.rb b/lib/bill_plan_command.rb index e46bb508ca644af1ba53788df157decebb1abda7..25a06e1bb4a8819d1410fc392b3e1340397981d9 100644 --- a/lib/bill_plan_command.rb +++ b/lib/bill_plan_command.rb @@ -1,9 +1,10 @@ # frozen_string_literal: true +require_relative "expiring_lock" + class BillPlanCommand def self.for(customer) return ForUnregistered.new(customer) unless customer.registered? - return ForNotExpired.new(customer) unless customer.expires_at <= Time.now unless customer.balance > customer.monthly_price return ForLowBalance.new(customer) @@ -16,19 +17,30 @@ class BillPlanCommand @customer = customer end + def reload_customer(db) + @customer = Command.execution.customer_repo.with(db: db) + .find(customer_id).sync + end + def call - billed = @customer.bill_plan(note: "Renew account plan") { |db| - @customer = Command.execution.customer_repo.with(db: db) - .find(@customer.customer_id).sync - @customer.balance > @customer.monthly_price && - @customer.expires_at <= Time.now - } - Command.reply do |reply| - reply.note_type = billed ? :info : :error - reply.note_text = "#{@customer.customer_id}#{billed ? '' : ' not'} billed" + ExpiringLock.new("jmp_customer_bill_plan-#{customer_id}").with { + @customer.bill_plan(note: "Renew account plan") { |db| + reload_customer(db).balance > @customer.monthly_price + } + }.then do |billed| + Command.reply do |reply| + reply.note_type = billed ? :info : :error + reply.note_text = "#{customer_id}#{billed ? '' : ' not'} billed" + end end end +protected + + def customer_id + @customer.customer_id + end + class ForLowBalance def initialize(customer) @customer = customer @@ -76,17 +88,4 @@ class BillPlanCommand end end end - - class ForNotExpired - def initialize(customer) - @customer = customer - end - - def call - Command.reply do |reply| - reply.note_type = :error - reply.note_text = "#{@customer.customer_id} is not expired" - end - end - end end diff --git a/lib/expiring_lock.rb b/lib/expiring_lock.rb index 42c967a71dc67d9e0811a6155a39573b11cf8e1d..87692cc49e6c241010433ba9f1175c92bf1808a3 100644 --- a/lib/expiring_lock.rb +++ b/lib/expiring_lock.rb @@ -7,11 +7,12 @@ class ExpiringLock end def with(els=nil) - REDIS.exists(@name).then do |exists| - next els&.call if exists == 1 + REDIS.set(@name, Time.now, "EX", @expiry, "NX").then do |result| + next els&.call if result.nil? - EMPromise.resolve(yield).then do |rval| - REDIS.setex(@name, @expiry, Time.now).then { rval } + EMPromise.resolve(yield).catch do |err| + REDIS.del(@name) + EMPromise.reject(err) end end end diff --git a/test/test_low_balance.rb b/test/test_low_balance.rb index d639d44bb00709bbb849e86b7301db70f4732d33..b8625661ebc18e9ab184270cff8912592647180c 100644 --- a/test/test_low_balance.rb +++ b/test/test_low_balance.rb @@ -10,9 +10,9 @@ CustomerFinancials::REDIS = Minitest::Mock.new class LowBalanceTest < Minitest::Test def test_for_locked ExpiringLock::REDIS.expect( - :exists, - EMPromise.resolve(1), - ["jmp_customer_low_balance-test"] + :set, + EMPromise.resolve(nil), + ["jmp_customer_low_balance-test", Time, "EX", 604800, "NX"] ) assert_kind_of LowBalance::Locked, LowBalance.for(customer).sync end @@ -20,20 +20,15 @@ class LowBalanceTest < Minitest::Test def test_for_no_auto_top_up ExpiringLock::REDIS.expect( - :exists, - EMPromise.resolve(0), - ["jmp_customer_low_balance-test"] + :set, + EMPromise.resolve("OK"), + ["jmp_customer_low_balance-test", Time, "EX", 604800, "NX"] ) CustomerFinancials::REDIS.expect( :smembers, EMPromise.resolve([]), ["jmp_customer_btc_addresses-test"] ) - ExpiringLock::REDIS.expect( - :setex, - EMPromise.resolve(nil), - ["jmp_customer_low_balance-test", 60 * 60 * 24 * 7, Time] - ) assert_kind_of( LowBalance, LowBalance.for(customer).sync @@ -44,14 +39,9 @@ class LowBalanceTest < Minitest::Test def test_for_auto_top_up ExpiringLock::REDIS.expect( - :exists, - EMPromise.resolve(0), - ["jmp_customer_low_balance-test"] - ) - ExpiringLock::REDIS.expect( - :setex, - EMPromise.resolve(nil), - ["jmp_customer_low_balance-test", 60 * 60 * 24 * 7, Time] + :set, + EMPromise.resolve("OK"), + ["jmp_customer_low_balance-test", Time, "EX", 604800, "NX"] ) assert_kind_of( LowBalance::AutoTopUp, diff --git a/test/test_web.rb b/test/test_web.rb index aacabc5ea1dadecdc642e1102ac86548de3efcd3..ab2f5be42760bf3628b19134a4c9aad0438ff13b 100644 --- a/test/test_web.rb +++ b/test/test_web.rb @@ -125,9 +125,9 @@ class WebTest < Minitest::Test def test_outbound_low_balance ExpiringLock::REDIS.expect( - :exists, - EMPromise.resolve(1), - ["jmp_customer_low_balance-customerid_low"] + :set, + EMPromise.resolve(nil), + ["jmp_customer_low_balance-customerid_low", Time, "EX", 604800, "NX"] ) post( @@ -161,15 +161,9 @@ class WebTest < Minitest::Test ) ExpiringLock::REDIS.expect( - :exists, - nil, - ["jmp_customer_low_balance-customerid_topup"] - ) - - ExpiringLock::REDIS.expect( - :setex, - nil, - ["jmp_customer_low_balance-customerid_topup", Integer, Time] + :set, + EMPromise.resolve("OK"), + ["jmp_customer_low_balance-customerid_topup", Time, "EX", 604800, "NX"] ) Customer::BLATHER.expect( @@ -307,9 +301,9 @@ class WebTest < Minitest::Test def test_inbound_low ExpiringLock::REDIS.expect( - :exists, - EMPromise.resolve(1), - ["jmp_customer_low_balance-customerid_low"] + :set, + EMPromise.resolve(nil), + ["jmp_customer_low_balance-customerid_low", Time, "EX", 604800, "NX"] ) post(