-
Notifications
You must be signed in to change notification settings - Fork 246
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggested path for JsonSchema.Net integration #1191
Changes from all commits
f25b0e0
f9eb5d0
8a6192a
09568a2
d66ea86
2be6fe7
a1db8b6
832efcd
6177047
c14ca66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
|
||
using System.IO; | ||
using System.Linq; | ||
using System.Text.Json; | ||
using System.Threading.Tasks; | ||
using Microsoft.OpenApi.Interfaces; | ||
using Microsoft.OpenApi.Models; | ||
|
@@ -50,7 +51,12 @@ public OpenApiDocument Read(TextReader input, out OpenApiDiagnostic diagnostic) | |
return new OpenApiDocument(); | ||
} | ||
|
||
return new OpenApiYamlDocumentReader(this._settings).Read(yamlDocument, out diagnostic); | ||
var asJsonNode = yamlDocument.ToJsonNode(); | ||
|
||
diagnostic = null; // TBD | ||
return asJsonNode.Deserialize<OpenApiDocument>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I honestly don't see how using Deserialize is going to work for us. Using an automatic deserialization mechanism strongly couples the serialized format to the in-memory model. We specifically wrote the deserializer classes so we can deserialize any version of OpenAPI into the "latest" in-memory model. It is not clear to me from this PR how you are proposing we continue allowing our users to import OpenAPI without caring what version of OAS the input format is. And to clarify, this is one of our most important features. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strictly speaking, this change isn't necessary because this code will be deleted.
How so? I'm doing the actual parsing with YamlSharp; both YAML and JSON are supported through this. It parses into the Going the other way, if you want JSON, you just use
This supports the same thing. I can drop a 2.0 or a 3.1 schema into my There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does the JsonDeserializer.Deserialize convert body parameters into a requestBody object? How does the JsonSerializer.Deserialize know how to convert produces/consumes into media type objects? We have many of these kinds of translations that System.Text.Json knows nothing about. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't follow. The paths use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In our V2 Deserializer, we convert parameters that are of type |
||
|
||
//return new OpenApiYamlDocumentReader(this._settings).Read(yamlDocument, out diagnostic); | ||
} | ||
|
||
/// <summary> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we are going to make the switch to do Yaml => JsonNodes => OpenAPIDocument. This will facilitate us to use the JsonSchema.Net Examples and hopefully get rid of OpenApiAny and all of the OpenApiPrimitive classes.