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.

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

Build a JsonDocument from an already positioned Utf8JsonReader #29008

Closed
bartonjs opened this issue Mar 19, 2019 · 3 comments
Closed

Build a JsonDocument from an already positioned Utf8JsonReader #29008

bartonjs opened this issue Mar 19, 2019 · 3 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Milestone

Comments

@bartonjs
Copy link
Member

public partial class JsonDocument
{
    /// <summary>
    ///   Attempts to parse one JSON value (including objects or arrays) from the provided reader.
    /// </summary>
    /// <param name="reader">The reader to read.</param>
    /// <param name="document">Receives the parsed document.</param>
    /// <returns>
    ///   <see langword="true"/> if a value was read and parsed into a JsonDocument,
    ///   <see langword="false"/> if the reader ran out of data while parsing.
    ///   All other situations result in an exception being thrown.
    /// </returns>
    /// <remarks>
    ///   <para>
    ///     Upon completion of this method <paramref name="reader"/> will positioned at the
    ///     final token in the JSON value.  If an exception is thrown, or <see langword="false"/>
    ///     is returned, the reader is reset to the state it was in when the method was called.
    ///   </para>
    ///
    ///   <para>
    ///     This method makes a copy of the data the reader acted on, there is no caller
    ///     requirement to maintain data integrity beyond the return of this method.
    ///   </para>
    /// </remarks>
    /// <exception cref="ArgumentException">
    ///   <paramref name="reader"/> is using unsupported options.
    /// </exception>
    /// <exception cref="ArgumentException">
    ///   The current <paramref name="reader"/> token does not start or represent a value.
    /// </exception>
    /// <exception cref="JsonReaderException">
    ///   A value could not be read from <paramref name="reader"/>.
    /// </exception>
    public static bool TryReadFrom(ref Utf8JsonReader reader, out JsonDocument document);

    /// <summary>
    ///   Parses one JSON value (including objects or arrays) from the provided reader.
    /// </summary>
    /// <param name="reader">The reader to read.</param>
    /// <returns>
    ///   A JsonDocument representing the value (and nested values) read from the reader.
    /// </returns>
    /// <remarks>
    ///   <para>
    ///     Upon completion of this method <paramref name="reader"/> will positioned at the
    ///     final token in the JSON value.  If an exception is thrown the reader is reset to
    ///     the state it was in when the method was called.
    ///   </para>
    ///
    ///   <para>
    ///     This method makes a copy of the data the reader acted on, there is no caller
    ///     requirement to maintain data integrity beyond the return of this method.
    ///   </para>
    /// </remarks>
    /// <exception cref="ArgumentException">
    ///   <paramref name="reader"/> is using unsupported options.
    /// </exception>
    /// <exception cref="ArgumentException">
    ///   The current <paramref name="reader"/> token does not start or represent a value.
    /// </exception>
    /// <exception cref="JsonReaderException">
    ///   A value could not be read from <paramref name="reader"/>.
    /// </exception>
    public static JsonDocument ReadFrom(ref Utf8JsonReader reader);
}

Why not Parse?

The Parse methods all have the behavior that they fail if there's unknown extra data. ReadFrom and TryReadFrom have a different model, since it's "one item from a prebuilt reader".

Why

This method empowers two main scenarios

1) A JSON payload is followed by some other content (another JSON payload, a delimiter, et cetera).

There's no specific case for this, but an imagined one would be a JSON payload in a MIME multipart document

Content-Type: multipart/mixed; boundary=gc0p4Jq0M2Yt08jU534c0p

MIME preamble.
--gc0p4Jq0M2Yt08jU534c0p

{ "this": { "example": { "contrived" : true } } }
--gc0p4Jq0M2Yt08jU534c0p
MIME epilogue.

The idea being that the reader can be used to report (via reader.BytesConsumed) how much JSON content there was, and the MIME enveloping validator can then determine that the next portion is a valid MIME header.

2) Only reading a sub-document

Particularly combined with the TextEquals method this allows for hybridization of the reader and the DOM.

private void ReadSomeConfigurationSection(ref Utf8JsonReader reader)
{
    while (reader.Read())
    {
        switch (reader.TokenType)
        {
            case JsonTokenType.EndObject:
                return;
            case JsonTokenType.PropertyName:
            {
                if (reader.TextEquals("segmentSize"))
                {
                    reader.ReadOrThrow();
                    _segmentSize = reader.GetInt32();
                }
                else if (reader.TextEquals("callbackData"))
                {
                    using (JsonDocument doc = JsonDocument.Parse(ref reader))
                    {
                        _callback(doc);
                    }
               }

               ...
@ghuntley
Copy link
Member

Being discussed at https://www.youtube.com/watch?v=vHqgsQ23GRg

@terrajobst
Copy link
Member

terrajobst commented Mar 19, 2019

  • Should we have a TryParse()? It looks like the caller would need to handle the "I need more data"-case. Forcing exception handling seems unfortunate.
  • It feel a bit odd that the fact we're taking a different type (Utf8JsonReader) is the key for wether this API does a partial parse (i.e. reads as much as it can but doens't throw when extra data is encountered.)

@terrajobst
Copy link
Member

terrajobst commented Mar 28, 2019

Video

We should have a consistent naming with the Parse methods. We suggest to go with ParseValue:

public partial class JsonDocument
{
    public static bool TryParseValue(ref Utf8JsonReader reader, out JsonDocument document);
    public static JsonDocument ParseValue(ref Utf8JsonReader reader);
}

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 3.0 milestone Feb 1, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 13, 2020
@bartonjs bartonjs removed their assignment Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

4 participants