Simplify cookies and add extra unit tests

Author: aguspe

rb/lib/selenium/webdriver/bidi/network/cookies.rb

@@ -21,21 +21,34 @@ module Selenium
   module WebDriver
     class BiDi
       class Cookies < Hash
+        KNOWN_KEYS = %i[name value domain path expiry http_only secure same_site size max_age].freeze
+
         def initialize(cookies = {})
           super()
           merge!(cookies)
         end
 
         def as_json
-          map do |name, value|
-            {
-              name: name.to_s,
-              value: {
-                type: 'string',
-                value: value.to_s
-              }
-            }
-          end
+          cookie_hash = {
+            name: self[:name].to_s,
+            value: {
+              type: 'string',
+              value: self[:value].to_s
+            },
+            domain: self[:domain],
+            path: self[:path],
+            size: self[:size],
+            httpOnly: self[:http_only],
+            secure: self[:secure],
+            sameSite: self[:same_site],
+            expiry: self[:expiry],
+            maxAge: self[:max_age]
+          }
+
+          extra_fields = except(KNOWN_KEYS)
+          cookie_hash.merge!(extra_fields) { |_k, old_val, _new_val| old_val }
+
+          [cookie_hash.compact]
         end
       end
     end # BiDi

rb/lib/selenium/webdriver/bidi/network/intercepted_response.rb

@@ -19,7 +19,7 @@
 
 require_relative 'credentials'
 require_relative 'headers'
-require_relative 'set_cookie_headers'
+require_relative 'cookies'
 
 module Selenium
   module WebDriver
