returns command error for findAndModify

FerretDB · chilagrow · May 19, 2023 · May 16, 2023 · May 17, 2023 · May 17, 2023
commit 833d344a66917a8ee70a4fedf4686eb5b00899fc
@@ -24,7 +24,6 @@ import (
 
 	"github.com/FerretDB/FerretDB/integration/setup"
 	"github.com/FerretDB/FerretDB/integration/shareddata"
-	"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
 )
 
 func TestFindAndModifyEmptyCollectionName(t *testing.T) {
@@ -140,20 +139,67 @@ func TestFindAndModifyErrors(t *testing.T) {
 		},
 		"SetUnsuitableValueType": {
 			command: bson.D{
-				{"update", bson.D{{"$set", bson.D{{"v.v.foo", "foo"}}}}},
+				{"update", bson.D{{"$set", bson.D{{"v.foo", "foo"}}}}},
 			},
 			err: &mongo.CommandError{
-				Code:    28,
-				Name:    "PathNotViable",
-				Message: "Plan executor error during findAndModify :: caused by :: Cannot create field 'foo' in element {v: \"foo\"}",
+				Code: 28,
+				Name: "PathNotViable",
+				Message: "Plan executor error during findAndModify :: caused by :: Cannot create field 'foo' " +
+					"in element {v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
 			},
-			altMessage: "Cannot create field 'foo' in element {v: { v: \"foo\" }}",
+			altMessage: "Cannot create field 'foo' in element " +
+				"{v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
+		},
+		"RenameEmptyFieldName": {
+			command: bson.D{
+				{"update", bson.D{{"$rename", bson.D{{"", "v"}}}}},
+			},
+			err: &mongo.CommandError{
+				Code:    56,
+				Name:    "EmptyFieldName",
+				Message: "An empty update path is not valid.",
+			},
+		},
+		"RenameEmptyPath": {
+			command: bson.D{
+				{"update", bson.D{{"$rename", bson.D{{"v.", "v"}}}}},
+			},
+			err: &mongo.CommandError{
+				Code:    56,
+				Name:    "EmptyFieldName",
+				Message: "The update path 'v.' contains an empty field name, which is not allowed.",
+			},
+		},
+		"RenameArrayInvalidIndex": {
+			command: bson.D{
+				{"update", bson.D{{"$rename", bson.D{{"v.-1", "f"}}}}},
+			},
+			err: &mongo.CommandError{
+				Code: 28,
+				Name: "PathNotViable",
+				Message: "Plan executor error during findAndModify :: caused by :: " +
+					"cannot use the part (v of v.-1) to traverse the element " +
+					"({v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]})",
+			},
+			altMessage: "cannot use path 'v.-1' to traverse the document",
+		},
+		"RenameUnsuitableValue": {
+			command: bson.D{
+				{"update", bson.D{{"$rename", bson.D{{"v.0.foo.0.bar.z", "f"}}}}},
+			},
+			err: &mongo.CommandError{
+				Code: 28,
+				Name: "PathNotViable",
+				Message: "Plan executor error during findAndModify :: caused by :: " +
+					"cannot use the part (bar of v.0.foo.0.bar.z) to traverse the element ({bar: \"hello\"})",
+			},
+			altMessage: "types.getByPath: can't access string by path \"z\"",
 		},
 	} {
 		name, tc := name, tc
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			ctx, collection := setup.Setup(t, shareddata.DocumentsStrings)
+			ctx, collection := setup.Setup(t, shareddata.ArrayDocuments)
 
 			findAndModify := "findAndModify"
 			if tc.findAndModify != "" {

@@ -81,3 +81,71 @@ func TestUpdateFieldSet(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateFieldErrors(t *testing.T) {
+	setup.SkipForTigris(t)
+
+	t.Parallel()
+
+	for name, tc := range map[string]struct {
+		id         string
+		update     bson.D
+		err        *mongo.WriteError
+		altMessage string
+	}{
+		"SetUnsuitableValueType": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$rename", bson.D{{"v.foo", "foo"}}}},
+			err: &mongo.WriteError{
+				Code: 28,
+				Message: "cannot use the part (v of v.foo) to traverse the element " +
+					"({v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]})",
+			},
+			altMessage: "cannot use path 'v.foo' to traverse the document",
+		},
+		"RenameEmptyFieldName": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$rename", bson.D{{"", "v"}}}},
+			err: &mongo.WriteError{
+				Code:    56,
+				Message: "An empty update path is not valid.",
+			},
+		},
+		"RenameEmptyPath": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$rename", bson.D{{"v.", "v"}}}},
+			err: &mongo.WriteError{
+				Code:    56,
+				Message: "The update path 'v.' contains an empty field name, which is not allowed.",
+			},
+		},
+		"RenameArrayInvalidIndex": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$rename", bson.D{{"v.-1", "f"}}}},
+			err: &mongo.WriteError{
+				Code: 28,
+				Message: "cannot use the part (v of v.-1) to traverse the element " +
+					"({v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]})",
+			},
+			altMessage: "cannot use path 'v.-1' to traverse the document",
+		},
+		"RenameUnsuitableValue": {
+			id:     "array-documents-nested",
+			update: bson.D{{"$rename", bson.D{{"v.0.foo.0.bar.z", "f"}}}},
+			err: &mongo.WriteError{
+				Code:    28,
+				Message: "cannot use the part (bar of v.0.foo.0.bar.z) to traverse the element ({bar: \"hello\"})",
+			},
+			altMessage: "types.getByPath: can't access string by path \"z\"",
+		},
+	} {
+		name, tc := name, tc
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			ctx, collection := setup.Setup(t, shareddata.ArrayDocuments)
+
+			_, err := collection.UpdateOne(ctx, bson.D{{"_id", tc.id}}, tc.update)
+			AssertEqualAltWriteError(t, *tc.err, tc.altMessage, err)
+		})
+	}
+}
@@ -128,7 +128,7 @@ func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
 			changed = changed || mulChanged
 
 		case "$rename":
