Cleanup old validation (#1179)

Refs #693.

internal/types/array.go

@@ -39,12 +39,6 @@ func MakeArray(capacity int) *Array {
 
 // NewArray creates an array with the given values.
 func NewArray(values ...any) (*Array, error) {
-	for i, value := range values {
-		if err := validateValue(value); err != nil {
-			return nil, fmt.Errorf("types.NewArray: index %d: %w", i, err)
-		}
-	}
-
 	return &Array{s: values}, nil
 }
 
@@ -88,22 +82,12 @@ func (a *Array) Set(index int, value any) error {
 		return fmt.Errorf("types.Array.Set: index %d is out of bounds [0-%d)", index, l)
 	}
 
-	if err := validateValue(value); err != nil {
-		return fmt.Errorf("types.Array.Set: %w", err)
-	}
-
 	a.s[index] = value
 	return nil
 }
 
 // Append appends given values to the array.
 func (a *Array) Append(values ...any) error {
-	for _, value := range values {
-		if err := validateValue(value); err != nil {
-			return fmt.Errorf("types.Array.Append: %w", err)
-		}
-	}
-
 	if a.s == nil {
 		a.s = values
 		return nil

internal/types/array_test.go

@@ -50,20 +50,6 @@ func TestArray(t *testing.T) {
 		value, err := a.Get(0)
 		assert.NoError(t, err)
 		assert.Equal(t, Null, value)
-
-		err = a.Append(42)
-		assert.EqualError(t, err, `types.Array.Append: types.validateValue: unsupported type: int (42)`)
-
-		err = a.Append(nil)
-		assert.EqualError(t, err, `types.Array.Append: types.validateValue: unsupported type: <nil> (<nil>)`)
-	})
-
-	t.Run("NewArray", func(t *testing.T) {
-		t.Parallel()
-
-		a, err := NewArray(int32(42), 42)
-		assert.Nil(t, a)
-		assert.EqualError(t, err, `types.NewArray: index 1: types.validateValue: unsupported type: int (42)`)
 	})
 
 	t.Run("DeepCopy", func(t *testing.T) {

internal/types/document.go

@@ -17,7 +17,6 @@ package types
 import (
 	"fmt"
 	"strconv"
-	"unicode/utf8"
 
 	"golang.org/x/exp/slices"
 
@@ -40,10 +39,10 @@ type Document struct {
 	keys []string
 }
 
-// ConvertDocument converts bson.Document to *types.Document and validates it.
+// ConvertDocument converts bson.Document to *types.Document.
 // It references the same data without copying it.
 //
-// TODO Remove this function.
+// TODO Remove this function: https://github.com/FerretDB/FerretDB/issues/260
 func ConvertDocument(d document) (*Document, error) {
 	if d == nil {
 		panic("types.ConvertDocument: d is nil")
@@ -54,10 +53,6 @@ func ConvertDocument(d document) (*Document, error) {
 		keys: d.Keys(),
 	}
 
-	if err := doc.validate(); err != nil {
-		return doc, fmt.Errorf("types.ConvertDocument: %w", err)
-	}
-
 	return doc, nil
 }
 
@@ -98,10 +93,6 @@ func NewDocument(pairs ...any) (*Document, error) {
 		}
 	}
 
-	if err := doc.validate(); err != nil {
-		return nil, fmt.Errorf("types.NewDocument: %w", err)
-	}
-
 	return doc, nil
 }
 
@@ -115,59 +106,6 @@ func (d *Document) DeepCopy() *Document {
 	return deepCopy(d).(*Document)
 }
 
-// isValidKey returns false if key is not a valid document field key.
-//
-// TODO That function should be removed once we have separate validation for command and data documents.
-func isValidKey(key string) bool {
-	if key == "" {
-		// TODO that should be valid only for command documents, not for data documents
-		return true
-	}
-
-	// forbid keys like $k (used by pjson representation), but allow $db (used by many commands)
-	if key[0] == '$' && len(key) <= 2 {
-		return false
-	}
-
-	return utf8.ValidString(key)
-}
-
-// validate checks if the document is valid.
-func (d *Document) validate() error {
-	if d == nil {
-		panic("types.Document.validate: d is nil")
-	}
-
-	if len(d.m) != len(d.keys) {
-		return fmt.Errorf("types.Document.validate: keys and values count mismatch: %d != %d", len(d.m), len(d.keys))
-	}
-
-	// TODO check that _id is not regex or array
-
-	prevKeys := make(map[string]struct{}, len(d.keys))
-	for _, key := range d.keys {
-		if !isValidKey(key) {
-			return fmt.Errorf("types.Document.validate: invalid key: %q", key)
-		}
-
-		value, ok := d.m[key]
-		if !ok {
-			return fmt.Errorf("types.Document.validate: key not found: %q", key)
-		}
-
-		if _, ok := prevKeys[key]; ok {
-			return fmt.Errorf("types.Document.validate: duplicate key: %q", key)
-		}
-		prevKeys[key] = struct{}{}
-
-		if err := validateValue(value); err != nil {
-			return fmt.Errorf("types.Document.validate: %w", err)
-		}
-	}
-
-	return nil
-}
-
 // Len returns the number of elements in the document.
 //
 // It returns 0 for nil Document.
@@ -216,17 +154,9 @@ func (d *Document) add(key string, value any) error {
 		return fmt.Errorf("types.Document.add: key already present: %q", key)
 	}
 
-	if !isValidKey(key) {
-		return fmt.Errorf("types.Document.add: invalid key: %q", key)
-	}
-
-	if err := validateValue(value); err != nil {
-		return fmt.Errorf("types.Document.validate: %w", err)
-	}
-
 	// update keys slice
 	if key == "_id" {
-		// TODO check that value is not regex or array
+		// TODO check that value is not regex or array: https://github.com/FerretDB/FerretDB/issues/1235
 
 		// ensure that _id is the first field
 		d.keys = slices.Insert(d.keys, 0, key)
@@ -258,17 +188,9 @@ func (d *Document) Get(key string) (any, error) {
 //
 // As a special case, _id always becomes the first key.
 func (d *Document) Set(key string, value any) error {
-	if !isValidKey(key) {
-		return fmt.Errorf("types.Document.Set: invalid key: %q", key)
-	}
-
-	if err := validateValue(value); err != nil {
-		return fmt.Errorf("types.Document.validate: %w", err)
-	}
-
 	// update keys slice
 	if key == "_id" {
-		// TODO check that value is not regex or array
+		// TODO check that value is not regex or array: https://github.com/FerretDB/FerretDB/issues/1235
 
 		// ensure that _id is the first field
 		if i := slices.Index(d.keys, key); i >= 0 {

internal/types/document_test.go

@@ -15,7 +15,6 @@
 package types
 
 import (
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -55,13 +54,6 @@ func TestDocument(t *testing.T) {
 		value, err := doc.Get("foo")
 		assert.NoError(t, err)
 		assert.Equal(t, Null, value)
-
-		err = doc.Set("bar", 42)
-		assert.EqualError(t, err, `types.Document.validate: types.validateValue: unsupported type: int (42)`)
-
-		err = doc.Set("bar", nil)
-		assert.EqualError(t, err, `types.Document.validate: types.validateValue: unsupported type: <nil> (<nil>)`)
-
 		assert.Equal(t, "foo", doc.Command())
 	})
 
@@ -71,10 +63,6 @@ func TestDocument(t *testing.T) {
 		doc, err := NewDocument(42, 42)
 		assert.Nil(t, doc)
 		assert.EqualError(t, err, `types.NewDocument: invalid key type: int`)
-
-		doc, err = NewDocument("foo", 42)
-		assert.Nil(t, doc)
-		assert.EqualError(t, err, `types.NewDocument: types.Document.validate: types.validateValue: unsupported type: int (42)`)
 	})
 
 	t.Run("DeepCopy", func(t *testing.T) {
@@ -109,73 +97,6 @@ func TestDocument(t *testing.T) {
 		assert.Equal(t, []string{"_id", "foo"}, doc.keys)
 	})
 
-	t.Run("Validate", func(t *testing.T) {
-		t.Parallel()
-
-		for _, tc := range []struct {
-			name string
-			doc  Document
-			err  error
-		}{{
-			name: "normal",
-			doc: Document{
-				keys: []string{"0"},
-				m:    map[string]any{"0": "foo"},
-			},
-		}, {
-			name: "empty",
-			doc:  Document{},
-		}, {
-			name: "different keys",
-			doc: Document{
-				keys: []string{"0"},
-				m:    map[string]any{"1": "foo"},
-			},
-			err: fmt.Errorf(`types.Document.validate: key not found: "0"`),
-		}, {
-			name: "duplicate keys",
-			doc: Document{
-				keys: []string{"0", "0"},
-				m:    map[string]any{"0": "foo"},
-			},
-			err: fmt.Errorf("types.Document.validate: keys and values count mismatch: 1 != 2"),
-		}, {
-			name: "duplicate and different keys",
-			doc: Document{
-				keys: []string{"0", "0"},
-				m:    map[string]any{"0": "foo", "1": "bar"},
-			},
-			err: fmt.Errorf(`types.Document.validate: duplicate key: "0"`),
-		}, {
-			name: "pjson keys",
-			doc: Document{
-				keys: []string{"$k"},
-				m:    map[string]any{"$k": "foo"},
-			},
-			err: fmt.Errorf(`types.Document.validate: invalid key: "$k"`),
-		}, {
-			name: "dollar keys",
-			doc: Document{
-				keys: []string{"$db"},
-				m:    map[string]any{"$db": "foo"},
-			},
-		}, {
-			name: "empty key",
-			doc: Document{
-				keys: []string{""},
-				m:    map[string]any{"": ""},
-			},
-		}} {
-			tc := tc
-			t.Run(tc.name, func(t *testing.T) {
-				t.Parallel()
-
-				err := tc.doc.validate()
-				assert.Equal(t, tc.err, err)
-			})
-		}
-	})
-
 	t.Run("SetByPath", func(t *testing.T) {
 		for _, tc := range []struct {
 			name     string

internal/types/types.go

@@ -93,44 +93,6 @@ type (
 // Null represents BSON value Null.
 var Null = NullType{}
 
-// validateValue validates value.
-//
-// TODO https://github.com/FerretDB/FerretDB/issues/260
-func validateValue(value any) error {
-	switch value := value.(type) {
-	case *Document:
-		return value.validate()
-	case *Array:
-		// It is impossible to construct invalid Array using exported function, methods, or type conversions,
-		// so no need to revalidate it.
-		return nil
-	case float64:
-		return nil
-	case string:
-		return nil
-	case Binary:
-		return nil
-	case ObjectID:
-		return nil
-	case bool:
-		return nil
-	case time.Time:
-		return nil
-	case NullType:
-		return nil
-	case Regex:
-		return nil
-	case int32:
-		return nil
-	case Timestamp:
-		return nil
-	case int64:
-		return nil
-	default:
-		return fmt.Errorf("types.validateValue: unsupported type: %[1]T (%[1]v)", value)
-	}
-}
-
 // deepCopy returns a deep copy of the given value.
 func deepCopy(value any) any {
 	if value == nil {

internal/util/testutil/equal.go

@@ -42,7 +42,7 @@ func AssertEqual[T types.Type](tb testing.TB, expected, actual T) bool {
 	return assert.Fail(tb, msg)
 }
 
-// AssertEqual asserts that two BSON slices are equal.
+// AssertEqualSlices asserts that two BSON slices are equal.
 func AssertEqualSlices[T types.Type](tb testing.TB, expected, actual []T) bool {
 	tb.Helper()
 

internal/wire/op_msg_test.go

@@ -143,10 +143,6 @@ var msgTestCases = []testCase{{
 			},
 		}},
 	},
-}, {
-	name:      "dollar_dot",
-	expectedB: testutil.MustParseDumpFile("testdata", "dollar_dot.hex"),
-	err:       `types.Document.validate: invalid key: "$."`,
 }, {
 	name:      "msg_fuzz1",
 	expectedB: testutil.MustParseDumpFile("testdata", "msg_fuzz1.hex"),

internal/wire/op_query.go

@@ -40,6 +40,7 @@ type OpQuery struct {
 
 func (query *OpQuery) msgbody() {}
 
+// readFrom composes an OpQuery from a buffered reader.
 func (query *OpQuery) readFrom(bufr *bufio.Reader) error {
 	if err := binary.Read(bufr, binary.LittleEndian, &query.Flags); err != nil {
 		return lazyerrors.Errorf("wire.OpQuery.ReadFrom (binary.Read): %w", err)