From ee951952e04a00735b02133df276db32cd4aa24c Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Wed, 1 Apr 2020 00:19:06 +0900 Subject: [PATCH 1/7] feat: add x idempotency header for each request --- .../common/base/http_default.rb | 18 ++++++++++++++++++ lib/appium_lib_core/driver.rb | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb index ad321263..3b82451e 100644 --- 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,15 @@ class Default < Selenium::WebDriver::Remote::Http::Default "appium/ruby_lib_core/#{VERSION} (#{::Selenium::WebDriver::Remote::Http::Common::DEFAULT_HEADERS['User-Agent']})" }.freeze + attr_accessor :enable_idempotency_header + + def initialize(open_timeout: nil, read_timeout: nil, enable_idempotency_header: true) + @open_timeout = open_timeout + @read_timeout = read_timeout + + @enable_idempotency_header = enable_idempotency_header + end + # Update server_url provided when ruby_lib _core created a default http client. # Set @http as nil to re-create http client for the server_url # @@ -66,6 +83,7 @@ def call(verb, url, command_hash) url = server_url.merge(url) unless url.is_a?(URI) headers = DEFAULT_HEADERS.dup headers['Cache-Control'] = 'no-cache' if verb == :get + headers[RequestHeaders::KEYS[:idempotency]] = SecureRandom.uuid if @enable_idempotency_header if command_hash payload = JSON.generate(command_hash) diff --git a/lib/appium_lib_core/driver.rb b/lib/appium_lib_core/driver.rb index b656ccf6..ba8abddb 100644 --- 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,11 @@ class Driver # @return [Appium::Core::Base::Http::Default] the http client attr_reader :http_client + # Return if 'x-idempotency-key' header is available in each request + # 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 @@ -376,7 +382,10 @@ 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 From 98d6816292c1391f61c7d38a84f296af3d6281e0 Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Wed, 1 Apr 2020 00:24:28 +0900 Subject: [PATCH 2/7] fix rubocop --- lib/appium_lib_core/common/base/http_default.rb | 2 +- lib/appium_lib_core/driver.rb | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb index 3b82451e..e7ceec63 100644 --- a/lib/appium_lib_core/common/base/http_default.rb +++ b/lib/appium_lib_core/common/base/http_default.rb @@ -22,7 +22,7 @@ class Base module Http module RequestHeaders KEYS = { - idempotency: 'X-Idempotency-Key' + idempotency: 'X-Idempotency-Key' }.freeze end diff --git a/lib/appium_lib_core/driver.rb b/lib/appium_lib_core/driver.rb index ba8abddb..3a946fcb 100644 --- a/lib/appium_lib_core/driver.rb +++ b/lib/appium_lib_core/driver.rb @@ -382,10 +382,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( - enable_idempotency_header: @enable_idempotency_header - ) + @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 From 12422fe441124568469d9a022c1e781e2adc5d5b Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Wed, 1 Apr 2020 09:10:41 +0900 Subject: [PATCH 3/7] move the uuid in init once --- lib/appium_lib_core/common/base/http_default.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb index e7ceec63..c677237d 100644 --- a/lib/appium_lib_core/common/base/http_default.rb +++ b/lib/appium_lib_core/common/base/http_default.rb @@ -34,13 +34,13 @@ class Default < Selenium::WebDriver::Remote::Http::Default "appium/ruby_lib_core/#{VERSION} (#{::Selenium::WebDriver::Remote::Http::Common::DEFAULT_HEADERS['User-Agent']})" }.freeze - attr_accessor :enable_idempotency_header + attr_accessor :idempotency_header def initialize(open_timeout: nil, read_timeout: nil, enable_idempotency_header: true) @open_timeout = open_timeout @read_timeout = read_timeout - @enable_idempotency_header = enable_idempotency_header + @idempotency_header = enable_idempotency_header ? { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid } : {} end # Update server_url provided when ruby_lib _core created a default http client. @@ -82,8 +82,8 @@ 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 @idempotency_header unless @idempotency_header.empty? headers['Cache-Control'] = 'no-cache' if verb == :get - headers[RequestHeaders::KEYS[:idempotency]] = SecureRandom.uuid if @enable_idempotency_header if command_hash payload = JSON.generate(command_hash) From 33137c6ad4feaab5441e3539ee2713f9a086bde3 Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Thu, 2 Apr 2020 10:37:12 +0900 Subject: [PATCH 4/7] tweak --- lib/appium_lib_core/common/base/http_default.rb | 12 ++++++------ test/unit/driver_test.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb index c677237d..ced8caba 100644 --- a/lib/appium_lib_core/common/base/http_default.rb +++ b/lib/appium_lib_core/common/base/http_default.rb @@ -34,13 +34,13 @@ class Default < Selenium::WebDriver::Remote::Http::Default "appium/ruby_lib_core/#{VERSION} (#{::Selenium::WebDriver::Remote::Http::Common::DEFAULT_HEADERS['User-Agent']})" }.freeze - attr_accessor :idempotency_header + attr_accessor :additional_headers def initialize(open_timeout: nil, read_timeout: nil, enable_idempotency_header: true) @open_timeout = open_timeout @read_timeout = read_timeout - @idempotency_header = enable_idempotency_header ? { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid } : {} + @additional_headers = enable_idempotency_header ? { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid } : {} end # Update server_url provided when ruby_lib _core created a default http client. @@ -82,20 +82,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 @idempotency_header unless @idempotency_header.empty? + 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/test/unit/driver_test.rb b/test/unit/driver_test.rb index eee3a1ff..58ee212e 100644 --- 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 = { From 6c033d855ef2a4df66db4a113805ae67c8a42e94 Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Fri, 3 Apr 2020 00:11:49 +0900 Subject: [PATCH 5/7] tweak format --- lib/appium_lib_core/common/base/http_default.rb | 6 +++++- lib/appium_lib_core/driver.rb | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/appium_lib_core/common/base/http_default.rb b/lib/appium_lib_core/common/base/http_default.rb index ced8caba..04d8a918 100644 --- a/lib/appium_lib_core/common/base/http_default.rb +++ b/lib/appium_lib_core/common/base/http_default.rb @@ -40,7 +40,11 @@ def initialize(open_timeout: nil, read_timeout: nil, enable_idempotency_header: @open_timeout = open_timeout @read_timeout = read_timeout - @additional_headers = enable_idempotency_header ? { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid } : {} + @additional_headers = if enable_idempotency_header + { RequestHeaders::KEYS[:idempotency] => SecureRandom.uuid } + else + {} + end end # Update server_url provided when ruby_lib _core created a default http client. diff --git a/lib/appium_lib_core/driver.rb b/lib/appium_lib_core/driver.rb index 3a946fcb..388e236f 100644 --- a/lib/appium_lib_core/driver.rb +++ b/lib/appium_lib_core/driver.rb @@ -105,7 +105,8 @@ class Driver # @return [Appium::Core::Base::Http::Default] the http client attr_reader :http_client - # Return if 'x-idempotency-key' header is available in each request + # Return if adding 'x-idempotency-key' header is each request. + # The key is unique for each http client instance. # https://github.com/appium/appium-base-driver/pull/400 # @return [Bool] attr_reader :enable_idempotency_header From 3d21e3e6452c8001161c8fdcded48c47c08f8c71 Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Fri, 3 Apr 2020 10:54:36 +0900 Subject: [PATCH 6/7] add tests --- test/unit/android/device/mjsonwp/commands_test.rb | 11 +++++++++++ test/unit/android/device/w3c/commands_test.rb | 11 +++++++++++ 2 files changed, 22 insertions(+) diff --git a/test/unit/android/device/mjsonwp/commands_test.rb b/test/unit/android/device/mjsonwp/commands_test.rb index 59738971..1f9a9ac4 100644 --- a/test/unit/android/device/mjsonwp/commands_test.rb +++ b/test/unit/android/device/mjsonwp/commands_test.rb @@ -49,6 +49,7 @@ 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 @@ -56,6 +57,16 @@ def test_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 index 82c26c61..c0c2dcf3 100644 --- a/test/unit/android/device/w3c/commands_test.rb +++ b/test/unit/android/device/w3c/commands_test.rb @@ -48,6 +48,7 @@ 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 @@ -55,6 +56,16 @@ def test_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) From c97f9f76025faaa45db3ec3b136512b1f8e3cdef Mon Sep 17 00:00:00 2001 From: Kazuaki Matsuo Date: Fri, 3 Apr 2020 11:06:38 +0900 Subject: [PATCH 7/7] update changelog --- CHANGELOG.md | 2 ++ lib/appium_lib_core/driver.rb | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d8e6aab..5be2e6da 100644 --- 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/driver.rb b/lib/appium_lib_core/driver.rb index 388e236f..8fb3d93e 100644 --- a/lib/appium_lib_core/driver.rb +++ b/lib/appium_lib_core/driver.rb @@ -106,7 +106,7 @@ class Driver attr_reader :http_client # Return if adding 'x-idempotency-key' header is each request. - # The key is unique for each http client instance. + # The key is unique for each http client instance. Defaults to true # https://github.com/appium/appium-base-driver/pull/400 # @return [Bool] attr_reader :enable_idempotency_header @@ -121,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.
- # Defaults to false. Then try to connect to http://127.0.0.1:#{port}/wd/hub. + # Defaults to false. Then try to connect to http://127.0.0.1:#{port}/wd/hub. # @return [String] attr_reader :custom_url