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.
grafana · Nov 5, 2024 · df900b8 · df900b8
1 parent 946e4cb
commit df900b8
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 34 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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/example-chromium.png b/example-chromium.png
diff --git 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"))