many to many add Identity user-CodePudding

Hi I'm trying to add an ApplicationUser (a derived class from IdentityUser) in my entity Entreprise with my Asp.Net MVC project. They have a many to many relationship between them and it seems to works since the table ApplicationUserEntreprise in my database gets updated. But when I try to do something like user.Entreprisew or entreprise.Users it always return me null

Here is my method addUser of my EntrepriseRepo where I try to do the link between my entities

public void AddUser(ApplicationUser user , Entreprise entreprise) {
            Entreprise entrepriseFromDb = base.FirstOrDefaultAsync(u => u.Id == entreprise.Id).Result;
            if (entrepriseFromDb.ApplicationUsers == null)
            {             
                entrepriseFromDb.ApplicationUsers = new List<ApplicationUser>();
                entrepriseFromDb.ApplicationUsers.Add(user);
            }
            else {
                entrepriseFromDb.ApplicationUsers.Add(user);
            }

            if (user.Entreprises == null) {
                user.Entreprises = new List<Entreprise>();
                user.Entreprises.Add(entrepriseFromDb);

            }
            else
            {
                user.Entreprises.Add(entrepriseFromDb);
            }
            
        }

My controller

public async Task AddUser(int id, string UserId)
        {
            Entreprise entreprise = await _services.Configuration.GetEntreprise(id);
            ApplicationUser user = await _userManager.FindByIdAsync(UserId);
            _services.Configuration.AddUserToEntrepriseAsync(user, entreprise);
            await Details(id);
        }

My service

public void AddUserToEntrepriseAsync(ApplicationUser user, Entreprise entreprise) {
            _uow.Entreprises.AddUser(user, entreprise);
            _uow.Save();

        }

Here is my SQL db

and here are my classes

ApplicationUser

 public class ApplicationUser : IdentityUser
  {
        public ICollection<Entreprise> Entreprises { get; set; }

    }

Entreprise

  public class Entreprise
    {
        public int Id { get; set; }
        public string Nom { get; set; }
        public string LogoURL { get; set; }
        public List<Groupe> Groupes { get; set; }
        public List<Periode> Periodes { get; set; }
        [NotMapped]
        public int GroupesCount { get; set; }
        [NotMapped]
        public int EquipementsCount { get; set; }
        [NotMapped]
        public int PeriodesCount { get; set; }
        
        public ICollection<ApplicationUser> ApplicationUsers{ get; set; }
    }

Is there any way I can get get my list of user by doing entreprise.User and my entreprise by doing user.Entreprises ?

CodePudding user response：

There are a couple issues with your code.

Firstly, don't use FirstOrDefault() as a crutch method to retrieve an entity. If you expect one entity that should be there, use Single(). If you expect one entity or possibly no entity, use SingleOrDefault(). If you expect multiple entities or possibly nothing, and only care about the first one, use FirstOrDefault(), however also only in combination with an OrderBy*() method.

Next, do not use .Result on async calls. Either use awaited async methods all the way through, or use a synchronous method.

Passing potentially detached entities around can lead to a lot of confusion in an application as you may get an entity that is detached, one that is tracked by the current DbContext, or one that is tracked by another DbContext. All 3 scenarios can lead to different exceptions. Passing entities around is also an overhead that isn't needed when you are fetching the entity from a DbContext anyways. I would recommend using a ViewModel/DTO for the ApplicationUser information that you might use to create a new user if necessary, and just passing the Enterprise ID. We also will be interested in any users associated with this enterprise so we should eager-load those with .Include():

public void AddUser(ApplicationUserViewModel userVm , int entrepriseId) 
{
    Entreprise entreprise = base
        .Include(e => e.ApplicationUsers)
        .Single(e => e.Id == entrepriseId);
    // ...

Next, avoid initializing collection references in code, instead initialize the properties in your entity. The code is "ok" but re-initializing a collection in a tracked entity can lead to problems, and this just requires extra conditional logic scattered around to check for empty sets before adding.

public class Enterprise
{
    // ...
    public virtual ICollection<ApplicationUser> ApplicationUsers { get; internal set; } = new List<ApplicationUser>();
}

The same would apply on the ApplicationUser initializing it's collection of Enterprises.

Before adding a user our code should assert that the user isn't already associated, as that could cause exceptions as well. You also should consider whether you are associating an existing user record, or creating a new user in this process. Ideally these actions should be atomic so creating a brand new user is a separate operation to associating a user to an enterprise. The danger with your code is where a DbContext might be tracking an

So instead of:

if (entrepriseFromDb.ApplicationUsers == null)
{             
    entrepriseFromDb.ApplicationUsers = new List<ApplicationUser>();
    entrepriseFromDb.ApplicationUsers.Add(user);
}
else 
{
    entrepriseFromDb.ApplicationUsers.Add(user);
}

if (user.Entreprises == null) {
    user.Entreprises = new List<Entreprise>();
    user.Entreprises.Add(entrepriseFromDb);
}
else
{
    user.Entreprises.Add(entrepriseFromDb);
}

... we do something more like:

if (userVm.Id != 0 && enterprise.ApplicationUsers.Any(u => u.Id == userVm.Id))
    return; // user is already associated.

ApplicationUser user = userVm.Id == 0 
     ? createUser(userVm)
     : base.ApplicationUsers.Single(u => u.Id == userVm.Id);

enterprise.ApplicationUsers.Add(user);
base.SaveChanges();  // Or saved further up the call stack/unit of work...

EF will manage bi-directional relationships automatically as we associate entities that are configured with relationships via navigation properties. We want to avoid doing anything like checking and assuming we need to initialize collections since that could easily lead to bugs if we mess something up on a bad assumption. The above code assumes we might get a new user or what should be an existing one. If we get an existing user we check whether the enterprise already has it and exit if it does. Otherwise we prepare to add it. If the Id is not present we create a new ApplicationUser using the details from the view model. This could be done using Automapper and .Map<ApplicationUser>(). Otherwise we fetch the User reference from the DbContext. We use Single() again here as a sanity check to ensure the provided user details are valid. If the caller passes a UserId of "101" and there is no record with that Id then the application should treat this as an exception as there is either something really wrong with the data state or it is being tampered with.

This would be simplified further with more atomic methods where a CreateUser action is separate to an AddUserToEnterprise action which could expect to always get an existing User reference. That way AddUserToEnterprise would just need to pass an EnterpriseId and a UserId rather than have conditional logic to handle new vs. existing users.