Support Native AOT

[hwoodiwiss]

This PR is a work in progress to get Native AoT working. Posting early, as I think it'll be good to get eyes on early.

AoT support as added by using a new generic serializer type and serialization factory to create it.

fixes #1347 (eventually)

Things still to do:

• [ ] Work out properly how to get JsonSerializerOptions injected, not sure we should be adding a new dependency on Microsoft.Extensions.Options
• [ ] Add better behaviour switching for .NET 8 builds, currenlty, .NET 8 builds will default to AOT compatible behaviour.
• [ ] Cleanup changes made for local testing
• [ ] Add general test coverage
• [ ] Add some sort of testing to validate the case of AOT compiled builds
• [ ] Bump Version (Major?)

[hwoodiwiss]

This might be a bit cleaner to integrate if we look to add .NET 8 as a first-class TFM first

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 49.36709% with 40 lines in your changes missing coverage. Please review.

Project coverage is 78.70%. Comparing base (54551ab) to head (b74891d). Report is 149 commits behind head on main.

Files with missing lines	Patch %	Lines
...MessageSerialization/SystemTextJsonSerializer`1.cs	48.21%	26 Missing and 3 partials :warning:
src/JustSaying/Extensions/JsonElementExtensions.cs	0.00%	7 Missing :warning:
...g/Fluent/ServiceResolver/DefaultServiceResolver.cs	33.33%	1 Missing and 1 partial :warning:
...njection.Microsoft/IServiceCollectionExtensions.cs	0.00%	1 Missing :warning:
...erialization/SystemTextJsonSerializationFactory.cs	66.66%	0 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1348      +/-   ##
==========================================
- Coverage   79.43%   78.70%   -0.74%     
==========================================
  Files         138      142       +4     
  Lines        3234     3306      +72     
  Branches      450      462      +12     
==========================================
+ Hits         2569     2602      +33     
- Misses        434      468      +34     
- Partials      231      236       +5     

Flag	Coverage Δ
linux	78.70% <49.36%> (-0.74%)	:arrow_down:
macos	47.88% <48.10%> (?)
windows	47.79% <48.10%> (?)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[martincostello]

• Add some sort of testing to validate the case of AOT compiled builds

We use this test project in Polly. In theory compiling it with the assemblies trim-rooted is enough to flush out issues that the analysers can't detect.

Bump Version (Major?)

Unless we have to break stuff, I'd class it just as a minor. I'd a minor planned for #1335 once I've validated it internally, so depending on how long both take to complete, we could have either an 8.2 then an 8.3, or ship them together as 8.2.

[hwoodiwiss]

Unless we have to break stuff, I'd class it just as a minor. I'd a minor planned for https://github.com/justeattakeaway/JustSaying/pull/1335 once I've validated it internally, so depending on how long both take to complete, we could have either an 8.2 then an 8.3, or ship them together as 8.2.

That makes sense. It makes sense to me that we should add the net8.0 tfm before this, in part just to reduce the overall size/complexity of this change, so maybe we could do: 8.2 - Batch Publish and net8.0 tfm 8.3 - AOT Support if it doesn't require breaking changes.

[slang25]

This is looking very nice 🙂 I had looked at this a while back in a local branch and the thing that I was concerned about was the JsonStringEnumConverter behaviour, which in an AOT environment will need to be added to the source generated serializer per-enum using JsonStringEnumConverter<TEnum>, which is fine but I think something that will be easily missed by consumers.

[hwoodiwiss]

This is looking very nice 🙂 I had looked at this a while back in a local branch and the thing that I was concerned about was the JsonStringEnumConverter behaviour, which in an AOT environment will need to be added to the source generated serializer per-enum using JsonStringEnumConverter<TEnum>, which is fine but I think something that will be easily missed by consumers.

Yeah, I'd agree, I think this is a known enough sharp edge of AOT compilation, that we could just guard the addition of the default JsonStringEnumConverter with RuntimeFeature.IsDynamicCodeSupported, I'm not sure if there's a better way that we could surface a warning around this to API consumers.

[slang25]

Another thought I had while browsing this change, now might be the time to create separate interfaces and implementation for the "message envelope" and message serializer/deserializer.

[hwoodiwiss]

Another thought I had while browsing this change, now might be the time to create separate interfaces and implementation for the "message envelope" and message serializer/deserializer.

Yeah, I had originally thought this change would require doing that anyway but found a way to avoid it. I think I'd prefer a v1 of AoT support to not alter the public API, at the cost of ergonomics for the AoT use-case, then we can look at building first-class support as part of the API changes that come for v8.