feat: Add x idempotency header (#259)

* feat: add x idempotency header for each request * fix rubocop * move the uuid in init once * tweak * tweak format * add tests * update changelog
appium · Apr 3, 2020 · 350ba7b · 350ba7b
1 parent 94f16d8
commit 350ba7b
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@ Read `release_notes.md` for commit level details.
 ## [Unreleased]
 
 ### Enhancements
+- Add `x-idempotency-key` header support (https://github.com/appium/appium-base-driver/pull/400)
+    - Can disable the header with `enable_idempotency_header: false` in `appium_lib` capability. Defaults to `true`.
 
 ### Bug fixes
 

diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb
@@ -12,12 +12,20 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+require 'securerandom'
+
 require_relative '../../version'
 
 module Appium
   module Core
     class Base
       module Http
+        module RequestHeaders
+          KEYS = {
+            idempotency: 'X-Idempotency-Key'
+          }.freeze
+        end
+
         class Default < Selenium::WebDriver::Remote::Http::Default
           DEFAULT_HEADERS = {
             'Accept' => CONTENT_TYPE,
@@ -26,6 +34,19 @@ class Default < Selenium::WebDriver::Remote::Http::Default
               "appium/ruby_lib_core/#{VERSION} (#{::Selenium::WebDriver::Remote::Http::Common::DEFAULT_HEADERS['User-Agent']})"
           }.freeze
 
+          attr_accessor :additional_headers
+
+          def initialize(open_timeout: nil, read_timeout: nil, enable_idempotency_header: true)
+            @open_timeout = open_timeout
+            @read_timeout = read_timeout
+
+            @additional_headers = if enable_idempotency_header
+                                    { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid }
+                                  else
+                                    {}
+                                  end
+          end
+
           # Update <code>server_url</code> provided when ruby_lib _core created a default http client.
           # Set <code>@http</code> as nil to re-create http client for the <code>server_url</code>
           #
@@ -65,19 +86,20 @@ def validate_url_param(scheme, host, port, path)
           def call(verb, url, command_hash)
             url      = server_url.merge(url) unless url.is_a?(URI)
             headers  = DEFAULT_HEADERS.dup
+            headers  = headers.merge @additional_headers unless @additional_headers.empty?
             headers['Cache-Control'] = 'no-cache' if verb == :get
 
             if command_hash
               payload                   = JSON.generate(command_hash)
               headers['Content-Length'] = payload.bytesize.to_s if [:post, :put].include?(verb)
-
-              ::Appium::Logger.info("   >>> #{url} | #{payload}")
-              ::Appium::Logger.debug("     > #{headers.inspect}")
             elsif verb == :post
               payload = '{}'
               headers['Content-Length'] = '2'
             end
 
+            ::Appium::Logger.info("   >>> #{url} | #{payload}")
+            ::Appium::Logger.info("     > #{headers.inspect}")
+
             request verb, url, headers, payload
           end
         end

diff --git a/lib/appium_lib_core/driver.rb b/lib/appium_lib_core/driver.rb
@@ -34,11 +34,12 @@ module Ios
     class Options
       attr_reader :custom_url, :default_wait, :export_session, :export_session_path,
                   :port, :wait_timeout, :wait_interval, :listener,
-                  :direct_connect
+                  :direct_connect, :enable_idempotency_header
 
       def initialize(appium_lib_opts)
         @custom_url = appium_lib_opts.fetch :server_url, nil
         @default_wait = appium_lib_opts.fetch :wait, Driver::DEFAULT_IMPLICIT_WAIT
+        @enable_idempotency_header = appium_lib_opts.fetch :enable_idempotency_header, true
 
         # bump current session id into a particular file
         @export_session = appium_lib_opts.fetch :export_session, false
@@ -104,6 +105,12 @@ class Driver
       # @return [Appium::Core::Base::Http::Default] the http client
       attr_reader :http_client
 
+      # Return if adding 'x-idempotency-key' header is each request.
+      # The key is unique for each http client instance. <code>Defaults to true</code>
+      # https://github.com/appium/appium-base-driver/pull/400
+      # @return [Bool]
+      attr_reader :enable_idempotency_header
+
       # Device type to request from the appium server
       # @return [Symbol] :android and :ios, for example
       attr_reader :device
@@ -114,7 +121,7 @@ class Driver
       attr_reader :automation_name
 
       # Custom URL for the selenium server. If set this attribute, ruby_lib_core try to handshake to the custom url.<br>
-      # Defaults to false. Then try to connect to <code>http://127.0.0.1:#{port}/wd/hub<code>.
+      # Defaults to false. Then try to connect to <code>http://127.0.0.1:#{port}/wd/hub</code>.
       # @return [String]
       attr_reader :custom_url
 
@@ -376,7 +383,9 @@ def start_driver(server_url: nil,
       private
 
       def create_http_client(http_client: nil, open_timeout: nil, read_timeout: nil)
-        @http_client = http_client || Appium::Core::Base::Http::Default.new
+        @http_client = http_client || Appium::Core::Base::Http::Default.new(
+          enable_idempotency_header: @enable_idempotency_header
+        )
 
         # open_timeout and read_timeout are explicit wait.
         @http_client.open_timeout = open_timeout if open_timeout
@@ -570,6 +579,7 @@ def set_appium_lib_specific_values(appium_lib_opts)
         opts = Options.new appium_lib_opts
 
         @custom_url ||= opts.custom_url # Keep existence capability if it's already provided
+        @enable_idempotency_header = opts.enable_idempotency_header
 
         @default_wait = opts.default_wait
 

diff --git a/test/unit/android/device/mjsonwp/commands_test.rb b/test/unit/android/device/mjsonwp/commands_test.rb
@@ -49,13 +49,24 @@ def test_shake
 
           def test_device_time
             stub_request(:get, "#{SESSION}/appium/device/system_time")
+              .with(body: {}.to_json)
               .to_return(headers: HEADER, status: 200, body: { value: 'device time' }.to_json)
 
             @driver.device_time
 
             assert_requested(:get, "#{SESSION}/appium/device/system_time", times: 1)
           end
 
+          def test_device_time_with_format
+            stub_request(:get, "#{SESSION}/appium/device/system_time")
+              .with(body: { format: 'YYYY-MM-DD' }.to_json)
+              .to_return(headers: HEADER, status: 200, body: { value: 'device time' }.to_json)
+
+            @driver.device_time('YYYY-MM-DD')
+
+            assert_requested(:get, "#{SESSION}/appium/device/system_time", times: 1)
+          end
+
           def test_open_notifications
             stub_request(:post, "#{SESSION}/appium/device/open_notifications")
               .to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

diff --git a/test/unit/android/device/w3c/commands_test.rb b/test/unit/android/device/w3c/commands_test.rb
@@ -48,13 +48,24 @@ def test_shake
 
           def test_device_time
             stub_request(:get, "#{SESSION}/appium/device/system_time")
+              .with(body: {}.to_json)
               .to_return(headers: HEADER, status: 200, body: { value: 'device time' }.to_json)
 
             @driver.device_time
 
             assert_requested(:get, "#{SESSION}/appium/device/system_time", times: 1)
           end
 
+          def test_device_time_with_format
+            stub_request(:get, "#{SESSION}/appium/device/system_time")
+              .with(body: { format: 'YYYY-MM-DD' }.to_json)
+              .to_return(headers: HEADER, status: 200, body: { value: 'device time' }.to_json)
+
+            @driver.device_time('YYYY-MM-DD')
+
+            assert_requested(:get, "#{SESSION}/appium/device/system_time", times: 1)
+          end
+
           def test_open_notifications
             stub_request(:post, "#{SESSION}/appium/device/open_notifications")
               .to_return(headers: HEADER, status: 200, body: { value: nil }.to_json)

diff --git a/test/unit/driver_test.rb b/test/unit/driver_test.rb
@@ -101,6 +101,16 @@ def test_default_timeout_for_http_client
       assert_equal '/wd/hub/', uri.path
     end
 
+    def test_http_client
+      client = ::Appium::Core::Base::Http::Default.new enable_idempotency_header: true
+      assert client.additional_headers.key?('X-Idempotency-Key')
+    end
+
+    def test_http_client_no_idempotency_header
+      client = ::Appium::Core::Base::Http::Default.new enable_idempotency_header: false
+      assert !client.additional_headers.key?('X-Idempotency-Key')
+    end
+
     def test_default_timeout_for_http_client_with_direct
       def android_mock_create_session_w3c_direct(core)
         response = {