[rb] implement client config class

rb/lib/selenium/webdriver/common.rb

@@ -18,6 +18,7 @@
 # under the License.
 
 require 'selenium/webdriver/common/error'
+require 'selenium/webdriver/common/client_config'
 require 'selenium/webdriver/common/local_driver'
 require 'selenium/webdriver/common/driver_finder'
 require 'selenium/webdriver/common/platform'

rb/lib/selenium/webdriver/common/client_config.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+# Licensed to the Software Freedom Conservancy (SFC) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The SFC licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+module Selenium
+  module WebDriver
+    #
+    # Immutable Configuration for HTTP clients.
+    #
+
+    ClientConfig = Data.define(
+      :open_timeout,
+      :read_timeout,
+      :proxy,
+      :extra_headers,
+      :user_agent,
+      :server_url
+    ) do
+      def initialize(
+        open_timeout: nil,
+        read_timeout: nil,
+        proxy: nil,
+        extra_headers: nil,
+        user_agent: nil,
+        server_url: nil
+      )
+        super
+      end
+    end
+  end
+end

rb/lib/selenium/webdriver/common/driver.rb

@@ -319,9 +319,9 @@ def ref
 
       attr_reader :bridge
 
-      def create_bridge(caps:, url:, http_client: nil)
+      def create_bridge(caps:, url: nil, http_client: nil, client_config: nil)
         klass = caps['webSocketUrl'] ? Remote::BiDiBridge : Remote::Bridge
-        klass.new(http_client: http_client, url: url).tap do |bridge|
+        klass.new(http_client: http_client, url: url, client_config: client_config).tap do |bridge|
           bridge.create_session(caps)
         end
       end

rb/lib/selenium/webdriver/remote/bridge.rb

@@ -54,17 +54,20 @@ def element_class
         # Initializes the bridge with the given server URL
         # @param [String, URI] url url for the remote server
         # @param [Object] http_client an HTTP client instance that implements the same protocol as Http::Default
+        # @param [ClientConfig] client_config configuration for the HTTP client
         # @api private
         #
 
-        def initialize(url:, http_client: nil)
-          uri = url.is_a?(URI) ? url : URI.parse(url)
-          uri.path += '/' unless uri.path.end_with?('/')
+        def initialize(url: nil, http_client: nil, client_config: nil)
+          if http_client && client_config
+            raise Error::WebDriverError, 'Cannot specify both http_client and client_config'
+          end
 
-          @http = http_client || Http::Default.new
-          @http.server_url = uri
-          @file_detector = nil
+          @http = http_client || build_http_client(client_config)
+          validate_server_url_args(url, client_config)
+          @http.server_url = normalize_url(url || client_config&.server_url)
 
+          @file_detector = nil
           @locator_converter = self.class.locator_converter
         end
 
@@ -89,10 +92,12 @@ def create_session(capabilities)
             extend(WebDriver::Firefox::Features)
           when 'msedge', 'MicrosoftEdge'
             extend(WebDriver::Edge::Features)
-          when 'Safari', 'Safari Technology Preview'
+          when 'safari', 'Safari', 'Safari Technology Preview'
             extend(WebDriver::Safari::Features)
           when 'internet explorer'
             extend(WebDriver::IE::Features)
+          else
+            WebDriver.logger.info "Unknown browser: #{capabilities[:browser_name]}", id: :session
           end
         end
 
@@ -662,6 +667,41 @@ def prepare_capabilities_payload(capabilities)
           capabilities = {alwaysMatch: capabilities} if !capabilities['alwaysMatch'] && !capabilities['firstMatch']
           {capabilities: capabilities}
         end
+
+        def create_http_client(client_config, url: nil)
+          http = build_http_client(client_config)
+          validate_server_url_args(url, client_config)
+          http.server_url = normalize_url(url || client_config&.server_url)
+          http
+        end
+
+        def build_http_client(client_config)
+          if client_config
+            http = Http::Default.new(open_timeout: client_config.open_timeout,
+                                     read_timeout: client_config.read_timeout)
+            http.proxy = client_config.proxy if client_config.proxy
+
+            Http::Common.extra_headers = client_config.extra_headers if client_config.extra_headers
+            Http::Common.user_agent = client_config.user_agent if client_config.user_agent
+          else
+            http = Http::Default.new
+          end
+          http
+        end
+
+        def validate_server_url_args(url, client_config)
+          if url && client_config&.server_url
+            raise Error::WebDriverError, 'Cannot specify url in both keyword and http_client'
+          elsif url.nil? && !client_config&.server_url
+            raise Error::WebDriverError, 'No server URL provided'
+          end
+        end
+
+        def normalize_url(url)
+          url = URI.parse(url) if url && !url.is_a?(URI)
+          url&.path += '/' unless url&.path&.end_with?('/')
+          url
+        end
       end # Bridge
     end # Remote
   end # WebDriver

