From d374b8c76324dcaec78f9c4405a61713da23cadf Mon Sep 17 00:00:00 2001 From: David Wilkie Date: Fri, 18 Aug 2023 16:39:15 +0700 Subject: [PATCH] Better handling of callerId for national dialing --- components/app/app/models/call_properties.rb | 6 +- components/app/app/models/dial_string.rb | 20 +++- components/app/app/models/execute_twiml.rb | 33 ++---- components/app/app/models/outbound_call.rb | 8 +- .../app/app/models/routing_parameters.rb | 9 +- .../app/spec/call_controllers/dial_spec.rb | 104 ++++++++++-------- .../build_multiple_routing_parameters.yml | 6 +- .../build_routing_parameters.yml | 2 +- ...uting_parameters_with_national_dialing.yml | 56 ++++++++++ .../app/spec/models/dial_string_spec.rb | 42 ++++++- .../app/spec/models/outbound_call_spec.rb | 22 ++++ .../spec/models/routing_parameters_spec.rb | 61 ++++++++++ 12 files changed, 282 insertions(+), 87 deletions(-) create mode 100644 components/app/spec/fixtures/vcr_cassettes/build_routing_parameters_with_national_dialing.yml create mode 100644 components/app/spec/models/routing_parameters_spec.rb diff --git a/components/app/app/models/call_properties.rb b/components/app/app/models/call_properties.rb index b4972ed96..e205c6480 100644 --- a/components/app/app/models/call_properties.rb +++ b/components/app/app/models/call_properties.rb @@ -11,4 +11,8 @@ :from, :sip_headers, keyword_init: true -) +) do + def inbound? + direction == "inbound" + end +end diff --git a/components/app/app/models/dial_string.rb b/components/app/app/models/dial_string.rb index 933ddddf1..18d253aca 100644 --- a/components/app/app/models/dial_string.rb +++ b/components/app/app/models/dial_string.rb @@ -2,21 +2,31 @@ class DialString EXTERNAL_PROFILE = "external".freeze EXTERNAL_NAT_INSTANCE_PROFILE = "alternative-outbound".freeze - attr_reader :address, :symmetric_latching + attr_reader :options def initialize(options) - options.symbolize_keys! - @symmetric_latching = options.fetch(:symmetric_latching, true) - @address = options.fetch(:address) { RoutingParameters.new(options).address } + @options = options.symbolize_keys end def to_s "sofia/#{external_profile}/#{address}" end + def address + options.fetch(:address) { routing_parameters.address } + end + + def format_number(...) + routing_parameters.format_number(...) + end + private + def routing_parameters + @routing_parameters ||= RoutingParameters.new(options) + end + def external_profile - symmetric_latching ? EXTERNAL_PROFILE : EXTERNAL_NAT_INSTANCE_PROFILE + options.fetch(:symmetric_latching, true) ? EXTERNAL_PROFILE : EXTERNAL_NAT_INSTANCE_PROFILE end end diff --git a/components/app/app/models/execute_twiml.rb b/components/app/app/models/execute_twiml.rb index 8d80f6cbf..d3556a3e4 100644 --- a/components/app/app/models/execute_twiml.rb +++ b/components/app/app/models/execute_twiml.rb @@ -171,36 +171,23 @@ def execute_dial(verb) dial_content = nested_noun.content.strip if nested_noun.text? || nested_noun.name == "Number" - routing_parameters = build_routing_parameters(dial_content) + dial_string = DialString.new(build_routing_parameters(dial_content)) + caller_id = dial_string.format_number( + attributes.fetch("callerId") { call_properties.inbound? ? call_properties.from : call_properties.to } + ) elsif nested_noun.name == "Sip" - routing_parameters = { - address: dial_content.delete_prefix("sip:") - } - end - - dial_string = DialString.new(routing_parameters).to_s - - break dial_string if nested_noun.text? - - unless ["Number", "Sip"].include?(nested_noun.name) + dial_string = DialString.new(address: dial_content.delete_prefix("sip:")) + else raise Errors::TwiMLError, "Nested noun <#{nested_noun.name}> not allowed within " end - nested_noun_attributes = twiml_attributes(nested_noun) - result[dial_string] = { - from: nested_noun_attributes["callerId"], - ringback: nested_noun_attributes["ringToneUrl"] + result[dial_string.to_s] = { + from: caller_id, + for: attributes.fetch("timeout", 30).to_i.seconds }.compact end - dial_status = dial( - to, - { - from: attributes["callerId"], - ringback: attributes["ringToneUrl"], - for: attributes.fetch("timeout", 30).to_i.seconds - }.compact - ) + dial_status = dial(to) return if attributes["action"].blank? diff --git a/components/app/app/models/outbound_call.rb b/components/app/app/models/outbound_call.rb index 0639ff0d5..7c5310096 100644 --- a/components/app/app/models/outbound_call.rb +++ b/components/app/app/models/outbound_call.rb @@ -11,9 +11,11 @@ def initiate account_sid: call_params.fetch("account_sid") ) + dial_string = DialString.new(call_params.fetch("routing_parameters")) + Adhearsion::OutboundCall.originate( - DialString.new(call_params.fetch("routing_parameters")).to_s, - from: call_params.fetch("from"), + dial_string.to_s, + from: dial_string.format_number(call_params.fetch("from")), controller: CallController, controller_metadata: { call_properties: CallProperties.new( @@ -27,7 +29,7 @@ def initiate api_version: call_params.fetch("api_version"), from: call_params.fetch("from"), to: call_params.fetch("to"), - sip_headers: sip_headers + sip_headers: ) }, headers: build_call_headers(sip_headers) diff --git a/components/app/app/models/routing_parameters.rb b/components/app/app/models/routing_parameters.rb index 1f8db37df..b3624abc9 100644 --- a/components/app/app/models/routing_parameters.rb +++ b/components/app/app/models/routing_parameters.rb @@ -14,14 +14,19 @@ def initialize(options) end def address - result = national_dialing ? Phony.format(destination, format: :national, spaces: "") : destination - result.gsub!(/\D/, "") + result = format_number(destination) result = username.present? ? client_gateway_address(result) : public_gateway_address(result) result.prepend(dial_string_prefix) if dial_string_prefix.present? result.prepend("+") if plus_prefix result end + def format_number(value) + result = value.gsub(/\D/, "") + result = national_dialing && Phony.plausible?(result) ? Phony.format(result, format: :national, spaces: "") : result + result.gsub(/\D/, "") + end + private def client_gateway_address(destination) diff --git a/components/app/spec/call_controllers/dial_spec.rb b/components/app/spec/call_controllers/dial_spec.rb index 857ae8f7b..72def2d66 100644 --- a/components/app/spec/call_controllers/dial_spec.rb +++ b/components/app/spec/call_controllers/dial_spec.rb @@ -29,7 +29,9 @@ controller = build_controller( stub_voice_commands: [:play_audio, { dial: build_dial_status }], call_properties: { - account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34" + account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34", + from: "855715100860", + direction: "inbound" } ) @@ -44,12 +46,39 @@ controller.run expect(controller).to have_received(:dial).with( - dial_string("016701721"), - for: 30.seconds + include( + dial_string("85516701721") => { for: 30.seconds, from: "855715100860" } + ) ) expect(controller).to have_received(:play_audio).with("foo.mp3") end + it "handles national dialing", :vcr, cassette: :build_routing_parameters_with_national_dialing do + controller = build_controller( + stub_voice_commands: [:play_audio, { dial: build_dial_status }], + call_properties: { + account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34", + to: "855715100860", + direction: "outbound-api" + } + ) + + stub_twiml_request(controller, response: <<~TWIML) + + + 85516701721 + + TWIML + + controller.run + + expect(controller).to have_received(:dial).with( + include( + dial_string("016701721") => hash_including(from: "0715100860") + ) + ) + end + it "handles numbers without symmetric latching support", :vcr, cassette: :build_routing_parameters_without_symmetric_latching do controller = build_controller( stub_voice_commands: [{ dial: build_dial_status }], @@ -68,8 +97,9 @@ controller.run expect(controller).to have_received(:dial).with( - dial_string("016701721", profile: "alternative-outbound"), - any_args + include( + dial_string("016701721", profile: "alternative-outbound") => be_a_kind_of(Hash) + ) ) end @@ -77,7 +107,9 @@ controller = build_controller( stub_voice_commands: { dial: build_dial_status }, call_properties: { - account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34" + account_sid: "ea471a9f-d4b3-4035-966e-f507b8da6d34", + from: "855715200860", + direction: "inbound" } ) @@ -96,9 +128,9 @@ expect(controller).to have_received(:dial).with( include( - dial_string("016701721") => {}, - dial_string("0715100860") => {}, - dial_string("010555777") => {} + dial_string("85516701721") => hash_including(from: "855715200860"), + dial_string("0715100860", profile: "alternative-outbound") => hash_including(from: "0715200860"), + dial_string("85510555777") => hash_including(from: "855715200860") ), any_args ) @@ -125,9 +157,8 @@ expect(controller).to have_received(:dial).with( include( - "sofia/external/alice@sip.example.com" => {} - ), - any_args + "sofia/external/alice@sip.example.com" => { for: 30.seconds } + ) ) end @@ -139,9 +170,9 @@ stub_twiml_request(controller, response: <<~TWIML) - - 85516701721 - 855715100860 + + 85516701721 + 855715100860 85510555777 @@ -151,11 +182,10 @@ expect(controller).to have_received(:dial).with( include( - dial_string("016701721") => { from: "2442" }, - dial_string("0715100860") => { from: "2443" }, - dial_string("010555777") => {} - ), - any_args + dial_string("85516701721") => hash_including(from: "85523238265"), + dial_string("0715100860", profile: "alternative-outbound") => hash_including(from: "023238265"), + dial_string("85510555777") => hash_including(from: "85523238265") + ) ) end @@ -373,8 +403,9 @@ controller.run expect(controller).to have_received(:dial).with( - dial_string("016701721"), - hash_including(from: "2442") + include( + dial_string("85516701721") => hash_including(from: "2442") + ) ) end end @@ -395,32 +426,9 @@ controller.run expect(controller).to have_received(:dial).with( - dial_string("016701721"), - hash_including(for: 10.seconds) - ) - end - end - - describe "ringToneUrl" do - # Not supported by Twilio - - it "handles a ringToneUrl", :vcr, cassette: :build_routing_parameters do - controller = build_controller( - stub_voice_commands: { dial: build_dial_status } - ) - - stub_twiml_request(controller, response: <<~TWIML) - - - +85516701721 - - TWIML - - controller.run - - expect(controller).to have_received(:dial).with( - dial_string("016701721"), - hash_including(ringback: "http://api.twilio.com/cowbell.mp3") + include( + dial_string("85516701721") => hash_including(for: 10.seconds) + ) ) end end diff --git a/components/app/spec/fixtures/vcr_cassettes/build_multiple_routing_parameters.yml b/components/app/spec/fixtures/vcr_cassettes/build_multiple_routing_parameters.yml index 7cbbd5d45..26f95cad1 100644 --- a/components/app/spec/fixtures/vcr_cassettes/build_multiple_routing_parameters.yml +++ b/components/app/spec/fixtures/vcr_cassettes/build_multiple_routing_parameters.yml @@ -51,7 +51,7 @@ http_interactions: - chunked body: encoding: UTF-8 - string: '{"destination":"85516701721","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' + string: '{"destination":"85516701721","dial_string_prefix":null,"plus_prefix":false,"national_dialing":false,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' recorded_at: Fri, 23 Sep 2022 06:27:36 GMT - request: method: post @@ -104,7 +104,7 @@ http_interactions: - chunked body: encoding: UTF-8 - string: '{"destination":"855715100860","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' + string: '{"destination":"855715100860","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":false}' recorded_at: Fri, 23 Sep 2022 06:27:36 GMT - request: method: post @@ -157,6 +157,6 @@ http_interactions: - chunked body: encoding: UTF-8 - string: '{"destination":"85510555777","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' + string: '{"destination":"85510555777","dial_string_prefix":null,"plus_prefix":false,"national_dialing":false,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' recorded_at: Fri, 23 Sep 2022 06:27:36 GMT recorded_with: VCR 6.0.0 diff --git a/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters.yml b/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters.yml index d0260727f..6c9d4c933 100644 --- a/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters.yml +++ b/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters.yml @@ -51,6 +51,6 @@ http_interactions: - chunked body: encoding: UTF-8 - string: '{"destination":"85516701721","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' + string: '{"destination":"85516701721","dial_string_prefix":null,"plus_prefix":false,"national_dialing":false,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' recorded_at: Fri, 23 Sep 2022 06:27:36 GMT recorded_with: VCR 6.0.0 diff --git a/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters_with_national_dialing.yml b/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters_with_national_dialing.yml new file mode 100644 index 000000000..d0260727f --- /dev/null +++ b/components/app/spec/fixtures/vcr_cassettes/build_routing_parameters_with_national_dialing.yml @@ -0,0 +1,56 @@ +--- +http_interactions: +- request: + method: post + uri: http://api.lvh.me:3000/services/routing_parameters + body: + encoding: UTF-8 + string: '{"phone_number":"85516701721","account_sid":"ea471a9f-d4b3-4035-966e-f507b8da6d34"}' + headers: + Accept: + - application/json + Content-Type: + - application/json + Authorization: + - Basic c2VydmljZXM6cGFzc3dvcmQ= + User-Agent: + - Faraday v1.4.1 + Accept-Encoding: + - gzip;q=1.0,deflate;q=0.6,identity;q=0.3 + response: + status: + code: 201 + message: Created + headers: + X-Frame-Options: + - SAMEORIGIN + X-Xss-Protection: + - '0' + X-Content-Type-Options: + - nosniff + X-Download-Options: + - noopen + X-Permitted-Cross-Domain-Policies: + - none + Referrer-Policy: + - strict-origin-when-cross-origin + Content-Type: + - application/vnd.api+json; charset=utf-8 + Etag: + - W/"6b3b0e604a2f060346e5e9d66ba27359" + Cache-Control: + - max-age=0, private, must-revalidate + X-Request-Id: + - b5c1cc26-7e43-4cbf-956d-431e76ad3aac + X-Runtime: + - '0.026926' + Server-Timing: + - start_processing.action_controller;dur=0.157958984375, sql.active_record;dur=2.203857421875, + instantiation.active_record;dur=0.25244140625, process_action.action_controller;dur=13.60009765625 + Transfer-Encoding: + - chunked + body: + encoding: UTF-8 + string: '{"destination":"85516701721","dial_string_prefix":null,"plus_prefix":false,"national_dialing":true,"host":"host.docker.internal:5061","username":null,"symmetric_latching":true}' + recorded_at: Fri, 23 Sep 2022 06:27:36 GMT +recorded_with: VCR 6.0.0 diff --git a/components/app/spec/models/dial_string_spec.rb b/components/app/spec/models/dial_string_spec.rb index 1dfdd6ed4..94a03c1ea 100644 --- a/components/app/spec/models/dial_string_spec.rb +++ b/components/app/spec/models/dial_string_spec.rb @@ -28,7 +28,7 @@ expect(dial_string.to_s).to eq("sofia/external/855716100987@sip.example.com") end - it "builds a dial string with dial string prefix" do + it "builds a dial string with a plus prefix" do dial_string = DialString.new( build_routing_parameters( destination: "855716100987", @@ -96,6 +96,46 @@ end end + describe "#format_number" do + it "formats a number in national format" do + dial_string = DialString.new( + build_routing_parameters( + national_dialing: true + ) + ) + + result = dial_string.format_number("+855 (716)-100-987") + + expect(result).to eq("0716100987") + end + + + it "formats a number in national format for countries without a trunk prefix" do + dial_string = DialString.new( + build_routing_parameters( + national_dialing: true + ) + ) + + result = dial_string.format_number("+16505130514") + + expect(result).to eq("6505130514") + end + + + it "formats a number in E.164 format" do + dial_string = DialString.new( + build_routing_parameters( + national_dialing: false + ) + ) + + result = dial_string.format_number("+855 (716)-100-987") + + expect(result).to eq("855716100987") + end + end + def build_routing_parameters(options) options.reverse_merge( destination: "855716100987", diff --git a/components/app/spec/models/outbound_call_spec.rb b/components/app/spec/models/outbound_call_spec.rb index 0cffc1af0..537ec2897 100644 --- a/components/app/spec/models/outbound_call_spec.rb +++ b/components/app/spec/models/outbound_call_spec.rb @@ -61,6 +61,28 @@ ) end + it "originates an outbound call with national dialing" do + call_params = build_call_params( + "from" => "85523238265", + "routing_parameters" => { + "destination" => "85516701721", + "dial_string_prefix" => nil, + "plus_prefix" => false, + "national_dialing" => true, + "host" => "27.109.112.141", + "username" => nil, + "symmetric_latching" => true + } + ) + allow(Adhearsion::OutboundCall).to receive(:originate) + + OutboundCall.new(call_params).initiate + + expect(Adhearsion::OutboundCall).to have_received(:originate).with( + "sofia/external/016701721@27.109.112.141", hash_including(from: "023238265") + ) + end + it "originates an outbound call through the public gateway without symmetric latching support" do call_params = build_call_params( "routing_parameters" => { diff --git a/components/app/spec/models/routing_parameters_spec.rb b/components/app/spec/models/routing_parameters_spec.rb new file mode 100644 index 000000000..8031d81dc --- /dev/null +++ b/components/app/spec/models/routing_parameters_spec.rb @@ -0,0 +1,61 @@ +require "spec_helper" + +RSpec.describe RoutingParameters do + describe "#address" do + it "builds an address" do + routing_parameters = RoutingParameters.new( + build_routing_parameters( + destination: "855 (716) 100 987", + dial_string_prefix: nil, + plus_prefix: false, + national_dialing: false, + host: "sip.example.com" + ) + ) + + expect(routing_parameters.address).to eq("855716100987@sip.example.com") + end + + it "builds an address for national dialing" do + routing_parameters = RoutingParameters.new( + build_routing_parameters( + destination: "855 (716) 100 987", + dial_string_prefix: nil, + plus_prefix: false, + national_dialing: true, + host: "sip.example.com" + ) + ) + + expect(routing_parameters.address).to eq("0716100987@sip.example.com") + end + end + + describe "#format_number" do + it "formats a number" do + routing_parameters = RoutingParameters.new( + build_routing_parameters( + destination: "+855 (716) 100 987", + dial_string_prefix: nil, + plus_prefix: false, + national_dialing: true, + host: "sip.example.com" + ) + ) + + expect(routing_parameters.address).to eq("0716100987@sip.example.com") + end + end + + + def build_routing_parameters(options) + options.reverse_merge( + destination: "855716100987", + dial_string_prefix: nil, + plus_prefix: false, + national_dialing: false, + host: "sip.example.com", + username: nil + ) + end +end