go/zodb: Handle common options in zurl in generic layer

Offload drivers from handling options such as ?read-only=1 and force
them to deal with such options only via DriverOptions, never zurl.

See added comment for details.

/reviewed-by @levin.zimmermann
/reviewed-on kirr/neo!4

go/zodb/storage.go

-// Copyright (C) 2017-2021  Nexedi SA and Contributors.
+// Copyright (C) 2017-2024  Nexedi SA and Contributors.
 //                          Kirill Smelkov <kirr@nexedi.com>
 //
 // This program is free software: you can Use, Study, Modify and Redistribute
@@ -25,11 +25,13 @@ import (
 	"fmt"
 	"log"
 	"net/url"
+	"strconv"
 	"strings"
 	"sync"
 
 	"lab.nexedi.com/kirr/go123/mem"
 	"lab.nexedi.com/kirr/go123/xcontext"
+	"lab.nexedi.com/kirr/go123/xerr"
 )
 
 // OpenOptions describes options for Open.
@@ -95,7 +97,9 @@ func RegisterDriver(scheme string, opener DriverOpener) {
 //
 // It is similar to Open but returns low-level IStorageDriver instead of IStorage.
 // Most users should use Open.
-func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorageDriver, at0 Tid, _ error) {
+func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorageDriver, at0 Tid, err error) {
+	defer xerr.Contextf(&err, "open driver %s %s", zurl, opt)
+
 	// no scheme -> file://
 	if !strings.Contains(zurl, ":") {
 		zurl = "file://" + zurl
@@ -106,10 +110,84 @@ func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorag
 		return nil, InvalidTid, err
 	}
 
-	// XXX commonly handle some options from url -> opt?
-	// (e.g. ?readonly=1 -> opt.ReadOnly=true + remove ?readonly=1 from URL)
-	// ----//---- nocache
+	// handle common options: remove them from url and merge with opt, so
+	// that drivers need to handle only driver specific things in their
+	// open routine.
+	//
+	// For example:
+	//
+	//	?read-only=1 <-> opt.ReadOnly=true + remove ?read-only=1 from URL.
+	//
+	// We make sure there is no conflict in between opt and URL as those
+	// two sources of control could be both seen as priority ones
+	// depending on situation and point of view:
+	//
+	// - when a user invokes a program with url amended via &read-only=1
+	//   he/she expects the storage to be opened in read-only mode
+	//   independently of program defaults.
+	//
+	// - when a developer invokes Open with `ReadOnly: true` the developer
+	//   also expects the storage to be opened in read-only mode
+	//   independently of zurl.
+	//
+	// So it is a conflict of expectations if there is e.g. &read-only=false
+	// in the url, and ReadOnly: true in the program. We don't try to
+	// resolve such conflicts and simply report an error.
+	//
+	// It is sometimes handy to have control over storage parameters via
+	// zurl query, but given there is possibility for above-described
+	// conflict, we do not expose all common options to be tunable via zurl.
+	// Instead we expose there only the absolute minimum - e.g. only common
+	// options that are already well established in zodburi/py.
+	q, err := url.ParseQuery(u.RawQuery)
+	if err != nil {
+		return nil, InvalidTid, err
+	}
+	type XBool struct {
+		value bool
+		ok    bool
+	}
+	boolOpt := func(name string) XBool {
+		x := XBool{}
+		if !q.Has(name) {
+			return x
+		}
+		x.ok = true
+		v := q.Get(name)
+		q.Del(name)
+		var err_ error
+		x.value, err_ = strconv.ParseBool(v)
+		if err == nil {
+			err = err_
+		}
+		return x
+	}
+
+	readonly  := boolOpt("read-only")
+	readonly_ := boolOpt("read_only")
+
+	if err != nil {
+		return nil, InvalidTid, err
+	}
+
+	if readonly.ok && readonly_.ok {
+		if readonly.value != readonly_.value {
+			return nil, InvalidTid, fmt.Errorf("conflicting values in between read-only and read_only in url")
+		}
+	}
+	if !readonly.ok {
+		readonly = readonly_
+	}
+	if readonly.ok {
+		if readonly.value != opt.ReadOnly {
+			return nil, InvalidTid, fmt.Errorf("conflicting value in between read-only in url and program options")
+		}
+	}
+
+	u.RawQuery = q.Encode()
+
 
+	// lookup the driver in registry and tail opening to the driver
 	opener, ok := driverRegistry[u.Scheme]
 	if !ok {
 		return nil, InvalidTid, fmt.Errorf("zodb: URL scheme \"%s:\" not supported", u.Scheme)
@@ -130,7 +208,9 @@ func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorag
 // get support for well-known storages.
 //
 // Storage authors should register their storages with RegisterDriver.
-func Open(ctx context.Context, zurl string, opt *OpenOptions) (IStorage, error) {
+func Open(ctx context.Context, zurl string, opt *OpenOptions) (_ IStorage, err error) {
+	defer xerr.Contextf(&err, "open %s %s", zurl, opt)
+
 	drvWatchq := make(chan Event)
 	drvOpt := &DriverOptions{
 		ReadOnly: opt.ReadOnly,
@@ -171,6 +251,37 @@ func Open(ctx context.Context, zurl string, opt *OpenOptions) (IStorage, error)
 	return stor, nil
 }
 
+// String represents OpenOptions in human-readable form.
+//
+// For example:
+//
+//	(read-only, no-cache)
+func (opt *OpenOptions) String() string {
+	v := []string{}
+	if opt.ReadOnly {
+		v = append(v, "read-only")
+	}
+	if opt.NoCache {
+		v = append(v, "no-cache")
+	}
+	return fmt.Sprintf("(%s)", strings.Join(v, ", "))
+}
+
+// String represents DriverOptions in human-readable form.
+//
+// For example
+//
+//	(read-only, watch)
+func (opt *DriverOptions) String() string {
+	v := []string{}
+	if opt.ReadOnly {
+		v = append(v, "read-only")
+	}
+	if opt.Watchq != nil {
+		v = append(v, "watch")
+	}
+	return fmt.Sprintf("(%s)", strings.Join(v, ", "))
+}
 
 
 // storage represents storage opened via Open.