views:

63

answers:

2
public partial class MembershipModule : BaseEntity<MembershipModule>
{
    /// <summary>
    /// Returns wheter a module is accessible
    /// </summary>
    public static bool IsAccessible(MembershipModule module)
    {
        // In absence of a module, no security applies to the page
        if(module == null)
            return true;

        return module.IsAccessible();
    }

    /// <summary>
    /// Returns whether the module is accessible
    /// </summary>
    /// <returns></returns>
    public bool IsAccessible()
    {
        // Skip unnecessary querying
        if(!MembershipUser.Connected)
            return this.Enabled && this.OfflineAccess;

        Guid accessGuid = MembershipAction.AccessGuid;

        // ModuleActions for this MembershipModule and the Access Action
        IQueryable<MembershipModuleAction> accessMA =
            from ma in LinqUtil.Context.MembershipModuleActions
            where ma.ModuleId.Equals(this.Id) && ma.ActionId.Equals(accessGuid)
            select ma;

        // RolePrivileges that grant access on this MembershipModule for the Current MembershipUser
        IQueryable<bool> accessRP =
            from rp in LinqUtil.Context.MembershipRolePrivileges
            where accessMA.Contains(rp.MembershipModuleAction)
            select rp.MembershipRole.MembershipUsers.Contains(MembershipUser.Current);

        return this.Enabled && (this.OfflineAccess || accessRP.ToList().FirstOrDefault());
    }

    /// <summary>
    /// Returns all accessible modules that can be accessed by the logged user
    /// </summary>
    /// <returns></returns>
    public static IEnumerable<MembershipModule> GetAccesible()
    {
        // Skip unnecessary querying
        if(!MembershipUser.Connected)
            return LinqUtil.Context.MembershipModules.Where(m => m.Enabled && m.OfflineAccess).ToList();

        Guid accessGuid = MembershipAction.AccessGuid;

        // ModuleActions for any MembershipModule with the Access Action
        IQueryable<MembershipModuleAction> accessMA =
            from ma in LinqUtil.Context.MembershipModuleActions
            where LinqUtil.Context.MembershipModules.Any(m => m.Enabled && m.Id.Equals(ma.ModuleId)) && ma.ActionId.Equals(accessGuid)
            select ma;

        // RolePrivileges that grant access on those MembershipModule for the Current MembershipUser
        IQueryable<MembershipRolePrivilege> accessRP =
            from rp in LinqUtil.Context.MembershipRolePrivileges
            where accessMA.Any(ma => rp.MembershipModuleAction.Id.Equals(ma.Id)) && rp.MembershipRole.MembershipUsers.Any(u => u.Id.Equals(MembershipUser.Current.Id))
            select rp;

        // Accessible modules
        var modules =
            from m in LinqUtil.Context.MembershipModules
            where accessMA.Any(ma => ma.MembershipModule.Id.Equals(m.Id)) && accessRP.Any(rp => rp.MembershipModuleAction.ModuleId.Equals(m.Id))
            select m;

        return modules.ToList();
    }

    /// <summary>
    /// Menu Items that can be displayed on the current web page
    /// </summary>
    public static IEnumerable<MembershipModule> GetMenuItems(string uriPrefix)
    {
        IEnumerable<MembershipModule> list = GetAccesible();

        return list.Where(m => m.UriPrefix.Equals(uriPrefix) && m.MenuItem).OrderBy(m => m.Position);
    }
}

This currently works, but for some reason I can't help but to think the code looks ugly (particularly the two very similar static and instance functions that get me the accessible pages

Any thoughts on how to refactor this to look more appealing?

Bonus question:

Guid accessGuid = MembershipAction.AccessGuid;

Without using that line, and just calling MembershipAction.AccessGuid in my query, I get an error telling me something obscure:

Class member MembershipAction.AccessGuid is unmapped.

The tricky parts comes here: in the static GetAccessible() function this doesn't happen. I can just add MembershipAction.AccessGuid to the query, but it's in the instance IsAccessible() function that I get this problem.

For the record, here's MembershipAction:

public partial class MembershipAction : BaseEntity<MembershipAction>
{
    public const string AccessPrivilege = "Access";

    private static Guid accessGuid = Guid.Empty;
    public static Guid AccessGuid
    {
        get
        {
            if(accessGuid == Guid.Empty)
            {
                IQueryable<Guid> query = from a in LinqUtil.Context.MembershipActions
                                         where a.Name.Equals(AccessPrivilege)
                                         select a.Id;

                accessGuid = query.ToList().FirstOrDefault();
            }

            return accessGuid;
        }
    }
}
A: 

Isn't it simply:

public static IEnumerable<MembershipModule> GetAccesible()
{
    // Skip unnecessary querying
    if(!MembershipUser.Connected)
        return LinqUtil.Context.MembershipModules.Where(m => m.Enabled && m.OfflineAccess).ToList();

    // Accessible modules
    var modules =
        from m in LinqUtil.Context.MembershipModules
        where m.IsAccessible()
        select m;

    return modules.ToList();
}
GôTô
A: 

Mentioning that this is personal opinion and we all have them here goes:

  1. Ditch the use of lambda expressions inside the queries. Pick a style and use it dude.
  2. Get rid of the ampersand in the where clause. You can get the same thing with another line starting with where.
  3. Don't be afraid to query another query. If it makes it easier to understand, there is no reason to not do it. The compiler will combine them into a single query for you.
  4. Many will point out that static methods often point to code smell. Why do you need both static and instance methods? Perhaps some clarity in the class is in order.
Kirk