From 6bcbb03f49312085b88664942839f03531138292 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 | 30 +++++++++++++++++++++---- 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, 41 insertions(+), 32 deletions(-) diff --git a/browser/browser_context_mapping.go b/browser/browser_context_mapping.go index 5ba69b95a..6e0548269 100644 --- a/browser/browser_context_mapping.go +++ b/browser/browser_context_mapping.go @@ -1,6 +1,7 @@ package browser import ( + "context" "errors" "fmt" "reflect" @@ -76,10 +77,11 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin }, "grantPermissions": func(permissions []string, opts sobek.Value) *sobek.Promise { return k6ext.Promise(vu.Context(), func() (any, error) { - popts := common.NewGrantPermissionsOptions() - popts.Parse(vu.Context(), opts) - - return nil, bc.GrantPermissions(permissions, popts) //nolint:wrapcheck + popts, err := parseGrantPermissionOptions(vu.Context(), opts) + if err != nil { + return nil, fmt.Errorf("parsing grant permission options: %w", err) + } + return nil, bc.GrantPermissions(permissions, popts) }) }, "setDefaultNavigationTimeout": bc.SetDefaultNavigationTimeout, @@ -185,3 +187,23 @@ func mapBrowserContext(vu moduleVU, bc *common.BrowserContext) mapping { //nolin }, } } + +// parseGrantPermissionOptions parses the options for the browserContext.grantPermissions API. +func parseGrantPermissionOptions(ctx context.Context, opts sobek.Value) (common.GrantPermissionsOptions, error) { + if !sobekValueExists(opts) { + return common.GrantPermissionsOptions{}, nil + } + + var g common.GrantPermissionsOptions + + o := opts.ToObject(k6ext.Runtime(ctx)) + 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..93c7708d0 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.Context(), 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"))