fix(core): eliminate all AOT-incompatible patterns for Native AOT support#523
Merged
Conversation
- Replace Activator.CreateInstance with new T() factory delegates in AbstractBootstrap extension point resolution (11 sites) - Replace ConstructorInfo.Invoke + Activator in GeneralUpdateBootstrap configure strategy with ResolveExtension - Add UpdateReportJsonContext source-gen context for HttpUpdateReporter.ReportAsync serialization - Switch UpdateRequestBuilder config loading to HttpParameterJsonContext.Default.UpdateRequest - Make JsonTypeInfo<T> required (non-nullable) in StorageManager and VersionService to eliminate reflection fallback paths Build verified: 0 errors, 0 IL2026/IL3050 trim warnings across all target frameworks (netstandard2.0/net8.0/net10.0) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make GeneralUpdate.Core publishable with Native AOT by removing reflection-based activation/serialization paths and switching to source-generated System.Text.Json metadata.
Changes:
- Replaces extension activation patterns in bootstrap configuration with factory delegates to avoid reflection.
- Introduces source-generated JSON context for
UpdateReportand updates reporter/config parsing to use generatedJsonTypeInfo. - Makes several JSON helper APIs require
JsonTypeInfoto avoid reflection fallbacks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/c#/GeneralUpdate.Core/Configuration/AbstractBootstrap.cs |
Adds extension factory delegates and updates resolution logic to avoid reflection. |
src/c#/GeneralUpdate.Core/Configuration/UpdateRequestBuilder.cs |
Switches config deserialization to source-generated JSON metadata. |
src/c#/GeneralUpdate.Core/Download/Reporting/IUpdateReporter.cs |
Switches reporting serialization to source-generated JSON metadata. |
src/c#/GeneralUpdate.Core/FileSystem/StorageManager.cs |
Requires JsonTypeInfo for JSON IO helpers and removes reflection fallbacks. |
src/c#/GeneralUpdate.Core/Network/VersionService.cs |
Requires JsonTypeInfo for response deserialization and removes reflection fallback. |
src/c#/GeneralUpdate.Core/JsonContext/UpdateReportJsonContext.cs |
Adds a source-generated JSON context for UpdateReport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
363
to
368
| protected TExtension? ResolveExtension<TExtension>() where TExtension : class | ||
| { | ||
| if (_extensions.TryGetValue(typeof(TExtension), out var t)) | ||
| return Activator.CreateInstance((Type)t) as TExtension; | ||
| if (_extensionFactories.TryGetValue(typeof(TExtension), out var factory)) | ||
| return factory() as TExtension; | ||
| if (_instances.TryGetValue(typeof(TExtension), out var instance)) | ||
| return instance as TExtension; |
Comment on lines
148
to
163
| /// <param name="typeInfo">JSON type info metadata for source generator serialization support. Required for Native AOT compatibility.</param> | ||
| /// <exception cref="ArgumentException">Thrown when <paramref name="targetPath"/> does not contain a valid directory path.</exception> | ||
| /// <exception cref="ArgumentNullException">Thrown when <paramref name="typeInfo"/> is null.</exception> | ||
| /// <remarks> | ||
| /// If the directory of the target file does not exist, it will be created automatically. | ||
| /// Supports source generator mode via <c>JsonTypeInfo</c>, which avoids runtime reflection in AOT compilation scenarios. | ||
| /// </remarks> | ||
| public static void CreateJson<T>(string targetPath, T obj, JsonTypeInfo<T>? typeInfo = null) where T : class | ||
| public static void CreateJson<T>(string targetPath, T obj, JsonTypeInfo<T> typeInfo) where T : class | ||
| { | ||
| var folderPath = Path.GetDirectoryName(targetPath) ?? | ||
| throw new ArgumentException("invalid path", nameof(targetPath)); | ||
|
|
||
| if (!Directory.Exists(folderPath)) | ||
| Directory.CreateDirectory(folderPath); | ||
|
|
||
| var jsonString = typeInfo != null ? JsonSerializer.Serialize(obj, typeInfo) : JsonSerializer.Serialize(obj); | ||
| var jsonString = JsonSerializer.Serialize(obj, typeInfo); | ||
| File.WriteAllText(targetPath, jsonString); |
Comment on lines
171
to
184
| /// <param name="path">The full path of the JSON file.</param> | ||
| /// <param name="typeInfo">Optional JSON type info metadata for source generator deserialization support.</param> | ||
| /// <param name="typeInfo">JSON type info metadata for source generator deserialization support. Required for Native AOT compatibility.</param> | ||
| /// <returns>The deserialized object instance; returns <c>default</c> if the file does not exist.</returns> | ||
| /// <exception cref="ArgumentNullException">Thrown when <paramref name="typeInfo"/> is null.</exception> | ||
| /// <remarks> | ||
| /// If the file does not exist, no exception is thrown and <c>null</c> is returned. | ||
| /// Supports source generator mode via <c>JsonTypeInfo</c>. | ||
| /// </remarks> | ||
| public static T? GetJson<T>(string path, JsonTypeInfo<T>? typeInfo = null) where T : class | ||
| public static T? GetJson<T>(string path, JsonTypeInfo<T> typeInfo) where T : class | ||
| { | ||
| if (File.Exists(path)) | ||
| { | ||
| var json = File.ReadAllText(path); | ||
| if (typeInfo != null) | ||
| { | ||
| return JsonSerializer.Deserialize(json, typeInfo); | ||
| } | ||
| return JsonSerializer.Deserialize<T>(json); | ||
| return JsonSerializer.Deserialize(json, typeInfo); | ||
| } |
Comment on lines
138
to
142
| } | ||
|
|
||
| var json = File.ReadAllText(configPath); | ||
| var config = JsonSerializer.Deserialize<UpdateRequest>(json, new JsonSerializerOptions | ||
| { | ||
| PropertyNameCaseInsensitive = true | ||
| }); | ||
| var config = JsonSerializer.Deserialize(json, JsonContext.HttpParameterJsonContext.Default.UpdateRequest); | ||
|
|
Comment on lines
356
to
360
| var r = await _sharedClient.SendAsync(req, cts.Token).ConfigureAwait(false); | ||
| r.EnsureSuccessStatusCode(); | ||
| var rj = await r.Content.ReadAsStringAsync().ConfigureAwait(false); | ||
| return ti == null ? JsonSerializer.Deserialize<T>(rj) : JsonSerializer.Deserialize(rj, ti); | ||
| return JsonSerializer.Deserialize(rj, ti); | ||
| } |
- Swap ResolveExtension lookup order: _instances wins over _extensionFactories so singleton instances (e.g. HttpAuth(provider)) always take precedence - Keep JsonTypeInfo<T> optional in StorageManager with null-guard at runtime for source compatibility while maintaining AOT safety - Add UpdateRequestConfigJsonContext with case-insensitive matching to preserve existing update_config.json behavior under source-gen - Guard against null JsonSerializer.Deserialize result in VersionService Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Eliminate all AOT/trimming-incompatible patterns in
GeneralUpdate.Coreso the library can be published with Native AOT (dotnet publish -aot).Changes
🔴 Reflection activation eliminated
AbstractBootstrap— ReplacedActivator.CreateInstancewithnew T()factory delegates (_extensionFactories) across all 11 extension registration methods. Added parallel_extensionFactoriesdictionary while preserving_extensionsfor type metadata queries.GeneralUpdateBootstrap.ConfigureStrategy— EliminatedConstructorInfo.Invoke+Activator.CreateInstancefor IDownloadPipeline; replaced withResolveExtension<IDownloadPipeline>().🔴 JSON serialization reflection eliminated
JsonContext/UpdateReportJsonContext.cs— Source-generated JSON context forUpdateReportwith camelCase policy.HttpUpdateReporter.ReportAsync— UsesUpdateReportJsonContext.Default.UpdateReportinstead ofJsonSerializerOptions.UpdateRequestBuilder.LoadFromConfigFile— UsesHttpParameterJsonContext.Default.UpdateRequestinstead of reflection-basedJsonSerializerOptions.🟡 Reflection fallback paths hardened
StorageManager.CreateJson/GetJson—JsonTypeInfo<T>changed from nullable (with reflection fallback) to required parameter.VersionService.SendAsync/PostAsync—JsonTypeInfo<T>changed from nullable to required parameter.Verification
dotnet build— 0 errors across all targets (netstandard2.0/net8.0/net10.0)JsonTypeInfo— no breaking changes to public APIFiles changed
Configuration/AbstractBootstrap.csConfiguration/UpdateRequestBuilder.csDownload/Reporting/IUpdateReporter.csFileSystem/StorageManager.csNetwork/VersionService.csJsonContext/UpdateReportJsonContext.cs🤖 Generated with Claude Code