Browse Source

Introduce a second api version.

Alex Aitken 6 years ago
parent
commit
f4af5607e6

+ 37 - 6
lib/shopify_api/api_version.rb

@@ -1,13 +1,36 @@
 module ShopifyAPI
   class ApiVersion
-    API_PREFIX = '/admin/'.freeze
+    class NoVersion < ApiVersion
+      API_PREFIX = '/admin/'.freeze
+
+      def initialize
+        @version_name = "no version"
+      end
+
+      def construct_api_path(path)
+        "#{API_PREFIX}#{path}"
+      end
+    end
+
+    class Unstable < ApiVersion
+      API_PREFIX = '/admin/api/'.freeze
+
+      def initialize
+        @version_name = "unstable"
+        @url = "#{API_PREFIX}unstable/"
+      end
+
+      def construct_api_path(path)
+        "#{@url}#{path}"
+      end
+    end
 
     def self.no_version
-      new
+      NoVersion.new
     end
 
-    def initialize
-      @version_name = "no version"
+    def self.unstable
+      Unstable.new
     end
 
     def to_s
@@ -18,8 +41,16 @@ module ShopifyAPI
       @version_name
     end
 
-    def construct_api_path(path)
-      "#{API_PREFIX}#{path}"
+    def ==(other)
+      other.class == self.class && to_s == other.to_s
+    end
+
+    def hash
+      version_name.hash
+    end
+
+    def construct_api_path(_path)
+      raise NotImplementedError
     end
   end
 end

+ 21 - 7
lib/shopify_api/resources/base.rb

@@ -29,11 +29,20 @@ module ShopifyAPI
     end
 
     class << self
-      if ActiveResource::VERSION::MAJOR >= 5 ||
-          (ActiveResource::VERSION::MAJOR == 4 && ActiveResource::VERSION::MINOR >= 1 )
-        threadsafe_attribute :_api_version
+      if respond_to?(:threadsafe_attribute)
+        threadsafe_attribute(:_api_version)
       else
-        cattr_accessor :_api_version
+        def _api_version
+          @api_version
+        end
+
+        def _api_version_defined?
+          defined?(@api_version)
+        end
+
+        def _api_version=(value)
+          @api_version = value
+        end
       end
 
       if ActiveResource::Base.respond_to?(:_headers) && ActiveResource::Base.respond_to?(:_headers_defined?)
@@ -62,6 +71,7 @@ module ShopifyAPI
         raise InvalidSessionError.new("Session cannot be nil") if session.nil?
         self.site = session.site
         self.headers.merge!('X-Shopify-Access-Token' => session.token)
+        self.api_version = session.api_version
       end
 
       def clear_session
@@ -73,11 +83,15 @@ module ShopifyAPI
       end
 
       def api_version
-        self._api_version ||= ApiVersion.no_version
+        if _api_version_defined?
+          _api_version
+        elsif superclass != Object && superclass.site
+          superclass.api_version.dup.freeze
+        end
       end
 
-      def api_version=(api_version)
-        self._api_version = api_version
+      def api_version=(value)
+        self._api_version = value
       end
 
       def prefix(options = {})

+ 18 - 0
test/api_version_test.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+require 'test_helper'
+
+class ApiVersionTest < Test::Unit::TestCase
+  test "no version creates url that start with /admin/" do
+    assert_equal(
+      "/admin/resource_path/id.json",
+      ShopifyAPI::ApiVersion.no_version.construct_api_path("resource_path/id.json")
+    )
+  end
+
+  test "unstable version creates url that start with /admin/api/unstable/" do
+    assert_equal(
+      "/admin/api/unstable/resource_path/id.json",
+      ShopifyAPI::ApiVersion.unstable.construct_api_path("resource_path/id.json")
+    )
+  end
+end

+ 26 - 0
test/base_test.rb

@@ -130,6 +130,32 @@ class BaseTest < Test::Unit::TestCase
     end
   end
 
+  test "using a different version changes the url" do
+    no_version = ShopifyAPI::Session.new('shop1.myshopify.com', 'token1', ShopifyAPI::ApiVersion.no_version)
+    unstable_version = ShopifyAPI::Session.new('shop2.myshopify.com', 'token2', ShopifyAPI::ApiVersion.unstable)
+
+    fake(
+      "shop",
+      url: "https://shop1.myshopify.com/admin/shop.json",
+      method: :get,
+      status: 201,
+      body: '{ "shop": { "id": 1 } }'
+    )
+    fake(
+      "shop",
+      url: "https://shop2.myshopify.com/admin/api/unstable/shop.json",
+      method: :get,
+      status: 201,
+      body: '{ "shop": { "id": 2 } }'
+    )
+
+    ShopifyAPI::Base.activate_session(no_version)
+    assert_equal 1, ShopifyAPI::Shop.current.id
+
+    ShopifyAPI::Base.activate_session(unstable_version)
+    assert_equal 2, ShopifyAPI::Shop.current.id
+  end
+
   def clear_header(header)
     [ActiveResource::Base, ShopifyAPI::Base, ShopifyAPI::Product].each do |klass|
       klass.headers.delete(header)

+ 20 - 9
test/detailed_log_subscriber_test.rb

@@ -1,16 +1,23 @@
 require 'test_helper'
 require "active_support/log_subscriber/test_helper"
+require 'json'
 
 class LogSubscriberTest < Test::Unit::TestCase
   include ActiveSupport::LogSubscriber::TestHelper
 
+  attr_reader :request_headers
+
   def setup
     super
     @page = { :page => { :id => 1, :title => 'Shopify API' } }.to_json
     @ua_header = "\"User-Agent\"=>\"ShopifyAPI/#{ShopifyAPI::VERSION} ActiveResource/#{ActiveResource::VERSION::STRING} Ruby/#{RUBY_VERSION}\""
