From 2bed43b0bb44f88d8afcf767fd0a720100b0ee53 Mon Sep 17 00:00:00 2001 From: Jordan Verasamy Date: Fri, 16 Nov 2018 11:35:35 -0500 Subject: [PATCH 1/4] implement backwards compatible hostname scheme reader --- .../exporters/magento/soap.rb | 10 ++- .../exporters/magento/soap_spec.rb | 70 +++++++++++++++---- 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/lib/shopify_transporter/exporters/magento/soap.rb b/lib/shopify_transporter/exporters/magento/soap.rb index 63f550d..fefe7ee 100644 --- a/lib/shopify_transporter/exporters/magento/soap.rb +++ b/lib/shopify_transporter/exporters/magento/soap.rb @@ -73,13 +73,21 @@ def call_with_retries(method, params, retry_count = 0) end def soap_client + default_scheme = 'https://' + hostname_with_scheme = protocol_provided?(@hostname) ? @hostname : default_scheme + @hostname + @soap_client ||= Savon.client( - wsdl: "#{@hostname}/api/v2_soap?wsdl", + wsdl: "#{hostname_with_scheme}/api/v2_soap?wsdl", open_timeout: 500, read_timeout: 500, ) end + def protocol_provided?(address) + pattern = /(http)s?:\/\/.*/ + (pattern =~ address) == 0 + end + def soap_session_id return @soap_session_id if @soap_session_id.present? diff --git a/spec/shopify_transporter/exporters/magento/soap_spec.rb b/spec/shopify_transporter/exporters/magento/soap_spec.rb index 67bf92b..13ee5df 100644 --- a/spec/shopify_transporter/exporters/magento/soap_spec.rb +++ b/spec/shopify_transporter/exporters/magento/soap_spec.rb @@ -5,7 +5,7 @@ module ShopifyTransporter module Exporters module Magento RSpec.describe Soap do - let(:init_params) do + let(:init_params_https) do { hostname: 'https://example.com', username: 'testuser', @@ -14,9 +14,27 @@ module Magento } end - def stub_client_call(mock_client) + let(:init_params_http) do + { + hostname: 'http://example.com', + username: 'testuser', + api_key: 'testapikey', + batch_config: {}, + } + end + + let(:init_params_no_scheme) do + { + hostname: 'example.com', + username: 'testuser', + api_key: 'testapikey', + batch_config: {}, + } + end + + def stub_client_call(mock_client, scheme: 'https') expect(Savon).to receive(:client).with( - wsdl: "https://example.com/api/v2_soap?wsdl", + wsdl: "#{scheme}://example.com/api/v2_soap?wsdl", open_timeout: 500, read_timeout: 500, ).and_return(mock_client) @@ -39,11 +57,37 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } end describe '#call' do + context 'determining host connection protocol' do + it 'passes along https if it is provided' do + mock_client = spy('mock_client') + stub_client_call(mock_client, scheme: 'https') + stub_login_call(mock_client) + + Soap.new(init_params_https).call(:test_call, {}) + end + + it 'passes along http if it is provided' do + mock_client = spy('mock_client') + stub_client_call(mock_client, scheme: 'http') + stub_login_call(mock_client) + + Soap.new(init_params_http).call(:test_call, {}) + end + + it 'assumes https if no scheme is provided' do + mock_client = spy('mock_client') + stub_client_call(mock_client, scheme: 'https') + stub_login_call(mock_client) + + Soap.new(init_params_no_scheme).call(:test_call, {}) + end + end + it 'raises FailedLoginError if login response does not contain a session id' do mock_client = spy('mock_client') stub_client_call(mock_client) - expect { Soap.new(init_params).call(:test_call, {}) }.to raise_error(Soap::FailedLoginError) + expect { Soap.new(init_params_https).call(:test_call, {}) }.to raise_error(Soap::FailedLoginError) end it 'raises FailedLoginError with the right message and format' do @@ -53,7 +97,7 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } expected_error_message = "Unable to obtain SOAP session ID from server.\n\nDetails:\n{:not_the_right_key=>0}" - expect { Soap.new(init_params).call(:test_call, {}) }.to raise_error(Soap::FailedLoginError, expected_error_message) + expect { Soap.new(init_params_https).call(:test_call, {}) }.to raise_error(Soap::FailedLoginError, expected_error_message) end it 'creates a session correctly if the login response has keys :login_response and :login_return' do @@ -61,7 +105,7 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } stub_client_call(mock_client) stub_login_call(mock_client) - Soap.new(init_params).call(:test_call, {}) + Soap.new(init_params_https).call(:test_call, {}) end it 'creates a session correctly if login response has keys :login_response_param and :result' do @@ -69,7 +113,7 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } stub_client_call(mock_client) stub_login_call(mock_client, body: { login_response_param: { result: '456' } }) - Soap.new(init_params).call(:test_call, {}) + Soap.new(init_params_https).call(:test_call, {}) end it 'calls Savon with the fn, session_id and params' do @@ -84,7 +128,7 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } }, ) - Soap.new(init_params).call(:test_call, {}) + Soap.new(init_params_https).call(:test_call, {}) end it 'retries soap calls up to 4 times with a delay when there is a savon error' do @@ -92,7 +136,7 @@ def stub_login_call(mock_client, body: { login_response: { login_return: '123' } stub_client_call(mock_client) stub_login_call(mock_client) - soap_instance = Soap.new(init_params) + soap_instance = Soap.new(init_params_https) retries = 0 expect(mock_client).to receive(:call).with( @@ -130,7 +174,7 @@ def expected_batching_filter(batch_key, batch_range_string) end it 'returns an enumerator with the results of #call for each batch specified' do - soap_client = Soap.new(init_params.merge( + soap_client = Soap.new(init_params_https.merge( batch_config: { 'first_id' => 0, 'last_id' => 7, @@ -151,7 +195,7 @@ def expected_batching_filter(batch_key, batch_range_string) end it 'works when first id and last id are the same' do - soap_client = Soap.new(init_params.merge( + soap_client = Soap.new(init_params_https.merge( batch_config: { 'first_id' => 0, 'last_id' => 0, @@ -170,7 +214,7 @@ def expected_batching_filter(batch_key, batch_range_string) end it 'correctly merges params passed in with the batching filter' do - soap_client = Soap.new(init_params.merge( + soap_client = Soap.new(init_params_https.merge( batch_config: { 'first_id' => 0, 'last_id' => 0, @@ -202,7 +246,7 @@ def expected_batching_filter(batch_key, batch_range_string) end it 'skips the batch and continues with the next batch if call raises an error' do - soap_client = Soap.new(init_params.merge( + soap_client = Soap.new(init_params_https.merge( batch_config: { 'first_id' => 0, 'last_id' => 7, From 1cd76353ca1707b3dd8b060d49729c1ccd4d5beb Mon Sep 17 00:00:00 2001 From: Jordan Verasamy Date: Wed, 21 Nov 2018 15:44:31 -0500 Subject: [PATCH 2/4] fix rubocop --- lib/shopify_transporter/exporters/magento/soap.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/shopify_transporter/exporters/magento/soap.rb b/lib/shopify_transporter/exporters/magento/soap.rb index fefe7ee..f931e26 100644 --- a/lib/shopify_transporter/exporters/magento/soap.rb +++ b/lib/shopify_transporter/exporters/magento/soap.rb @@ -84,7 +84,7 @@ def soap_client end def protocol_provided?(address) - pattern = /(http)s?:\/\/.*/ + pattern = %r{(http)s?:\/\/.*} (pattern =~ address) == 0 end From 48cdcdaa84b9a662ffe1ae42c11785bf4e81392f Mon Sep 17 00:00:00 2001 From: Jordan Verasamy Date: Wed, 21 Nov 2018 18:29:47 -0500 Subject: [PATCH 3/4] use URI library --- lib/shopify_transporter/exporters/magento/soap.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/shopify_transporter/exporters/magento/soap.rb b/lib/shopify_transporter/exporters/magento/soap.rb index f931e26..fffa8ab 100644 --- a/lib/shopify_transporter/exporters/magento/soap.rb +++ b/lib/shopify_transporter/exporters/magento/soap.rb @@ -73,9 +73,6 @@ def call_with_retries(method, params, retry_count = 0) end def soap_client - default_scheme = 'https://' - hostname_with_scheme = protocol_provided?(@hostname) ? @hostname : default_scheme + @hostname - @soap_client ||= Savon.client( wsdl: "#{hostname_with_scheme}/api/v2_soap?wsdl", open_timeout: 500, @@ -83,9 +80,9 @@ def soap_client ) end - def protocol_provided?(address) - pattern = %r{(http)s?:\/\/.*} - (pattern =~ address) == 0 + def hostname_with_scheme + uri = URI.parse(@hostname) + uri.scheme.present? ? @hostname : 'https://' + @hostname end def soap_session_id From 851dd3e6e2976576e13b679d88a85825bf1cadf1 Mon Sep 17 00:00:00 2001 From: Jordan Verasamy Date: Thu, 22 Nov 2018 12:18:45 -0500 Subject: [PATCH 4/4] one line --- lib/shopify_transporter/exporters/magento/soap.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/shopify_transporter/exporters/magento/soap.rb b/lib/shopify_transporter/exporters/magento/soap.rb index fffa8ab..4e0de17 100644 --- a/lib/shopify_transporter/exporters/magento/soap.rb +++ b/lib/shopify_transporter/exporters/magento/soap.rb @@ -81,8 +81,7 @@ def soap_client end def hostname_with_scheme - uri = URI.parse(@hostname) - uri.scheme.present? ? @hostname : 'https://' + @hostname + URI.parse(@hostname).scheme.present? ? @hostname : 'https://' + @hostname end def soap_session_id