Merge pull request #60 from DripEmail/refactor_request_data_clumping

Refactor request data clumping

Author: wjohnstondrip

CHANGELOG.md

@@ -10,11 +10,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### Changed
 - `Drip::Client#url_prefix` parameter no longer includes `/vN` part of the URL in order to prepare for Shopper Activity API. This breaks backwards compatibility for this option, but there is no expected production usage of this parameter.
-- `Drip::Client#get`, `Drip::Client#post`, `Drip::Client#put`, and `Drip::Client#delete` methods no longer auto-prepend `/v2` if the given path starts with `v2/` or `v3/`. This behavior is deprecated and will produce a warning. If you are using one of these methods and get this warning, just add `v2/` to the beginning of the path when you call it.
+- `Drip::Client#get`, `Drip::Client#post`, `Drip::Client#put`, and `Drip::Client#delete` are deprecated. If you are using these to hit a Drip API endpoint, please file a ticket or PR to fix the use case.
 - `Drip::Client#generate_resource` is deprecated and will be removed in a future version.
 - `Drip::Client#content_type` is deprecated and will be removed in a future version. It is no longer used internally, effective immediately.
 - When using the block form of parameter initialization, a configuration object is provided instead of the client itself.
 - Calling configuration setters on `Drip::Client` is deprecated.
+- `Drip::Client::REDIRECT_LIMIT` constant is now private. This is supposed to be an implementation detail and shouldn't leak.
 
 ### Removed
 - Drop support for Ruby 2.1.

lib/drip/client.rb

@@ -1,4 +1,5 @@
 require "drip/errors"
+require "drip/request"
 require "drip/response"
 require "drip/client/accounts"
 require "drip/client/broadcasts"
@@ -9,6 +10,7 @@
 require "drip/client/custom_fields"
 require "drip/client/events"
 require "drip/client/forms"
+require "drip/client/http_client"
 require "drip/client/orders"
 require "drip/client/subscribers"
 require "drip/client/tags"
@@ -20,7 +22,7 @@
 require "json"
 
 module Drip
-  class Client # rubocop:disable Metrics/ClassLength
+  class Client
     include Accounts
     include Broadcasts
     include Campaigns
@@ -36,8 +38,6 @@ class Client # rubocop:disable Metrics/ClassLength
     include Workflows
     include WorkflowTriggers
 
-    REDIRECT_LIMIT = 10
-
     Drip::Client::Configuration::CONFIGURATION_FIELDS.each do |config_key|
       define_method(config_key) do
         @config.public_send(config_key)
@@ -68,69 +68,31 @@ def content_type
       JSON_API_CONTENT_TYPE
     end
 
-    def get(url, options = {})
-      make_request(Net::HTTP::Get, make_uri(url), options)
-    end
-
-    def post(url, options = {})
-      make_request(Net::HTTP::Post, make_uri(url), options)
+    Drip::Request::VERB_CLASS_MAPPING.keys.each do |verb|
+      define_method(verb) do |path, options = {}|
+        warn "[DEPRECATED] Drip::Client##{verb} please use the API endpoint specific methods"
+        make_json_api_request(verb, "v2/#{path}", options)
+      end
     end
 
-    def put(url, options = {})
-      make_request(Net::HTTP::Put, make_uri(url), options)
-    end
+  private
 
-    def delete(url, options = {})
-      make_request(Net::HTTP::Delete, make_uri(url), options)
+    def make_json_api_request(http_verb, path, options = {})
+      make_request Drip::Request.new(http_verb, make_uri(path), options, JSON_API_CONTENT_TYPE)
     end
 
-  private
-
     def private_generate_resource(key, *args)
       # No reason for this to be part of the public API, so making a duplicate method to make it private.
       { key => args }
     end
 
     def make_uri(path)
-      if !path.start_with?("v2/") && !path.start_with?("v3/")
-        warn "[DEPRECATED] Automatically prepended path with 'v2/'"
-        path = "v2/#{path}"
-      end
       URI(@config.url_prefix) + URI(path)
     end
 