@@ -35,7 +35,7 @@ def initialize(network, request)
         def continue
           network.continue_response(
             id: id,
-            cookies: set_cookie_headers.as_json,
+            cookies: cookies.as_json,
             headers: headers.as_json,
             credentials: credentials.as_json,
             reason: reason
@@ -50,8 +50,8 @@ def headers
           @headers ||= Headers.new
         end
 
-        def set_cookie_headers(set_cookie_headers = {})
-          @set_cookie_headers ||= SetCookieHeaders.new(set_cookie_headers)
+        def cookies(cookies = {})
+          @cookies ||= Cookies.new(cookies)
         end
       end
     end # BiDi

rb/lib/selenium/webdriver/bidi/network/set_cookie_headers.rb

@@ -2,9 +2,11 @@ module Selenium
   module WebDriver
     class BiDi
       class Cookies
-        def initialize: (Hash[Symbol, String] cookies) -> void
+        KNOWN_KEYS: Array[Symbol]
 
-        def as_json: () -> Hash[Symbol, String]
+        def initialize: (Hash[Symbol, String | Integer | bool] cookies) -> void
+
+        def as_json: () -> Array[Hash[Symbol, untyped]]
       end
     end
   end

rb/sig/lib/selenium/webdriver/bidi/network/cookies.rbs

@@ -2,25 +2,25 @@ module Selenium
   module WebDriver
     class BiDi
       class InterceptedResponse < InterceptedItem
-        @reason: untyped
+        @cookies: Hash[Symbol, String | Integer]?
+
+        @reason: String
 
         @credentials: untyped
 
         @headers: untyped
 
-        @set_cookie_headers: untyped
-
-        attr_accessor reason: untyped
+        attr_accessor reason: String
 
         def initialize: (untyped network, untyped request) -> void
 
         def continue: () -> untyped
 
+        def cookies:(?Hash[Symbol, String | Integer]? set_cookie_headers) -> untyped
+
         def credentials: (?username: untyped?, ?password: untyped?) -> untyped
 
         def headers: () -> untyped
-
-        def set_cookie_headers: (?untyped? set_cookie_headers) -> untyped
       end
     end
   end

rb/sig/lib/selenium/webdriver/bidi/network/intercepted_response.rbs

@@ -149,7 +149,17 @@ module WebDriver
             request.url = url_for('formPage.html')
             request.headers['foo'] = 'bar'
             request.headers['baz'] = 'qux'
-            request.cookies['foo'] = 'bar'
+            request.cookies({
+                              name: 'test',
+                              value: 'value4',
+                              domain: 'example.com',
+                              path: '/path',
+                              size: 1234,
+                              http_only: true,
+                              secure: true,
+                              same_site: 'Strict',
+                              expiry: 1234
+                            })
             request.body = ({test: 'example'})
             request.continue
           end
@@ -159,28 +169,6 @@ module WebDriver
         end
       end
 
-      it 'removes a header from a request handler' do
-        reset_driver!(web_socket_url: true) do |driver|
-          network = described_class.new(driver)
-          network.add_request_handler do |request|
-            request.add_header('foo', 'bar')
-            request.remove_header('foo')
-            expect(request.headers).to be_empty
-          end
-        end
-      end
-
-      it 'removes a cookie from a request handler' do
-        reset_driver!(web_socket_url: true) do |driver|
-          network = described_class.new(driver)
-          network.add_request_handler do |request|
-            request.add_cookie('foo', 'bar')
-            request.remove_cookie('foo')
-            expect(request.cookies).to be_empty
-          end
-        end
-      end
-
       it 'fails a request' do
         reset_driver!(web_socket_url: true) do |driver|
           network = described_class.new(driver)
@@ -256,16 +244,16 @@ module WebDriver
             response.headers['foo'] = 'bar'
             response.credentials.username = 'foo'
             response.credentials.password = 'bar'
-            response.set_cookie_headers({
-                                          name: 'foo',
-                                          domain: 'localhost',
-                                          http_only: true,
-                                          expiry: '1_000_000',
-                                          max_age: 1_000,
-                                          path: '/',
-                                          same_site: 'none',
-                                          secure: false
-                                        })
+            response.cookies({
+                               name: 'foo',
+                               domain: 'localhost',
+                               http_only: true,
+                               expiry: '1_000_000',
+                               max_age: 1_000,
+                               path: '/',
+                               same_site: 'none',
+                               secure: false
+                             })
             response.continue
           end
           driver.navigate.to url_for('formPage.html')

rb/spec/integration/selenium/webdriver/network_spec.rb

@@ -24,24 +24,76 @@ module Selenium
   module WebDriver
     class BiDi
       describe Cookies do
-        let(:cookies) { described_class.new }
-
         it 'returns the cookies as json' do
+          cookies = described_class.new
           cookies['key4'] = 'value4'
           cookies['session_id'] = 'xyz123'
 
           formatted_cookies = cookies.as_json
           expect(formatted_cookies).to be_an(Array)
-          expect(formatted_cookies.size).to eq(2)
+          expect(formatted_cookies.first['key4']).to eq('value4')
+          expect(formatted_cookies.first['session_id']).to eq('xyz123')
+        end
+
+        it 'serializes the cookies needed for request' do
+          cookies =
+            described_class.new(
+              {
+                name: 'test',
+                value: 'value4',
+                domain: 'example.com',
+                path: '/path',
+                size: 1234,
+                http_only: true,
+                secure: true,
+                same_site: 'Strict',
+                expiry: 1234
+              }
+            )
+
+          formatted_cookies = cookies.as_json
+          expect(formatted_cookies).to be_an(Array)
+          expect(formatted_cookies.size).to eq(1)
+
+          request_cookies = formatted_cookies.first
+          expect(request_cookies[:name]).to eq('test')
+          expect(request_cookies[:value][:type]).to eq('string')
+          expect(request_cookies[:value][:value]).to eq('value4')
+          expect(request_cookies[:domain]).to eq('example.com')
+          expect(request_cookies[:path]).to eq('/path')
+          expect(request_cookies[:expiry]).to eq(1234)
+          expect(request_cookies[:httpOnly]).to be(true)
+          expect(request_cookies[:secure]).to be(true)
+          expect(request_cookies[:sameSite]).to eq('Strict')
+          expect(request_cookies[:size]).to eq(1234)
+        end
 
-          key4_item = formatted_cookies.find { |h| h[:name] == 'key4' }
-          expect(key4_item).not_to be_nil
-          expect(key4_item[:value][:type]).to eq('string')
-          expect(key4_item[:value][:value]).to eq('value4')
+        it 'serializes the cookies needed for response' do
+          cookies = described_class.new({
+                                          name: 'test',
+                                          value: 'bar',
+                                          domain: 'localhost',
+                                          http_only: true,
+                                          expiry: '1_000_000',
+                                          max_age: 1_000,
+                                          path: '/',
+                                          same_site: 'lax',
+                                          secure: false
+                                        })
 
-          session_item = formatted_cookies.find { |h| h[:name] == 'session_id' }
-          expect(session_item).not_to be_nil
-          expect(session_item[:value][:value]).to eq('xyz123')
+          formatted_cookies = cookies.as_json
+          expect(formatted_cookies).to be_an(Array)
+          expect(formatted_cookies.size).to eq(1)
+          response_cookies = formatted_cookies.first
+          expect(response_cookies[:value][:type]).to eq('string')
+          expect(response_cookies[:value][:value]).to eq('bar')
+          expect(response_cookies[:domain]).to eq('localhost')
+          expect(response_cookies[:path]).to eq('/')
+          expect(response_cookies[:expiry]).to eq('1_000_000')
+          expect(response_cookies[:httpOnly]).to be(true)
+          expect(response_cookies[:secure]).to be(false)
+          expect(response_cookies[:sameSite]).to eq('lax')
+          expect(response_cookies[:maxAge]).to eq(1_000)
         end
       end
     end