Skip to content

Commit

Permalink
findAndModify returns command error (#2646)
Browse files Browse the repository at this point in the history 
Closes #1903.
  • Loading branch information
@chilagrow
chilagrow authored May 19, 2023
1 parent faddeeb commit bd5274f
Show file tree
Hide file tree
Showing 13 changed files with 655 additions and 177 deletions.
276 changes: 273 additions & 3 deletions integration/findandmodify_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package integration

import (
"math"
"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -57,8 +58,10 @@ func TestFindAndModifyEmptyCollectionName(t *testing.T) {
func TestFindAndModifyErrors(t *testing.T) {
t.Parallel()

for name, tc := range map[string]struct {
command bson.D
for name, tc := range map[string]struct { //nolint:vet // it is used for test only
command bson.D
provider shareddata.Provider // optional, default uses shareddata.ArrayDocuments

err *mongo.CommandError
altMessage string
}{
Expand Down Expand Up @@ -122,11 +125,278 @@ func TestFindAndModifyErrors(t *testing.T) {
},
altMessage: "BSON field 'findAndModify.upsert' is the wrong type 'string', expected type 'bool'",
},
"SetUnsuitableValue": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"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: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
altMessage: "Cannot create field 'foo' in element " +
"{v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
"RenameEmptyFieldName": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"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{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"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{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"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{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"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\"",
},
"IncTypeMismatch": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$inc", bson.D{{"v", "string"}}}}},
},
err: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "Cannot increment with non-numeric argument: {v: \"string\"}",
},
},
"IncUnsuitableValue": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$inc", bson.D{{"v.foo", 1}}}}},
},
err: &mongo.CommandError{
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: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
"IncNonNumeric": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$inc", bson.D{{"v.0.foo.0.bar", 1}}}}},
},
err: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "Plan executor error during findAndModify :: caused by :: " +
"Cannot apply $inc to a value of non-numeric type. " +
"{_id: \"array-documents-nested\"} has the field 'bar' of non-numeric type string",
},
altMessage: "Cannot apply $inc to a value of non-numeric type. " +
"{_id: \"array-documents-nested\"} has the field 'bar' of non-numeric type string",
},
"IncInt64BadValue": {
command: bson.D{
{"query", bson.D{{"_id", "int64-max"}}},
{"update", bson.D{{"$inc", bson.D{{"v", math.MaxInt64}}}}},
},
err: &mongo.CommandError{
Code: 2,
Name: "BadValue",
Message: "Plan executor error during findAndModify :: caused by :: " +
"Failed to apply $inc operations to current value " +
"((NumberLong)9223372036854775807) for document {_id: \"int64-max\"}",
},
provider: shareddata.Int64s,
altMessage: "Failed to apply $inc operations to current value " +
"((NumberLong)9223372036854775807) for document {_id: \"int64-max\"}",
},
"IncInt32BadValue": {
command: bson.D{
{"query", bson.D{{"_id", "int32"}}},
{"update", bson.D{{"$inc", bson.D{{"v", math.MaxInt64}}}}},
},
err: &mongo.CommandError{
Code: 2,
Name: "BadValue",
Message: "Plan executor error during findAndModify :: caused by :: " +
"Failed to apply $inc operations to current value " +
"((NumberInt)42) for document {_id: \"int32\"}",
},
provider: shareddata.Int32s,
altMessage: "Failed to apply $inc operations to current value " +
"((NumberInt)42) for document {_id: \"int32\"}",
},
"MaxUnsuitableValue": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$max", bson.D{{"v.foo", 1}}}}},
},
err: &mongo.CommandError{
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: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
"MinUnsuitableValue": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$min", bson.D{{"v.foo", 1}}}}},
},
err: &mongo.CommandError{
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: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
"MulTypeMismatch": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$mul", bson.D{{"v", "string"}}}}},
},
err: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "Cannot multiply with non-numeric argument: {v: \"string\"}",
},
},
"MulTypeMismatchNonExistent": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$mul", bson.D{{"non-existent", "string"}}}}},
},
err: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "Cannot multiply with non-numeric argument: {non-existent: \"string\"}",
},
},
"MulUnsuitableValue": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$mul", bson.D{{"v.foo", 1}}}}},
},
err: &mongo.CommandError{
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: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
"MulNonNumeric": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$mul", bson.D{{"v.0.foo.0.bar", 1}}}}},
},
err: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "Plan executor error during findAndModify :: caused by :: " +
"Cannot apply $mul to a value of non-numeric type. " +
"{_id: \"array-documents-nested\"} has the field 'bar' of non-numeric type string",
},
altMessage: "Cannot apply $mul to a value of non-numeric type. " +
"{_id: \"array-documents-nested\"} has the field 'bar' of non-numeric type string",
},
"MulInt64BadValue": {
command: bson.D{
{"query", bson.D{{"_id", "int64-max"}}},
{"update", bson.D{{"$mul", bson.D{{"v", math.MaxInt64}}}}},
},
err: &mongo.CommandError{
Code: 2,
Name: "BadValue",
Message: "Failed to apply $mul operations to current value " +
"((NumberLong)9223372036854775807) for document {_id: \"int64-max\"}",
},
provider: shareddata.Int64s,
altMessage: "Plan executor error during findAndModify :: caused by :: " +
"Failed to apply $mul operations to current value " +
"((NumberLong)9223372036854775807) for document {_id: \"int64-max\"}",
},
"MulInt32BadValue": {
command: bson.D{
{"query", bson.D{{"_id", "int32"}}},
{"update", bson.D{{"$mul", bson.D{{"v", math.MaxInt64}}}}},
},
err: &mongo.CommandError{
Code: 2,
Name: "BadValue",
Message: "Plan executor error during findAndModify :: caused by :: " +
"Failed to apply $mul operations to current value " +
"((NumberInt)42) for document {_id: \"int32\"}",
},
provider: shareddata.Int32s,
altMessage: "Failed to apply $mul operations to current value " +
"((NumberInt)42) for document {_id: \"int32\"}",
},
"MulEmptyPath": {
command: bson.D{
{"query", bson.D{{"_id", "array-documents-nested"}}},
{"update", bson.D{{"$mul", 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.",
},
},
} {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
t.Parallel()
ctx, collection := setup.Setup(t, shareddata.DocumentsStrings)

provider := tc.provider
if provider == nil {
provider = shareddata.ArrayDocuments
}

ctx, collection := setup.Setup(t, provider)

command := bson.D{{"findAndModify", collection.Name()}}
command = append(command, tc.command...)
Expand Down
32 changes: 23 additions & 9 deletions integration/update_compat_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,13 @@ import (

// updateCompatTestCase describes update compatibility test case.
type updateCompatTestCase struct {
update bson.D // required if replace is nil
replace bson.D // required if update is nil
filter bson.D // defaults to bson.D{{"_id", id}}
resultType compatTestCaseResultType // defaults to nonEmptyResult
providers []shareddata.Provider // defaults to shareddata.AllProviders()
update bson.D // required if replace is nil
replace bson.D // required if update is nil
filter bson.D // defaults to bson.D{{"_id", id}}
updateOpts *options.UpdateOptions // defaults to nil
replaceOpts *options.ReplaceOptions // defaults to nil
resultType compatTestCaseResultType // defaults to nonEmptyResult
providers []shareddata.Provider // defaults to shareddata.AllProviders()

skip string // skips test if non-empty
skipForTigris string // skips test for Tigris if non-empty
Expand Down Expand Up @@ -108,11 +110,11 @@ func testUpdateCompat(t *testing.T, testCases map[string]updateCompatTestCase) {
// TODO replace with UpdateMany/ReplaceMany
// https://github.com/FerretDB/FerretDB/issues/1507
if update != nil {
targetUpdateRes, targetErr = targetCollection.UpdateOne(ctx, filter, update)
compatUpdateRes, compatErr = compatCollection.UpdateOne(ctx, filter, update)
targetUpdateRes, targetErr = targetCollection.UpdateOne(ctx, filter, update, tc.updateOpts)
compatUpdateRes, compatErr = compatCollection.UpdateOne(ctx, filter, update, tc.updateOpts)
} else {
targetUpdateRes, targetErr = targetCollection.ReplaceOne(ctx, filter, replace)
compatUpdateRes, compatErr = compatCollection.ReplaceOne(ctx, filter, replace)
targetUpdateRes, targetErr = targetCollection.ReplaceOne(ctx, filter, replace, tc.replaceOpts)
compatUpdateRes, compatErr = compatCollection.ReplaceOne(ctx, filter, replace, tc.replaceOpts)
}

if targetErr != nil {
Expand Down Expand Up @@ -441,6 +443,18 @@ func TestUpdateCompat(t *testing.T) {
"ReplaceEmptyDocument": {
replace: bson.D{},
},
"ReplaceNonExistentUpsert": {
filter: bson.D{{"non-existent", "no-match"}},
replace: bson.D{{"_id", "new"}},
replaceOpts: &options.ReplaceOptions{Upsert: pointer.ToBool(true)},
resultType: emptyResult,
},
"UpdateNonExistentUpsert": {
filter: bson.D{{"_id", "non-existent"}},
update: bson.D{{"$set", bson.D{{"v", int32(42)}}}},
updateOpts: &options.UpdateOptions{Upsert: pointer.ToBool(true)},
resultType: emptyResult,
},
}

testUpdateCompat(t, testCases)
Expand Down
Loading

0 comments on commit bd5274f

Please sign in to comment.