Merge branch 'overage-limit'

Stephen Paul Weber created

* overage-limit:
  auto_top_up_amount and monthly_overage_limit from CustomerRepo
  Move Bandwidth Tn remote operations to BandwidthTnRepo
  Use value_semantics to DRY up CustomerRepo
  Use NotFound exception for all customer not found cases

Change summary

lib/bandwidth_tn_repo.rb   | 21 +++++++++
lib/customer.rb            | 14 ++----
lib/customer_info.rb       | 20 +++-----
lib/customer_plan.rb       | 21 ++++++--
lib/customer_repo.rb       | 89 ++++++++++++++++++++++-----------------
lib/low_balance.rb         | 20 ++++----
test/test_customer_info.rb | 16 -------
test/test_customer_repo.rb |  1 
test/test_helper.rb        | 28 ++++++-----
test/test_low_balance.rb   | 16 +-----
10 files changed, 127 insertions(+), 119 deletions(-)

Detailed changes

lib/bandwidth_tn_repo.rb 🔗

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require "ruby-bandwidth-iris"
+
+class BandwidthTnRepo
+	def find(tel)
+		BandwidthIris::Tn.new(telephone_number: tel).get_details
+	end
+
+	def put_lidb_name(tel, lidb_name)
+		BandwidthIris::Lidb.create(
+			lidb_tn_groups: { lidb_tn_group: {
+				telephone_numbers: [tel.sub(/\A\+1/, "")],
+				subscriber_information: lidb_name,
+				use_type: "RESIDENTIAL", visibility: "PUBLIC"
+			} }
+		)
+	rescue BandwidthIris::Errors::GenericError
+		raise "Could not set CNAM, please contact support"
+	end
+end

lib/customer.rb 🔗

@@ -19,10 +19,11 @@ require_relative "./trivial_backend_sgx_repo"
 class Customer
 	extend Forwardable
 
-	attr_reader :customer_id, :balance, :jid
+	attr_reader :customer_id, :balance, :jid, :tndetails
 
 	def_delegators :@plan, :active?, :activate_plan_starting_now, :bill_plan,
-	               :currency, :merchant_account, :plan_name, :auto_top_up_amount
+	               :currency, :merchant_account, :plan_name,
+	               :auto_top_up_amount, :monthly_overage_limit
 	def_delegators :@sgx, :register!, :registered?, :set_ogm_url,
 	               :fwd, :transcription_enabled
 	def_delegators :@usage, :usage_report, :message_usage, :incr_message_usage
@@ -32,6 +33,7 @@ class Customer
 		jid,
 		plan: CustomerPlan.new(customer_id),
 		balance: BigDecimal(0),
+		tndetails: {},
 		sgx: TrivialBackendSgxRepo.new.get(customer_id)
 	)
 		@plan = plan
@@ -39,6 +41,7 @@ class Customer
 		@customer_id = customer_id
 		@jid = jid
 		@balance = balance
+		@tndetails = tndetails
 		@sgx = sgx
 	end
 
@@ -84,13 +87,6 @@ class Customer
 		stanza_to(iq, &IQ_MANAGER.method(:write)).then(&:vcard)
 	end
 
-	def tndetails
-		return unless registered?
-
-		@tndetails ||=
-			BandwidthIris::Tn.new(telephone_number: registered?.phone).get_details
-	end
-
 	def ogm(from_tel=nil)
 		CustomerOGM.for(@sgx.ogm_url, -> { fetch_vcard_temp(from_tel) })
 	end

lib/customer_info.rb 🔗

@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 require "bigdecimal"
+require "forwardable"
 require "relative_time"
 require "value_semantics/monkey_patched"
 require_relative "proxied_jid"
@@ -8,17 +9,15 @@ require_relative "customer_plan"
 require_relative "form_template"
 
 class PlanInfo
+	extend Forwardable
+
+	def_delegators :plan, :expires_at, :auto_top_up_amount
+
 	def self.for(plan)
 		return EMPromise.resolve(NoPlan.new) unless plan&.plan_name
 
-		EMPromise.all([
-			plan.activation_date,
-			plan.auto_top_up_amount
-		]).then do |adate, atua|
-			new(
-				plan: plan, start_date: adate,
-				auto_top_up_amount: atua
-			)
+		plan.activation_date.then do |adate|
+			new(plan: plan, start_date: adate)
 		end
 	end
 
@@ -39,11 +38,6 @@ class PlanInfo
 	value_semantics do
 		plan CustomerPlan
 		start_date Time
-		auto_top_up_amount Integer
-	end
-
-	def expires_at
-		plan.expires_at
 	end
 
 	def template

lib/customer_plan.rb 🔗

