-
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 5 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> | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,130 @@ | ||||||||||||||||||||||||||||||||||
using System; | ||||||||||||||||||||||||||||||||||
using System.Collections.Generic; | ||||||||||||||||||||||||||||||||||
using System.Linq; | ||||||||||||||||||||||||||||||||||
using System.Text.Json.Nodes; | ||||||||||||||||||||||||||||||||||
using SharpYaml; | ||||||||||||||||||||||||||||||||||
using SharpYaml.Serialization; | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
namespace Microsoft.OpenApi.Readers | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||
/// Provides extensions to convert YAML models to JSON models. | ||||||||||||||||||||||||||||||||||
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. This is awesome. Thanks! Maggie is currently doing a spike to try and convert the ParseNode classes to work off JsonNode instead of YamlNode. If she is able to do that, then this class will enable us to continue supporting YAML parsing. 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. This code is basically copied from my YamlToJsonNode package. That lib uses YamlDotNet which is the basis for YamlSharp, so it's the same code for "different" types. |
||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||
public static class YamlConverter | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||
/// Converts all of the documents in a YAML stream to <see cref="JsonNode"/>s. | ||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||
/// <param name="yaml">The YAML stream.</param> | ||||||||||||||||||||||||||||||||||
/// <returns>A collection of nodes representing the YAML documents in the stream.</returns> | ||||||||||||||||||||||||||||||||||
public static IEnumerable<JsonNode> ToJsonNode(this YamlStream yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return yaml.Documents.Select(x => x.ToJsonNode()); | ||||||||||||||||||||||||||||||||||
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.
Suggested change
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 think they're using a compiler version that supports this. 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. Evidence would suggest otherwise
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. Sure, okay. I'm not worried about these nuances right now. Let's please focus on making it work.
This is an inconsequential, editorial change. |
||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||
/// Converts a single YAML document to a <see cref="JsonNode"/>. | ||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||
/// <param name="yaml">The YAML document.</param> | ||||||||||||||||||||||||||||||||||
/// <returns>A `JsonNode` representative of the YAML document.</returns> | ||||||||||||||||||||||||||||||||||
public static JsonNode ToJsonNode(this YamlDocument yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return yaml.RootNode.ToJsonNode(); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||
/// Converts a single YAML node to a <see cref="JsonNode"/>. | ||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||
/// <param name="yaml">The YAML node.</param> | ||||||||||||||||||||||||||||||||||
/// <returns>A `JsonNode` representative of the YAML node.</returns> | ||||||||||||||||||||||||||||||||||
/// <exception cref="NotSupportedException">Thrown for YAML that is not compatible with JSON.</exception> | ||||||||||||||||||||||||||||||||||
public static JsonNode ToJsonNode(this YamlNode yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return yaml switch | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
YamlMappingNode map => map.ToJsonObject(), | ||||||||||||||||||||||||||||||||||
YamlSequenceNode seq => seq.ToJsonArray(), | ||||||||||||||||||||||||||||||||||
YamlScalarNode scalar => scalar.ToJsonValue(), | ||||||||||||||||||||||||||||||||||
_ => throw new NotSupportedException("This yaml isn't convertible to JSON") | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
/// <summary> | ||||||||||||||||||||||||||||||||||
/// Converts a single JSON node to a <see cref="YamlNode"/>. | ||||||||||||||||||||||||||||||||||
/// </summary> | ||||||||||||||||||||||||||||||||||
/// <param name="json"></param> | ||||||||||||||||||||||||||||||||||
/// <returns></returns> | ||||||||||||||||||||||||||||||||||
/// <exception cref="NotSupportedException"></exception> | ||||||||||||||||||||||||||||||||||
public static YamlNode ToYamlNode(this JsonNode json) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return json switch | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
null => null, | ||||||||||||||||||||||||||||||||||
JsonObject obj => obj.ToYamlMapping(), | ||||||||||||||||||||||||||||||||||
JsonArray arr => arr.ToYamlSequence(), | ||||||||||||||||||||||||||||||||||
JsonValue val => val.ToYamlScalar(), | ||||||||||||||||||||||||||||||||||
_ => throw new NotSupportedException("This isn't a supported JsonNode") | ||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static JsonObject ToJsonObject(this YamlMappingNode yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
var node = new JsonObject(); | ||||||||||||||||||||||||||||||||||
foreach (var keyValuePair in yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
var key = ((YamlScalarNode)keyValuePair.Key).Value!; | ||||||||||||||||||||||||||||||||||
node[key] = keyValuePair.Value.ToJsonNode(); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
return node; | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static YamlMappingNode ToYamlMapping(this JsonObject obj) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return new YamlMappingNode(obj.ToDictionary(x => (YamlNode)new YamlScalarNode(x.Key), | ||||||||||||||||||||||||||||||||||
x => x.Value!.ToYamlNode())); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static JsonArray ToJsonArray(this YamlSequenceNode yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
var node = new JsonArray(); | ||||||||||||||||||||||||||||||||||
foreach (var value in yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
node.Add(value.ToJsonNode()); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
return node; | ||||||||||||||||||||||||||||||||||
Comment on lines
+91
to
+97
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.
Suggested change
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. Your suggestion allocates an extra array. 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. right, sorry I meant this
Suggested change
|
||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static YamlSequenceNode ToYamlSequence(this JsonArray arr) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return new YamlSequenceNode(arr.Select(x => x!.ToYamlNode())); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static JsonValue ToJsonValue(this YamlScalarNode yaml) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
switch (yaml.Style) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
case ScalarStyle.Plain: | ||||||||||||||||||||||||||||||||||
return decimal.TryParse(yaml.Value, out var d) | ||||||||||||||||||||||||||||||||||
? JsonValue.Create(d) | ||||||||||||||||||||||||||||||||||
: bool.TryParse(yaml.Value, out var b) | ||||||||||||||||||||||||||||||||||
? JsonValue.Create(b) | ||||||||||||||||||||||||||||||||||
: JsonValue.Create(yaml.Value)!; | ||||||||||||||||||||||||||||||||||
case ScalarStyle.SingleQuoted: | ||||||||||||||||||||||||||||||||||
case ScalarStyle.DoubleQuoted: | ||||||||||||||||||||||||||||||||||
case ScalarStyle.Literal: | ||||||||||||||||||||||||||||||||||
case ScalarStyle.Folded: | ||||||||||||||||||||||||||||||||||
case ScalarStyle.Any: | ||||||||||||||||||||||||||||||||||
return JsonValue.Create(yaml.Value)!; | ||||||||||||||||||||||||||||||||||
default: | ||||||||||||||||||||||||||||||||||
throw new ArgumentOutOfRangeException(); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
private static YamlScalarNode ToYamlScalar(this JsonValue val) | ||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||
return new YamlScalarNode(val.ToJsonString()); | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
using System; | ||
using System.Text.Json.Serialization; | ||
using System.Text.Json; | ||
using Json.Schema; | ||
using Json.More; | ||
|
||
namespace Microsoft.OpenApi.Draft4Support | ||
{ | ||
[SchemaKeyword(Name)] | ||
[SchemaSpecVersion(Draft4SupportData.Draft4Version)] | ||
[SchemaSpecVersion(SpecVersion.Draft202012)] | ||
[JsonConverter(typeof(Draft4ExclusiveMaximumKeywordJsonConverter))] | ||
internal class Draft4ExclusiveMaximumKeyword : IJsonSchemaKeyword, IEquatable<Draft4ExclusiveMaximumKeyword> | ||
{ | ||
public const string Name = "exclusiveMaximum"; | ||
|
||
private readonly ExclusiveMaximumKeyword _numberSupport; | ||
|
||
/// <summary> | ||
/// The ID. | ||
/// </summary> | ||
public bool? BoolValue { get; } | ||
|
||
public decimal? NumberValue => _numberSupport?.Value; | ||
|
||
/// <summary> | ||
/// Creates a new <see cref="IdKeyword"/>. | ||
/// </summary> | ||
/// <param name="value">Whether the `minimum` value should be considered exclusive.</param> | ||
public Draft4ExclusiveMaximumKeyword(bool value) | ||
{ | ||
BoolValue = value; | ||
} | ||
|
||
public Draft4ExclusiveMaximumKeyword(decimal value) | ||
{ | ||
_numberSupport = new ExclusiveMaximumKeyword(value); | ||
} | ||
|
||
public void Evaluate(EvaluationContext context) | ||
{ | ||
// TODO: do we need to validate that the right version of the keyword is being used? | ||
if (BoolValue.HasValue) | ||
{ | ||
context.EnterKeyword(Name); | ||
if (!BoolValue.Value) | ||
{ | ||
context.NotApplicable(() => "exclusiveMinimum is false; minimum validation is sufficient"); | ||
return; | ||
} | ||
|
||
var limit = context.LocalSchema.GetMinimum(); | ||
if (!limit.HasValue) | ||
{ | ||
context.NotApplicable(() => "minimum not present"); | ||
return; | ||
} | ||
|
||
var schemaValueType = context.LocalInstance.GetSchemaValueType(); | ||
if (schemaValueType is not (SchemaValueType.Number or SchemaValueType.Integer)) | ||
{ | ||
context.WrongValueKind(schemaValueType); | ||
return; | ||
} | ||
|
||
var number = context.LocalInstance!.AsValue().GetNumber(); | ||
|
||
if (limit == number) | ||
context.LocalResult.Fail(Name, ErrorMessages.ExclusiveMaximum, ("received", number), ("limit", BoolValue)); | ||
context.ExitKeyword(Name, context.LocalResult.IsValid); | ||
} | ||
else | ||
{ | ||
_numberSupport.Evaluate(context); | ||
} | ||
} | ||
|
||
/// <summary>Indicates whether the current object is equal to another object of the same type.</summary> | ||
/// <param name="other">An object to compare with this object.</param> | ||
/// <returns>true if the current object is equal to the <paramref name="other">other</paramref> parameter; otherwise, false.</returns> | ||
public bool Equals(Draft4ExclusiveMaximumKeyword other) | ||
{ | ||
if (ReferenceEquals(null, other)) return false; | ||
if (ReferenceEquals(this, other)) return true; | ||
return Equals(BoolValue, other.BoolValue); | ||
} | ||
|
||
/// <summary>Determines whether the specified object is equal to the current object.</summary> | ||
/// <param name="obj">The object to compare with the current object.</param> | ||
/// <returns>true if the specified object is equal to the current object; otherwise, false.</returns> | ||
public override bool Equals(object obj) | ||
{ | ||
return Equals(obj as Draft4ExclusiveMaximumKeyword); | ||
} | ||
|
||
/// <summary>Serves as the default hash function.</summary> | ||
/// <returns>A hash code for the current object.</returns> | ||
public override int GetHashCode() | ||
{ | ||
return BoolValue.GetHashCode(); | ||
} | ||
} | ||
|
||
internal class Draft4ExclusiveMaximumKeywordJsonConverter : JsonConverter<Draft4ExclusiveMaximumKeyword> | ||
{ | ||
public override Draft4ExclusiveMaximumKeyword Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) | ||
{ | ||
return reader.TokenType switch | ||
{ | ||
JsonTokenType.True or JsonTokenType.False => new Draft4ExclusiveMaximumKeyword(reader.GetBoolean()), | ||
JsonTokenType.Number => new Draft4ExclusiveMaximumKeyword(reader.GetDecimal()), | ||
_ => throw new JsonException("Expected boolean or number") | ||
}; | ||
} | ||
|
||
public override void Write(Utf8JsonWriter writer, Draft4ExclusiveMaximumKeyword value, JsonSerializerOptions options) | ||
{ | ||
if (value.BoolValue.HasValue) | ||
{ | ||
writer.WriteBoolean(Draft4ExclusiveMaximumKeyword.Name, value.BoolValue.Value); | ||
} | ||
else | ||
{ | ||
writer.WriteNumber(Draft4ExclusiveMaximumKeyword.Name, value.NumberValue.Value); | ||
} | ||
} | ||
} | ||
} |
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.