Merge pull request #615 from Shopify/nullversionbug

ApiVersion::NullVersion tweaks

lib/shopify_api/api_version.rb

@@ -25,6 +25,7 @@ module ShopifyAPI
       end
 
       def find_version(version_or_handle)
+        raise ArgumentError, "NullVersion is not a valid version or version handle." if version_or_handle == NullVersion
         return version_or_handle if version_or_handle.is_a?(ApiVersion)
         handle = version_or_handle.to_s
         @versions ||= {}
@@ -174,6 +175,14 @@ module ShopifyAPI
 
     class NullVersion
       class << self
+        def new(*_args)
+          raise NoMethodError, "NullVersion is an abstract class and cannot be instantiated."
+        end
+
+        def matches?(version)
+          version.nil? || version == self
+        end
+
         def raise_not_set_error(*_args)
           raise ApiVersionNotSetError, "You must set ShopifyAPI::Base.api_version before making a request."
         end

lib/shopify_api/resources/base.rb

@@ -68,7 +68,7 @@ module ShopifyAPI
       end
 
       def api_version=(version)
-        self._api_version = version.nil? ? ApiVersion::NullVersion : ApiVersion.find_version(version)
+        self._api_version = ApiVersion::NullVersion.matches?(version) ? ApiVersion::NullVersion : ApiVersion.find_version(version)
       end
 
       def prefix(options = {})

lib/shopify_api/session.rb

@@ -127,11 +127,11 @@ module ShopifyAPI
     end
 
     def api_version=(version)
-      @api_version = version.nil? ? nil : ApiVersion.find_version(version)
+      @api_version = ApiVersion::NullVersion.matches?(version) ? ApiVersion::NullVersion : ApiVersion.find_version(version)
     end
 
     def valid?
-      domain.present? && token.present? && api_version.present?
+      domain.present? && token.present? && api_version.is_a?(ApiVersion)
     end
 
     def expires_in

test/api_version_test.rb

@@ -50,6 +50,15 @@ class ApiVersionTest < Test::Unit::TestCase
     end
   end
 
+  test "find_version raises ArgumentError when given an ShopifyAPI::ApiVersion::NullVersion object" do
+    ShopifyAPI::ApiVersion.clear_known_versions
+    ShopifyAPI::ApiVersion.version_lookup_mode = :define_on_unknown
+    assert_equal :define_on_unknown, ShopifyAPI::ApiVersion.version_lookup_mode
+    assert_raises ArgumentError do
+      ShopifyAPI::ApiVersion.find_version(ShopifyAPI::ApiVersion::NullVersion)
+    end
+  end
+
   test 'two versions with the same version number are equal' do
     version_1 = ShopifyAPI::ApiVersion.new(handle: '2018-09')
     version_2 = ShopifyAPI::ApiVersion.new(handle: '2018-09')
@@ -125,6 +134,12 @@ class ApiVersionTest < Test::Unit::TestCase
     end
   end
 
+  test "NullVersion cannot be instantiated and raises NoMethodError if attempted" do
+    assert_raises(NoMethodError) do
+      ShopifyAPI::ApiVersion::NullVersion.new
+    end
+  end
+
   test "handle_to_date converts a version handle to a date" do
     version_1 = ShopifyAPI::ApiVersion.new(handle: '2019-01')
     version_2 = ShopifyAPI::ApiVersion.new(handle: 'unstable')

test/base_test.rb

@@ -168,6 +168,11 @@ class BaseTest < Test::Unit::TestCase
     assert_equal ShopifyAPI::ApiVersion::NullVersion, ShopifyAPI::Base.api_version
   end
 
+  test "#api_version= ShopifyAPI::ApiVersion::NullVersion should set ApiVersion to ShopifyAPI::ApiVersion::NullVersion" do
+    ShopifyAPI::Base.api_version = ShopifyAPI::ApiVersion::NullVersion
+    assert_equal ShopifyAPI::ApiVersion::NullVersion, ShopifyAPI::Base.api_version
+  end
+
   def clear_header(header)
     [ActiveResource::Base, ShopifyAPI::Base, ShopifyAPI::Product].each do |klass|
       klass.headers.delete(header)

test/session_test.rb

@@ -21,6 +21,9 @@ class SessionTest < Test::Unit::TestCase
   test "not be valid without an api version" do
     session = ShopifyAPI::Session.new(domain: "testshop.myshopify.com", token: "any-token", api_version: nil)
     assert_not session.valid?
+
+    session = ShopifyAPI::Session.new(domain: "testshop.myshopify.com", token: "any-token", api_version: ShopifyAPI::ApiVersion::NullVersion)
+    assert_not session.valid?
   end
 
   test "be valid with any token, any url and version" do