Add write timeout configuration (backwards compatible) (#589)

* Revert "Revert "Accept write_timeout configuration, and supply it along to savoy (#587)""

This reverts commit f43ead8.

* Extract savon_params as a variable for manipulation

* extract Configuration#savon_params and test it directly

* Only supply and allow setting write_timeout when Savon supports it

* These tests need to execute against both old and new versions of Savon

Which means we can't actually instantiate the connection when
_pretending_ to have a recent savon version, or we'll trick our own
compatibility checks into passing Savon parameters it doesn't support.

lib/netsuite/configuration.rb

@@ -19,7 +19,13 @@ def attributes
     end
 
     def connection(params={}, credentials={}, soap_header_extra_info={})
-      client = Savon.client({
+      client = Savon.client(savon_params(params, credentials, soap_header_extra_info))
+      cache_wsdl(client)
+      client
+    end
+
+    def savon_params(params={}, credentials={}, soap_header_extra_info={})
+      full_params = {
         wsdl: cached_wsdl || wsdl,
         endpoint: endpoint,
         read_timeout: read_timeout,
@@ -32,11 +38,13 @@ def connection(params={}, credentials={}, soap_header_extra_info={})
         log_level: log_level,
         log: !silent, # turn off logging entirely if configured
         proxy: proxy,
-      }.update(params))
-      cache_wsdl(client)
-      return client
+      }
+      full_params.update(params)
+      full_params.update(write_timeout: write_timeout) if supports_write_timeout?
+      full_params
     end
 
+
     def filters(list = nil)
       if list
         self.filters = list
@@ -365,6 +373,20 @@ def open_timeout(timeout = nil)
       end
     end
 
+    def write_timeout=(timeout)
+      write_timeout_not_supported! unless supports_write_timeout?
+      attributes[:write_timeout] = timeout
+    end
+
+    def write_timeout(timeout = nil)
+      if timeout
+        write_timeout_not_supported! unless supports_write_timeout?
+        self.write_timeout = timeout
+      else
+        attributes[:write_timeout]
+      end
+    end
+
     def log=(path)
       attributes[:log] = path
     end
@@ -427,5 +449,15 @@ def multi_tenant!
     def multi_tenant?
       @multi_tenant
     end
+
+    private
+
+    def supports_write_timeout?
+      Savon::VERSION >= "2.13.0"
+    end
+
+    def write_timeout_not_supported!
+      fail(ConfigurationError, "Savon doesn't support write_timeout until version 2.13.0")
+    end
   end
 end

spec/netsuite/configuration_spec.rb

@@ -66,6 +66,86 @@
     end
   end
 
+  describe "#savon_params" do
+    it 'includes the expectd logging details' do
+      fake_logger = double "fake logger"
+      config.logger = fake_logger
+      config.log_level = :fake_level
+      config.silent = true
+
+      expect(config.savon_params).to include(logger: fake_logger)
+      expect(config.savon_params).to include(log_level: :fake_level)
+      expect(config.savon_params).to include(log: false)
+    end
+
+    context "when savon is on a recent version" do
+      before { stub_const "Savon::VERSION", "2.13.0" }
+
+      it 'includes the expected timeouts' do
+        config.read_timeout = 9.1
+        config.open_timeout = 10.2
+        config.write_timeout = 11.3
+        expect(config.savon_params).to include(read_timeout: be_within(0.01).of(9.1))
+        expect(config.savon_params).to include(open_timeout: be_within(0.01).of(10.2))
+        expect(config.savon_params).to include(write_timeout: be_within(0.01).of(11.3))
+      end
+    end
+
+    context "when savon is on an older version" do
+      before { stub_const "Savon::VERSION", "2.12.1" }
+
+      it 'includes the expected timeouts' do
+        config.read_timeout = 9.1
+        config.open_timeout = 10.2
+        expect(config.savon_params).to include(read_timeout: be_within(0.01).of(9.1))
+        expect(config.savon_params).to include(open_timeout: be_within(0.01).of(10.2))
+        expect(config.savon_params).not_to include(:write_timeout)
+      end
+    end
+
+    it 'constructs the appropriate soap headers' do
+      config.soap_header = { :foo => "foo-1" }
+      credentials = { :bar => "bar-2" }
+      soap_header_extra_info = { :baz => "baz-3" }
+
+      savon_params = config.savon_params({}, credentials, soap_header_extra_info)
+      expect(savon_params[:soap_header]).to include(:foo => "foo-1", :baz => "baz-3")
+      expect(savon_params[:soap_header]).to include("platformMsgs:passport")
+    end
+
+    it 'includes the correct wsdl' do
+      uncached_wsdl = double('uncached wsdl')
+      allow(config).to receive(:wsdl).and_return(uncached_wsdl)
+
+      cached_wsdl = double('cached wsdl')
+      allow(config).to receive(:cached_wsdl).and_return(cached_wsdl)
+      expect(config.savon_params[:wsdl]).to eq(cached_wsdl)
+
+      allow(config).to receive(:cached_wsdl).and_return(nil)
+      expect(config.savon_params[:wsdl]).to eq(uncached_wsdl)
+    end
+
+    it 'includes the other parameters' do
+      config.endpoint = "fake endpoint"
+      expect(config.savon_params).to include(:endpoint => "fake endpoint")
+
+      expect(config.savon_params).to include(:pretty_print_xml => true)
+
+      config.filters = [:a, :b]
+      expect(config.savon_params).to include(:filters => [:a, :b])
+
+      config.proxy = "fake proxy"
+      expect(config.savon_params).to include(:proxy => "fake proxy")
+
+      expect(config.savon_params[:namespaces]).to eq(config.namespaces)
+    end
+
+    it 'merges in the supplied params' do
+      supplied_params = { :foo => "bar", :baz => "blam" }
+      expect(config.savon_params(supplied_params)).to include(supplied_params)
+    end
+  end
+
   describe '#connection' do
     EXAMPLE_ENDPOINT = 'https://1023.suitetalk.api.netsuite.com/services/NetSuitePort_2020_2'
     before(:each) do
@@ -497,20 +577,43 @@
   end
 
   describe 'timeouts' do
-    it 'has defaults' do
-      expect(config.read_timeout).to eql(60)
-      expect(config.open_timeout).to be_nil
+    context "when savon is on a recent version" do
+      before { stub_const "Savon::VERSION", "2.13.0" }
+
+      it 'has defaults' do
+        expect(config.read_timeout).to eql(60)
+        expect(config.open_timeout).to be_nil
+        expect(config.write_timeout).to be_nil
+      end
+
+      it 'sets timeouts' do
+        config.read_timeout = 100
+        config.open_timeout = 60
+        config.write_timeout = 14
+
+        expect(config.read_timeout).to eql(100)
+        expect(config.open_timeout).to eql(60)
+        expect(config.write_timeout).to eql(14)
+      end
     end
 
-    it 'sets timeouts' do
-      config.read_timeout = 100
-      config.open_timeout = 60
+    context "when savon is on an older version" do
+      before { stub_const "Savon::VERSION", "2.12.1" }
+
+      it 'has defaults' do
+        expect(config.read_timeout).to eql(60)
+        expect(config.open_timeout).to be_nil
+      end
 
-      expect(config.read_timeout).to eql(100)
-      expect(config.open_timeout).to eql(60)
+      it 'sets timeouts' do
+        config.read_timeout = 100
+        config.open_timeout = 60
+        expect { config.write_timeout = 14 }.to raise_error(NetSuite::ConfigurationError, /doesn't support/)
+        expect { config.write_timeout(15) }.to raise_error(NetSuite::ConfigurationError, /doesn't support/)
 
-      # ensure no exception is raised
-      config.connection
+        expect(config.read_timeout).to eql(100)
+        expect(config.open_timeout).to eql(60)
+      end
     end
   end