Merge pull request #548 from Shopify/add-api-versions-for-url-prefix-control

Add api versions for url prefix control

lib/shopify_api.rb

@@ -6,6 +6,7 @@ require 'digest/md5'
 require 'base64'
 require 'active_resource/detailed_log_subscriber'
 require 'shopify_api/limits'
+require 'shopify_api/api_version'
 require 'shopify_api/json_format'
 require 'active_resource/json_errors'
 require 'shopify_api/disable_prefix_check'
@@ -27,3 +28,5 @@ if ShopifyAPI::Base.respond_to?(:connection_class)
 else
   require 'active_resource/connection_ext'
 end
+
+ShopifyAPI::ApiVersion.define_known_versions

lib/shopify_api/api_version.rb

@@ -0,0 +1,88 @@
+# frozen_string_literal: true
+module ShopifyAPI
+  class ApiVersion
+    class UnknownVersion < StandardError; end
+
+    class NoVersion < ApiVersion
+      API_PREFIX = '/admin/'
+      GRAPHQL_PATH = '/admin/api/graphql.json'
+
+      def initialize
+        @version_name = "no_version"
+      end
+
+      def construct_api_path(path)
+        "#{API_PREFIX}#{path}"
+      end
+
+      def construct_graphql_path
+        GRAPHQL_PATH
+      end
+    end
+
+    class Unstable < ApiVersion
+      API_PREFIX = '/admin/api/unstable/'
+
+      def initialize
+        @version_name = "unstable"
+      end
+
+      def construct_api_path(path)
+        "#{API_PREFIX}#{path}"
+      end
+
+      def construct_graphql_path
+        construct_api_path('graphql.json')
+      end
+    end
+
+    def self.coerce_to_version(version_or_name)
+      return version_or_name if version_or_name.is_a?(ApiVersion)
+
+      @versions ||= {}
+      @versions.fetch(version_or_name.to_s) do
+        raise UnknownVersion, "#{version_or_name} is not in the defined version set: #{@versions.keys.join(', ')}"
+      end
+    end
+
+    def self.define_version(version)
+      @versions ||= {}
+
+      @versions[version.name] = version
+    end
+
+    def self.clear_defined_versions
+      @versions = {}
+    end
+
+    def self.define_known_versions
+      define_version(NoVersion.new)
+      define_version(Unstable.new)
+    end
+
+    def to_s
+      @version_name
+    end
+    alias_method :name, :to_s
+
+    def inspect
+      @version_name
+    end
+
+    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
+
+    def construct_graphql_path
+      raise NotImplementedError
+    end
+  end
+end

lib/shopify_api/resources/asset.rb

@@ -54,7 +54,7 @@ module ShopifyAPI
         path_prefix = params[:theme_id] ? "themes/#{params[:theme_id]}/" : ""
         resource = find(
           :one,
-          from: "#{api_prefix}#{path_prefix}assets.#{format.extension}",
+          from: api_version.construct_api_path("#{path_prefix}assets.#{format.extension}"),
           params: params
         )
         resource.prefix_options[:theme_id] = params[:theme_id] if resource && params[:theme_id]

lib/shopify_api/resources/base.rb

@@ -11,8 +11,6 @@ module ShopifyAPI
                                   "ActiveResource/#{ActiveResource::VERSION::STRING}",
                                   "Ruby/#{RUBY_VERSION}"].join(' ')
 
-    API_PREFIX = '/admin/'.freeze
-
     def encode(options = {})
       same = dup
       same.attributes = {self.class.element_name => same.attributes} if self.class.format.extension == 'json'
@@ -31,6 +29,22 @@ module ShopifyAPI
     end
 
     class << self
+      if respond_to?(:threadsafe_attribute)
+        threadsafe_attribute(:_api_version)
+      else
+        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?)
         def headers
           if _headers_defined?
@@ -57,21 +71,31 @@ 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
         self.site = nil
         self.password = nil
         self.user = nil
+        self.api_version = nil
         self.headers.delete('X-Shopify-Access-Token')
       end
 
-      def api_prefix
-        API_PREFIX
+      def api_version
+        if _api_version_defined?
+          _api_version
+        elsif superclass != Object && superclass.site
+          superclass.api_version.dup.freeze
+        end
+      end
+
+      def api_version=(value)
+        self._api_version = value
       end
 
       def prefix(options = {})
-        "#{api_prefix}#{resource_prefix(options)}"
+        api_version.construct_api_path(resource_prefix(options))
       end
 
       def prefix_source

lib/shopify_api/resources/graphql.rb

@@ -7,7 +7,7 @@ module ShopifyAPI
   class GraphQL
     def initialize
       uri = Base.site.dup
-      uri.path = '/admin/api/graphql.json'
+      uri.path = Base.api_version.construct_graphql_path
       @http = ::GraphQL::Client::HTTP.new(uri.to_s) do
         define_method(:headers) do |_context|
           Base.headers

lib/shopify_api/resources/shop.rb

@@ -10,7 +10,9 @@ module ShopifyAPI
       end
     else
       def self.current(options = {})
