Merge pull request #23 from zendesk/pschambacher/faraday

Remove bare Net HTTP and make compatible with Faraday clients.

.rspec

@@ -0,0 +1 @@
+--color

Gemfile

@@ -1,3 +1,5 @@
 source 'https://rubygems.org'
 
 gemspec
+
+gem 'byebug', platform: :ruby

Gemfile.lock

@@ -8,10 +8,14 @@ GEM
   specs:
     addressable (2.4.0)
     bump (0.4.2)
+    byebug (8.2.5)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
     diff-lcs (1.2.5)
+    faraday (0.9.2)
+      multipart-post (>= 1.2, < 3)
     hashdiff (0.2.3)
+    multipart-post (2.0.0)
     rake (0.9.2.2)
     rspec (3.4.0)
       rspec-core (~> 3.4.0)
@@ -37,6 +41,8 @@ PLATFORMS
 
 DEPENDENCIES
   bump
+  byebug
+  faraday
   rake
   rspec
   ticket_sharing!

lib/ticket_sharing.rb

@@ -0,0 +1,9 @@
+module TicketSharing
+  def self.connection
+    @connection ||= Faraday.new
+  end
+
+  def self.connection=(new_connection)
+    @connection = new_connection
+  end
+end

lib/ticket_sharing/agreement.rb

@@ -23,14 +23,14 @@ def send_to(url)
       client = Client.new(url)
       @response = client.post(relative_url, self.to_json)
 
-      @response.code.to_i
+      @response.status
     end
 
     def update_partner(url)
       client = Client.new(url, authentication_token)
       @response = client.put(relative_url, self.to_json)
 
-      @response.code.to_i
+      @response.status
     end
 
     def relative_url

lib/ticket_sharing/client.rb

@@ -1,3 +1,4 @@
+require 'ticket_sharing'
 require 'ticket_sharing/error'
 require 'ticket_sharing/request'
 
@@ -6,6 +7,8 @@ class Client
     def initialize(base_url, credentials=nil)
       @base_url    = base_url
       @credentials = credentials
+
+      @requester = TicketSharing::Request.new(TicketSharing.connection)
     end
 
     def post(path, body, options={})
@@ -29,19 +32,19 @@ def success?
     def send_request(method, path, body, options)
       headers = {'X-Ticket-Sharing-Token' => @credentials} if @credentials
       options = options.merge(:body => body, :headers => headers)
-      response = TicketSharing::Request.new.request(method, @base_url + path, options)
+      response = @requester.request(method, @base_url + path, options)
 
       handle_response(response)
     end
 
     def handle_response(response)
-      @success = case response.code.to_i
+      @success = case response.status
       when (200..299)
         true
       when 401, 403, 404, 405, 408, 410, 422, 500..599
         false
       else
