Factor out CustomerRepo

Stephen Paul Weber created

Using the Repository pattern to encapsulate the fetch and create operations on
the persistence layer for a domain object.  These were not really factories in
the classic sense, but rather "fetch from persisitence layer" methods, and so
they now have a home.

Change summary

lib/command_list.rb        | 10 +--
lib/customer.rb            | 31 -------------
lib/customer_repo.rb       | 42 ++++++++++++++++++
sgx_jmp.rb                 | 29 +++++++-----
test/test_command_list.rb  | 65 ++++++++--------------------
test/test_customer.rb      | 61 ---------------------------
test/test_customer_repo.rb | 90 ++++++++++++++++++++++++++++++++++++++++
7 files changed, 173 insertions(+), 155 deletions(-)

Detailed changes

lib/command_list.rb 🔗

@@ -3,12 +3,10 @@
 class CommandList
 	include Enumerable
 
-	def self.for(jid)
-		Customer.for_jid(jid).catch { nil }.then do |customer|
-			EMPromise.resolve(customer&.registered?).catch { nil }.then do |reg|
-				next Registered.for(customer, reg.phone) if reg
-				CommandList.new
-			end
+	def self.for(customer)
+		EMPromise.resolve(customer&.registered?).catch { nil }.then do |reg|
+			next Registered.for(customer, reg.phone) if reg
+			CommandList.new
 		end
 	end
 

lib/customer.rb 🔗

@@ -12,37 +12,6 @@ require_relative "./plan"
 require_relative "./sip_account"
 
 class Customer
-	def self.for_jid(jid)
-		REDIS.get("jmp_customer_id-#{jid}").then do |customer_id|
-			raise "No customer id" unless customer_id
-			for_customer_id(customer_id)
-		end
-	end
-
-	def self.for_customer_id(customer_id)
-		result = DB.query_defer(<<~SQL, [customer_id])
-			SELECT COALESCE(balance,0) AS balance, plan_name, expires_at
-			FROM customer_plans LEFT JOIN balances USING (customer_id)
-			WHERE customer_id=$1 LIMIT 1
-		SQL
-		result.then do |rows|
-			new(customer_id, **rows.first&.transform_keys(&:to_sym) || {})
-		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

lib/customer_repo.rb 🔗

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require_relative "customer"
+
+class CustomerRepo
+	def initialize(redis: REDIS, db: DB, braintree: BRAINTREE)
+		@redis = redis
+		@db = db
+		@braintree = braintree
+	end
+
+	def find(customer_id)
+		result = @db.query_defer(<<~SQL, [customer_id])
+			SELECT COALESCE(balance,0) AS balance, plan_name, expires_at
+			FROM customer_plans LEFT JOIN balances USING (customer_id)
+			WHERE customer_id=$1 LIMIT 1
+		SQL
+		result.then do |rows|
+			Customer.new(customer_id, **rows.first&.transform_keys(&:to_sym) || {})
+		end
+	end
+
+	def find_by_jid(jid)
+		@redis.get("jmp_customer_id-#{jid}").then do |customer_id|
+			raise "No customer id" unless customer_id
+			find(customer_id)
+		end
+	end
+
+	def 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
+				Customer.new(cid)
+			end
+		end
+	end
+end

sgx_jmp.rb 🔗

@@ -44,6 +44,7 @@ require_relative "lib/btc_sell_prices"
 require_relative "lib/buy_account_credit_form"
 require_relative "lib/command_list"
 require_relative "lib/customer"
+require_relative "lib/customer_repo"
 require_relative "lib/electrum"
 require_relative "lib/error_to_send"
 require_relative "lib/em"
@@ -170,7 +171,7 @@ before nil, to: /\Acustomer_/, from: /(\A|@)#{CONFIG[:sgx]}(\/|\Z)/ do |s|
 	StatsD.increment("stanza_customer")
 
 	sentry_hub = new_sentry_hub(s, name: "stanza_customer")
