Upgrade backend to .NET 5 #1198

johnthagen · 2021-06-18T13:50:24Z

Closes #1200
Closes #1149

Update to C# 9.0
- Use new is not null syntax
Disable ASP.NET Razor page configuration by switching to AddControllersWithViews(), which is not used by The Combine, as recommended for ASP.NET Core 3+ (https://stackoverflow.com/questions/62251347)
Remove SetCompatibilityVersion no-op (https://stackoverflow.com/a/59748346)
Port to System.Text.Json for all JSON serialization into and out of API endpoints (default for ASP.NET Core 3.0+) (Migrate away from deprecated Newtonsoft JSON serializer #1149)
Set some class attributes used for serialization to properties (get/set) to enable proper discovery during OpenAPI schema generation
Replace C# LGTM usage with a GitHub Action that uses the same underlying CodeQL engine

This change is

codecov-commenter · 2021-06-18T13:51:44Z

Codecov Report

Merging #1198 (39193f0) into master (041ecb7) will decrease coverage by 0.01%.
The diff coverage is 22.22%.

@@            Coverage Diff             @@
##           master    #1198      +/-   ##
==========================================
- Coverage   44.73%   44.71%   -0.02%     
==========================================
  Files         266      266              
  Lines        8068     8077       +9     
  Branches      528      528              
==========================================
+ Hits         3609     3612       +3     
- Misses       4043     4049       +6     
  Partials      416      416

Flag	Coverage Δ
backend	`57.29% <29.72%> (-0.06%)`	⬇️
frontend	`33.45% <5.88%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
Backend/Controllers/InviteController.cs	`0.00% <0.00%> (ø)`
Backend/Controllers/UserController.cs	`37.50% <0.00%> (-1.04%)`	⬇️
Backend/Repositories/ProjectRepository.cs	`0.00% <0.00%> (ø)`
Backend/Repositories/UserRepository.cs	`0.00% <0.00%> (ø)`
Backend/Startup.cs	`0.00% <0.00%> (ø)`
src/api/api/invite-api.ts	`6.38% <0.00%> (ø)`
src/api/api/merge-api.ts	`4.22% <0.00%> (ø)`
src/api/api/user-api.ts	`11.11% <0.00%> (ø)`
src/api/api/user-edit-api.ts	`2.38% <0.00%> (ø)`
src/api/api/user-role-api.ts	`2.40% <0.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 041ecb7...39193f0. Read the comment docs.

johnthagen · 2021-06-18T17:54:32Z

.NET 5 has an improved JSON Serializer: #1200

This would allow us to avoid some limitations with the current serializer:

System.NotSupportedException: Deserialization of reference types without parameterless constructor is not supported. Type 'BackendFramework.Models.UserEditStepWrapper'

jasonleenaylor

Reviewed 10 of 13 files at r3.
Reviewable status: 10 of 13 files reviewed, all discussions resolved (waiting on @imnasnainaec)

docs/user_guide/docs/licenses/backend_licenses.txt, line 92 at r3 (raw file):

Version:2.1.0

Hmm, this is confusing a couple of ways.

Just above we are including the license for version 2.2.0 Given my experience with .Net assembly references we almost certainly aren't using both.
I guess the package versions here don't actually match the asp.net version? So these new ones presumably appeared from the use of System.Text.Json.Serialization?

johnthagen · 2021-06-18T18:07:38Z

docs/user_guide/docs/licenses/backend_licenses.txt, line 92 at r3 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

Version:2.1.0
Hmm, this is confusing a couple of ways.

Just above we are including the license for version 2.2.0 Given my experience with .Net assembly references we almost certainly aren't using both.

I guess the package versions here don't actually match the asp.net version? So these new ones presumably appeared from the use of System.Text.Json.Serialization?

We're including --include-transitive in the dotnet-project-licenses command, so that might have something to do with it? Due to how the dependencies are described in C# packages, it might be difficult for a license checker looking from the side to know how the final linking happens. Perhaps that is why there is duplication?

jasonleenaylor · 2021-06-18T18:55:00Z

Backend/Controllers/UserController.cs, line 41 at r5 (raw file):

[ProducesResponseType(StatusCodes.Status200OK)]
``

Do we need to add 404 here now?

jasonleenaylor

Reviewed 5 of 16 files at r4.
Reviewable status: 14 of 27 files reviewed, all discussions resolved (waiting on @imnasnainaec and @jasonleenaylor)

docs/user_guide/docs/licenses/backend_licenses.txt, line 92 at r3 (raw file):

Previously, johnthagen wrote…

We're including --include-transitive in the dotnet-project-licenses command, so that might have something to do with it? Due to how the dependencies are described in C# packages, it might be difficult for a license checker looking from the side to know how the final linking happens. Perhaps that is why there is duplication?

Perhaps, and the change to 5.0 didn't really change this. 🤷‍♂️

johnthagen · 2021-06-18T19:14:26Z

Backend/Controllers/UserController.cs, line 41 at r5 (raw file):

Previously, jasonleenaylor (Jason Naylor) wrote…

[ProducesResponseType(StatusCodes.Status200OK)]
``
Do we need to add 404 here now?

Per discussions with @imnasnainaec documenting non-200 responses is of pretty low value because it doesn't change the generated type script code. It ends up being a bunch of bookkeeping without any real value.

@imnasnainaec Please correct me if I'm mischaracterizing our conclusion about this.

johnthagen · 2021-06-18T19:24:45Z

Some LGTM logs:

[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.Core.Desktop 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.Core.Desktop 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.DictionaryServices 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.DictionaryServices 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.Lift 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.Lift 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.Core.Desktop 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.Core.Desktop 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.DictionaryServices 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.DictionaryServices 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]   /usr/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Package SIL.Lift 8.0.0 is not compatible with net5.0 (.NETCoreApp,Version=v5.0). Package SIL.Lift 8.0.0 supports: net461 (.NETFramework,Version=v4.6.1) [/opt/src/BackendFramework.sln]
[2021-06-18 18:55:19] [build-stdout]     0 Warning(s)
[2021-06-18 18:55:19] [build-stdout]     6 Error(s)

johnthagen · 2021-06-25T14:21:10Z

@jasonleenaylor If we decide to accept this PR, the following administrative steps will need to be performed:

Disable C# LGTM analysis: https://help.semmle.com/lgtm-enterprise/admin/help/disabling-analysis-language.html
Disable LGTM C# as a required PR check. Settings | Branches | Branch Protection Rules | Status checks that are required | Remove LGTM analysis: C#
- (I can do this if granted temporary admin of TheCombine)
Settings -> Security and Analysis -> Code Scanning to make sure there is a failing commit status if any alert is found.

johnthagen · 2021-06-25T16:37:58Z

Edit: For now we decided to stick to the defaults.

We may want to include some of the additional queries that CodeQL supports, but this does risk having more false positives:

security-extended	Queries of lower severity and precision than the default queries
security-and-quality	Queries from security-extended, plus maintainability and reliability queries

jasonleenaylor

Reviewed 1 of 13 files at r3, 10 of 16 files at r4, 2 of 2 files at r7, 1 of 1 files at r8.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

jasonleenaylor

Reviewed 1 of 1 files at r9.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @imnasnainaec)

For debugging after #1198.

Disable configuring unused Razor pages

5238993

johnthagen added backend maintenance Issue that makes it difficult to maintain the software or to upgrade installations post-release. labels Jun 18, 2021

johnthagen self-assigned this Jun 18, 2021

Merge branch 'master' into update-backend-json-serializer

afd4aa3

johnthagen changed the title ~~Disable configuring unused Razor pages~~ Migrate away from deprecated Newtonsoft JSON serializer Jun 18, 2021

johnthagen and others added 3 commits June 18, 2021 10:00

Remove SetCompatibilityVersion no-op

a822cb0

Merge branch 'master' into update-backend-json-serializer

1816371

Merge branch 'master' into update-backend-json-serializer

3ca6cb8

This comment has been minimized.

Sign in to view

Port System.Text.Json for serialization of API endpoints

230bf77

johnthagen marked this pull request as ready for review June 18, 2021 17:38

fmt

3df2671

johnthagen requested a review from imnasnainaec June 18, 2021 17:46

johnthagen marked this pull request as draft June 18, 2021 17:57

jasonleenaylor reviewed Jun 18, 2021

View reviewed changes

Upgrade backend to .NET 5.0

a7e0304

johnthagen changed the title ~~Migrate away from deprecated Newtonsoft JSON serializer~~ Upgrade backend to .NET 5 Jun 18, 2021

Update GitHub actions

e38a90a

johnthagen mentioned this pull request Jun 18, 2021

Update backend dependencies #1203

Closed

johnthagen marked this pull request as ready for review June 18, 2021 18:51

fmt

42fe6bb

jasonleenaylor reviewed Jun 18, 2021

View reviewed changes

Try to force a custom C# build on LGTM

ac1b222

johnthagen added 2 commits June 25, 2021 07:32

Merge branch 'master' into update-backend-json-serializer

bfb8db6

Run C# CodeQL analysis through GitHub Actions

539ce8b

johnthagen marked this pull request as draft June 25, 2021 14:06

Roll back C#-specific LGTM settings

549bd11

johnthagen marked this pull request as ready for review June 25, 2021 14:23

johnthagen mentioned this pull request Jun 25, 2021

Enable configuration to fail CI if new alerts are introduced github/codeql-action#598

Closed

jasonleenaylor approved these changes Jun 25, 2021

View reviewed changes

Merge branch 'master' into update-backend-json-serializer

39193f0

jasonleenaylor approved these changes Jun 25, 2021

View reviewed changes

johnthagen merged commit 5475dcd into master Jun 25, 2021

johnthagen deleted the update-backend-json-serializer branch June 25, 2021 18:16

This was referenced Jun 25, 2021

Update backend dependencies #1215

Merged

Remove dependency on deprecated WebApiCompatShim #1220

Merged

Document how to set the backend to launch with auto reloading #1172

Closed

Refactor backend to use idiomatic C# 9 conventions #1232

Merged

imnasnainaec mentioned this pull request Jul 8, 2021

Update launch.json .NET folder 3.1 -> 5.0. #1245

Merged

imnasnainaec added a commit that referenced this pull request Jul 8, 2021

Update launch.json .NET folder 3.1 -> 5.0. (#1245)

f755f79

For debugging after #1198.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade backend to .NET 5 #1198

Upgrade backend to .NET 5 #1198

johnthagen commented Jun 18, 2021 •

edited

Loading

codecov-commenter commented Jun 18, 2021 •

edited

Loading

This comment has been minimized.

johnthagen commented Jun 18, 2021

jasonleenaylor left a comment

johnthagen commented Jun 18, 2021

jasonleenaylor commented Jun 18, 2021

jasonleenaylor left a comment

johnthagen commented Jun 18, 2021

johnthagen commented Jun 18, 2021

johnthagen commented Jun 25, 2021 •

edited by jasonleenaylor

Loading

johnthagen commented Jun 25, 2021 •

edited

Loading

jasonleenaylor left a comment

jasonleenaylor left a comment

Upgrade backend to .NET 5 #1198

Upgrade backend to .NET 5 #1198

Conversation

johnthagen commented Jun 18, 2021 • edited Loading

codecov-commenter commented Jun 18, 2021 • edited Loading

Codecov Report

This comment has been minimized.

johnthagen commented Jun 18, 2021

jasonleenaylor left a comment

Choose a reason for hiding this comment

johnthagen commented Jun 18, 2021

jasonleenaylor commented Jun 18, 2021

jasonleenaylor left a comment

Choose a reason for hiding this comment

johnthagen commented Jun 18, 2021

johnthagen commented Jun 18, 2021

johnthagen commented Jun 25, 2021 • edited by jasonleenaylor Loading

johnthagen commented Jun 25, 2021 • edited Loading

jasonleenaylor left a comment

Choose a reason for hiding this comment

jasonleenaylor left a comment

Choose a reason for hiding this comment

johnthagen commented Jun 18, 2021 •

edited

Loading

codecov-commenter commented Jun 18, 2021 •

edited

Loading

johnthagen commented Jun 25, 2021 •

edited by jasonleenaylor

Loading

johnthagen commented Jun 25, 2021 •

edited

Loading