From 708d712b72de90812cb052b2c116a17922850407 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Mon, 26 Jan 2026 15:36:58 +0800 Subject: [PATCH 1/4] feat: implement high-ROI milestone 3 administrative operations Add support for 7 commonly-used MongoDB administrative methods: Index Management: - db.collection.createIndex(keys, options?) - db.collection.dropIndex(index) - db.collection.dropIndexes() Collection Management: - db.collection.drop() - db.collection.renameCollection(newName, dropTarget?) Database Management: - db.createCollection(name) - db.dropDatabase() Lower-ROI methods (stats, serverStatus, etc.) remain as planned operations with PlannedOperationError for fallback to mongosh. Co-Authored-By: Claude Opus 4.5 --- admin_test.go | 262 +++++++++++++++++++++++ error_test.go | 16 +- internal/executor/admin.go | 262 +++++++++++++++++++++++ internal/executor/executor.go | 15 ++ internal/testutil/container.go | 16 ++ internal/translator/collection.go | 281 +++++++++++++++++++++++++ internal/translator/database.go | 55 +++++ internal/translator/method_registry.go | 18 +- internal/translator/types.go | 21 ++ internal/translator/visitor.go | 25 ++- 10 files changed, 944 insertions(+), 27 deletions(-) create mode 100644 admin_test.go create mode 100644 internal/executor/admin.go diff --git a/admin_test.go b/admin_test.go new file mode 100644 index 0000000..8d88a65 --- /dev/null +++ b/admin_test.go @@ -0,0 +1,262 @@ +package gomongo_test + +import ( + "context" + "fmt" + "slices" + "testing" + + "github.com/bytebase/gomongo" + "github.com/bytebase/gomongo/internal/testutil" + "github.com/stretchr/testify/require" + "go.mongodb.org/mongo-driver/v2/bson" +) + +func TestCreateIndex(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection first + _, err := db.Client.Database(dbName).Collection("users").InsertOne(ctx, bson.M{"name": "alice"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create an index on the 'name' field + result, err := gc.Execute(ctx, dbName, `db.users.createIndex({ name: 1 })`) + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, 1, result.RowCount) + require.Contains(t, result.Rows[0], "name") + }) +} + +func TestCreateIndexWithOptions(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_opts_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection first + _, err := db.Client.Database(dbName).Collection("users").InsertOne(ctx, bson.M{"email": "alice@example.com"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create a unique index with a custom name + result, err := gc.Execute(ctx, dbName, `db.users.createIndex({ email: 1 }, { name: "email_unique_idx" })`) + require.NoError(t, err) + require.NotNil(t, result) + require.Equal(t, 1, result.RowCount) + require.Equal(t, "email_unique_idx", result.Rows[0]) + }) +} + +func TestDropIndex(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_drop_idx_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection with an index + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertOne(ctx, bson.M{"name": "alice"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create an index first + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ name: 1 }, { name: "name_idx" })`) + require.NoError(t, err) + + // Drop the index by name + result, err := gc.Execute(ctx, dbName, `db.users.dropIndex("name_idx")`) + require.NoError(t, err) + require.NotNil(t, result) + require.Contains(t, result.Rows[0], `"ok": 1`) + }) +} + +func TestDropIndexes(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_drop_idxs_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection with indexes + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertOne(ctx, bson.M{"name": "alice", "email": "alice@example.com"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create some indexes + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ name: 1 })`) + require.NoError(t, err) + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ email: 1 })`) + require.NoError(t, err) + + // Drop all indexes + result, err := gc.Execute(ctx, dbName, `db.users.dropIndexes()`) + require.NoError(t, err) + require.NotNil(t, result) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify only _id index remains + idxResult, err := gc.Execute(ctx, dbName, `db.users.getIndexes()`) + require.NoError(t, err) + require.Equal(t, 1, idxResult.RowCount) // Only _id index + }) +} + +func TestDropCollection(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_drop_coll_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection + _, err := db.Client.Database(dbName).Collection("tobedeleted").InsertOne(ctx, bson.M{"x": 1}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Verify collection exists + result, err := gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 1, result.RowCount) + + // Drop the collection + result, err = gc.Execute(ctx, dbName, `db.tobedeleted.drop()`) + require.NoError(t, err) + require.Equal(t, "true", result.Rows[0]) + + // Verify collection is gone + result, err = gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 0, result.RowCount) + }) +} + +func TestCreateCollection(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_coll_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) + + // Create a new collection + result, err := gc.Execute(ctx, dbName, `db.createCollection("newcollection")`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify collection exists + collResult, err := gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 1, collResult.RowCount) + require.Equal(t, "newcollection", collResult.Rows[0]) + }) +} + +func TestDropDatabase(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_drop_db_%s", db.Name) + // No defer cleanup since we're dropping the database + + ctx := context.Background() + + // Create a database by inserting a document + _, err := db.Client.Database(dbName).Collection("test").InsertOne(ctx, bson.M{"x": 1}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Verify database exists + result, err := gc.Execute(ctx, dbName, `show dbs`) + require.NoError(t, err) + require.True(t, slices.Contains(result.Rows, dbName), "database should exist before drop") + + // Drop the database + result, err = gc.Execute(ctx, dbName, `db.dropDatabase()`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + require.Contains(t, result.Rows[0], fmt.Sprintf(`"dropped": "%s"`, dbName)) + }) +} + +func TestRenameCollection(t *testing.T) { + testutil.RunOnMongoDBOnly(t, func(t *testing.T, db testutil.TestDB) { + // renameCollection requires admin privileges and may not work on all DB types + dbName := fmt.Sprintf("testdb_rename_coll_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection with documents + _, err := db.Client.Database(dbName).Collection("oldname").InsertOne(ctx, bson.M{"x": 1}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Rename the collection + result, err := gc.Execute(ctx, dbName, `db.oldname.renameCollection("newname")`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify old collection is gone and new one exists + collResult, err := gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 1, collResult.RowCount) + require.Equal(t, "newname", collResult.Rows[0]) + + // Verify data is preserved + findResult, err := gc.Execute(ctx, dbName, `db.newname.find()`) + require.NoError(t, err) + require.Equal(t, 1, findResult.RowCount) + require.Contains(t, findResult.Rows[0], `"x": 1`) + }) +} + +func TestRenameCollectionWithDropTarget(t *testing.T) { + testutil.RunOnMongoDBOnly(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_rename_drop_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create source collection + _, err := db.Client.Database(dbName).Collection("source").InsertOne(ctx, bson.M{"x": 1}) + require.NoError(t, err) + + // Create target collection that will be dropped + _, err = db.Client.Database(dbName).Collection("target").InsertOne(ctx, bson.M{"y": 2}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Rename with dropTarget = true + result, err := gc.Execute(ctx, dbName, `db.source.renameCollection("target", true)`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify only target exists with source data + collResult, err := gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 1, collResult.RowCount) + require.Equal(t, "target", collResult.Rows[0]) + + // Verify it has source data, not old target data + findResult, err := gc.Execute(ctx, dbName, `db.target.find()`) + require.NoError(t, err) + require.Equal(t, 1, findResult.RowCount) + require.Contains(t, findResult.Rows[0], `"x": 1`) + }) +} diff --git a/error_test.go b/error_test.go index 3675b02..7ee64bc 100644 --- a/error_test.go +++ b/error_test.go @@ -34,13 +34,14 @@ func TestPlannedOperation(t *testing.T) { gc := gomongo.NewClient(db.Client) ctx := context.Background() - // createIndex is a planned M3 operation - should return PlannedOperationError - _, err := gc.Execute(ctx, dbName, "db.users.createIndex({ name: 1 })") + // createIndexes is a planned M3 operation - should return PlannedOperationError + // (createIndex is now implemented, so we use createIndexes instead) + _, err := gc.Execute(ctx, dbName, "db.users.createIndexes([{ key: { name: 1 } }])") require.Error(t, err) var plannedErr *gomongo.PlannedOperationError require.ErrorAs(t, err, &plannedErr) - require.Equal(t, "createIndex()", plannedErr.Operation) + require.Equal(t, "createIndexes()", plannedErr.Operation) }) } @@ -83,9 +84,12 @@ func TestUnsupportedOptionError(t *testing.T) { func TestMethodRegistryStats(t *testing.T) { total := gomongo.MethodRegistryStats() - // Registry should contain M3 (22) planned methods - // M2 write operations have been implemented and removed from the registry - require.Equal(t, 22, total, "expected 22 planned methods in registry (M3: 22)") + // Registry should contain 15 planned methods after M3 high-ROI implementations + // M3 high-ROI methods implemented (removed from registry): + // - createIndex, dropIndex, dropIndexes (index management: 3) + // - drop, createCollection, dropDatabase, renameCollection (collection management: 4) + // M3 remaining planned methods: 15 (originally 22) + require.Equal(t, 15, total, "expected 15 planned methods in registry (M3 remaining)") // Log stats for visibility t.Logf("Method Registry Stats: total=%d planned methods", total) diff --git a/internal/executor/admin.go b/internal/executor/admin.go new file mode 100644 index 0000000..f082b4e --- /dev/null +++ b/internal/executor/admin.go @@ -0,0 +1,262 @@ +package executor + +import ( + "context" + "fmt" + + "github.com/bytebase/gomongo/internal/translator" + "go.mongodb.org/mongo-driver/v2/bson" + "go.mongodb.org/mongo-driver/v2/mongo" + "go.mongodb.org/mongo-driver/v2/mongo/options" +) + +// executeCreateIndex executes a db.collection.createIndex() command. +func executeCreateIndex(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + collection := client.Database(database).Collection(op.Collection) + + indexModel := mongo.IndexModel{ + Keys: op.IndexKeys, + } + + // Build index options from the stored options + if op.IndexName != "" { + opts := options.Index().SetName(op.IndexName) + indexModel.Options = opts + } + + indexName, err := collection.Indexes().CreateOne(ctx, indexModel) + if err != nil { + return nil, fmt.Errorf("createIndex failed: %w", err) + } + + return &Result{ + Rows: []string{indexName}, + RowCount: 1, + }, nil +} + +// executeDropIndex executes a db.collection.dropIndex() command. +func executeDropIndex(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + collection := client.Database(database).Collection(op.Collection) + + var err error + if op.IndexName != "" { + // Drop by index name + err = collection.Indexes().DropOne(ctx, op.IndexName) + } else if op.IndexKeys != nil { + // Drop by index key specification - need to find the index name first + cursor, listErr := collection.Indexes().List(ctx) + if listErr != nil { + return nil, fmt.Errorf("dropIndex failed: %w", listErr) + } + defer func() { _ = cursor.Close(ctx) }() + + var indexName string + for cursor.Next(ctx) { + var idx bson.M + if decodeErr := cursor.Decode(&idx); decodeErr != nil { + return nil, fmt.Errorf("dropIndex failed: %w", decodeErr) + } + // Check if keys match + if keysMatch(idx["key"], op.IndexKeys) { + indexName, _ = idx["name"].(string) + break + } + } + if indexName == "" { + return nil, fmt.Errorf("dropIndex failed: index not found") + } + err = collection.Indexes().DropOne(ctx, indexName) + } else { + return nil, fmt.Errorf("dropIndex failed: no index specified") + } + + if err != nil { + return nil, fmt.Errorf("dropIndex failed: %w", err) + } + + response := bson.M{"ok": 1} + jsonBytes, err := bson.MarshalExtJSONIndent(response, false, false, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal failed: %w", err) + } + + return &Result{ + Rows: []string{string(jsonBytes)}, + RowCount: 1, + }, nil +} + +// keysMatch compares two index key specifications. +func keysMatch(a any, b bson.D) bool { + switch keys := a.(type) { + case bson.D: + if len(keys) != len(b) { + return false + } + for i, elem := range keys { + if elem.Key != b[i].Key { + return false + } + // Compare values (could be int32, int64, string, etc.) + if !valuesEqual(elem.Value, b[i].Value) { + return false + } + } + return true + case bson.M: + if len(keys) != len(b) { + return false + } + for _, elem := range b { + val, ok := keys[elem.Key] + if !ok { + return false + } + if !valuesEqual(val, elem.Value) { + return false + } + } + return true + } + return false +} + +// valuesEqual compares two values that could be different numeric types. +func valuesEqual(a, b any) bool { + // Convert both to int64 for comparison if they're numeric + aInt, aOk := toInt64(a) + bInt, bOk := toInt64(b) + if aOk && bOk { + return aInt == bInt + } + // Otherwise compare directly + return a == b +} + +func toInt64(v any) (int64, bool) { + switch n := v.(type) { + case int: + return int64(n), true + case int32: + return int64(n), true + case int64: + return n, true + case float64: + return int64(n), true + } + return 0, false +} + +// executeDropIndexes executes a db.collection.dropIndexes() command. +func executeDropIndexes(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + collection := client.Database(database).Collection(op.Collection) + + var err error + if op.IndexName == "*" || op.IndexName == "" { + // Drop all indexes (except _id) + err = collection.Indexes().DropAll(ctx) + } else { + // Drop specific index + err = collection.Indexes().DropOne(ctx, op.IndexName) + } + + if err != nil { + return nil, fmt.Errorf("dropIndexes failed: %w", err) + } + + response := bson.M{"ok": 1} + jsonBytes, err := bson.MarshalExtJSONIndent(response, false, false, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal failed: %w", err) + } + + return &Result{ + Rows: []string{string(jsonBytes)}, + RowCount: 1, + }, nil +} + +// executeDrop executes a db.collection.drop() command. +func executeDrop(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + collection := client.Database(database).Collection(op.Collection) + + err := collection.Drop(ctx) + if err != nil { + return nil, fmt.Errorf("drop failed: %w", err) + } + + return &Result{ + Rows: []string{"true"}, + RowCount: 1, + }, nil +} + +// executeCreateCollection executes a db.createCollection() command. +func executeCreateCollection(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + db := client.Database(database) + + err := db.CreateCollection(ctx, op.Collection) + if err != nil { + return nil, fmt.Errorf("createCollection failed: %w", err) + } + + response := bson.M{"ok": 1} + jsonBytes, err := bson.MarshalExtJSONIndent(response, false, false, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal failed: %w", err) + } + + return &Result{ + Rows: []string{string(jsonBytes)}, + RowCount: 1, + }, nil +} + +// executeDropDatabase executes a db.dropDatabase() command. +func executeDropDatabase(ctx context.Context, client *mongo.Client, database string) (*Result, error) { + err := client.Database(database).Drop(ctx) + if err != nil { + return nil, fmt.Errorf("dropDatabase failed: %w", err) + } + + response := bson.M{"ok": 1, "dropped": database} + jsonBytes, err := bson.MarshalExtJSONIndent(response, false, false, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal failed: %w", err) + } + + return &Result{ + Rows: []string{string(jsonBytes)}, + RowCount: 1, + }, nil +} + +// executeRenameCollection executes a db.collection.renameCollection() command. +func executeRenameCollection(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { + // MongoDB's renameCollection command needs to be run on admin database + // The source is in the form "database.collection" + command := bson.D{ + {Key: "renameCollection", Value: database + "." + op.Collection}, + {Key: "to", Value: database + "." + op.NewName}, + } + if op.DropTarget != nil && *op.DropTarget { + command = append(command, bson.E{Key: "dropTarget", Value: true}) + } + + result := client.Database("admin").RunCommand(ctx, command) + if err := result.Err(); err != nil { + return nil, fmt.Errorf("renameCollection failed: %w", err) + } + + response := bson.M{"ok": 1} + jsonBytes, err := bson.MarshalExtJSONIndent(response, false, false, "", " ") + if err != nil { + return nil, fmt.Errorf("marshal failed: %w", err) + } + + return &Result{ + Rows: []string{string(jsonBytes)}, + RowCount: 1, + }, nil +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index 5a1156d..41c243e 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -60,6 +60,21 @@ func Execute(ctx context.Context, client *mongo.Client, database string, op *tra return executeFindOneAndReplace(ctx, client, database, op) case translator.OpFindOneAndDelete: return executeFindOneAndDelete(ctx, client, database, op) + // M3: Administrative Operations + case translator.OpCreateIndex: + return executeCreateIndex(ctx, client, database, op) + case translator.OpDropIndex: + return executeDropIndex(ctx, client, database, op) + case translator.OpDropIndexes: + return executeDropIndexes(ctx, client, database, op) + case translator.OpDrop: + return executeDrop(ctx, client, database, op) + case translator.OpCreateCollection: + return executeCreateCollection(ctx, client, database, op) + case translator.OpDropDatabase: + return executeDropDatabase(ctx, client, database) + case translator.OpRenameCollection: + return executeRenameCollection(ctx, client, database, op) default: return nil, fmt.Errorf("unsupported operation: %s", statement) } diff --git a/internal/testutil/container.go b/internal/testutil/container.go index aed6adb..bb6fa6f 100644 --- a/internal/testutil/container.go +++ b/internal/testutil/container.go @@ -224,3 +224,19 @@ func RunOnAllDBs(t *testing.T, testFn func(t *testing.T, db TestDB)) { }) } } + +// RunOnMongoDBOnly runs a test function only on MongoDB databases (not DocumentDB). +// Use this for tests that require MongoDB-specific features like renameCollection. +func RunOnMongoDBOnly(t *testing.T, testFn func(t *testing.T, db TestDB)) { + t.Helper() + dbs := GetAllClients(t) + for _, db := range dbs { + // Skip DocumentDB as some operations like renameCollection may not be supported + if db.Name == "documentdb" { + continue + } + t.Run(db.Name, func(t *testing.T) { + testFn(t, db) + }) + } +} diff --git a/internal/translator/collection.go b/internal/translator/collection.go index f4fa33d..6363880 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -1825,3 +1825,284 @@ func (v *visitor) extractFindOneAndModifyArgs(methodName string, args mongodb.IA return } } + +// extractCreateIndexArgs extracts arguments from CreateIndexMethodContext. +func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) { + method, ok := ctx.(*mongodb.CreateIndexMethodContext) + if !ok { + return + } + + args := method.Arguments() + if args == nil { + v.err = fmt.Errorf("createIndex() requires a key specification") + return + } + + argsCtx, ok := args.(*mongodb.ArgumentsContext) + if !ok { + v.err = fmt.Errorf("createIndex() requires a key specification") + return + } + + allArgs := argsCtx.AllArgument() + if len(allArgs) == 0 { + v.err = fmt.Errorf("createIndex() requires a key specification") + return + } + + // First argument: keys document (required) + firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("createIndex() key specification must be a document") + return + } + + valueCtx := firstArg.Value() + if valueCtx == nil { + v.err = fmt.Errorf("createIndex() key specification must be a document") + return + } + + docValue, ok := valueCtx.(*mongodb.DocumentValueContext) + if !ok { + v.err = fmt.Errorf("createIndex() key specification must be a document") + return + } + + keys, err := convertDocument(docValue.Document()) + if err != nil { + v.err = fmt.Errorf("invalid key specification: %w", err) + return + } + v.operation.IndexKeys = keys + + // Second argument: options (optional) + if len(allArgs) >= 2 { + secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) + if !ok { + return + } + + optionsValueCtx := secondArg.Value() + if optionsValueCtx == nil { + return + } + + optionsDocValue, ok := optionsValueCtx.(*mongodb.DocumentValueContext) + if !ok { + v.err = fmt.Errorf("createIndex() options must be a document") + return + } + + options, err := convertDocument(optionsDocValue.Document()) + if err != nil { + v.err = fmt.Errorf("invalid options: %w", err) + return + } + + // Extract supported options + for _, opt := range options { + switch opt.Key { + case "name": + if val, ok := opt.Value.(string); ok { + v.operation.IndexName = val + } else { + v.err = fmt.Errorf("createIndex() name must be a string") + return + } + case "unique", "sparse", "background", "expireAfterSeconds", "partialFilterExpression", + "collation", "wildcardProjection", "hidden": + // These are valid options, we store them in a generic way + // For now, we only explicitly handle "name", others are passed through + default: + // Allow any option, driver will validate + } + } + // Store the entire options document for the driver + v.operation.Collation = options // Reusing Collation field as options container + } + + if len(allArgs) > 2 { + v.err = fmt.Errorf("createIndex() takes at most 2 arguments") + return + } +} + +// extractDropIndexArgs extracts arguments from DropIndexMethodContext. +func (v *visitor) extractDropIndexArgs(ctx mongodb.IDropIndexMethodContext) { + method, ok := ctx.(*mongodb.DropIndexMethodContext) + if !ok { + return + } + + arg := method.Argument() + if arg == nil { + v.err = fmt.Errorf("dropIndex() requires an index name or key specification") + return + } + + argCtx, ok := arg.(*mongodb.ArgumentContext) + if !ok { + return + } + + valueCtx := argCtx.Value() + if valueCtx == nil { + v.err = fmt.Errorf("dropIndex() requires an index name or key specification") + return + } + + // dropIndex can accept a string (index name) or document (index key spec) + switch val := valueCtx.(type) { + case *mongodb.LiteralValueContext: + strLit, ok := val.Literal().(*mongodb.StringLiteralValueContext) + if !ok { + v.err = fmt.Errorf("dropIndex() argument must be a string or document") + return + } + v.operation.IndexName = unquoteString(strLit.StringLiteral().GetText()) + case *mongodb.DocumentValueContext: + doc, err := convertDocument(val.Document()) + if err != nil { + v.err = fmt.Errorf("invalid index specification: %w", err) + return + } + v.operation.IndexKeys = doc + default: + v.err = fmt.Errorf("dropIndex() argument must be a string or document") + } +} + +// extractDropIndexesArgs extracts arguments from DropIndexesMethodContext. +func (v *visitor) extractDropIndexesArgs(ctx mongodb.IDropIndexesMethodContext) { + method, ok := ctx.(*mongodb.DropIndexesMethodContext) + if !ok { + return + } + + // dropIndexes() can be called without arguments (drops all indexes except _id) + // or with a single argument (index name, array of names, or "*") + arg := method.Argument() + if arg == nil { + // No argument means drop all indexes (represented as "*" to the driver) + v.operation.IndexName = "*" + return + } + + argCtx, ok := arg.(*mongodb.ArgumentContext) + if !ok { + return + } + + valueCtx := argCtx.Value() + if valueCtx == nil { + v.operation.IndexName = "*" + return + } + + switch val := valueCtx.(type) { + case *mongodb.LiteralValueContext: + strLit, ok := val.Literal().(*mongodb.StringLiteralValueContext) + if !ok { + v.err = fmt.Errorf("dropIndexes() argument must be a string or array") + return + } + v.operation.IndexName = unquoteString(strLit.StringLiteral().GetText()) + case *mongodb.ArrayValueContext: + // Array of index names - we'll just drop "*" for simplicity + // The driver handles dropping individual indexes + v.operation.IndexName = "*" + default: + v.err = fmt.Errorf("dropIndexes() argument must be a string or array") + } +} + +// extractRenameCollectionArgs extracts arguments from RenameCollectionMethodContext. +func (v *visitor) extractRenameCollectionArgs(ctx mongodb.IRenameCollectionMethodContext) { + method, ok := ctx.(*mongodb.RenameCollectionMethodContext) + if !ok { + return + } + + args := method.Arguments() + if args == nil { + v.err = fmt.Errorf("renameCollection() requires a new collection name") + return + } + + argsCtx, ok := args.(*mongodb.ArgumentsContext) + if !ok { + v.err = fmt.Errorf("renameCollection() requires a new collection name") + return + } + + allArgs := argsCtx.AllArgument() + if len(allArgs) == 0 { + v.err = fmt.Errorf("renameCollection() requires a new collection name") + return + } + + // First argument: new collection name (required) + firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("renameCollection() new name must be a string") + return + } + + valueCtx := firstArg.Value() + if valueCtx == nil { + v.err = fmt.Errorf("renameCollection() new name must be a string") + return + } + + literalValue, ok := valueCtx.(*mongodb.LiteralValueContext) + if !ok { + v.err = fmt.Errorf("renameCollection() new name must be a string") + return + } + + stringLiteral, ok := literalValue.Literal().(*mongodb.StringLiteralValueContext) + if !ok { + v.err = fmt.Errorf("renameCollection() new name must be a string") + return + } + + v.operation.NewName = unquoteString(stringLiteral.StringLiteral().GetText()) + + // Second argument: dropTarget boolean (optional) + if len(allArgs) >= 2 { + secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) + if !ok { + return + } + + dropTargetValueCtx := secondArg.Value() + if dropTargetValueCtx == nil { + return + } + + literalVal, ok := dropTargetValueCtx.(*mongodb.LiteralValueContext) + if !ok { + v.err = fmt.Errorf("renameCollection() dropTarget must be a boolean") + return + } + + switch literalVal.Literal().(type) { + case *mongodb.TrueLiteralContext: + dropTarget := true + v.operation.DropTarget = &dropTarget + case *mongodb.FalseLiteralContext: + dropTarget := false + v.operation.DropTarget = &dropTarget + default: + v.err = fmt.Errorf("renameCollection() dropTarget must be a boolean") + return + } + } + + if len(allArgs) > 2 { + v.err = fmt.Errorf("renameCollection() takes at most 2 arguments") + return + } +} diff --git a/internal/translator/database.go b/internal/translator/database.go index b6989dc..e23d3d2 100644 --- a/internal/translator/database.go +++ b/internal/translator/database.go @@ -101,3 +101,58 @@ func (v *visitor) extractGetCollectionInfosArgs(ctx *mongodb.GetCollectionInfosC return } } + +// extractCreateCollectionArgs extracts arguments from CreateCollectionContext. +func (v *visitor) extractCreateCollectionArgs(ctx *mongodb.CreateCollectionContext) { + args := ctx.Arguments() + if args == nil { + v.err = fmt.Errorf("createCollection() requires a collection name") + return + } + + argsCtx, ok := args.(*mongodb.ArgumentsContext) + if !ok { + v.err = fmt.Errorf("createCollection() requires a collection name") + return + } + + allArgs := argsCtx.AllArgument() + if len(allArgs) == 0 { + v.err = fmt.Errorf("createCollection() requires a collection name") + return + } + + // First argument: collection name (required) + firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("createCollection() collection name must be a string") + return + } + + valueCtx := firstArg.Value() + if valueCtx == nil { + v.err = fmt.Errorf("createCollection() collection name must be a string") + return + } + + literalValue, ok := valueCtx.(*mongodb.LiteralValueContext) + if !ok { + v.err = fmt.Errorf("createCollection() collection name must be a string") + return + } + + stringLiteral, ok := literalValue.Literal().(*mongodb.StringLiteralValueContext) + if !ok { + v.err = fmt.Errorf("createCollection() collection name must be a string") + return + } + + v.operation.Collection = unquoteString(stringLiteral.StringLiteral().GetText()) + + // Second argument: options (optional) - we don't extract options for basic implementation + // The driver will handle any valid options + if len(allArgs) > 2 { + v.err = fmt.Errorf("createCollection() takes at most 2 arguments") + return + } +} diff --git a/internal/translator/method_registry.go b/internal/translator/method_registry.go index e34b6df..5461801 100644 --- a/internal/translator/method_registry.go +++ b/internal/translator/method_registry.go @@ -17,24 +17,16 @@ type methodInfo struct { // methodRegistry contains only methods we plan to implement (M2, M3). // If a method is NOT in this registry, it's unsupported (throw error, no fallback). // If a method IS in this registry, it's planned (fallback to mongosh). +// Note: Methods that are now implemented have been removed from this registry. var methodRegistry = map[string]methodInfo{ // ============================================================ - // MILESTONE 3: Administrative Operations (22 methods) + // MILESTONE 3: Administrative Operations (remaining planned) // ============================================================ - // Index Management (4) - "collection:createIndex": {status: statusPlanned}, + // Index Management (1 remaining - createIndexes has lower ROI) "collection:createIndexes": {status: statusPlanned}, - "collection:dropIndex": {status: statusPlanned}, - "collection:dropIndexes": {status: statusPlanned}, - // Collection Management (4) - "database:createCollection": {status: statusPlanned}, - "collection:drop": {status: statusPlanned}, - "collection:renameCollection": {status: statusPlanned}, - "database:dropDatabase": {status: statusPlanned}, - - // Database Information (7) + // Database Information (7) - lower ROI, keep as planned "database:stats": {status: statusPlanned}, "collection:stats": {status: statusPlanned}, "database:serverStatus": {status: statusPlanned}, @@ -43,7 +35,7 @@ var methodRegistry = map[string]methodInfo{ "database:hostInfo": {status: statusPlanned}, "database:listCommands": {status: statusPlanned}, - // Collection Information (7) + // Collection Information (7) - lower ROI, keep as planned "collection:dataSize": {status: statusPlanned}, "collection:storageSize": {status: statusPlanned}, "collection:totalIndexSize": {status: statusPlanned}, diff --git a/internal/translator/types.go b/internal/translator/types.go index a31753a..9cca90c 100644 --- a/internal/translator/types.go +++ b/internal/translator/types.go @@ -29,6 +29,14 @@ const ( OpFindOneAndUpdate OpFindOneAndReplace OpFindOneAndDelete + // M3: Administrative Operations + OpCreateIndex + OpDropIndex + OpDropIndexes + OpDrop + OpCreateCollection + OpDropDatabase + OpRenameCollection ) // Operation represents a parsed MongoDB operation. @@ -70,4 +78,17 @@ type Operation struct { BypassDocumentValidation *bool // bypass schema validation Comment any // comment for server logs/profiling WriteConcern bson.D // write concern settings (w, j, wtimeout) + + // M3: Administrative operation fields + IndexKeys bson.D // createIndex key specification + IndexName string // dropIndex index name (or createIndex name option) + NewName string // renameCollection new collection name + DropTarget *bool // renameCollection dropTarget option + IndexModels []IndexModel // createIndexes array of index specifications +} + +// IndexModel represents a single index specification for createIndexes. +type IndexModel struct { + Keys bson.D + Options bson.D } diff --git a/internal/translator/visitor.go b/internal/translator/visitor.go index 380c99e..11c132f 100644 --- a/internal/translator/visitor.go +++ b/internal/translator/visitor.go @@ -60,6 +60,11 @@ func (v *visitor) visitDbStatement(ctx mongodb.IDbStatementContext) { case *mongodb.GetCollectionInfosContext: v.operation.OpType = OpGetCollectionInfos v.extractGetCollectionInfosArgs(c) + case *mongodb.CreateCollectionContext: + v.operation.OpType = OpCreateCollection + v.extractCreateCollectionArgs(c) + case *mongodb.DropDatabaseContext: + v.operation.OpType = OpDropDatabase } } @@ -213,21 +218,25 @@ func (v *visitor) visitMethodCall(ctx mongodb.IMethodCallContext) { v.operation.OpType = OpFindOneAndDelete v.extractFindOneAndDeleteArgs(mc.FindOneAndDeleteMethod()) - // Planned M3 index operations - return PlannedOperationError for fallback + // Supported M3 index operations case mc.CreateIndexMethod() != nil: - v.handleUnsupportedMethod("collection", "createIndex") + v.operation.OpType = OpCreateIndex + v.extractCreateIndexArgs(mc.CreateIndexMethod()) case mc.CreateIndexesMethod() != nil: - v.handleUnsupportedMethod("collection", "createIndexes") + v.handleUnsupportedMethod("collection", "createIndexes") // Lower ROI, keep as planned case mc.DropIndexMethod() != nil: - v.handleUnsupportedMethod("collection", "dropIndex") + v.operation.OpType = OpDropIndex + v.extractDropIndexArgs(mc.DropIndexMethod()) case mc.DropIndexesMethod() != nil: - v.handleUnsupportedMethod("collection", "dropIndexes") + v.operation.OpType = OpDropIndexes + v.extractDropIndexesArgs(mc.DropIndexesMethod()) - // Planned M3 collection management - return PlannedOperationError for fallback + // Supported M3 collection management case mc.DropMethod() != nil: - v.handleUnsupportedMethod("collection", "drop") + v.operation.OpType = OpDrop case mc.RenameCollectionMethod() != nil: - v.handleUnsupportedMethod("collection", "renameCollection") + v.operation.OpType = OpRenameCollection + v.extractRenameCollectionArgs(mc.RenameCollectionMethod()) // Planned M3 stats operations - return PlannedOperationError for fallback case mc.StatsMethod() != nil: From 280a5afde6dbd2354cadb739457e3d93cdd1e768 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Mon, 26 Jan 2026 15:58:33 +0800 Subject: [PATCH 2/4] fix: address PR review comments - createCollection(): reject options argument (not supported yet) - dropIndexes(): reject array argument instead of silently dropping all - createIndex(): reject unsupported options instead of ignoring them - Remove unused IndexModels field and IndexModel type Co-Authored-By: Claude Opus 4.5 --- internal/translator/collection.go | 23 ++++++++++++----------- internal/translator/database.go | 9 +++++---- internal/translator/types.go | 15 ++++----------- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/internal/translator/collection.go b/internal/translator/collection.go index 6363880..c62cc0b 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -1878,6 +1878,7 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) v.operation.IndexKeys = keys // Second argument: options (optional) + // Currently only the "name" option is supported. if len(allArgs) >= 2 { secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) if !ok { @@ -1901,7 +1902,7 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) return } - // Extract supported options + // Only "name" option is currently supported for _, opt := range options { switch opt.Key { case "name": @@ -1911,16 +1912,15 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) v.err = fmt.Errorf("createIndex() name must be a string") return } - case "unique", "sparse", "background", "expireAfterSeconds", "partialFilterExpression", - "collation", "wildcardProjection", "hidden": - // These are valid options, we store them in a generic way - // For now, we only explicitly handle "name", others are passed through default: - // Allow any option, driver will validate + // Reject unsupported options to avoid silently ignoring them + v.err = &UnsupportedOptionError{ + Method: "createIndex()", + Option: opt.Key, + } + return } } - // Store the entire options document for the driver - v.operation.Collation = options // Reusing Collation field as options container } if len(allArgs) > 2 { @@ -2010,9 +2010,10 @@ func (v *visitor) extractDropIndexesArgs(ctx mongodb.IDropIndexesMethodContext) } v.operation.IndexName = unquoteString(strLit.StringLiteral().GetText()) case *mongodb.ArrayValueContext: - // Array of index names - we'll just drop "*" for simplicity - // The driver handles dropping individual indexes - v.operation.IndexName = "*" + // Array of index names is not currently supported in this translator. + // Silently broadening scope to drop all indexes ("*") would be unsafe. + v.err = fmt.Errorf("dropIndexes() with an array of index names is not supported; use a single index name or \"*\" to drop all indexes") + return default: v.err = fmt.Errorf("dropIndexes() argument must be a string or array") } diff --git a/internal/translator/database.go b/internal/translator/database.go index e23d3d2..094e270 100644 --- a/internal/translator/database.go +++ b/internal/translator/database.go @@ -149,10 +149,11 @@ func (v *visitor) extractCreateCollectionArgs(ctx *mongodb.CreateCollectionConte v.operation.Collection = unquoteString(stringLiteral.StringLiteral().GetText()) - // Second argument: options (optional) - we don't extract options for basic implementation - // The driver will handle any valid options - if len(allArgs) > 2 { - v.err = fmt.Errorf("createCollection() takes at most 2 arguments") + // Second argument: options (optional) + // Options are currently not supported; reject calls that attempt to use them + // to avoid silently ignoring user-specified options. + if len(allArgs) > 1 { + v.err = fmt.Errorf("createCollection() options argument is not supported yet") return } } diff --git a/internal/translator/types.go b/internal/translator/types.go index 9cca90c..997c6c1 100644 --- a/internal/translator/types.go +++ b/internal/translator/types.go @@ -80,15 +80,8 @@ type Operation struct { WriteConcern bson.D // write concern settings (w, j, wtimeout) // M3: Administrative operation fields - IndexKeys bson.D // createIndex key specification - IndexName string // dropIndex index name (or createIndex name option) - NewName string // renameCollection new collection name - DropTarget *bool // renameCollection dropTarget option - IndexModels []IndexModel // createIndexes array of index specifications -} - -// IndexModel represents a single index specification for createIndexes. -type IndexModel struct { - Keys bson.D - Options bson.D + IndexKeys bson.D // createIndex key specification + IndexName string // dropIndex index name (or createIndex name option) + NewName string // renameCollection new collection name + DropTarget *bool // renameCollection dropTarget option } From f8ddcd3aba071f9df69307b6587615fd6f0644af Mon Sep 17 00:00:00 2001 From: h3n4l Date: Mon, 26 Jan 2026 16:06:55 +0800 Subject: [PATCH 3/4] feat: enhance M3 admin operations with extended options support Address PR review feedback by implementing proper support instead of rejecting unsupported features: createCollection options: - capped: boolean for capped collections - size: size in bytes for capped collections - max: max documents for capped collections - validator: validation rules document - validationLevel/validationAction: validation settings createIndex options: - unique: boolean for unique indexes - sparse: boolean for sparse indexes - expireAfterSeconds: TTL index support - background: accepted but deprecated (no-op) dropIndexes enhancements: - Support array of index names to drop multiple specific indexes Co-Authored-By: Claude Opus 4.5 --- admin_test.go | 161 ++++++++++++++++++++++++++++++ internal/executor/admin.go | 55 +++++++++- internal/translator/collection.go | 51 ++++++++-- internal/translator/database.go | 84 +++++++++++++++- internal/translator/helpers.go | 30 ++++++ internal/translator/types.go | 34 +++++-- unicode_test.go | 4 +- 7 files changed, 392 insertions(+), 27 deletions(-) diff --git a/admin_test.go b/admin_test.go index 8d88a65..4c2ac58 100644 --- a/admin_test.go +++ b/admin_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "slices" + "strings" "testing" "github.com/bytebase/gomongo" @@ -260,3 +261,163 @@ func TestRenameCollectionWithDropTarget(t *testing.T) { require.Contains(t, findResult.Rows[0], `"x": 1`) }) } + +func TestCreateCollectionWithOptions(t *testing.T) { + // Capped collections are not supported on DocumentDB + testutil.RunOnMongoDBOnly(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_coll_opts_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) + + // Create a capped collection + result, err := gc.Execute(ctx, dbName, `db.createCollection("cappedcoll", { capped: true, size: 1048576 })`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify collection exists + collResult, err := gc.Execute(ctx, dbName, `show collections`) + require.NoError(t, err) + require.Equal(t, 1, collResult.RowCount) + require.Equal(t, "cappedcoll", collResult.Rows[0]) + }) +} + +func TestCreateCollectionWithMaxDocuments(t *testing.T) { + // Capped collections are not supported on DocumentDB + testutil.RunOnMongoDBOnly(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_coll_max_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) + + // Create a capped collection with max documents + result, err := gc.Execute(ctx, dbName, `db.createCollection("cappedmax", { capped: true, size: 1048576, max: 100 })`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + }) +} + +func TestDropIndexesWithArray(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_drop_idxs_arr_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + // Create a collection with indexes + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertOne(ctx, bson.M{"name": "alice", "email": "alice@example.com", "age": 30}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create indexes with explicit names + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ name: 1 }, { name: "name_idx" })`) + require.NoError(t, err) + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ email: 1 }, { name: "email_idx" })`) + require.NoError(t, err) + _, err = gc.Execute(ctx, dbName, `db.users.createIndex({ age: 1 }, { name: "age_idx" })`) + require.NoError(t, err) + + // Drop two indexes using an array + result, err := gc.Execute(ctx, dbName, `db.users.dropIndexes(["name_idx", "email_idx"])`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], `"ok": 1`) + + // Verify only _id and age_idx remain + idxResult, err := gc.Execute(ctx, dbName, `db.users.getIndexes()`) + require.NoError(t, err) + require.Equal(t, 2, idxResult.RowCount) // _id + age_idx + }) +} + +func TestCreateIndexWithUniqueOption(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_unique_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + _, err := db.Client.Database(dbName).Collection("users").InsertOne(ctx, bson.M{"email": "alice@example.com"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create a unique index + result, err := gc.Execute(ctx, dbName, `db.users.createIndex({ email: 1 }, { unique: true, name: "email_unique" })`) + require.NoError(t, err) + require.Equal(t, "email_unique", result.Rows[0]) + + // Try to insert a duplicate - should fail + _, err = gc.Execute(ctx, dbName, `db.users.insertOne({ email: "alice@example.com" })`) + require.Error(t, err) + // Different databases may use different error messages + errStr := err.Error() + require.True(t, strings.Contains(errStr, "duplicate key") || strings.Contains(errStr, "Duplicate key"), + "expected duplicate key error, got: %s", errStr) + }) +} + +func TestCreateIndexWithSparseOption(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_sparse_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + _, err := db.Client.Database(dbName).Collection("users").InsertOne(ctx, bson.M{"name": "alice"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create a sparse index + result, err := gc.Execute(ctx, dbName, `db.users.createIndex({ email: 1 }, { sparse: true, name: "email_sparse" })`) + require.NoError(t, err) + require.Equal(t, "email_sparse", result.Rows[0]) + + // Documents without the indexed field should still be insertable + _, err = gc.Execute(ctx, dbName, `db.users.insertOne({ name: "bob" })`) + require.NoError(t, err) + }) +} + +func TestCreateIndexWithTTLOption(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_ttl_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + _, err := db.Client.Database(dbName).Collection("sessions").InsertOne(ctx, bson.M{"createdAt": bson.M{"$date": "2024-01-01T00:00:00Z"}}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create a TTL index + result, err := gc.Execute(ctx, dbName, `db.sessions.createIndex({ createdAt: 1 }, { expireAfterSeconds: 3600, name: "session_ttl" })`) + require.NoError(t, err) + require.Equal(t, "session_ttl", result.Rows[0]) + }) +} + +func TestCreateIndexWithBackgroundOption(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_create_idx_bg_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + + _, err := db.Client.Database(dbName).Collection("users").InsertOne(ctx, bson.M{"name": "alice"}) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // Create an index with background option (deprecated but should be accepted) + result, err := gc.Execute(ctx, dbName, `db.users.createIndex({ name: 1 }, { background: true })`) + require.NoError(t, err) + require.Contains(t, result.Rows[0], "name") + }) +} diff --git a/internal/executor/admin.go b/internal/executor/admin.go index f082b4e..60bfdfb 100644 --- a/internal/executor/admin.go +++ b/internal/executor/admin.go @@ -18,9 +18,28 @@ func executeCreateIndex(ctx context.Context, client *mongo.Client, database stri Keys: op.IndexKeys, } - // Build index options from the stored options + // Build index options + opts := options.Index() + hasOptions := false + if op.IndexName != "" { - opts := options.Index().SetName(op.IndexName) + opts.SetName(op.IndexName) + hasOptions = true + } + if op.IndexUnique != nil && *op.IndexUnique { + opts.SetUnique(true) + hasOptions = true + } + if op.IndexSparse != nil && *op.IndexSparse { + opts.SetSparse(true) + hasOptions = true + } + if op.IndexTTL != nil { + opts.SetExpireAfterSeconds(*op.IndexTTL) + hasOptions = true + } + + if hasOptions { indexModel.Options = opts } @@ -153,7 +172,14 @@ func executeDropIndexes(ctx context.Context, client *mongo.Client, database stri collection := client.Database(database).Collection(op.Collection) var err error - if op.IndexName == "*" || op.IndexName == "" { + if len(op.IndexNames) > 0 { + // Drop each index in the array + for _, name := range op.IndexNames { + if dropErr := collection.Indexes().DropOne(ctx, name); dropErr != nil { + return nil, fmt.Errorf("dropIndexes failed for index %q: %w", name, dropErr) + } + } + } else if op.IndexName == "*" || op.IndexName == "" { // Drop all indexes (except _id) err = collection.Indexes().DropAll(ctx) } else { @@ -196,7 +222,28 @@ func executeDrop(ctx context.Context, client *mongo.Client, database string, op func executeCreateCollection(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { db := client.Database(database) - err := db.CreateCollection(ctx, op.Collection) + // Build create collection options + opts := options.CreateCollection() + if op.Capped != nil && *op.Capped { + opts.SetCapped(true) + } + if op.CollectionSize != nil { + opts.SetSizeInBytes(*op.CollectionSize) + } + if op.CollectionMax != nil { + opts.SetMaxDocuments(*op.CollectionMax) + } + if op.Validator != nil { + opts.SetValidator(op.Validator) + } + if op.ValidationLevel != "" { + opts.SetValidationLevel(op.ValidationLevel) + } + if op.ValidationAction != "" { + opts.SetValidationAction(op.ValidationAction) + } + + err := db.CreateCollection(ctx, op.Collection, opts) if err != nil { return nil, fmt.Errorf("createCollection failed: %w", err) } diff --git a/internal/translator/collection.go b/internal/translator/collection.go index c62cc0b..f850209 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -1902,7 +1902,6 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) return } - // Only "name" option is currently supported for _, opt := range options { switch opt.Key { case "name": @@ -1912,8 +1911,36 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) v.err = fmt.Errorf("createIndex() name must be a string") return } + case "unique": + if val, ok := opt.Value.(bool); ok { + v.operation.IndexUnique = &val + } else { + v.err = fmt.Errorf("createIndex() unique must be a boolean") + return + } + case "sparse": + if val, ok := opt.Value.(bool); ok { + v.operation.IndexSparse = &val + } else { + v.err = fmt.Errorf("createIndex() sparse must be a boolean") + return + } + case "expireAfterSeconds": + if val, ok := toInt32(opt.Value); ok { + v.operation.IndexTTL = &val + } else { + v.err = fmt.Errorf("createIndex() expireAfterSeconds must be a number") + return + } + case "background": + // background option is deprecated but still accepted for compatibility + // The Go driver ignores it since MongoDB 4.2 + if _, ok := opt.Value.(bool); !ok { + v.err = fmt.Errorf("createIndex() background must be a boolean") + return + } + // Silently ignore - it's deprecated and has no effect default: - // Reject unsupported options to avoid silently ignoring them v.err = &UnsupportedOptionError{ Method: "createIndex()", Option: opt.Key, @@ -2010,10 +2037,22 @@ func (v *visitor) extractDropIndexesArgs(ctx mongodb.IDropIndexesMethodContext) } v.operation.IndexName = unquoteString(strLit.StringLiteral().GetText()) case *mongodb.ArrayValueContext: - // Array of index names is not currently supported in this translator. - // Silently broadening scope to drop all indexes ("*") would be unsafe. - v.err = fmt.Errorf("dropIndexes() with an array of index names is not supported; use a single index name or \"*\" to drop all indexes") - return + // Array of index names - iterate and extract each name + arr, err := convertArray(val.Array()) + if err != nil { + v.err = fmt.Errorf("invalid index names array: %w", err) + return + } + var indexNames []string + for i, elem := range arr { + name, ok := elem.(string) + if !ok { + v.err = fmt.Errorf("dropIndexes() array element %d must be a string", i) + return + } + indexNames = append(indexNames, name) + } + v.operation.IndexNames = indexNames default: v.err = fmt.Errorf("dropIndexes() argument must be a string or array") } diff --git a/internal/translator/database.go b/internal/translator/database.go index 094e270..619e129 100644 --- a/internal/translator/database.go +++ b/internal/translator/database.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/bytebase/parser/mongodb" + "go.mongodb.org/mongo-driver/v2/bson" ) func (v *visitor) extractGetCollectionInfosArgs(ctx *mongodb.GetCollectionInfosContext) { @@ -150,10 +151,85 @@ func (v *visitor) extractCreateCollectionArgs(ctx *mongodb.CreateCollectionConte v.operation.Collection = unquoteString(stringLiteral.StringLiteral().GetText()) // Second argument: options (optional) - // Options are currently not supported; reject calls that attempt to use them - // to avoid silently ignoring user-specified options. - if len(allArgs) > 1 { - v.err = fmt.Errorf("createCollection() options argument is not supported yet") + if len(allArgs) >= 2 { + secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) + if !ok { + return + } + + optionsValueCtx := secondArg.Value() + if optionsValueCtx == nil { + return + } + + optionsDocValue, ok := optionsValueCtx.(*mongodb.DocumentValueContext) + if !ok { + v.err = fmt.Errorf("createCollection() options must be a document") + return + } + + options, err := convertDocument(optionsDocValue.Document()) + if err != nil { + v.err = fmt.Errorf("invalid options: %w", err) + return + } + + for _, opt := range options { + switch opt.Key { + case "capped": + if val, ok := opt.Value.(bool); ok { + v.operation.Capped = &val + } else { + v.err = fmt.Errorf("createCollection() capped must be a boolean") + return + } + case "size": + if val, ok := toInt64(opt.Value); ok { + v.operation.CollectionSize = &val + } else { + v.err = fmt.Errorf("createCollection() size must be a number") + return + } + case "max": + if val, ok := toInt64(opt.Value); ok { + v.operation.CollectionMax = &val + } else { + v.err = fmt.Errorf("createCollection() max must be a number") + return + } + case "validator": + if doc, ok := opt.Value.(bson.D); ok { + v.operation.Validator = doc + } else { + v.err = fmt.Errorf("createCollection() validator must be a document") + return + } + case "validationLevel": + if val, ok := opt.Value.(string); ok { + v.operation.ValidationLevel = val + } else { + v.err = fmt.Errorf("createCollection() validationLevel must be a string") + return + } + case "validationAction": + if val, ok := opt.Value.(string); ok { + v.operation.ValidationAction = val + } else { + v.err = fmt.Errorf("createCollection() validationAction must be a string") + return + } + default: + v.err = &UnsupportedOptionError{ + Method: "createCollection()", + Option: opt.Key, + } + return + } + } + } + + if len(allArgs) > 2 { + v.err = fmt.Errorf("createCollection() takes at most 2 arguments") return } } diff --git a/internal/translator/helpers.go b/internal/translator/helpers.go index 403d4cc..8ef06fc 100644 --- a/internal/translator/helpers.go +++ b/internal/translator/helpers.go @@ -181,3 +181,33 @@ func convertRegExpConstructor(ctx mongodb.IRegExpConstructorContext) (bson.Regex return bson.Regex{Pattern: pattern, Options: options}, nil } + +// toInt64 converts various numeric types to int64. +func toInt64(v any) (int64, bool) { + switch n := v.(type) { + case int: + return int64(n), true + case int32: + return int64(n), true + case int64: + return n, true + case float64: + return int64(n), true + } + return 0, false +} + +// toInt32 converts various numeric types to int32. +func toInt32(v any) (int32, bool) { + switch n := v.(type) { + case int: + return int32(n), true + case int32: + return n, true + case int64: + return int32(n), true + case float64: + return int32(n), true + } + return 0, false +} diff --git a/internal/translator/types.go b/internal/translator/types.go index 997c6c1..027624b 100644 --- a/internal/translator/types.go +++ b/internal/translator/types.go @@ -71,17 +71,29 @@ type Operation struct { ReturnDocument *string // "before" or "after" for findOneAnd* operations // M2: Additional write operation options - Ordered *bool // insertMany ordered option - Collation bson.D // collation settings for string comparison - ArrayFilters bson.A // array element filters for update operations - Let bson.D // variables for aggregation expressions - BypassDocumentValidation *bool // bypass schema validation - Comment any // comment for server logs/profiling - WriteConcern bson.D // write concern settings (w, j, wtimeout) + Ordered *bool // insertMany ordered option + Collation bson.D // collation settings for string comparison + ArrayFilters bson.A // array element filters for update operations + Let bson.D // variables for aggregation expressions + BypassDocumentValidation *bool // bypass schema validation + Comment any // comment for server logs/profiling + WriteConcern bson.D // write concern settings (w, j, wtimeout) // M3: Administrative operation fields - IndexKeys bson.D // createIndex key specification - IndexName string // dropIndex index name (or createIndex name option) - NewName string // renameCollection new collection name - DropTarget *bool // renameCollection dropTarget option + IndexKeys bson.D // createIndex key specification + IndexName string // dropIndex index name (or createIndex name option) + IndexNames []string // dropIndexes array of index names + NewName string // renameCollection new collection name + DropTarget *bool // renameCollection dropTarget option + IndexUnique *bool // createIndex unique option + IndexSparse *bool // createIndex sparse option + IndexTTL *int32 // createIndex expireAfterSeconds option + + // createCollection options + Capped *bool // createCollection capped option + CollectionSize *int64 // createCollection size option (for capped) + CollectionMax *int64 // createCollection max option (for capped) + ValidationLevel string // createCollection validationLevel option + ValidationAction string // createCollection validationAction option + Validator bson.D // createCollection validator option } diff --git a/unicode_test.go b/unicode_test.go index 948dba0..df4c8e0 100644 --- a/unicode_test.go +++ b/unicode_test.go @@ -137,10 +137,10 @@ func TestUnicodeRoundTrip(t *testing.T) { for _, row := range result.Rows { allRows += row } - require.Contains(t, allRows, "张三") // Chinese + require.Contains(t, allRows, "张三") // Chinese require.Contains(t, allRows, "田中太郎") // Japanese require.Contains(t, allRows, "김철수") // Korean - require.Contains(t, allRows, "محمد") // Arabic + require.Contains(t, allRows, "محمد") // Arabic require.Contains(t, allRows, "🎉") // Emoji }) } From 618e3a6a4de1d923b3b3f87524782200a783d816 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Mon, 26 Jan 2026 16:16:43 +0800 Subject: [PATCH 4/4] refactor: centralize numeric helper functions in translator package Address PR review comment about duplicate toInt64 helper by: - Exporting ToInt64 and ToInt32 from translator/helpers.go - Removing duplicate toInt64 from executor/admin.go - Using translator.ToInt64 in executor for numeric comparisons Co-Authored-By: Claude Opus 4.5 --- internal/executor/admin.go | 18 ++---------------- internal/translator/collection.go | 2 +- internal/translator/database.go | 4 ++-- internal/translator/helpers.go | 8 ++++---- 4 files changed, 9 insertions(+), 23 deletions(-) diff --git a/internal/executor/admin.go b/internal/executor/admin.go index 60bfdfb..0ef7efc 100644 --- a/internal/executor/admin.go +++ b/internal/executor/admin.go @@ -144,8 +144,8 @@ func keysMatch(a any, b bson.D) bool { // valuesEqual compares two values that could be different numeric types. func valuesEqual(a, b any) bool { // Convert both to int64 for comparison if they're numeric - aInt, aOk := toInt64(a) - bInt, bOk := toInt64(b) + aInt, aOk := translator.ToInt64(a) + bInt, bOk := translator.ToInt64(b) if aOk && bOk { return aInt == bInt } @@ -153,20 +153,6 @@ func valuesEqual(a, b any) bool { return a == b } -func toInt64(v any) (int64, bool) { - switch n := v.(type) { - case int: - return int64(n), true - case int32: - return int64(n), true - case int64: - return n, true - case float64: - return int64(n), true - } - return 0, false -} - // executeDropIndexes executes a db.collection.dropIndexes() command. func executeDropIndexes(ctx context.Context, client *mongo.Client, database string, op *translator.Operation) (*Result, error) { collection := client.Database(database).Collection(op.Collection) diff --git a/internal/translator/collection.go b/internal/translator/collection.go index f850209..a69738d 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -1926,7 +1926,7 @@ func (v *visitor) extractCreateIndexArgs(ctx mongodb.ICreateIndexMethodContext) return } case "expireAfterSeconds": - if val, ok := toInt32(opt.Value); ok { + if val, ok := ToInt32(opt.Value); ok { v.operation.IndexTTL = &val } else { v.err = fmt.Errorf("createIndex() expireAfterSeconds must be a number") diff --git a/internal/translator/database.go b/internal/translator/database.go index 619e129..cd0c002 100644 --- a/internal/translator/database.go +++ b/internal/translator/database.go @@ -184,14 +184,14 @@ func (v *visitor) extractCreateCollectionArgs(ctx *mongodb.CreateCollectionConte return } case "size": - if val, ok := toInt64(opt.Value); ok { + if val, ok := ToInt64(opt.Value); ok { v.operation.CollectionSize = &val } else { v.err = fmt.Errorf("createCollection() size must be a number") return } case "max": - if val, ok := toInt64(opt.Value); ok { + if val, ok := ToInt64(opt.Value); ok { v.operation.CollectionMax = &val } else { v.err = fmt.Errorf("createCollection() max must be a number") diff --git a/internal/translator/helpers.go b/internal/translator/helpers.go index 8ef06fc..b0f194a 100644 --- a/internal/translator/helpers.go +++ b/internal/translator/helpers.go @@ -182,8 +182,8 @@ func convertRegExpConstructor(ctx mongodb.IRegExpConstructorContext) (bson.Regex return bson.Regex{Pattern: pattern, Options: options}, nil } -// toInt64 converts various numeric types to int64. -func toInt64(v any) (int64, bool) { +// ToInt64 converts various numeric types to int64. +func ToInt64(v any) (int64, bool) { switch n := v.(type) { case int: return int64(n), true @@ -197,8 +197,8 @@ func toInt64(v any) (int64, bool) { return 0, false } -// toInt32 converts various numeric types to int32. -func toInt32(v any) (int32, bool) { +// ToInt32 converts various numeric types to int32. +func ToInt32(v any) (int32, bool) { switch n := v.(type) { case int: return int32(n), true