Using static management class to control db transactions through an EF6

Question

I have the class below to manage my specific implementation of a membership system using entity framework 6.1

Is the use of static access correct in this case? as far as I understood that EF context is not thread-safe so I'm instantiating one with every call through the using statement. My second question, is the use of Boolean variable okay to signal failure in db transactions? an important point about this class is that it's going to be used quite frequently as the authentication wont rely on any session established with the user, it's also important to note that the users of this system will be quite limited.

Edit: I'm aware I can use the new identity system or any of the membership providers offered by ASP.NET but this custom provider is quite specific to a Windows authentication system and the use of any cookie-based system is not possible.

public static class AccountManager
{
    private enum Role { User, Comment, CommentControl, Sysadmin }

    public static bool IsUserInRole(string username, string role)
    {
        using (var ctx = new AccountsDbCtx())
        {
            var status = false;
            var foundAcc = ctx.Accounts.Where(x => x.Username == username);
            if (foundAcc.Any())
            {
                var s = foundAcc.SingleOrDefault();
                if (s != null)
                {
                    var exitingRole = s.Roles.Where(x => x.Name.Equals(role, StringComparison.InvariantCultureIgnoreCase));
                    if (exitingRole.Any())
                    {
                        status = true;
                    }
                }
            }
            return status;
        } 
    }

    public static bool IsUserCached(string username)
    {
        using (var ctx = new AccountsDbCtx())
        {
            var result = true;
            var foundAcc = ctx.Accounts.Where(x => x.Username == username);
            if (!foundAcc.Any())
            {
                result = false;
            }
            return result;
        }
    }

    public static bool CacheUser(string username)
    {
        using (var ctx = new AccountsDbCtx())
        {
            var status = false;

            try
            {
                var newAcc = new Account
                {
                    Active = true,
                    Username = username,
                };
                ctx.Accounts.Add(newAcc);
                ctx.SaveChanges();

                AssignRole(newAcc.Username, Role.User);
                status = true;
            }
            catch
            { }

            return status;
        }
    }

    public static bool AssignRoleComment(string username)
    {
        return AssignRole(username, Role.Comment);
    }

    public static bool AssignRoleCommentControl(string username)
    {
        return AssignRole(username, Role.CommentControl);
    }

    public static bool AssignRoleSysAdmin(string username)
    {
        return AssignRole(username, Role.Sysadmin);
    }

    private static bool AssignRole(string accountName, Role role)
    {
        using (var ctx = new AccountsDbCtx())
        {
            var completed = false;
            var roleToAssign = ctx.Roles.SingleOrDefault(x => x.Role1 == (int)role);

            try
            {
                var currentAccount = ctx.Accounts.SingleOrDefault(x => x.Username.Contains(accountName));

                if (currentAccount != null && roleToAssign != null) currentAccount.Roles.Add(roleToAssign);

                ctx.SaveChanges();

                completed = true;

            }
            catch
            {}

            return completed;
        }
    }
}

Answer 1

umm yes, you could do it that way.

as far as I understood that EF context is not thread-safe so I'm instantiating one with every call through the using statement

Where did you read this? I'm not denying it it's just that I have not read this before and I haven't experienced too many problems with using EF in the case of one instance per request.

My second question, is the use of Boolean variable ok to signal failure in db transactions

Maybe. But how are you going to know what actually went wrong? All that's going to happen is you know something went wrong but when it comes to diagnosing it you will be at a dead end because you have swallowed the exception (so to speak). Given that your exceptions seeming to be handling cases such as the database offline, or connection issues I think it would be best to bubble those up as that would be a core system fault and you would want to know more info.

Here's an alternative solution using a non-static class and sharing the Database context between methods.

NOTE: I would actually consider removing the default empty constructor and IDisposable if you were using an IOC framework to instantiate the context as that would be responsible for your instance lifetime. However I'm not sure how your Account Manager would be created.

public class AccountManager : IDisposable
{
    private enum Role { User, Comment, CommentControl, Sysadmin }
    private readonly AccountsDbCtx dbContext;

    public AccountManager(AccountsDbCtx dbContext)
    {
        this.dbContext = dbContext;
    }

    public AccountManager() : this(new AccountsDbCtx())
    {
    }

    public void Dispose() 
    {
        dbContext.Dispose();
    {

    public bool IsUserInRole(string username, string role)
    {
        var account = dbContext.Accounts.SingleOrDefault(x => x.Username == username);

        if(account == null)
        {
            return false;
        }

        var existingRole = account.Roles.Where(x => x.Name.Equals(role, StringComparison.InvariantCultureIgnoreCase));

        return existingRole.Any();      
    }

    public bool IsUserCached(string username)
    {
        return dbContext.Accounts.Any(x => x.Username == username); 
    }

    public void CacheUser(string username)
    {
        dbContext.Accounts.Add(new Account
        {
            Active = true,
            Username = username
        });

        dbContext.Accounts.Add(newAcc);
        dbContext.SaveChanges();

        AssignRoleUser(username);       
    }

    public bool AssignRoleUser(string username)
    {
        return AssignRole(username, Role.User);
    }

    public bool AssignRoleComment(string username)
    {
        return AssignRole(username, Role.Comment);
    }

    public bool AssignRoleCommentControl(string username)
    {
        return AssignRole(username, Role.CommentControl);
    }

    public bool AssignRoleSysAdmin(string username)
    {
        return AssignRole(username, Role.Sysadmin);
    }

    private bool AssignRole(string accountName, Role role)
    {
        // Why would a role not exist in our database?
        var roleToAssign = dbContext.Roles.Single(x => x.Role1 == (int)role);               
        var currentAccount = dbContext.Accounts.SingleOrDefault(x => x.Username.Contains(accountName));

        if (currentAccount == null)
            return false;

        currentAccount.Roles.Add(roleToAssign);
        dbContext.SaveChanges();

        return IsUserInRole(accountName, role); 
    }
}