From 2e7ad6bbc32d946e00da50de79a3436b29a930f9 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Tue, 24 Mar 2026 11:39:16 +0800 Subject: [PATCH 1/5] feat: support deprecated mongosh methods and Int32 string args (BYT-9080) Fix 5 mongosh compatibility issues that caused unnecessary fallbacks: - pretty(): treat as no-op cursor method (output already formatted) - insert(): translate to insertOne/insertMany based on argument type - count(): translate to countDocuments - update(): translate to updateOne/updateMany based on multi option - Int32("string")/NumberInt("string"): accept string arguments in parser Co-Authored-By: Claude Opus 4.6 (1M context) --- compat_test.go | 209 ++++++++++++++++++++++++++++ go.mod | 2 + go.sum | 2 - internal/translator/bson_helpers.go | 10 +- internal/translator/collection.go | 207 +++++++++++++++++++++++++++ internal/translator/visitor.go | 11 ++ 6 files changed, 436 insertions(+), 5 deletions(-) create mode 100644 compat_test.go diff --git a/compat_test.go b/compat_test.go new file mode 100644 index 0000000..a29e008 --- /dev/null +++ b/compat_test.go @@ -0,0 +1,209 @@ +package gomongo_test + +import ( + "context" + "fmt" + "testing" + + "github.com/bytebase/gomongo" + "github.com/bytebase/gomongo/internal/testutil" + "github.com/stretchr/testify/require" + "go.mongodb.org/mongo-driver/v2/bson" +) + +// Tests for mongosh compatibility fixes (BYT-9080). + +func TestPrettyNoOp(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_pretty_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertMany(ctx, []any{ + bson.M{"name": "alice", "age": 30}, + bson.M{"name": "bob", "age": 25}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // pretty() should be a no-op cursor method + result, err := gc.Execute(ctx, dbName, `db.users.find().pretty()`) + require.NoError(t, err) + require.Equal(t, 2, len(result.Value)) + + // pretty() chained after sort + result, err = gc.Execute(ctx, dbName, `db.users.find().sort({name: 1}).pretty()`) + require.NoError(t, err) + require.Equal(t, 2, len(result.Value)) + rows := valuesToStrings(result.Value) + require.Contains(t, rows[0], `"alice"`) + + // aggregate().pretty() + result, err = gc.Execute(ctx, dbName, `db.users.aggregate([{$match: {name: "alice"}}]).pretty()`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + }) +} + +func TestDeprecatedInsert(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_insert_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) + + // insert() with a single document should behave like insertOne() + result, err := gc.Execute(ctx, dbName, `db.users.insert({name: "alice", age: 30})`) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify the document was inserted + result, err = gc.Execute(ctx, dbName, `db.users.find({name: "alice"})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + + // insert() with an array should behave like insertMany() + result, err = gc.Execute(ctx, dbName, `db.users.insert([{name: "bob"}, {name: "charlie"}])`) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify all documents + result, err = gc.Execute(ctx, dbName, `db.users.find()`) + require.NoError(t, err) + require.Equal(t, 3, len(result.Value)) + + // insert() via getCollection + result, err = gc.Execute(ctx, dbName, `db.getCollection("users").insert({name: "dave"})`) + require.NoError(t, err) + require.NotNil(t, result) + + result, err = gc.Execute(ctx, dbName, `db.users.find()`) + require.NoError(t, err) + require.Equal(t, 4, len(result.Value)) + }) +} + +func TestDeprecatedCount(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_count_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + collection := db.Client.Database(dbName).Collection("items") + _, err := collection.InsertMany(ctx, []any{ + bson.M{"name": "a", "status": "active"}, + bson.M{"name": "b", "status": "active"}, + bson.M{"name": "c", "status": "inactive"}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // count() with no args should return total count + result, err := gc.Execute(ctx, dbName, `db.items.count()`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + require.Equal(t, int64(3), result.Value[0]) + + // count({}) with empty filter + result, err = gc.Execute(ctx, dbName, `db.items.count({})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + require.Equal(t, int64(3), result.Value[0]) + + // count() with filter + result, err = gc.Execute(ctx, dbName, `db.items.count({status: "active"})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + require.Equal(t, int64(2), result.Value[0]) + }) +} + +func TestDeprecatedUpdate(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_update_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertMany(ctx, []any{ + bson.M{"name": "alice", "age": 30}, + bson.M{"name": "bob", "age": 25}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // update() defaults to updateOne behavior + result, err := gc.Execute(ctx, dbName, `db.users.update({name: "alice"}, {$set: {age: 31}})`) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify the update + findResult, err := gc.Execute(ctx, dbName, `db.users.findOne({name: "alice"})`) + require.NoError(t, err) + require.Equal(t, 1, len(findResult.Value)) + require.Contains(t, valueToJSON(findResult.Value[0]), `31`) + + // update() with multi: true should behave like updateMany + _, err = collection.InsertOne(ctx, bson.M{"name": "alice", "age": 20}) + require.NoError(t, err) + + result, err = gc.Execute(ctx, dbName, `db.users.update({name: "alice"}, {$set: {age: 99}}, {multi: true})`) + require.NoError(t, err) + require.NotNil(t, result) + + // Verify both alices were updated + findResult, err = gc.Execute(ctx, dbName, `db.users.find({name: "alice"})`) + require.NoError(t, err) + require.Equal(t, 2, len(findResult.Value)) + for _, v := range findResult.Value { + require.Contains(t, valueToJSON(v), `99`) + } + + // update() via getCollection + result, err = gc.Execute(ctx, dbName, `db.getCollection("users").update({name: "bob"}, {$set: {age: 26}})`) + require.NoError(t, err) + require.NotNil(t, result) + }) +} + +func TestInt32StringArg(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_int32str_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + gc := gomongo.NewClient(db.Client) + + // Int32("0") should parse the string as int32 + result, err := gc.Execute(ctx, dbName, `db.items.insertOne({val: Int32("123")})`) + require.NoError(t, err) + require.NotNil(t, result) + + findResult, err := gc.Execute(ctx, dbName, `db.items.find()`) + require.NoError(t, err) + require.Equal(t, 1, len(findResult.Value)) + json := valueToJSON(findResult.Value[0]) + require.Contains(t, json, `"val"`) + require.Contains(t, json, `123`) + + // NumberInt("456") should also work + result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: NumberInt("456")})`) + require.NoError(t, err) + require.NotNil(t, result) + + // Long("789") already supported in grammar, verify it works end-to-end + result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: Long("789")})`) + require.NoError(t, err) + require.NotNil(t, result) + + // NumberLong("1774250313") — from real user report + result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: NumberLong("1774250313")})`) + require.NoError(t, err) + require.NotNil(t, result) + }) +} diff --git a/go.mod b/go.mod index a9a84e6..11e3df5 100644 --- a/go.mod +++ b/go.mod @@ -76,3 +76,5 @@ require ( ) replace github.com/antlr4-go/antlr/v4 => github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920 + +replace github.com/bytebase/parser => ../parser diff --git a/go.sum b/go.sum index 75686f2..9f9e98c 100644 --- a/go.sum +++ b/go.sum @@ -8,8 +8,6 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920 h1:IfmPt5o5R70NKtOrs+QHOoCgViYZelZysGxVBvV4ybA= github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920/go.mod h1:ykhjIPiv0IWpu3OGXCHdz2eUSe8UNGGD6baqjs8jSuU= -github.com/bytebase/parser v0.0.0-20260130090605-effef73942d9 h1:q5MnVPWlV/p3MPe60SysVoUaEnyeS6+OOOk0F3DLLK8= -github.com/bytebase/parser v0.0.0-20260130090605-effef73942d9/go.mod h1:jeak/EfutSOAuWKvrFIT2IZunhWprM7oTFBRgZ9RCxo= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= diff --git a/internal/translator/bson_helpers.go b/internal/translator/bson_helpers.go index 0a39aa4..73f6a26 100644 --- a/internal/translator/bson_helpers.go +++ b/internal/translator/bson_helpers.go @@ -178,18 +178,22 @@ func convertLongHelper(ctx mongodb.ILongHelperContext) (int64, error) { return strconv.ParseInt(numStr, 10, 64) } -// convertInt32Helper converts Int32(123) or NumberInt(123) to int32. +// convertInt32Helper converts Int32(123), Int32("123"), NumberInt(123), or NumberInt("123") to int32. func convertInt32Helper(ctx mongodb.IInt32HelperContext) (int32, error) { helper, ok := ctx.(*mongodb.Int32HelperContext) if !ok { return 0, fmt.Errorf("invalid Int32 helper context") } - if helper.NUMBER() == nil { + var numStr string + if helper.NUMBER() != nil { + numStr = helper.NUMBER().GetText() + } else if helper.StringLiteral() != nil { + numStr = unquoteString(helper.StringLiteral().GetText()) + } else { return 0, nil } - numStr := helper.NUMBER().GetText() i, err := strconv.ParseInt(numStr, 10, 32) if err != nil { return 0, err diff --git a/internal/translator/collection.go b/internal/translator/collection.go index 0857ee1..6bf9240 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -4,6 +4,7 @@ import ( "fmt" "strconv" + "github.com/bytebase/gomongo/types" "github.com/bytebase/parser/mongodb" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -2225,3 +2226,209 @@ func (v *visitor) extractCreateIndexesArgs(ctx mongodb.ICreateIndexesMethodConte return } } + +// extractDeprecatedInsertArgs handles the deprecated insert() method. +// insert(doc) → insertOne, insert([doc1, doc2]) → insertMany. +func (v *visitor) extractDeprecatedInsertArgs(ctx mongodb.ICollectionInsertMethodContext) { + method, ok := ctx.(*mongodb.CollectionInsertMethodContext) + if !ok { + return + } + + args := method.Arguments() + if args == nil { + v.err = fmt.Errorf("insert() requires a document or array argument") + return + } + + argsCtx, ok := args.(*mongodb.ArgumentsContext) + if !ok { + v.err = fmt.Errorf("insert() requires a document or array argument") + return + } + + allArgs := argsCtx.AllArgument() + if len(allArgs) == 0 { + v.err = fmt.Errorf("insert() requires a document or array argument") + return + } + + firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("insert() requires a document or array argument") + return + } + + valueCtx := firstArg.Value() + if valueCtx == nil { + v.err = fmt.Errorf("insert() requires a document or array argument") + return + } + + switch val := valueCtx.(type) { + case *mongodb.DocumentValueContext: + // Single document → insertOne + v.operation.OpType = types.OpInsertOne + doc, err := convertDocument(val.Document()) + if err != nil { + v.err = fmt.Errorf("invalid document: %w", err) + return + } + v.operation.Document = doc + case *mongodb.ArrayValueContext: + // Array of documents → insertMany + v.operation.OpType = types.OpInsertMany + arr, err := convertArray(val.Array()) + if err != nil { + v.err = fmt.Errorf("invalid documents array: %w", err) + return + } + var docs []bson.D + for i, elem := range arr { + doc, ok := elem.(bson.D) + if !ok { + v.err = fmt.Errorf("insert() element %d must be a document", i) + return + } + docs = append(docs, doc) + } + v.operation.Documents = docs + default: + v.err = fmt.Errorf("insert() argument must be a document or array") + return + } +} + +// extractDeprecatedCountArgs handles the deprecated count() collection method. +// count(filter?) → countDocuments(filter?). +func (v *visitor) extractDeprecatedCountArgs(ctx mongodb.ICollectionCountMethodContext) { + method, ok := ctx.(*mongodb.CollectionCountMethodContext) + if !ok { + return + } + v.extractArgumentsForCountDocuments(method.Arguments()) +} + +// extractDeprecatedUpdateArgs handles the deprecated update() method. +// update(filter, update, options?) — options.multi determines updateOne vs updateMany. +func (v *visitor) extractDeprecatedUpdateArgs(ctx mongodb.IUpdateMethodContext) { + method, ok := ctx.(*mongodb.UpdateMethodContext) + if !ok { + return + } + + args := method.Arguments() + if args == nil { + v.err = fmt.Errorf("update() requires filter and update arguments") + return + } + + argsCtx, ok := args.(*mongodb.ArgumentsContext) + if !ok { + v.err = fmt.Errorf("update() requires filter and update arguments") + return + } + + allArgs := argsCtx.AllArgument() + if len(allArgs) < 2 { + v.err = fmt.Errorf("update() requires filter and update arguments") + return + } + + // Default to updateOne; check for multi option later. + v.operation.OpType = types.OpUpdateOne + + // First argument: filter + firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("update() filter must be a document") + return + } + filterValueCtx := firstArg.Value() + if filterValueCtx == nil { + v.err = fmt.Errorf("update() filter must be a document") + return + } + filterDocValue, ok := filterValueCtx.(*mongodb.DocumentValueContext) + if !ok { + v.err = fmt.Errorf("update() filter must be a document") + return + } + filter, err := convertDocument(filterDocValue.Document()) + if err != nil { + v.err = fmt.Errorf("invalid filter: %w", err) + return + } + v.operation.Filter = filter + + // Second argument: update + secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) + if !ok { + v.err = fmt.Errorf("update() update must be a document or array") + return + } + updateValueCtx := secondArg.Value() + if updateValueCtx == nil { + v.err = fmt.Errorf("update() update must be a document or array") + return + } + switch uv := updateValueCtx.(type) { + case *mongodb.DocumentValueContext: + update, err := convertDocument(uv.Document()) + if err != nil { + v.err = fmt.Errorf("invalid update: %w", err) + return + } + v.operation.Update = update + case *mongodb.ArrayValueContext: + pipeline, err := convertArray(uv.Array()) + if err != nil { + v.err = fmt.Errorf("invalid update pipeline: %w", err) + return + } + v.operation.Update = pipeline + default: + v.err = fmt.Errorf("update() update must be a document or array") + return + } + + // Third argument: options (optional) + if len(allArgs) >= 3 { + thirdArg, ok := allArgs[2].(*mongodb.ArgumentContext) + if !ok { + return + } + optionsValueCtx := thirdArg.Value() + if optionsValueCtx == nil { + return + } + optionsDocValue, ok := optionsValueCtx.(*mongodb.DocumentValueContext) + if !ok { + v.err = fmt.Errorf("update() 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 "multi": + if val, ok := opt.Value.(bool); ok && val { + v.operation.OpType = types.OpUpdateMany + } + case "upsert": + if val, ok := opt.Value.(bool); ok { + v.operation.Upsert = &val + } + default: + v.err = &UnsupportedOptionError{ + Method: "update()", + Option: opt.Key, + } + return + } + } + } +} diff --git a/internal/translator/visitor.go b/internal/translator/visitor.go index 06bcca6..2deb0be 100644 --- a/internal/translator/visitor.go +++ b/internal/translator/visitor.go @@ -234,6 +234,15 @@ func (v *visitor) visitCollectionMethodCall(ctx mongodb.ICollectionMethodCallCon v.operation.OpType = types.OpRenameCollection v.extractRenameCollectionArgs(mc.RenameCollectionMethod()) + // Deprecated methods (translated to modern equivalents) + case mc.CollectionInsertMethod() != nil: + v.extractDeprecatedInsertArgs(mc.CollectionInsertMethod()) + case mc.CollectionCountMethod() != nil: + v.operation.OpType = types.OpCountDocuments + v.extractDeprecatedCountArgs(mc.CollectionCountMethod()) + case mc.UpdateMethod() != nil: + v.extractDeprecatedUpdateArgs(mc.UpdateMethod()) + // Collection information commands case mc.StatsMethod() != nil: v.operation.OpType = types.OpCollectionStats @@ -281,6 +290,8 @@ func (v *visitor) visitCursorMethodCall(ctx mongodb.ICursorMethodCallContext) { v.extractMax(mc.MaxMethod()) case mc.MinMethod() != nil: v.extractMin(mc.MinMethod()) + case mc.PrettyMethod() != nil: + // pretty() is a no-op — output is already formatted. default: methodName := extractMethodNameFromText(mc.GetText()) if methodName != "" { From ae930bde566f5a2da40ab11e9aa6b11d406c9688 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Tue, 24 Mar 2026 11:48:29 +0800 Subject: [PATCH 2/5] feat: support pretty() no-op and Int32 string args (BYT-9080) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 2 mongosh compatibility issues that caused unnecessary fallbacks: - pretty(): treat as no-op cursor method (output already formatted) - Int32("string")/NumberInt("string"): accept string arguments in parser Deprecated methods (insert, count, update) intentionally left as unsupported to keep gomongo simple — they fall back to mongosh. Co-Authored-By: Claude Opus 4.6 (1M context) --- compat_test.go | 124 ------------------ internal/translator/collection.go | 206 ------------------------------ internal/translator/visitor.go | 9 -- 3 files changed, 339 deletions(-) diff --git a/compat_test.go b/compat_test.go index a29e008..6a98562 100644 --- a/compat_test.go +++ b/compat_test.go @@ -47,130 +47,6 @@ func TestPrettyNoOp(t *testing.T) { }) } -func TestDeprecatedInsert(t *testing.T) { - testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { - dbName := fmt.Sprintf("testdb_insert_%s", db.Name) - defer testutil.CleanupDatabase(t, db.Client, dbName) - - ctx := context.Background() - gc := gomongo.NewClient(db.Client) - - // insert() with a single document should behave like insertOne() - result, err := gc.Execute(ctx, dbName, `db.users.insert({name: "alice", age: 30})`) - require.NoError(t, err) - require.NotNil(t, result) - - // Verify the document was inserted - result, err = gc.Execute(ctx, dbName, `db.users.find({name: "alice"})`) - require.NoError(t, err) - require.Equal(t, 1, len(result.Value)) - - // insert() with an array should behave like insertMany() - result, err = gc.Execute(ctx, dbName, `db.users.insert([{name: "bob"}, {name: "charlie"}])`) - require.NoError(t, err) - require.NotNil(t, result) - - // Verify all documents - result, err = gc.Execute(ctx, dbName, `db.users.find()`) - require.NoError(t, err) - require.Equal(t, 3, len(result.Value)) - - // insert() via getCollection - result, err = gc.Execute(ctx, dbName, `db.getCollection("users").insert({name: "dave"})`) - require.NoError(t, err) - require.NotNil(t, result) - - result, err = gc.Execute(ctx, dbName, `db.users.find()`) - require.NoError(t, err) - require.Equal(t, 4, len(result.Value)) - }) -} - -func TestDeprecatedCount(t *testing.T) { - testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { - dbName := fmt.Sprintf("testdb_count_%s", db.Name) - defer testutil.CleanupDatabase(t, db.Client, dbName) - - ctx := context.Background() - collection := db.Client.Database(dbName).Collection("items") - _, err := collection.InsertMany(ctx, []any{ - bson.M{"name": "a", "status": "active"}, - bson.M{"name": "b", "status": "active"}, - bson.M{"name": "c", "status": "inactive"}, - }) - require.NoError(t, err) - - gc := gomongo.NewClient(db.Client) - - // count() with no args should return total count - result, err := gc.Execute(ctx, dbName, `db.items.count()`) - require.NoError(t, err) - require.Equal(t, 1, len(result.Value)) - require.Equal(t, int64(3), result.Value[0]) - - // count({}) with empty filter - result, err = gc.Execute(ctx, dbName, `db.items.count({})`) - require.NoError(t, err) - require.Equal(t, 1, len(result.Value)) - require.Equal(t, int64(3), result.Value[0]) - - // count() with filter - result, err = gc.Execute(ctx, dbName, `db.items.count({status: "active"})`) - require.NoError(t, err) - require.Equal(t, 1, len(result.Value)) - require.Equal(t, int64(2), result.Value[0]) - }) -} - -func TestDeprecatedUpdate(t *testing.T) { - testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { - dbName := fmt.Sprintf("testdb_update_%s", db.Name) - defer testutil.CleanupDatabase(t, db.Client, dbName) - - ctx := context.Background() - collection := db.Client.Database(dbName).Collection("users") - _, err := collection.InsertMany(ctx, []any{ - bson.M{"name": "alice", "age": 30}, - bson.M{"name": "bob", "age": 25}, - }) - require.NoError(t, err) - - gc := gomongo.NewClient(db.Client) - - // update() defaults to updateOne behavior - result, err := gc.Execute(ctx, dbName, `db.users.update({name: "alice"}, {$set: {age: 31}})`) - require.NoError(t, err) - require.NotNil(t, result) - - // Verify the update - findResult, err := gc.Execute(ctx, dbName, `db.users.findOne({name: "alice"})`) - require.NoError(t, err) - require.Equal(t, 1, len(findResult.Value)) - require.Contains(t, valueToJSON(findResult.Value[0]), `31`) - - // update() with multi: true should behave like updateMany - _, err = collection.InsertOne(ctx, bson.M{"name": "alice", "age": 20}) - require.NoError(t, err) - - result, err = gc.Execute(ctx, dbName, `db.users.update({name: "alice"}, {$set: {age: 99}}, {multi: true})`) - require.NoError(t, err) - require.NotNil(t, result) - - // Verify both alices were updated - findResult, err = gc.Execute(ctx, dbName, `db.users.find({name: "alice"})`) - require.NoError(t, err) - require.Equal(t, 2, len(findResult.Value)) - for _, v := range findResult.Value { - require.Contains(t, valueToJSON(v), `99`) - } - - // update() via getCollection - result, err = gc.Execute(ctx, dbName, `db.getCollection("users").update({name: "bob"}, {$set: {age: 26}})`) - require.NoError(t, err) - require.NotNil(t, result) - }) -} - func TestInt32StringArg(t *testing.T) { testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { dbName := fmt.Sprintf("testdb_int32str_%s", db.Name) diff --git a/internal/translator/collection.go b/internal/translator/collection.go index 6bf9240..571161b 100644 --- a/internal/translator/collection.go +++ b/internal/translator/collection.go @@ -4,7 +4,6 @@ import ( "fmt" "strconv" - "github.com/bytebase/gomongo/types" "github.com/bytebase/parser/mongodb" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -2227,208 +2226,3 @@ func (v *visitor) extractCreateIndexesArgs(ctx mongodb.ICreateIndexesMethodConte } } -// extractDeprecatedInsertArgs handles the deprecated insert() method. -// insert(doc) → insertOne, insert([doc1, doc2]) → insertMany. -func (v *visitor) extractDeprecatedInsertArgs(ctx mongodb.ICollectionInsertMethodContext) { - method, ok := ctx.(*mongodb.CollectionInsertMethodContext) - if !ok { - return - } - - args := method.Arguments() - if args == nil { - v.err = fmt.Errorf("insert() requires a document or array argument") - return - } - - argsCtx, ok := args.(*mongodb.ArgumentsContext) - if !ok { - v.err = fmt.Errorf("insert() requires a document or array argument") - return - } - - allArgs := argsCtx.AllArgument() - if len(allArgs) == 0 { - v.err = fmt.Errorf("insert() requires a document or array argument") - return - } - - firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) - if !ok { - v.err = fmt.Errorf("insert() requires a document or array argument") - return - } - - valueCtx := firstArg.Value() - if valueCtx == nil { - v.err = fmt.Errorf("insert() requires a document or array argument") - return - } - - switch val := valueCtx.(type) { - case *mongodb.DocumentValueContext: - // Single document → insertOne - v.operation.OpType = types.OpInsertOne - doc, err := convertDocument(val.Document()) - if err != nil { - v.err = fmt.Errorf("invalid document: %w", err) - return - } - v.operation.Document = doc - case *mongodb.ArrayValueContext: - // Array of documents → insertMany - v.operation.OpType = types.OpInsertMany - arr, err := convertArray(val.Array()) - if err != nil { - v.err = fmt.Errorf("invalid documents array: %w", err) - return - } - var docs []bson.D - for i, elem := range arr { - doc, ok := elem.(bson.D) - if !ok { - v.err = fmt.Errorf("insert() element %d must be a document", i) - return - } - docs = append(docs, doc) - } - v.operation.Documents = docs - default: - v.err = fmt.Errorf("insert() argument must be a document or array") - return - } -} - -// extractDeprecatedCountArgs handles the deprecated count() collection method. -// count(filter?) → countDocuments(filter?). -func (v *visitor) extractDeprecatedCountArgs(ctx mongodb.ICollectionCountMethodContext) { - method, ok := ctx.(*mongodb.CollectionCountMethodContext) - if !ok { - return - } - v.extractArgumentsForCountDocuments(method.Arguments()) -} - -// extractDeprecatedUpdateArgs handles the deprecated update() method. -// update(filter, update, options?) — options.multi determines updateOne vs updateMany. -func (v *visitor) extractDeprecatedUpdateArgs(ctx mongodb.IUpdateMethodContext) { - method, ok := ctx.(*mongodb.UpdateMethodContext) - if !ok { - return - } - - args := method.Arguments() - if args == nil { - v.err = fmt.Errorf("update() requires filter and update arguments") - return - } - - argsCtx, ok := args.(*mongodb.ArgumentsContext) - if !ok { - v.err = fmt.Errorf("update() requires filter and update arguments") - return - } - - allArgs := argsCtx.AllArgument() - if len(allArgs) < 2 { - v.err = fmt.Errorf("update() requires filter and update arguments") - return - } - - // Default to updateOne; check for multi option later. - v.operation.OpType = types.OpUpdateOne - - // First argument: filter - firstArg, ok := allArgs[0].(*mongodb.ArgumentContext) - if !ok { - v.err = fmt.Errorf("update() filter must be a document") - return - } - filterValueCtx := firstArg.Value() - if filterValueCtx == nil { - v.err = fmt.Errorf("update() filter must be a document") - return - } - filterDocValue, ok := filterValueCtx.(*mongodb.DocumentValueContext) - if !ok { - v.err = fmt.Errorf("update() filter must be a document") - return - } - filter, err := convertDocument(filterDocValue.Document()) - if err != nil { - v.err = fmt.Errorf("invalid filter: %w", err) - return - } - v.operation.Filter = filter - - // Second argument: update - secondArg, ok := allArgs[1].(*mongodb.ArgumentContext) - if !ok { - v.err = fmt.Errorf("update() update must be a document or array") - return - } - updateValueCtx := secondArg.Value() - if updateValueCtx == nil { - v.err = fmt.Errorf("update() update must be a document or array") - return - } - switch uv := updateValueCtx.(type) { - case *mongodb.DocumentValueContext: - update, err := convertDocument(uv.Document()) - if err != nil { - v.err = fmt.Errorf("invalid update: %w", err) - return - } - v.operation.Update = update - case *mongodb.ArrayValueContext: - pipeline, err := convertArray(uv.Array()) - if err != nil { - v.err = fmt.Errorf("invalid update pipeline: %w", err) - return - } - v.operation.Update = pipeline - default: - v.err = fmt.Errorf("update() update must be a document or array") - return - } - - // Third argument: options (optional) - if len(allArgs) >= 3 { - thirdArg, ok := allArgs[2].(*mongodb.ArgumentContext) - if !ok { - return - } - optionsValueCtx := thirdArg.Value() - if optionsValueCtx == nil { - return - } - optionsDocValue, ok := optionsValueCtx.(*mongodb.DocumentValueContext) - if !ok { - v.err = fmt.Errorf("update() 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 "multi": - if val, ok := opt.Value.(bool); ok && val { - v.operation.OpType = types.OpUpdateMany - } - case "upsert": - if val, ok := opt.Value.(bool); ok { - v.operation.Upsert = &val - } - default: - v.err = &UnsupportedOptionError{ - Method: "update()", - Option: opt.Key, - } - return - } - } - } -} diff --git a/internal/translator/visitor.go b/internal/translator/visitor.go index 2deb0be..1c612c6 100644 --- a/internal/translator/visitor.go +++ b/internal/translator/visitor.go @@ -234,15 +234,6 @@ func (v *visitor) visitCollectionMethodCall(ctx mongodb.ICollectionMethodCallCon v.operation.OpType = types.OpRenameCollection v.extractRenameCollectionArgs(mc.RenameCollectionMethod()) - // Deprecated methods (translated to modern equivalents) - case mc.CollectionInsertMethod() != nil: - v.extractDeprecatedInsertArgs(mc.CollectionInsertMethod()) - case mc.CollectionCountMethod() != nil: - v.operation.OpType = types.OpCountDocuments - v.extractDeprecatedCountArgs(mc.CollectionCountMethod()) - case mc.UpdateMethod() != nil: - v.extractDeprecatedUpdateArgs(mc.UpdateMethod()) - // Collection information commands case mc.StatsMethod() != nil: v.operation.OpType = types.OpCollectionStats From 05cb02db9e800b3db7e3d5959e342d36d204e935 Mon Sep 17 00:00:00 2001 From: h3n4l Date: Tue, 24 Mar 2026 11:53:08 +0800 Subject: [PATCH 3/5] refactor: move tests to proper files, mark replace as temporary - Move TestPrettyNoOp to collection_test.go (cursor method behavior) - Move TestInt32StringArg to bson_helpers_test.go (BSON helper behavior) - Delete compat_test.go - Add TODO comment on replace directive (remove after parser published) Co-Authored-By: Claude Opus 4.6 (1M context) --- collection_test.go | 34 ++++++++++ compat_test.go | 85 ------------------------ go.mod | 1 + internal/translator/bson_helpers_test.go | 30 +++++++++ 4 files changed, 65 insertions(+), 85 deletions(-) delete mode 100644 compat_test.go diff --git a/collection_test.go b/collection_test.go index 0aaf4ba..c137fb1 100644 --- a/collection_test.go +++ b/collection_test.go @@ -2262,3 +2262,37 @@ func TestCursorMinMethod(t *testing.T) { require.Equal(t, 2, len(result.Value)) }) } + +func TestPrettyNoOp(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := fmt.Sprintf("testdb_pretty_%s", db.Name) + defer testutil.CleanupDatabase(t, db.Client, dbName) + + ctx := context.Background() + collection := db.Client.Database(dbName).Collection("users") + _, err := collection.InsertMany(ctx, []any{ + bson.M{"name": "alice", "age": 30}, + bson.M{"name": "bob", "age": 25}, + }) + require.NoError(t, err) + + gc := gomongo.NewClient(db.Client) + + // pretty() should be a no-op cursor method + result, err := gc.Execute(ctx, dbName, `db.users.find().pretty()`) + require.NoError(t, err) + require.Equal(t, 2, len(result.Value)) + + // pretty() chained after sort + result, err = gc.Execute(ctx, dbName, `db.users.find().sort({name: 1}).pretty()`) + require.NoError(t, err) + require.Equal(t, 2, len(result.Value)) + rows := valuesToStrings(result.Value) + require.Contains(t, rows[0], `"alice"`) + + // aggregate().pretty() + result, err = gc.Execute(ctx, dbName, `db.users.aggregate([{$match: {name: "alice"}}]).pretty()`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + }) +} diff --git a/compat_test.go b/compat_test.go deleted file mode 100644 index 6a98562..0000000 --- a/compat_test.go +++ /dev/null @@ -1,85 +0,0 @@ -package gomongo_test - -import ( - "context" - "fmt" - "testing" - - "github.com/bytebase/gomongo" - "github.com/bytebase/gomongo/internal/testutil" - "github.com/stretchr/testify/require" - "go.mongodb.org/mongo-driver/v2/bson" -) - -// Tests for mongosh compatibility fixes (BYT-9080). - -func TestPrettyNoOp(t *testing.T) { - testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { - dbName := fmt.Sprintf("testdb_pretty_%s", db.Name) - defer testutil.CleanupDatabase(t, db.Client, dbName) - - ctx := context.Background() - collection := db.Client.Database(dbName).Collection("users") - _, err := collection.InsertMany(ctx, []any{ - bson.M{"name": "alice", "age": 30}, - bson.M{"name": "bob", "age": 25}, - }) - require.NoError(t, err) - - gc := gomongo.NewClient(db.Client) - - // pretty() should be a no-op cursor method - result, err := gc.Execute(ctx, dbName, `db.users.find().pretty()`) - require.NoError(t, err) - require.Equal(t, 2, len(result.Value)) - - // pretty() chained after sort - result, err = gc.Execute(ctx, dbName, `db.users.find().sort({name: 1}).pretty()`) - require.NoError(t, err) - require.Equal(t, 2, len(result.Value)) - rows := valuesToStrings(result.Value) - require.Contains(t, rows[0], `"alice"`) - - // aggregate().pretty() - result, err = gc.Execute(ctx, dbName, `db.users.aggregate([{$match: {name: "alice"}}]).pretty()`) - require.NoError(t, err) - require.Equal(t, 1, len(result.Value)) - }) -} - -func TestInt32StringArg(t *testing.T) { - testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { - dbName := fmt.Sprintf("testdb_int32str_%s", db.Name) - defer testutil.CleanupDatabase(t, db.Client, dbName) - - ctx := context.Background() - gc := gomongo.NewClient(db.Client) - - // Int32("0") should parse the string as int32 - result, err := gc.Execute(ctx, dbName, `db.items.insertOne({val: Int32("123")})`) - require.NoError(t, err) - require.NotNil(t, result) - - findResult, err := gc.Execute(ctx, dbName, `db.items.find()`) - require.NoError(t, err) - require.Equal(t, 1, len(findResult.Value)) - json := valueToJSON(findResult.Value[0]) - require.Contains(t, json, `"val"`) - require.Contains(t, json, `123`) - - // NumberInt("456") should also work - result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: NumberInt("456")})`) - require.NoError(t, err) - require.NotNil(t, result) - - // Long("789") already supported in grammar, verify it works end-to-end - result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: Long("789")})`) - require.NoError(t, err) - require.NotNil(t, result) - - // NumberLong("1774250313") — from real user report - result, err = gc.Execute(ctx, dbName, `db.items.insertOne({val: NumberLong("1774250313")})`) - require.NoError(t, err) - require.NotNil(t, result) - }) -} diff --git a/go.mod b/go.mod index 11e3df5..c8b59cf 100644 --- a/go.mod +++ b/go.mod @@ -77,4 +77,5 @@ require ( replace github.com/antlr4-go/antlr/v4 => github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920 +// TODO: Remove after parser is published with Int32 string arg support. replace github.com/bytebase/parser => ../parser diff --git a/internal/translator/bson_helpers_test.go b/internal/translator/bson_helpers_test.go index bd6fb05..9906b2a 100644 --- a/internal/translator/bson_helpers_test.go +++ b/internal/translator/bson_helpers_test.go @@ -204,3 +204,33 @@ func TestDecimal128Helper(t *testing.T) { require.NotNil(t, price) }) } + +func TestInt32StringArg(t *testing.T) { + testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { + dbName := "testdb_int32str_" + db.Name + defer testutil.CleanupDatabase(t, db.Client, dbName) + + gc := gomongo.NewClient(db.Client) + ctx := context.Background() + + // Int32("123") should parse the string as int32 + _, err := gc.Execute(ctx, dbName, `db.test.insertOne({val: Int32("123")})`) + require.NoError(t, err) + + result, err := gc.Execute(ctx, dbName, `db.test.findOne({})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + doc, ok := result.Value[0].(bson.D) + require.True(t, ok) + val := getDocField(doc, "val") + require.Equal(t, int32(123), val) + + // NumberInt("456") should also work + _, err = gc.Execute(ctx, dbName, `db.test.insertOne({val2: NumberInt("456")})`) + require.NoError(t, err) + + // NumberLong("1774250313") — from real user report + _, err = gc.Execute(ctx, dbName, `db.test.insertOne({val3: NumberLong("1774250313")})`) + require.NoError(t, err) + }) +} From 9c7c2387361699b5be9e08a6403c4f96ab78af6e Mon Sep 17 00:00:00 2001 From: h3n4l Date: Tue, 24 Mar 2026 12:01:34 +0800 Subject: [PATCH 4/5] chore: upgrade parser to 3b4d6e7, remove replace directive Update github.com/bytebase/parser to include Int32/NumberInt string argument support (parser#64). Remove temporary local replace directive. Co-Authored-By: Claude Opus 4.6 (1M context) --- go.mod | 5 +---- go.sum | 2 ++ 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index c8b59cf..3bcfc1d 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.24.5 require ( github.com/antlr4-go/antlr/v4 v4.13.1 - github.com/bytebase/parser v0.0.0-20260130090605-effef73942d9 + github.com/bytebase/parser v0.0.0-20260324035613-3b4d6e704551 github.com/google/uuid v1.6.0 github.com/stretchr/testify v1.11.1 github.com/testcontainers/testcontainers-go v0.40.0 @@ -76,6 +76,3 @@ require ( ) replace github.com/antlr4-go/antlr/v4 => github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920 - -// TODO: Remove after parser is published with Int32 string arg support. -replace github.com/bytebase/parser => ../parser diff --git a/go.sum b/go.sum index 9f9e98c..c1d7adb 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERo github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920 h1:IfmPt5o5R70NKtOrs+QHOoCgViYZelZysGxVBvV4ybA= github.com/bytebase/antlr/v4 v4.0.0-20240827034948-8c385f108920/go.mod h1:ykhjIPiv0IWpu3OGXCHdz2eUSe8UNGGD6baqjs8jSuU= +github.com/bytebase/parser v0.0.0-20260324035613-3b4d6e704551 h1:jmOIMyIEXoMWZ+TK91STbBMPRom7S/tWxosxYZDg9jk= +github.com/bytebase/parser v0.0.0-20260324035613-3b4d6e704551/go.mod h1:jeak/EfutSOAuWKvrFIT2IZunhWprM7oTFBRgZ9RCxo= github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8= github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI= From 541b383aeefa9730e236d1868e37c4da19680dda Mon Sep 17 00:00:00 2001 From: h3n4l Date: Tue, 24 Mar 2026 14:13:31 +0800 Subject: [PATCH 5/5] test: strengthen assertions per review feedback - Assert NumberInt("456") stores as int32(456), not just no-error - Assert NumberLong("1774250313") stores as int64(1774250313) - Insert in reverse order in pretty() sort test so assertion is meaningful Co-Authored-By: Claude Opus 4.6 (1M context) --- collection_test.go | 5 +++-- internal/translator/bson_helpers_test.go | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/collection_test.go b/collection_test.go index c137fb1..7d911a1 100644 --- a/collection_test.go +++ b/collection_test.go @@ -2270,9 +2270,10 @@ func TestPrettyNoOp(t *testing.T) { ctx := context.Background() collection := db.Client.Database(dbName).Collection("users") + // Insert in reverse alphabetical order so sort assertions are meaningful. _, err := collection.InsertMany(ctx, []any{ - bson.M{"name": "alice", "age": 30}, bson.M{"name": "bob", "age": 25}, + bson.M{"name": "alice", "age": 30}, }) require.NoError(t, err) @@ -2283,7 +2284,7 @@ func TestPrettyNoOp(t *testing.T) { require.NoError(t, err) require.Equal(t, 2, len(result.Value)) - // pretty() chained after sort + // pretty() chained after sort — inserted bob first, sort should put alice first result, err = gc.Execute(ctx, dbName, `db.users.find().sort({name: 1}).pretty()`) require.NoError(t, err) require.Equal(t, 2, len(result.Value)) diff --git a/internal/translator/bson_helpers_test.go b/internal/translator/bson_helpers_test.go index 9906b2a..c90dcc6 100644 --- a/internal/translator/bson_helpers_test.go +++ b/internal/translator/bson_helpers_test.go @@ -229,8 +229,24 @@ func TestInt32StringArg(t *testing.T) { _, err = gc.Execute(ctx, dbName, `db.test.insertOne({val2: NumberInt("456")})`) require.NoError(t, err) + result, err = gc.Execute(ctx, dbName, `db.test.findOne({val2: {$exists: true}})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + doc, ok = result.Value[0].(bson.D) + require.True(t, ok) + val2 := getDocField(doc, "val2") + require.Equal(t, int32(456), val2) + // NumberLong("1774250313") — from real user report _, err = gc.Execute(ctx, dbName, `db.test.insertOne({val3: NumberLong("1774250313")})`) require.NoError(t, err) + + result, err = gc.Execute(ctx, dbName, `db.test.findOne({val3: {$exists: true}})`) + require.NoError(t, err) + require.Equal(t, 1, len(result.Value)) + doc, ok = result.Value[0].(bson.D) + require.True(t, ok) + val3 := getDocField(doc, "val3") + require.Equal(t, int64(1774250313), val3) }) }