Skip to content

Commit e5c7d30

Browse files
sebastian-ederersebastian-ederer
committed
fix: add guard in TimescaleCSharpMigrationOperationGenerator to avoid empty migrationBuilder statement to lead to syntax errors.
1 parent fe0efc0 commit e5c7d30

File tree

3 files changed

+239
-6
lines changed

3 files changed

+239
-6
lines changed

‎src/Eftdb.Design/TimescaleCSharpMigrationOperationGenerator.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ protected override void Generate(MigrationOperation operation, IndentedStringBui
1717
ReorderPolicyOperationGenerator? reorderPolicyOperationGenerator = null;
1818
ContinuousAggregateOperationGenerator? continuousAggregateOperationGenerator = null;
1919

20-
List<string> statements = [];
20+
List<string> statements;
2121
bool suppressTransaction = false;
2222

2323
switch (operation)
@@ -60,7 +60,14 @@ protected override void Generate(MigrationOperation operation, IndentedStringBui
6060

6161
default:
6262
base.Generate(operation, builder);
63-
break;
63+
return;
64+
}
65+
66+
// Guard: if no statements were generated, output a no-op SQL comment to maintain valid C# syntax.
67+
if (statements.Count == 0)
68+
{
69+
builder.Append(".Sql(@\"-- No SQL generated for this operation\")");
70+
return;
6471
}
6572

6673
SqlBuilderHelper.BuildQueryString(statements, builder, suppressTransaction);

‎tests/Eftdb.Tests/Eftdb.Tests.csproj‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
1818
</PackageReference>
1919
<PackageReference Include="EFCore.NamingConventions" Version="9.0.0" />
20-
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="9.0.8">
21-
<PrivateAssets>all</PrivateAssets>
22-
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
23-
</PackageReference>
20+
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="9.0.8" />
2421
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.0.0" />
2522
<PackageReference Include="Moq" Version="4.20.72" />
2623
<PackageReference Include="Testcontainers.PostgreSql" Version="4.8.1" />

‎tests/Eftdb.Tests/Generators/TimescaleCSharpMigrationOperationGeneratorTests.cs‎

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
using CmdScale.EntityFrameworkCore.TimescaleDB.Design;
2+
using CmdScale.EntityFrameworkCore.TimescaleDB.Operations;
3+
using Microsoft.EntityFrameworkCore.Design;
4+
using Microsoft.EntityFrameworkCore.Infrastructure;
5+
using Microsoft.EntityFrameworkCore.Migrations.Design;
6+
using Moq;
7+
8+
namespace CmdScale.EntityFrameworkCore.TimescaleDB.Tests.Generators
9+
{
10+
#pragma warning disable EF1001 // Internal EF Core API usage.
11+
12+
/// <summary>
13+
/// Tests for TimescaleCSharpMigrationOperationGenerator to ensure proper C# code generation.
14+
/// </summary>
15+
public class TimescaleCSharpMigrationOperationGeneratorTests
16+
{
17+
#region Empty Statements Guard Tests
18+
19+
[Fact]
20+
public void Generate_CreateHypertable_WithValidOperation_GeneratesValidCSharp()
21+
{
22+
// Arrange
23+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
24+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
25+
IndentedStringBuilder builder = new();
26+
27+
CreateHypertableOperation operation = new()
28+
{
29+
TableName = "sensor_data",
30+
Schema = "public",
31+
TimeColumnName = "timestamp",
32+
ChunkTimeInterval = "7 days"
33+
};
34+
35+
// Act
36+
// Note: We call the protected method via the public interface indirectly
37+
// by using reflection or by testing via the public Generate method
38+
generator.Generate("migrationBuilder", [operation], builder);
39+
40+
// Assert
41+
string result = builder.ToString();
42+
Assert.Contains("migrationBuilder", result);
43+
Assert.Contains(".Sql(@\"", result);
44+
Assert.Contains("create_hypertable", result);
45+
Assert.Contains(";", result);
46+
Assert.DoesNotContain("migrationBuilder;", result); // This would be invalid C#
47+
}
48+
49+
[Fact]
50+
public void Generate_CreateHypertable_WithMigrateData_GeneratesValidCSharp()
51+
{
52+
// Arrange
53+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
54+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
55+
IndentedStringBuilder builder = new();
56+
57+
CreateHypertableOperation operation = new()
58+
{
59+
TableName = "sensor_data",
60+
Schema = "public",
61+
TimeColumnName = "timestamp",
62+
ChunkTimeInterval = "7 days",
63+
MigrateData = true
64+
};
65+
66+
// Act
67+
generator.Generate("migrationBuilder", [operation], builder);
68+
69+
// Assert
70+
string result = builder.ToString();
71+
Assert.Contains("migrate_data => true", result);
72+
Assert.DoesNotContain("migrationBuilder;", result);
73+
}
74+
75+
[Fact]
76+
public void Generate_AlterHypertable_WithNoChanges_GeneratesValidCSharpOrNoOp()
77+
{
78+
// Arrange
79+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
80+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
81+
IndentedStringBuilder builder = new();
82+
83+
// An alter operation with no actual changes should still generate valid C#
84+
AlterHypertableOperation operation = new()
85+
{
86+
TableName = "sensor_data",
87+
Schema = "public"
88+
};
89+
90+
// Act
91+
generator.Generate("migrationBuilder", [operation], builder);
92+
93+
// Assert
94+
string result = builder.ToString();
95+
96+
// The result should either be empty (no operation generated) or contain valid C#
97+
// It should NEVER contain just "migrationBuilder;" without a method call
98+
if (!string.IsNullOrWhiteSpace(result))
99+
{
100+
Assert.DoesNotContain("migrationBuilder;", result.Replace(" ", "").Replace("\n", "").Replace("\r", ""));
101+
// If there's content, it should have a proper method call
102+
if (result.Contains("migrationBuilder"))
103+
{
104+
Assert.Contains(".Sql(@\"", result);
105+
}
106+
}
107+
}
108+
109+
[Fact]
110+
public void Generate_AddReorderPolicy_GeneratesValidCSharp()
111+
{
112+
// Arrange
113+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
114+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
115+
IndentedStringBuilder builder = new();
116+
117+
AddReorderPolicyOperation operation = new()
118+
{
119+
TableName = "sensor_data",
120+
Schema = "public",
121+
IndexName = "sensor_data_time_idx"
122+
};
123+
124+
// Act
125+
generator.Generate("migrationBuilder", [operation], builder);
126+
127+
// Assert
128+
string result = builder.ToString();
129+
Assert.Contains("migrationBuilder", result);
130+
Assert.Contains(".Sql(@\"", result);
131+
Assert.Contains("add_reorder_policy", result);
132+
Assert.DoesNotContain("migrationBuilder;", result);
133+
}
134+
135+
[Fact]
136+
public void Generate_DropReorderPolicy_GeneratesValidCSharp()
137+
{
138+
// Arrange
139+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
140+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
141+
IndentedStringBuilder builder = new();
142+
143+
DropReorderPolicyOperation operation = new()
144+
{
145+
TableName = "sensor_data",
146+
Schema = "public"
147+
};
148+
149+
// Act
150+
generator.Generate("migrationBuilder", [operation], builder);
151+
152+
// Assert
153+
string result = builder.ToString();
154+
Assert.Contains("migrationBuilder", result);
155+
Assert.Contains(".Sql(@\"", result);
156+
Assert.Contains("remove_reorder_policy", result);
157+
Assert.DoesNotContain("migrationBuilder;", result);
158+
}
159+
160+
[Fact]
161+
public void Generate_CreateContinuousAggregate_GeneratesValidCSharp()
162+
{
163+
// Arrange
164+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
165+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
166+
IndentedStringBuilder builder = new();
167+
168+
CreateContinuousAggregateOperation operation = new()
169+
{
170+
MaterializedViewName = "hourly_stats",
171+
Schema = "public",
172+
ParentName = "sensor_data",
173+
TimeBucketWidth = "1 hour",
174+
TimeBucketSourceColumn = "timestamp",
175+
TimeBucketGroupBy = true,
176+
AggregateFunctions = ["COUNT(*)"]
177+
};
178+
179+
// Act
180+
generator.Generate("migrationBuilder", [operation], builder);
181+
182+
// Assert
183+
string result = builder.ToString();
184+
Assert.Contains("migrationBuilder", result);
185+
Assert.Contains(".Sql(@\"", result);
186+
Assert.Contains("CREATE MATERIALIZED VIEW", result);
187+
Assert.DoesNotContain("migrationBuilder;", result);
188+
}
189+
190+
[Fact]
191+
public void Generate_DropContinuousAggregate_GeneratesValidCSharp()
192+
{
193+
// Arrange
194+
CSharpMigrationOperationGeneratorDependencies dependencies = CreateDependencies();
195+
TimescaleCSharpMigrationOperationGenerator generator = new(dependencies);
196+
IndentedStringBuilder builder = new();
197+
198+
DropContinuousAggregateOperation operation = new()
199+
{
200+
MaterializedViewName = "hourly_stats",
201+
Schema = "public"
202+
};
203+
204+
// Act
205+
generator.Generate("migrationBuilder", [operation], builder);
206+
207+
// Assert
208+
string result = builder.ToString();
209+
Assert.Contains("migrationBuilder", result);
210+
Assert.Contains(".Sql(@\"", result);
211+
Assert.Contains("DROP MATERIALIZED VIEW", result);
212+
Assert.DoesNotContain("migrationBuilder;", result);
213+
}
214+
215+
#endregion
216+
217+
#region Helper Methods
218+
219+
private static CSharpMigrationOperationGeneratorDependencies CreateDependencies()
220+
{
221+
Mock<ICSharpHelper> mockCSharpHelper = new();
222+
return new CSharpMigrationOperationGeneratorDependencies(mockCSharpHelper.Object);
223+
}
224+
225+
#endregion
226+
}
227+
228+
#pragma warning restore EF1001 // Internal EF Core API usage.
229+
}

0 commit comments

Comments
 (0)