-
Notifications
You must be signed in to change notification settings - Fork 16
Replace JsonSchema.Net with NJsonSchema #295
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
base: main
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| namespace Microsoft.Teams.AI; | ||
|
|
||
| /// <summary> | ||
| /// Abstraction for JSON schema validation, decoupling from specific schema library implementations. | ||
| /// </summary> | ||
| public interface IJsonSchema | ||
| { | ||
| /// <summary> | ||
| /// Validates a JSON string against this schema. | ||
| /// </summary> | ||
| JsonSchemaValidationResult Validate(string json); | ||
|
|
||
| /// <summary> | ||
| /// Serializes this schema to a JSON string. | ||
| /// </summary> | ||
| string ToJson(); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Result of validating JSON against a schema. | ||
| /// </summary> | ||
| public class JsonSchemaValidationResult | ||
| { | ||
| /// <summary> | ||
| /// Whether the JSON is valid against the schema. | ||
| /// </summary> | ||
| public bool IsValid { get; init; } | ||
|
|
||
| /// <summary> | ||
| /// List of validation errors, if any. | ||
| /// </summary> | ||
| public IReadOnlyList<JsonSchemaValidationError> Errors { get; init; } = []; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Represents a validation error from schema validation. | ||
| /// </summary> | ||
| /// <param name="Path">The JSON path where the error occurred.</param> | ||
| /// <param name="Message">The error message.</param> | ||
| public record JsonSchemaValidationError(string Path, string Message); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,89 @@ | ||||||||||||||||||||||||||||||||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||||||||||||||||||||||||||||||||
| // Licensed under the MIT License. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| using NJsonSchema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| namespace Microsoft.Teams.AI; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// NJsonSchema-based implementation of IJsonSchema. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public class JsonSchemaWrapper : IJsonSchema | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| private readonly JsonSchema _schema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| private JsonSchemaWrapper(JsonSchema schema) => _schema = schema; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// Creates a schema from a .NET type using reflection. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public static IJsonSchema FromType(Type type) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromType(type); | ||||||||||||||||||||||||||||||||
| return new JsonSchemaWrapper(schema); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||||
| /// Creates a schema from a JSON schema string. | ||||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||||
| public static IJsonSchema FromJson(string json) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromJsonAsync(json).GetAwaiter().GetResult(); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| var schema = JsonSchema.FromJsonAsync(json).GetAwaiter().GetResult(); | |
| var schema = JsonSchema.FromJsonAsync(json).ConfigureAwait(false).GetAwaiter().GetResult(); |
Copilot
AI
Jan 28, 2026
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.
The JsonSchemaProperty creation only copies Type and Description from the wrapped schema, but it doesn't copy other potentially important properties like Format, Pattern, MinLength, MaxLength, Minimum, Maximum, Enum values, etc. This could result in incomplete validation when complex schemas are used as properties. Consider either using the entire wrapped schema directly or copying all relevant schema properties.
| Description = wrapper._schema.Description | |
| }; | |
| Description = wrapper._schema.Description, | |
| Format = wrapper._schema.Format, | |
| Pattern = wrapper._schema.Pattern, | |
| MinLength = wrapper._schema.MinLength, | |
| MaxLength = wrapper._schema.MaxLength, | |
| Minimum = wrapper._schema.Minimum, | |
| Maximum = wrapper._schema.Maximum | |
| }; | |
| foreach (var enumValue in wrapper._schema.Enumeration) | |
| { | |
| property.Enumeration.Add(enumValue); | |
| } |
Copilot
AI
Jan 28, 2026
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.
The CreateObjectSchema method silently skips properties when propSchema is not a JsonSchemaWrapper instance. This could lead to incomplete schema definitions if an implementation of IJsonSchema other than JsonSchemaWrapper is passed. Consider either throwing an exception when propSchema is not a JsonSchemaWrapper, or handling all IJsonSchema implementations properly. Alternatively, document this limitation in the method's XML documentation.
Copilot
AI
Jan 28, 2026
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.
The CreateObjectSchema method adds properties to RequiredProperties regardless of whether the property was successfully added to Properties. If a property's schema is not a JsonSchemaWrapper (and is thus skipped at lines 44-52), it will still be added to RequiredProperties at line 56. This inconsistency could lead to validation errors where the schema requires properties that don't exist in the schema definition.
| } | |
| if (required) | |
| { | |
| resultSchema.RequiredProperties.Add(name); | |
| if (required) | |
| { | |
| resultSchema.RequiredProperties.Add(name); | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,4 @@ | |
| await context.Send("hi from teams..."); | ||
| }); | ||
|
|
||
| app.Run(); | ||
| app.Run(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,4 +40,4 @@ | |
| state.Save(context); | ||
| }); | ||
|
|
||
| app.Run(); | ||
| app.Run(); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,4 +507,4 @@ function cancelSettings() { | |
| </body> | ||
| </html> | ||
| """; | ||
| } | ||
| } | ||
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.
The IJsonSchema interface and JsonSchemaWrapper class lack a JsonConverter attribute or implementation. When Function.ToString() is called (line 118), it serializes the Parameters property which is of type IJsonSchema. Without a custom JsonConverter, the default serializer may not properly serialize the internal NJsonSchema.JsonSchema object, potentially resulting in incomplete or incorrect JSON output. Consider adding a JsonConverter for IJsonSchema that calls ToJson() on the instance.