Binding Complex Entities inside complex Entities to Requests

Viewed 33

I am at the moment not at my work Laptop so I will only roughly write my Code.

I have created the "Create" View with Visual Studio. In order to create something the User has to choose from some Dropdowns which are filled from Enums and from some Tables. I now want to get the choosen data from the Request, but I don´t know how to bind them to Models, since all Tutorials are only about "simple" Models.

I have Entities(reduced Code):

 class Dept{
 int DeptId{get; set;}
 string Name;
 User Leader;
 ICollection<User> Users;
 Category Cat;
 User Substitute;
}

 class User{
 int UserId;
 string Name;
}

 enum Category {
 Bla=0, Somethine=1,
}

In my Create.cshtml I am defining Dept as the Model.

class DeptController(){
   public DeptController(){

   }
   [HttpGet]
   public Create(){
     ViewBag.Users = new SelectList(userRepository.GetAll().toString());
      //I do this with several more Entities.
   
     return View();
    }
   [HttpPost]
   public Create([Bind("//nearly everything except id")]Dept dep){
   if(ModelState.isValid){}
   }
}

Create.cshtml:

@model Dept
<div class="col">
 <input asp-for="Leader">
<select asp-for="Leader" asp-items="ViewBag.Users"></select>
</div>
<div class="col">
 <input asp-for="Substitute">
<select asp-for="Substitute" asp-items="ViewBag.Users"></select>
</div>
<div class=col">
       <input asp-for="Category">
<select asp-for="Category" asp-items=Html.Enumlist(Category)></select>
</div>

I have now three specific problems:

1.) I am unable to get the data for the "Subentities" of "Dept". When I am selecting an Substitute or Leader, it will not be bound to the Dept Entity. However, I can get it with the Parameter (string Leader) in the methodsignatur.

But, when I do it like that I would have to add like ten Parameters to the Signatur And then I have to call their Repositories to actually load the Entity and then add them to the returned "Dept" to save this new Entity. Or would it be possible to add the "virtual UserId" to the Dept Entity so that I only have to define the Id on an User instead of the full Entity? ... There must be an easier way, right?

2.) When I click on "Submit", but the Validation is not correct it will not save the Values which I have selected from the Viewbag and Enums. Quite annoying.

3.) Is there an more efficient way to load the Users? The Create Method is quite often used, but new Users are not quite common... It seems counterintuitive to always load everything.

1 Answers

You may need to refine your question with code examples of each specific code issues you are encountering, but from your description it looks like much of what you are having issues with are limitations of the Repository pattern your project has implemented, and eager loading.

For example's sake let's remove the Repository out of the equation and explain this using just a DbContext:

If we are loading a Department to send data to a view, a department has a collection of users associated to it where one user is nominated as a Leader and another is nominated as a Substitute. My guess is from your description you're loading the Dept, then making separate calls to load the users. With EF you would normally instead want to ensure that this related data is eager loaded. (Loading associations with the main query)

using (var context = new AppDbContext())
{
    var dept = context.Departments
        .Include(x => x.Users)
        .Include(x => x.Leader)
        .Include(x => x.Substitute)
        .Single(x => x.DepartmentId == departmentId);

   // ...
}

Include is used to ensure that these associated entities are fetched (if present). This is very important when we want to fetch an entity to update, and check/update it's associations.

The next problem that I would caution to consider changing is the notion of passing Entities to a view as a model. This is generally a poor practice that unfortunately ends up in a lot of examples out there, even official ones. It is a poor practice because it uses more memory, is prone to performance penalties for lazy loading, and can lead to sending more information to a view if used incorrectly, especially when combined with things like form posting. It is particularly dangerous/confusing when what you think is an Entity is passed back to the controller on a POST. It's not an entity, it is a block of data cast to an entity with whatever details the client-side view happened to have on hand. Best case it is an incomplete representation, worst case it has the potential to be filled with tampered with data intercepted by malicious actors using browser debugging tools or extensions. Instead, views should consume and return ViewModels (Just POCOs wrapping some data for/from the view) containing just the information they need. This cuts down on the size of data being passed around, and what is exposed about your data model.

So for instance if we are creating a new Department, have a CreateDepartmentViewModel, vs. an UpdateDepartmentViewModel, thinking about what data each of these scenarios needs. Note that if these viewModels contain a collection of UserViewModels for Users, when positing the CreateDepartmentViewModel back as part of the form POST, by default this collection will not be sent back unless you explicitly map out each available user using hidden properties etc. for the MVC client Form and/or Javascript to know how to re-construct the model to send back. This is the issue you are probably encountering now with sending the Entity to the view and expecting to somehow get that complete entity back. MVC is a great tool, but it does a lot behind the scenes which until you understand what it's doing, the results can seem rather confusing.

