From 5b5455ca5ceb526d2c0db9332398650cea1cccbc Mon Sep 17 00:00:00 2001 From: Volodymyr Byno Date: Thu, 16 May 2019 08:53:05 +0300 Subject: [PATCH] Introduce bang methods which raise exceptions when API returns errors e.g.: mws_api.subscriptions.register_destination!(...) Deprecate old methods (with no bangs) Update documentation --- README.markdown | 23 +++++++- lib/mws.rb | 6 +- lib/mws/api.rb | 8 +++ lib/mws/api/base.rb | 26 +++++++++ spec/mws-rb/api/base_spec.rb | 108 ++++++++++++++++++++++++++++++++++- spec/spec_helper.rb | 5 +- 6 files changed, 169 insertions(+), 7 deletions(-) create mode 100644 lib/mws/api.rb diff --git a/README.markdown b/README.markdown index bc6ae22..6177c6c 100644 --- a/README.markdown +++ b/README.markdown @@ -32,13 +32,14 @@ Using in a simple ruby file: To access the apis you can use: ```ruby - mws_api._api_name_._action_to_calll(params={}) + mws_api._api_name_._action_to_call!(params={}) ``` +The method above will return an error if any `ErrorResponse` is returned from MWS Let's say we want to retrieve a list of orders using MWS orders api: ```ruby - mws_api.orders.list_orders( + mws_api.orders.list_orders!( "MarketplaceId.Id.1" => "marketplace id", created_after: Time.new(2013, 1, 1) ) @@ -73,6 +74,24 @@ You can check on the MWS documentation section all actions and params needed: - http://docs.developer.amazonservices.com/en_US/sellers/index.html - http://docs.developer.amazonservices.com/en_US/subscriptions/Subscriptions_Overview.html - http://docs.developer.amazonservices.com/en_UK/merch_fulfill/index.html + +## Warnings + +There are equivalent methods with no bangs. They are almost the same except they don't raise an exception if MWS returns `ErrorResponse`. Their functionality will be the same as `bang`-methods in the next major version. For now, if you use them, you'll get a warning: + +``` +[WARNING] `list_orders` will have the same functionality as `list_orders!` in the next major version. +``` + +If you want to supress those warnings, you may configure it: + +```ruby +# config/initializers/mws.rb +MWS.config do |c| + c.display_warnings = false # is true by default +end +``` + ## LICENSE Copyright (c) Jhimy Fernandes Villar diff --git a/lib/mws.rb b/lib/mws.rb index 135b2ce..9c32d09 100644 --- a/lib/mws.rb +++ b/lib/mws.rb @@ -9,6 +9,7 @@ require_relative 'mws/query' require_relative 'mws/connection' +require_relative 'mws/api' require_relative 'mws/api/base' require_relative 'mws/api/orders' require_relative 'mws/api/reports' @@ -27,7 +28,8 @@ module MWS class << self attr_accessor :aws_access_key_id, :aws_secret_access_key, - :user_agent + :user_agent, + :display_warnings def config yield self @@ -37,4 +39,6 @@ def new(options = {}) MWS::Connection.new(options) end end + + self.display_warnings = true end diff --git a/lib/mws/api.rb b/lib/mws/api.rb new file mode 100644 index 0000000..7a06855 --- /dev/null +++ b/lib/mws/api.rb @@ -0,0 +1,8 @@ +module MWS + # API module + module API + ResponseError = Class.new(StandardError) + + RESPONSE_ERROR_KEY = 'ErrorResponse'.freeze + end +end diff --git a/lib/mws/api/base.rb b/lib/mws/api/base.rb index a5dccf2..424389e 100644 --- a/lib/mws/api/base.rb +++ b/lib/mws/api/base.rb @@ -7,6 +7,7 @@ class Base # TODO: Temporary solution, move to configuration DEFAULT_TIMEOUT = 2000 USER_AGENT = 'User-Agent'.freeze + STRICT_MEHTHOD_ENDING = '!'.freeze def initialize(connection) @verb ||= :get @@ -43,12 +44,26 @@ def build_query_from_params(action, params) def method_missing(name, *args) if self.class::ACTIONS.include?(name) + print_upcoming_functionality_warning(name) if MWS.display_warnings + call(name, *args) + elsif self.class::ACTIONS.include?(method_without_bang(name)) + response = call(method_without_bang(name), *args) + + return response unless response.key?(MWS::API::RESPONSE_ERROR_KEY) + + raise MWS::API::ResponseError, response.to_json else super end end + def respond_to_missing?(name, include_private = false) + self.class::ACTIONS.include?(name) || + self.class::ACTIONS.include?(method_without_bang(name)) || + super + end + def http_request_options @http_request_options ||= begin options = { timeout: DEFAULT_TIMEOUT } @@ -56,6 +71,17 @@ def http_request_options options end end + + private + + def method_without_bang(name) + name.to_s.chomp(STRICT_MEHTHOD_ENDING).to_sym + end + + def print_upcoming_functionality_warning(name) + warn "[WARNING] `#{name}` will have the same functionality as `#{name}!` " \ + "in the next major version." + end end end end diff --git a/spec/mws-rb/api/base_spec.rb b/spec/mws-rb/api/base_spec.rb index f1846ca..5c93f78 100644 --- a/spec/mws-rb/api/base_spec.rb +++ b/spec/mws-rb/api/base_spec.rb @@ -40,10 +40,112 @@ def initialize(connection) end describe 'method_missing to call actions' do - before(:each) { allow(HTTParty).to receive(:get).and_return({}) } + before(:each) { allow(HTTParty).to receive(:get).and_return(result) } - it 'should raise exception if Actions do not contain the action name' do - expect { test_api.action_not_found }.to raise_error(NoMethodError) + context 'regular method call' do + let(:result) { {} } + + it 'should raise exception if Actions do not contain the action name' do + expect { test_api.action_not_found }.to raise_error(NoMethodError) + end + + it 'makes api call and return result if Actions contain the action name' do + expect(test_api.test_action).to eq result + end + + context 'displaying warnings' do + subject { test_api.test_action } + + before do + @original_display_warnings = MWS.display_warnings + MWS.config { |config| config.display_warnings = display_warnings } + end + + after do + subject + MWS.config { |config| config.display_warnings = @original_display_warnings } + end + + context 'is true' do + let(:display_warnings) { true } + + let(:expected_deprication_warning) do + '[WARNING] `test_action` will have the same functionality as `test_action!` ' \ + "in the next major version.\n" + end + + it 'prints a deprication warning' do + expect { subject } + .to output(expected_deprication_warning) + .to_stderr + end + end + + context 'is false' do + let(:display_warnings) { false } + + it 'prints nothing' do + expect { subject }.not_to output.to_stderr + end + end + end + end + + context 'bang method' do + context 'API response does not contain errors' do + let(:result) { {} } + + it 'raises exception if Actions do not contain the action name' do + expect { test_api.action_not_found! }.to raise_error(NoMethodError) + end + + it 'makes api call and return result if Actions contain the action name' do + expect(test_api.test_action!).to eq result + end + end + + context 'API response contains errors' do + let(:result) { { 'ErrorResponse' => {} } } + + it 'makes api call and returns error' do + expect { test_api.test_action! } + .to raise_error MWS::API::ResponseError, '{"ErrorResponse":{}}' + end + end + end + end + + describe '#respond_to_missing?' do + subject { test_api.respond_to?(method_name) } + + context 'Actions contain method' do + let(:method_name) { 'test_action' } + + it { is_expected.to be true } + end + + context 'Actions do not contain method' do + let(:method_name) { 'missed_test_action' } + + it { is_expected.to be false } + end + + context 'Actions contain bang method' do + let(:method_name) { 'test_action!' } + + it { is_expected.to be true } + end + + context 'Actions do not contain bang method' do + let(:method_name) { 'missed_test_action!' } + + it { is_expected.to be false } + end + + context 'regular Ruby method' do + let(:method_name) { 'nil?' } + + it { is_expected.to be true } end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d676e2a..df3f04f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,4 +14,7 @@ config.filter_sensitive_data('DUMMY_AWS_SELLER_ID') { ENV['AWS_SELLER_ID'] } end -RSpec.configure(&:raise_errors_for_deprecations!) +RSpec.configure do |config| + config.before { MWS.display_warnings = false } + config.raise_errors_for_deprecations! +end