-	Customer.for_customer_id(
+	CustomerRepo.new.find(
 		s.to.node.delete_prefix("customer_")
 	).then { |customer|
 		sentry_hub.current_scope.set_user(
@@ -195,7 +196,7 @@ message(
 		&.find { |el| el["jid"].to_s.start_with?("customer_") }
 	pass unless address
 
-	Customer.for_customer_id(
+	CustomerRepo.new.find(
 		Blather::JID.new(address["jid"].to_s).node.delete_prefix("customer_")
 	).then(&:jid).then { |customer_jid|
 		m.from = m.from.with(domain: CONFIG[:component][:jid])
@@ -218,7 +219,7 @@ message do |m|
 
 	sentry_hub = new_sentry_hub(m, name: "message")
 	today = Time.now.utc.to_date
-	Customer.for_jid(m.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(m.from.stripped).then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id, jid: m.from.stripped.to_s
 		)
@@ -337,7 +338,11 @@ disco_items node: "http://jabber.org/protocol/commands" do |iq|
 	sentry_hub = new_sentry_hub(iq, name: iq.node)
 	reply = iq.reply
 
-	CommandList.for(iq.from.stripped).then { |list|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).catch {
+		nil
+	}.then { |customer|
+		CommandList.for(customer)
+	}.then { |list|
 		reply.items = list.map do |item|
 			Blather::Stanza::DiscoItems::Item.new(
 				iq.to,
@@ -370,12 +375,12 @@ command :execute?, node: "jabber:iq:register", sessionid: nil do |iq|
 
 	sentry_hub = new_sentry_hub(iq, name: iq.node)
 	EMPromise.resolve(nil).then {
-		Customer.for_jid(iq.from.stripped)
+		CustomerRepo.new.find_by_jid(iq.from.stripped)
 	}.catch {
 		sentry_hub.add_breadcrumb(Sentry::Breadcrumb.new(
 			message: "Customer.create"
 		))
-		Customer.create(iq.from.stripped)
+		CustomerRepo.new.create(iq.from.stripped)
 	}.then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id,
@@ -412,7 +417,7 @@ command node: [
 	StatsD.increment("command", tags: ["node:#{iq.node}"])
 
 	sentry_hub = new_sentry_hub(iq, name: iq.node)
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id,
 			jid: iq.from.stripped.to_s
@@ -429,7 +434,7 @@ command :execute?, node: "credit cards", sessionid: nil do |iq|
 	reply = iq.reply
 	reply.status = :completed
 
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		oob = OOB.find_or_create(reply.command)
 		oob.url = CONFIG[:credit_card_url].call(
 			reply.to.stripped.to_s.gsub("\\", "%5C"),
@@ -451,7 +456,7 @@ command :execute?, node: "top up", sessionid: nil do |iq|
 	reply = iq.reply
 	reply.allowed_actions = [:complete]
 
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		BuyAccountCreditForm.for(customer).then do |credit_form|
 			credit_form.add_to_form(reply.form)
 			COMMAND_MANAGER.write(reply).then { |iq2| [customer, credit_form, iq2] }
@@ -480,7 +485,7 @@ command :execute?, node: "alt top up", sessionid: nil do |iq|
 	reply.status = :executing
 	reply.allowed_actions = [:complete]
 
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id,
 			jid: iq.from.stripped.to_s
@@ -500,7 +505,7 @@ command :execute?, node: "reset sip account", sessionid: nil do |iq|
 	StatsD.increment("command", tags: ["node:#{iq.node}"])
 
 	sentry_hub = new_sentry_hub(iq, name: iq.node)
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id,
 			jid: iq.from.stripped.to_s
@@ -519,7 +524,7 @@ command :execute?, node: "usage", sessionid: nil do |iq|
 	sentry_hub = new_sentry_hub(iq, name: iq.node)
 	report_for = (Date.today..(Date.today << 1))
 
-	Customer.for_jid(iq.from.stripped).then { |customer|
+	CustomerRepo.new.find_by_jid(iq.from.stripped).then { |customer|
 		sentry_hub.current_scope.set_user(
 			id: customer.customer_id,
 			jid: iq.from.stripped.to_s

test/test_command_list.rb 🔗

@@ -8,22 +8,13 @@ CommandList::REDIS = Minitest::Mock.new
 
 class CommandListTest < Minitest::Test
 	def test_for_no_customer
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.reject("not found"),
-			["none"]
-		)
-		assert_instance_of CommandList, CommandList.for("none").sync
+		assert_instance_of CommandList, CommandList.for(nil).sync
 	end
 	em :test_for_no_customer
 
 	def test_for_unregistered
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.resolve(OpenStruct.new(registered?: false)),
-			["unregistered"]
-		)
-		assert_instance_of CommandList, CommandList.for("unregistered").sync
+		customer = OpenStruct.new(registered?: false)
+		assert_instance_of CommandList, CommandList.for(customer).sync
 	end
 	em :test_for_unregistered
 
@@ -33,17 +24,13 @@ class CommandListTest < Minitest::Test
 			EMPromise.resolve(nil),
 			["catapult_fwd-1"]
 		)
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.resolve(OpenStruct.new(
-				registered?: OpenStruct.new(phone: "1"),
-				payment_methods: EMPromise.resolve([])
-			)),
-			["registered"]
+		customer = OpenStruct.new(
+			registered?: OpenStruct.new(phone: "1"),
+			payment_methods: EMPromise.resolve([])
 		)
 		assert_equal(
 			["CommandList::Registered"],
-			CommandList.for("registered").sync
+			CommandList.for(customer).sync
 			.class.ancestors.map(&:name).grep(/\ACommandList::/)
 		)
 	end
@@ -55,17 +42,13 @@ class CommandListTest < Minitest::Test
 			EMPromise.resolve("tel:1"),
 			["catapult_fwd-1"]
 		)
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.resolve(OpenStruct.new(
-				registered?: OpenStruct.new(phone: "1"),
-				payment_methods: EMPromise.resolve([])
-			)),
-			["registered"]
+		customer = OpenStruct.new(
+			registered?: OpenStruct.new(phone: "1"),
+			payment_methods: EMPromise.resolve([])
 		)
 		assert_equal(
 			CommandList::HAS_FORWARDING,
-			CommandList::HAS_FORWARDING & CommandList.for("registered").sync.to_a
+			CommandList::HAS_FORWARDING & CommandList.for(customer).sync.to_a
 		)
 	end
 	em :test_for_registered_with_fwd
@@ -76,18 +59,14 @@ class CommandListTest < Minitest::Test
 			EMPromise.resolve(nil),
 			["catapult_fwd-1"]
 		)
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.resolve(OpenStruct.new(
-				registered?: OpenStruct.new(phone: "1"),
-				plan_name: "test",
-				payment_methods: EMPromise.resolve([:boop])
-			)),
-			["registered"]
+		customer = OpenStruct.new(
+			registered?: OpenStruct.new(phone: "1"),
+			plan_name: "test",
+			payment_methods: EMPromise.resolve([:boop])
 		)
 		assert_equal(
 			CommandList::HAS_CREDIT_CARD,
-			CommandList::HAS_CREDIT_CARD & CommandList.for("registered").sync.to_a
+			CommandList::HAS_CREDIT_CARD & CommandList.for(customer).sync.to_a
 		)
 	end
 	em :test_for_registered_with_credit_card
@@ -98,18 +77,14 @@ class CommandListTest < Minitest::Test
 			EMPromise.resolve(nil),
 			["catapult_fwd-1"]
 		)
-		CommandList::Customer.expect(
-			:for_jid,
-			EMPromise.resolve(OpenStruct.new(
-				registered?: OpenStruct.new(phone: "1"),
-				currency: :USD
-			)),
-			["registered"]
+		customer = OpenStruct.new(
+			registered?: OpenStruct.new(phone: "1"),
+			currency: :USD
 		)
 
 		assert_equal(
 			CommandList::HAS_CURRENCY,
-			CommandList::HAS_CURRENCY & CommandList.for("registered").sync.to_a
+			CommandList::HAS_CURRENCY & CommandList.for(customer).sync.to_a
 		)
 	end
 	em :test_for_registered_with_currency

test/test_customer.rb 🔗

@@ -20,67 +20,6 @@ class SipAccount
 end
 
 class CustomerTest < Minitest::Test
-	def test_for_jid
-		Customer::REDIS.expect(
-			:get,
-			EMPromise.resolve(1),
-			["jmp_customer_id-test@example.com"]
-		)
-		Customer::DB.expect(
-			:query_defer,
-			EMPromise.resolve([{ balance: 1234, plan_name: "test_usd" }]),
-			[String, [1]]
-		)
-		customer = Customer.for_jid("test@example.com").sync
-		assert_kind_of Customer, customer
-		assert_equal 1234, customer.balance
-		assert_equal "merchant_usd", customer.merchant_account
-	end
-	em :test_for_jid
-
-	def test_for_jid_not_found
-		Customer::REDIS.expect(
-			:get,
-			EMPromise.resolve(nil),
-			["jmp_customer_id-test2@example.com"]
-		)
-		assert_raises do
-			Customer.for_jid("test2@example.com").sync
-		end
-	end
-	em :test_for_jid_not_found
-
-	def test_for_customer_id_not_found
-		Customer::DB.expect(
-			:query_defer,
-			EMPromise.resolve([]),
-			[String, [7357]]
-		)
-		customer = Customer.for_customer_id(7357).sync
-		assert_equal BigDecimal.new(0), customer.balance
-	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

test/test_customer_repo.rb 🔗

@@ -0,0 +1,90 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "customer_repo"
+
+class CustomerRepoTest < Minitest::Test
+	def mkrepo(
+		redis: Minitest::Mock.new,
+		db: Minitest::Mock.new,
+		braintree: Minitest::Mock.new
+	)
+		CustomerRepo.new(redis: redis, db: db, braintree: braintree)
+	end
+
+	def test_find_by_jid
+		redis = Minitest::Mock.new
+		db = Minitest::Mock.new
+		repo = mkrepo(redis: redis, db: db)
+		redis.expect(
+			:get,
+			EMPromise.resolve(1),
+			["jmp_customer_id-test@example.com"]
+		)
+		db.expect(
+			:query_defer,
+			EMPromise.resolve([{ balance: 1234, plan_name: "test_usd" }]),
+			[String, [1]]
+		)
+		customer = repo.find_by_jid("test@example.com").sync
+		assert_kind_of Customer, customer
+		assert_equal 1234, customer.balance
+		assert_equal "merchant_usd", customer.merchant_account
+		assert_mock redis
+		assert_mock db
+	end
+	em :test_find_by_jid
+
+	def test_find_by_jid_not_found
+		redis = Minitest::Mock.new
+		repo = mkrepo(redis: redis)
+		redis.expect(
+			:get,
+			EMPromise.resolve(nil),
+			["jmp_customer_id-test2@example.com"]
+		)
+		assert_raises do
+			repo.find_by_jid("test2@example.com").sync
+		end
+		assert_mock redis
+	end
+	em :test_find_by_jid_not_found
+
+	def test_find_db_empty
+		db = Minitest::Mock.new
+		repo = mkrepo(db: db)
+		db.expect(
+			:query_defer,
+			EMPromise.resolve([]),
+			[String, [7357]]
+		)
+		customer = repo.find(7357).sync
+		assert_equal BigDecimal.new(0), customer.balance
+		assert_mock db
+	end
+	em :test_find_db_empty
+
+	def test_create
+		redis = Minitest::Mock.new
+		braintree = Minitest::Mock.new
+		repo = mkrepo(redis: redis, braintree: braintree)
+		braintree_customer = Minitest::Mock.new
+		braintree.expect(:customer, braintree_customer)
+		braintree_customer.expect(:create, EMPromise.resolve(
+			OpenStruct.new(success?: true, customer: OpenStruct.new(id: "test"))
+		))
+		redis.expect(
+			:msetnx,
+			EMPromise.resolve(1),
+			[
+				"jmp_customer_id-test@example.com", "test",
+				"jmp_customer_jid-test", "test@example.com"
+			]
+		)
+		assert_kind_of Customer, repo.create("test@example.com").sync
+		assert_mock braintree
+		assert_mock braintree_customer
+		assert_mock redis
+	end
+	em :test_create
+end