When we send a model to a view, this model goes to the view engine on the web server to build the resulting HTML based on the configured cshtml template you have configured. So in your cshtml you have a @model which is set to your Entity and in that template you can access all of the properties in that model. (So you do things like iterate over @Model.Users and populate a dropdown selection, etc.) Ultimately this generates HTML and Javascript that goes to the client browser. Any properties that are explicitly mapped to things like input controls etc. that resulting Form and Javascript will know about when figuring out what data to send back on a POST control. This is why often you need to add things like a Hidden input bound to @Model.Id if you don't have a text input or such for that ID otherwise the ID doesn't get sent back with the Form POST. The data that gets posted back are just name value pairs, so again MVC can do a bit of black box translation. You could have a POST method that just contains each field by name:

[HttpPost]
public ActionResult Create(int id, string name, int leaderUserId, ....)

... or you could have a POST method that contains a class representing the Model you passed to the view, which MVC will populate with the matching values:

[HttpPost]
public ActionResult Create(Department dept)

In the first case it should seem clear that what you are getting back is just data values that you will need to ultimately fetch entities to apply that data to. In the second case it's easy to assume that you're somehow still working with an Entity that you had already loaded from the DbContext when it was passed to the view. It isn't, they are the exact same thing, just wrapped. The temptation in the second case is that for a Create I should just be able to do:

using (var context = new AppDbContext())
{
    context.Departments.Add(department);
    context.SaveChanges();
}

and for something like an Update:

using (var context = new AppDbContext())
{
    context.Attach(department);
    context.Entry(department).State = EntityState.Modified;
    context.SaveChanges();
}

And for simple cases, that code works, and you will find examples out there doing just this. Except in the real world examples it doesn't work when you are dealing with associated entities (Department has user references, etc.) and it's really quite dangerous, and prone to unexpected behavior especially in the update scenario. (Trusting the data coming someone's browser)

Handling data sent from the client browser should always involve validating the values sent, loading the respective data from the database, populating entities to be created/updated, and saving. Libraries like Automapper can be set up to make this simpler, but for example a case where we want to update a Leader and Substitute user on a Department along with department details that can be changed:

An UpdateDepartmentViewModel would need something like:

[Serializable]
public class UpdateDepartmentViewModel
{
    public int DepartmentId { get; set; }
    public string Name { get; set; }
    public Category Category { get; set; }
    public ICollection<UserViewModel> Users { get; set; } = new List<UserViewModel>();        
    public int? LeaderUserId { get; set; }
    public int? SubstituteUserId { get; set; }
}

When we send this to the View engine, the Users collection will be used to render the drop downs for picking a Leader and Substitute. However, on a POST it will not be sent back but we should have mapped our form controls to bind to the LeaderUserId and SubstituteUserId if those were selected.

When we perform our update:

[HttpPost]
public ActionResult Update(DepartmentViewModel departmentVm)
{
    if (departmentVm == null) throw new ArgumentNullException(nameof(departmentVm));

    using (var context = new AppDbContext())
    {
       // TODO: Also consider permissions checks... Does the current user have access to update the Department nominated by the view model? Data could be tampered with.
       var department = context.Departments
            .Include(x => x.Leader)
            .Include(x => x.Substitute)
            .Single(x => x.DepartmentId == departmentVm.DepartmentId);

        //TODO: consider validating the department name i.e. for uniqueness, any rules around leader/substitute selection like can they be the same user ID? Even if validated client-side via JavaScript, always validate server-side as well.

        var leader = departmentVm.LeaderUserId.HasValue
            ? context.Users.Single(x => x.UserId == departmentVm.LeaderUserId.Value);
            : null;
        var substitute = departmentVm.SubstituteUserId.HasValue
            ? context.Users.Single(x => x.UserId == departmentVm.SubstituteUserId.Value);
            : null;

        department.Name = departmentVm.Name;
        department.Category = departmentVm.Category;
        department.Leader = leader;
        department.Substitute = substitute;
        context.SaveChanges();
   }   

   return Ok();
}

Note that when fetching our Department we eager load any existing Leader/Substutute relationship so we can update it. This example assumes we can also remove it if there might have been an existing relationship that the user was allowed to remove by clearing the selection.

This will be a bit to read & absorb, but even working with what you have already the important points:

  1. Data coming from a view, even if type cast as an Entity, is just data that could be supplied by the client. Don't trust it, and know that it is only as complete as the client browser could assemble. Using ViewModels can help avoid confusing data entities with data that is just type cast.

  2. When loading data to update, use Include to ensure any relationships that might be touched are loaded.

Lastly, regarding the Repository: If you are using something like a Generic Repository pattern (I.e. Repository<Department>) I would strongly recommend removing it as these patterns designed to try and abstract away the fact that you're working with a DbContext will lead to all kinds of problems and limitations. The Repository pattern does have a place, such as to enforce low-level data filtering assertions & rules, and enable unit testing, but it needs to be done in a way so as not to criple/confuse EF's capabilities around projection, pagination, etc. IMHO the Generic Repository is an anti-pattern when used with Entity Framework.

Related