@@ -3,19 +3,32 @@
 require "forwardable"
 
 require_relative "em"
+require_relative "plan"
 
 class CustomerPlan
 	extend Forwardable
 
-	attr_reader :expires_at
+	attr_reader :expires_at, :auto_top_up_amount, :monthly_overage_limit
 
 	def_delegator :@plan, :name, :plan_name
 	def_delegators :@plan, :currency, :merchant_account, :monthly_price
 
-	def initialize(customer_id, plan: nil, expires_at: Time.now)
+	def self.for(customer_id, plan_name: nil, **kwargs)
+		new(customer_id, plan: plan_name&.then(&Plan.method(:for)), **kwargs)
+	end
+
+	def initialize(
+		customer_id,
+		plan: nil,
+		expires_at: Time.now,
+		auto_top_up_amount: 0,
+		monthly_overage_limit: 0
+	)
 		@customer_id = customer_id
 		@plan = plan || OpenStruct.new
 		@expires_at = expires_at
+		@auto_top_up_amount = auto_top_up_amount
+		@monthly_overage_limit = monthly_overage_limit
 	end
 
 	def active?
@@ -30,10 +43,6 @@ class CustomerPlan
 		)
 	end
 
-	def auto_top_up_amount
-		REDIS.get("jmp_customer_auto_top_up_amount-#{@customer_id}").then(&:to_i)
-	end
-
 	def bill_plan
 		EM.promise_fiber do
 			DB.transaction do

lib/customer_repo.rb 🔗

@@ -1,23 +1,21 @@
 # frozen_string_literal: true
 
 require "lazy_object"
+require "value_semantics/monkey_patched"
 
+require_relative "bandwidth_tn_repo"
 require_relative "customer"
 require_relative "polyfill"
 
 class CustomerRepo
 	class NotFound < RuntimeError; end
 
-	def initialize(
-		redis: LazyObject.new { REDIS },
-		db: LazyObject.new { DB },
-		braintree: LazyObject.new { BRAINTREE },
-		sgx_repo: TrivialBackendSgxRepo.new
-	)
-		@redis = redis
-		@db = db
-		@braintree = braintree
-		@sgx_repo = sgx_repo
+	value_semantics do
+		redis             Anything(), default: LazyObject.new { REDIS }
+		db                Anything(), default: LazyObject.new { DB }
+		braintree         Anything(), default: LazyObject.new { BRAINTREE }
+		sgx_repo          Anything(), default: TrivialBackendSgxRepo.new
+		bandwidth_tn_repo Anything(), default: BandwidthTnRepo.new
 	end
 
 	def find(customer_id)
@@ -33,7 +31,7 @@ class CustomerRepo
 			find($1)
 		else
 			@redis.get("jmp_customer_id-#{jid}").then do |customer_id|
-				raise "No customer" unless customer_id
+				raise NotFound, "No customer" unless customer_id
 
 				find_inner(customer_id, jid)
 			end
@@ -64,22 +62,11 @@ class CustomerRepo
 	end
 
 	def put_lidb_name(customer, lidb_name)
-		BandwidthIris::Lidb.create(
-			customer_order_id: customer.customer_id,
-			lidb_tn_groups: { lidb_tn_group: {
-				telephone_numbers: [customer.registered?.phone.sub(/\A\+1/, "")],
-				subscriber_information: lidb_name,
-				use_type: "RESIDENTIAL", visibility: "PUBLIC"
-			} }
-		)
-	rescue BandwidthIris::Errors::GenericError
-		raise "Could not set CNAM, please contact support"
+		@bandwidth_tn_repo.put_lidb_name(customer.registered?.phone, lidb_name)
 	end
 
-	def put_transcription_enabled(customer, transcription_enabled)
-		@sgx_repo.put_transcription_enabled(
-			customer.customer_id, transcription_enabled
-		)
+	def put_transcription_enabled(customer, enabled)
+		@sgx_repo.put_transcription_enabled(customer.customer_id, enabled)
 	end
 
 	def put_fwd(customer, customer_fwd)
@@ -93,14 +80,17 @@ protected
 		TrivialBackendSgxRepo.new.get(customer_id).with(registered?: false)
 	end
 
-	def hydrate_plan(customer_id, raw_customer)
-		raw_customer.dup.tap do |data|
-			data[:plan] = CustomerPlan.new(
-				customer_id,
-				plan: data.delete(:plan_name)&.then(&Plan.method(:for)),
-				expires_at: data.delete(:expires_at)
-			)
-		end
+	def mget(*keys)
+		@redis.mget(keys).then { |values| Hash[keys.zip(values)] }
+	end
+
+	def fetch_redis(customer_id)
+		mget(
+			"jmp_customer_auto_top_up_amount-#{customer_id}",
+			"jmp_customer_monthly_overage_limit-#{customer_id}"
+		).then { |r|
+			r.transform_keys { |k| k.match(/^jmp_customer_([^-]+)/)[1].to_sym }
+		}
 	end
 
 	SQL = <<~SQL
