diff --git a/admin_test.go b/admin_test.go index 7c76376..3856873 100644 --- a/admin_test.go +++ b/admin_test.go @@ -92,6 +92,27 @@ func TestCreateIndexWithOptions(t *testing.T) { }) } +// TestCreateIndexArithmeticTTL pins the BYT-9950 statement: arithmetic +// expressions are not supported, and Execute must return a ParseError instead +// of silently skipping the statement. +func TestCreateIndexArithmeticTTL(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() + + gc := gomongo.NewClient(db.Client) + + _, err := gc.Execute(ctx, dbName, `db.cs_customer_frequency.createIndex( + { trans_date: 1 }, + { expireAfterSeconds: 90 * 24 * 60 * 60, name: "cs_customer_frequency_idx2" } +);`) + var pe *gomongo.ParseError + require.ErrorAs(t, err, &pe) + }) +} + func TestDropIndex(t *testing.T) { testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) { dbName := fmt.Sprintf("testdb_drop_idx_%s", db.Name) diff --git a/go.mod b/go.mod index 693d6bf..c4935de 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/bytebase/gomongo go 1.25.7 require ( - github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722 + github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635 github.com/google/uuid v1.6.0 github.com/stretchr/testify v1.11.1 github.com/testcontainers/testcontainers-go v0.41.0 diff --git a/go.sum b/go.sum index 2188140..d1509d9 100644 --- a/go.sum +++ b/go.sum @@ -6,8 +6,8 @@ github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEK github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= -github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722 h1:zeMIExVxJVNoU3DZfrKrMMrdZTmUJ264bihZ3fDNcqo= -github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722/go.mod h1:EVG8nQbNPUnUBGKn6J0niSGOs3jgfKo3TciY22R4bfo= +github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635 h1:JBDU5vAImt6qBPy4bUDgWhuuc2NQzR6WTMtKRL51YjE= +github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635/go.mod h1:Gp5RN3FM07f9/FFOTZCeu8duTAIMeuinkjjBCrs2Dw4= 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/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= diff --git a/internal/translator/translator.go b/internal/translator/translator.go index e15a0b5..c66e296 100644 --- a/internal/translator/translator.go +++ b/internal/translator/translator.go @@ -5,10 +5,13 @@ import ( "github.com/bytebase/gomongo/types" "github.com/bytebase/omni/mongo" + "github.com/bytebase/omni/mongo/ast" "github.com/bytebase/omni/mongo/parser" ) -// Parse parses a MongoDB shell statement and returns the operation. +// Parse parses a single MongoDB shell statement and returns the operation. +// The input must contain exactly one executable statement: parse errors and +// extra statements are rejected rather than silently dropped. // // Input that contains no executable statements (e.g., only comments or // whitespace) is treated as a no-op: Parse returns an Operation with @@ -28,13 +31,23 @@ func Parse(statement string) (*Operation, error) { return nil, err } - // Find the first non-empty statement. + var node ast.Node for _, s := range stmts { - if !s.Empty() { - return translateNode(s.AST) + if s.Empty() { + continue } + if node != nil { + return nil, &ParseError{ + Line: s.Start.Line, + Column: s.Start.Column, + Message: "expected a single statement, got multiple", + } + } + node = s.AST } - - // Comment-only / whitespace-only input: no-op, no error. - return &Operation{OpType: types.OpNoOp}, nil + if node == nil { + // Comment-only / whitespace-only input: no-op, no error. + return &Operation{OpType: types.OpNoOp}, nil + } + return translateNode(node) } diff --git a/internal/translator/translator_test.go b/internal/translator/translator_test.go new file mode 100644 index 0000000..2192494 --- /dev/null +++ b/internal/translator/translator_test.go @@ -0,0 +1,77 @@ +package translator + +import ( + "errors" + "testing" + + "github.com/bytebase/gomongo/types" +) + +// TestParseSingleStatementContract pins the strict single-statement contract: +// parse errors and extra statements are rejected rather than silently dropped +// (BYT-9950: dropped statements made migrations appear to skip commands). +func TestParseSingleStatementContract(t *testing.T) { + t.Run("valid single statement", func(t *testing.T) { + op, err := Parse(`db.users.find({ name: "alice" })`) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if op.OpType != types.OpFind { + t.Errorf("expected OpFind, got %v", op.OpType) + } + }) + + t.Run("comment-only input is a no-op", func(t *testing.T) { + op, err := Parse("// just a comment\n") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if op.OpType != types.OpNoOp { + t.Errorf("expected OpNoOp, got %v", op.OpType) + } + }) + + t.Run("invalid statement is rejected", func(t *testing.T) { + _, err := Parse("db.users.find({ name: })") + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("expected *ParseError, got %T: %v", err, err) + } + }) + + t.Run("valid statement followed by invalid is rejected", func(t *testing.T) { + _, err := Parse("db.users.find();\nthis is not mongosh") + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("expected *ParseError, got %T: %v", err, err) + } + if pe.Line != 2 { + t.Errorf("expected error on line 2, got %d", pe.Line) + } + }) + + t.Run("multiple valid statements are rejected", func(t *testing.T) { + _, err := Parse("db.users.find();\ndb.orders.find();") + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("expected *ParseError, got %T: %v", err, err) + } + if pe.Line != 2 { + t.Errorf("expected error on line 2, got %d", pe.Line) + } + }) +} + +// TestParseCreateIndexArithmeticTTL pins the BYT-9950 statement: arithmetic +// expressions are not supported, and Parse must surface the error instead of +// silently dropping the statement. +func TestParseCreateIndexArithmeticTTL(t *testing.T) { + _, err := Parse(`db.cs_customer_frequency.createIndex( + { trans_date: 1 }, + { expireAfterSeconds: 90 * 24 * 60 * 60, name: "cs_customer_frequency_idx2" } +);`) + var pe *ParseError + if !errors.As(err, &pe) { + t.Fatalf("expected *ParseError, got %T: %v", err, err) + } +}