Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return command error from findAndModify #2646

Merged
merged 17 commits into from
May 19, 2023
Merged

Return command error from findAndModify #2646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7ed9193
lower case findandmodify works
chilagrow May 16, 2023
cafc324
set returns command error for findAndModify
chilagrow May 17, 2023
833d344
returns command error for findAndModify
chilagrow May 17, 2023
9bd8961
merge from main
chilagrow May 17, 2023
7c24d6c
inc returns command error for findAndModify
chilagrow May 17, 2023
0fe2e5f
max returns command error for findAndModify
chilagrow May 17, 2023
1a34783
min returns command error for findAndModify
chilagrow May 17, 2023
5a1691f
mul returns command error for findAndModify
chilagrow May 17, 2023
ac24108
clean up
chilagrow May 17, 2023
6860223
add comments
chilagrow May 17, 2023
f38ee13
Merge branch 'main' into issue-1903-findandmodify-error
AlekSi May 17, 2023
4c3e926
remove js driver specific test
chilagrow May 18, 2023
9222059
remove unused code
chilagrow May 18, 2023
c005d0a
add uncovered test case for update with upsert
chilagrow May 19, 2023
748d3af
use compat when we can
chilagrow May 19, 2023
e05dbd0
fix non compiling code
chilagrow May 19, 2023
2106ffa
Merge branch 'main' into issue-1903-findandmodify-error
May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
276 changes: 273 additions & 3 deletions integration/findandmodify_test.go
View file
Open in desktop
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
Open in desktop
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