-        raise TicketSharing::Error.new(%Q{#{response.code} "#{response.message}"\n\n#{response.body}})
+        raise TicketSharing::Error.new(%Q{#{response.status}\n\n#{response.body}})
       end
       response
     end

lib/ticket_sharing/request.rb

@@ -1,3 +1,4 @@
+require 'ticket_sharing'
 require 'ticket_sharing/error'
 
 module TicketSharing
@@ -6,67 +7,66 @@ class Request
 
     CA_PATH = "/etc/ssl/certs"
 
-    def request(method, url, options = {})
-      request_class = case method
-      when :get    then Net::HTTP::Get
-      when :post   then Net::HTTP::Post
-      when :put    then Net::HTTP::Put
-      when :delete then Net::HTTP::Delete
-      else
-        raise ArgumentError, "Unsupported method: #{method.inspect}"
-      end
+    def initialize(connection = TicketSharing.connection)
+      @connection = connection
+    end
 
-      response = send_request(request_class, url, options)
+    def request(method, url, options = {})
+      raise ArgumentError, "Unsupported method: #{method.inspect}" unless %i(get post put delete).include?(method)
 
-      follow_redirects!(request_class, response, options)
+      response = send_request(method, url, options)
+      follow_redirects!(method, response, options)
     end
 
     private
 
-    def send_request(request_class, url, options)
-      uri = URI.parse(url)
-      request = build_request(request_class, uri, options)
-      send!(request, uri, options)
-    end
+    def send_request(method, url, options)
+      response = nil
 
-    def build_request(request_class, uri, options)
-      request = request_class.new(uri.path)
-      request['Accept'] = 'application/json'
-      request['Content-Type'] = 'application/json'
-
-      (options[:headers] || {}).each do |k, v|
-        request[k] = v
+      with_ssl_connection(options) do
+        response = @connection.send(method) do |request|
+          configure_request(request, url, options)
+        end
       end
 
-      request.body = options[:body]
-
-      request
+      response
     end
 
-    def send!(request, uri, options)
-      http = Net::HTTP.new(uri.host, uri.port)
+    def with_ssl_connection(options)
+      ssl_config = {}
+      ssl_config[:ca_path] = CA_PATH if File.exist?(CA_PATH)
+      ssl_config[:verify]  = false   if options[:ssl] && options[:ssl][:verify] == false
+
+      old_configuration = @connection.instance_variable_get(:@ssl)
+      @connection.instance_variable_set(:@ssl, ssl_config) unless ssl_config.empty?
+      yield
+    ensure
+      @connection.instance_variable_set(:@ssl, old_configuration)
+    end
 
-      if uri.scheme == 'https'
-        http.use_ssl = true
-        http.ca_path = CA_PATH if File.exist?(CA_PATH)
+    def configure_request(request, url, options)
+      uri = URI.parse(url)
 
-        if options[:ssl] && options[:ssl][:verify] == false
-          http.verify_mode = OpenSSL::SSL::VERIFY_NONE
-        end
+      request.url url
+      {
+        'Accept'       => 'application/json',
+        'Content-Type' => 'application/json'
+      }.merge(options[:headers] || {}).each do |h, v|
+        request.headers[h] = v
       end
 
-      http.start { |http| http.request(request) }
+      request.body = options[:body]
     end
 
-    def follow_redirects!(request_class, response, options)
+    def follow_redirects!(method, response, options)
       redirects = 0
-      while (300..399).include?(response.code.to_i)
+      while (300..399).include?(response.status)
         if redirects >= MAX_REDIRECTS
           raise TicketSharing::TooManyRedirects
         else
           redirects += 1
         end
-        response = send_request(request_class, response['Location'], options)
+        response = send_request(method, response['Location'], options)
       end
       response
     end

lib/ticket_sharing/ticket.rb

@@ -48,21 +48,21 @@ def send_to(url)
       client = Client.new(url, agreement.authentication_token)
       @response = client.post(relative_url, self.to_json)
 
-      @response.code.to_i
+      @response.status
     end
 
     def update_partner(url)
       client = Client.new(url, agreement.authentication_token)
       @response = client.put(relative_url, self.to_json)
 
-      @response.code.to_i
+      @response.status
     end
 
     def unshare(base_url)
       client = Client.new(base_url, agreement.authentication_token)
       @response = client.delete(relative_url)
 
-      @response.code.to_i
+      @response.status
     end
 
     def relative_url

spec/models/client_spec.rb

@@ -20,7 +20,7 @@ def do_request(method, options={})
     client, response = do_request(:post)
     expect(client).to be_success
     expect(response).to_not be_nil
-    expect(response.code).to eq('200')
+    expect(response.status).to eq(200)
 
     expect(expected_request).to have_been_requested
   end
@@ -41,7 +41,7 @@ def do_request(method, options={})
 
     client, response = do_request(:post)
     expect(client).to be_success
-    expect(response.code).to eq('201')
+    expect(response.status).to eq(201)
 
     expect(expected_request).to have_been_requested
   end
@@ -55,7 +55,7 @@ def do_request(method, options={})
       .and_return(body: 'the final url', status: 201)
 
     response = client.post('/', '')
-    expect(response.code).to eq('201')
+    expect(response.status).to eq(201)
     expect(response.body).to eq('the final url')
 
     expect(client).to be_success
@@ -75,11 +75,11 @@ def do_request(method, options={})
   it 'handles a failing post with 400 response' do
     the_body = "{'error': 'the error'}"
     stub_request(:post, @base_url + @path)
-      .and_return(body: the_body, status: [400, 'Bad Request'])
+      .and_return(body: the_body, status: 400)
 
     expect {
       client, response = do_request(:post)
-    }.to raise_error(TicketSharing::Error, %Q{400 "Bad Request"\n\n} + the_body)
+    }.to raise_error(TicketSharing::Error, %Q{400\n\n} + the_body)
   end
 
   it 'handles a failing post with 403 response' do
@@ -89,7 +89,7 @@ def do_request(method, options={})
 
     client, response = do_request(:post)
     expect(client).to_not be_success
-    expect(response.code).to eq('403')
+    expect(response.status).to eq(403)
   end
 
   it 'handles a failing post with a 405 response' do
@@ -117,7 +117,7 @@ def do_request(method, options={})
 
     client, response = do_request(:post)
     expect(client).to_not be_success
-    expect(response.code).to eq('410')
+    expect(response.status).to eq(410)
   end
 
   it 'handles a failing post with 422 response' do
@@ -127,7 +127,7 @@ def do_request(method, options={})
 
     client, response = do_request(:post)
     expect(client).to_not be_success
-    expect(response.code).to eq('422')
+    expect(response.status).to eq(422)
   end
 
   it 'handles a failing post with a 5xx response' do

spec/models/request_spec.rb

@@ -37,7 +37,7 @@
     expected_request2 = stub_request(:post, 'http://example.com/sharing/1')
 
     response = described_class.new.request(:post, 'http://example.com/sharing', body: 'body')
-    expect(response.code).to eq('200')
+    expect(response.status).to eq(200)
 
     expect(expected_request1).to have_been_requested
     expect(expected_request2).to have_been_requested
@@ -50,7 +50,7 @@
       .with(headers: { 'X-Foo' => '1' })
 
     response = described_class.new.request(:post, 'http://example.com/sharing', headers: {'X-Foo' => '1'})
-    expect(response.code).to eq('200') # got redirected ?
+    expect(response.status).to eq(200) # got redirected ?
 
     expect(expected_request1).to have_been_requested
     expect(expected_request2).to have_been_requested
@@ -65,9 +65,9 @@
     expect(expected_request).to have_been_requested
   end
 
-  it 'does not set special verify_mode without option' do
+  it 'set verify_mode to VERIFY_PEER without option' do
     expected_request = stub_request(:post, 'https://example.com/sharing')
-    expect_any_instance_of(Net::HTTP).to receive(:verify_mode=).never
+    expect_any_instance_of(Net::HTTP).to receive(:verify_mode=).with(OpenSSL::SSL::VERIFY_PEER)
 
     described_class.new.request(:post, 'https://example.com/sharing')
 

spec/spec_helper.rb

@@ -1 +1,2 @@
 require 'webmock/rspec'
+require 'faraday'

ticket_sharing.gemspec

@@ -15,5 +15,6 @@ Gem::Specification.new 'ticket_sharing', '1.2.0' do |s|
   s.add_development_dependency 'bump'
   s.add_development_dependency 'webmock'
   s.add_development_dependency 'rspec'
-
+  s.add_development_dependency 'faraday'
+
 end