Do various small cleanups (#2561)

FerretDB · May 3, 2023 · 717f0fa · 717f0fa
1 parent 30dd842
commit 717f0fa
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 14 deletions.
diff --git a/integration/Taskfile.yml b/integration/Taskfile.yml
@@ -5,7 +5,7 @@ version: 3
 vars:
   BENCHTIME: 5s
   BENCHNAME: '{{default "." .BENCHNAME}}'
-  RACEFLAG: -race={{ne OS "windows"}}
+  RACEFLAG: -race={{and (ne OS "windows") (ne ARCH "arm") (ne ARCH "riscv64")}}
 
 tasks:
   env-data:

diff --git a/internal/handlers/common/params.go b/internal/handlers/common/params.go
@@ -25,12 +25,13 @@ import (
 	"github.com/FerretDB/FerretDB/internal/util/must"
 )
 
-// GetRequiredParam returns doc's value for key or protocol error for missing or invalid parameter.
+// GetRequiredParam returns doc's value for key
+// or protocol error for missing key or invalid type.
 func GetRequiredParam[T types.Type](doc *types.Document, key string) (T, error) {
 	var zero T
 
-	v, err := doc.Get(key)
-	if err != nil {
+	v, _ := doc.Get(key)
+	if v == nil {
 		msg := fmt.Sprintf("required parameter %q is missing", key)
 		return zero, commonerrors.NewCommandErrorMsgWithArgument(commonerrors.ErrBadValue, msg, key)
 	}
@@ -44,13 +45,15 @@ func GetRequiredParam[T types.Type](doc *types.Document, key string) (T, error)
 	return res, nil
 }
 
-// GetOptionalParam returns doc's value for key, default value for missing parameter, or protocol error for invalid parameter.
+// GetOptionalParam returns doc's value for key, default value for missing parameter,
+// or protocol error for invalid type.
 func GetOptionalParam[T types.Type](doc *types.Document, key string, defaultValue T) (T, error) {
-	v, err := doc.Get(key)
-	if err != nil {
+	v, _ := doc.Get(key)
+	if v == nil {
 		return defaultValue, nil
 	}
 
+	// require exact type; do not threat nulls as default values in this helper
 	res, ok := v.(T)
 	if !ok {
 		msg := fmt.Sprintf(
@@ -64,13 +67,27 @@ func GetOptionalParam[T types.Type](doc *types.Document, key string, defaultValu
 	return res, nil
 }
 
+// GetOptionalNullParam returns doc's value for key, default value for missing parameter or null,
+// or protocol error for other invalid type.
+func GetOptionalNullParam[T types.Type](doc *types.Document, key string, defaultValue T) (T, error) {
+	v, err := GetOptionalParam(doc, key, defaultValue)
+	if err != nil {
+		// the only possible error here is type mismatch, so the key is present
+		if _, ok := must.NotFail(doc.Get(key)).(types.NullType); ok {
+			err = nil
+		}
+	}
+
+	return v, err
+}
+
 // GetBoolOptionalParam returns doc's bool value for key.
 // Non-zero double, long, and int values return true.
 // Zero values for those types, as well as nulls and missing fields, return false.
 // Other types return a protocol error.
 func GetBoolOptionalParam(doc *types.Document, key string) (bool, error) {
-	v, err := doc.Get(key)
-	if err != nil {
+	v, _ := doc.Get(key)
+	if v == nil {
 		return false, nil
 	}
 
@@ -495,8 +512,8 @@ func multiplyLongSafely(v1, v2 int64) (int64, error) {
 
 // GetOptionalPositiveNumber returns doc's value for key or protocol error for invalid parameter.
 func GetOptionalPositiveNumber(document *types.Document, key string) (int32, error) {
-	v, err := document.Get(key)
-	if err != nil {
+	v, _ := document.Get(key)
+	if v == nil {
 		return 0, nil
 	}
 

diff --git a/internal/types/document.go b/internal/types/document.go
@@ -236,6 +236,15 @@ func (d *Document) Has(key string) bool {
 
 // Get returns a value at the given key.
 // If the key is duplicated, it panics.
+//
+// The only possible error is returned for a missing key.
+// In that case, Get returns nil for the value.
+// The new code is encouraged to do this:
+//
+//	v, _ := d.Get("key")
+//	if v == nil { ... }
+//
+// The error value will be removed in the future.
 func (d *Document) Get(key string) (any, error) {
 	if d.isKeyDuplicate(key) {
 		panic(fmt.Sprintf("types.Document.Get: key is duplicated: %s", key))

diff --git a/internal/util/iterator/count.go b/internal/util/iterator/count.go
@@ -19,7 +19,7 @@ import "errors"
 // ConsumeCount returns the number of elements in the iterator.
 // ErrIteratorDone error is returned as nil; any other error is returned as-is.
 //
-// Iterator is always closed at the end.
+// It always closes the iterator.
 func ConsumeCount[K, V any](iter Interface[K, V]) (int, error) {
 	defer iter.Close()
 

diff --git a/internal/util/iterator/values.go b/internal/util/iterator/values.go
@@ -23,7 +23,7 @@ import (
 // ConsumeValues consumes all values from iterator until it is done.
 // ErrIteratorDone error is returned as nil; any other error is returned as-is.
 //
-// Iterator is always closed at the end.
+// It always closes the iterator.
 func ConsumeValues[K, V any](iter Interface[K, V]) ([]V, error) {
 	defer iter.Close()
 

diff --git a/internal/util/must/must.go b/internal/util/must/must.go
@@ -15,6 +15,8 @@
 // Package must provides helper functions that panic on error.
 package must
 
+import "fmt"
+
 // NotFail panics if the error is not nil, returns res otherwise.
 //
 // Use that function only for static initialization, test code, or code that "can't" fail.
@@ -36,6 +38,17 @@ func NoError(err error) {
 	}
 }
 
+// NotBeZero panics if argument has zero value.
+//
+// Use that function only for static initialization, test code, or code that "can't" fail.
+// When in doubt, don't.
+func NotBeZero[T comparable](v T) {
+	var zero T
+	if v == zero {
+		panic(fmt.Sprintf("v has zero value (%#v)", v))
+	}
+}
+
 // BeTrue panic if the b is not true.
 //
 // Use that function only for static initialization, test code, or statemants that

diff --git a/tools/Taskfile.yml b/tools/Taskfile.yml
@@ -3,7 +3,7 @@
 version: 3
 
 vars:
-  RACEFLAG: -race={{ne OS "windows"}}
+  RACEFLAG: -race={{and (ne OS "windows") (ne ARCH "arm") (ne ARCH "riscv64")}}
 
 tasks:
   tools-test: