Refactor Request class

Instead of monkey patching rest-client, we can use it's public methods and implement our own Get using net/http
realestate-com-au · Jan 19, 2016 · 308633a · 308633a
1 parent 08e2de5
commit 308633a
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 182 deletions.
diff --git a/Gemfile b/Gemfile
@@ -1,6 +1,6 @@
 source 'https://rubygems.org'
 
-gem 'rest-client', '~> 1.7.2'
+gem 'rest-client'
 gem 'rspec'
 gem 'rake'
 gem 'byebug'

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -9,10 +9,14 @@ GEM
     crack (0.4.2)
       safe_yaml (~> 1.0.0)
     diff-lcs (1.2.5)
+    domain_name (0.5.25)
+      unf (>= 0.0.5, < 1.0.0)
+    http-cookie (1.0.2)
+      domain_name (~> 0.5)
     json (1.8.1)
     method_source (0.8.2)
-    mime-types (2.4.3)
-    netrc (0.10.3)
+    mime-types (2.99)
+    netrc (0.11.0)
     pry (0.10.1)
       coderay (~> 1.1.0)
       method_source (~> 0.8.1)
@@ -21,7 +25,8 @@ GEM
       byebug (~> 4.0)
       pry (~> 0.10)
     rake (10.4.2)
-    rest-client (1.7.3)
+    rest-client (1.8.0)
+      http-cookie (>= 1.0.2, < 2.0)
       mime-types (>= 1.16, < 3.0)
       netrc (~> 0.7)
     rspec (3.2.0)
@@ -39,6 +44,9 @@ GEM
     rspec-support (3.2.2)
     safe_yaml (1.0.4)
     slop (3.6.0)
+    unf (0.1.4)
+      unf_ext
+    unf_ext (0.0.7.1)
     webmock (1.21.0)
       addressable (>= 2.3.6)
       crack (>= 0.3.2)
@@ -52,6 +60,9 @@ DEPENDENCIES
   pry
   pry-byebug
   rake
-  rest-client (~> 1.7.2)
+  rest-client
   rspec
   webmock
+
+BUNDLED WITH
+   1.11.2
diff --git a/lib/akamai_rspec/request.rb b/lib/akamai_rspec/request.rb
@@ -1,138 +1,68 @@
 require 'rest-client'
-require 'akamai_rspec'
+require 'delegate'
 
-module RestClient
+module AkamaiRSpec
   class Request
-    @@akamai_network = 'prod'
-    @@akamai_stg_domain = 'overwrite me'
-    @@akamai_prod_domain = 'overwrite me'
-
-    def self.domain
-      env = @@akamai_network
-      case env.downcase
-      when 'staging'
-        @@akamai_stg_domain
-      else
-        @@akamai_prod_domain
-      end
-    end
-
-    def self.akamai_network(env)
-      @@akamai_network = env
-    end
+    extend Forwardable
 
-    def self.stg_domain(domain)
+    def self.stg_domain=(domain)
       @@akamai_stg_domain = domain
     end
 
-    def self.prod_domain(domain)
+    def self.prod_domain=(domain)
       @@akamai_prod_domain = domain
     end
 
-    def self.http_url(url)
-      url = "/#{url}" unless url.start_with?('/')
-      "http://#{domain}#{url}"
-    end
-
-    def self.https_url(url)
-      url = "/#{url}" unless url.start_with?('/')
-      "https://#{domain}#{url}"
-    end
-
-    def self.options
-      AkamaiHeaders.akamai_debug_headers
-    end
-
-    def self.http_get(url, options, cookies = {})
-      get_with_debug_headers(http_url(url), options, cookies)
-    end
-
-    def self.https_get(url, options, cookies = {})
-      get_with_debug_headers(https_url(url), options, cookies)
-    end
-
-    def self.get_with_debug_headers(url, options, cookies = {})
-      headers = options.merge(options).merge(cookies)
-      do_get_no_verify(url, headers) { |response, _, _| response }
-    end
-
-    def self.do_get_no_verify(url, additional_headers = {}, &block)
-      headers = (options[:headers] || {}).merge(additional_headers)
-      RestClient::Request.execute(options.merge(
-        method: :get,
-        url: url,
-        verify_ssl: false,
-        headers: headers), &(block || @block))
+    def self.network=(env)
+      @@env = env
     end
 
-    def self.responsify(maybe_a_url)
-      if maybe_a_url.is_a? RestClient::Response
-        maybe_a_url
-      else
-        begin
-          RestClient.get(maybe_a_url, options)
-        rescue RestClient::RequestFailed => exception
-          # Return the original request
-          exception.response
-        end
-      end
+    def self.get(url)
+      new.get(url)
     end
 
-    def self.request_cache_miss(url)
-      url += url.include?('?') ? '&' : '?'
-      url += SecureRandom.hex
-      RestClient.get(url, options)
+    def self.get_with_debug_headers(url)
+      new.get(url, AkamaiHeaders.akamai_debug_headers)
     end
-  end
-end
 
-module AkamaiRSpec
-  class Request
-    def self.stg_domain=(domain)
-      @@akamai_stg_domain = domain
-    end
+    def initialize
+      @@env ||= 'prod'
 
-    def self.prod_domain=(domain)
-      @@akamai_prod_domain = domain
-    end
-
-    def initialize(env)
-      @env = env
-      @domain = case env.downcase
+      @domain = case @@env.downcase
                 when 'staging'
                   @@akamai_stg_domain
                 else
                   @@akamai_prod_domain
                 end
+
+      @rest_client = RestClient::Request.new(method: :get,
+                                              url: 'fakeurl.com',
+                                              verify_ssl: false)
     end
 
-    def get(url)
-      uri = parse_url(url)
+    delegate [:parse_url_with_auth, :stringify_headers] => :@rest_client
+
+    def get(url, headers = {})
+      uri = parse_url_with_auth(url)
 
-      req = build_request(uri, headers)
+      req = build_request(uri, stringify_headers(headers))
 
       req['Host'] = uri.hostname
       uri.hostname = @domain
 
-      Net::HTTP.start(uri.hostname, uri.port) do |http|
+      @response = Net::HTTP.start(uri.hostname, uri.port) do |http|
         http.request(req, nil) { |http_response| http_response }
       end
-    end
-
-    def parse_url(url)
-      uri = URI.parse(url)
-      if uri.hostname.nil?
-        raise URI::InvalidURIError.new("bad URI(no host provided): #{url}")
-      end
 
-      uri
+      self
     end
 
     def headers
-      AkamaiHeaders.akamai_debug_headers.inject({}) do |result, (key, value)|
-        key = key.to_s.capitalize
-        result[key] = value.to_s
-        result
+      headers = Hash[@response.to_hash.map{ |k, v| [k.gsub(/-/,'_').downcase.to_sym, v] }]
+      headers.map do |k, v|
+        if v.is_a?(Array) && v.size == 1
+          []
+        end
       end
     end
 

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -3,7 +3,7 @@
 require 'akamai_rspec'
 
 DOMAIN = 'www.example.com.edgesuite.net'
-RestClient::Request.prod_domain(DOMAIN)
+AkamaiRSpec::Request.prod_domain = DOMAIN
 
 def stub_headers(url, headers, body = 'abc')
   stub_request(:any, DOMAIN + url).to_return(

diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb
@@ -1,99 +1,30 @@
 require 'spec_helper'
 require 'rspec/expectations'
-require 'rest-client'
-
-describe RestClient::Request do
-  let(:stg_domain) { 'www.example.com.edgesuite-staging.net' }
-  let(:prod_domain) { 'www.example.com.edgesuite.net' }
-  before do
-    RestClient::Request.stg_domain(stg_domain)
-    RestClient::Request.prod_domain(prod_domain)
-  end
-
-  describe '#domain' do
-    it 'should select staging' do
-      RestClient::Request.akamai_network('staging')
-      expect(RestClient::Request.domain).to eq(stg_domain)
-    end
-
-    it 'should default to prod' do
-      RestClient::Request.akamai_network('not staging')
-      expect(RestClient::Request.domain).to eq(prod_domain)
-    end
-  end
-
-  describe '#http_url' do
-    it 'should succeed without leading /' do
-      path = 'something'
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.http_url(path)).to eq("http://#{prod_domain}/#{path}")
-    end
-
-    it 'should succeed with a leading /' do
-      path = '/something'
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.http_url(path)).to eq("http://#{prod_domain}#{path}")
-    end
-
-    it 'should succeed with an empty path' do
-      path = ''
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.http_url(path)).to eq("http://#{prod_domain}/")
-    end
-  end
-
-  describe '#https_url' do
-    it 'should succeed without leading /' do
-      path = 'something'
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.https_url(path)).to eq("https://#{prod_domain}/#{path}")
-    end
-
-    it 'should succeed with a leading /' do
-      path = '/something'
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.https_url(path)).to eq("https://#{prod_domain}#{path}")
-    end
-
-    it 'should succeed with an empty path' do
-      path = ''
-      RestClient::Request.akamai_network('prod')
-      expect(RestClient::Request.https_url(path)).to eq("https://#{prod_domain}/")
-    end
-  end
-
-  describe '#responsify' do
-    let(:url) { 'nonexistantdomain' }
-    before do
-      stub_request(:any, url).to_return(
-        body: 'abc', status: [500, 'message'])
-    end
-    it 'should not raise an exception when a RestClient exception is raised' do
-      expect { RestClient::Request.responsify(url) }.to_not raise_error
-    end
-  end
-end
 
 describe AkamaiRSpec::Request do
   let(:stg_domain) { 'www.example.com.edgesuite-staging.net' }
   let(:prod_domain) { 'www.example.com.edgesuite.net' }
+  let(:url) { 'example.com' }
   let(:network) { 'prod' }
   before do
     AkamaiRSpec::Request.stg_domain = stg_domain
     AkamaiRSpec::Request.prod_domain = prod_domain
+    AkamaiRSpec::Request.network = network
+    stub_status(prod_domain, 200)
+    stub_status(stg_domain, 200)
   end
 
-  subject { described_class.new(network).get('http://examples.com') }
-
   describe '#get' do
+    subject { described_class.get(url) }
+
     context 'prod domain' do
       it 'queries the right domain' do
         expect(Net::HTTP).to receive(:start).with(prod_domain, anything)
         subject
       end
     end
 
-    context 'stating domain' do
+    context 'staging domain' do
       let(:network) { 'staging' }
       it 'quereis the right domain' do
         expect(Net::HTTP).to receive(:start).with(stg_domain, anything)