Native aot with ~options builder methods~ just the right attributes on the test contexts#648
Conversation
|
I did notice that the serialization, after going into the The options still has the full resolver chain at this point. Whatever is happening, it's in the external code between this line and going back into the |
…r-methods-proposedfixes Proposed fixes to get tests passing again
| /// <param name="typeInfo">An explicit typeInfo to use for looking up the Converter. If not provided, options.GetTypeInfo will be used.</param> | ||
| /// <returns>The value that was converted.</returns> | ||
| public static void Write<T>(this JsonSerializerOptions options, Utf8JsonWriter writer, T? value, JsonTypeInfo<T>? typeInfo) | ||
| public static void Write<T>(this JsonSerializerOptions options, Utf8JsonWriter writer, T value, JsonTypeInfo<T> typeInfo) |
There was a problem hiding this comment.
Not sure if I mentioned this in another PR, but the options and typeInfo parameters defined here are redundant and mixing them together can result in consistency problems. Either resolve the typeInfo from the options.GetTypeInfo method or get the Options instance from the typeInfo parameter.
There was a problem hiding this comment.
The TypeInfo here is used just to get the Converter, and we're using a JsonTypeInfo as parameter to make sure the caller has tagged the type on a JsonSerializerContext. The options parameter is used for all caller-defined types and any other configurations.
There was a problem hiding this comment.
(I'm still fuzzy on why the type info is tied to an options in the first place. They seem to be independent concepts.)
jevansaks
left a comment
There was a problem hiding this comment.
I left a couple comments that can be addressed now or later. Please update the PR description based on the work you've done too, it looks a little stale.
Update
(see also "original intent" below)
This PR updates all of the serializer contexts back to internal while ensuring that all of the types are supported.
Additionally, it creates several helper methods to work around a known STJ bug. These methods are used throughout the libraries to ensure that the options passed in by the user are respected while still being able to use the serializer context JTIs, which are needed for AOT.
The end result is that contexts are hidden, and users don't need to have annotations for all of the types contained within these libraries; only those which they serialize directly. For example, for JsonSchema, this will likely be only
JsonSchemaandEvaluationResults.Original intent of the PR
This implements "sharing" the contexts via
JsonSerializerOptionsextensions that simply tack on the context to the options'.TypeInfoResolverChain.There is a single bug that I'm experiencing, and I don't know why. Specifically
JsonPointerwon't serialize inside theJson.Schema.EvaluationResultsConverter.Write()method. When attempting to serialize the.Detailscollection (which isIReadOnlyList<EvaluationResults>), I pass in the options, and I have confirmed that at this point all of the resolvers are present in the options. Later, through nested serialization, it comes back into the.Write()method with theJsonSchemaSerializerContext.Default.Optionsonly, which means that it's lost all of the type resolvers somehow, and it fails onJsonPointer.Now, I know that I can just add
JsonPointerto the internal serializer context, but I'd really like to figure out why the options object is changing as this is the root cause.