Skip to content

Commit

Permalink
Add insert documents type validation (#2946)
Browse files Browse the repository at this point in the history
  • Loading branch information
Dmitry committed Jun 30, 2023
1 parent 11a2389 commit 206abe8
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 18 deletions.
30 changes: 30 additions & 0 deletions integration/insert_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,32 @@ func TestInsertCommandErrors(t *testing.T) {
},
altMessage: "E11000 duplicate key error collection: TestInsertCommandErrors-InsertDuplicateKey.TestInsertCommandErrors-InsertDuplicateKey",
},
"InsertDuplicateKeyOrdered": {
toInsert: []any{
bson.D{{"foo", "bar"}},
bson.D{{"_id", "double"}},
},
ordered: true,
werr: &mongo.WriteError{
Code: 11000,
Index: 1,
Message: `E11000 duplicate key error collection: ` +
`TestInsertCommandErrors-InsertDuplicateKeyOrdered.TestInsertCommandErrors-InsertDuplicateKeyOrdered index: _id_ dup key: { _id: "double" }`,
},
altMessage: `E11000 duplicate key error collection: TestInsertCommandErrors-InsertDuplicateKeyOrdered.TestInsertCommandErrors-InsertDuplicateKeyOrdered`,
},
"InsertArray": {
toInsert: []any{
bson.D{{"a", int32(1)}},
bson.A{},
},
ordered: true,
cerr: &mongo.CommandError{
Code: 14,
Name: "TypeMismatch",
Message: "BSON field 'insert.documents.1' is the wrong type 'array', expected type 'object'",
},
},
} {
name, tc := name, tc
t.Run(name, func(t *testing.T) {
Expand All @@ -87,11 +113,15 @@ func TestInsertCommandErrors(t *testing.T) {

if tc.cerr != nil {
AssertEqualAltCommandError(t, *tc.cerr, tc.altMessage, err)
return
}

if tc.werr != nil {
AssertEqualAltWriteError(t, *tc.werr, tc.altMessage, err)
return
}

require.NoError(t, err)
})
}
}
19 changes: 19 additions & 0 deletions internal/handlers/common/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@
package common

import (
"fmt"

"go.uber.org/zap"

"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
"github.com/FerretDB/FerretDB/internal/handlers/commonparams"
"github.com/FerretDB/FerretDB/internal/types"
"github.com/FerretDB/FerretDB/internal/util/must"
)

// InsertParams represents the parameters for an insert command.
Expand All @@ -45,5 +49,20 @@ func GetInsertParams(document *types.Document, l *zap.Logger) (*InsertParams, er
return nil, err
}

for i := 0; i < params.Docs.Len(); i++ {
doc := must.NotFail(params.Docs.Get(i))

if _, ok := doc.(*types.Document); !ok {
return nil, commonerrors.NewCommandErrorMsg(
commonerrors.ErrTypeMismatch,
fmt.Sprintf(
"BSON field 'insert.documents.%d' is the wrong type '%s', expected type 'object'",
i,
commonparams.AliasFromType(doc),
),
)
}
}

return &params, nil
}
25 changes: 7 additions & 18 deletions internal/handlers/pg/msg_insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/FerretDB/FerretDB/internal/handlers/common"
"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
"github.com/FerretDB/FerretDB/internal/handlers/commonparams"
"github.com/FerretDB/FerretDB/internal/handlers/pg/pgdb"
"github.com/FerretDB/FerretDB/internal/types"
"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
Expand Down Expand Up @@ -87,7 +86,7 @@ func insertMany(ctx context.Context, dbPool *pgdb.Pool, qp *pgdb.QueryParams, do
for i := 0; i < docs.Len(); i++ {
doc := must.NotFail(docs.Get(i))

err := insertDocument(ctx, tx, qp, doc)
err := insertDocument(ctx, tx, qp, doc.(*types.Document))
if err != nil {
return err
}
Expand All @@ -98,7 +97,7 @@ func insertMany(ctx context.Context, dbPool *pgdb.Pool, qp *pgdb.QueryParams, do
// try inserting one document at a time
if err != nil {
for i := 0; i < docs.Len(); i++ {
doc := must.NotFail(docs.Get(i))
doc := must.NotFail(docs.Get(i)).(*types.Document)

err := insertDocumentSeparately(ctx, dbPool, qp, doc)

Expand Down Expand Up @@ -126,22 +125,12 @@ func insertMany(ctx context.Context, dbPool *pgdb.Pool, qp *pgdb.QueryParams, do
}

// insertDocument prepares and executes actual INSERT request to Postgres in provided transaction.
func insertDocument(ctx context.Context, tx pgx.Tx, qp *pgdb.QueryParams, doc any) error {
d, ok := doc.(*types.Document)
if !ok {
return commonerrors.NewCommandErrorMsg(
commonerrors.ErrBadValue,
fmt.Sprintf("document has invalid type %s", commonparams.AliasFromType(doc)),
)
}

toInsert := d

if !toInsert.Has("_id") {
toInsert.Set("_id", types.NewObjectID())
func insertDocument(ctx context.Context, tx pgx.Tx, qp *pgdb.QueryParams, doc *types.Document) error {
if !doc.Has("_id") {
doc.Set("_id", types.NewObjectID())
}

err := pgdb.InsertDocument(ctx, tx, qp.DB, qp.Collection, toInsert)
err := pgdb.InsertDocument(ctx, tx, qp.DB, qp.Collection, doc)

switch {
case err == nil:
Expand All @@ -168,7 +157,7 @@ func insertDocument(ctx context.Context, tx pgx.Tx, qp *pgdb.QueryParams, doc an
// insertDocumentSeparately prepares and executes actual INSERT request to Postgres in separate transaction.
//
// It should be used in places where we don't want to rollback previous inserted documents on error.
func insertDocumentSeparately(ctx context.Context, dbPool *pgdb.Pool, qp *pgdb.QueryParams, doc any) error {
func insertDocumentSeparately(ctx context.Context, dbPool *pgdb.Pool, qp *pgdb.QueryParams, doc *types.Document) error {
return dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error {
return insertDocument(ctx, tx, qp, doc)
})
Expand Down

0 comments on commit 206abe8

Please sign in to comment.