From 998bb019620004e698f5c39db3515704265527e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C4=B0nan=C3=A7=20G=C3=BCm=C3=BC=C5=9F?= Date: Mon, 4 Nov 2024 11:31:59 -0500 Subject: [PATCH] Refactor GrantPermissionOptions parsing to mapping - Turns GrantPermissionsOptions to a value type from a pointer as this type is not need to be used to be mutated. - Moves Sobek transformation to the mapping layer. - Errors out if an incorrect option is given. This way we don't surprise users and train them to use the API correctly. Otherwise, hiding these errors might lead to unexpected issues. --- browser/browser_context_mapping.go | 33 ++++++++++++++++++++----- browser/sync_browser_context_mapping.go | 9 ++++--- common/browser_context.go | 4 +-- common/browser_context_options.go | 20 --------------- tests/browser_context_test.go | 10 ++++++-- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/browser/browser_context_mapping.go b/browser/browser_context_mapping.go index 5ba69b95a..3981cbffd 100644 --- a/browser/browser_context_mapping.go +++ b/browser/browser_context_mapping.go @@ -74,13 +74,14 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin return bc.Cookies(urls...) //nolint:wrapcheck }) }, - "grantPermissions": func(permissions []string, opts sobek.Value) *sobek.Promise { + "grantPermissions": func(permissions []string, opts sobek.Value) (*sobek.Promise, error) { + popts, err := parseGrantPermissionOptions(vu.Runtime(), opts) + if err != nil { + return nil, fmt.Errorf("parsing grant permission options: %w", err) + } return k6ext.Promise(vu.Context(), func() (any, error) { - popts := common.NewGrantPermissionsOptions() - popts.Parse(vu.Context(), opts) - - return nil, bc.GrantPermissions(permissions, popts) //nolint:wrapcheck - }) + return nil, bc.GrantPermissions(permissions, popts) + }), nil }, "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, "setDefaultTimeout": bc.SetDefaultTimeout, @@ -185,3 +186,23 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin }, } } + +// parseGrantPermissionOptions parses the options for the browserContext.grantPermissions API. +func parseGrantPermissionOptions(rt *sobek.Runtime, opts sobek.Value) (common.GrantPermissionsOptions, error) { + if !sobekValueExists(opts) { + return common.GrantPermissionsOptions{}, nil + } + + var g common.GrantPermissionsOptions + + o := opts.ToObject(rt) + for _, k := range o.Keys() { + if k == "origin" { + g.Origin = o.Get(k).String() + continue + } + return common.GrantPermissionsOptions{}, fmt.Errorf("unknown option: %s", k) + } + + return g, nil +} diff --git a/browser/sync_browser_context_mapping.go b/browser/sync_browser_context_mapping.go index b304a64a0..50f419149 100644 --- a/browser/sync_browser_context_mapping.go +++ b/browser/sync_browser_context_mapping.go @@ -49,10 +49,11 @@ func syncMapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //n "close": bc.Close, "cookies": bc.Cookies, "grantPermissions": func(permissions []string, opts sobek.Value) error { - pOpts := common.NewGrantPermissionsOptions() - pOpts.Parse(vu.Context(), opts) - - return bc.GrantPermissions(permissions, pOpts) //nolint:wrapcheck + popts, err := parseGrantPermissionOptions(vu.Runtime(), opts) + if err != nil { + return fmt.Errorf("parsing grant permission options: %w", err) + } + return bc.GrantPermissions(permissions, popts) }, "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, "setDefaultTimeout": bc.SetDefaultTimeout, diff --git a/common/browser_context.go b/common/browser_context.go index 65d6bda52..3e2257dce 100644 --- a/common/browser_context.go +++ b/common/browser_context.go @@ -131,7 +131,7 @@ func NewBrowserContext( } if len(opts.Permissions) > 0 { - err := b.GrantPermissions(opts.Permissions, NewGrantPermissionsOptions()) + err := b.GrantPermissions(opts.Permissions, GrantPermissionsOptions{}) if err != nil { return nil, err } @@ -206,7 +206,7 @@ func (b *BrowserContext) Close() error { } // GrantPermissions enables the specified permissions, all others will be disabled. -func (b *BrowserContext) GrantPermissions(permissions []string, opts *GrantPermissionsOptions) error { +func (b *BrowserContext) GrantPermissions(permissions []string, opts GrantPermissionsOptions) error { b.logger.Debugf("BrowserContext:GrantPermissions", "bctxid:%v", b.id) permsToProtocol := map[string]cdpbrowser.PermissionType{ diff --git a/common/browser_context_options.go b/common/browser_context_options.go index 26ba16429..32a86764c 100644 --- a/common/browser_context_options.go +++ b/common/browser_context_options.go @@ -150,23 +150,3 @@ func (w *WaitForEventOptions) Parse(ctx context.Context, optsOrPredicate sobek.V type GrantPermissionsOptions struct { Origin string } - -// NewGrantPermissionsOptions returns a new GrantPermissionsOptions. -func NewGrantPermissionsOptions() *GrantPermissionsOptions { - return &GrantPermissionsOptions{} -} - -// Parse parses the options from opts if opts exists in the sobek runtime. -func (g *GrantPermissionsOptions) Parse(ctx context.Context, opts sobek.Value) { - rt := k6ext.Runtime(ctx) - - if sobekValueExists(opts) { - opts := opts.ToObject(rt) - for _, k := range opts.Keys() { - if k == "origin" { - g.Origin = opts.Get(k).String() - break - } - } - } -} diff --git a/tests/browser_context_test.go b/tests/browser_context_test.go index 13cd437f3..c4589fbcc 100644 --- a/tests/browser_context_test.go +++ b/tests/browser_context_test.go @@ -912,7 +912,10 @@ func TestBrowserContextGrantPermissions(t *testing.T) { bCtx, err := tb.NewContext(nil) require.NoError(t, err) - err = bCtx.GrantPermissions([]string{tc.permission}, common.NewGrantPermissionsOptions()) + err = bCtx.GrantPermissions( + []string{tc.permission}, + common.GrantPermissionsOptions{}, + ) if tc.wantErr == "" { assert.NoError(t, err) @@ -968,7 +971,10 @@ func TestBrowserContextClearPermissions(t *testing.T) { require.False(t, hasPermission(tb, p, "geolocation")) - err = bCtx.GrantPermissions([]string{"geolocation"}, common.NewGrantPermissionsOptions()) + err = bCtx.GrantPermissions( + []string{"geolocation"}, + common.GrantPermissionsOptions{}, + ) require.NoError(t, err) require.True(t, hasPermission(tb, p, "geolocation"))