-        find(:one, options.merge(from: "#{api_prefix}shop.#{format.extension}"))
+        find(:one, options.merge(
+          from: api_version.construct_api_path("shop.#{format.extension}")
+        ))
       end
     end
 

lib/shopify_api/session.rb

@@ -11,6 +11,7 @@ module ShopifyAPI
     self.myshopify_domain = 'myshopify.com'
 
     attr_accessor :url, :token, :name, :extra
+    attr_reader :api_version
 
     class << self
 
@@ -18,11 +19,12 @@ module ShopifyAPI
         params.each { |k,value| public_send("#{k}=", value) }
       end
 
-      def temp(domain, token, &block)
-        session = new(domain, token)
+      def temp(domain, token, api_version = :no_version, &_block)
+        session = new(domain, token, api_version)
         original_site = ShopifyAPI::Base.site.to_s
         original_token = ShopifyAPI::Base.headers['X-Shopify-Access-Token']
-        original_session = new(original_site, original_token)
+        original_version = ShopifyAPI::Base.api_version
+        original_session = new(original_site, original_token, original_version)
 
         begin
           ShopifyAPI::Base.activate_session(session)
@@ -65,8 +67,9 @@ module ShopifyAPI
       end
     end
 
-    def initialize(url, token = nil, extra = {})
+    def initialize(url, token = nil, api_version = :no_version, extra = {})
       self.url = self.class.prepare_url(url)
+      self.api_version = api_version
       self.token = token
       self.extra = extra
     end
@@ -106,6 +109,10 @@ module ShopifyAPI
       "https://#{url}"
     end
 
+    def api_version=(version)
+      @api_version = ApiVersion.coerce_to_version(version)
+    end
+
     def valid?
       url.present? && token.present?
     end

test/api_version_test.rb

@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+require 'test_helper'
+
+class ApiVersionTest < Test::Unit::TestCase
+  def teardown
+    super
+    ShopifyAPI::ApiVersion.clear_defined_versions
+    ShopifyAPI::ApiVersion.define_known_versions
+  end
+
+  test "no version creates url that start with /admin/" do
+    assert_equal(
+      "/admin/resource_path/id.json",
+      ShopifyAPI::ApiVersion::NoVersion.new.construct_api_path("resource_path/id.json")
+    )
+  end
+
+  test "no version creates graphql url that start with /admin/api" do
+    assert_equal(
+      "/admin/api/graphql.json",
+      ShopifyAPI::ApiVersion::NoVersion.new.construct_graphql_path
+    )
+  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.new.construct_api_path("resource_path/id.json")
+    )
+  end
+
+  test "unstable version creates graphql url that start with /admin/api/unstable/" do
+    assert_equal(
+      "/admin/api/unstable/graphql.json",
+      ShopifyAPI::ApiVersion::Unstable.new.construct_graphql_path
+    )
+  end
+
+  test "coerce_to_version returns any version object given" do
+    version = ShopifyAPI::ApiVersion::Unstable.new
+    assert_same(version, ShopifyAPI::ApiVersion.coerce_to_version(version))
+  end
+
+  test "coerce_to_version converts a known version into a version object" do
+    versions = [
+      ShopifyAPI::ApiVersion::Unstable.new,
+      ShopifyAPI::ApiVersion::NoVersion.new,
+    ]
+
+    assert_equal(versions, [
+      ShopifyAPI::ApiVersion.coerce_to_version('unstable'),
+      ShopifyAPI::ApiVersion.coerce_to_version(:no_version),
+    ])
+  end
+
+  test "coerce_to_version raises when coercing a string that doesn't match a known version" do
+    assert_raises ShopifyAPI::ApiVersion::UnknownVersion do
+      ShopifyAPI::ApiVersion.coerce_to_version('made up version')
+    end
+  end
+
+  test "additional defined versions will also be coerced" do
+    versions = [
+      TestApiVersion.new('my_name'),
+      TestApiVersion.new('other_name'),
+    ]
+
+    versions.each do |version|
+      ShopifyAPI::ApiVersion.define_version(version)
+    end
+
+    assert_equal(versions, [
+      ShopifyAPI::ApiVersion.coerce_to_version('my_name'),
+      ShopifyAPI::ApiVersion.coerce_to_version('other_name'),
+    ])
+  end
+
+  class TestApiVersion < ShopifyAPI::ApiVersion
+    def initialize(name)
+      @version_name = name
+    end
+  end
+end

test/base_test.rb

@@ -86,6 +86,8 @@ class BaseTest < Test::Unit::TestCase
   end
 
   test "prefix= will forward to resource when the value does not start with admin" do
+    ShopifyAPI::Base.activate_session(@session1)
+
     TestResource.prefix = 'a/regular/path/'
 
     assert_equal('/admin/a/regular/path/', TestResource.prefix)
@@ -128,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', :no_version)
+    unstable_version = ShopifyAPI::Session.new('shop2.myshopify.com', 'token2', :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)

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", :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

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", :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.coerce_to_version(: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))