From 81fab28721d10f8d4b0fdd42d24794f5cdd649ca Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Sun, 22 Jul 2018 00:09:01 +0900 Subject: [PATCH 1/5] Support multiple endpoints for redundancy --- lib/cm_sms/configuration.rb | 11 +++++++---- lib/cm_sms/request.rb | 4 ++-- spec/cm_sms/configuration_spec.rb | 22 ++++++++++++++++------ spec/cm_sms/request_spec.rb | 14 ++++++++++++++ spec/cm_sms_spec.rb | 2 +- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/lib/cm_sms/configuration.rb b/lib/cm_sms/configuration.rb index 14e48a9..871faf4 100644 --- a/lib/cm_sms/configuration.rb +++ b/lib/cm_sms/configuration.rb @@ -4,16 +4,19 @@ class ProductTokenMissing < ArgumentError; end class EndpointMissing < ArgumentError; end class PathMissing < ArgumentError; end - ENDPOINT = 'https://sgw01.cm.nl'.freeze + ENDPOINTS = %w[https://sgw01.cm.nl https://sgw02.cm.nl].map(&:freeze).freeze PATH = '/gateway.ashx'.freeze DCS = 0 - attr_accessor :from, :to, :product_token, :endpoint, :path, :dcs + attr_accessor :from, :to, :product_token + attr_writer :endpoints, :path, :dcs alias api_key= product_token= + alias endpoint= endpoints= - def endpoint - @endpoint || ENDPOINT + def endpoints + endpoints = Array(@endpoints) + endpoints.empty? ? ENDPOINTS : endpoints end def path diff --git a/lib/cm_sms/request.rb b/lib/cm_sms/request.rb index 1bf89ef..069099c 100644 --- a/lib/cm_sms/request.rb +++ b/lib/cm_sms/request.rb @@ -8,12 +8,12 @@ class Request def initialize(body) @body = body - @endpoint = CmSms.config.endpoint + @endpoint = CmSms.config.endpoints.sample @path = CmSms.config.path end def perform - raise CmSms::Configuration::EndpointMissing, "Please provide an valid api endpoint.\nIf you leave this config blank, the default will be set to https://sgw01.cm.nl." if @endpoint.nil? || @endpoint.empty? + raise CmSms::Configuration::EndpointMissing, 'Please provide an valid api endpoint.' if @endpoint.nil? || @endpoint.empty? raise CmSms::Configuration::PathMissing, "Please provide an valid api path.\nIf you leave this config blank, the default will be set to /gateway.ashx." if @path.nil? || @path.empty? uri = URI.parse(@endpoint) diff --git a/spec/cm_sms/configuration_spec.rb b/spec/cm_sms/configuration_spec.rb index 3fbe891..f08ae10 100644 --- a/spec/cm_sms/configuration_spec.rb +++ b/spec/cm_sms/configuration_spec.rb @@ -3,7 +3,7 @@ RSpec.describe CmSms::Configuration do it 'has a endpoit set in constant' do - expect(CmSms::Configuration::ENDPOINT).to eq 'https://sgw01.cm.nl' + expect(CmSms::Configuration::ENDPOINTS).to eq %w[https://sgw01.cm.nl https://sgw02.cm.nl] end it 'has a path default' do @@ -12,20 +12,30 @@ let(:config) { described_class.new } - describe '#endpoint' do + describe '#endpoints' do context 'when endpoint is set through setter' do subject(:resource) do config.endpoint = 'http://local.host' config end it 'returns the setted endpoint' do - expect(resource.endpoint).to eq 'http://local.host' + expect(resource.endpoints).to eq ['http://local.host'] end end - context 'when endpoint is not set' do - it 'returns the default enpoint set in constant' do - expect(config.endpoint).to eq CmSms::Configuration::ENDPOINT + context 'when endpoints is set through setter' do + subject(:resource) do + config.endpoints = %w[http://local.host http://other.host] + config + end + it 'returns the setted endpoint' do + expect(resource.endpoints).to eq %w[http://local.host http://other.host] + end + end + + context 'when endpoints is not set' do + it 'returns the default endpoints set in constant' do + expect(config.endpoints).to eq CmSms::Configuration::ENDPOINTS end end end diff --git a/spec/cm_sms/request_spec.rb b/spec/cm_sms/request_spec.rb index a1901ed..3eeff7b 100644 --- a/spec/cm_sms/request_spec.rb +++ b/spec/cm_sms/request_spec.rb @@ -14,6 +14,20 @@ let(:request_body) { message.to_xml } let(:request) { described_class.new(request_body) } + describe '@endpoint' do + before { CmSms.configuration.endpoints = nil } + + context 'endpoint is randomized to sgw01' do + before { srand(0) } + it { expect(request.instance_variable_get('@endpoint')).to eq 'https://sgw01.cm.nl' } + end + + context 'endpoint is randomized to sgw02' do + before { srand(1) } + it { expect(request.instance_variable_get('@endpoint')).to eq 'https://sgw02.cm.nl' } + end + end + describe '#perform' do context 'when the API endpoint is missing' do let(:resource) do diff --git a/spec/cm_sms_spec.rb b/spec/cm_sms_spec.rb index 37568b7..463d085 100644 --- a/spec/cm_sms_spec.rb +++ b/spec/cm_sms_spec.rb @@ -21,7 +21,7 @@ expect(CmSms.config.from).to eq '+41 44 111 22 33' expect(CmSms.config.to).to eq '+41 44 111 22 33' expect(CmSms.config.product_token).to eq 'SOMETOKEN' - expect(CmSms.config.endpoint).to eq 'http://example.com' + expect(CmSms.config.endpoints).to eq ['http://example.com'] expect(CmSms.config.path).to eq '/example' end end From 99a5c5cbdf7d27f0447b27417132d6159b28940f Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Thu, 23 Aug 2018 05:05:46 +0900 Subject: [PATCH 2/5] Support configurable product_token and endpoints on message --- lib/cm_sms/message.rb | 14 ++++++-- lib/cm_sms/request.rb | 4 +-- spec/cm_sms/message_spec.rb | 70 ++++++++++++++++++++++++++++++++++--- spec/cm_sms/request_spec.rb | 9 ++++- 4 files changed, 88 insertions(+), 9 deletions(-) diff --git a/lib/cm_sms/message.rb b/lib/cm_sms/message.rb index 526c4ce..6d94bcb 100644 --- a/lib/cm_sms/message.rb +++ b/lib/cm_sms/message.rb @@ -12,6 +12,7 @@ class ToUnplausible < ArgumentError; end class DCSNotNumeric < ArgumentError; end attr_accessor :from, :to, :body, :dcs, :reference + attr_reader :product_token, :endpoints def initialize(attributes = {}) @from = attributes[:from] @@ -20,7 +21,16 @@ def initialize(attributes = {}) @body = attributes[:body] @reference = attributes[:reference] - @product_token = CmSms.config.product_token + self.product_token = attributes[:product_token] + self.endpoints = attributes[:endpoints] + end + + def product_token=(value) + @product_token = value || CmSms.config.product_token + end + + def endpoints=(value) + @endpoints = value ? Array(value) : CmSms.config.endpoints end def dcs_numeric? @@ -64,7 +74,7 @@ def product_token_present? end def request - Request.new(to_xml) + Request.new(to_xml, @endpoints) end def deliver diff --git a/lib/cm_sms/request.rb b/lib/cm_sms/request.rb index 069099c..7766412 100644 --- a/lib/cm_sms/request.rb +++ b/lib/cm_sms/request.rb @@ -6,9 +6,9 @@ class Request attr_reader :response - def initialize(body) + def initialize(body, endpoints = nil) @body = body - @endpoint = CmSms.config.endpoints.sample + @endpoint = (endpoints || CmSms.config.endpoints).sample @path = CmSms.config.path end diff --git a/spec/cm_sms/message_spec.rb b/spec/cm_sms/message_spec.rb index eb9965f..70ff27e 100644 --- a/spec/cm_sms/message_spec.rb +++ b/spec/cm_sms/message_spec.rb @@ -4,12 +4,17 @@ RSpec.describe CmSms::Message do let(:message_body) { 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirood tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At v' } + let(:product_token){ nil } + let(:endpoints) { nil } + let(:message) do message = described_class.new message.from = 'ACME' message.to = '+41 44 111 22 33' message.body = message_body message.reference = 'Ref:123' + message.product_token = product_token + message.endpoints = endpoints message end @@ -110,15 +115,57 @@ end end - describe '#product_token_present?' do - context 'when a valid product_token is provided' do + describe '#endpoints' do + context 'when a endpoints configured at config level' do + before { CmSms.configure { |config| config.endpoints = %w[bazqux bingbaz] } } + it { expect(message.endpoints).to eq %w[bazqux bingbaz] } + + context 'when a endpoints set on message' do + let(:endpoints) { 'foobar' } + it { expect(message.endpoints).to eq ['foobar'] } + end + end + + context 'when no endpoints is provided' do + before { CmSms.configure { |config| config.endpoints = nil } } + it { expect(message.endpoints).to eq %w[https://sgw01.cm.nl https://sgw02.cm.nl] } + + context 'when a endpoints set on message' do + let(:endpoints) { 'foobar' } + it { expect(message.endpoints).to eq ['foobar'] } + end + end + end + + describe '#product_token and #product_token_present?' do + context 'when a product_token configured at config level' do before { CmSms.configure { |config| config.product_token = 'SOMETOKEN' } } - it { expect(message.product_token_present?).to be true } + it { expect(message.product_token).to eq 'SOMETOKEN' } + it { expect(message.product_token_present?).to eq true } + + context 'when a product_token set on message' do + let(:product_token) { 'MSGTOKEN' } + it { expect(message.product_token).to eq 'MSGTOKEN' } + it { expect(message.product_token_present?).to eq true } + end end context 'when no product_token is provided' do before { CmSms.configure { |config| config.product_token = nil } } - it { expect(message.product_token_present?).to be false } + it { expect(message.product_token).to eq nil } + it { expect(message.product_token_present?).to eq false } + + context 'when a product_token set on message' do + let(:product_token) { 'MSGTOKEN' } + it { expect(message.product_token).to eq 'MSGTOKEN' } + it { expect(message.product_token_present?).to eq true } + end + end + + context 'when no product_token is blank' do + before { CmSms.configure { |config| config.product_token = '' } } + it { expect(message.product_token).to eq '' } + it { expect(message.product_token_present?).to eq false } end end @@ -208,7 +255,22 @@ end describe '#request' do + before { CmSms.configure { |config| config.endpoints = nil } } + it { expect(message.request).to be_kind_of(CmSms::Request) } + + it do + expect(CmSms::Request).to receive(:new).with(message.to_xml, %w[https://sgw01.cm.nl https://sgw02.cm.nl]) + message.request + end + + context 'when endpoints set' do + let(:endpoints){ 'foobar' } + it do + expect(CmSms::Request).to receive(:new).with(message.to_xml, %w[foobar]) + message.request + end + end end describe '#to_xml' do diff --git a/spec/cm_sms/request_spec.rb b/spec/cm_sms/request_spec.rb index 3eeff7b..1de6074 100644 --- a/spec/cm_sms/request_spec.rb +++ b/spec/cm_sms/request_spec.rb @@ -12,7 +12,8 @@ message end let(:request_body) { message.to_xml } - let(:request) { described_class.new(request_body) } + let(:endpoints) { nil } + let(:request) { described_class.new(request_body, endpoints) } describe '@endpoint' do before { CmSms.configuration.endpoints = nil } @@ -26,6 +27,12 @@ before { srand(1) } it { expect(request.instance_variable_get('@endpoint')).to eq 'https://sgw02.cm.nl' } end + + context 'when endpoints arg set' do + let(:endpoints) { %w[foobar bazqux] } + before { srand(0) } + it { expect(request.instance_variable_get('@endpoint')).to eq 'foobar' } + end end describe '#perform' do From 111f37da39ef6065592c45e45b5bf9a4c89e3b8f Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Thu, 23 Aug 2018 05:11:30 +0900 Subject: [PATCH 3/5] Fix rubocop --- spec/cm_sms/message_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/cm_sms/message_spec.rb b/spec/cm_sms/message_spec.rb index 70ff27e..51bb8f4 100644 --- a/spec/cm_sms/message_spec.rb +++ b/spec/cm_sms/message_spec.rb @@ -4,8 +4,8 @@ RSpec.describe CmSms::Message do let(:message_body) { 'Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirood tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua. At v' } - let(:product_token){ nil } - let(:endpoints) { nil } + let(:product_token) { nil } + let(:endpoints) { nil } let(:message) do message = described_class.new @@ -265,7 +265,7 @@ end context 'when endpoints set' do - let(:endpoints){ 'foobar' } + let(:endpoints) { 'foobar' } it do expect(CmSms::Request).to receive(:new).with(message.to_xml, %w[foobar]) message.request From 935cc8d492362adb2265e86701fd4e8343327e2a Mon Sep 17 00:00:00 2001 From: shields Date: Mon, 1 Jun 2020 17:02:44 +0900 Subject: [PATCH 4/5] Add HTTP timeout when contacting CM --- lib/cm_sms/configuration.rb | 11 ++++++--- lib/cm_sms/request.rb | 3 ++- spec/cm_sms/configuration_spec.rb | 38 +++++++++++++++++++++++-------- 3 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/cm_sms/configuration.rb b/lib/cm_sms/configuration.rb index 871faf4..0e4ef97 100644 --- a/lib/cm_sms/configuration.rb +++ b/lib/cm_sms/configuration.rb @@ -5,11 +5,12 @@ class EndpointMissing < ArgumentError; end class PathMissing < ArgumentError; end ENDPOINTS = %w[https://sgw01.cm.nl https://sgw02.cm.nl].map(&:freeze).freeze - PATH = '/gateway.ashx'.freeze - DCS = 0 + PATH = '/gateway.ashx'.freeze + DCS = 0 + TIMEOUT = 10 attr_accessor :from, :to, :product_token - attr_writer :endpoints, :path, :dcs + attr_writer :endpoints, :path, :dcs, :timeout alias api_key= product_token= alias endpoint= endpoints= @@ -27,6 +28,10 @@ def dcs @dcs || DCS end + def timeout + @timeout || TIMEOUT + end + def defaults @defaults ||= { from: from, to: to, dcs: dcs } end diff --git a/lib/cm_sms/request.rb b/lib/cm_sms/request.rb index 7766412..33a4d10 100644 --- a/lib/cm_sms/request.rb +++ b/lib/cm_sms/request.rb @@ -17,7 +17,8 @@ def perform raise CmSms::Configuration::PathMissing, "Please provide an valid api path.\nIf you leave this config blank, the default will be set to /gateway.ashx." if @path.nil? || @path.empty? uri = URI.parse(@endpoint) - Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https') do |http| + timeout = CmSms.config.timeout + Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == 'https', open_timeout: timeout, read_timeout: timeout) do |http| @response = Response.new(http.post(@path, body, 'Content-Type' => 'application/xml')) end response diff --git a/spec/cm_sms/configuration_spec.rb b/spec/cm_sms/configuration_spec.rb index f08ae10..54b2e21 100644 --- a/spec/cm_sms/configuration_spec.rb +++ b/spec/cm_sms/configuration_spec.rb @@ -18,7 +18,7 @@ config.endpoint = 'http://local.host' config end - it 'returns the setted endpoint' do + it 'returns the set endpoint' do expect(resource.endpoints).to eq ['http://local.host'] end end @@ -28,7 +28,7 @@ config.endpoints = %w[http://local.host http://other.host] config end - it 'returns the setted endpoint' do + it 'returns the set endpoint' do expect(resource.endpoints).to eq %w[http://local.host http://other.host] end end @@ -46,13 +46,13 @@ config.path = '/example' config end - it 'returns the setted path' do + it 'returns the set path' do expect(resource.path).to eq '/example' end end context 'when path is not set' do - it 'returns the default enpoint set in constant' do + it 'returns the default path set in constant' do expect(config.path).to eq CmSms::Configuration::PATH end end @@ -64,25 +64,43 @@ config.dcs = 8 config end - it 'returns the setted dcs' do + it 'returns the set dcs' do expect(resource.dcs).to eq 8 end end context 'when dcs is not set' do - it 'returns the default enpoint set in constant' do + it 'returns the default dcs set in constant' do expect(config.dcs).to eq CmSms::Configuration::DCS end end end + describe '#timeout' do + context 'when timeout is set through setter' do + subject(:resource) do + config.timeout = 20 + config + end + it 'returns the set timeout' do + expect(resource.timeout).to eq 20 + end + end + + context 'when timeout is not set' do + it 'returns the default timeout set in constant' do + expect(config.timeout).to eq CmSms::Configuration::TIMEOUT + end + end + end + describe '#product_token' do context 'when product_token is set through setter' do subject(:resource) do config.product_token = 'SOMETOKEN' config end - it 'returns the setted product_token' do + it 'returns the set product_token' do expect(resource.product_token).to eq 'SOMETOKEN' end end @@ -92,7 +110,7 @@ config.api_key = 'SOMEKEY' config end - it 'returns the setted product_token' do + it 'returns the set product_token' do expect(resource.product_token).to eq 'SOMEKEY' end end @@ -104,7 +122,7 @@ config.from = 'me' config end - it 'returns the setted from' do + it 'returns the set from' do expect(resource.from).to eq 'me' end end @@ -116,7 +134,7 @@ config.to = 'you' config end - it 'returns the setted to' do + it 'returns the set to' do expect(resource.to).to eq 'you' end end From 69f1eaf4c397fe095535b84d8d7019d3e33eab00 Mon Sep 17 00:00:00 2001 From: shields Date: Tue, 2 Jun 2020 15:52:32 +0900 Subject: [PATCH 5/5] Change API endpoint to https://gw.cmtelecom.com --- lib/cm_sms/configuration.rb | 2 +- spec/cm_sms/configuration_spec.rb | 2 +- spec/cm_sms/message_spec.rb | 4 ++-- spec/cm_sms/request_spec.rb | 8 ++++---- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/cm_sms/configuration.rb b/lib/cm_sms/configuration.rb index 0e4ef97..ebb9358 100644 --- a/lib/cm_sms/configuration.rb +++ b/lib/cm_sms/configuration.rb @@ -4,7 +4,7 @@ class ProductTokenMissing < ArgumentError; end class EndpointMissing < ArgumentError; end class PathMissing < ArgumentError; end - ENDPOINTS = %w[https://sgw01.cm.nl https://sgw02.cm.nl].map(&:freeze).freeze + ENDPOINTS = %w[https://gw.cmtelecom.com].map(&:freeze).freeze PATH = '/gateway.ashx'.freeze DCS = 0 TIMEOUT = 10 diff --git a/spec/cm_sms/configuration_spec.rb b/spec/cm_sms/configuration_spec.rb index 54b2e21..a9ff4ea 100644 --- a/spec/cm_sms/configuration_spec.rb +++ b/spec/cm_sms/configuration_spec.rb @@ -3,7 +3,7 @@ RSpec.describe CmSms::Configuration do it 'has a endpoit set in constant' do - expect(CmSms::Configuration::ENDPOINTS).to eq %w[https://sgw01.cm.nl https://sgw02.cm.nl] + expect(CmSms::Configuration::ENDPOINTS).to eq %w[https://gw.cmtelecom.com] end it 'has a path default' do diff --git a/spec/cm_sms/message_spec.rb b/spec/cm_sms/message_spec.rb index 51bb8f4..bddbcc0 100644 --- a/spec/cm_sms/message_spec.rb +++ b/spec/cm_sms/message_spec.rb @@ -128,7 +128,7 @@ context 'when no endpoints is provided' do before { CmSms.configure { |config| config.endpoints = nil } } - it { expect(message.endpoints).to eq %w[https://sgw01.cm.nl https://sgw02.cm.nl] } + it { expect(message.endpoints).to eq %w[https://gw.cmtelecom.com] } context 'when a endpoints set on message' do let(:endpoints) { 'foobar' } @@ -260,7 +260,7 @@ it { expect(message.request).to be_kind_of(CmSms::Request) } it do - expect(CmSms::Request).to receive(:new).with(message.to_xml, %w[https://sgw01.cm.nl https://sgw02.cm.nl]) + expect(CmSms::Request).to receive(:new).with(message.to_xml, %w[https://gw.cmtelecom.com]) message.request end diff --git a/spec/cm_sms/request_spec.rb b/spec/cm_sms/request_spec.rb index 1de6074..3ae5dc9 100644 --- a/spec/cm_sms/request_spec.rb +++ b/spec/cm_sms/request_spec.rb @@ -18,14 +18,14 @@ describe '@endpoint' do before { CmSms.configuration.endpoints = nil } - context 'endpoint is randomized to sgw01' do + context 'endpoint is randomized to first' do before { srand(0) } - it { expect(request.instance_variable_get('@endpoint')).to eq 'https://sgw01.cm.nl' } + it { expect(request.instance_variable_get('@endpoint')).to eq 'https://gw.cmtelecom.com' } end - context 'endpoint is randomized to sgw02' do + context 'endpoint is randomized to second' do before { srand(1) } - it { expect(request.instance_variable_get('@endpoint')).to eq 'https://sgw02.cm.nl' } + it { expect(request.instance_variable_get('@endpoint')).to eq 'https://gw.cmtelecom.com' } end context 'when endpoints arg set' do