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

Implement asynchronous support in ODataJsonLightParameterReader #2353

Merged
merged 3 commits into from
Mar 29, 2022
Merged

Implement asynchronous support in ODataJsonLightParameterReader #2353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
921062c
Implement asynchronous support in ODataJsonLightParameterReader
gathogojr Mar 16, 2022
645e9fc
Update Public API baseline
gathogojr Mar 23, 2022
1ac05ea
Code quality improvements
gathogojr Mar 28, 2022
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
86 changes: 71 additions & 15 deletions src/Microsoft.OData.Core/JsonLight/ODataJsonLightParameterReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
namespace Microsoft.OData.JsonLight
{
#region Namespaces

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Microsoft.OData.Metadata;
using Microsoft.OData.Edm;
using Microsoft.OData.Json;
using Microsoft.OData.Metadata;

#endregion Namespaces

Expand Down Expand Up @@ -70,8 +70,8 @@ protected override bool ReadAtStartImplementation()
this.jsonLightParameterDeserializer.ReadPayloadStart(
ODataPayloadKind.Parameter,
this.propertyAndAnnotationCollector,
/*isReadingNestedPayload*/false,
/*allowEmptyPayload*/true);
isReadingNestedPayload: false,
allowEmptyPayload: true);

return this.ReadAtStartImplementationSynchronously();
}
Expand All @@ -86,7 +86,7 @@ protected override bool ReadAtStartImplementation()
/// When the new state is Resource, the reader is positioned at the starting '{' of the resource payload.
/// When the new state is Resource Set or Collection, the reader is positioned at the starting '[' of the resource set or collection payload.
/// </remarks>
protected override Task<bool> ReadAtStartImplementationAsync()
protected override async Task<bool> ReadAtStartImplementationAsync()
{
Debug.Assert(this.State == ODataParameterReaderState.Start, "this.State == ODataParameterReaderState.Start");
Debug.Assert(this.jsonLightParameterDeserializer.JsonReader.NodeType == JsonNodeType.None, "Pre-Condition: expected JsonNodeType.None");
Expand All @@ -96,14 +96,14 @@ protected override Task<bool> ReadAtStartImplementationAsync()

// The parameter payload looks like "{ param1 : value1, ..., paramN : valueN }", where each value can be primitive, complex, collection, entity, resource set or collection.
// Position the reader on the first node
return this.jsonLightParameterDeserializer.ReadPayloadStartAsync(
await this.jsonLightParameterDeserializer.ReadPayloadStartAsync(
ODataPayloadKind.Parameter,
this.propertyAndAnnotationCollector,
/*isReadingNestedPayload*/false,
/*allowEmptyPayload*/true)
isReadingNestedPayload: false,
allowEmptyPayload: true).ConfigureAwait(false);

.FollowOnSuccessWith(t =>
this.ReadAtStartImplementationSynchronously());
return await this.ReadAtStartInternalImplementationAsync()
.ConfigureAwait(false);
}

/// <summary>
Expand Down Expand Up @@ -131,9 +131,18 @@ protected override bool ReadNextParameterImplementation()
/// When the new state is Resource, the reader is positioned at the starting '{' of the resource payload.
/// When the new state is Resource Set or Collection, the reader is positioned at the starting '[' of the resource set or collection payload.
/// </remarks>
protected override Task<bool> ReadNextParameterImplementationAsync()
protected override async Task<bool> ReadNextParameterImplementationAsync()
{
return TaskUtils.GetTaskForSynchronousOperation<bool>(this.ReadNextParameterImplementationSynchronously);
Debug.Assert(
this.State != ODataParameterReaderState.Start &&
this.State != ODataParameterReaderState.Exception &&
this.State != ODataParameterReaderState.Completed,
"The current state must not be Start, Exception or Completed.");

this.PopScope(this.State);

return await this.jsonLightParameterDeserializer.ReadNextParameterAsync(this.propertyAndAnnotationCollector)
.ConfigureAwait(false);
}

/// <summary>
Expand All @@ -153,7 +162,17 @@ protected override ODataReader CreateResourceReader(IEdmStructuredType expectedR
/// <returns>An <see cref="ODataReader"/> to read the resource value of type <paramref name="expectedResourceType"/>.</returns>
protected override Task<ODataReader> CreateResourceReaderAsync(IEdmStructuredType expectedResourceType)
{
return TaskUtils.GetTaskForSynchronousOperation<ODataReader>(() => this.CreateResourceReaderSynchronously(expectedResourceType));
Debug.Assert(expectedResourceType != null, "expectedResourceType != null");

return Task.FromResult<ODataReader>(
new ODataJsonLightReader(
this.jsonLightInputContext,
navigationSource: null,
expectedResourceType: expectedResourceType,
readingResourceSet: false,
readingParameter: true,
readingDelta: false,
listener: this));
}

/// <summary>
Expand All @@ -173,7 +192,17 @@ protected override ODataReader CreateResourceSetReader(IEdmStructuredType expect
/// <returns>An <see cref="ODataReader"/> to read the resource set value of type <paramref name="expectedResourceType"/>.</returns>
protected override Task<ODataReader> CreateResourceSetReaderAsync(IEdmStructuredType expectedResourceType)
{
return TaskUtils.GetTaskForSynchronousOperation<ODataReader>(() => this.CreateResourceSetReaderSynchronously(expectedResourceType));
Debug.Assert(expectedResourceType != null, "expectedResourceType != null");

return Task.FromResult<ODataReader>(
new ODataJsonLightReader(
this.jsonLightInputContext,
navigationSource: null,
expectedResourceType: expectedResourceType,
readingResourceSet: true,
readingParameter: true,
readingDelta: false,
listener: this));
}

/// <summary>
Expand Down Expand Up @@ -203,7 +232,11 @@ protected override ODataCollectionReader CreateCollectionReader(IEdmTypeReferenc
/// </remarks>
protected override Task<ODataCollectionReader> CreateCollectionReaderAsync(IEdmTypeReference expectedItemTypeReference)
{
return TaskUtils.GetTaskForSynchronousOperation<ODataCollectionReader>(() => this.CreateCollectionReaderSynchronously(expectedItemTypeReference));
Debug.Assert(this.jsonLightInputContext.Model.IsUserModel(), "Should have verified that we created the parameter reader with a user model.");
Debug.Assert(expectedItemTypeReference != null, "expectedItemTypeReference != null");

return Task.FromResult<ODataCollectionReader>(
new ODataJsonLightCollectionReader(this.jsonLightInputContext, expectedItemTypeReference, listener: this));
}

/// <summary>
Expand Down Expand Up @@ -288,5 +321,28 @@ private ODataCollectionReader CreateCollectionReaderSynchronously(IEdmTypeRefere
Debug.Assert(expectedItemTypeReference != null, "expectedItemTypeReference != null");
return new ODataJsonLightCollectionReader(this.jsonLightInputContext, expectedItemTypeReference, this /*IODataReaderListener*/);
}

/// <summary>
/// Asynchronous implementation of the reader logic when in state 'Start'.
/// </summary>
/// <returns>true if more items can be read from the reader; otherwise false.</returns>
/// <remarks>
/// Pre-Condition: JsonNodeType.None: assumes that the JSON reader has not been used yet.
/// Post-Condition: When the new state is Value, the reader is positioned at the closing '}' or at the name of the next parameter.
/// When the new state is Resource, the reader is positioned at the starting '{' of the resource payload.
/// When the new state is Resource Set or Collection, the reader is positioned at the starting '[' of the resource set or collection payload.
/// </remarks>
private async Task<bool> ReadAtStartInternalImplementationAsync()
{
if (this.jsonLightInputContext.JsonReader.NodeType == JsonNodeType.EndOfInput)
{
this.PopScope(ODataParameterReaderState.Start);
this.EnterScope(ODataParameterReaderState.Completed, null, null);
return false;
}

return await this.jsonLightParameterDeserializer.ReadNextParameterAsync(this.propertyAndAnnotationCollector)
.ConfigureAwait(false);
}
}
}
51 changes: 50 additions & 1 deletion src/Microsoft.OData.Core/ODataParameterReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
namespace Microsoft.OData
{
#region Namespaces
using System.Diagnostics.CodeAnalysis;

using System;
using System.Threading.Tasks;

#endregion Namespaces
Expand Down Expand Up @@ -75,5 +76,53 @@ public abstract object Value
/// <summary> Asynchronously reads the next item from the message payload. </summary>
/// <returns>A task that when completed indicates whether more items were read.</returns>
public abstract Task<bool> ReadAsync();

/// <summary>
/// This method asynchronously creates an <see cref="ODataReader"/> to read the resource value when the state is ODataParameterReaderState.Resource.
/// </summary>
/// <returns>
/// A task that represents the asynchronous operation.
/// The value of the TResult parameter contains an <see cref="ODataReader"/> to read the resource value when the state is ODataParameterReaderState.Resource.
/// </returns>
/// <remarks>
/// When the state is ODataParameterReaderState.Resource, the Name property of the <see cref="ODataParameterReader"/> returns the name of the parameter
/// and the Value property of the <see cref="ODataParameterReader"/> returns null. Calling this method in any other state will cause an ODataException to be thrown.
/// </remarks>
public virtual Task<ODataReader> CreateResourceReaderAsync()
{
throw new NotImplementedException();
}

/// <summary>
/// This method asynchronously creates an <see cref="ODataReader"/> to read the resource set value when the state is ODataParameterReaderState.ResourceSet.
/// </summary>
/// <returns>
/// A task that represents the asynchronous operation.
/// The value of the TResult parameter contains an <see cref="ODataReader"/> to read the resource set value when the state is ODataParameterReaderState.ResourceSet.
/// </returns>
/// <remarks>
/// When the state is ODataParameterReaderState.ResourceSet, the Name property of the <see cref="ODataParameterReader"/> returns the name of the parameter
/// and the Value property of the <see cref="ODataParameterReader"/> returns null. Calling this method in any other state will cause an ODataException to be thrown.
/// </remarks>
public virtual Task<ODataReader> CreateResourceSetReaderAsync()
{
throw new NotImplementedException();
}

/// <summary>
/// Asynchronously creates an <see cref="Microsoft.OData.ODataCollectionReader" /> to read the collection value when the state is ODataParameterReaderState.Collection.
/// </summary>
/// <returns>
/// A task that represents the asynchronous operation.
/// The value of the TResult parameter contains an <see cref="ODataCollectionReader"/> to read the collection value when the state is ODataParameterReaderState.Collection.
/// </returns>
/// <remarks>
/// When the state is ODataParameterReaderState.Collection, the Name property of the <see cref="ODataParameterReader"/> returns the name of the parameter
/// and the Value property of the <see cref="ODataParameterReader"/> returns null. Calling this method in any other state will cause an ODataException to be thrown.
/// </remarks>
public virtual Task<ODataCollectionReader> CreateCollectionReaderAsync()
{
throw new NotImplementedException();
}
}
}
67 changes: 49 additions & 18 deletions src/Microsoft.OData.Core/ODataParameterReaderCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace Microsoft.OData
{
#region Namespaces

using System;
using System.Collections.Generic;
using System.Diagnostics;
Expand All @@ -15,6 +16,7 @@ namespace Microsoft.OData
using System.Threading.Tasks;
using Microsoft.OData.Edm;
using Microsoft.OData.Metadata;

#endregion Namespaces

/// <summary>
Expand All @@ -35,7 +37,7 @@ internal abstract class ODataParameterReaderCore : ODataParameterReader, IODataR
private readonly HashSet<string> parametersRead = new HashSet<string>(StringComparer.Ordinal);

/// <summary>Tracks the state of the sub-reader.</summary>
private SubReaderState subReaderState;
protected SubReaderState subReaderState;

/// <summary>
/// Constructor.
Expand All @@ -53,7 +55,7 @@ protected ODataParameterReaderCore(
}

/// <summary>Enum to track the state of the sub-reader.</summary>
private enum SubReaderState
protected enum SubReaderState
{
/// <summary>No sub-reader has been created for the current parameter.</summary>
None,
Expand Down Expand Up @@ -182,7 +184,7 @@ public override ODataCollectionReader CreateCollectionReader()
public override sealed bool Read()
{
this.VerifyCanRead(true);
return this.InterceptException(this.ReadSynchronously);
return this.InterceptException((thisParam) => thisParam.ReadSynchronously());
}

/// <summary>
Expand All @@ -192,7 +194,7 @@ public override sealed bool Read()
public override sealed Task<bool> ReadAsync()
{
this.VerifyCanRead(false);
return this.ReadAsynchronously().FollowOnFaultWith(t => this.EnterScope(ODataParameterReaderState.Exception, null, null));
return this.InterceptExceptionAsync((thisParam) => thisParam.ReadAsynchronously());
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you update the this.ReadAsynchronously to be "truly async"? I don't think I've seen it on the diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@habbes The "truly async" implementation of ReadAsynchronously is in ODataParameterReaderCoreAsync class

}

/// <summary>
Expand Down Expand Up @@ -432,6 +434,27 @@ protected virtual Task<bool> ReadAsynchronously()
return TaskUtils.GetTaskForSynchronousOperation<bool>(this.ReadImplementation);
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be "truly async" 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.

@habbes The "truly async" implementation of ReadAsynchronously is contained in ODataParameterReaderCoreAsync. Because ReadAsync method is declared as abstract and ODataParameterReaderCore needs to override it, we're just wrapping the synchronous method in a task as a stub implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, is that worth mentioning in a comment?

}

/// <summary>
/// Verifies that one of CreateResourceReader(), CreateResourceSetReader(), CreateCollectionReader(),
/// CreateResourceReaderAsync(), CreateResourceSetReaderAsync() or CreateCollectionReaderAsync() can be called.
/// </summary>
/// <param name="expectedState">The expected state of the reader.</param>
protected void VerifyCanCreateSubReader(ODataParameterReaderState expectedState)
{
this.inputContext.VerifyNotDisposed();

if (this.State != expectedState)
{
throw new ODataException(Strings.ODataParameterReaderCore_InvalidCreateReaderMethodCalledForState(ODataParameterReaderCore.GetCreateReaderMethodName(expectedState), this.State));
}

if (this.subReaderState != SubReaderState.None)
{
Debug.Assert(this.Name != null, "this.Name != null");
throw new ODataException(Strings.ODataParameterReaderCore_CreateReaderAlreadyCalled(ODataParameterReaderCore.GetCreateReaderMethodName(expectedState), this.Name));
}
}

/// <summary>
/// Gets the corresponding create reader method name for the given state.
/// </summary>
Expand All @@ -444,36 +467,44 @@ private static string GetCreateReaderMethodName(ODataParameterReaderState state)
}

/// <summary>
/// Verifies that one of CreateResourceReader(), CreateResourceSetReader() or CreateCollectionReader() can be called.
/// Catch any exception thrown by the action passed in; in the exception case move the reader into
/// state ExceptionThrown and then rethrow the exception.
/// </summary>
/// <param name="expectedState">The expected state of the reader.</param>
private void VerifyCanCreateSubReader(ODataParameterReaderState expectedState)
/// <typeparam name="T">The type returned from the <paramref name="action"/> to execute.</typeparam>
/// <param name="action">The action to execute.</param>
/// <returns>The result of executing the <paramref name="action"/>.</returns>
private T InterceptException<T>(Func<ODataParameterReaderCore, T> action)
{
this.inputContext.VerifyNotDisposed();
if (this.State != expectedState)
try
{
throw new ODataException(Strings.ODataParameterReaderCore_InvalidCreateReaderMethodCalledForState(ODataParameterReaderCore.GetCreateReaderMethodName(expectedState), this.State));
return action(this);
}

if (this.subReaderState != SubReaderState.None)
catch (Exception e)
{
Debug.Assert(this.Name != null, "this.Name != null");
throw new ODataException(Strings.ODataParameterReaderCore_CreateReaderAlreadyCalled(ODataParameterReaderCore.GetCreateReaderMethodName(expectedState), this.Name));
if (ExceptionUtils.IsCatchableExceptionType(e))
{
this.EnterScope(ODataParameterReaderState.Exception, null, null);
}

throw;
}
}

/// <summary>
/// Catch any exception thrown by the action passed in; in the exception case move the reader into
/// state ExceptionThrown and then rethrow the exception.
/// </summary>
/// <typeparam name="T">The type returned from the <paramref name="action"/> to execute.</typeparam>
/// <typeparam name="TResult">The type returned from the <paramref name="action"/></typeparam>
/// <param name="action">The action to execute.</param>
/// <returns>The result of executing the <paramref name="action"/>.</returns>
private T InterceptException<T>(Func<T> action)
/// <returns>
/// A task that represents the asynchronous operation.
/// The value of the TResult parameter contains the result of executing the <paramref name="action"/>.
/// </returns>
private async Task<TResult> InterceptExceptionAsync<TResult>(Func<ODataParameterReaderCore, Task<TResult>> action)
{
try
{
return action();
return await action(this).ConfigureAwait(false);
}
catch (Exception e)
{
Expand Down
Loading