-			changed, err = processRenameFieldExpression(doc, updateV.(*types.Document))
+			changed, err = processRenameFieldExpression(command, doc, updateV.(*types.Document))
 			if err != nil {
 				return false, err
 			}
@@ -241,7 +241,7 @@ func processSetFieldExpression(command string, doc, setDoc *types.Document, setO
 
 // processRenameFieldExpression changes document according to $rename operator.
 // If the document was changed it returns true.
-func processRenameFieldExpression(doc *types.Document, update *types.Document) (bool, error) {
+func processRenameFieldExpression(command string, doc *types.Document, update *types.Document) (bool, error) {
 	update.SortFieldsByKey()
 
 	var changed bool
@@ -250,7 +250,11 @@ func processRenameFieldExpression(doc *types.Document, update *types.Document) (
 		renameRawValue := must.NotFail(update.Get(key))
 
 		if key == "" || renameRawValue == "" {
-			return changed, commonerrors.NewWriteErrorMsg(commonerrors.ErrEmptyName, "An empty update path is not valid.")
+			return changed, newUpdateError(
+				commonerrors.ErrEmptyName,
+				"An empty update path is not valid.",
+				command,
+			)
 		}
 
 		// this is covered in validateRenameExpression
@@ -260,13 +264,13 @@ func processRenameFieldExpression(doc *types.Document, update *types.Document) (
 		if err != nil {
 			var pathErr *types.DocumentPathError
 			if errors.As(err, &pathErr) && pathErr.Code() == types.ErrDocumentPathEmptyKey {
-				return false, commonerrors.NewWriteErrorMsg(
+				return false, newUpdateError(
 					commonerrors.ErrEmptyName,
-					fmt.Sprintf("Cannot apply $rename to a value of non-numeric type. "+
-						"{_id: %s} has the field '%s' of non-numeric type object",
-						must.NotFail(doc.Get("_id")),
+					fmt.Sprintf(
+						"The update path '%s' contains an empty field name, which is not allowed.",
 						key,
 					),
+					command,
 				)
 			}
 		}
@@ -289,13 +293,14 @@ func processRenameFieldExpression(doc *types.Document, update *types.Document) (
 			}
 
 			if dpe.Code() == types.ErrDocumentPathArrayInvalidIndex {
-				return false, commonerrors.NewWriteErrorMsg(
+				return false, newUpdateError(
 					commonerrors.ErrUnsuitableValueType,
-					fmt.Sprintf("cannot use path %s to traverse the document", sourcePath),
+					fmt.Sprintf("cannot use path '%s' to traverse the document", sourcePath),
+					command,
 				)
 			}
 
-			return changed, commonerrors.NewWriteErrorMsg(commonerrors.ErrUnsuitableValueType, dpe.Error())
+			return changed, newUpdateError(commonerrors.ErrUnsuitableValueType, dpe.Error(), command)
 		}
 
 		// Remove old document

@@ -72,7 +72,7 @@ const (
 	ErrInvalidID = ErrorCode(53) // InvalidID
 
 	// ErrEmptyName indicates that the field name is empty.
-	ErrEmptyName = ErrorCode(56) // EmptyName
+	ErrEmptyName = ErrorCode(56) // EmptyFieldName
 
 	// ErrCommandNotFound indicates unknown command input.
 	ErrCommandNotFound = ErrorCode(59) // CommandNotFound