AspNetCoreOData
AspNetCoreOData copied to clipboard
Cannot avoid using synchronous calls in OData with entity framework
Assemblies affected Microsoft.AspNetCore.OData 8.2.5 (but probably all of them
Describe the bug
When using an IQueryable generated via entity framework the default behaviour is to have synchronous calls to the database rather than using it as an IAsyncEnumerable to avoid this, there is a workaround for this (described below) but I think the default behaviour should be to do this Asynchronously. Also if you add $count=true
then the counting will always happen synchronously and it's not even obvious what sort of workaround there would be.
Reproduction steps
Checkout the repo https://github.com/Shiney/AsyncEfCoreOdata/blob/main/AsyncEfCoreOdata/Controllers/Controller.cs here. Then run it, which should go to .../Odata/Customers/$filter=Id%20eq%201%20or%20Id%20eq%203&$count=true
This will then throw an error because I've configured non async DbDataReaders to not work.
The crucial code looks like this
/// <summary>
/// Throw an error if the reader is not async
/// </summary>
private class ReaderInterceptor : DbCommandInterceptor
{
public override DbDataReader ReaderExecuted(DbCommand command, CommandExecutedEventData eventData, DbDataReader result)
{
throw new InvalidOperationException("Expect all calls to read to be async");
}
}
[EnableQuery]
public async Task<ActionResult> Get()
{
var options = new DbContextOptionsBuilder<MyDbContext>()
.AddInterceptors(new ReaderInterceptor())
.UseSqlServer(
"Server=(localdb)\\mssqllocaldb;TrustServerCertificate=True;Trusted_Connection=True;MultipleActiveResultSets=true;pooling=true;Command Timeout=300;");
await Task.Delay(100);
// do not dispose so that lives for the whole request
// this is a bad practice, but it is done here to demonstrate the issue
var context = new MyDbContext(options.Options);
// Use a Values statement to return customers
return Ok(context.Database.SqlQueryRaw<Customer>(
"Select 1 as Id, 'Customer 1' as Name union all Select 2 as Id, 'Customer 2' as Name union all Select 3 as Id, 'Customer 3' as Name"));
}
}
public class Customer
{
public int Id { get; set; }
public string Name { get; set; }
}
Partial workaround
If it wasn't for the $count=true
I am able to partially work around this by using a custom attribute (see here for working example https://github.com/Shiney/AsyncEfCoreOdata/blob/partial-workaround/AsyncEfCoreOdata/Controllers/Controller.cs)
public class CustomEnableQueryAttribute : EnableQueryAttribute
{
public override void OnActionExecuted(ActionExecutedContext actionExecutedContext)
{
base.OnActionExecuted(actionExecutedContext);
if (actionExecutedContext.Result is ObjectResult { DeclaredType: null } objectResult)
{
if (objectResult.Value is IAsyncEnumerable<Customer>)
{
objectResult.DeclaredType = typeof(IAsyncEnumerable<Customer>);
}
}
}
}
Because this means we hit the condition to use the IAsyncEnumerable in ODataResourceSetSerializer.WriteObjectInlineAsync
However I couldn't work out a similar workaround for the count property.
Maybe the check in ODataResourceSetSerializer.WriteObjectInlineAsync
should check if writeContex.Type
is assignable to an IAsyncEnumerable
@ElizabethOkerio Is it related to the previous IAsyncEnumerable fix?
@Shiney The ODataFeature
class includes a property called TotalCountFunc
, which accepts a Func<long>
delegate. This delegate allows you to provide a custom implementation for retrieving the total count asynchronously.
public Func<long> TotalCountFunc { get; set; }
Then you can set this from the controller like this:
Request.ODataFeature().TotalCountFunc = //customfunc
@ElizabethOkerio would it make sense to also provide a version of that property with a Func<Task<long>>
signature, to allow for consumers to use an async method for the count as well?
This delegate allows you to provide a custom implementation for retrieving the total count asynchronously.
I feel there is a misunderstanding here on what is meant by "asynchronously". Without exposing that with a Task
, it is not possible to use an async
call to do it. Did you mean "separately" instead of "asynchronously"?
@julealgon you are right.
It does looks like there is a way to do this attached to this item on the WebAPI repository https://github.com/OData/WebApi/issues/2325 it seems a lot more involved thought than my workaround to make it all think the system think it's an async enumerable. It's also quite scary as it would be quite easy to implement something like that and implement some sort of subtle bug where you counted the data after you'd done the skip/top rather than befor.
Also if you add $count=true then the counting will always happen synchronously and it's not even obvious what sort of workaround there would be.
Similar behavior appears if the PageSize is specified for the EnableQuery attribute.