config cleanup with additional test coverage

History.md

@@ -1,5 +1,10 @@
 ## 1.0.0 (TBD)
 
+* [#168] `Hubspot.configure` will raise when given none or multiple ways to
+  authenticate with the HubSpot API.
+
+[#168]: https://github.com/adimichele/hubspot-ruby/pull/168
+
 * Updates the endpoints referenced in `CompanyProperties` to match the new
   HubSpot [CompanyProperty endpoints].
 

lib/hubspot/config.rb

@@ -3,30 +3,31 @@
 
 module Hubspot
   class Config
-
     CONFIG_KEYS = [
       :hapikey, :base_url, :portal_id, :logger, :access_token, :client_id,
       :client_secret, :redirect_uri
     ]
     DEFAULT_LOGGER = Logger.new(nil)
+    DEFAULT_BASE_URL = "https://api.hubapi.com".freeze
 
     class << self
       attr_accessor *CONFIG_KEYS
 
       def configure(config)
         config.stringify_keys!
         @hapikey = config["hapikey"]
-        @base_url = config["base_url"] || "https://api.hubapi.com"
+        @base_url = config["base_url"] || DEFAULT_BASE_URL
         @portal_id = config["portal_id"]
         @logger = config["logger"] || DEFAULT_LOGGER
         @access_token = config["access_token"]
         @client_id = config["client_id"] if config["client_id"].present?
         @client_secret = config["client_secret"] if config["client_secret"].present?
         @redirect_uri = config["redirect_uri"] if config["redirect_uri"].present?
 
-        unless access_token.present? ^ hapikey.present?
-          Hubspot::ConfigurationError.new("You must provide either an access_token or an hapikey")
+        unless authentication_uncertain?
+          raise Hubspot::ConfigurationError.new("You must provide either an access_token or an hapikey")
         end
+
         if access_token.present?
           Hubspot::Connection.headers("Authorization" => "Bearer #{access_token}")
         end
@@ -35,7 +36,7 @@ def configure(config)
 
       def reset!
         @hapikey = nil
-        @base_url = "https://api.hubapi.com"
+        @base_url = DEFAULT_BASE_URL
         @portal_id = nil
         @logger = DEFAULT_LOGGER
         @access_token = nil
@@ -47,6 +48,12 @@ def ensure!(*params)
           raise Hubspot::ConfigurationError.new("'#{p}' not configured") unless instance_variable_get "@#{p}"
         end
       end
+
+      private
+
+      def authentication_uncertain?
+        access_token.present? ^ hapikey.present?
+      end
     end
 
     reset!

lib/hubspot/utils.rb

@@ -107,12 +107,8 @@ def compare_property_lists(klass, source, target)
       end
 
       def with_hapikey(hapikey)
-        begin
-          Hubspot.configure(hapikey: hapikey) unless hapikey.blank?
-          yield if block_given?
-        ensure
-          Hubspot.configure(hapikey: ENV['HUBSPOT_API_KEY']) unless hapikey.blank?
-        end
+        Hubspot.configure(hapikey: hapikey)
+        yield if block_given?
       end
 
       private

spec/lib/hubspot/config_spec.rb

@@ -1,52 +1,86 @@
 describe Hubspot::Config do
-  describe "#configure" do
-    let(:config){ {hapikey: "demo", base_url: "http://api.hubapi.com/v2", portal_id: "62515"} }
-    subject{ Hubspot::Config.configure(config) }
+  describe ".configure" do
+    it "sets the hapikey config" do
+      hapikey = "demo"
 
-    it "changes the hapikey config" do
-      expect{ subject }.to change(Hubspot::Config, :hapikey).to("demo")
+      config = Hubspot::Config.configure(hapikey: hapikey)
+
+      expect(config.hapikey).to eq(hapikey)
     end
 
     it "changes the base_url" do
-      expect{ subject }.to change(Hubspot::Config, :base_url).to("http://api.hubapi.com/v2")
+      base_url = "https://api.hubapi.com/v2"
+
+      config = Hubspot::Config.configure(
+        hapikey: "123abc",
+        base_url: base_url
+      )
+
+      expect(config.base_url).to eq(base_url)
     end
 
     it "sets a default value for base_url" do
-      Hubspot::Config.base_url.should == "https://api.hubapi.com"
+      config = Hubspot::Config.configure(hapikey: "123abc")
+
+      expect(config.base_url).to eq("https://api.hubapi.com")
     end
 
     it "sets a value for portal_id" do
-      expect{ subject }.to change(Hubspot::Config, :portal_id).to("62515")
+      portal_id = "62515"
+
+      config = Hubspot::Config.configure(
+        hapikey: "123abc",
+        portal_id: portal_id
+      )
+
+      expect(config.portal_id).to eq(portal_id)
+    end
+
+    it "raises when an authentication approach is not provided" do
+      expect {
+        Hubspot::Config.configure({})
+      }.to raise_error(Hubspot::ConfigurationError)
+    end
+
+    it "raises when two authentication approaches are provided" do
+      expect {
+        Hubspot::Config.configure({
+          hapikey: "123abc",
+          access_token: "456def",
+        })
+      }.to raise_error(Hubspot::ConfigurationError)
     end
   end
 
-  describe "#reset!" do
-    let(:config){ {hapikey: "demo", base_url: "http://api.hubapi.com/v2", portal_id: "62515"} }
-    before{ Hubspot::Config.configure(config) }
-    subject{ Hubspot::Config.reset! }
-    it "clears out the config" do
-      subject
-      Hubspot::Config.hapikey.should be_nil
-      Hubspot::Config.base_url.should == "https://api.hubapi.com"
-      Hubspot::Config.portal_id.should be_nil
+  describe ".reset!" do
+    it "resets the config values" do
+      Hubspot::Config.configure(hapikey: "123abc", portal_id: "456def")
+
+      Hubspot::Config.reset!
+
+      expect(Hubspot::Config.hapikey).to be nil
+      expect(Hubspot::Config.portal_id).to be nil
     end
   end
 
-  describe "#ensure!" do
-    subject{ Hubspot::Config.ensure!(:hapikey, :base_url, :portal_id)}
-    before{ Hubspot::Config.configure(config) }
+  describe ".ensure!" do
+    context "when a specified parameter is missing" do
+      it "raises an error" do
+        Hubspot::Config.configure(hapikey: "123abc")
 
-    context "with a missing parameter" do
-      let(:config){ {hapikey: "demo", base_url: "http://api.hubapi.com/v2"} }
-      it "should raise an error" do
-        expect { subject }.to raise_error Hubspot::ConfigurationError
+        expect {
+          Hubspot::Config.ensure!(:portal_id)
+        }.to raise_error(Hubspot::ConfigurationError)
       end
     end
 
-    context "with all requried parameters" do
-      let(:config){ {hapikey: "demo", base_url: "http://api.hubapi.com/v2", portal_id: "62515"} }
-      it "should not raise an error" do
-        expect { subject }.to_not raise_error
+    context "when all specified parameters are present" do
+      it "does not raise an error" do
+        Hubspot::Config.configure(hapikey: "123abc", portal_id: "456def")
+
+        expect {
+          Hubspot::Config.ensure!(:portal_id)
+        }.not_to raise_error
       end
     end
   end