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

Support listIndexes command #1960

Merged
merged 30 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
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
72 changes: 72 additions & 0 deletions integration/commands_administration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,3 +986,75 @@ func TestCommandsAdministrationCurrentOp(t *testing.T) {
_, ok := must.NotFail(doc.Get("inprog")).(*types.Array)
assert.True(t, ok)
}

func TestCommandsAdministrationListIndexes(t *testing.T) {
w84thesun marked this conversation as resolved.
Show resolved Hide resolved
t.Parallel()

ctx, targetCollections, compatCollections := setup.SetupCompat(t)

for i := range targetCollections {
targetCollection := targetCollections[i]
compatCollection := compatCollections[i]

t.Run(targetCollection.Name(), func(t *testing.T) {
t.Parallel()

targetCur, targetErr := targetCollection.Indexes().List(ctx)
compatCur, compatErr := compatCollection.Indexes().List(ctx)

require.NoError(t, compatErr)
assert.Equal(t, compatErr, targetErr)

targetRes := FetchAll(t, ctx, targetCur)
compatRes := FetchAll(t, ctx, compatCur)

// TODO Use simple assert.Equal after https://github.com/FerretDB/FerretDB/issues/1384
// assert.Equal(t, compatRes, targetRes)
assert.Empty(t, targetRes)
assert.NotEmpty(t, compatRes)
})
}
}

// TestCommandsAdministrationRunCommandListIndexes tests the behavior when listIndexes is called through RunCommand.
// It's handy to use it to test the correctness of errors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should test through RunCommand in general: we want compatibility with apps through drivers, not with specs/docs via RunCommand. There are some exceptions like drop that hide some errors, but they are exceptions. There is no need to over-test simple commands like listIndexes

func TestCommandsAdministrationRunCommandListIndexes(t *testing.T) {
t.Parallel()

ctx, targetCollections, compatCollections := setup.SetupCompat(t)
targetCollection := targetCollections[0]
compatCollection := compatCollections[0]

for name, tc := range map[string]struct {
collectionName any
expectedError *mongo.CommandError
}{
"non-existent-collection": {
collectionName: "non-existent-collection",
},
"invalid-collection-name": {
collectionName: 42,
},
} {
name, tc := name, tc

t.Run(name, func(t *testing.T) {
t.Parallel()

var targetRes bson.D
targetErr := targetCollection.Database().RunCommand(
ctx, bson.D{{"listIndexes", tc.collectionName}},
).Decode(&targetRes)

var compatRes bson.D
compatErr := compatCollection.Database().RunCommand(
ctx, bson.D{{"listIndexes", tc.collectionName}},
).Decode(&targetRes)

require.Nil(t, targetRes)
require.Nil(t, compatRes)

AssertMatchesCommandError(t, compatErr, targetErr)
})
}
}
69 changes: 58 additions & 11 deletions internal/handlers/pg/msg_listindexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,13 @@ package pg

