Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enable format validation and fix bugs. #3

Closed
wants to merge 2 commits into from
Closed

Enable format validation and fix bugs. #3

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c65a29
Enable format validation and fix bugs.
eiriktsarpalis Jun 5, 2024
0b36530
Revert changes to DateTimeOffset
eiriktsarpalis Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/JsonSchemaMapper/JsonSchemaMapper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -840,6 +840,8 @@ public SimpleTypeJsonSchema(JsonSchemaType schemaType, string? format = null, st
public bool IsIeeeFloatingPoint { get; }
}

private const string Iso8601DateTimeRegex = @"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})?$";

private static readonly Dictionary<Type, SimpleTypeJsonSchema> s_simpleTypeInfo = new()
{
[typeof(object)] = new(JsonSchemaType.Any),
Expand Down Expand Up @@ -867,8 +869,8 @@ public SimpleTypeJsonSchema(JsonSchemaType schemaType, string? format = null, st
[typeof(byte[])] = new(JsonSchemaType.String),
[typeof(Memory<byte>)] = new(JsonSchemaType.String),
[typeof(ReadOnlyMemory<byte>)] = new(JsonSchemaType.String),
[typeof(DateTime)] = new(JsonSchemaType.String, format: "date-time"),
[typeof(DateTimeOffset)] = new(JsonSchemaType.String, format: "date-time"),
[typeof(DateTime)] = new(JsonSchemaType.String, pattern: Iso8601DateTimeRegex),
[typeof(DateTimeOffset)] = new(JsonSchemaType.String, pattern: Iso8601DateTimeRegex),

// TimeSpan is represented as a string in the format "[-][d.]hh:mm:ss[.fffffff]".
[typeof(TimeSpan)] = new(JsonSchemaType.String, pattern: @"^-?(\d+\.)?\d{2}:\d{2}:\d{2}(\.\d{1,7})?$"),
Expand All @@ -878,7 +880,7 @@ public SimpleTypeJsonSchema(JsonSchemaType schemaType, string? format = null, st
#endif
[typeof(Guid)] = new(JsonSchemaType.String, format: "uuid"),
[typeof(Uri)] = new(JsonSchemaType.String, format: "uri"),
[typeof(Version)] = new(JsonSchemaType.String, format: @"^\d+(\.\d+){1,3}$"),
[typeof(Version)] = new(JsonSchemaType.String, pattern: @"^\d+(\.\d+){1,3}$"),
[typeof(JsonDocument)] = new(JsonSchemaType.Any),
[typeof(JsonElement)] = new(JsonSchemaType.Any),
[typeof(JsonNode)] = new(JsonSchemaType.Any),
Expand Down
2 changes: 1 addition & 1 deletion tests/JsonSchemaMapper.Tests/Helpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ Instance JSON document matches the specified schema.
private static EvaluationResults EvaluateSchemaCore(JsonNode schema, JsonNode? instance)
{
JsonSchema jsonSchema = JsonSerializer.Deserialize(schema, Context.Default.JsonSchema)!;
EvaluationOptions options = new() { OutputFormat = OutputFormat.List };
EvaluationOptions options = new() { OutputFormat = OutputFormat.List, RequireFormatValidation = true };
return jsonSchema.Evaluate(instance, options);
}

Expand Down
7 changes: 4 additions & 3 deletions tests/JsonSchemaMapper.Tests/TestTypes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ public static IEnumerable<ITestData> GetTestDataCore()
yield return new TestData<ReadOnlyMemory<byte>>(new byte[] { 1, 2, 3 }, ExpectedJsonSchema: """{"type":"string"}""");
yield return new TestData<DateTime>(
Value: new(2021, 1, 1),
AdditionalValues: [DateTime.MinValue, DateTime.MaxValue]);
AdditionalValues: [DateTime.MinValue, DateTime.MaxValue],
ExpectedJsonSchema: """{"type":"string","pattern":"^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(?:\\.\\d\u002B)?(?:Z|[\u002B-]\\d{2}:\\d{2})?$"}""");

yield return new TestData<DateTimeOffset>(
Value: new(new DateTime(2021, 1, 1), TimeSpan.Zero),
AdditionalValues: [DateTimeOffset.MinValue, DateTimeOffset.MaxValue],
ExpectedJsonSchema: """{"type":"string","format": "date-time"}""");
ExpectedJsonSchema: """{"type":"string","pattern":"^\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(?:\\.\\d\u002B)?(?:Z|[\u002B-]\\d{2}:\\d{2})?$"}""");
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to contemplate here is that this change is trading off human readability for precision. Could this be something that could create problems with downstream consumers?

cc @captainsafia @eerhardt

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Note: I'm not even a JSON schema novice)

Is it possible to define custom "formats" at all? Could we make a "dotnet-date-time" format where the pattern could be 100% accurate, but in all the places it is used, users just see "format": "dotnet-date-time", so it is more readable?

I agree readability is important. But it feels like precision is necessary because getting false positive schema errors for valid data is pretty annoying.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to define custom "formats" at all?

OpenAPI v3 is permissive about allowing you to define custom formats (e.g. we defined one for unsigned integers) although I dunno that dotnet-datet-time would be one that we'd pursue.

Is {type: "string", format: "date-time", pattern: "the-regex-here" } a permissible schema here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is {type: "string", format: "date-time", pattern: "the-regex-here" } a permissible schema here?

It would result in false negatives in implementations that do validate for the format keyword.

Copy link

@gregsdennis gregsdennis Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to define custom "formats" at all? Could we make a "dotnet-date-time" format...

Yes, custom formats are permissible, however you have the same problem that format isn't validated by default.

With custom formats, you now have the added problem that your format probably wouldn't be supported/understood by the validator, so even if it wanted to validate, it wouldn't know how (without custom code).

https://docs.json-everything.net/schema/basics/#schema-format

Here's what my validator supports.

Copy link
Owner Author

@eiriktsarpalis eiriktsarpalis Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I reverted some of the changes because it turns out this was only needed for DateTime, but not DateTimeOffset. This is because the built-in converter for DateTime omits TZ offsets in the representation if the instance is of DateTimeKind.Unspecified.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test failures are being triggered because I'm feeding my tests with DateTime.MaxValue which has an undefined TZ. I wonder if supporting DateTimeKind.Unspecified is the right trade-off over using format : date-time.


yield return new TestData<TimeSpan>(
Value: new(hours: 5, minutes: 13, seconds: 3),
Expand All @@ -74,7 +75,7 @@ public static IEnumerable<ITestData> GetTestDataCore()
#endif
yield return new TestData<Guid>(Guid.Empty);
yield return new TestData<Uri>(new("http://example.com"));
yield return new TestData<Version>(new(1, 2, 3, 4), ExpectedJsonSchema: """{"type":"string","format":"^\\d+(\\.\\d+){1,3}$"}""");
yield return new TestData<Version>(new(1, 2, 3, 4), ExpectedJsonSchema: """{"type":"string","pattern":"^\\d+(\\.\\d+){1,3}$"}""");
yield return new TestData<JsonDocument>(JsonDocument.Parse("""[{ "x" : 42 }]"""), ExpectedJsonSchema: "{}");
yield return new TestData<JsonElement>(JsonDocument.Parse("""[{ "x" : 42 }]""").RootElement, ExpectedJsonSchema: "{}");
yield return new TestData<JsonNode>(JsonNode.Parse("""[{ "x" : 42 }]"""));
Expand Down
Loading