Sorry, future maintainer...


  • Impossible Mission Players - A

    Yay Entity Framework?

            //Create a new user if they don't exist yet, otherwise, return the user
            private Permits getUser(string username)
            {
                Permits userDetails = db.Permits.Find(username);
                //Stop cachine the details in case it's changed elsewhere (i.e. admin interface)
                //Bad, since it effectively doubles the db requests, but the admin interface ain't talking to the Role Provider...
                if (userDetails != null) db.Entry(userDetails).Reload();
                //Default condition if user doesn't exist is to use the default
                if (userDetails==null)
                {
                    userDetails = new Permits();
                    userDetails.WIN_LOGIN = username;
                    userDetails.MODIFIED_ON = userDetails.CREATED_ON = DateTime.Now;
                    userDetails.MODIFIED_BY = userDetails.CREATED_BY = "System";
    
                    //Note that by default everyone is disabled. Should probably make a site setting for that...
                    db.Entry(userDetails).State = System.Data.Entity.EntityState.Added;
                    try
                    {
                        db.SaveChanges();
                    }
                    catch (Exception)
                    {
                        //swallow the exception :(
                    }
    
    
                }
                return userDetails;
    
            }
    

    Part of a Custom Role Provider for an ASP .Net MVC site with Entity Framework.



  • @Tsaukpaetra said:

    Create a new user if they don't exist yet, otherwise, return the user

    This code is already doomed from the start.



  • Why do you do a get of the user, and then a reload of that same object? You don't store your DbContext somewhere, do you?

    If you do, stop doing that because a DbContext is absolutely not thread-safe and re-use will cause all kinds of funky issues.

    edit: and I agree with what @LB_ wrote. Creating a new user if one doesn't exist is not really needed, I think? Why store users that you don't know anything about except their login name?



  • Not if it's just some plain e-voting site. :stuck_out_tongue:


  • Impossible Mission Players - A

    @AlexMedia said:

    store your DbContext somewhere, do you?

    I don't think so? Not intentionally? At least, it's not disposing very well in any case, for whatever reason it seems to cache entities unusually, and this means modifications in another action (AKA Admin screen) don't reflect until a site recycle. @AlexMedia said:

    Creating a new user if one doesn't exist is not really needed, I think? Why store users that you don't know anything about except their login name?

    It piggyback off the active directory authentication. Normally we have an Access Monitor that would create the account, but that's not set up ATM, so the interim is to dummy-fi the account and then they just enable it when needed.
    Apparently it's supposed to provide traceability or something, NFC how because we already store the domain id in the submissions....
    The main purpose though is to provide priviledge flags (or permissions), so it's no big deal that we're creating dumb entries for most users. They'll get upgraded when necessary.



  • @Tsaukpaetra said:

    I don't think so? Not intentionally?
    If you are creating your DbContext in the constructor of your role provider, then you most likely are. The ctor is called only when your web application is being initialised and is then reused.

    Just create a new instance of your DbContext in the
    getUser method (or where you're calling it from) and dispose of it when needed. This will stop the unnecessary reloads and caching that goes wrong.

    At least, it's not disposing very well in any case, for whatever reason it seems to cache entities unusually, and this means modifications in another action (AKA Admin screen) don't reflect until a site recycle.

    EF is doing exactly what it is designed to do: if an object has been retrieved before, EF will not update the object when it is retrieved a second time in the same DbContext. Hence the reload.

    If you instantiate a new DbContext everything will go fine.


  • Impossible Mission Players - A

    @AlexMedia said:

    Just create a new instance of your DbContext in the getUser method (or where you're calling it from) and dispose of it when needed.

    Ah. I was just following the template it provides in the default "Create Controller" command, which makes a context as a member at the class level (I think I'm describing it correctly), which (translated/obfuscated) looks like so:

        public class CustomRoleProvider : RoleProvider
        {
            //Hold a context to the app db?
            private appContainer db = new appContainer();
    
            private Permits getUser(string username)
            { 
            //blah code from OP
            }
    
    

    Luckily, I think I should be fine moving that declaration into getUser (as mentioned), but just to be sure I understand properly, this doesn't cause it to close and re-open the database connection each time, right?



  • @Tsaukpaetra said:

    but just to be sure I understand properly, this doesn't cause it to close and re-open the database connection each time, right?

    It will. If you don't want that, then you'll have to either store the roles in cache, or figure out a way to have just one DbContext open during the lifetime of your request.


  • Impossible Mission Players - A

    @AlexMedia said:

    store the roles in cache

    This would be fine (and in fact, would be preferred), except that I can't easily sync two different context's caches between two controllers like that.

    I believe the best solution is to implement the other functions of the role provider (the ones that add/remove roles) and have the admin screen call those functions instead, essentially moving the whole process more towards the way the MVC expects it to (just inside a private class instead of exposed on the MVC model directly).
    Fun thing is, this will probably involve some fun bits with reflection (:giggity:), but at least I have some of the things I need anyway:

            // Translate the Permits object into a role list
            public static string[] TranslateToCustomRoles(Permits userDetails)
            {
                //Note that this absolutely depends on userDetails NOT being null!
                List<String> userRoles = new List<string>();
                List<System.Reflection.PropertyInfo> properties = userDetails.GetType().GetProperties().Where(x=>x.Name.StartsWith("is")).ToList();
                foreach (var p in properties)
                {
                    bool val = Convert.ToBoolean(p.GetValue(userDetails, null));
                    //If user has this role enabled, add it to the roles list
                    if (val)    userRoles.Add(p.Name.Substring(2));
                }
                return userRoles.ToArray();
            }
    

  • Impossible Mission Players - A

    @Tsaukpaetra said:

    I believe the best solution is to implement the other functions of the role provider (the ones that add/remove roles) and have the admin screen call those functions instead

    I think I may have come up with a simpler bodge even than this!
    Since we only really need to refresh the particular user if their permissions have changed, we only need to tell the role provider to do this when needed.

    I thus commandeered the AddUsersToRoles function (which is apparently only called when the application wants to do it) to have the Entry().Reload() call in it. Thus, when I've changed it elsewhere, I call the AddUserToRole function with a special role name and everything is a little bit more convoluted in the world! Woot!

    Edit: I might still eventually implement the rest of the function so it actually works as expected, but for now, this is all I should (theoretically) need...



  • That truly is awful :facepalm:

    Do you use a IoC container (such as Unity) in your solution? If so, can't you use that to your advantage to get an instance to a DbContext that is still open?


  • Impossible Mission Players - A

    @AlexMedia said:

    Do you use a IoC container (such as Unity) in your solution?

    Um... Not precisely? This is

    @Tsaukpaetra said:

    an ASP .Net MVC site with Entity Framework.
    perhaps I should have clarified it's a web site?

    I suppose I could grab the Role Provider and cast it as the custom Role Provider I wrote and use it that way, but that almost seems like more work than that even.



  • You can still use Inversion of Control with ASP.NET MVC. In fact, the framework supports it by providing you with the DependendyResolver class. :)


  • Impossible Mission Players - A

    @AlexMedia said:

    DependendyResolver class.

    Oh shit. Is this the same thing that prompted my predecessor to Factorize All The Things and Interface All The Factories?!?!

    I'm not sure it's worth the effort, it's already excruciatingly painful to figure out where stuff is coming from, I'd rather not do that to a basic class like the Role Provider.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.