import (
"context"
"fmt"

"github.com/jackc/pgx/v4"

"github.com/FerretDB/FerretDB/internal/handlers/common"
"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
"github.com/FerretDB/FerretDB/internal/handlers/pg/pgdb"
"github.com/FerretDB/FerretDB/internal/types"
"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
"github.com/FerretDB/FerretDB/internal/util/must"
Expand All @@ -26,7 +31,10 @@ import (

// MsgListIndexes implements HandlerInterface.
func (h *Handler) MsgListIndexes(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, error) {
// TODO https://github.com/FerretDB/FerretDB/issues/278
dbPool, err := h.DBPool(ctx)
if err != nil {
return nil, lazyerrors.Error(err)
}

document, err := msg.Document()
if err != nil {
Expand All @@ -35,22 +43,61 @@ func (h *Handler) MsgListIndexes(ctx context.Context, msg *wire.OpMsg) (*wire.Op

common.Ignored(document, h.L, "comment", "cursor")

firstBatch := must.NotFail(types.NewArray(
must.NotFail(types.NewDocument(
"v", float64(2),
"key", must.NotFail(types.NewDocument(
"_id", float64(1),
)),
"name", "_id_",
)),
))
var db string

if db, err = common.GetRequiredParam[string](document, "$db"); err != nil {
return nil, err
}

var collectionParam any

if collectionParam, err = document.Get(document.Command()); err != nil {
return nil, err
}

collection, ok := collectionParam.(string)
if !ok {
return nil, commonerrors.NewCommandErrorMsgWithArgument(
commonerrors.ErrBadValue,
fmt.Sprintf("collection name has invalid type %s", common.AliasFromType(collectionParam)),
document.Command(),
)
}

var exists bool

if err = dbPool.InTransactionRetry(ctx, func(tx pgx.Tx) error {
exists, err = pgdb.CollectionExists(ctx, tx, db, collection)
return err
}); err != nil {
return nil, err
}

if !exists {
return nil, commonerrors.NewCommandErrorMsg(
commonerrors.ErrNamespaceNotFound,
fmt.Sprintf("ns does not exist: %s.%s", db, collection),
)
}

// TODO Uncomment this response when we support indexes for _id: https://github.com/FerretDB/FerretDB/issues/1384.
//firstBatch := must.NotFail(types.NewArray(
// must.NotFail(types.NewDocument(
// "v", float64(2),
// "key", must.NotFail(types.NewDocument(
// "_id", float64(1),
// )),
// "name", "_id_",
// )),
//))
firstBatch := must.NotFail(types.NewArray())

var reply wire.OpMsg
must.NoError(reply.SetSections(wire.OpMsgSection{
Documents: []*types.Document{must.NotFail(types.NewDocument(
"cursor", must.NotFail(types.NewDocument(
// TODO "ns" field
"id", int64(0),
"ns", fmt.Sprintf("%s.%s", db, collection),
"firstBatch", firstBatch,
)),
"ok", float64(1),
Expand Down
62 changes: 51 additions & 11 deletions internal/handlers/tigris/msg_listindexes.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ package tigris

import (
"context"
"fmt"

"github.com/FerretDB/FerretDB/internal/handlers/common"
"github.com/FerretDB/FerretDB/internal/handlers/commonerrors"
"github.com/FerretDB/FerretDB/internal/types"
"github.com/FerretDB/FerretDB/internal/util/lazyerrors"
"github.com/FerretDB/FerretDB/internal/util/must"
Expand All @@ -26,7 +28,10 @@ import (

// MsgListIndexes implements HandlerInterface.
func (h *Handler) MsgListIndexes(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, error) {
// TODO https://github.com/FerretDB/FerretDB/issues/278
dbPool, err := h.DBPool(ctx)
if err != nil {
return nil, lazyerrors.Error(err)
}

document, err := msg.Document()
if err != nil {
Expand All @@ -35,22 +40,57 @@ func (h *Handler) MsgListIndexes(ctx context.Context, msg *wire.OpMsg) (*wire.Op

common.Ignored(document, h.L, "comment", "cursor")

firstBatch := must.NotFail(types.NewArray(
must.NotFail(types.NewDocument(
"v", float64(2),
"key", must.NotFail(types.NewDocument(
"_id", float64(1),
)),
"name", "_id_",
)),
))
var db string

if db, err = common.GetRequiredParam[string](document, "$db"); err != nil {
return nil, err
}

var collectionParam any

if collectionParam, err = document.Get(document.Command()); err != nil {
return nil, err
}

collection, ok := collectionParam.(string)
if !ok {
return nil, commonerrors.NewCommandErrorMsgWithArgument(
commonerrors.ErrBadValue,
fmt.Sprintf("collection name has invalid type %s", common.AliasFromType(collectionParam)),
document.Command(),
)
}

exists, err := dbPool.CollectionExists(ctx, db, collection)
if err != nil {
return nil, lazyerrors.Error(err)
}

if !exists {
return nil, commonerrors.NewCommandErrorMsg(
commonerrors.ErrNamespaceNotFound,
fmt.Sprintf("ns does not exist: %s.%s", db, collection),
)
}

// TODO Uncomment this response when we support indexes for _id: https://github.com/FerretDB/FerretDB/issues/1384.
//firstBatch := must.NotFail(types.NewArray(
// must.NotFail(types.NewDocument(
// "v", float64(2),
// "key", must.NotFail(types.NewDocument(
// "_id", float64(1),
// )),
// "name", "_id_",
// )),
//))
firstBatch := must.NotFail(types.NewArray())

var reply wire.OpMsg
must.NoError(reply.SetSections(wire.OpMsgSection{
Documents: []*types.Document{must.NotFail(types.NewDocument(
"cursor", must.NotFail(types.NewDocument(
// TODO "ns" field
"id", int64(0),
"ns", fmt.Sprintf("%s.%s", db, collection),
"firstBatch", firstBatch,
)),
"ok", float64(1),
Expand Down
6 changes: 3 additions & 3 deletions internal/handlers/tigris/tigrisdb/collections.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func (tdb *TigrisDB) CreateCollectionIfNotExist(ctx context.Context, db, collect
return false, lazyerrors.Error(err)
}

exists, err := tdb.collectionExists(ctx, db, collection)
exists, err := tdb.CollectionExists(ctx, db, collection)
if err != nil {
return false, lazyerrors.Error(err)
}
Expand Down Expand Up @@ -64,8 +64,8 @@ func (tdb *TigrisDB) CreateCollectionIfNotExist(ctx context.Context, db, collect
}
}

// collectionExists returns true if collection exists.
func (tdb *TigrisDB) collectionExists(ctx context.Context, db, collection string) (bool, error) {
// CollectionExists returns true if collection exists.
func (tdb *TigrisDB) CollectionExists(ctx context.Context, db, collection string) (bool, error) {
_, err := tdb.Driver.UseDatabase(db).DescribeCollection(ctx, collection)
switch err := err.(type) {
case nil:
Expand Down
2 changes: 1 addition & 1 deletion internal/handlers/tigris/tigrisdb/insert.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func (tdb *TigrisDB) InsertManyDocuments(ctx context.Context, db, collection str
return nil
}

if ok, _ := tdb.collectionExists(ctx, db, collection); !ok {
if ok, _ := tdb.CollectionExists(ctx, db, collection); !ok {
doc := must.NotFail(docs.Get(0)).(*types.Document)

schema, err := tjson.DocumentSchema(doc)
Expand Down
8 changes: 4 additions & 4 deletions website/docs/reference/supported_commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -654,10 +654,10 @@ db.aggregate()
| | `nameOnly` | | ✅ | |
| | `authorizedDatabases` | | ⚠️ | Ingored |
| | `comment` | | ⚠️ | Ingored |
| `listIndexes` | | | | [Issue](https://github.com/FerretDB/FerretDB/issues/278) |
| | `cursor.batchSize` | | ⚠️ | |
| | `comment` | | ⚠️ | |
| `logRotate` | | | ❌ | [Issue](https://github.com/FerretDB/FerretDB/issues/278) |
| `listIndexes` | | | | |
| | `cursor.batchSize` | | ⚠️ | Ignored |
| | `comment` | | ⚠️ | Ignored |
| `logRotate` | | | ❌ | [Issue](https://github.com/FerretDB/FerretDB/issues/1959) |
| | `<target>` | | ⚠️ | |
| | `comment` | | ⚠️ | |
| `reIndex` | | | ❌ | [Issue](https://github.com/FerretDB/FerretDB/issues/1516) |
Expand Down