feat: add JsonConverter for OpenApiSchema System.Text.Json serialization#2915
feat: add JsonConverter for OpenApiSchema System.Text.Json serialization#2915Mahdigln wants to merge 1 commit into
Conversation
baywet
left a comment
baywet
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
Let's a couple of comments to help get this merged
| doc.RootElement.GetProperty("type").GetString().Should().Be("string"); | ||
| doc.RootElement.GetProperty("description").GetString().Should().Be("A simple string"); |
There was a problem hiding this comment.
let's use the assert API. I thought we had gotten rid of FluentAssertions?
| /// <summary> | ||
| /// Initializes a new instance of <see cref="OpenApiSchemaJsonConverter"/> targeting OpenAPI 3.1. | ||
| /// </summary> | ||
| public OpenApiSchemaJsonConverter() : this(OpenApiSpecVersion.OpenApi3_1) { } |
There was a problem hiding this comment.
The default should be 3.2 like it is for serialization helpers. (maybe all those should rely on a common constant?)
| var schemaJson = document.RootElement.GetRawText(); | ||
|
|
||
| var wrapper = string.Concat( | ||
| "{\"openapi\":\"3.1.0\",\"info\":{\"title\":\"temp\",\"version\":\"0.0.0\"},", |
There was a problem hiding this comment.
this is going to break down based on the version that's passed in the constructor.
The templates should either be version specific or the constructor should guard against versions that are not supported.
| /// var json = JsonSerializer.Serialize(schema, options); | ||
| /// </code> | ||
| /// </remarks> | ||
| public sealed class OpenApiSchemaJsonConverter : JsonConverter<OpenApiSchema> |
There was a problem hiding this comment.
in theory this could be expanded to any model from this library (operations, path items, etc)
| "{\"openapi\":\"3.1.0\",\"info\":{\"title\":\"temp\",\"version\":\"0.0.0\"},", | ||
| "\"components\":{\"schemas\":{\"schema\":", schemaJson, "}}}"); | ||
|
|
||
| var result = OpenApiDocument.Parse(wrapper); |
There was a problem hiding this comment.
why not use the model factory instead of creating an empty document?
|
|
||
| var result = OpenApiDocument.Parse(wrapper); | ||
| IOpenApiSchema? schema = null; | ||
| result.Document?.Components?.Schemas?.TryGetValue("schema", out schema); |
There was a problem hiding this comment.
This might break if any reference is used, the unit tests should at least account for this.
| public override OpenApiSchema? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
| { | ||
| using var document = JsonDocument.ParseValue(ref reader); | ||
| var schemaJson = document.RootElement.GetRawText(); |
There was a problem hiding this comment.
If this ends up in a hot path, it'll lead to a lot of string allocations
2 participants
Description
Adds
OpenApiSchemaJsonConverter, aSystem.Text.Json.JsonConverter<OpenApiSchema>that produces clean OpenAPI wire format output instead of the default verbose reflection-based serialization.Type of Change
Related Issue(s)
Closes #2299
Changes Made
OpenApiSchemaJsonConverterinsrc/Microsoft.OpenApi/Converters/Read(deserialization) support in addition toWrite(serialization)PublicAPI.Unshipped.txtwith new public API surfaceTesting
Checklist
Versions applicability
Additional Notes
Usage example: