Skip to content

Migrate solution from .NET 7 to .NET 9 - #67

Open
devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1786361194-net9-migration
Open

devin-ai-integration[bot] wants to merge 2 commits into
mainfrom
devin/1786361194-net9-migration

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 10, 2026

Copy link
Copy Markdown

Summary

Retargets all six projects to net9.0 and moves the dependency graph onto the 9.x lines (EF Core / OpenAPI 9.0.18, Npgsql 9.0.5 + Npgsql EF provider & NodaTime 9.0.4, Swashbuckle 9.0.6, HealthChecks.NpgSql 9.0.0, Mapster 7.4.0), plus current stable test tooling (Test.Sdk 18.8.1, xunit 2.9.3, runner.visualstudio 3.1.5, FakeItEasy 9.0.1, coverlet 6.0.4).

Dockerfile base images bumped to aspnet:9.0 / sdk:9.0; CI now uses dotnet-version: 9.0.x with actions/checkout@v4 and actions/setup-dotnet@v4 (it was previously pinned to 6.0.x, i.e. not even matching the old target).

No source changes were needed — restore, build, and test all succeed on net9.0 (20/20 tests pass) with no new warnings beyond the three pre-existing nullable warnings (Program.cs AddNpgSql connection string, two null literals in EducationsControllerTests).

Link to Devin session: https://app.devin.ai/sessions/b6a7699f1e004c54b18271326667eac3
Requested by: @detectiveharree


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 potential issues.

Open in Devin Review

Comment on lines +10 to +16
<PackageReference Include="Microsoft.EntityFrameworkCore" Version="9.0.18" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="9.0.18">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.10" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="7.0.10">
<PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="9.0.18" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="9.0.18">

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: EF migration snapshot still records EF Core 7 product version

The retarget updates EF Core packages to 9.0.18, but the existing migration artifacts still declare .HasAnnotation("ProductVersion", "7.0.10") (src/Infrastructure/Migrations/SampleDbContextModelSnapshot.cs:20 and src/Infrastructure/Migrations/20240113141226_Initialize.Designer.cs:23). This is metadata only and does not break existing migrations, but the next Add-Migration under EF 9 will regenerate the snapshot; teams sometimes prefer regenerating explicitly during a major-version migration so the checked-in snapshot matches the toolchain.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread Dockerfile
Comment on lines +1 to 3
FROM mcr.microsoft.com/dotnet/aspnet:9.0 AS base
WORKDIR /app
EXPOSE 5000

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Base image default port change in .NET 8+ only masked by compose env var

mcr.microsoft.com/dotnet/aspnet:9.0 defaults to ASPNETCORE_HTTP_PORTS=8080 (unlike the 7.0 image, which defaulted to port 80). The Dockerfile keeps EXPOSE 5000 and relies on ASPNETCORE_URLS=http://+:5000 supplied by docker-compose.yml. Running the image standalone (docker run -p 5000:5000) will now silently listen on 8080 instead, so consider setting ENV ASPNETCORE_HTTP_PORTS=5000 (or ASPNETCORE_URLS) in the Dockerfile to keep the image self-consistent.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +13 to 19
<PackageReference Include="FakeItEasy" Version="9.0.1" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="18.8.1" />
<PackageReference Include="xunit" Version="2.9.3" />
<PackageReference Include="xunit.runner.visualstudio" Version="3.1.5">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Test tooling major bumps worth verifying in CI

Microsoft.NET.Test.Sdk moves to an 18.x major and xunit.runner.visualstudio to 3.1.5 while xunit remains on the v2 line (2.9.3). The v3 VSTest runner does support xunit v2 assemblies, but the combination plus the Test.Sdk major bump changes the default test host/platform behavior, so confirm dotnet test --no-build in .github/workflows/dotnet.yml still discovers and runs all 20 tests rather than reporting zero tests as a pass.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration

Copy link
Copy Markdown
Author

Runtime testing of the .NET 9 migration

Tested end-to-end with docker compose up -d --build against the real Postgres container, driving the Educations API through Swagger UI and cross-checking every write with psql.

Testing surfaced two runtime regressions caused by the version bumps; both are now fixed in this PR (commit 9d3ce9d).

Fixed 1: docker-compose.yml connection string invalid under Npgsql 9

With the original compose config, every endpoint returned HTTP 500 even though Postgres was up and seeded:

Stock compose returns 500

The string used the legacy key IntegratedSecurity=true. Isolated with a standalone console app on the exact same string:

Npgsql Result
7.0.6 (pre-PR) OPEN OK
8.0.3 ArgumentException: Couldn't set integratedsecurity
9.0.5 (this PR) ArgumentException: Couldn't set integratedsecurity

EducationsController swallows the exception, so the only symptom is a bare 500 with nothing in the logs.

-ConnectionStrings__DefaultConnection=User ID=sara;Password=mysecretpassword;Server=postgres_container;Port=5432;Database=SampleDB;IntegratedSecurity=true;Pooling=true;
+ConnectionStrings__DefaultConnection=Host=postgres_container;Port=5432;Database=SampleDB;Username=sara;Password=mysecretpassword;Pooling=true;

appsettings.json was already in the modern Host=...;Username=... form.

Fixed 2: README's update-database step failed under EF Core 9

Microsoft.EntityFrameworkCore.Migrations.PendingModelChangesWarning:
The model for context 'SampleDbContext' changes each time it is built.

SampleDbContext.OnModelCreating seeded with Guid.NewGuid(); EF Core 7 logged this as a warning, EF Core 9 promotes it to an error. The seed id is now hard-coded to the one already baked into 20240113141226_Initialize (c92ea179-…), so there is no schema or data change. dotnet ef migrations has-pending-model-changes now reports No changes have been made to the model since the last migration.

Verified on .NET 9

Full CRUD lifecycle: create → read → update → restart the API container → delete, cross-checked in Postgres.

Update persisted across container restart

Swashbuckle 9 renders correctly and EF Core 9's generated migration SQL applies cleanly:

Swagger UI on .NET 9

Pre-existing (not a regression): /health returns 404 — left unchanged

Program.cs calls AddHealthChecks().AddNpgSql(...) but never app.MapHealthChecks("/health"), so the endpoint has never existed. Out of scope for this migration.

/health 404

Full assertion list
Check Result
aspnet:9.0 / sdk:9.0 images build; containers start; Kestrel on :5000 pass
EF Core 9 migration SQL applies to Postgres pass
dotnet ef reports no pending model changes (after fix 2) pass
Swagger UI (Swashbuckle 9) loads, 5 ops + schema pass
Compose config serves requests (after fix 1) pass
GET list → seed row pass
POST → 200 + server-assigned GUID pass
GET by id pass
PUT → 200, change persisted on re-GET pass
Survives docker restart webapi_container pass
DELETE → 200, row gone in API and psql pass
/health responds fail (pre-existing 404)
Unit tests (20/20) on net9.0 pass

Caveat: the CRUD run was executed with a temporary local override supplying the corrected connection string; that same fix is now committed, but the compose stack has not been re-run against the committed config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant