-
Notifications
You must be signed in to change notification settings - Fork 29
Description
Currently the Query method makes calls to the underlying providers in a sync fashion. Ideally we want to be able to make these async. I hope we can achieve this in 8 but understand if it gets pushed to 9.
The OData library is inherently sync and in some cases forces the sync evaluation of the linq queries. There's a multitude of issues logged on the libraries regarding this issue but it doesn't seem to be going anywhere. I mentioned in another issue that if you specify PageSize in the options to the ODataQuerySettings it forces sync evaluation. Count is applied in sync.
I note in the Table Controllers implementation there's this comment.
// Note that some IQueryable providers cannot execute all queries against the data source, so we have
// to switch to in-memory processing for those queries. This is done by calling ToListAsync() on the
// IQueryable. This is not ideal, but it is the only way to support all of the OData query options.
I'm guessing that the reason that the call isn't async is because the ToListAsync() would introduce a dependency on the underlying provider. In my case this would be cosmos but it would be the same for EFCore as well.
This is my current override of the query method. There's some cosmos specific code there which we would try and get away from.
[HttpGet]
[ActionName("QueryAsync")]
[ProducesResponseType(StatusCodes.Status200OK)]
public override async Task<IActionResult> QueryAsync(CancellationToken cancellationToken = default)
{
Logger.LogInformation("QueryAsync: {querystring}", HttpContext.Request.QueryString);
await AuthorizeRequestAsync(TableOperation.Query, null, cancellationToken).ConfigureAwait(false);
_ = BuildServiceProvider(Request);
IQueryable<TEntity> dataset = (await Repository.AsQueryableAsync(cancellationToken).ConfigureAwait(false))
.ApplyDataView(AccessControlProvider.GetDataView())
.ApplyDeletedView(Request, Options.EnableSoftDelete);
ODataValidationSettings validationSettings = new() {
MaxTop = Options.MaxTop,
AllowedQueryOptions = AllowedQueryOptions.Select |
AllowedQueryOptions.Filter |
AllowedQueryOptions.OrderBy |
AllowedQueryOptions.Skip |
AllowedQueryOptions.Top |
AllowedQueryOptions.Count
};
ODataQuerySettings querySettings = new() {
PageSize = Options.PageSize,
EnsureStableOrdering = true,
IgnoredQueryOptions = AllowedQueryOptions.Filter | AllowedQueryOptions.OrderBy | AllowedQueryOptions.Skip | AllowedQueryOptions.Top
};
ODataQueryContext queryContext = new(EdmModel, typeof(TEntity), new ODataPath());
ODataQueryOptions<TEntity> queryOptions = new(queryContext, Request);
try
{
queryOptions.Validate(validationSettings);
}
catch (ODataException validationException)
{
Logger.LogWarning("Query: Error when validating query: {Message}", validationException.Message);
return BadRequest(validationException.Message);
}
if(queryOptions.Filter is not null)
{
dataset = (IQueryable<TEntity>)queryOptions.Filter.ApplyTo(dataset, new ODataQuerySettings());
}
int count = 0;
if (queryOptions.Count is not null)
{
count = await dataset.CountAsync(cancellationToken).ConfigureAwait(false);
}
dataset = (IQueryable<TEntity>)queryOptions.ApplyTo(dataset, new ODataQuerySettings()
{
EnsureStableOrdering = true,
IgnoredQueryOptions = AllowedQueryOptions.Select | AllowedQueryOptions.Filter
});
FeedIterator<TEntity> feedIterator = dataset.ToFeedIterator();
List<TEntity> entities = [];
while (feedIterator.HasMoreResults)
{
FeedResponse<TEntity> feedResponse = await feedIterator.ReadNextAsync(cancellationToken).ConfigureAwait(false);
entities.AddRange(feedResponse);
}
IQueryable<object>? results = queryOptions.ApplyTo(entities.AsQueryable(), querySettings) as IQueryable<object>;
PagedResult result = BuildPagedResult(queryOptions, results, count);
Logger.LogInformation("Query: {Count} items being returned", result.Items.Count());
return Ok(result);
}
Basically...
- I've restricted the options to the ones supported.
- I'm applying the filter if it's specified.
- Executing the count if its requested.
- Applying the result of the query options with the exception of select and filter. (Could probably add Count to the exceptions as well as we're doing it outside and select seems to transform the result and break the query chain)
- Executing the query against the provider. (albeit very cosmos specific).
- Then apply the rest of the options which might contain select.
I haven't applied the client side evaluation catches at the moment but the count and execution parts could be wrapped in an async version.
My proposal is that we introduce delegate methods for CountAsync and ListAsync either through options or on the repository that we can pass the queryable to for the execution. Something like...
Func<IQueryable<TEntity>, CancellationToken, Task<int>> CountFunc { get; }
Func<IQueryable<TEntity>, CancellationToken, Task<List<TEntity>>> ListFunc { get; } // Maybe return IQueryable instead?
Set like follows:
public Func<IQueryable<TEntity>, CancellationToken, Task<int>> CountFunc => async (queryable, cancellationToken) => await queryable.CountAsync(cancellationToken); // Provider specific count func
public Func<IQueryable<TEntity>, CancellationToken, Task<List<TEntity>>> ListFunc => async (queryable, cancellationToken) => await queryable.ToListAsync(cancellationToken); // Provider specifc ToList func
Called as follows:
count = await Repository.CountFunc(dataset ,cancellationToken).ConfigureAwait(false);
List<TEntity> entities = await Repository.ListFunc(dataset, cancellationToken).ConfigureAwait(false);
Happy to put some code together for this for review.
Cheers
Rich