@@ -109,13 +99,36 @@ protected
 		WHERE customer_id=$1 LIMIT 1
 	SQL
 
+	def fetch_sql(customer_id)
+		@db.query_defer(SQL, [customer_id]).then do |rows|
+			rows.first&.transform_keys(&:to_sym) || { balance: 0 }
+		end
+	end
+
+	def fetch_all(customer_id)
+		EMPromise.all([
+			@sgx_repo.get(customer_id),
+			fetch_sql(customer_id),
+			fetch_redis(customer_id)
+		]).then { |sgx, sql, redis| [sgx, sql.merge(redis)] }
+	end
+
+	def tndetails(sgx)
+		return unless sgx.registered?
+
+		LazyObject.new { @bandwidth_tn_repo.find(sgx.registered?.phone) }
+	end
+
 	def find_inner(customer_id, jid)
-		result = @db.query_defer(SQL, [customer_id])
-		EMPromise.all([@sgx_repo.get(customer_id), result]).then do |(sgx, rows)|
-			data = hydrate_plan(
-				customer_id, rows.first&.transform_keys(&:to_sym) || {}
+		fetch_all(customer_id).then do |(sgx, data)|
+			Customer.new(
+				customer_id, Blather::JID.new(jid),
+				sgx: sgx, balance: data[:balance], tndetails: tndetails(sgx),
+				plan: CustomerPlan.for(
+					customer_id,
+					**data.delete_if { |(k, _)| k == :balance }
+				)
 			)
-			Customer.new(customer_id, Blather::JID.new(jid), sgx: sgx, **data)
 		end
 	end
 end

lib/low_balance.rb 🔗

@@ -11,15 +11,13 @@ class LowBalance
 			"jmp_customer_low_balance-#{customer.customer_id}",
 			expiry: 60 * 60 * 24 * 7
 		).with(-> { Locked.new }) do
-			customer.auto_top_up_amount.then do |auto_top_up_amount|
-				for_auto_top_up_amount(customer, auto_top_up_amount)
-			end
+			for_auto_top_up_amount(customer)
 		end
 	end
 
-	def self.for_auto_top_up_amount(customer, auto_top_up_amount)
-		if auto_top_up_amount.positive?
-			AutoTopUp.new(customer, auto_top_up_amount)
+	def self.for_auto_top_up_amount(customer)
+		if customer.auto_top_up_amount.positive?
+			AutoTopUp.new(customer)
 		else
 			customer.btc_addresses.then do |btc_addresses|
 				new(customer, btc_addresses)
@@ -49,15 +47,17 @@ class LowBalance
 	end
 
 	class AutoTopUp
-		def initialize(customer, auto_top_up_amount)
+		def initialize(customer)
 			@customer = customer
-			@auto_top_up_amount = auto_top_up_amount
 			@message = Blather::Stanza::Message.new
 			@message.from = CONFIG[:notify_from]
 		end
 
 		def sale
-			Transaction.sale(@customer, amount: @auto_top_up_amount).then do |tx|
+			Transaction.sale(
+				@customer,
+				amount: @customer.auto_top_up_amount
+			).then do |tx|
 				tx.insert.then { tx }
 			end
 		end
@@ -70,7 +70,7 @@ class LowBalance
 			}.catch { |e|
 				@message.body =
 					"Automatic top-up transaction for " \
-					"$#{@auto_top_up_amount} failed: #{e.message}"
+					"$#{customer.auto_top_up_amount} failed: #{e.message}"
 			}.then { @customer.stanza_to(@message) }
 		end
 	end

test/test_customer_info.rb 🔗

@@ -18,13 +18,6 @@ class CustomerInfoTest < Minitest::Test
 	def test_info_does_not_crash
 		sgx = Minitest::Mock.new
 		sgx.expect(:registered?, false)
