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

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

Open
Forevka opened this issue Oct 16, 2024 · 6 comments
Assignees
Labels

Comments

@Forevka
Copy link

Forevka commented Oct 16, 2024

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

@Forevka Forevka added the bug Something isn't working label Oct 16, 2024
@Forevka Forevka changed the title Asynchronous calls to database with EnableQuery attribute [with workaround solution] Asynchronous calls to database with EnableQuery attribute [workaround solution] Oct 16, 2024
@julealgon
Copy link
Contributor

Duplicate of:

@xuzhg xuzhg added feature and removed bug Something isn't working labels Oct 21, 2024
@WanjohiSammy WanjohiSammy self-assigned this Oct 22, 2024
@WanjohiSammy
Copy link
Contributor

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

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

as suggested here OData/WebApi#2598

@julealgon
Copy link
Contributor

julealgon commented Oct 24, 2024

@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.

EDIT: As correctly pointed out by @rpallares, the above is actually not correct in this case. Because EnableQuery will handle the result object by trying to see whether it is an IQueryable before applying its filters, and because EFCore's AsAsyncEnumerable will return an object that still implements IQueryable (basically, the DbSet itself, but typed to IAsyncEnumerable), the filters will still modify the underlying query and be executed in the database.

I was coming from a position of direct usages, where one would do AsAsyncEnumerable and keep chaining operators (say, from System.Linq.Async). Those would of course not be executed in the database since they are not the IQueryable<T> versions of the methods. However, the situation here is different so this does not apply at all.

Thank you for the correction @rpallares .

@rpallares
Copy link
Contributor

@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.

In fact not. Because thé iasyncenumerable object returned by entityframework IS also an iqueryable. And enablequeryattribue IS smart enough to use both interfaces.

With two limitations, the count exposed here and also pagination that load the quey result instead of stream it.

@rpallares
Copy link
Contributor

Duplicate of:

* [Need to support async queries #775](https://github.com/OData/AspNetCoreOData/issues/775)

Note exactly a duplicate. This one talk about the include count whereas the issue you linked talk about executing the query synchonously when pagination is activated.
Both should be addressed.

@julealgon
Copy link
Contributor

julealgon commented Nov 7, 2024

@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.

In fact not. Because thé iasyncenumerable object returned by entityframework IS also an iqueryable. And enablequeryattribue IS smart enough to use both interfaces.

With two limitations, the count exposed here and also pagination that load the quey result instead of stream it.

You are of course absolutely correct @rpallares . I've amended my previous post to highlight this. Thanks for the correction and sorry for the confusion @WanjohiSammy and @Forevka .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants