Update M.E.DependencyModel to use S.T.Json source for netstandard2.0 #5009
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Text.Json; | ||
|
|
||
| namespace Microsoft.Extensions.DependencyModel | ||
| { | ||
| internal static class JsonTextReaderExtensions | ||
| { | ||
| internal static bool TryReadStringProperty(this ref Utf8JsonReader reader, out string name, out string value) | ||
| { | ||
| name = null; | ||
| value = null; | ||
| if (reader.Read() && reader.TokenType == JsonTokenType.PropertyName) | ||
| { | ||
| name = reader.GetString(); | ||
|
|
||
| if (reader.Read()) | ||
| { | ||
| if (reader.TokenType == JsonTokenType.String) | ||
| { | ||
| value = reader.GetString(); | ||
| } | ||
| else | ||
| { | ||
| reader.Skip(); | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| internal static void Skip(this ref Utf8JsonReader jsonReader) | ||
|
There was a problem hiding this comment. There is an issue to add this API: https://github.com/dotnet/corefx/issues/33295 |
||
| { | ||
| if (jsonReader.TokenType == JsonTokenType.PropertyName) | ||
| { | ||
| jsonReader.Read(); | ||
| } | ||
|
|
||
| if (jsonReader.TokenType == JsonTokenType.StartObject || jsonReader.TokenType == JsonTokenType.StartArray) | ||
| { | ||
| int depth = jsonReader.CurrentDepth; | ||
| while (jsonReader.Read() && depth <= jsonReader.CurrentDepth) | ||
| { | ||
| } | ||
| } | ||
| } | ||
|
|
||
| internal static void ReadStartObject(this ref Utf8JsonReader reader) | ||
| { | ||
| reader.Read(); | ||
| CheckStartObject(ref reader); | ||
| } | ||
|
|
||
| internal static void CheckStartObject(ref Utf8JsonReader reader) | ||
| { | ||
| if (reader.TokenType != JsonTokenType.StartObject) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "{"); | ||
| } | ||
| } | ||
|
|
||
| internal static void CheckEndObject(this ref Utf8JsonReader reader) | ||
| { | ||
| if (reader.TokenType != JsonTokenType.EndObject) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "}"); | ||
| } | ||
| } | ||
|
|
||
| internal static string[] ReadStringArray(this ref Utf8JsonReader reader) | ||
| { | ||
| reader.Read(); | ||
| if (reader.TokenType != JsonTokenType.StartArray) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "["); | ||
| } | ||
|
|
||
| var items = new List<string>(); | ||
|
|
||
| while (reader.Read() && reader.TokenType == JsonTokenType.String) | ||
| { | ||
| items.Add(reader.GetString()); | ||
| } | ||
|
|
||
| if (reader.TokenType != JsonTokenType.EndArray) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "]"); | ||
| } | ||
|
|
||
| return items.ToArray(); | ||
| } | ||
|
|
||
| internal static string ReadAsString(this ref Utf8JsonReader reader) | ||
| { | ||
| reader.Read(); | ||
| if (reader.TokenType == JsonTokenType.Null) | ||
| { | ||
| return null; | ||
| } | ||
| if (reader.TokenType != JsonTokenType.String) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "a JSON string token"); | ||
| } | ||
| return reader.GetString(); | ||
| } | ||
|
|
||
| internal static bool ReadAsBoolean(this ref Utf8JsonReader reader) | ||
| { | ||
| reader.Read(); | ||
| if (reader.TokenType != JsonTokenType.True && reader.TokenType != JsonTokenType.False) | ||
| { | ||
| throw CreateUnexpectedException(ref reader, "a JSON true or false literal token"); | ||
| } | ||
| return reader.GetBoolean(); | ||
| } | ||
|
|
||
| internal static bool ReadAsNullableBoolean(this ref Utf8JsonReader reader, bool defaultValue) | ||
| { | ||
| reader.Read(); | ||
| if (reader.TokenType != JsonTokenType.True && reader.TokenType != JsonTokenType.False) | ||
| { | ||
| if (reader.TokenType == JsonTokenType.Null) | ||
| { | ||
| return defaultValue; | ||
| } | ||
| throw CreateUnexpectedException(ref reader, "a JSON true or false literal token"); | ||
| } | ||
| return reader.GetBoolean(); | ||
| } | ||
|
|
||
| internal static Exception CreateUnexpectedException(ref Utf8JsonReader reader, string expected) | ||
| { | ||
| return new FormatException($"Unexpected character encountered, excepted '{expected}' " + | ||
| $"at line {reader.CurrentState._lineNumber} position {reader.CurrentState._bytePositionInLine}"); | ||
|
There was a problem hiding this comment. The There was a problem hiding this comment. IMO you should only be using public APIs from the reader. If position info isn't already public then this is a good argument for providing it. If you need it here then other users of the reader will also likely use it. There was a problem hiding this comment.
Agreed. However, I don't really understand the need for this method. Why not rely on the exceptions thrown by the JsonReader itself? But, you are right that we may want to expose these publicly. cc @eerhardt There was a problem hiding this comment. Looks like the code was that way since it was migrated from using I assume it is so we can give "good" exception messages about exactly where we expect a new object/array/etc. What is the exception message you get when you rely on the exceptions thrown by the JsonReader itself? There was a problem hiding this comment. You get a The message string is generally one of the following: For example, for invalid JSON, you could see a message like: Now, assuming that the JSON itself is valid, and we are trying to validate the schema, calling GetString on something other than json string would throw an invalid operation exception with an invalid cast message: This exception message wouldn't contain the info about where we expect a new object/array. There was a problem hiding this comment.
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| // Copyright (c) .NET Foundation and contributors. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using System; | ||
| using System.Buffers; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
|
|
||
| namespace Microsoft.Extensions.DependencyModel | ||
| { | ||
| internal struct StreamBufferWriter : IBufferWriter<byte>, IDisposable | ||
|
There was a problem hiding this comment. We should consider adding such an adapter between IBufferWriter and Stream, publicly. There was a problem hiding this comment. Yes please. We have something similar in SignalR https://github.com/aspnet/AspNetCore/blob/958e9049894137bb08c0d239dca0cda7c850a691/src/SignalR/common/Shared/MemoryBufferWriter.cs#L14 (ignore the fact that it's also a Stream, we just did that for other reasons). There was a problem hiding this comment. Also the PipeWriter implemenation over a Stream is 90% similar https://github.com/aspnet/AspNetCore/blob/958e9049894137bb08c0d239dca0cda7c850a691/src/Http/Http/src/StreamPipeWriter.cs. There was a problem hiding this comment. This is something that @JeremyKuhne might have looked into. There was a problem hiding this comment. |
||
| { | ||
| private Stream _stream; | ||
| private byte[] _rentedBuffer; | ||
|
|
||
| private const int MinimumBufferSize = 256; | ||
|
|
||
| public StreamBufferWriter(Stream stream, int bufferSize = MinimumBufferSize) | ||
| { | ||
| if (bufferSize <= 0) | ||
| throw new ArgumentException(nameof(bufferSize)); | ||
|
|
||
| _stream = stream ?? throw new ArgumentNullException(nameof(stream)); | ||
|
|
||
| _rentedBuffer = ArrayPool<byte>.Shared.Rent(bufferSize); | ||
| } | ||
|
|
||
| public void Advance(int count) | ||
| { | ||
| _stream.Write(_rentedBuffer, 0, count); | ||
| _rentedBuffer.AsSpan(0, count).Clear(); | ||
ahsonkhan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| /// <summary> | ||
| /// Returns rented buffer back to the pool | ||
| /// </summary> | ||
| public void Dispose() | ||
| { | ||
| _rentedBuffer.AsSpan().Clear(); | ||
| ArrayPool<byte>.Shared.Return(_rentedBuffer); | ||
| _rentedBuffer = null; | ||
| } | ||
|
|
||
| public Memory<byte> GetMemory(int sizeHint = 0) | ||
| { | ||
| if (sizeHint < 0) | ||
| throw new ArgumentException(nameof(sizeHint)); | ||
|
|
||
| if (sizeHint == 0) | ||
| { | ||
| int newSize = _rentedBuffer.Length == 0 ? MinimumBufferSize : checked(_rentedBuffer.Length * 2); | ||
| byte[] temp = _rentedBuffer; | ||
| _rentedBuffer = ArrayPool<byte>.Shared.Rent(newSize); | ||
| temp.AsSpan().Clear(); | ||
ahsonkhan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ArrayPool<byte>.Shared.Return(temp); | ||
| } | ||
| else if (sizeHint > _rentedBuffer.Length) | ||
| { | ||
| var temp = _rentedBuffer; | ||
| _rentedBuffer = ArrayPool<byte>.Shared.Rent(sizeHint); | ||
| temp.AsSpan().Clear(); | ||
| ArrayPool<byte>.Shared.Return(temp); | ||
| } | ||
| Debug.Assert(_rentedBuffer.Length > 0); | ||
| return _rentedBuffer; | ||
| } | ||
|
|
||
| public Span<byte> GetSpan(int sizeHint = 0) | ||
| { | ||
| if (sizeHint < 0) | ||
| throw new ArgumentException(nameof(sizeHint)); | ||
|
|
||
| if (sizeHint == 0) | ||
| { | ||
| int newSize = _rentedBuffer.Length == 0 ? MinimumBufferSize : checked(_rentedBuffer.Length * 2); | ||
| byte[] temp = _rentedBuffer; | ||
| _rentedBuffer = ArrayPool<byte>.Shared.Rent(newSize); | ||
| temp.AsSpan().Clear(); | ||
| ArrayPool<byte>.Shared.Return(temp); | ||
| } | ||
| else if (sizeHint > _rentedBuffer.Length) | ||
| { | ||
| var temp = _rentedBuffer; | ||
| _rentedBuffer = ArrayPool<byte>.Shared.Rent(sizeHint); | ||
| temp.AsSpan().Clear(); | ||
| ArrayPool<byte>.Shared.Return(temp); | ||
| } | ||
| Debug.Assert(_rentedBuffer.Length > 0); | ||
| return _rentedBuffer; | ||
ahsonkhan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
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.
It feels like all these methods are things that most consumers would want... and it is obviously a non-trivial amount of code.
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.
@eerhardt A lot of the code in these extension methods is just about changing the exception type, and combining the move and get operations. If InvalidOperationException were acceptable then
ReadAsString()is justThe return value of Next is important when using the reader in async/chunked mode; and an exception model is what it is...