-    def make_request(verb_klass, uri, options, step = 0)
-      raise TooManyRedirectsError, 'too many HTTP redirects' if step >= REDIRECT_LIMIT
-
+    def make_request(drip_request)
       build_response do
-        Net::HTTP.start(uri.host, uri.port, connection_options(uri.scheme)) do |http|
-          if verb_klass == Net::HTTP::Get
-            uri.query = URI.encode_www_form(options)
-          end
-
-          request = verb_klass.new uri
-
-          unless verb_klass == Net::HTTP::Get
-            request.body = options.to_json
-          end
-
-          request['User-Agent'] = "Drip Ruby v#{Drip::VERSION}"
-          request['Content-Type'] = JSON_API_CONTENT_TYPE
-          request['Accept'] = "*/*"
-
-          if @config.access_token
-            request['Authorization'] = "Bearer #{@config.access_token}"
-          else
-            request.basic_auth @config.api_key, ""
-          end
-
-          response = http.request request
-          if response.is_a?(Net::HTTPRedirection)
-            return make_request(verb_klass, URI(response["Location"]), options, step + 1)
-          else
-            response
-          end
-        end
+        Drip::Client::HTTPClient.new(@config).make_request(drip_request)
       end
     end
 
@@ -140,18 +102,5 @@ def build_response(&block)
     rescue JSON::ParserError
       Drip::Response.new(response.code.to_i, nil)
     end
-
-    def connection_options(uri_scheme)
-      options = { use_ssl: uri_scheme == "https" }
-
-      if @config.http_open_timeout
-        options[:open_timeout] = @config.http_open_timeout
-        options[:ssl_timeout] = @config.http_open_timeout
-      end
-
-      options[:read_timeout] = @config.http_timeout if @config.http_timeout
-
-      options
-    end
   end
 end

lib/drip/client/accounts.rb

@@ -6,7 +6,7 @@ module Accounts
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#accounts
       def accounts
-        get "v2/accounts"
+        make_json_api_request :get, "v2/accounts"
       end
 
       # Public: Fetch an account.
@@ -16,7 +16,7 @@ def accounts
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#accounts
       def account(id)
-        get "v2/accounts/#{id}"
+        make_json_api_request :get, "v2/accounts/#{id}"
       end
     end
   end

lib/drip/client/broadcasts.rb

@@ -12,7 +12,7 @@ module Broadcasts
       # Returns a Drip::Response
       # See https://www.getdrip.com/docs/rest-api#broadcasts
       def broadcasts(options = {})
-        get "v2/#{account_id}/broadcasts", options
+        make_json_api_request :get, "v2/#{account_id}/broadcasts", options
       end
 
       # Public: Fetch a broadcast.
@@ -22,7 +22,7 @@ def broadcasts(options = {})
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#broadcasts
       def broadcast(id)
-        get "v2/#{account_id}/broadcasts/#{id}"
+        make_json_api_request :get, "v2/#{account_id}/broadcasts/#{id}"
       end
     end
   end

lib/drip/client/campaign_subscriptions.rb

@@ -8,7 +8,7 @@ module CampaignSubscriptions
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs.rest-api#campaign_subscriptions
       def campaign_subscriptions(subscriber_id)
-        get "v2/#{account_id}/subscribers/#{subscriber_id}/campaign_subscriptions"
+        make_json_api_request :get, "v2/#{account_id}/subscribers/#{subscriber_id}/campaign_subscriptions"
       end
     end
   end

lib/drip/client/campaigns.rb

@@ -14,7 +14,7 @@ module Campaigns
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs.rest-api#campaigns
       def campaigns(options = {})
-        get "v2/#{account_id}/campaigns", options
+        make_json_api_request :get, "v2/#{account_id}/campaigns", options
       end
 
       # Public: Fetch a campaign.
