Skip asking backend about registration when we know the tel already

Stephen Paul Weber created 3 years ago

This is an optimization.  If we are looking up by tel already, then we don't
need to ask the backend sgx if they have a tel or what it is, we know that, so
just use that information directly and save us a call.

Change summary

lib/bwmsgsv2_repo.rb            | 18 +++++++-----
lib/customer_repo.rb            | 14 +++++-----
lib/trivial_backend_sgx_repo.rb | 13 ++++++++-
test/test_web.rb                | 47 +++++++++++++++++++++++++++++++++-
4 files changed, 73 insertions(+), 19 deletions(-)

Detailed changes

lib/bwmsgsv2_repo.rb

@@ -21,9 +21,9 @@ class Bwmsgsv2Repo
 		@trivial_repo = TrivialBackendSgxRepo.new(jid: jid, **kwargs)
 	end
 
-	def get(customer_id)
-		sgx = @trivial_repo.get(customer_id)
-		fetch_raw(sgx.from_jid).then do |(((ogm_url, fwd_time, fwd), flags), reg)|
+	def get(customer_id, tel: nil)
+		sgx = @trivial_repo.get(customer_id, tel: tel)
+		fetch_raw(sgx).then do |(((ogm_url, fwd_time, fwd), flags), reg)|
 			sgx.with(
 				ogm_url: ogm_url,
 				fwd: CustomerFwd.for(uri: fwd, timeout: fwd_time),
@@ -63,14 +63,16 @@ protected
 		end
 	end
 
-	def fetch_raw(from_jid)
-		registration(from_jid).then do |r|
-			EMPromise.all([from_redis(from_jid, r ? r.phone : nil), r])
+	def fetch_raw(sgx)
+		registration(sgx).then do |r|
+			EMPromise.all([from_redis(sgx.from_jid, r ? r.phone : nil), r])
 		end
 	end
 
-	def registration(from_jid)
-		@ibr_repo.registered?(@jid, from: from_jid)
+	def registration(sgx)
+		EMPromise.resolve(sgx.registered?)
+	rescue NotLoaded::NotLoadedError
+		@ibr_repo.registered?(@jid, from: sgx.from_jid)
 	end
 
 	def from_redis(from_jid, tel)

lib/customer_repo.rb

@@ -34,11 +34,11 @@ class CustomerRepo
 		end
 
 		ID = Struct.new(:customer_id) do
-			def keys(redis)
+			def keys(redis, tel: nil)
 				redis.get("jmp_customer_jid-#{customer_id}").then do |jid|
 					raise NotFound, "No jid" unless jid
 
-					[customer_id, Blather::JID.new(jid)]
+					[customer_id, Blather::JID.new(jid), *tel]
 				end
 			end
 		end
@@ -52,11 +52,11 @@ class CustomerRepo
 				end
 			end
 
-			def keys(redis)
+			def keys(redis, tel: nil)
 				redis.get("jmp_customer_id-#{jid}").then do |customer_id|
 					raise NotFound, "No customer" unless customer_id
 
-					[customer_id, Blather::JID.new(jid)]
+					[customer_id, Blather::JID.new(jid), *tel]
 				end
 			end
 		end
@@ -66,7 +66,7 @@ class CustomerRepo
 				redis.get("catapult_jid-#{tel}").then do |jid|
 					raise NotFound, "No jid" unless jid
 
-					JID.for(jid).keys(redis)
+					JID.for(jid).keys(redis, tel: tel)
 				end
 			end
 		end
@@ -184,10 +184,10 @@ protected
 		LazyObject.new { @bandwidth_tn_repo.find(sgx.registered?.phone) || {} }
 	end
 
-	def find_inner(cid, jid)
+	def find_inner(cid, jid, tel=nil)
 		set_user.call(customer_id: cid, jid: jid)
 		EMPromise.all([
-			@sgx_repo.get(cid).then { |sgx| { sgx: sgx } },
+			@sgx_repo.get(cid, tel: tel).then { |sgx| { sgx: sgx } },
 			@db.query_one(SQL, cid, default: {}),
 			fetch_redis(cid)
 		]).then { |all| all.reduce(&:merge) }.then do |data|

lib/trivial_backend_sgx_repo.rb

@@ -16,14 +16,23 @@ class TrivialBackendSgxRepo
 		@with = with
 	end
 
-	def get(customer_id)
+	def get(customer_id, tel: nil)
 		BackendSgx.new(
 			jid: @jid, creds: @creds,
 			from_jid: Blather::JID.new("customer_#{customer_id}", @component_jid),
 			ogm_url: NotLoaded.new(:ogm_url),
 			fwd: NotLoaded.new(:fwd_timeout),
 			transcription_enabled: NotLoaded.new(:transcription_enabled),
-			registered?: NotLoaded.new(:registered?)
+			registered?: tel ? ibr_for(tel) : NotLoaded.new(:registered?)
 		).with(@with)
 	end
+
+protected
+
+	def ibr_for(tel)
+		ibr = Blather::Stanza::Iq::IBR.new
+		ibr.registered = true
+		ibr.phone = tel
+		ibr
+	end
 end

test/test_web.rb

@@ -27,7 +27,9 @@ class WebTest < Minitest::Test
 				"jmp_customer_monthly_overage_limit-customerid_topup" => "99999",
 				"catapult_jid-[phone]" => "customer_customerid_topup@component",
 				"jmp_customer_jid-customerid_limit" => "[e-mail]",
-				"catapult_jid-[phone]" => "customer_customerid_limit@component"
+				"catapult_jid-[phone]" => "customer_customerid_limit@component",
+				"jmp_customer_jid-customerid2" => "[e-mail]",
+				"catapult_jid-[phone]" => "customer_customerid2@component"
 			),
 			db: FakeDB.new(
 				["customerid"] => [{
@@ -35,6 +37,11 @@ class WebTest < Minitest::Test
 					"plan_name" => "test_usd",
 					"expires_at" => Time.now + 100
 				}],
+				["customerid2"] => [{
+					"balance" => BigDecimal(10),
+					"plan_name" => "test_usd",
+					"expires_at" => Time.now + 100
+				}],
 				["customerid_low"] => [{
 					"balance" => BigDecimal("0.01"),
 					"plan_name" => "test_usd",
@@ -58,7 +65,9 @@ class WebTest < Minitest::Test
 					"catapult_fwd-[phone]" => "xmpp:[e-mail]",
 					"catapult_fwd_timeout-customer_customerid_low@component" => "30",
 					"catapult_fwd-[phone]" => "xmpp:[e-mail]",
-					"catapult_fwd_timeout-customer_customerid_limit@component" => "30"
+					"catapult_fwd_timeout-customer_customerid_limit@component" => "30",
+					"catapult_fwd-[phone]" => "xmpp:[e-mail]",
+					"catapult_fwd_timeout-customer_customerid2@component" => "30"
 				),
 				ibr_repo: FakeIBRRepo.new(
 					"sgx" => {
@@ -321,6 +330,40 @@ class WebTest < Minitest::Test
 	end
 	em :test_inbound
 
+	def test_inbound_no_bwmsgsv2
+		CustomerFwd::BANDWIDTH_VOICE.expect(
+			:create_call,
+			OpenStruct.new(data: OpenStruct.new(call_id: "ocall")),
+			["test_bw_account"],
+			body: Matching.new do |arg|
+				assert_equal(
+					"http://example.org/inbound/calls/acall?customer_id=customerid2",
+					arg.answer_url
+				)
+			end
+		)
+
+		post(
+			"/inbound/calls",
+			{
+				from: "[phone]",
+				to: "[phone]",
+				callId: "acall"
+			}.to_json,
+			{ "CONTENT_TYPE" => "application/json" }
+		)
+
+		assert last_response.ok?
+		assert_equal(
+			"<?xml version=\"1.0\" encoding=\"utf-8\" ?><Response>" \
+			"<Ring answerCall=\"false\" duration=\"300\" />" \
+			"</Response>",
+			last_response.body
+		)
+		assert_mock CustomerFwd::BANDWIDTH_VOICE
+	end
+	em :test_inbound_no_bwmsgsv2
+
 	def test_inbound_low
 		ExpiringLock::REDIS.expect(
 			:set,