Let me start by giving some code examples that are very common, very useful and used A LOT throughout the (web) application:
int? contactId = InfrastructureHelper.User.GetContactId()
int[] departmentIds = InfrastructureHelper.GetAuhthorizedDepartmentIds()
bool allowed = InfrastructureHelper.User.IsAllowedInPortal()
InfrastructureHelper.User.GetUserLanguage();
Just a few of some most used methods I call during the application life cycle.
While this all seems very cool and useful, it creates problem when used with Entity Framework and in a heavily async web application. Before in .NET Framework and with everything being synchronous (oh boy, those times), this was no problem. But now I am starting to see the problems and I need a solution.
Solution desires:
- Static if possible, to avoid injecting a class everywhere
- It caches data for doing (additional) authorization checks (departments, controller-based authorization on top of the default role based authorization).
- Thread-safe, because that's of course the problem now, I get the exception below here:
System.InvalidOperationException: 'A second operation started on this context before a previous operation completed. This is usually caused by different threads using the same instance of DbContext, however instance members are not guaranteed to be thread safe. This could also be caused by a nested query being evaluated on the client, if this is the case rewrite the query avoiding nested invocations.'
So I understand how to use DI and the different possibilities, I have seen other questions that ask a similar question but the solution is working around the fact that I need a instance of Entity Framework injected.
InfrastructureHelper holds a bunch of static members and 2 important interfaces, ICurrentSession and ICurrentUser. These interfaces are implemented differently per web application within the solution. The implementations are not static but do contain static members to cache some more data. ICurrentSession is not the problem here, that implementation of just injects IHttpContextAccessor. But CurrentPortalUser that implements ICurrentUser does inject repositories, let's give some code to read:
public class CurrentPortalUser : ICurrentUser
{
private readonly IUserRepository _userRepository;
private readonly IControllerActionRepository _controllerActionRepository;
private readonly IDepartmentRepository _departmentRepository;
private static List<DepartmentModel> _allDepartments = new List<DepartmentModel>();
private static List<RoleModel> _allRoles = new List<RoleModel>();
private static List<ControllerActionModel> _allControllerActionRoles = new List<ControllerActionModel>();
/// <summary>
/// Simpler version of _allControllerActionRoles, Key: actionNamespace, Value: roleIds;
/// </summary>
private static Dictionary<string, string[]> _allControllerActionRoleIds = new Dictionary<string, string[]>();
/// <summary>
/// Key: CountryId, Value: DepartmentId
/// </summary>
private static Dictionary<int, int> _departmentCountryList = new Dictionary<int, int>();
private readonly IHttpContextAccessor _httpContextAccessor;
public CurrentPortalUser(IHttpContextAccessor httpContextAccessor, IUserRepository userRepository, IControllerActionRepository controllerActionRepository, IDepartmentRepository departmentRepository)
{
_httpContextAccessor = httpContextAccessor;
_userRepository = userRepository;
_controllerActionRepository = controllerActionRepository;
_departmentRepository = departmentRepository;
}
public void ReloadDepartmentAndRoleData()
{
_allDepartments = _departmentRepository.GetAll().ToList();
int[] departmentIds = _allDepartments.Select(x => x.DepartmentId).ToArray();
List<DepartmentCountryModel> allDepartmentCountries = _departmentRepository.GetDepartmentOperatingCountries(departmentIds);
_departmentCountryList = allWarehouseCountries.ToDictionary(x => x.CountryId, y => y.DepartmentId);
//load role and controller configuration, basically tells me what role is allowed to execute what controller action within the web app.
_allRoles = _userRepository.GetRoles().ToList();
_allControllerActionRoles = _controllerActionRepository.GetAllWithRoles().ToList();
_allControllerActionRoleIds = _allControllerActionRoles.ToDictionary(x => x.NameSpace.ToLower(), y => y.Roles.Select(z => z.RoleId).ToArray());
}
public List<DepartmentModel> GetAllowedDepartments()
{
if (!_allDepartments.Any())//store in memory, just like we are going to store all roles in memory
{
ReloadDepartmentAndRoleData();
}
if (!_allRoles.Any())
{
//load role and controller configuration
ReloadDepartmentAndRoleData();
}
if (IsInRole(RoleEnum.SuperAdmin))
{
return _allDepartments;
}
List<DepartmentModel> allowedDepartments = new List<DepartmentModel>();
foreach (DepartmentModel department in _allDepartments)
{
if (IsInRole(department.Name))
{
allowedDepartments.Add(department);
}
}
return allowedDepartments;
}
public string GetUserId()
{
if (_httpContextAccessor.HttpContext.User.Identity.IsAuthenticated)
{
return _httpContextAccessor.HttpContext.User.FindFirst(ClaimTypes.Name)?.Value;
}
return null;
}
public int? GetContactId()
{
int? contactId = null;
if (_httpContextAccessor.HttpContext.User.Identity.IsAuthenticated)
{
contactId = _userRepository.GetContactId(GetUserId());
}
return contactId;
}
}
There is more to it, it basically comes down to obtain some data, if it's cached then get it from cache, else get it from the database and cache it or put it into session.
So now comes the part where I show you awful code which I am a bit embarrassed for :).
public stattic class InfrastructureHelper
{
//..more static members that are heavily used here, but these one are a bit-less depended on a database, as long as data can be obtained from IcurrentUser
public static ICurrentUser User
{
get
{
return _container.GetInstance<ICurrentUser>();
}
}
//yup..that's static...
private static ISimpleContainer _container;
//this method is used via the Startup class, so only once
public static void Initialize(ISimpleContainer container)
{
_container = container;
}
}
//Since I don't have Asp.Net references in the project of InfrastructureHelper, I use a simple interface that of course does the same as the normal container
public interface ISimpleContainer
{
TService GetInstance<TService>() where TService : class;
}
Last but not least, the registration of ICurrentUser is Transient (within Startup class)
services.AddTransient<ICurrentSession, CurrentPortalSession>();
services.AddTransient<ICurrentUser, CurrentPortalUser>();
I get the reported exception at InfrastructureHelper.User.GetContactId(), because during or after login, it is used a lot at the same time. I could try to avoid that some requests are being executed at the same time or build a lock around that, but that just doesn't feel right.
Looking forward to your responses! Sorry of this is a very much of an anti-pattern, it's a piece of old code that has been easily copied to a new asp.net core environment without rechecking things because it's usability is so great :).
Update
So as per the comments, I was looking for a way to successfully inject the InfrastructureHelper everywhere. But when I try to inject this into every repository (make it part of the BaseRepository), then of course a circular dependency will be detected: ICurrentUser is depended on some repositories, and every repository is depended on InfrastructureHelper(and thus ICurrentUser).