rb/lib/selenium/webdriver/remote/http/common.rb

@@ -40,6 +40,10 @@ def user_agent
 
           attr_writer :server_url
 
+          def server_url?
+            !@server_url.nil?
+          end
+
           def quit_errors
             [IOError]
           end

rb/lib/selenium/webdriver/remote/http/default.rb

@@ -34,8 +34,8 @@ class Default < Common
           # @param [Numeric] open_timeout - Open timeout to apply to HTTP client.
           # @param [Numeric] read_timeout - Read timeout (seconds) to apply to HTTP client.
           def initialize(open_timeout: nil, read_timeout: nil)
-            @open_timeout = open_timeout
-            @read_timeout = read_timeout
+            @open_timeout = open_timeout if open_timeout
+            @read_timeout = read_timeout if read_timeout
             super()
           end
 

rb/sig/lib/selenium/webdriver/common/client_config.rbs

@@ -0,0 +1,28 @@
+module Selenium
+  module WebDriver
+
+    ClientConfig: Class
+
+    class ClientConfig
+      attr_reader open_timeout: Integer?
+      attr_reader read_timeout: Integer?
+      attr_reader proxy: Selenium::WebDriver::Proxy?
+      attr_reader extra_headers: Hash[String, String]?
+      attr_reader user_agent: String?
+      attr_reader server_url: (String | URI)?
+
+      def initialize: (
+          ?open_timeout: Integer?,
+          ?read_timeout: Integer?,
+          ?proxy: Selenium::WebDriver::Proxy?,
+          ?extra_headers: Hash[String, String]?,
+          ?user_agent: String?,
+          ?server_url: (String | URI)?
+        ) -> void
+
+      def with: (**untyped) -> ClientConfig
+      def to_h: -> Hash[Symbol, untyped]
+      def deconstruct_keys: (Array[Symbol]?) -> Hash[Symbol, untyped]
+    end
+  end
+end

rb/sig/lib/selenium/webdriver/common/driver.rbs

@@ -69,7 +69,7 @@ module Selenium
 
       attr_reader bridge: untyped
 
-      def create_bridge: (caps: untyped, url: untyped, ?http_client: untyped) -> untyped
+      def create_bridge: (caps: Remote::Capabilities, url: (String | URI)?, ?http_client: Remote::Http::Common?, ?client_config: ClientConfig?) -> Remote::Bridge
 
       def service_url: (untyped service) -> untyped
 

rb/sig/lib/selenium/webdriver/remote/bridge.rbs

@@ -30,7 +30,7 @@ module Selenium
 
         attr_reader capabilities: untyped
 
-        def initialize: (url: String | URI, ?http_client: untyped?) -> void
+        def initialize: (?url: (String | URI)?, ?http_client: untyped?, ?client_config: ClientConfig?) -> void
 
         def bidi: -> WebDriver::Error::WebDriverError
 
@@ -248,6 +248,14 @@ module Selenium
 
         def convert_locator: (untyped how, untyped what) -> ::Array[untyped]
 
+        def create_http_client: (?ClientConfig? client_config, ?url: (String | URI)?) -> Http::Common
+
+        def build_http_client: (ClientConfig? client_config) -> Http::Common
+
+        def validate_server_url_args: ((String | URI)? url, ClientConfig? client_config) -> void
+
+        def normalize_url: ((String | URI)? url) -> URI?
+
         ESCAPE_CSS_REGEXP: ::Regexp
 
         UNICODE_CODE_POINT: 30

rb/sig/lib/selenium/webdriver/remote/http/common.rbs

@@ -11,25 +11,27 @@ module Selenium
 
           @common_headers: Hash[String, untyped]
 
-          attr_accessor self.extra_headers: Hash[String, untyped]
+          attr_accessor self.extra_headers: Hash[String, untyped]?
 
-          attr_writer self.user_agent: String
+          attr_writer self.user_agent: String?
 
           def self.user_agent: -> String
 
