Skip to content

Commit 67ba6e2

Browse files
h3n4lclaude
andauthored
feat: enforce single-statement contract in translator.Parse (#31)
* feat: enforce single-statement contract in translator.Parse With omni's mongo.Parse now strict (any statement parse error fails the whole input), translator.Parse no longer silently ignores content after the first statement: extra statements are rejected with a ParseError instead of being dropped. Comment-only input remains a no-op. Also pins the BYT-9950 statement: createIndex with a constant arithmetic expireAfterSeconds (90 * 24 * 60 * 60) folds to an int32 TTL and creates the index end to end. Requires bumping github.com/bytebase/omni to a version with strict mongo.Parse (folding landed in omni#393); the bump lands in this PR once the omni change merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: bump omni for strict mongo.Parse and arithmetic folding Picks up bytebase/omni#395 (Parse fails on any statement parse error) and bytebase/omni#393 (constant arithmetic folding), which the new single-statement contract and TTL-index tests rely on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: pin arithmetic TTL as a parse error after folding revert omni#393 (constant arithmetic folding) was reverted in omni#396: arithmetic expressions in mongosh statements are intentionally unsupported. The BYT-9950 tests now assert that translator.Parse and Execute return a ParseError instead of silently skipping the statement. Bumps omni past the revert. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 8526a44 commit 67ba6e2

5 files changed

Lines changed: 121 additions & 10 deletions

File tree

admin_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,27 @@ func TestCreateIndexWithOptions(t *testing.T) {
9292
})
9393
}
9494

