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

Additional tests for System.Text.Json #32705

Merged
merged 9 commits into from
Feb 29, 2020
Merged

Additional tests for System.Text.Json #32705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4b5f5c2
added multi-segment DateTime tests
alanisaac Feb 23, 2020
a4c5847
added missing null argument test
alanisaac Feb 23, 2020
a8374d9
added null argument tests and missing check for SerializeAsync
alanisaac Feb 23, 2020
8090c69
removed unreachable defensive code block
alanisaac Feb 23, 2020
1e8e33c
sealed class type for dictionary converters and removed unreachable code
alanisaac Feb 23, 2020
7a6e0cb
added additional test permutation
alanisaac Feb 26, 2020
823f147
added slices to DateTime conversion tests
alanisaac Feb 26, 2020
256de92
Revert "sealed class type for dictionary converters and removed unrea…
alanisaac Feb 29, 2020
02ab96b
sealed class type for internal json converters
alanisaac Feb 29, 2020
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
2 changes: 1 addition & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonCollectionConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Text.Json.Serialization
/// </summary>
internal abstract class JsonCollectionConverter<TCollection, TElement> : JsonResumableConverter<TCollection>
{
internal override ClassType ClassType => ClassType.Enumerable;
internal sealed override ClassType ClassType => ClassType.Enumerable;
internal override Type ElementType => typeof(TElement);
}
}
2 changes: 1 addition & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonDictionaryConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace System.Text.Json.Serialization
/// </summary>
internal abstract class JsonDictionaryConverter<T> : JsonResumableConverter<T>
{
internal override ClassType ClassType => ClassType.Dictionary;
internal sealed override ClassType ClassType => ClassType.Dictionary;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should seal the overrides in JsonCollectionConverter and JsonObjectConverter as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 02ab96b

protected internal abstract bool OnWriteResume(Utf8JsonWriter writer, T dictionary, JsonSerializerOptions options, ref WriteStack state);
}
}
2 changes: 1 addition & 1 deletion src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonObjectConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace System.Text.Json.Serialization
/// </summary>
internal abstract class JsonObjectConverter<T> : JsonResumableConverter<T>
{
internal override ClassType ClassType => ClassType.Object;
internal sealed override ClassType ClassType => ClassType.Object;
internal sealed override Type? ElementType => null;
}
}
8 changes: 3 additions & 5 deletions ...raries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Write.Stream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ public static partial class JsonSerializer
/// <param name="cancellationToken">The <see cref="System.Threading.CancellationToken"/> which may be used to cancel the write operation.</param>
public static Task SerializeAsync<TValue>(Stream utf8Json, TValue value, JsonSerializerOptions? options = null, CancellationToken cancellationToken = default)
{
if (utf8Json == null)
Copy link
Member

@ahsonkhan ahsonkhan Feb 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Nice catch. We should update the docs with the new exceptions:
https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer.serializeasync?view=netcore-3.1#System_Text_Json_JsonSerializer_SerializeAsync_System_IO_Stream_System_Object_System_Type_System_Text_Json_JsonSerializerOptions_System_Threading_CancellationToken_

If you get the chance, please add the exceptions this (and other SerializeAsync overloads) throw, here (if you are up for it, this would need a PR in the docs repo):
https://github.com/dotnet/dotnet-api-docs/blob/1e6b2e6052185c6744054acc10cd06e435a4fb8b/xml/System.Text.Json/JsonSerializer.xml#L546-L563

See other overloads that have exception docs as an example:
https://docs.microsoft.com/en-us/dotnet/api/system.text.json.jsonserializer.serialize?view=netcore-3.1#System_Text_Json_JsonSerializer_Serialize__1_System_Text_Json_Utf8JsonWriter___0_System_Text_Json_JsonSerializerOptions_

cc @carlossanlop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do, thanks for the pointers to the docs code and example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created issue dotnet/dotnet-api-docs#3961

throw new ArgumentNullException(nameof(utf8Json));

return WriteAsyncCore(utf8Json, value, typeof(TValue), options, cancellationToken);
}

Expand Down Expand Up @@ -65,11 +68,6 @@ private static async Task WriteAsyncCore(Stream utf8Json, object? value, Type in
return;
}

if (inputType == null)
{
inputType = value.GetType();
}

WriteStack state = default;
state.InitializeRoot(inputType, options, supportContinuation: true);

Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.Text.Json/tests/Serialization/Stream.ReadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ namespace System.Text.Json.Serialization.Tests
public static partial class StreamTests
{
[Fact]
public static async Task NullArgumentFail()
public static async Task ReadNullArgumentFail()
{
await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.DeserializeAsync<string>((Stream)null));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.DeserializeAsync((Stream)null, (Type)null));
Copy link
Member

Choose a reason for hiding this comment

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

