Merge pull request #498 from Shopify/warn-when-deprecated-header-is-returned

Log warning when Shopify indicates deprecated API call was performed

lib/active_resource/detailed_log_subscriber.rb

@@ -1,6 +1,17 @@
 module ActiveResource
   class DetailedLogSubscriber < ActiveSupport::LogSubscriber
     def request(event)
+      log_request_response_details(event)
+      warn_on_deprecated_header(event)
+    end
+
+    def logger
+      ActiveResource::Base.logger
+    end
+
+    private
+
+    def log_request_response_details(event)
       data = event.payload[:data]
       headers = data.extract_options!
       request_body = data.first
@@ -10,8 +21,20 @@ module ActiveResource
       info "Response:\n#{event.payload[:response].body}"
     end
 
-    def logger
-      ActiveResource::Base.logger
+    def warn_on_deprecated_header(event)
+      payload = event.payload
+
+      payload[:response].each do |header_name, header_value|
+        case header_name.downcase
+        when 'x-shopify-api-deprecated-reason'
+          warning_message = <<~MSG
+            [DEPRECATED] ShopifyAPI made a call to #{payload[:method].upcase} #{payload[:path]}, and this call made
+            use of a deprecated endpoint, behaviour, or parameter. See #{header_value} for more details.
+          MSG
+
+          warn warning_message
+        end
+      end
     end
   end
 end

lib/shopify_api/connection.rb

@@ -13,15 +13,17 @@ module ShopifyAPI
     module RequestNotification
       def request(method, path, *arguments)
         super.tap do |response|
-          notify_about_request(response, arguments)
+          notify_about_request(method, path, response, arguments)
         end
       rescue => e
-        notify_about_request(e.response, arguments) if e.respond_to?(:response)
+        notify_about_request(method, path, e.response, arguments) if e.respond_to?(:response)
         raise
       end
 
-      def notify_about_request(response, arguments)
+      def notify_about_request(method, path, response, arguments)
         ActiveSupport::Notifications.instrument("request.active_resource_detailed") do |payload|
+          payload[:method]   = method
+          payload[:path]     = path
           payload[:response] = response
           payload[:data]     = arguments
         end

shopify_api.gemspec

@@ -34,4 +34,6 @@ Gem::Specification.new do |s|
   s.add_development_dependency("minitest", ">= 4.0")
   s.add_development_dependency("rake")
   s.add_development_dependency("timecop")
+  s.add_development_dependency("pry")
+  s.add_development_dependency("pry-byebug")
 end

test/detailed_log_subscriber_test.rb

@@ -30,7 +30,6 @@ class LogSubscriberTest < Test::Unit::TestCase
     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]
-
   end
 
   test "logging on #find with an error" do
@@ -46,4 +45,23 @@ class LogSubscriberTest < Test::Unit::TestCase
     assert_equal "Headers: {\"Accept\"=>\"application/json\", #{@ua_header}}", @logger.logged(:info)[2]
     assert_equal "Response:", @logger.logged(:info)[3]
   end
+
+  test "warns when the server responds with a x-shopify-api-deprecated-reason header" do
+    fake(
+      "pages/1",
+      method: :get,
+      body: @page,
+      x_shopify_api_deprecated_reason: 'https://help.shopify.com/en/api/getting-started/api-deprecations'
+    )
+
+    ShopifyAPI::Page.find(1)
+
+    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{See https:\/\/help.shopify.com\/en\/api\/getting-started\/api-deprecations for more details.},
+      @logger.logged(:warn).first
+    )
+  end
 end

test/test_helper.rb

@@ -2,6 +2,7 @@ require 'rubygems'
 require 'minitest/autorun'
 require 'fakeweb'
 require 'mocha/setup'
+require 'pry'
 
 $LOAD_PATH.unshift(File.dirname(__FILE__))
 $LOAD_PATH.unshift(File.join(File.dirname(__FILE__), '..', 'lib'))