-          attr_writer server_url: String
+          attr_writer server_url?: URI
 
           def quit_errors: () -> Array[untyped]
 
           def close: () -> nil
 
           def call: (untyped verb, untyped url, untyped command_hash) -> untyped
 
+          def server_url?: -> bool
+
           private
 
           def common_headers: -> Hash[String, untyped]
 
-          def server_url: () -> String
+          def server_url: () -> URI
 
           def request: (*untyped) -> untyped
 

rb/sig/lib/selenium/webdriver/remote/http/default.rbs

@@ -4,21 +4,21 @@ module Selenium
       module Http
         # @api private
         class Default < Common
-          @open_timeout: untyped
+          @open_timeout: Numeric
 
-          @read_timeout: untyped
+          @read_timeout: Numeric
 
           @http: untyped
 
-          @proxy: untyped
+          @proxy: Proxy
 
-          attr_writer proxy: untyped
+          attr_writer proxy: Proxy?
 
-          attr_accessor open_timeout: untyped
+          attr_accessor open_timeout: Numeric
 
-          attr_accessor read_timeout: untyped
+          attr_accessor read_timeout: Numeric
 
-          def initialize: (?open_timeout: untyped?, ?read_timeout: untyped?) -> void
+          def initialize: (?open_timeout: Numeric?, ?read_timeout: Numeric?) -> void
 
           def close: () -> untyped
 
@@ -40,7 +40,7 @@ module Selenium
 
           def proxy: () -> untyped
 
-          def use_proxy?: () -> untyped
+          def use_proxy?: () -> bool
         end
       end
     end

rb/spec/unit/selenium/webdriver/remote/bridge_spec.rb

@@ -51,8 +51,63 @@ module Remote
         end
 
         describe '#initialize' do
-          it 'raises ArgumentError if passed invalid options' do
-            expect { described_class.new(foo: 'bar') }.to raise_error(ArgumentError)
+          it 'creates default http client' do
+            bridge = described_class.new(url: 'http://localhost')
+            expect(bridge.http).to be_an_instance_of(WebDriver::Remote::Http::Default)
+          end
+
+          it 'uses provided implementation of http client' do
+            url = 'http://localhost'
+            custom_http = instance_double(Remote::Http::Default, server_url?: false, 'server_url=': url)
+            bridge = described_class.new(http_client: custom_http, url: url)
+            expect(bridge.http).to eq(custom_http)
+          end
+
+          it 'errors if http client and client config are both provided' do
+            custom_http = instance_double(Remote::Http::Default)
+            client_config = ClientConfig.new(server_url: 'http://localhost')
+            expect {
+              described_class.new(http_client: custom_http, client_config: client_config)
+            }.to raise_error(Error::WebDriverError, /cannot specify both/i)
+          end
+
+          it 'errors if neither http client nor client config are provided' do
+            expect {
+              described_class.new
+            }.to raise_error(Error::WebDriverError, /no server url provided/i)
+          end
+
+          it 'errors if url specified in keyword and client config' do
+            client_config = ClientConfig.new(server_url: 'http://localhost')
+            expect {
+              described_class.new(url: 'http://localhost', client_config: client_config)
+            }.to raise_error(Error::WebDriverError, /Cannot specify url in both/i)
+          end
+
+          it 'creates http client from client_config' do
+            proxy = instance_double(Proxy)
+            extra_headers = {'X-Custom-Header' => 'custom_value'}
+            custom_user_agent = 'Custom User Agent'
+            client_config = ClientConfig.new(
+              open_timeout: 15,
+              read_timeout: 25,
+              proxy: proxy,
+              extra_headers: extra_headers,
+              user_agent: custom_user_agent,
+              server_url: 'http://localhost'
+            )
+            original_user_agent = WebDriver::Remote::Http::Common.user_agent
+            bridge = described_class.new(client_config: client_config)
+
+            expect(bridge.http).to be_an_instance_of(WebDriver::Remote::Http::Default)
+            expect(bridge.http.open_timeout).to eq(15)
+            expect(bridge.http.read_timeout).to eq(25)
+            expect(bridge.http.instance_variable_get(:@proxy)).to eq(proxy)
+            expect(WebDriver::Remote::Http::Common.extra_headers).to eq(extra_headers)
+            expect(WebDriver::Remote::Http::Common.user_agent).to eq(custom_user_agent)
+          ensure
+            WebDriver::Remote::Http::Common.extra_headers = nil
+            WebDriver::Remote::Http::Common.user_agent = original_user_agent
           end
         end