While we are doing this, let's add all permutations. For example: have the stream parameter be null, but the type parameter be non-null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 7a6e0cb

await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.DeserializeAsync((Stream)null, typeof(string)));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.DeserializeAsync(new MemoryStream(), (Type)null));
}

Expand Down
7 changes: 7 additions & 0 deletions src/libraries/System.Text.Json/tests/Serialization/Stream.WriteTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ namespace System.Text.Json.Serialization.Tests
{
public static partial class StreamTests
{
[Fact]
public static async Task WriteNullArgumentFail()
{
await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.SerializeAsync((Stream)null, 1));
await Assert.ThrowsAsync<ArgumentNullException>(async () => await JsonSerializer.SerializeAsync((Stream)null, 1, typeof(int)));
}

[Fact]
public static async Task VerifyValueFail()
{
Expand Down
76 changes: 76 additions & 0 deletions src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,82 @@ public static void TestSingleStringsMultiSegment()
}
}

[Fact]
public static void TestMultiSegmentStringConversionToDateTime()
{
string jsonString = "\"1997-07-16\"";
string expectedString = "1997-07-16";
int expectedTokenLength = 10;
DateTime expectedDateTime = DateTime.Parse(expectedString);

byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

ReadOnlySequence<byte> sequence = JsonTestHelper.CreateSegments(utf8);

for (int j = 0; j < utf8.Length; j++)
{
var utf8JsonReader = new Utf8JsonReader(sequence.Slice(0, j), isFinalBlock: false, default);
ReadDateTimeHelper(ref utf8JsonReader, expectedDateTime, expectedTokenLength);

Assert.Equal(0, utf8JsonReader.TokenStartIndex);

long consumed = utf8JsonReader.BytesConsumed;
utf8JsonReader = new Utf8JsonReader(sequence.Slice(consumed), isFinalBlock: true, utf8JsonReader.CurrentState);
ReadDateTimeHelper(ref utf8JsonReader, expectedDateTime, expectedTokenLength);
}
}

private static void ReadDateTimeHelper(ref Utf8JsonReader jsonReader, DateTime expectedValue, long expectedTokenLength)
{
while (jsonReader.Read())
{
if (jsonReader.TokenType == JsonTokenType.String)
{
long tokenLength = jsonReader.HasValueSequence ? jsonReader.ValueSequence.Length : jsonReader.ValueSpan.Length;
Assert.Equal(expectedTokenLength, tokenLength);
Assert.Equal(expectedValue, jsonReader.GetDateTime());
Copy link
Member

Choose a reason for hiding this comment

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

nit: We missed the asserts for TryGetDateTime and TryGetDateTimeOffset APIs.

}
}
}

[Fact]
public static void TestMultiSegmentStringConversionToDateTimeOffset()
{
string jsonString = "\"1997-07-16\"";
string expectedString = "1997-07-16";
int expectedTokenLength = 10;
DateTimeOffset expectedDateTimeOffset = DateTimeOffset.Parse(expectedString);

byte[] utf8 = Encoding.UTF8.GetBytes(jsonString);

ReadOnlySequence<byte> sequence = JsonTestHelper.CreateSegments(utf8);

for (int j = 0; j < utf8.Length; j++)
{
var utf8JsonReader = new Utf8JsonReader(sequence.Slice(0, j), isFinalBlock: false, default);
ReadDateTimeOffsetHelper(ref utf8JsonReader, expectedDateTimeOffset, expectedTokenLength);

Assert.Equal(0, utf8JsonReader.TokenStartIndex);

long consumed = utf8JsonReader.BytesConsumed;
utf8JsonReader = new Utf8JsonReader(sequence.Slice(consumed), isFinalBlock: true, utf8JsonReader.CurrentState);
ReadDateTimeOffsetHelper(ref utf8JsonReader, expectedDateTimeOffset, expectedTokenLength);
}
}

private static void ReadDateTimeOffsetHelper(ref Utf8JsonReader jsonReader, DateTimeOffset expectedValue, long expectedTokenLength)
{
while (jsonReader.Read())
{
if (jsonReader.TokenType == JsonTokenType.String)
{
long tokenLength = jsonReader.HasValueSequence ? jsonReader.ValueSequence.Length : jsonReader.ValueSpan.Length;
Assert.Equal(expectedTokenLength, tokenLength);
Assert.Equal(expectedValue, jsonReader.GetDateTimeOffset());
}
}
}

private static void SpanSequenceStatesAreEqualInvalidJson(byte[] dataUtf8, ReadOnlySequence<byte> sequence, int maxDepth, JsonCommentHandling commentHandling)
{
var stateSpan = new JsonReaderState(new JsonReaderOptions { CommentHandling = commentHandling, MaxDepth = maxDepth });
Expand Down