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

Add support for ExecuteResultAsync #113

Open
thuannguy opened this issue Dec 13, 2017 · 8 comments
Open

Add support for ExecuteResultAsync #113

thuannguy opened this issue Dec 13, 2017 · 8 comments
Milestone

Comments

@thuannguy
Copy link

@thuannguy thuannguy commented Dec 13, 2017

The Asp.Net core counterpart has support for ExecuteResultAsync which is very useful. Without that, it is very hard to call a method asynchronously from ExecuteResult. Could you please consider adding support for it?

@abatishchev

This comment has been minimized.

Copy link

@abatishchev abatishchev commented Dec 13, 2017

So does IHttpActionResult.ExecuteAsync(). Or you're looking for something different?

@thuannguy

This comment has been minimized.

Copy link
Author

@thuannguy thuannguy commented Dec 13, 2017

Please let me clarify the request. I meant ActionResult of Asp.Net MVC as in the simplified example below:

public class MyController : Controller
{ 
    public ActionResult Resolve()
    {
        // Simplified code
        return new MyActionResult(...);    // the ExecuteResult method of this action result will be invoked by MVC itself.
    }
}

Therefore, IHttpActionResult.ExecuteAsync is not the one I need 😃

@jdaigle

This comment has been minimized.

Copy link

@jdaigle jdaigle commented Dec 13, 2017

If I were to create a PR adding support for async MVC ActionFilters and async MVC ActionResults, how likely is Microsoft to seriously consider it? I feel this could be a big win for users that can’t upgrade to Core (i.e. having to support a hybrid WebForms app).

I already created a prototype several months ago, including refactoring much of the Begin/End async code internal to the Controller execution pipeline - it’s not difficult to add support, but doing so without introducing breaking changes is the hard part.

@thuannguy

This comment has been minimized.

Copy link
Author

@thuannguy thuannguy commented Dec 13, 2017

@jdaigle yep, I can't upgrade to Core because many libs I need aren't available there yet and AFAICT it will take .Net/Asp.Net/Azure team a long time to support them. Therefore, I have a (slim) hope that supporting async ExecuteResult would be a less resistant path 😛

@mkArtakMSFT

This comment has been minimized.

Copy link
Member

@mkArtakMSFT mkArtakMSFT commented Dec 13, 2017

@thuannguy, which libraries are missing, that block you from switching to ASP.NET Core instead?
@jdaigle, unfortunately we do not plan to add/accept this feature. Rather, we'd be happy to enable missing APIs in ASP.NET Core instead to enable the switch.

@jdaigle

This comment has been minimized.

Copy link

@jdaigle jdaigle commented Dec 14, 2017

@mkArtakMSFT That's sad to hear, but not surprising given the scope of the changes. And unfortunately it's not really "missing APIs" so much as having a giant hybrid WebForms and MVC5 application that we'll need to support and add new features to for some time to come.

We might have considered "forking" MVC5 and using a private build to get the features we want. Except that strong-naming prevents us from doing that (and I suppose that you're unlikely to release the signing key for all to use).

But it might be possible to reimplement most of the MVC pipeline in a separate project, since most of the pipeline is implemented in Controller. We just need to create our own implementation of IAsyncController. So much the async implementation could be significant cleaned up and simplified using System.Threading.Tasks.Task, rather than the Begin/End asynchronous programming model.

@thuannguy

This comment has been minimized.

Copy link
Author

@thuannguy thuannguy commented Dec 14, 2017

@mkArtakMSFT
Some of them are
AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#852
and AzureAD/azure-activedirectory-identitymodel-extensions-for-dotnet#476. I can understand that they have limited resources and thus the WSTrust request won't be ready anytime soon.

I have the same "giant hybrid WebForms and MVC5 application" situation which is another adversary. The ability to do true async controller now would ease my pain a lot before I can plan for a full upgrade to Core.

@mkArtakMSFT

This comment has been minimized.

Copy link
Member

@mkArtakMSFT mkArtakMSFT commented Dec 14, 2017

@jdaigle, it is possible to get the same identity on your builds as you'd get from Microsoft provided builds using so-called Public Signing. Hope this helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.