Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

ViewEngine requires more context, specifically the actual controller that handled the request. #787

Closed
ghstahl opened this issue Jul 13, 2014 · 7 comments

Comments

@ghstahl
Copy link

ghstahl commented Jul 13, 2014

My need: I need to know know what controller handled the request from within the ViewEngine.
in MVC 5

public override ViewEngineResult FindView(ControllerContext controllerContext, string viewName, string masterName, bool useCache)
{
    var name = controllerContext.Controller.GetType().Assembly.GetName().Name;
    SetLocationFormats(name);

    return base.FindView(controllerContext, viewName, masterName, useCache);
}

Notice the ControllerContext.

in MVC 6 a ViewEngine inherits from IViewEngine, which has no concept of the real controller.

public interface IViewEngine
{
    ViewEngineResult FindView([NotNull] IDictionary<string, object> context, [NotNull] string viewName);

    ViewEngineResult FindPartialView(
        [NotNull] IDictionary<string, object> context,
        [NotNull] string partialViewName);
}

Questions:

  1. Why was ControllerContext removed?
  2. How can I be made whole again as my Views all live in a versioned directories, and I have a mapping that matches a controller's assembly to a versioned directory?
    I simply fix up the search paths in my view engine to only look in the proper directories.
    \plugins\SomeArea.1.0.0.34\Views\

I would argue that the view engines need as much context information that you can give them.

I traced the problem to the ViewResult class, where;

public override async Task ExecuteResultAsync([NotNull] ActionContext context)
{
}

ActionContext context, would be fantastic to have passed in when you call FindView, instead of cherry picking context.RouteData.Values. Pass the entire context in please.

Thanks

@davidfowl
Copy link
Member

I'd change the bug title to something more specific. There's tons of breaking changes.

@ghstahl ghstahl changed the title Breaking changes in ViewEngine from MVC 5 to 6 ViewEngine requires more context, specifically the actual controller that handled the request. Jul 13, 2014
@pranavkm
Copy link
Contributor

Why was ControllerContext removed?

Remnant from an earlier design where the RazorViewEngine didn't depend on Mvc.Core. There's no good reason for it not have the ActionContext (which is a direct substitute for ControllerContext) any more.

@ghstahl
Copy link
Author

ghstahl commented Jul 14, 2014

pranavkm, can I read into your statement that I can count on a change in IViewEngine.
i.e something like.

public interface IViewEngine
{
    ViewEngineResult FindView([NotNull] ActionContext context, [NotNull] string viewName);
    ViewEngineResult FindPartialView( [NotNull] ActionContext context, 
    [NotNull] string partialViewName);
}

A good reason to do this is that;

  1. It is a super set of what you are passing now, as ActionContext contains the Dictionary you were passing.
  2. As framework designers, you would want implementers of custom view engines to have as much contextual information as possible.
  3. It wouldn't harm your current design, it just makes it more flexible.

I would really like to get a nod on this direction so that at least I can change the code myself and continue with my research in how a plugin architecture on top of this new design is going to work. I want to be ready to go day one when you guys release this thing.

@ghstahl
Copy link
Author

ghstahl commented Jul 15, 2014

I changed the my local copy of the code to the following;

public interface IViewEngine
{
    ViewEngineResult FindView([NotNull] ActionContext context, [NotNull] string viewName);
    ViewEngineResult FindPartialView( [NotNull] ActionContext context, 
    [NotNull] string partialViewName);
}

The fixup was very small and the MvcSample.Web works.
The good news is that it does not run afoul of the new design and puts back the flexibility that external ViewEngine authors need.

Hope this helps.

@yishaigalatzer
Copy link
Contributor

There are a few things to consider here:

  1. Controller is not a top level concept anymore, action is. Although the default actiondescriptorprovider finds actions through reflection over controller classes, that's not a requirement of the framework and actiondescriptor can be built in different ways that don't directly depend on having a controller class.
  2. You can have multiple controllers with the same name but in different namespaces.
  3. Using ActionContext in the view engine makes it less flexible from being consumed outside the method execution pipeline, but that part might be fine.

@lodejard we are interested in your feedback here, @pranavkm will stop in person.

Overall I think the design suggested here and in #802 is fine.

@yishaigalatzer yishaigalatzer added this to the 6.0.0-alpha3 milestone Jul 17, 2014
@ghstahl
Copy link
Author

ghstahl commented Jul 17, 2014

Yishai,

Thanks for the change.

My head is still stuck a bit in the previous MVC designs. When I said Controller, I really meant what "Assembly" originated this action. For all my cases that was the controller. I look forward to seeing the alternatives to having a controller, but the same applies..what "Assembly" originated that action.

MVC5: My plugin design had the assembly and content in a version-ed folder, so I needed to know the assembly to find what versioned folder. I then fixed up the search paths in my inherited version of the RazorViewEngine. i.e. /Plugins/myArea.1.0.0.34/Area/{2}/Views/{1}/{0}, etc

MVC6: I see pulling the versioned folder concept forward
My plan with the new design is to have my controllers in a vNext classlibrary, which creates a nuget package as output. This nuget package would also have my content(cshtml, images,etc). So far this looks very promising, but I will need to know how to add custom content to the nuget in the vNext classlibrary.

BTW: You guys are a bit heavy handed with the internal/private in the new RazorViewEngine. Not much I can do now by inheriting from it. If I can't inherit, I guess I have to copy it, which is too bad as 99% is unchanged. There are a lot of great generic helper classes I have seen in the framework, all marked as internal..Argh.

My WOW moment was when I did the following;

  1. Created a vNext classLibrary, and put in a simple controller.
  2. Attributed the controller with an area name.
  3. Built and posted it to Myget: https://www.myget.org/F/herb/api/v2/package/HerbController/1.0.0
  4. Using your MvcSample.Web, I made sure that the Nuget.config had my MyGet feed, updated the project.Json to pull my package. ZERO CODE change in MvcSample.WEB
  5. ran the app, navigated to my route "/Herb/Home/Index", My controller fired.
    WOW that's plugin delivery happiness.

@rynowak
Copy link
Member

rynowak commented Jul 17, 2014

@ghstahl We'd love to hear from you about specific things that can be improved or made more reusable (including RazorViewEngine). Open bugs telling us what you're trying to accomplish and why it's hard.

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

No branches or pull requests

6 participants