95+
// TestCreateIndexArithmeticTTL pins the BYT-9950 statement: arithmetic
96+
// expressions are not supported, and Execute must return a ParseError instead
97+
// of silently skipping the statement.
98+
func TestCreateIndexArithmeticTTL(t *testing.T) {
99+
testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) {
100+
dbName := fmt.Sprintf("testdb_create_idx_ttl_%s", db.Name)
101+
defer testutil.CleanupDatabase(t, db.Client, dbName)
102+
103+
ctx := context.Background()
104+
105+
gc := gomongo.NewClient(db.Client)
106+
107+
_, err := gc.Execute(ctx, dbName, `db.cs_customer_frequency.createIndex(
108+
{ trans_date: 1 },
109+
{ expireAfterSeconds: 90 * 24 * 60 * 60, name: "cs_customer_frequency_idx2" }
110+
);`)
111+
var pe *gomongo.ParseError
112+
require.ErrorAs(t, err, &pe)
113+
})
114+
}
115+
95116
func TestDropIndex(t *testing.T) {
96117
testutil.RunOnAllDBs(t, func(t *testing.T, db testutil.TestDB) {
97118
dbName := fmt.Sprintf("testdb_drop_idx_%s", db.Name)

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ module github.com/bytebase/gomongo
33
go 1.25.7
44

55
require (
6-
github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722
6+
github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635
77
github.com/google/uuid v1.6.0
88
github.com/stretchr/testify v1.11.1
99
github.com/testcontainers/testcontainers-go v0.41.0

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEK
66
github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
77
github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY=
88
github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU=
9-
github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722 h1:zeMIExVxJVNoU3DZfrKrMMrdZTmUJ264bihZ3fDNcqo=
10-
github.com/bytebase/omni v0.0.0-20260509021101-01140a7b9722/go.mod h1:EVG8nQbNPUnUBGKn6J0niSGOs3jgfKo3TciY22R4bfo=
9+
github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635 h1:JBDU5vAImt6qBPy4bUDgWhuuc2NQzR6WTMtKRL51YjE=
10+
github.com/bytebase/omni v0.0.0-20260730024920-8e82223f7635/go.mod h1:Gp5RN3FM07f9/FFOTZCeu8duTAIMeuinkjjBCrs2Dw4=
1111
github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK343L8=
1212
github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE=
1313
github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM=

internal/translator/translator.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ import (
55

66
"github.com/bytebase/gomongo/types"
77
"github.com/bytebase/omni/mongo"
8+
"github.com/bytebase/omni/mongo/ast"
89
"github.com/bytebase/omni/mongo/parser"
910
)
1011

11-
// Parse parses a MongoDB shell statement and returns the operation.
12+
// Parse parses a single MongoDB shell statement and returns the operation.
13+
// The input must contain exactly one executable statement: parse errors and
14+
// extra statements are rejected rather than silently dropped.
1215
//
1316
// Input that contains no executable statements (e.g., only comments or
1417
// whitespace) is treated as a no-op: Parse returns an Operation with
@@ -28,13 +31,23 @@ func Parse(statement string) (*Operation, error) {
2831
return nil, err
2932
}
3033

31-
// Find the first non-empty statement.
34+
var node ast.Node
3235
for _, s := range stmts {
33-
if !s.Empty() {
34-
return translateNode(s.AST)
36+
if s.Empty() {
37+
continue
3538
}
39+
if node != nil {
40+
return nil, &ParseError{
41+
Line: s.Start.Line,
42+
Column: s.Start.Column,
43+
Message: "expected a single statement, got multiple",
44+
}
45+
}
46+
node = s.AST
3647
}
37-
38-
// Comment-only / whitespace-only input: no-op, no error.
39-
return &Operation{OpType: types.OpNoOp}, nil
48+
if node == nil {
49+
// Comment-only / whitespace-only input: no-op, no error.
50+
return &Operation{OpType: types.OpNoOp}, nil
51+
}
52+
return translateNode(node)
4053
}
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package translator
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/bytebase/gomongo/types"
8+
)
9+
10+
// TestParseSingleStatementContract pins the strict single-statement contract:
11+
// parse errors and extra statements are rejected rather than silently dropped
12+
// (BYT-9950: dropped statements made migrations appear to skip commands).
13+
func TestParseSingleStatementContract(t *testing.T) {
14+
t.Run("valid single statement", func(t *testing.T) {
15+
op, err := Parse(`db.users.find({ name: "alice" })`)
16+
if err != nil {
17+
t.Fatalf("unexpected error: %v", err)
18+
}
19+
if op.OpType != types.OpFind {
20+
t.Errorf("expected OpFind, got %v", op.OpType)
21+
}
22+
})
23+
24+
t.Run("comment-only input is a no-op", func(t *testing.T) {
25+
op, err := Parse("// just a comment\n")
26+
if err != nil {
27+
t.Fatalf("unexpected error: %v", err)
28+
}
29+
if op.OpType != types.OpNoOp {
30+
t.Errorf("expected OpNoOp, got %v", op.OpType)
31+
}
32+
})
33+
34+
t.Run("invalid statement is rejected", func(t *testing.T) {
35+
_, err := Parse("db.users.find({ name: })")
36+
var pe *ParseError
37+
if !errors.As(err, &pe) {
38+
t.Fatalf("expected *ParseError, got %T: %v", err, err)
39+
}
40+
})
41+
42+
t.Run("valid statement followed by invalid is rejected", func(t *testing.T) {
43+
_, err := Parse("db.users.find();\nthis is not mongosh")
44+
var pe *ParseError
45+
if !errors.As(err, &pe) {
46+
t.Fatalf("expected *ParseError, got %T: %v", err, err)
47+
}
48+
if pe.Line != 2 {
49+
t.Errorf("expected error on line 2, got %d", pe.Line)
50+
}
51+
})
52+
53+
t.Run("multiple valid statements are rejected", func(t *testing.T) {
54+
_, err := Parse("db.users.find();\ndb.orders.find();")
55+
var pe *ParseError
56+
if !errors.As(err, &pe) {
57+
t.Fatalf("expected *ParseError, got %T: %v", err, err)
58+
}
59+
if pe.Line != 2 {
60+
t.Errorf("expected error on line 2, got %d", pe.Line)
61+
}
62+
})
63+
}
64+
65+
// TestParseCreateIndexArithmeticTTL pins the BYT-9950 statement: arithmetic
66+
// expressions are not supported, and Parse must surface the error instead of
67+
// silently dropping the statement.
68+
func TestParseCreateIndexArithmeticTTL(t *testing.T) {
69+
_, err := Parse(`db.cs_customer_frequency.createIndex(
70+
{ trans_date: 1 },
71+
{ expireAfterSeconds: 90 * 24 * 60 * 60, name: "cs_customer_frequency_idx2" }
72+
);`)
73+
var pe *ParseError
74+
if !errors.As(err, &pe) {
75+
t.Fatalf("expected *ParseError, got %T: %v", err, err)
76+
}
77+
}

0 commit comments

Comments
 (0)