JSON continuation tests #42393

devsko · 2020-09-17T17:18:18Z

See #42158
comment and comment and comment

Test only

Test several scenarios in System.Text.Json where deserialization has to continue after the next chunk of data is available.

- Test continuation at every position inside the tested object - Many member with primitive and nullable types - One more level of nested object - All combinations of class/struct for tested and nested object - tested and nested object with parametrized ctor for some properties

…ted object

Tweak the payload and expect `JsonException`

devsko · 2020-09-17T19:11:58Z

@layomia This is not yet ready for review. I hope to get done what I have in mind tomorrow.

devsko · 2020-09-17T21:40:33Z

Is there anything else about continuation / chunked buffer?

ahsonkhan · 2020-09-17T22:19:53Z

Is there anything else about continuation / chunked buffer?

Since you asked, how about continuation where the token being split isn't null but other types, like whitespace (\r\n), or true/false boolean, or some large string token, or a number?

In the Utf8JsonReader specific tests, a lot of those scenarios are covered, by building ROSequence with segments splits across a whole variety of locations within the JSON, and with partial data/state to test re-entrancy. But most of those are for relatively small payloads (due to test time), and the async Deserializer API for streams could benefit from that type of extensive coverage too :)

runtime/src/libraries/System.Text.Json/tests/Utf8JsonReaderTests.MultiSegment.cs

Lines 474 to 643 in e0e1919

    
           // TestCaseType is only used to give the json strings a descriptive name. 
        
           [Theory] 
        
           // Skipping large JSON since slicing them (O(n^2)) is too slow. 
        
           [MemberData(nameof(SmallTestCases))] 
        
           public static void TestJsonReaderUtf8SegmentSizeOne(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               ReadPartialSegmentSizeOne(compactData, type, jsonString); 
        
           } 
        
           // TestCaseType is only used to give the json strings a descriptive name. 
        
           [Theory] 
        
           [MemberData(nameof(LargeTestCases))] 
        
           public static void TestJsonReaderLargeUtf8SegmentSizeOne(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               // Skipping really large JSON on Browser to prevent OOM 
        
               if (PlatformDetection.IsBrowser && (type == TestCaseType.Json40KB || type == TestCaseType.Json400KB || type == TestCaseType.ProjectLockJson)) 
        
               { 
        
                   return; 
        
               } 
        
               ReadFullySegmentSizeOne(compactData, type, jsonString); 
        
           } 
        
           // TestCaseType is only used to give the json strings a descriptive name. 
        
           [Theory] 
        
           [OuterLoop] 
        
           [MemberData(nameof(LargeTestCases))] 
        
           public static void TestJsonReaderLargestUtf8SegmentSizeOne(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               // Skipping really large JSON since slicing them (O(n^2)) is too slow. 
        
               if (type == TestCaseType.Json40KB || type == TestCaseType.Json400KB || type == TestCaseType.ProjectLockJson) 
        
               { 
        
                   return; 
        
               } 
        
               ReadPartialSegmentSizeOne(compactData, type, jsonString); 
        
           } 
        
           private static void ReadPartialSegmentSizeOne(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               // Remove all formatting/indendation 
        
               if (compactData) 
        
               { 
        
                   jsonString = JsonTestHelper.GetCompactString(jsonString); 
        
               } 
        
               byte[] dataUtf8 = Encoding.UTF8.GetBytes(jsonString); 
        
               Stream stream = new MemoryStream(dataUtf8); 
        
               TextReader reader = new StreamReader(stream, Encoding.UTF8, false, 1024, true); 
        
               string expectedStr = JsonTestHelper.NewtonsoftReturnStringHelper(reader); 
        
               ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(dataUtf8, 1); 
        
               for (int j = 0; j < dataUtf8.Length; j++) 
        
               { 
        
                   var utf8JsonReader = new Utf8JsonReader(sequence.Slice(0, j), isFinalBlock: false, default); 
        
                   byte[] resultSequence = JsonTestHelper.ReaderLoop(dataUtf8.Length, out int length, ref utf8JsonReader); 
        
                   string actualStrSequence = Encoding.UTF8.GetString(resultSequence, 0, length); 
        
                   long consumed = utf8JsonReader.BytesConsumed; 
        
                   utf8JsonReader = new Utf8JsonReader(sequence.Slice(consumed), isFinalBlock: true, utf8JsonReader.CurrentState); 
        
                   resultSequence = JsonTestHelper.ReaderLoop(dataUtf8.Length, out length, ref utf8JsonReader); 
        
                   actualStrSequence += Encoding.UTF8.GetString(resultSequence, 0, length); 
        
                   string message = $"Expected consumed: {dataUtf8.Length - consumed}, Actual consumed: {utf8JsonReader.BytesConsumed}, Index: {j}"; 
        
                   Assert.True(dataUtf8.Length - consumed == utf8JsonReader.BytesConsumed, message); 
        
                   Assert.Equal(expectedStr, actualStrSequence); 
        
               } 
        
           } 
        
           private static void ReadFullySegmentSizeOne(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               // Remove all formatting/indendation 
        
               if (compactData) 
        
               { 
        
                   jsonString = JsonTestHelper.GetCompactString(jsonString); 
        
               } 
        
               byte[] dataUtf8 = Encoding.UTF8.GetBytes(jsonString); 
        
               Stream stream = new MemoryStream(dataUtf8); 
        
               TextReader reader = new StreamReader(stream, Encoding.UTF8, false, 1024, true); 
        
               string expectedStr = JsonTestHelper.NewtonsoftReturnStringHelper(reader); 
        
               ReadOnlySequence<byte> sequence = JsonTestHelper.GetSequence(dataUtf8, 1); 
        
               var utf8JsonReader = new Utf8JsonReader(sequence, isFinalBlock: true, default); 
        
               byte[] resultSequence = JsonTestHelper.ReaderLoop(dataUtf8.Length, out int length, ref utf8JsonReader); 
        
               string actualStrSequence = Encoding.UTF8.GetString(resultSequence, 0, length); 
        
               Assert.Equal(expectedStr, actualStrSequence); 
        
           } 
        
           [Theory] 
        
           [MemberData(nameof(SmallTestCases))] 
        
           public static void TestPartialJsonReaderMultiSegment(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               _ = type; 
        
               // Remove all formatting/indendation 
        
               if (compactData) 
        
               { 
        
                   jsonString = JsonTestHelper.GetCompactString(jsonString); 
        
               } 
        
               byte[] dataUtf8 = Encoding.UTF8.GetBytes(jsonString); 
        
               ReadOnlyMemory<byte> dataMemory = dataUtf8; 
        
               List<ReadOnlySequence<byte>> sequences = JsonTestHelper.GetSequences(dataMemory); 
        
               for (int i = 0; i < sequences.Count; i++) 
        
               { 
        
                   ReadOnlySequence<byte> sequence = sequences[i]; 
        
                   var json = new Utf8JsonReader(sequence, isFinalBlock: true, default); 
        
                   while (json.Read()) 
        
                       ; 
        
                   Assert.Equal(sequence.Length, json.BytesConsumed); 
        
                   Assert.True(sequence.Slice(json.Position).IsEmpty); 
        
               } 
        
               for (int i = 0; i < sequences.Count; i++) 
        
               { 
        
                   ReadOnlySequence<byte> sequence = sequences[i]; 
        
                   var json = new Utf8JsonReader(sequence); 
        
                   while (json.Read()) 
        
                       ; 
        
                   Assert.Equal(sequence.Length, json.BytesConsumed); 
        
                   Assert.True(sequence.Slice(json.Position).IsEmpty); 
        
               } 
        
           } 
        
           [Theory] 
        
           [OuterLoop] 
        
           [MemberData(nameof(SmallTestCases))] 
        
           public static void TestPartialJsonReaderSlicesMultiSegment(bool compactData, TestCaseType type, string jsonString) 
        
           { 
        
               _ = type; 
        
               // Remove all formatting/indendation 
        
               if (compactData) 
        
               { 
        
                   jsonString = JsonTestHelper.GetCompactString(jsonString); 
        
               } 
        
               byte[] dataUtf8 = Encoding.UTF8.GetBytes(jsonString); 
        
               ReadOnlyMemory<byte> dataMemory = dataUtf8; 
        
               List<ReadOnlySequence<byte>> sequences = JsonTestHelper.GetSequences(dataMemory); 
        
               for (int i = 0; i < sequences.Count; i++) 
        
               { 
        
                   ReadOnlySequence<byte> sequence = sequences[i]; 
        
                   for (int j = 0; j < dataUtf8.Length; j++) 
        
                   { 
        
                       var json = new Utf8JsonReader(sequence.Slice(0, j), isFinalBlock: false, default); 
        
                       while (json.Read()) 
        
                           ; 
        
                       long consumed = json.BytesConsumed; 
        
                       JsonReaderState jsonState = json.CurrentState; 
        
                       byte[] consumedArray = sequence.Slice(0, consumed).ToArray(); 
        
                       Assert.Equal(consumedArray, sequence.Slice(0, json.Position).ToArray()); 
        
                       json = new Utf8JsonReader(sequence.Slice(consumed), isFinalBlock: true, jsonState); 
        
                       while (json.Read()) 
        
                           ; 
        
                       Assert.Equal(dataUtf8.Length - consumed, json.BytesConsumed); 
        
                   } 
        
               } 
        
           }