@@ -24,7 +24,7 @@ def campaigns(options = {})
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#campaigns
       def campaign(id)
-        get "v2/#{account_id}/campaigns/#{id}"
+        make_json_api_request :get, "v2/#{account_id}/campaigns/#{id}"
       end
 
       # Public: Activate a campaign.
@@ -34,7 +34,7 @@ def campaign(id)
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#campaigns
       def activate_campaign(id)
-        post "v2/#{account_id}/campaigns/#{id}/activate"
+        make_json_api_request :post, "v2/#{account_id}/campaigns/#{id}/activate"
       end
 
       # Public: Pause a campaign.
@@ -44,7 +44,7 @@ def activate_campaign(id)
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#campaigns
       def pause_campaign(id)
-        post "v2/#{account_id}/campaigns/#{id}/pause"
+        make_json_api_request :post, "v2/#{account_id}/campaigns/#{id}/pause"
       end
 
       # Public: List everyone subscribed to a campaign.
@@ -68,7 +68,7 @@ def pause_campaign(id)
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#campaigns
       def campaign_subscribers(id, options = {})
-        get "v2/#{account_id}/campaigns/#{id}/subscribers", options
+        make_json_api_request :get, "v2/#{account_id}/campaigns/#{id}/subscribers", options
       end
     end
   end

lib/drip/client/conversions.rb

@@ -10,7 +10,7 @@ module Conversions
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#conversions
       def conversions(options = {})
-        get "v2/#{account_id}/goals", options
+        make_json_api_request :get, "v2/#{account_id}/goals", options
       end
 
       # Public: Fetch a conversion.
@@ -20,7 +20,7 @@ def conversions(options = {})
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#conversions
       def conversion(id)
-        get "v2/#{account_id}/goals/#{id}"
+        make_json_api_request :get, "v2/#{account_id}/goals/#{id}"
       end
     end
   end

lib/drip/client/custom_fields.rb

@@ -6,7 +6,7 @@ module CustomFields
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#custom_fields
       def custom_fields
-        get "v2/#{account_id}/custom_field_identifiers"
+        make_json_api_request :get, "v2/#{account_id}/custom_field_identifiers"
       end
     end
   end

lib/drip/client/events.rb

@@ -14,7 +14,7 @@ module Events
       # See https://www.getdrip.com/docs/rest-api#record_event
       def track_event(email, action, properties = {}, options = {})
         data = options.merge({ "email" => email, "action" => action, "properties" => properties })
-        post "v2/#{account_id}/events", private_generate_resource("events", data)
+        make_json_api_request :post, "v2/#{account_id}/events", private_generate_resource("events", data)
       end
 
       # Public: Track a collection of events all at once.
@@ -28,7 +28,7 @@ def track_event(email, action, properties = {}, options = {})
       # See https://www.getdrip.com/docs/rest-api#event_batches
       def track_events(events)
         url = "v2/#{account_id}/events/batches"
-        post url, private_generate_resource("batches", { "events" => events })
+        make_json_api_request :post, url, private_generate_resource("batches", { "events" => events })
       end
 
       # Public: Fetch all custom event actions.
@@ -41,7 +41,7 @@ def track_events(events)
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#events
       def event_actions(options = {})
-        get "v2/#{account_id}/event_actions", options
+        make_json_api_request :get, "v2/#{account_id}/event_actions", options
       end
     end
   end

lib/drip/client/forms.rb

@@ -6,7 +6,7 @@ module Forms
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#forms
       def forms
-        get "v2/#{account_id}/forms"
+        make_json_api_request :get, "v2/#{account_id}/forms"
       end
 
       # Public: Fetch a form.
@@ -16,7 +16,7 @@ def forms
       # Returns a Drip::Response.
       # See https://www.getdrip.com/docs/rest-api#forms
       def form(id)
-        get "v2/#{account_id}/forms/#{id}"
+        make_json_api_request :get, "v2/#{account_id}/forms/#{id}"
       end
     end
   end