+    @request_headers = "Headers: {\"Accept\"=>\"application/json\", " \
+      "#{@ua_header}, \"X-Shopify-Access-Token\"=>\"access_token\"}"
 
     ShopifyAPI::Base.clear_session
-    ShopifyAPI::Base.site = "https://this-is-my-test-shop.myshopify.com/admin"
+    session = ShopifyAPI::Session.new("https://this-is-my-test-shop.myshopify.com", "access_token", ShopifyAPI::ApiVersion.no_version)
+
+    ShopifyAPI::Base.activate_session(session)
 
     ActiveResource::LogSubscriber.attach_to :active_resource
     ActiveResource::DetailedLogSubscriber.attach_to :active_resource_detailed
@@ -25,11 +32,15 @@ class LogSubscriberTest < Test::Unit::TestCase
 
     ShopifyAPI::Page.find(1)
 
-    assert_equal 4, @logger.logged(:info).size
-    assert_equal "GET https://this-is-my-test-shop.myshopify.com:443/admin/pages/1.json", @logger.logged(:info)[0]
-    assert_match /\-\-\> 200/, @logger.logged(:info)[1]
-    assert_equal "Headers: {\"Accept\"=>\"application/json\", #{@ua_header}}", @logger.logged(:info)[2]
-    assert_match /Response:\n\{\"page\"\:\{((\"id\"\:1)|(\"title\"\:\"Shopify API\")),((\"id\"\:1)|(\"title\"\:\"Shopify API\"))\}\}/,  @logger.logged(:info)[3]
+    assert_equal(4, @logger.logged(:info).size)
+    assert_equal("GET https://this-is-my-test-shop.myshopify.com:443/admin/pages/1.json", @logger.logged(:info)[0])
+    assert_match(/\-\-\> 200/, @logger.logged(:info)[1])
+    assert_equal(request_headers, @logger.logged(:info)[2])
+    assert_match(
+      %r{(Response:\n\{\"page\"\:\{\"id\"\:1,\"title\"\:\"Shopify API\"\}\})|
+        (Response:\n\{\"page\"\:\{\"title\"\:\"Shopify API\",\"id\"\:1\}\})},
+      @logger.logged(:info)[3]
+    )
   end
 
   test "logging on #find with an error" do
@@ -43,7 +54,7 @@ class LogSubscriberTest < Test::Unit::TestCase
       assert_equal(4, @logger.logged(:info).size)
       assert_equal("GET https://this-is-my-test-shop.myshopify.com:443/admin/pages/2.json", @logger.logged(:info)[0])
       assert_match(/\-\-\> 404/, @logger.logged(:info)[1])
-      assert_equal("Headers: {\"Accept\"=>\"application/json\", #{@ua_header}}", @logger.logged(:info)[2])
+      assert_equal(request_headers, @logger.logged(:info)[2])
       assert_equal("Response:", @logger.logged(:info)[3])
     else
       assert_equal(2, @logger.logged(:error).size)
@@ -51,7 +62,7 @@ class LogSubscriberTest < Test::Unit::TestCase
       assert_match(/\-\-\> 404/, @logger.logged(:error)[1])
 
       assert_equal(2, @logger.logged(:info).size)
-      assert_equal("Headers: {\"Accept\"=>\"application/json\", #{@ua_header}}", @logger.logged(:info)[0])
+      assert_equal(request_headers, @logger.logged(:info)[0])
       assert_equal("Response:", @logger.logged(:info)[1])
     end
   end
@@ -68,7 +79,7 @@ class LogSubscriberTest < Test::Unit::TestCase
 
     assert_equal 1, @logger.logged(:warn).size
 
-    assert_match %r{\[DEPRECATED\] ShopifyAPI made a call to GET \/admin\/pages\/1.json}, @logger.logged(:warn).first
+    assert_match(%r{\[DEPRECATED\] ShopifyAPI made a call to GET \/admin\/pages\/1.json}, @logger.logged(:warn).first)
     assert_match(
       %r{See https:\/\/help.shopify.com\/en\/api\/getting-started\/api-deprecations for more details.},
       @logger.logged(:warn).first

+ 5 - 4
test/test_helper.rb

@@ -42,9 +42,9 @@ class Test::Unit::TestCase < Minitest::Unit::TestCase
     end
 
     ShopifyAPI::Base.clear_session
-    ShopifyAPI::Base.site = "https://this-is-my-test-shop.myshopify.com/admin"
-    ShopifyAPI::Base.password = nil
-    ShopifyAPI::Base.user = nil
+    session = ShopifyAPI::Session.new("https://this-is-my-test-shop.myshopify.com", "token_test_helper", ShopifyAPI::ApiVersion.no_version)
+
+    ShopifyAPI::Base.activate_session(session)
   end
 
   def teardown
@@ -85,12 +85,13 @@ class Test::Unit::TestCase < Minitest::Unit::TestCase
     body   = options.has_key?(:body) ? options.delete(:body) : load_fixture(endpoint)
     format = options.delete(:format) || :json
     method = options.delete(:method) || :get
+    api_version = options.delete(:api_version) || ShopifyAPI::ApiVersion.no_version
     extension = ".#{options.delete(:extension)||'json'}" unless options[:extension]==false
 
     url = if options.has_key?(:url)
       options[:url]
     else
-      "https://this-is-my-test-shop.myshopify.com/admin/#{endpoint}#{extension}"
+      "https://this-is-my-test-shop.myshopify.com#{api_version.construct_api_path("#{endpoint}#{extension}")}"
     end
 
     FakeWeb.register_uri(method, url, {:body => body, :status => 200, :content_type => "text/#{format}", :content_length => 1}.merge(options))