devsko · 2020-09-18T11:21:14Z

how about continuation where the token being split isn't null but other types, like whitespace (\r\n), or true/false boolean, or some large string token, or a number?

All tests here split the payload once on every single character. Thus all tokens are tested how they work when split into 2 chunks including all mentioned examples except whitespaces. I will add them by enabling WriteIndented

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.cs

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs

Added dictionary test

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.cs

more tweaks

devsko · 2020-09-18T20:04:10Z

I'd say that's it. Thanks for your suggestions, help and reviews. Really appreciated. Feel free to change whatever you want - or wait 2 weeks. See you - peace

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs

stephentoub · 2020-10-23T14:12:38Z

@devsko, thanks for your efforts here. Are you still working on this?

layomia

LGTM - @devsko we can merge this once conflicts and #42393 (comment) are resolved.

layomia · 2020-11-02T19:15:58Z

I pushed a commit to finish this PR.

devsko added 18 commits September 12, 2020 17:09

Repro dotnet#42070

d004c08

formatting

788494b

namespace

3554afc

Fix

dc1cc2f

never forget the header

7861772

Addressed feedback

af37649

refactoring

86bd9bd

Test with original repro data from dotnet#42070

329bc83

custom converter to ensure the padding is written in front of the tes…

a7b9c36

…ted object

merge

c29bd48

rename

68d7d24

test data moved to Strings.resx

3a8bf00

Merge branch 'fix-42070' into streamtest

e38c884

Using test data from SR

3710a49

Generalize continuation tests for payloads of any length

78a542c

Tweak the payload and expect `JsonException`

merge

e79ae0c

merge

677cec0

Dotnet-GitSync-Bot added the area-Meta label Sep 17, 2020

layomia added area-System.Text.Json and removed area-Meta labels Sep 17, 2020

layomia added this to the 5.0.0 milestone Sep 17, 2020

layomia requested review from jozkee, layomia and steveharter September 17, 2020 17:41

layomia mentioned this pull request Sep 17, 2020

[release/5.0-rc2] Prevent JSON async deserialization errors when partial buffer ends in the middle of null token (#42158) #42359

Merged

Test deserialize with Utf8JsonReader and ReadOnlySequence

f87130d

devsko marked this pull request as ready for review September 17, 2020 21:39

Again with value typed nested object

bf4d91b

Add tests for splitted whitespaces

8362d82

ahsonkhan approved these changes Sep 18, 2020

View reviewed changes

ahsonkhan reviewed Sep 18, 2020

View reviewed changes

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.cs Outdated Show resolved Hide resolved

steveharter reviewed Sep 18, 2020

View reviewed changes

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs Show resolved Hide resolved

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs Show resolved Hide resolved

Addressed feedback

0a87d16

Added dictionary test

layomia reviewed Sep 18, 2020

View reviewed changes

devsko added 2 commits September 18, 2020 21:41

Validate line and position of failure in tweaked payloads

8a5b20d

more tweaks

Fixed comment

79beb30

devsko force-pushed the streamtest branch from c9360cc to 79beb30 Compare September 18, 2020 19:53

ahsonkhan reviewed Oct 1, 2020

View reviewed changes

src/libraries/System.Text.Json/tests/Serialization/ContinuationTests.NullToken.cs Outdated Show resolved Hide resolved

layomia approved these changes Oct 29, 2020

View reviewed changes

layomia added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 29, 2020

Merge remote-tracking branch 'upstream/master' into streamtest

c0c8ba0

layomia self-assigned this Nov 2, 2020

layomia removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Nov 2, 2020

ahsonkhan approved these changes Nov 2, 2020

View reviewed changes

layomia merged commit e691753 into dotnet:master Nov 2, 2020

ghost locked as resolved and limited conversation to collaborators Dec 7, 2020

devsko deleted the streamtest branch March 5, 2021 18:11

JSON continuation tests #42393

JSON continuation tests #42393

Uh oh!

Conversation

devsko commented Sep 17, 2020

Uh oh!

devsko commented Sep 17, 2020

Uh oh!

devsko commented Sep 17, 2020

Uh oh!

ahsonkhan commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devsko commented Sep 18, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

devsko commented Sep 18, 2020

Uh oh!

Uh oh!

stephentoub commented Oct 23, 2020

Uh oh!

layomia left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

layomia commented Nov 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ahsonkhan commented Sep 17, 2020 •

edited

Loading

layomia left a comment •

edited

Loading