Mvc: Microsoft.AspNetCore.Mvc.ViewFeatures.ViewResultExecutor.ExecuteAsync no longer virtual.

[rjamesnw]

Microsoft.AspNetCore.Mvc.ViewFeatures.ViewResultExecutor.ExecuteAsync was changed from virtual to non-virtual. I had code working perfectly, hooking into this execution point, and now I cannot. I consider this a regression, not progress. :/

Is this a Bug or Feature request?:

Regression perhaps - this was not the case in v2.0.4.

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

N/A

Description of the problem:

Microsoft.AspNetCore.Mvc.ViewFeatures.ViewResultExecutor.ExecuteAsync is no longer virtual. This should be changed back, since there's no good reason not to.

Also, the IView view parameter was removed, which I also needed. PartialViewResultExecutor.ExecuteAsync has this parameter available - why the inconsistency?
_(update: turns out this part is less of an issue since the base ViewExecutor.ExecuteAsync() virtual method gets called, and contains this value there)_

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

Microsoft.AspNetCore.Mvc.ViewFeatures 2.1.0

[mkArtakMSFT]

Thanks for contacting us, @rjamesnw.
@pranavkm, @kichalla was this intentional guys?

[kichalla]

ViewResultExecutor was in an Internal namespace in 2.0.0 (and was made public in 2.1.0, and that's why our Api Check tool probably didn't catch it) , so depending on that type is risky as internal types are subject to change without notice.

Its probably just an oversight that we missed virtual on this particular one when we made it public, but @rynowak probably has a good idea.

@rynowak for quick reference. Commit: https://github.com/aspnet/Mvc/commit/38712609bb2291202bfa760ee62348691de0ddae#diff-63078ca699219f3019bd243b9fe84fb9

[rjamesnw]

“Virtual” was not “missed”, it was intentionally removed. The “PartialViewResultExecuter” was also moved and it didn’t get changed.
; Very inconsistent. I modified my code to encapsulate it for now so I have it working again, but I hope it’s just a temporary bandaid. ;)

[rynowak]

I can't think of a reason why it's non-virtual. We can add it back, it's not a breaking change.

[mkArtakMSFT]

Thanks guys. I'll try to include this as part of the 2.1.3 patch.

[kichalla]

Just so that we are on same page, the fix for this patch is only to make it virtual. We cannot change the signature of the method as mentioned here:

Also, the IView view parameter was removed, which I also needed. PartialViewResultExecutor.ExecuteAsync has this parameter available - why the inconsistency?

[rjamesnw]

Because the found IView parameter was removed I now have to copy and paste the view execution code from the MVC source because you provide no way now to hook into the process after the view is found and immediately before it gets rendered. Now if the MVC executor process changes I risk the code breaking and having to resync the code. I had events I triggered on the found view immediately before and after execution, but now I would have to put the event triggers in a Find() overload, which a user might call without any intention to render a view. Still, at least it will be back to being virtual so it helps.

A better design would be to have Find as is, and executor as it was, and let MVC call Find() to get the view and “Execute()” with the found view as a parameter. Anything else is regression and NOT progress at all. Explain why it is better otherwise?

[rynowak]

I suggest overriding https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewExecutor.cs#L142 or https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewExecutor.cs#L208

These are documented APIs on a public type that are will be called for views and partial views. This is something you could do immediately to unblock yourself.

[rjamesnw]

@rynowak I wasn't blocked, I went through the source and figured out my own workaround. ;) That said, I completely missed virtual ExecuteAsync() in the base ViewExecutor class that ViewResultExecutor inherits from. I think that is the solution here, since ViewResultExecutor.ExecuteAsync() calls base.ExecuteAsync() which is virtual. I didn't think to override and hook into that one, thanks! ;) I'll try that instead. 👍

_Update: It works as before now by overriding the base ViewExecutor.ExecuteAsync() method, thanks. :) I think this is actually the best solution going forward for my needs, as it provides even more details._

[mkArtakMSFT]

Thanks @rynowak.
@kichalla, please confirm whether the API under question is used at all. If not, per the offline discussion we had with @rynowak, let's obsolete it as part of 2.2 release.

[mkArtakMSFT]

@pranavkm, please look through the thread. We should obsolete an API here, not sure what exactly though :)

[pranavkm]

I don't think there's anything to do here. The API wasn't public in 2.0 and as of 2.1 it is. The solution that @rjamesnw has now sounds great. If there's a specific need to get to the IView instance from the executor, I'd recommend copying the code from the ViewResultExecutor as a starting point.