From e5b1edb0fe70c5273b9ae60dadf9b597b8012771 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 18 May 2021 13:35:11 -0500 Subject: [PATCH 1/3] Inject BackendSgx per customer Instead of being a singleton that represents the entire relationship with the backend, the object is now per-customer (since any meaningful method requires a customer anyway for the from JID at least) and can be delegated to directly from Customer. --- lib/backend_sgx.rb | 19 +++++++++++------- lib/customer.rb | 14 +++++-------- sgx_jmp.rb | 1 - test/test_backend_sgx.rb | 8 ++++---- test/test_helper.rb | 2 -- test/test_registration.rb | 42 +++++++++++++++++++-------------------- 6 files changed, 42 insertions(+), 44 deletions(-) diff --git a/lib/backend_sgx.rb b/lib/backend_sgx.rb index b7fcec2b7867bd16cf87234cc751afdc26072442..2414afb650492397f0497a78c38ddf70e2a391f0 100644 --- a/lib/backend_sgx.rb +++ b/lib/backend_sgx.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true class BackendSgx - def initialize(jid=CONFIG[:sgx], creds=CONFIG[:creds]) + def initialize(customer_id, jid=CONFIG[:sgx], creds=CONFIG[:creds]) + @customer_id = customer_id @jid = jid @creds = creds end - def register!(customer_id, tel) - ibr = mkibr(:set, customer_id) + def register!(tel) + ibr = mkibr(:set) ibr.nick = @creds[:account] ibr.username = @creds[:username] ibr.password = @creds[:password] @@ -15,8 +16,8 @@ class BackendSgx IQ_MANAGER.write(ibr) end - def registered?(customer_id) - IQ_MANAGER.write(mkibr(:get, customer_id)).catch { nil }.then do |result| + def registered? + IQ_MANAGER.write(mkibr(:get)).catch { nil }.then do |result| if result&.respond_to?(:registered?) && result&.registered? result else @@ -27,9 +28,13 @@ class BackendSgx protected - def mkibr(type, customer_id) + def from_jid + "customer_#{@customer_id}@#{CONFIG[:component][:jid]}" + end + + def mkibr(type) ibr = IBR.new(type, @jid) - ibr.from = "customer_#{customer_id}@#{CONFIG[:component][:jid]}" + ibr.from = from_jid ibr end end diff --git a/lib/customer.rb b/lib/customer.rb index f0115a195f40d573e2daa6fb51758a04ed0a0816..f4f4186983ae0a2324730c147c9dc39fe0df4664 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -2,6 +2,7 @@ require "forwardable" +require_relative "./backend_sgx" require_relative "./ibr" require_relative "./payment_methods" require_relative "./plan" @@ -30,17 +31,20 @@ class Customer attr_reader :customer_id, :balance def_delegator :@plan, :name, :plan_name def_delegators :@plan, :currency, :merchant_account + def_delegators :@sgx, :register!, :registered? def initialize( customer_id, plan_name: nil, expires_at: Time.now, - balance: BigDecimal.new(0) + balance: BigDecimal.new(0), + sgx: BackendSgx.new(customer_id) ) @plan = plan_name && Plan.for(plan_name) @expires_at = expires_at @customer_id = customer_id @balance = balance + @sgx = sgx end def with_plan(plan_name) @@ -82,14 +86,6 @@ class Customer @plan && @expires_at > Time.now end - def register!(tel) - BACKEND_SGX.register!(customer_id, tel) - end - - def registered? - BACKEND_SGX.registered?(customer_id) - end - protected def charge_for_plan diff --git a/sgx_jmp.rb b/sgx_jmp.rb index 14554fe83b43028bfe4f4489d7be98cf967d9582..2d825c94a2adadd24223bb172b782f0097a3e1ab 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -77,7 +77,6 @@ class AsyncBraintree end BRAINTREE = AsyncBraintree.new(**CONFIG[:braintree]) -BACKEND_SGX = BackendSgx.new def panic(e) m = e.respond_to?(:message) ? e.message : e diff --git a/test/test_backend_sgx.rb b/test/test_backend_sgx.rb index 0b11b252159df354c8f5a96cba863eeb6dee3071..c2434d3d51a4a93e796940cc97471d538fd598cd 100644 --- a/test/test_backend_sgx.rb +++ b/test/test_backend_sgx.rb @@ -7,7 +7,7 @@ BackendSgx::IQ_MANAGER = Minitest::Mock.new class BackendSgxTest < Minitest::Test def setup - @sgx = BackendSgx.new + @sgx = BackendSgx.new("test") end def test_registered @@ -19,7 +19,7 @@ class BackendSgxTest < Minitest::Test assert_equal "customer_test@component", ibr.from.to_s end] ) - assert @sgx.registered?("test").sync + assert @sgx.registered?.sync end em :test_registered @@ -32,7 +32,7 @@ class BackendSgxTest < Minitest::Test assert_equal "customer_test@component", ibr.from.to_s end] ) - refute @sgx.registered?("test").sync + refute @sgx.registered?.sync end em :test_registered_not_registered @@ -48,7 +48,7 @@ class BackendSgxTest < Minitest::Test assert_equal "+15555550000", ibr.phone end] ) - @sgx.register!("test", "+15555550000") + @sgx.register!("+15555550000") BackendSgx::IQ_MANAGER.verify end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 74e1ad54f2fbd9028bdc554537f4a24da58662a4..39786502ebfeaf2e3d4e56b0ec595a9693617190 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -70,8 +70,6 @@ CONFIG = { credit_card_url: ->(*) { "http://creditcard.example.com" } }.freeze -BACKEND_SGX = Minitest::Mock.new(BackendSgx.new) - BLATHER = Class.new { def <<(*); end }.new.freeze diff --git a/test/test_registration.rb b/test/test_registration.rb index 43386545739afac5614e5e70a5988abc13d27c77..71171118f53a07ed5159ccdb41e70c0c31917eff 100644 --- a/test/test_registration.rb +++ b/test/test_registration.rb @@ -5,31 +5,34 @@ require "registration" class RegistrationTest < Minitest::Test def test_for_registered - BACKEND_SGX.expect( - :registered?, - EMPromise.resolve(OpenStruct.new(phone: "+15555550000")), - ["test"] + sgx = OpenStruct.new( + registered?: EMPromise.resolve(OpenStruct.new(phone: "+15555550000")) ) iq = Blather::Stanza::Iq::Command.new iq.from = "test@example.com" - result = Registration.for(iq, Customer.new("test"), Minitest::Mock.new).sync + result = Registration.for( + iq, + Customer.new("test", sgx: sgx), + Minitest::Mock.new + ).sync assert_kind_of Registration::Registered, result end em :test_for_registered def test_for_activated - BACKEND_SGX.expect( - :registered?, - EMPromise.resolve(nil), - ["test"] - ) + sgx = OpenStruct.new(registered?: EMPromise.resolve(nil)) web_manager = WebRegisterManager.new web_manager["test@example.com"] = "+15555550000" iq = Blather::Stanza::Iq::Command.new iq.from = "test@example.com" result = Registration.for( iq, - Customer.new("test", plan_name: "test_usd", expires_at: Time.now + 999), + Customer.new( + "test", + plan_name: "test_usd", + expires_at: Time.now + 999, + sgx: sgx + ), web_manager ).sync assert_kind_of Registration::Finish, result @@ -37,18 +40,14 @@ class RegistrationTest < Minitest::Test em :test_for_activated def test_for_not_activated_with_customer_id - BACKEND_SGX.expect( - :registered?, - EMPromise.resolve(nil), - ["test"] - ) + sgx = OpenStruct.new(registered?: EMPromise.resolve(nil)) web_manager = WebRegisterManager.new web_manager["test@example.com"] = "+15555550000" iq = Blather::Stanza::Iq::Command.new iq.from = "test@example.com" result = Registration.for( iq, - Customer.new("test"), + Customer.new("test", sgx: sgx), web_manager ).sync assert_kind_of Registration::Activation, result @@ -505,11 +504,12 @@ class RegistrationTest < Minitest::Test Registration::Finish::REDIS = Minitest::Mock.new def setup + @sgx = Minitest::Mock.new(BackendSgx.new("test")) iq = Blather::Stanza::Iq::Command.new iq.from = "test\\40example.com@cheogram.com" @finish = Registration::Finish.new( iq, - Customer.new("test"), + Customer.new("test", sgx: @sgx), "+15555550000" ) end @@ -548,10 +548,10 @@ class RegistrationTest < Minitest::Test "Content-Type" => "application/json" } ).to_return(status: 201) - BACKEND_SGX.expect( + @sgx.expect( :register!, EMPromise.resolve(OpenStruct.new(error?: false)), - ["test", "+15555550000"] + ["+15555550000"] ) Registration::Finish::REDIS.expect( :set, @@ -580,7 +580,7 @@ class RegistrationTest < Minitest::Test ) @finish.write.sync assert_requested create_order - BACKEND_SGX.verify + @sgx.verify Registration::Finish::REDIS.verify Registration::Finish::BLATHER.verify end From dabca132291e2c3d1ca03e272b2cd556956c2e84 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 18 May 2021 13:37:04 -0500 Subject: [PATCH 2/3] Break out CustomerPlan We had Plan and Customer but the relationship between the two lived entirely in Customer, which was growing quite large. Break that relationship out into its own concept and give it a name. --- lib/customer.rb | 66 +++++++---------------------------------- lib/customer_plan.rb | 69 +++++++++++++++++++++++++++++++++++++++++++ test/test_customer.rb | 19 ++++++------ 3 files changed, 89 insertions(+), 65 deletions(-) create mode 100644 lib/customer_plan.rb diff --git a/lib/customer.rb b/lib/customer.rb index f4f4186983ae0a2324730c147c9dc39fe0df4664..9f087de59ffe74adc20afdc0e7b6006d614bfd21 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -2,6 +2,7 @@ require "forwardable" +require_relative "./customer_plan" require_relative "./backend_sgx" require_relative "./ibr" require_relative "./payment_methods" @@ -29,8 +30,8 @@ class Customer extend Forwardable attr_reader :customer_id, :balance - def_delegator :@plan, :name, :plan_name - def_delegators :@plan, :currency, :merchant_account + def_delegators :@plan, :active?, :activate_plan_starting_now, :bill_plan, + :currency, :merchant_account, :plan_name def_delegators :@sgx, :register!, :registered? def initialize( @@ -40,8 +41,11 @@ class Customer balance: BigDecimal.new(0), sgx: BackendSgx.new(customer_id) ) - @plan = plan_name && Plan.for(plan_name) - @expires_at = expires_at + @plan = CustomerPlan.new( + customer_id, + plan: plan_name && Plan.for(plan_name), + expires_at: expires_at + ) @customer_id = customer_id @balance = balance @sgx = sgx @@ -51,29 +55,11 @@ class Customer self.class.new( @customer_id, balance: @balance, - expires_at: @expires_at, + expires_at: expires_at, plan_name: plan_name ) end - def bill_plan - EM.promise_fiber do - DB.transaction do - charge_for_plan - add_one_month_to_current_plan unless activate_plan_starting_now - end - end - end - - def activate_plan_starting_now - DB.exec(<<~SQL, [@customer_id, plan_name]).cmd_tuples.positive? - INSERT INTO plan_log - (customer_id, plan_name, date_range) - VALUES ($1, $2, tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month')) - ON CONFLICT DO NOTHING - SQL - end - def payment_methods @payment_methods ||= BRAINTREE @@ -82,37 +68,5 @@ class Customer .then(PaymentMethods.method(:for_braintree_customer)) end - def active? - @plan && @expires_at > Time.now - end - -protected - - def charge_for_plan - params = [ - @customer_id, - "#{@customer_id}-bill-#{plan_name}-at-#{Time.now.to_i}", - -@plan.monthly_price - ] - DB.exec(<<~SQL, params) - INSERT INTO transactions - (customer_id, transaction_id, created_at, amount) - VALUES ($1, $2, LOCALTIMESTAMP, $3) - SQL - end - - def add_one_month_to_current_plan - DB.exec(<<~SQL, [@customer_id]) - UPDATE plan_log SET date_range=range_merge( - date_range, - tsrange( - LOCALTIMESTAMP, - GREATEST(upper(date_range), LOCALTIMESTAMP) + '1 month' - ) - ) - WHERE - customer_id=$1 AND - date_range && tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month') - SQL - end + protected def_delegator :@plan, :expires_at end diff --git a/lib/customer_plan.rb b/lib/customer_plan.rb new file mode 100644 index 0000000000000000000000000000000000000000..6c315167ac53bc4e85b1b975280db57de599c45b --- /dev/null +++ b/lib/customer_plan.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require "forwardable" + +class CustomerPlan + extend Forwardable + + attr_reader :expires_at + def_delegator :@plan, :name, :plan_name + def_delegators :@plan, :currency, :merchant_account + + def initialize(customer_id, plan: nil, expires_at: Time.now) + @customer_id = customer_id + @plan = plan + @expires_at = expires_at + end + + def active? + @plan && @expires_at > Time.now + end + + def bill_plan + EM.promise_fiber do + DB.transaction do + charge_for_plan + add_one_month_to_current_plan unless activate_plan_starting_now + end + end + end + + def activate_plan_starting_now + DB.exec(<<~SQL, [@customer_id, plan_name]).cmd_tuples.positive? + INSERT INTO plan_log + (customer_id, plan_name, date_range) + VALUES ($1, $2, tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month')) + ON CONFLICT DO NOTHING + SQL + end + +protected + + def charge_for_plan + params = [ + @customer_id, + "#{@customer_id}-bill-#{plan_name}-at-#{Time.now.to_i}", + -@plan.monthly_price + ] + DB.exec(<<~SQL, params) + INSERT INTO transactions + (customer_id, transaction_id, created_at, amount) + VALUES ($1, $2, LOCALTIMESTAMP, $3) + SQL + end + + def add_one_month_to_current_plan + DB.exec(<<~SQL, [@customer_id]) + UPDATE plan_log SET date_range=range_merge( + date_range, + tsrange( + LOCALTIMESTAMP, + GREATEST(upper(date_range), LOCALTIMESTAMP) + '1 month' + ) + ) + WHERE + customer_id=$1 AND + date_range && tsrange(LOCALTIMESTAMP, LOCALTIMESTAMP + '1 month') + SQL + end +end diff --git a/test/test_customer.rb b/test/test_customer.rb index b2de096be347c5b303fe6ab7c142930883551d0d..4225b5831a44ebcd7adfac3b3648d0e7a8462257 100644 --- a/test/test_customer.rb +++ b/test/test_customer.rb @@ -5,6 +5,7 @@ require "customer" Customer::REDIS = Minitest::Mock.new Customer::DB = Minitest::Mock.new +CustomerPlan::DB = Minitest::Mock.new class CustomerTest < Minitest::Test def test_for_jid @@ -49,11 +50,11 @@ class CustomerTest < Minitest::Test em :test_for_customer_id_not_found def test_bill_plan_activate - Customer::DB.expect(:transaction, nil) do |&block| + CustomerPlan::DB.expect(:transaction, nil) do |&block| block.call true end - Customer::DB.expect( + CustomerPlan::DB.expect( :exec, nil, [ @@ -65,22 +66,22 @@ class CustomerTest < Minitest::Test end ] ) - Customer::DB.expect( + CustomerPlan::DB.expect( :exec, OpenStruct.new(cmd_tuples: 1), [String, ["test", "test_usd"]] ) Customer.new("test", plan_name: "test_usd").bill_plan.sync - Customer::DB.verify + CustomerPlan::DB.verify end em :test_bill_plan_activate def test_bill_plan_update - Customer::DB.expect(:transaction, nil) do |&block| + CustomerPlan::DB.expect(:transaction, nil) do |&block| block.call true end - Customer::DB.expect( + CustomerPlan::DB.expect( :exec, nil, [ @@ -92,14 +93,14 @@ class CustomerTest < Minitest::Test end ] ) - Customer::DB.expect( + CustomerPlan::DB.expect( :exec, OpenStruct.new(cmd_tuples: 0), [String, ["test", "test_usd"]] ) - Customer::DB.expect(:exec, nil, [String, ["test"]]) + CustomerPlan::DB.expect(:exec, nil, [String, ["test"]]) Customer.new("test", plan_name: "test_usd").bill_plan.sync - Customer::DB.verify + CustomerPlan::DB.verify end em :test_bill_plan_update end From a3f4e2701ae66bda07c998bbb1391ebbb12fb755 Mon Sep 17 00:00:00 2001 From: Stephen Paul Weber Date: Tue, 18 May 2021 13:40:50 -0500 Subject: [PATCH 3/3] Create customer_id if it does not exist before we start registration --- lib/customer.rb | 13 +++++++++++++ lib/registration.rb | 9 +++------ sgx_jmp.rb | 2 +- test/test_customer.rb | 21 +++++++++++++++++++++ test/test_registration.rb | 7 ------- 5 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/customer.rb b/lib/customer.rb index 9f087de59ffe74adc20afdc0e7b6006d614bfd21..d5584888eebc4c6d3bff0319cce596c85be819b5 100644 --- a/lib/customer.rb +++ b/lib/customer.rb @@ -27,6 +27,19 @@ class Customer end end + def self.create(jid) + BRAINTREE.customer.create.then do |result| + raise "Braintree customer create failed" unless result.success? + cid = result.customer.id + REDIS.msetnx( + "jmp_customer_id-#{jid}", cid, "jmp_customer_jid-#{cid}", jid + ).then do |redis_result| + raise "Saving new customer to redis failed" unless redis_result == 1 + new(cid) + end + end + end + extend Forwardable attr_reader :customer_id, :balance diff --git a/lib/registration.rb b/lib/registration.rb index 89355050b140bfcc21f17de057d4c4d9f42b9608..c22add55f34f1d613eb76e1107dffc19d12c6782 100644 --- a/lib/registration.rb +++ b/lib/registration.rb @@ -6,7 +6,7 @@ require_relative "./oob" class Registration def self.for(iq, customer, web_register_manager) - EMPromise.resolve(customer&.registered?).then do |registered| + customer.registered?.then do |registered| if registered Registered.new(iq, registered.phone) else @@ -36,13 +36,10 @@ class Registration class Activation def self.for(iq, customer, tel) - if customer&.active? + if customer.active? Finish.new(iq, customer, tel) - elsif customer - EMPromise.resolve(new(iq, customer, tel)) else - # Create customer_id - raise "TODO" + EMPromise.resolve(new(iq, customer, tel)) end end diff --git a/sgx_jmp.rb b/sgx_jmp.rb index 2d825c94a2adadd24223bb172b782f0097a3e1ab..810153e33742a6c220d3dbf649ec72671cb520e6 100644 --- a/sgx_jmp.rb +++ b/sgx_jmp.rb @@ -177,7 +177,7 @@ end command :execute?, node: "jabber:iq:register", sessionid: nil do |iq| Customer.for_jid(iq.from.stripped).catch { - nil + Customer.create(iq.from.stripped) }.then { |customer| Registration.for( iq, diff --git a/test/test_customer.rb b/test/test_customer.rb index 4225b5831a44ebcd7adfac3b3648d0e7a8462257..400c85de6b3a7eb3546e397bc6c9ca19907cc083 100644 --- a/test/test_customer.rb +++ b/test/test_customer.rb @@ -3,6 +3,7 @@ require "test_helper" require "customer" +Customer::BRAINTREE = Minitest::Mock.new Customer::REDIS = Minitest::Mock.new Customer::DB = Minitest::Mock.new CustomerPlan::DB = Minitest::Mock.new @@ -49,6 +50,26 @@ class CustomerTest < Minitest::Test end em :test_for_customer_id_not_found + def test_create + braintree_customer = Minitest::Mock.new + Customer::BRAINTREE.expect(:customer, braintree_customer) + braintree_customer.expect(:create, EMPromise.resolve( + OpenStruct.new(success?: true, customer: OpenStruct.new(id: "test")) + )) + Customer::REDIS.expect( + :msetnx, + EMPromise.resolve(1), + [ + "jmp_customer_id-test@example.com", "test", + "jmp_customer_jid-test", "test@example.com" + ] + ) + assert_kind_of Customer, Customer.create("test@example.com").sync + braintree_customer.verify + Customer::REDIS.verify + end + em :test_create + def test_bill_plan_activate CustomerPlan::DB.expect(:transaction, nil) do |&block| block.call diff --git a/test/test_registration.rb b/test/test_registration.rb index 71171118f53a07ed5159ccdb41e70c0c31917eff..89376e432381ea187925fd0026c5bd6af32f83ab 100644 --- a/test/test_registration.rb +++ b/test/test_registration.rb @@ -54,13 +54,6 @@ class RegistrationTest < Minitest::Test end em :test_for_not_activated_with_customer_id - def test_for_not_activated_without_customer_id - skip "customer_id creation not implemented yet" - iq = Blather::Stanza::Iq::Command.new - Registration.for(iq, nil, Minitest::Mock.new).sync - end - em :test_for_not_activated_without_customer_id - class ActivationTest < Minitest::Test Registration::Activation::COMMAND_MANAGER = Minitest::Mock.new def setup