-		sgx.expect(:registered?, false)
-
-		CustomerPlan::REDIS.expect(
-			:get,
-			EMPromise.resolve(nil),
-			["jmp_customer_auto_top_up_amount-test"]
-		)
 
 		CustomerPlan::DB.expect(
 			:query_defer,
@@ -42,16 +35,9 @@ class CustomerInfoTest < Minitest::Test
 	def test_admin_info_does_not_crash
 		sgx = Minitest::Mock.new
 		sgx.expect(:registered?, false)
-		sgx.expect(:registered?, false)
 		fwd = CustomerFwd.for(uri: "tel:+12223334444", timeout: 15)
 		sgx.expect(:fwd, fwd)
 
-		CustomerPlan::REDIS.expect(
-			:get,
-			EMPromise.resolve(nil),
-			["jmp_customer_auto_top_up_amount-test"]
-		)
-
 		CustomerPlan::DB.expect(
 			:query_defer,
 			EMPromise.resolve([{ "start_date" => Time.now }]),
@@ -67,7 +53,6 @@ class CustomerInfoTest < Minitest::Test
 	def test_inactive_info_does_not_crash
 		sgx = Minitest::Mock.new
 		sgx.expect(:registered?, false)
-		sgx.expect(:registered?, false)
 
 		plan = CustomerPlan.new("test", plan: nil, expires_at: nil)
 		cust = Customer.new(
@@ -84,7 +69,6 @@ class CustomerInfoTest < Minitest::Test
 	def test_inactive_admin_info_does_not_crash
 		sgx = Minitest::Mock.new
 		sgx.expect(:registered?, false)
-		sgx.expect(:registered?, false)
 		sgx.expect(:fwd, CustomerFwd::None.new(uri: nil, timeout: nil))
 
 		plan = CustomerPlan.new("test", plan: nil, expires_at: nil)

test/test_customer_repo.rb 🔗

@@ -153,7 +153,6 @@ class CustomerRepoTest < Minitest::Test
 			:post,
 			"https://dashboard.bandwidth.com/v1.0/accounts//lidbs"
 		).with(body: {
-			CustomerOrderId: "test",
 			LidbTnGroups: {
 				LidbTnGroup: {
 					TelephoneNumbers: "5556667777",

test/test_helper.rb 🔗

@@ -39,19 +39,21 @@ require "tel_selections"
 $VERBOSE = nil
 Sentry.init
 
-def customer(customer_id="test", plan_name: nil, **kwargs)
-	jid = kwargs.delete(:jid) || Blather::JID.new("#{customer_id}@example.net")
-	if plan_name
-		expires_at = kwargs.delete(:expires_at) || Time.now
-		plan = CustomerPlan.new(
-			customer_id,
-			plan: Plan.for(plan_name),
-			expires_at: expires_at
-		)
-		Customer.new(customer_id, jid, plan: plan, **kwargs)
-	else
-		Customer.new(customer_id, jid, **kwargs)
-	end
+def customer(
+	customer_id="test",
+	plan_name: nil,
+	jid: Blather::JID.new("#{customer_id}@example.net"),
+	expires_at: Time.now,
+	auto_top_up_amount: 0,
+	**kwargs
+)
+	plan = CustomerPlan.for(
+		customer_id,
+		plan_name: plan_name,
+		expires_at: expires_at,
+		auto_top_up_amount: auto_top_up_amount
+	)
+	Customer.new(customer_id, jid, plan: plan, **kwargs)
 end
 
 CONFIG = {

test/test_low_balance.rb 🔗

@@ -24,11 +24,6 @@ class LowBalanceTest < Minitest::Test
 			EMPromise.resolve(0),
 			["jmp_customer_low_balance-test"]
 		)
-		CustomerPlan::REDIS.expect(
-			:get,
-			EMPromise.resolve(nil),
-			["jmp_customer_auto_top_up_amount-test"]
-		)
 		Customer::REDIS.expect(
 			:smembers,
 			EMPromise.resolve([]),
@@ -53,11 +48,6 @@ class LowBalanceTest < Minitest::Test
 			EMPromise.resolve(0),
 			["jmp_customer_low_balance-test"]
 		)
-		CustomerPlan::REDIS.expect(
-			:get,
-			EMPromise.resolve("15"),
-			["jmp_customer_auto_top_up_amount-test"]
-		)
 		ExpiringLock::REDIS.expect(
 			:setex,
 			EMPromise.resolve(nil),
@@ -65,7 +55,7 @@ class LowBalanceTest < Minitest::Test
 		)
 		assert_kind_of(
 			LowBalance::AutoTopUp,
-			LowBalance.for(customer).sync
+			LowBalance.for(customer(auto_top_up_amount: 15)).sync
 		)
 		assert_mock ExpiringLock::REDIS
 	end
@@ -75,8 +65,8 @@ class LowBalanceTest < Minitest::Test
 		LowBalance::AutoTopUp::Transaction = Minitest::Mock.new
 
 		def setup
-			@customer = customer
-			@auto_top_up = LowBalance::AutoTopUp.new(@customer, 100)
+			@customer = customer(auto_top_up_amount: 100)
+			@auto_top_up = LowBalance::AutoTopUp.new(@customer)
 		end
 
 		def test_notify!