OData / AspNetCoreOData

ASP.NET Core OData: A server library built upon ODataLib and ASP.NET Core
Other
457 stars 156 forks source link

Asynchronous calls to database with EnableQuery attribute [workaround solution] #1326

Open Forevka opened 1 week ago

Forevka commented 1 week ago

Assemblies affected ASP.NET Core OData 8.x and ASP.NET Core OData 9.x

Describe the bug All database calls that are made in order to fetch data from IQueryable<> that returned from controller route are made synchronous

Reproduce steps

public class AppraiseRequestController(
    AppraiSysContext dbContext, 
    UserIdentityAccessor userIdentityAccessor
)
    : ODataController
{
    [HttpGet]
    [EnableQuery]
    [AuthorizeActionFilter(ResourceConstant.AppraisalRequest, ActionsConstant.Read)]
    public IQueryable<AppraiseRequest> Get()
    {
        var query = dbContext.AppraiseRequests
                .Where(a => a.TenantId == userIdentityAccessor.UserIdentity.TenantId);

        return query;
    }
}

Data Model Sample model (but it's don't really matter)

public partial class AppraiseRequest : ITenantSpecific
{
    // db-type: integer 
    public int Id { get; set; }

    // db-type: integer 
    public int AppraiseTypeId { get; set; }

    // db-type: integer 
    public int StatusId { get; set; }
}

Expected behavior I don't really know on which side the problem is, but: I expect that if my database connector and linq provider implements Async versions, EnableQuery attribute will use this overloads.

Screenshots How it's now: sync

As you see on screenshot, database calls made synchronous

Additional context Workaround was created in case anybody need it, but I hope it will be fixed inside library


/// <summary>
/// It's workaround for OData EnableQuery attribute that forces to use synchronous database calls instead of async version
/// this code was used from: https://github.com/OData/WebApi/issues/2598
/// and: https://github.com/OData/WebApi/issues/2325
/// </summary>
public sealed class EnableQueryAsyncAttribute : EnableQueryAttribute
{
    private static readonly MethodInfo ReadInternalMethod = typeof(EnableQueryAsyncAttribute)
        .GetMethods(BindingFlags.Static | BindingFlags.NonPublic)
        .Single(method => method.Name == nameof(ReadInternal));

    public override void ValidateQuery(HttpRequest httpRequest, ODataQueryOptions queryOptions)
    {
        httpRequest.HttpContext.Items["_ODataQueryOptions"] = queryOptions;

        base.ValidateQuery(httpRequest, queryOptions);
    }

    public override async Task OnResultExecutionAsync(ResultExecutingContext context, ResultExecutionDelegate next)
    {
        if (context.Result is ObjectResult { Value: IQueryable queryable } objectResult)
        {
            var cancellationToken = context.HttpContext.RequestAborted;

            var queryOptions = context.HttpContext.Items["_ODataQueryOptions"] as ODataQueryOptions;
            var request = context.HttpContext.Request;

            //if $count is included in query
            if (queryOptions?.Count is { Value: true })
            {
                var filteredQueryable = (queryOptions.Filter == null ? queryable : queryOptions.Filter.ApplyTo(queryable, new ODataQuerySettings()))
                    as IQueryable<dynamic>;
                var count = await filteredQueryable.LongCountAsync(cancellationToken).ConfigureAwait(false);

                // Setting the TotalCount causes oData to not execute the TotalCountFunc.
                request.ODataFeature().TotalCount = count;
                if (count == 0)
                {
                    // No need to have oData execute the queryable.
                    var instance = Activator.CreateInstance(typeof(List<>).MakeGenericType(queryable.ElementType));
                    objectResult.Value = new OkObjectResult(instance);
                }
            }

            //asynchronous call to db for results
            var queryableType = queryable.GetType();
            if (queryableType.GetInterfaces().Any(x =>
                    x.IsGenericType &&
                    x.GetGenericTypeDefinition() == typeof(IAsyncEnumerable<>)))
            {
                var readInternalMethod = ReadInternalMethod.MakeGenericMethod(queryableType.GenericTypeArguments[0]);

                var invoked = readInternalMethod.Invoke(null, new object[] { queryable, cancellationToken })!;

                var result = await (Task<ICollection>)invoked;

                objectResult.Value = result;
            }
        }

        _ = await next().ConfigureAwait(false);
    }

    private static async Task<ICollection> ReadInternal<T>(object value, CancellationToken cancellationToken)
    {
        var asyncEnumerable = (IAsyncEnumerable<T>)value;
        var result = new List<T>();

        await foreach (var item in asyncEnumerable.WithCancellation(cancellationToken))
        {
            cancellationToken.ThrowIfCancellationRequested();

            result.Add(item);
        }

        return result;
    }
}

How to use:

public class AppraiseRequestController(
    AppraiSysContext dbContext, 
    UserIdentityAccessor userIdentityAccessor
)
    : ODataController
{
    [HttpGet]
    [EnableQueryAsync]
    [AuthorizeActionFilter(ResourceConstant.AppraisalRequest, ActionsConstant.Read)]
    public IQueryable<AppraiseRequest> Get()
    {
        var query = dbContext.AppraiseRequests
                .Where(a => a.TenantId == userIdentityAccessor.UserIdentity.TenantId);

        return query;
    }
}

result: async all calls are made async

julealgon commented 1 week ago

Duplicate of:

WanjohiSammy commented 1 day ago

@Forevka Are you able to achieve the async calls using:

[HttpGet]
[EnableQuery]
public IAsyncEnumerable<Customer> Get()
{
   return this.context.Customers.AsAsyncEnumerable();
}

as suggested here https://github.com/OData/WebApi/issues/2598

julealgon commented 1 day ago

@WanjohiSammy I'm pretty sure the moment you call AsAsyncEnumerable, anything else called on top of it (all the modifiers applied by EnableQuery) will be executed in memory, and not in the database, as it leaves the queryable realm.

So that is definitely not a good solution. It would only be usable if you injected ODataQueryOptions<T> and applied it yourself before calling AsAsyncEnumerable.