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

Supporting open generic models in Views #4865

Closed
mansoor-omrani opened this issue Dec 21, 2017 · 17 comments
Closed

Supporting open generic models in Views #4865

mansoor-omrani opened this issue Dec 21, 2017 · 17 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Milestone

Comments

@mansoor-omrani
Copy link

The story:

The idea came to my mind when summarizing multiple views using partials. I thought it would be great if we could define partial views with open generic models.

I thought at first to post the idea in ASP.NET uservoice website. When I searched there, I noticed the same idea was suggested a few years ago https://aspnet.uservoice.com/forums/41201-asp-net-mvc/suggestions/2706166-support-for-generics-in-razor-view.

There, @danroth27 had directed users to a new track ( #727 ) in ASP.NET github and asked users to comment practical examples/use cases of the suggestion.

After visiting the issue, I noticed that it was closed, because nobody had given any comment. I didn't know if someone was still allowed to comment on a closed issue or not, but I dared to try it.

I was happy to see @Eilon's quick reply some hours later still welcoming use cases for this idea.

As the scenario I wanted to mention was a little long, he recommended me to post it as new issue.

So, here am I.

The scenario involves multiple pieces of code fragments (C#, Razor views), but I do my best to make it short and only mention the meat.

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Dec 21, 2017

Intro
Suppose we need to implement an administration panel for a website consisting of various modules, such as Blogs, Photos, News, Products, etc.

Data Layer
We use EF code first for our models and data layer.

Model objects
We use a base class for all of our model classes as below.

public class BaseEntity
{
   public int Id { get; set; }
   public bool IsDeleted { get; set; }
   public DateTime? CreatedDate { get; set; }
   public virtual User CreatedBy {  get; set; }
   public DateTime? ModifiedDate { get; set; }
   public virtual User ModifiedBy {  get; set; }
   [Timestamp]
   public byte[] RowVersion { get; set; }
}

This structure provides us the following features:

  • Auditing : when a record was inserted, by whom it was inserted, when it was last updated, who has updated it (CreatedDate, CreatedBy, ModifiedDate, ModifiedBy)
  • Logical deletion of records (IsDeleted property)
  • Concurrency management (RowVersion property)

This is our model classes:

public class Blog: EntityBase
{
   public string Title { get; set; }
   public string Content { get; set; }
   public string ImageUrl { get; set; }
}
public class Photo: EntityBase
{
   public string Title { get; set; }
   public string SmallImageUrl { get; set; }
   public string LargeImageUrl { get; set; }
}
public class News: EntityBase
{
   public string Title { get; set; }
   public string Content { get; set; }
   public string Source { get; set; }
   public string ThumbnailUrl { get; set; }
}
public class Product: EntityBase
{
   public string Title { get; set; }
   public decimal Price { get; set; }
   public string SmallImageUrl { get; set; }
   public string LargeImageUrl { get; set; }
}

We don't add a BlogDate, NewsDate or Author property to Blog and News models. We use CreatedDate and CreatedBy properties to satisfy this need.

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Dec 21, 2017

For the administration panel we create an "Admin" area in our MVC application.

There, for each module, we create a controller. So, we have a BlogController, NewsController, PhotoController and ProductController.

I don't go through the code of the data layer, repositories, the controllers and their actions. They are not our concern.

I go right to the Views which is the topic of this post.

Views
We use HTML tables to provide a simple data grid with edit/delete links for each module.

This is the View of Index action in our BlogController:

@model IEnumerable<Blog>
@{
    ViewBag.Title = "Blogs";
}

<table class="table table-striped">
    <thead>
        <tr>
            <th>Id</th>
            <th>Title</th>
            <th>Image</th>
            <th>Created Date</th>
            <th>Created By</th>
            <th>Modified Date</th>
            <th>Modified By</th>
            <th></th>
            <th></th>
        </tr>
    </thead>
    <tbody>
@foreach (var item in Model)
{
        <tr>
            <td>@item.Id</td>
            <td>@item.Title</td>
            <td><img src="@item.ImageUrl"/></td>
            <td>@item.CreatedDate</td>
            <td>@(item.CreatedBy?.UserName)</td>
            <td>@item.ModifiedDate</td>
            <td>@(item.ModifiedBy?.UserName)</td>
            <td>@Html.Action("Edit", new { Id = item.Id })</td>
            <td>@Html.Action("Delete", new { Id = item.Id })</td>
        </tr>
}
    </tbody>
</table>

@Html.Pager(ViewBag,CurrentPage, ViewBag.PageSize, ViewBag.PageCount, ViewBag.RecordCount)

The View receives and shows one page of data. Thus, a pager is displayed at the bottom of the page using an extension method named Html.Pager() and the arguments are passed to it from ViewBag.

This suffices the whole structure of the page.

Not surprisingly, the Views for the Index action in all our other controllers, (NewsController, PhotoController and ProductController) follow a similar code.

For example, this is the view for the Index action of ProductController.

@model IEnumerable<Product>
@{
    ViewBag.Title = "Products";
}

<table class="table table-striped">
    <thead>
        <tr>
            <th>Id</th>
            <th>Title</th>
            <th>Price</th>
            <th>Image</th>
            <th>Created Date</th>
            <th>Created By</th>
            <th>Modified Date</th>
            <th>Modified By</th>
            <th></th>
            <th></th>
        </tr>
    </thead>
    <tbody>
@foreach (var item in Model)
{
        <tr>
            <td>@item.Id</td>
            <td>@item.Title</td>
            <td>@item.Price</td>
            <td><img src="@item.SmallImageUrl"/></td>
            <td>@item.CreatedDate</td>
            <td>@(item.CreatedBy?.UserName)</td>
            <td>@item.ModifiedDate</td>
            <td>@(item.ModifiedBy?.UserName)</td>
            <td>@Html.Action("Edit", new { Id = item.Id })</td>
            <td>@Html.Action("Delete", new { Id = item.Id })</td>
        </tr>
}
    </tbody>
</table>

@Html.Pager(ViewBag,CurrentPage, ViewBag.PageSize, ViewBag.PageCount, ViewBag.RecordCount)

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Dec 21, 2017

The Idea

Now let's go for the presented idea.

It would be a great help if we could define a generic View like AdminGrid.cstml like below:

@model IEnumerable<T> where T: EntityBase

<table class="table table-striped">
    <thead>
        <tr>
            <th>Id</th>
            @Html.Partial<T>("AdminGridHead")
            <th>Created Date</th>
            <th>Created By</th>
            <th>Modified Date</th>
            <th>Modified By</th>
            <th></th>
            <th></th>
        </tr>
    </thead>
    <tbody>
@foreach (var item in Model)
{
        <tr>
            <td>@item.Id</td>
            @Html.Partial<T>("AdminGridBody", item)
            <td>@item.CreatedDate</td>
            <td>@(item.CreatedBy?.UserName)</td>
            <td>@item.ModifiedDate</td>
            <td>@(item.ModifiedBy?.UserName)</td>
            <td>@Html.Action("Edit", new { Id = item.Id })</td>
            <td>@Html.Action("Delete", new { Id = item.Id })</td>
        </tr>
}
    </tbody>
</table>

@Html.Pager(ViewBag,CurrentPage, ViewBag.PageSize, ViewBag.PageCount, ViewBag.RecordCount)

We are in fact extracting the repetitive or common HTML code out of all of our views into a single View. This is somehow following the DRY principle.

Notice that I used two generic Html.Partial() method calls in this AdminGrid View, namely AdminGridHead and AdminGridBody to provide a way for the developer to implement custom HTML codes based on each type for the generic T parameter.

It's important to know that, despite we have a single AdminGrid.cshtml View, we should have separate AdminGridHead and AdminGridBody partial views, each one for a type that will be used for the generic T parameter in AdminGrid's model part, i.e. Blog, News, Photo, Product, etc.

added at 2017-12-23
The following could be AdminGridHead and AdminGridBody partial views for Blog type.

// AdminGridHead
<th>Title</th>
<th>Image</th>
// AdminGridBody
@model Blog
<td>@Html.DisplayFor(m => m.Title)</td>
<td>@Html.DisplayFor(m => m.Image)</td>

The partial views for other models have a similar code.

These views could be defined using a file naming convention like below (use the name of the concrete type in the partial view filename):

  • AdminGridHead.Blog.cshtml
  • AdminGridBody.Blog.cshtml
  • AdminGridHead.News.cshtml
  • AdminGridBody.News.cshtml
  • AdminGridHead.Photo.cshtml
  • AdminGridBody.Photo.cshtml
  • AdminGridHead.Product.cshtml
  • AdminGridBody.Product.cshtml

added at 2017-12-23
We can also another naming convention in our view engine to find these partial views. For example we can have a folder for each partial view based on its name and put the partial view files in it based on the type of the generic T parameter.

  • AdminGridHead
    • Blog.cshtml
    • News.cshtml
    • Photo.cshtml
    • Product.cshtml
  • AdminGridBody
    • Blog.cshtml
    • News.cshtml
    • Photo.cshtml
    • Product.cshtml

Although, each of these Views will have a concrete model not a generic model, based on the fact that they are called using a generic Html.Partial<T>() extension method (which we don't have at the moment), they in turn aim to the topic of this post (supporting generic models in Views).

added at 2017-12-23
The thing is, we should map the string name of the partial view, ("AdminGridHead", "AdminGridBody") to a file on the file system, the same as we do for other views or partial views. But for these specific partial views that are not a single file (the string "AdminGridHead" maps to multiple files), we need to use the generic T parameter in Html.Partial() extension method. That is why, we should have generic overloads of Html.Partial(0 extension method.

Now, supposing we have the ability of defining Views with generic models, in our Index actions we should return the model using a generic View<T>().

public class BlogController: Controller
{
   // ...
  public ActionResult Index(int page = 1, int pagesize = 5)
  {
      var model = BlogRepository.GetPage(page, pagesize);

      return View<Blog>(model);
  }
}

It is important to call a generic View<T>() in place of the non-generic View() method which is inherited to controllers.

I explain the reason a little further.

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Dec 21, 2017

This was the whole scenario.

I will explain potential problems we have in front of us when implementing this feature in future comments.

Because, when I looked at MVC's source code, I noticed that it's not that easy to implement this feature as it seems in the first glance (what I personally thought too at first!).

The key point is, we need to carry the generic T, from the top api like Html.Partial() extension method down to the ViewEngine, ViewPageActivator and other classes that play role in creating the C# class of the views, finding the files of views and instantiating from the view class.

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Dec 21, 2017

Now, why we need a generic View<T>() method in our controllers? Why we can't call the old View() method in our actions to return a View whose model is generic?

The reason is that, the View cannot be instantiated without its generic parameters known.

Suppose we have the following View named AdminGrid.cshtml which has a generic IEnumerable model.

@model IEnumeranle<T> where T: EntityBase
<table>
@foreach (var item in Model)
{
<tr>
   <td>@item.Id</td>
   <td>@item.CreatedDate</td>
</tr>
}
</table>

Let's suppose such a View is supported.

The C# class that is required to be generated for this View which should inherit WebViewPage would be something like this:

using System;
using System.Collections.Generic;
using System.Web;
using System.Web.Mvc;
using System.Web.WebPages;

public partial class _Areas_Admin_Views_Shared_AdminGrid_cshtml<T> : System.Web.Mvc.WebViewPage<IEnumerable<T>>  where T: EntityBase
{
	public _Areas_Admin_Views_Shared_AdminGrid_cshtml()
	{
	}
	public override void Execute()
	{
		WriteLiteral("<table>");

		foreach (var item in Model)
		{
   		        WriteLiteral("<tr>");
			WriteLiteral("<td>");
			Write(item.Id);
			WriteLiteral("</td>");
			WriteLiteral("<td>");
			Write(item.CreatedDate);
			WriteLiteral("</td>");
   		        WriteLiteral("</tr>");
		}
		
		WriteLiteral("</table");
	}
}

The important thing in this class is that despite classes that are generated for other Views it is still a generic class with a generic T parameter.

This class cannot be instantiated unless we provide its generic parameter when instantiating it.

This is why we need a generic View<T>() method in our controllers and specify its generic parameter, so that the ViewEngine (and ViewPageActivator at the end) are able to instantiate the view from its generated class.

@mansoor-omrani
Copy link
Author

I was thinking that what happens if our view has a generic model with multiple generic types.

@model MyModel<T, U> // ...

Clearly, we can no longer use the generic View<()> method in Controller class.

I think the correct way to implement this feature is to add overload methods with a Type[] array argument to which we can pass an array of types that are required when instiating from the class of the generic Views. This way we can support a model with any number of generic type parameters.

This also applies to IViewPageActivator that is responsible for instantiating views. In fact, we need to add another Create() method to the interface with a Type[] parameter.

public interface IViewPageActivator
{
   object Create(ControllerContext controllerContext, Type type);
   object Create(ControllerContext controllerContext, Type type, Type[] genericTypes);
}

The following could be the implementation of this new method in the DefaultViewPageActivator:

public object Create(ControllerContext controllerContext, Type type, Type[] genericTypes)
{
	try
	{
		var genericType = type.MakeGenericType(genericTypes);
		return Activator.CreateInstance(genericType);
	}
	catch (MissingMethodException exception)
	{
		// ...
	}
}

@ashleybaldock
Copy link

+1 to this, I just spent a little while puzzling out that you can't do this (trying to setup a partial view to render pagination controls for various pages with different models, taking a PaginatedList). You can do this without explicitly specifying the @model type but it would be nice to have support for @model to check it.

@kramffud
Copy link

kramffud commented Sep 1, 2018

From a Milestone to the Backlog on May 14? Where is this? I can't believe I haven't seen what I consider to be an overwhelming Use Case for this, albeit one at which I am fully aware of the countless eyes that will roll. Send in a generic model collection, reflect on its items' property names for column headers and then iterate through the list to display those items' values. Reflection, slow, I get it. But how much dev time would be saved for simple key/value lookup lists, like Country? Let the eye rolling begin.

@RemiGaudin
Copy link

+1
Generics is not a cutting edge feature anymore, this is a must have in C#, and I cannot imagine an enterprise grade C# project without generics now. I'm really surprised that we don't hear much more noises about it.
Maybe for some reasons implementing this missing feature on Razor views is really hard for the MS team?

@mansoor-omrani
Copy link
Author

Actually I made my hands dirty a little to implement this feature.

As far as the codes were in MVC source code I handled the changes and applied appropriate changes to provide open generics feature in views.

But when I went deeper I reached to Razor Engine source code and there I stuck. The Razor Engine source code was very sophisticated. I couldn't invest more time on that. So, I dropped the case.

Clearly the folks in Razor Engine project can apply the needed changes faster.

@jzabroski
Copy link

@mansoor-omrani Can you upload your progress to GitHub? And point out where in the Razor Engine it's complicated?

@mansoor-omrani
Copy link
Author

Yes. Sure. I'll upload the changes I applied.

@mansoor-omrani
Copy link
Author

mansoor-omrani commented Sep 29, 2018

I tried to push the changes. But it failed. This is the messages I received.

Authentication failed. You may not have permission to access the repository or the repository may have been archived. Open options and verify that you're signed in with an account that has permission to access this repository.

By the way, I was working on "https://github.com/aspnet/AspNetWebStack" repository.

@mansoor-omrani
Copy link
Author

I opened a new issue in the aforementioned branch.

aspnet/AspNetWebStack#200

@mansoor-omrani
Copy link
Author

@mansoor-omrani Can you upload your progress to GitHub? And point out where in the Razor Engine it's complicated?

I created a pull request for code changes in my fork. I was working on legacy MVC.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 13, 2018
@aspnet-hello aspnet-hello added this to the Backlog milestone Dec 13, 2018
@aspnet-hello aspnet-hello added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Dec 13, 2018
@realtorfico
Copy link

I strongly support the need for passing a generic item into the model of a view. This is very much needed in all future versions of Asp.Net Core.

@pranavkm pranavkm added the c label Aug 21, 2019
@mkArtakMSFT
Copy link
Member

This is not something we plan to do.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

9 participants