Add support to follow 302 redirects in do_http (#416)

* Follow 302 redirect responses * Don't redirect for upload 302 responses
ruckus · Feb 19, 2018 · 50e9dec · 50e9dec
1 parent 9f04296
commit 50e9dec
Show file tree

Hide file tree

Showing 21 changed files with 140 additions and 26 deletions.
diff --git a/lib/quickbooks/service/base_service.rb b/lib/quickbooks/service/base_service.rb
@@ -239,7 +239,12 @@ def do_http(method, url, body, headers) # throws IntuitRequestException
           else
             raise "Do not know how to perform that HTTP operation"
           end
-        check_response(response, :request => body)
+
+        if response.code.to_i == 302 && [:get, :post].include?(method)
+          do_http(method, response['location'], body, headers)
+        else
+          check_response(response, :request => body)
+        end
       end
 
       def add_query_string_to_url(url, params)

diff --git a/quickbooks-ruby.gemspec b/quickbooks-ruby.gemspec
@@ -23,6 +23,6 @@ Gem::Specification.new do |gem|
   gem.add_development_dependency 'rake', '10.1.0'
   gem.add_development_dependency 'simplecov', '0.7.1'
   gem.add_development_dependency 'rr',     '~> 1.0.2'
-  gem.add_development_dependency 'rspec',  '2.13.0'
+  gem.add_development_dependency 'rspec',  '2.14.1'
   gem.add_development_dependency 'fakeweb', '1.3.0'
 end
diff --git a/spec/lib/quickbooks/service/access_token_spec.rb b/spec/lib/quickbooks/service/access_token_spec.rb
@@ -5,7 +5,7 @@
 
   it "can renew a token" do
     xml = fixture("access_token_200.xml")
-    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.renew
     response.error?.should == false
@@ -14,7 +14,7 @@
   it "fails to renew if the token has expired" do
     xml = fixture("access_token_270.xml")
 
-    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.renew
     response.error?.should == true
@@ -25,7 +25,7 @@
   it "fails to renew if the request is out-of-bounds" do
     xml = fixture("access_token_212.xml")
 
-    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.renew
     response.error?.should == true
@@ -36,7 +36,7 @@
   it "fails to renew if the app is not approved" do
     xml = fixture("access_token_24.xml")
 
-    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::RENEW_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.renew
     response.error?.should == true
@@ -46,15 +46,15 @@
 
   it "can successfully disconnect" do
     xml = fixture("disconnect_200.xml")
-    stub_request(:get, Quickbooks::Service::AccessToken::DISCONNECT_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::DISCONNECT_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.disconnect
     response.error?.should == false
   end
 
   it "can fail to disconnect if the auth token is invalid" do
     xml = fixture("disconnect_270.xml")
-    stub_request(:get, Quickbooks::Service::AccessToken::DISCONNECT_URL, ["200", "OK"], xml, true)
+    stub_request(:get, Quickbooks::Service::AccessToken::DISCONNECT_URL, ["200", "OK"], xml, {}, true)
 
     response = @service.disconnect
     response.error?.should == true

diff --git a/spec/lib/quickbooks/service/account_spec.rb b/spec/lib/quickbooks/service/account_spec.rb
@@ -5,7 +5,7 @@
 
   it "can query for accounts" do
     xml = fixture("accounts.xml")
-    stub_request(:get, @service.url_for_query, ["200", "OK"], xml, true)
+    stub_request(:get, @service.url_for_query, ["200", "OK"], xml, {}, true)
 
     accounts = @service.query
     accounts.entries.count.should == 10

diff --git a/spec/lib/quickbooks/service/attachable_spec.rb b/spec/lib/quickbooks/service/attachable_spec.rb
@@ -5,7 +5,7 @@
 
   it "can query for attachables" do
     xml = fixture("attachables.xml")
-    stub_request(:get, @service.url_for_query, ["200", "OK"], xml, true)
+    stub_request(:get, @service.url_for_query, ["200", "OK"], xml, {}, true)
 
     attachables = @service.query
     attachables.entries.count.should == 1
@@ -16,7 +16,7 @@
 
   it "can create an attachble" do
     xml = fixture("attachable_create_response.xml")
-    stub_request(:post, @service.url_for_resource('attachable'), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource('attachable'), ["200", "OK"], xml, {}, true)
 
     attachable = Quickbooks::Model::Attachable.new
     attachable.file_name = "monkey.jpg"

diff --git a/spec/lib/quickbooks/service/base_service_spec.rb b/spec/lib/quickbooks/service/base_service_spec.rb
@@ -47,6 +47,111 @@
     end
   end
 
+  describe 'do_http' do
+    let(:base_url) { 'http://example.com/'}
+
+    [:get, :post, :upload].each do |request_method|
+      context 'when no access_token exists' do
+        before do
+          construct_service :base_service, nil
+        end
+
+        it "should raise RunTimeError" do
+          expect { @service.send(:do_http, request_method, base_url, nil, {}) }.to raise_error
+        end
+      end
+    end
+
+    context 'when an access token exists' do
+      [:get, :post].each do |request_method|
+        context "when the method is #{request_method}" do
+          before do
+            construct_service :base_service
+            @service.stub(:do_http).and_call_original
+          end
+
+          context 'when a non-302 response is received' do
+            before do
+              stub_request(request_method, base_url, ["200", "OK"], fixture("items.xml"))
+            end
+
+            it "calls do_http only once" do
+              @service.send(:do_http, request_method, base_url, nil, {})
+              @service.should have_received(:do_http).once
+            end
+          end
+
+          context 'when a 302 response is received' do
+            let(:headers) do
+              {
+                "Content-Type" => "application/xml",
+                "Accept" => "application/xml",
+                "Accept-Encoding" => "gzip, deflate"
+              }
+            end
+            let(:redirect_location) { "#{base_url}elsewhere" }
+
+            before do
+              stub_request(request_method, base_url, ["302", "Found"], fixture("items.xml"), :location => redirect_location)
+              stub_request(request_method, redirect_location, ["200", "OK"], fixture("items.xml"))
+            end
+
+            it "calls do_http twice" do
+              @service.send(:do_http, request_method, base_url, nil, headers)
+              @service.should have_received(:do_http).with(request_method, base_url, nil, headers).once
+              @service.should have_received(:do_http).with(request_method, redirect_location, nil, headers).once
+            end
+          end
+        end
+      end
+
+      context 'when the method is upload' do
+        before do
+          construct_service :base_service
+          @service.stub(:do_http).and_call_original
+        end
+
+        context 'when a non-302 response is received' do
+          before do
+            stub_request(:post, base_url, ["200", "OK"], fixture("items.xml"))
+          end
+
+          it "calls do_http only once" do
+            @service.send(:do_http, :upload, base_url, nil, {})
+            @service.should have_received(:do_http).once
+          end
+        end
+
+        context 'when a 302 response is received' do
+          let(:headers) do
+            {
+              "Content-Type" => "application/xml",
+              "Accept" => "application/xml",
+              "Accept-Encoding" => "gzip, deflate"
+            }
+          end
+          let(:redirect_location) { "#{base_url}elsewhere" }
+
+          before do
+            stub_request(:post, base_url, ["302", "Found"], fixture("items.xml"), :location => redirect_location)
+          end
+
+          it "calls do_http only once" do
+            begin
+              @service.send(:do_http, :upload, base_url, nil, {})
+            rescue
+              @service.should have_received(:do_http).once
+            end
+          end
+
+          it 'raises an error' do
+            expect { @service.send(:do_http, :upload, base_url, nil, headers) }.to raise_error(RuntimeError)
+          end
+        end
+      end
+    end
+  end
+
   describe 'check_response' do
     before do
       construct_service :base_service

diff --git a/spec/lib/quickbooks/service/bill_payment_spec.rb b/spec/lib/quickbooks/service/bill_payment_spec.rb
@@ -52,6 +52,7 @@
                  @service.url_for_resource(resource),
                  ["200", "OK"],
                  fixture("fetch_bill_payment_by_id.xml"),
+                 {},
                  true)
 
     update_response = @service.update(bill_payment, :sparse => true)

diff --git a/spec/lib/quickbooks/service/class_spec.rb b/spec/lib/quickbooks/service/class_spec.rb
@@ -59,7 +59,7 @@
     classs.sync_token = 1
     classs.id = 2
     xml = fixture("deleted_class.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
     classs.valid_for_deletion?.should == true
     response = @service.delete(classs)
     response.name.should == "#{classs.name} (Deleted)"

diff --git a/spec/lib/quickbooks/service/customer_spec.rb b/spec/lib/quickbooks/service/customer_spec.rb
@@ -79,7 +79,7 @@
     customer.id = 1
 
     xml = fixture("fetch_customer_by_id.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
 
     customer.valid_for_update?.should == true
     update_response = @service.update(customer, :sparse => true)
@@ -94,7 +94,7 @@
     customer.id = 1
 
     xml = fixture("deleted_customer.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
 
     customer.valid_for_deletion?.should == true
     response = @service.delete(customer)

diff --git a/spec/lib/quickbooks/service/department_spec.rb b/spec/lib/quickbooks/service/department_spec.rb
@@ -56,7 +56,7 @@
     department.sync_token = 1
     department.id = 2
     xml = fixture("deleted_department.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
     department.valid_for_deletion?.should == true
     response = @service.delete(department)
     response.name.should == "#{department.name} (Deleted)"

diff --git a/spec/lib/quickbooks/service/deposit_spec.rb b/spec/lib/quickbooks/service/deposit_spec.rb
@@ -48,6 +48,7 @@
                  @service.url_for_resource(resource),
                  ["200", "OK"],
                  fixture("fetch_deposit_by_id.xml"),
+                 {},
                  true)
 
     deposit.line_items << Quickbooks::Model::DepositLineItem.new

diff --git a/spec/lib/quickbooks/service/employee_spec.rb b/spec/lib/quickbooks/service/employee_spec.rb
@@ -77,7 +77,7 @@
     employee.sync_token = 1
     employee.id = 2
     xml = fixture("deleted_employee.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
     employee.valid_for_deletion?.should == true
     response = @service.delete(employee)
     response.display_name.should == "#{employee.display_name} (Deleted)"

diff --git a/spec/lib/quickbooks/service/item_spec.rb b/spec/lib/quickbooks/service/item_spec.rb
@@ -59,7 +59,7 @@
     item.description = nil
 
     xml = fixture("fetch_item_by_id.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
 
     update_response = @service.update(item, :sparse => true)
     update_response.name.should == 'Plush Baby Doll'
@@ -74,7 +74,7 @@
     item.id = 1
 
     xml = fixture("item_delete_success_response.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
 
     response = @service.delete(item)
     response.active?.should be_nil

diff --git a/spec/lib/quickbooks/service/payment_method_spec.rb b/spec/lib/quickbooks/service/payment_method_spec.rb
@@ -66,7 +66,7 @@ module Service
         payment_method.sync_token = 0
         payment_method.id = 7
         xml = fixture("deleted_payment_method.xml")
-        stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+        stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
         payment_method.valid_for_deletion?.should == true
         response = @service.delete(payment_method)
         response.name.should == "#{payment_method.name} (Deleted)"

diff --git a/spec/lib/quickbooks/service/payment_spec.rb b/spec/lib/quickbooks/service/payment_spec.rb
@@ -47,6 +47,7 @@
                  @service.url_for_resource(resource),
                  ["200", "OK"],
                  fixture("fetch_payment_by_id.xml"),
+                 {},
                  true)
 
     update_response = @service.update(payment, :sparse => true)

diff --git a/spec/lib/quickbooks/service/purchase_order_spec.rb b/spec/lib/quickbooks/service/purchase_order_spec.rb
@@ -8,7 +8,7 @@
     model = Quickbooks::Model::PurchaseOrder
     purchase_order = model.new
     xml = fixture("deleted_purchase_order.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, false)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, false)
     expect(@service.delete(purchase_order)).to eq true
 
   end

diff --git a/spec/lib/quickbooks/service/term_spec.rb b/spec/lib/quickbooks/service/term_spec.rb
@@ -42,6 +42,7 @@
                  @service.url_for_resource(resource),
                  ["200", "OK"],
                  fixture("fetch_term_by_id.xml"),
+                 {},
                  true)
 
     updated_term = @service.update(term, :sparse => true)

diff --git a/spec/lib/quickbooks/service/upload_spec.rb b/spec/lib/quickbooks/service/upload_spec.rb
@@ -5,7 +5,7 @@
 
   it "can create an upload" do
     xml = fixture("upload_create_response.xml")
-    stub_request(:post, @service.url_for_resource('upload'), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource('upload'), ["200", "OK"], xml, {}, true)
 
     attachable = Quickbooks::Model::Attachable.new
     attachable.file_name = "monkey.jpg"

diff --git a/spec/lib/quickbooks/service/vendor_spec.rb b/spec/lib/quickbooks/service/vendor_spec.rb
@@ -78,7 +78,7 @@
     vendor.sync_token = 4
     vendor.id = 1129
     xml = fixture("deleted_vendor.xml")
-    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, true)
+    stub_request(:post, @service.url_for_resource(model::REST_RESOURCE), ["200", "OK"], xml, {}, true)
     vendor.valid_for_deletion?.should == true
     response = @service.delete(vendor)
     response.display_name.should == "#{vendor.display_name} (Deleted)"

diff --git a/spec/support/net_helpers.rb b/spec/support/net_helpers.rb
@@ -3,7 +3,7 @@ module NetHelpers
   # +strict+ indicates whether we want to use a regex for a matching URL, which is needed
   # for URLs that use query params. If you don't need use query params
   # than its suggested to use strict = true
-  def stub_request(method, url, status = ["200", "OK"], body = nil, strict = true)
+  def stub_request(method, url, status = ["200", "OK"], body = nil, headers = {}, strict = true)
     if !strict
       if url.is_a?(String)
         url = url.gsub('?', '\?')
@@ -12,7 +12,7 @@ def stub_request(method, url, status = ["200", "OK"], body = nil, strict = true)
     else
       uri = url
     end
-    FakeWeb.register_uri(method, uri, :status => status, :body => body)
+    FakeWeb.register_uri(method, uri, { :status => status, :body => body }.merge(headers))
   end
 end
 

diff --git a/spec/support/oauth_helpers.rb b/spec/support/oauth_helpers.rb
@@ -12,9 +12,9 @@ def construct_oauth
     OAuth::AccessToken.new(oauth_consumer, "token", "secret")
   end
 
-  def construct_service(model)
+  def construct_service(model, access_token=construct_oauth)
     @service = "Quickbooks::Service::#{model.to_s.camelcase}".constantize.new
-    @service.access_token = construct_oauth
+    @service.access_token = access_token
     @service.company_id = "9991111222"
     @service
   end