Introduce bang methods which raise exceptions when API returns errors

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

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

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

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,19 +44,44 @@ 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 }
           options.merge!(headers: { USER_AGENT => MWS.user_agent }) if MWS.user_agent.present?
           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

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
 

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