DataTables.Queryable icon indicating copy to clipboard operation
DataTables.Queryable copied to clipboard

Support passing up CancellationTokens to ToPagedListAsync().

Open VictorioBerra opened this issue 5 years ago • 4 comments

ASPNET Core automatically wires up a CancellationToken in your controller actions, so it would be really helpful if we could add ToListAsync() to the paging stuff.

MVC will automatically bind any CancellationToken parameters in an action method to the HttpContext.RequestAborted token, using the CancellationTokenModelBinder. This model binder is registered automatically when you call services.AddMvc() (or services.AddMvcCore()) in Startup.ConfigureServices(). via https://andrewlock.net/using-cancellationtokens-in-asp-net-core-mvc-controllers/

We would put it here and pass it all the way up to the calling code https://github.com/AlexanderKrutov/DataTables.Queryable/blob/master/DataTables.Queryable/PagedList.cs#L78 and this way we could avoid doing Task.Factory stuff like here https://github.com/AlexanderKrutov/DataTables.Queryable/blob/master/DataTables.Queryable/QueryableExtensions.cs#L78

VictorioBerra avatar Oct 25 '18 22:10 VictorioBerra

I have been attempting this, turns out I have bumped into an issue with the ToListAsync() and CountAsync() in EFCore and keeping net40 as target framework.

Honestly, I think we should drop support for net40 and only support netstandard. This is exactly the kind of problem netstandard is trying to solve.

VictorioBerra avatar Oct 29 '18 14:10 VictorioBerra

Hi @VictorioBerra, thank you so much for the pull request! Unfortunately I can not approve this without additional changes. The are some reasons why:

  1. We should not discard the net40 support. The library is used in some projects targeted to this platform, so we must keep it.
  2. Your changes require that the library depends on Microsoft.EntityFrameworkCore. Initially the lib is designed to work with any data source, not only EF. For example, it can be in-memory data set, and we should not limit users in this case.
  3. CancellationToken support is definetely needful for ASP.Net. But I'm not sure how it will work with different web servers (like Nancy which I'm use with the DataTables.Queryable). Additional investigation is needed here. I think it should be more universal way to integrate CancellationToken support without linking with EF and ASP.Net.

I appreciate your contribution and will keep the pull as a starting point for further investigation.

AlexanderKrutov avatar Oct 30 '18 07:10 AlexanderKrutov

Thanks a lot for looking at this I appreciate it. For now I think I can solve my immediate issue by doing my own paging so I'm the one calling ToListAsync and passing down the CancellationToken. It would be nice to see this implemented some day though.

-Tory

VictorioBerra avatar Oct 30 '18 12:10 VictorioBerra

You could keep the ASPNET Core web port though. I may PR you some tests when I can get around to it.

VictorioBerra avatar Oct 30 '18 15:10 VictorioBerra