Skip to content

Commit 5a510cf

Browse files
hhvrchhvrc
committed
Fix integration test harness and duplicate Npgsql enum registrations
- Test fixture now injects config via UseSetting so values reach Program.cs before ApiConfig validation (ConfigureAppConfiguration overlays only apply at Build() in minimal hosting) - Make RepoServerContext.MapEnums the single source of enum mappings with pinned DB names; drop the duplicate HasPostgresEnum block and map enums in the migration-tool path too - Register only the pooled DbContext factory and derive scoped contexts from it; the AddDbContextPool + AddPooledDbContextFactory combo applied MapEnums twice and broke type mapping on first model use - Order advisories by CLR severity rank in memory since MapEnum creates PG enum labels alphabetically - Opt into the Microsoft.Testing.Platform dotnet test runner via global.json and update the documented test command
1 parent afd8f6b commit 5a510cf

8 files changed

Lines changed: 70 additions & 55 deletions

File tree

‎CLAUDE.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,8 @@ docker/
134134

135135
## Commands
136136
- Build: `dotnet build RepositoryServer/RepositoryServer.csproj`
137-
- Test: `dotnet test RepositoryServer.Tests/RepositoryServer.Tests.csproj`
137+
- Test: `dotnet test --project RepositoryServer.Tests/RepositoryServer.Tests.csproj`
138+
(Microsoft.Testing.Platform runner, opted in via `global.json`; the legacy positional
139+
project path is rejected — use `--project`)
138140
- Docker: `docker build -f docker/RepositoryServer.Dockerfile .`
139141
- Migration: `dotnet ef migrations add <Name> --project RepositoryServer`

‎RepositoryServer.Tests/Integration/Tests/AdvisoriesAdminControllerTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public async Task Get_ReturnsAllAdvisoriesOrderedBySeverity()
8888
await Assert.That(body).IsNotNull();
8989
await Assert.That(body!).HasCount(3);
9090

91-
// Ordered by the PG enum ordinal: Critical (0), Warning (1), Info (2).
91+
// Ordered by severity rank: Critical (0), Warning (1), Info (2).
9292
await Assert.That(body[0].Severity).IsEqualTo("critical");
9393
await Assert.That(body[1].Severity).IsEqualTo("warning");
9494
await Assert.That(body[2].Severity).IsEqualTo("info");

‎RepositoryServer.Tests/Integration/WebApplicationFactory.cs‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -88,26 +88,32 @@ protected override void ConfigureWebHost(IWebHostBuilder builder)
8888
{
8989
builder.UseEnvironment("Testing");
9090

91-
builder.ConfigureAppConfiguration((_, cfg) =>
91+
// Program.cs reads and validates ApiConfig BEFORE builder.Build(). For minimal-hosting
92+
// apps, ConfigureAppConfiguration overlays are only applied at Build() time — too late.
93+
// UseSetting goes through host configuration, which the deferred host builder flattens
94+
// into command-line args for Program.Main, so Program's `.AddCommandLine(args)` picks
95+
// these up before the config is validated.
96+
var settings = new Dictionary<string, string?>
9297
{
93-
cfg.AddInMemoryCollection(new Dictionary<string, string?>
94-
{
95-
["Db:Conn"] = PostgreSql.Container.GetConnectionString(),
96-
["Db:SkipMigration"] = "true",
97-
["Db:Debug"] = "false",
98-
99-
["AdminToken"] = TestAdminToken.Value,
100-
101-
["Repo:CdnBaseUrl"] = "https://cdn-test.openshock.example/repo",
102-
103-
["Firmware:CdnBaseUrl"] = "https://cdn-test.openshock.example/firmware",
104-
["Firmware:CiCd:Audience"] = "openshock-repository-server-test",
105-
["Firmware:Storage:Type"] = "Local",
106-
["Firmware:Storage:Local:BasePath"] = _cdnStoragePath,
107-
["Firmware:StagedReleaseTtl"] = "01:00:00",
108-
["Firmware:EditingReleaseTtl"] = "7.00:00:00",
109-
});
110-
});
98+
["Db:Conn"] = PostgreSql.Container.GetConnectionString(),
99+
["Db:SkipMigration"] = "true",
100+
["Db:Debug"] = "false",
101+
102+
["AdminToken"] = TestAdminToken.Value,
103+
104+
["Repo:CdnBaseUrl"] = "https://cdn-test.openshock.example/repo",
105+
106+
["Firmware:CdnBaseUrl"] = "https://cdn-test.openshock.example/firmware",
107+
["Firmware:CiCd:Audience"] = "openshock-repository-server-test",
108+
["Firmware:Storage:Type"] = "Local",
109+
["Firmware:Storage:Local:BasePath"] = _cdnStoragePath,
110+
["Firmware:StagedReleaseTtl"] = "01:00:00",
111+
["Firmware:EditingReleaseTtl"] = "7.00:00:00",
112+
};
113+
foreach (var (key, value) in settings)
114+
{
115+
builder.UseSetting(key, value);
116+
}
111117

112118
builder.ConfigureTestServices(services =>
113119
{

‎RepositoryServer/Controllers/V2/Firmware/Admin/AdvisoriesController.cs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,14 @@ public async Task<IActionResult> CreateAdvisory([FromBody] UpsertFirmwareAdvisor
4848
[HttpGet]
4949
public async Task<IActionResult> ListAdvisories(CancellationToken ct)
5050
{
51-
var rows = await _db.FirmwareAdvisories
51+
// Postgres sorts enum columns by label creation order (alphabetical with MapEnum),
52+
// not severity rank — order on the CLR enum value in memory instead.
53+
var rows = await _db.FirmwareAdvisories.ToListAsync(ct);
54+
55+
return Ok(rows
5256
.OrderBy(a => a.Severity)
5357
.ThenBy(a => a.Title)
54-
.ToListAsync(ct);
55-
56-
return Ok(rows.Select(ToDto));
58+
.Select(ToDto));
5759
}
5860

5961
[HttpPut("{id:guid}")]

‎RepositoryServer/Controllers/V2/Firmware/ManifestController.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,10 @@ public async Task<IActionResult> GetManifest(CancellationToken ct)
101101
})
102102
.ToList();
103103

104-
var advisories = await _db.FirmwareAdvisories
104+
// Postgres sorts enum columns by label creation order (alphabetical with MapEnum),
105+
// not severity rank — order on the CLR enum value in memory instead.
106+
var advisoryRows = await _db.FirmwareAdvisories.ToListAsync(ct);
107+
var advisories = advisoryRows
105108
.OrderBy(a => a.Severity)
106109
.ThenBy(a => a.Title)
107110
.Select(a => new FirmwareAdvisoryDto
@@ -112,7 +115,7 @@ public async Task<IActionResult> GetManifest(CancellationToken ct)
112115
AffectedVersions = a.AffectedVersions,
113116
Url = a.Url
114117
})
115-
.ToListAsync(ct);
118+
.ToList();
116119

117120
var response = new FirmwareManifestResponse
118121
{

‎RepositoryServer/Program.cs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,15 +169,16 @@
169169

170170
// <---- Postgres EF Core ---->
171171

172-
builder.Services.AddDbContextPool<RepoServerContext>(dbBuilder =>
173-
{
174-
RepoServerContext.ConfigureOptionsBuilder(dbBuilder, config.Db.Conn, config.Db.Debug);
175-
});
176-
172+
// Register the pooled factory only and derive scoped contexts from it. Adding
173+
// AddDbContextPool alongside would apply a second options configuration to the same
174+
// options builder (EF applies all registered configurations cumulatively), duplicating
175+
// the Npgsql enum definitions and breaking type mapping.
177176
builder.Services.AddPooledDbContextFactory<RepoServerContext>(dbBuilder =>
178177
{
179178
RepoServerContext.ConfigureOptionsBuilder(dbBuilder, config.Db.Conn, config.Db.Debug);
180179
});
180+
builder.Services.AddScoped(sp =>
181+
sp.GetRequiredService<IDbContextFactory<RepoServerContext>>().CreateDbContext());
181182

182183

183184

‎RepositoryServer/RepoServerDb/RepoServerContext.cs‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
3030
{
3131
if (_migrationTool)
3232
{
33-
// Migration tooling: plain connection string, no MapEnum (to avoid duplicate enum schemas)
34-
optionsBuilder.UseNpgsql("Host=localhost;Database=repo-server;Username=openshock;Password=openshock");
33+
optionsBuilder.UseNpgsql("Host=localhost;Database=repo-server;Username=openshock;Password=openshock", MapEnums);
3534
optionsBuilder.UseExceptionProcessor();
3635
optionsBuilder.EnableSensitiveDataLogging();
3736
optionsBuilder.EnableDetailedErrors();
@@ -57,19 +56,28 @@ public RepoServerContext(DbContextOptions<RepoServerContext> options)
5756
{
5857
}
5958

59+
/// <summary>
60+
/// Single source of truth for PostgreSQL enum mappings. Since Npgsql 9, MapEnum also
61+
/// injects the enum definitions into the EF model, so no HasPostgresEnum calls may
62+
/// coexist with these — that duplicates the definitions and breaks type mapping.
63+
/// Names are pinned explicitly because they don't all match the default translation
64+
/// (firmware_release_note_type vs. ReleaseNoteSectionType).
65+
/// </summary>
66+
protected static void MapEnums(Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.NpgsqlDbContextOptionsBuilder npgsqlBuilder)
67+
{
68+
npgsqlBuilder.MapEnum<ReleaseChannel>("release_channel");
69+
npgsqlBuilder.MapEnum<FirmwareArtifactType>("firmware_artifact_type");
70+
npgsqlBuilder.MapEnum<ReleaseNoteSectionType>("firmware_release_note_type");
71+
npgsqlBuilder.MapEnum<FirmwareChipArchitecture>("firmware_chip_architecture");
72+
npgsqlBuilder.MapEnum<ReleaseStatus>("release_status");
73+
npgsqlBuilder.MapEnum<RepositoryProvider>("repository_provider");
74+
npgsqlBuilder.MapEnum<AdvisorySeverity>("advisory_severity");
75+
}
76+
6077
public static void ConfigureOptionsBuilder(DbContextOptionsBuilder optionsBuilder, string connectionString,
6178
bool debug)
6279
{
63-
optionsBuilder.UseNpgsql(connectionString, npgsqlBuilder =>
64-
{
65-
npgsqlBuilder.MapEnum<ReleaseChannel>();
66-
npgsqlBuilder.MapEnum<FirmwareArtifactType>();
67-
npgsqlBuilder.MapEnum<ReleaseNoteSectionType>();
68-
npgsqlBuilder.MapEnum<FirmwareChipArchitecture>();
69-
npgsqlBuilder.MapEnum<ReleaseStatus>();
70-
npgsqlBuilder.MapEnum<RepositoryProvider>();
71-
npgsqlBuilder.MapEnum<AdvisorySeverity>();
72-
});
80+
optionsBuilder.UseNpgsql(connectionString, MapEnums);
7381

7482
optionsBuilder.UseExceptionProcessor();
7583

@@ -112,16 +120,6 @@ public static void ConfigureOptionsBuilder(DbContextOptionsBuilder optionsBuilde
112120

113121
protected override void OnModelCreating(ModelBuilder modelBuilder)
114122
{
115-
// PostgreSQL enum mappings
116-
modelBuilder
117-
.HasPostgresEnum("release_channel", ["stable", "beta", "develop"])
118-
.HasPostgresEnum("firmware_artifact_type", ["merged", "app", "bootloader", "partitions", "static_fs"])
119-
.HasPostgresEnum("firmware_release_note_type", ["warning", "info", "breaking", "section"])
120-
.HasPostgresEnum("firmware_chip_architecture", ["xtensa", "risc_v"])
121-
.HasPostgresEnum("release_status", ["staging", "editing", "published", "archived", "aborted"])
122-
.HasPostgresEnum("repository_provider", ["github"])
123-
.HasPostgresEnum("advisory_severity", ["critical", "warning", "info"]);
124-
125123
// Shared source-code repository registry
126124
modelBuilder.Entity<SourceRepository>(entity =>
127125
{

‎global.json‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,8 @@
33
"version": "10.0.0",
44
"rollForward": "latestMajor",
55
"allowPrerelease": false
6+
},
7+
"test": {
8+
"runner": "Microsoft.Testing.Platform"
69
}
7-
}
10+
}

0 commit comments

Comments
 (0)