EF6 Collection was modified; enumeration operation may not execute

Question

I need to add adin users into a table (and child table) where they don't already exist.

I have the following code which throws the error: "Collection was modified; enumeration operation may not execute." on the line "_userPrefsContext.UserRecs.Add(userRec);"

var allUserRecs = _userPrefsContext.UserRecs.ToList();

        foreach (string adminUser in adminUsers.Where(x => x.Length > 0))
        {
            domain = adminUser.Split('\\')[0];
            login = adminUser.Split('\\')[1];

            var userRec = new Domain.UserPrefs.BamUser()
            {
                AdLogonDomain = domain,
                AdLogonId = login,
                UserViews = colUserView
            };

            if (allUserRecs.Where(x => x.AdLogonDomain == domain
                                    && x.AdLogonId == login).Count() == 0)
            {
                //don't currently have a user rec for this login so create one
                _userPrefsContext.UserRecs.Add(userRec);
            }
        }
        _userPrefsContext.SaveChanges();

I have seen similar posts on SO but in each case they seem to be iterating over the collection into which they are adding. In my case, I'm just trying to iterate over a list of strings.

Where did I go wrong?

Now edited to the following (still the same error):

            List<string> adminUsers = WebConfigurationManager.AppSettings["AdminUsers"].Split(';').ToList();
        adminUsers = adminUsers.Where(x => x.Length > 0).ToList();

        string domain = "", login = "";
        string userViewString = Helpers.ViewNames.UserView.ToString();

        int viewId = _userPrefsContext.ViewRecs.Where(x => x.Name == userViewString).Select(x => x.BamViewId).Single();

        Domain.UserPrefs.UserView userView = new Domain.UserPrefs.UserView()
        {
            BamViewId = viewId
        };

        List<Domain.UserPrefs.UserView> colUserView = new List<Domain.UserPrefs.UserView>();
        colUserView.Add(userView);

        var allUserRecs = _userPrefsContext.UserRecs.ToList();
        string[] users = new string[adminUsers.Count()];
        int y = 0;
        foreach(string user in adminUsers)
        {
            users[y] = user;
            y++;
        }

        int total = adminUsers.Count;
        for (var i = 0; i < total; i++)
        {

            domain = users[i].Split('\\')[0];
            login = users[i].Split('\\')[1];

            if (allUserRecs.Where(x => x.AdLogonDomain == domain
                                    && x.AdLogonId == login).Count() == 0)
            {
                //don't currently have a user rec for this login so create one
                _userPrefsContext.UserRecs.Add(new Domain.UserPrefs.BamUser()
                {
                    AdLogonDomain = domain,
                    AdLogonId = login,
                    UserViews = colUserView
                });
            }
        }
        _userPrefsContext.SaveChanges();

Answer 1

UserViews is defined as a domain entity as follows:

public virtual ICollection<UserView> UserViews { get; set; }

To make this work I changed from

var userRec = new Domain.UserPrefs.BamUser()
            {
                AdLogonDomain = domain,
                AdLogonId = login,
                UserViews = colUserView
            };

tovar userRec = new Domain.UserPrefs.BamUser() { AdLogonDomain = domain, AdLogonId = login, UserViews = colUserView.ToList() };

To be honest, I don't know why the .ToList() is needed - if anyone does then I think a explanation may be helpful to many!

Answer 2

Just for suggestion its better your are using any replace of count. any working much faster than count. use this code

 if (!allUserRecs.any(x => x.AdLogonDomain == domain                                   && x.AdLogonId == login))
            {
                //don't currently have a user rec for this login so create one
                _userPrefsContext.UserRecs.Add(new Domain.UserPrefs.BamUser()
                {
                    AdLogonDomain = domain,
                    AdLogonId = login,
                    UserViews = colUserView
                });
            }

Replace if

if (allUserRecs.Where(x => x.AdLogonDomain == domain
                                    && x.AdLogonId == login).Count() == 0)
            {
                //don't currently have a user rec for this login so create one
                _userPrefsContext.UserRecs.Add(new Domain.UserPrefs.BamUser()
                {
                    AdLogonDomain = domain,
                    AdLogonId = login,
                    UserViews = colUserView
                });
            }

Answer 3

The issue is you have a list that you are then running a where clause in the foreach. This means it has to requery the list on each run but if you have modified the list during the run the query may be wrong now.

This would be a better solution:

var allUserRecs = _userPrefsContext.UserRecs.Where(x => x.Length > 0).ToList();

    foreach (string adminUser in adminUsers)
    {
        domain = adminUser.Split('\\')[0];
        login = adminUser.Split('\\')[1];

        var userRec = new Domain.UserPrefs.BamUser()
        {
            AdLogonDomain = domain,
            AdLogonId = login,
            UserViews = colUserView
        };

        if (allUserRecs.Where(x => x.AdLogonDomain == domain
                                && x.AdLogonId == login).Count() == 0)
        {
            //don't currently have a user rec for this login so create one
            _userPrefsContext.UserRecs.Add(userRec);
        }
    }
    _userPrefsContext.SaveChanges();