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.

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
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
mul returns command error for findAndModify
  • Loading branch information
chilagrow committed May 17, 2023
commit 5a1691febfab493837803d91c575dcdd08120b4e
95 changes: 95 additions & 0 deletions integration/findandmodify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,101 @@ func TestFindAndModifyErrors(t *testing.T) {
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) {
Expand Down
62 changes: 62 additions & 0 deletions integration/update_field_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,68 @@ func TestUpdateFieldErrors(t *testing.T) {
"{v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
},
"MulTypeMismatch": {
id: "array-documents-nested",
update: bson.D{{"$mul", bson.D{{"v", "string"}}}},
err: &mongo.WriteError{
Code: 14,
Message: "Cannot multiply with non-numeric argument: {v: \"string\"}",
},
},
"MulTypeMismatchNonExistent": {
id: "array-documents-nested",
update: bson.D{{"$mul", bson.D{{"non-existent", "string"}}}},
err: &mongo.WriteError{
Code: 14,
Message: "Cannot multiply with non-numeric argument: {non-existent: \"string\"}",
},
},
"MulUnsuitableValue": {
id: "array-documents-nested",
update: bson.D{{"$mul", bson.D{{"v.foo", 1}}}},
err: &mongo.WriteError{
Code: 28,
Message: "Cannot create field 'foo' in element " +
"{v: [ { foo: [ { bar: \"hello\" }, { bar: \"world\" } ] } ]}",
},
},
"MulNonNumeric": {
id: "array-documents-nested",
update: bson.D{{"$mul", bson.D{{"v.0.foo.0.bar", 1}}}},
err: &mongo.WriteError{
Code: 14,
Message: "Cannot apply $mul to a value of non-numeric type. " +
"{_id: \"array-documents-nested\"} has the field 'bar' of non-numeric type string",
},
},
"MulInt64BadValue": {
id: "int64-max",
update: bson.D{{"$mul", bson.D{{"v", math.MaxInt64}}}},
err: &mongo.WriteError{
Code: 2,
Message: "Failed to apply $mul operations to current value " +
"((NumberLong)9223372036854775807) for document {_id: \"int64-max\"}",
},
provider: shareddata.Int64s,
},
"MulInt32BadValue": {
id: "int32",
update: bson.D{{"$mul", bson.D{{"v", math.MaxInt64}}}},
err: &mongo.WriteError{
Code: 2,
Message: "Failed to apply $mul operations to current value " +
"((NumberInt)42) for document {_id: \"int32\"}",
},
provider: shareddata.Int32s,
},
"MulEmptyPath": {
id: "array-documents-nested",
update: bson.D{{"$mul", bson.D{{"v.", "v"}}}},
err: &mongo.WriteError{
Code: 56,
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) {
Expand Down
6 changes: 5 additions & 1 deletion internal/handlers/common/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,8 +388,12 @@ func multiplyNumbers(v1, v2 any) (any, error) {
case float64:
return float64(v1) * v2, nil
case int32:
return multiplyLongSafely(v1, int64(v2))
v, err := multiplyLongSafely(v1, int64(v2))
if err != nil {
return 0, commonparams.ErrIntExceeded
}

return v, nil
case int64:
return multiplyLongSafely(v1, v2)

Expand Down
50 changes: 20 additions & 30 deletions internal/handlers/common/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func UpdateDocument(command string, doc, update *types.Document) (bool, error) {
case "$mul":
var mulChanged bool

if mulChanged, err = processMulFieldExpression(doc, updateV); err != nil {
if mulChanged, err = processMulFieldExpression(command, doc, updateV); err != nil {
return false, err
}

Expand Down Expand Up @@ -566,17 +566,9 @@ func processMinFieldExpression(command string, doc *types.Document, updateV any)

// processMulFieldExpression updates document according to $mul operator.
// If the document was changed it returns true.
func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {
mulDoc, ok := updateV.(*types.Document)
if !ok {
return false, commonerrors.NewWriteErrorMsg(
commonerrors.ErrFailedToParse,
fmt.Sprintf(`Modifiers operate on fields but we found type %[1]s instead. `+
`For example: {$mod: {<field>: ...}} not {$rename: %[1]s}`,
commonparams.AliasFromType(updateV),
),
)
}
func processMulFieldExpression(command string, doc *types.Document, updateV any) (bool, error) {
// updateV is document, checked in ValidateUpdateOperators.
mulDoc := updateV.(*types.Document)

var changed bool

Expand All @@ -588,14 +580,8 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {

path, err = types.NewPathFromString(mulKey)
if err != nil {
return false, commonerrors.NewWriteErrorMsg(
commonerrors.ErrEmptyName,
fmt.Sprintf("Cannot apply $mul to a value of non-numeric type. "+
"{_id: %s} has the field '%s' of non-numeric type object",
must.NotFail(doc.Get("_id")),
mulKey,
),
)
// ValidateUpdateOperators checked already $mul contains valid path.
panic(err)
}

if !doc.HasByPath(path) {
Expand All @@ -608,17 +594,19 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {
case int64:
mulValue = int64(0)
default:
return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrTypeMismatch,
fmt.Sprintf(`Cannot multiply with non-numeric argument: {%s: %#v}`, mulKey, mulValue),
command,
)
}

err := doc.SetByPath(path, mulValue)
if err != nil {
return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrUnsuitableValueType,
err.Error(),
command,
)
}

Expand Down Expand Up @@ -648,10 +636,8 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {

err = doc.SetByPath(path, multiplied)
if err != nil {
return false, commonerrors.NewWriteErrorMsg(
commonerrors.ErrUnsuitableValueType,
fmt.Sprintf(`Cannot create field in element {%s: %v}`, path.Prefix(), docValue),
)
// after successfully getting value from path, setting it back cannot fail.
panic(err)
}

// A change from int32(0) to int64(0) is considered changed.
Expand All @@ -666,21 +652,22 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {
continue

case errors.Is(err, commonparams.ErrUnexpectedLeftOpType):
return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrTypeMismatch,
fmt.Sprintf(
`Cannot multiply with non-numeric argument: {%s: %#v}`,
mulKey,
mulValue,
),
command,
)
case errors.Is(err, commonparams.ErrUnexpectedRightOpType):
k := mulKey
if path.Len() > 1 {
k = path.Suffix()
}

return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrTypeMismatch,
fmt.Sprintf(
`Cannot apply $mul to a value of non-numeric type. `+
Expand All @@ -689,24 +676,27 @@ func processMulFieldExpression(doc *types.Document, updateV any) (bool, error) {
k,
commonparams.AliasFromType(docValue),
),
command,
)
case errors.Is(err, commonparams.ErrLongExceededPositive), errors.Is(err, commonparams.ErrLongExceededNegative):
return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrBadValue,
fmt.Sprintf(
`Failed to apply $mul operations to current value ((NumberLong)%d) for document {_id: "%s"}`,
docValue,
must.NotFail(doc.Get("_id")),
),
command,
)
case errors.Is(err, commonparams.ErrIntExceeded):
return false, commonerrors.NewWriteErrorMsg(
return false, newUpdateError(
commonerrors.ErrBadValue,
fmt.Sprintf(
`Failed to apply $mul operations to current value ((NumberInt)%d) for document {_id: "%s"}`,
docValue,
must.NotFail(doc.Get("_id")),
),
command,
)
default:
return false, err
Expand Down