From c5e88ef69fdd1dc5b0fc4b35de08ecaff54f6013 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Thu, 4 Apr 2013 00:30:28 +0600 Subject: [PATCH] Fixes: #U4-2041, #U4-2040 - user cache is not invalidated when permissions change across LB environments, also streamlined how caching is handled in these classes which now use the standardized events way to do things. Fixes a performance issue and ensures that the cache is not invalidated for 'micro' operations during permission changes. --- .../Cache/CacheRefresherEventHandler.cs | 43 +++- .../Cache/DistributedCacheExtensions.cs | 5 + .../umbraco/users/UserPermissions.cs | 57 ++--- src/umbraco.cms/businesslogic/Permission.cs | 218 ++++++++++++++---- 4 files changed, 246 insertions(+), 77 deletions(-) diff --git a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs index d1ddd03827..806a7b2236 100644 --- a/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs +++ b/src/Umbraco.Web/Cache/CacheRefresherEventHandler.cs @@ -77,6 +77,12 @@ namespace Umbraco.Web.Cache User.Saving += UserSaving; User.Deleting += UserDeleting; + //Bind to permission events + + Permission.New += PermissionNew; + Permission.Updated += PermissionUpdated; + Permission.Deleted += PermissionDeleted; + //Bind to template events //NOTE: we need to bind to legacy and new API events currently: http://issues.umbraco.org/issue/U4-1979 @@ -105,7 +111,7 @@ namespace Umbraco.Web.Cache MediaService.Moving += MediaServiceMoving; MediaService.Trashing += MediaServiceTrashing; } - + #region Dictionary event handlers static void LocalizationServiceSavedDictionaryItem(ILocalizationService sender, Core.Events.SaveEventArgs e) @@ -301,6 +307,22 @@ namespace Umbraco.Web.Cache #endregion #region User event handlers + + static void PermissionDeleted(UserPermission sender, DeleteEventArgs e) + { + InvalidateCacheForPermissionsChange(sender); + } + + static void PermissionUpdated(UserPermission sender, SaveEventArgs e) + { + InvalidateCacheForPermissionsChange(sender); + } + + static void PermissionNew(UserPermission sender, NewEventArgs e) + { + InvalidateCacheForPermissionsChange(sender); + } + static void UserDeleting(User sender, System.EventArgs e) { DistributedCache.Instance.RemoveUserCache(sender.Id); @@ -309,7 +331,24 @@ namespace Umbraco.Web.Cache static void UserSaving(User sender, System.EventArgs e) { DistributedCache.Instance.RefreshUserCache(sender.Id); - } + } + + private static void InvalidateCacheForPermissionsChange(UserPermission sender) + { + if (sender.User != null) + { + DistributedCache.Instance.RefreshUserCache(sender.User.Id); + } + if (sender.UserId > -1) + { + DistributedCache.Instance.RefreshUserCache(sender.UserId); + } + if (sender.Nodes.Any()) + { + DistributedCache.Instance.RefreshAllUserCache(); + } + } + #endregion #region Template event handlers diff --git a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs index 1f72b2daed..228854b9b3 100644 --- a/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs +++ b/src/Umbraco.Web/Cache/DistributedCacheExtensions.cs @@ -22,6 +22,11 @@ namespace Umbraco.Web.Cache public static void RefreshUserCache(this DistributedCache dc, int userId) { dc.Refresh(new Guid(DistributedCache.UserCacheRefresherId), userId); + } + + public static void RefreshAllUserCache(this DistributedCache dc) + { + dc.RefreshAll(new Guid(DistributedCache.UserCacheRefresherId)); } #endregion diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/users/UserPermissions.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/users/UserPermissions.cs index 018e6d9ff5..a9ee42a082 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/users/UserPermissions.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/users/UserPermissions.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Text; using System.Xml; using System.IO; @@ -7,6 +8,7 @@ using System.Collections; using System.Web.UI.WebControls; using System.Data.SqlClient; using System.Data; +using Umbraco.Web.Security; using umbraco; using umbraco.BusinessLogic; using System.Web; @@ -43,39 +45,34 @@ namespace umbraco.cms.presentation.user /// /// /// - public void SaveNewPermissions(int[] nodeIDs, List actions, bool replaceChildren) + public void SaveNewPermissions(int[] nodeIDs, List actions, bool replaceChildren) { //ensure permissions that are permission assignable - List permissions = actions.FindAll( - delegate(IAction a) - { - return (a.CanBePermissionAssigned); - } - ); + var permissions = actions.FindAll(a => (a.CanBePermissionAssigned)); //ensure that only the nodes that the user has permissions to update are updated - List lstNoPermissions = new List(); - foreach (int nodeID in nodeIDs) + var lstNoPermissions = new List(); + foreach (var nodeId in nodeIDs) { - string nodeActions = UmbracoEnsuredPage.CurrentUser.GetPermissions(GetNodePath(nodeID)); - List lstActions = umbraco.BusinessLogic.Actions.Action.FromString(nodeActions); + var nodeActions = WebSecurity.CurrentUser.GetPermissions(GetNodePath(nodeId)); + var lstActions = BusinessLogic.Actions.Action.FromString(nodeActions); if (lstActions == null || !lstActions.Contains(ActionRights.Instance)) - lstNoPermissions.Add(nodeID); + lstNoPermissions.Add(nodeId); } //remove the nodes that the user doesn't have permission to update - List lstNodeIDs = new List(); + var lstNodeIDs = new List(); lstNodeIDs.AddRange(nodeIDs); - foreach (int noPermission in lstNoPermissions) + foreach (var noPermission in lstNoPermissions) lstNodeIDs.Remove(noPermission); nodeIDs = lstNodeIDs.ToArray(); //get the complete list of node ids that this change will affect - List allNodes = new List(); + var allNodes = new List(); if (replaceChildren) - foreach (int nodeID in nodeIDs) + foreach (var nodeId in nodeIDs) { - allNodes.Add(nodeID); - allNodes.AddRange(FindChildNodes(nodeID)); + allNodes.Add(nodeId); + allNodes.AddRange(FindChildNodes(nodeId)); } else allNodes.AddRange(nodeIDs); @@ -85,18 +82,18 @@ namespace umbraco.cms.presentation.user //if permissions are to be assigned, then assign them if (permissions.Count > 0) - foreach (umbraco.interfaces.IAction oPer in permissions) + { + foreach (var oPer in permissions) + { InsertPermissions(allNodes.ToArray(), oPer); + } + } else { //If there are NO permissions for this node, we need to assign the ActionNull permission otherwise //the node will inherit from it's parent. InsertPermissions(nodeIDs, ActionNull.Instance); - } - - //clear umbraco cache (this is the exact syntax umbraco uses... which should be a public method). - HttpRuntime.Cache.Remove(string.Format("UmbracoUser{0}", m_user.Id.ToString())); - //TODO:can also set a user property which will flush the cache! + } } /// @@ -154,18 +151,8 @@ namespace umbraco.cms.presentation.user private void InsertPermissions(IEnumerable nodeIDs, IAction permission) { - foreach (int i in nodeIDs) - InsertPermission(i, permission); + Permission.MakeNew(m_user, nodeIDs.Select(x => new CMSNode(x)), permission.Letter, true); } - - private void InsertPermission(int nodeId, IAction permission) - { - //create a new CMSNode object but don't initialize (this prevents a db query) - var node = new CMSNode(nodeId, false); - Permission.MakeNew(m_user, node, permission.Letter); - } - - } } \ No newline at end of file diff --git a/src/umbraco.cms/businesslogic/Permission.cs b/src/umbraco.cms/businesslogic/Permission.cs index 773231ce14..29a2f7ecc4 100644 --- a/src/umbraco.cms/businesslogic/Permission.cs +++ b/src/umbraco.cms/businesslogic/Permission.cs @@ -2,19 +2,21 @@ using System; using System.Collections; using System.Collections.Specialized; using System.Data; +using System.Linq; using System.Runtime.CompilerServices; - +using Umbraco.Core.Events; using umbraco.DataLayer; using umbraco.cms.businesslogic; using System.Collections.Generic; +using DeleteEventArgs = umbraco.cms.businesslogic.DeleteEventArgs; namespace umbraco.BusinessLogic { - /// - /// Summary description for Permission. - /// - public class Permission - { + /// + /// Summary description for Permission. + /// + public class Permission + { public int NodeId { get; private set; } public int UserId { get; private set; } @@ -30,23 +32,44 @@ namespace umbraco.BusinessLogic get { return Application.SqlHelper; } } - [MethodImpl(MethodImplOptions.Synchronized)] - public static void MakeNew(BusinessLogic.User User, cms.businesslogic.CMSNode Node, char PermissionKey) - { - IParameter[] parameters = new IParameter[] { SqlHelper.CreateParameter("@userId", User.Id), - SqlHelper.CreateParameter("@nodeId", Node.Id), - SqlHelper.CreateParameter("@permission", PermissionKey.ToString()) }; - // Method is synchronized so exists remains consistent (avoiding race condition) - bool exists = SqlHelper.ExecuteScalar("SELECT COUNT(userId) FROM umbracoUser2nodePermission WHERE userId = @userId AND nodeId = @nodeId AND permission = @permission", - parameters) > 0; - if (!exists) { - SqlHelper.ExecuteNonQuery("INSERT INTO umbracoUser2nodePermission (userId, nodeId, permission) VALUES (@userId, @nodeId, @permission)", - parameters); - // clear user cache to ensure permissions are re-loaded - User.GetUser(User.Id).FlushFromCache(); + public static void MakeNew(User User, CMSNode Node, char PermissionKey) + { + MakeNew(User, Node, PermissionKey, true); + } + + [MethodImpl(MethodImplOptions.Synchronized)] + internal static void MakeNew(User user, IEnumerable nodes, char permissionKey, bool raiseEvents) + { + var asArray = nodes.ToArray(); + foreach (var node in asArray) + { + var parameters = new[] { SqlHelper.CreateParameter("@userId", user.Id), + SqlHelper.CreateParameter("@nodeId", node.Id), + SqlHelper.CreateParameter("@permission", permissionKey.ToString()) }; + + // Method is synchronized so exists remains consistent (avoiding race condition) + var exists = SqlHelper.ExecuteScalar( + "SELECT COUNT(userId) FROM umbracoUser2nodePermission WHERE userId = @userId AND nodeId = @nodeId AND permission = @permission", + parameters) > 0; + + if (exists) return; + + SqlHelper.ExecuteNonQuery( + "INSERT INTO umbracoUser2nodePermission (userId, nodeId, permission) VALUES (@userId, @nodeId, @permission)", + parameters); } - } + + if (raiseEvents) + { + OnNew(new UserPermission(user, asArray, new[] { permissionKey }), new NewEventArgs()); + } + } + + private static void MakeNew(User User, CMSNode Node, char PermissionKey, bool raiseEvents) + { + MakeNew(User, new[] {Node}, PermissionKey, raiseEvents); + } /// /// Returns the permissions for a user @@ -74,7 +97,7 @@ namespace umbraco.BusinessLogic /// /// Returns the permissions for a node /// - /// + /// /// public static IEnumerable GetNodePermissions(CMSNode node) { @@ -100,10 +123,20 @@ namespace umbraco.BusinessLogic /// /// public static void DeletePermissions(User user, CMSNode node) + { + DeletePermissions(user, node, true); + } + + internal static void DeletePermissions(User user, CMSNode node, bool raiseEvents) { // delete all settings on the node for this user SqlHelper.ExecuteNonQuery("delete from umbracoUser2NodePermission where userId = @userId and nodeId = @nodeId", - SqlHelper.CreateParameter("@userId", user.Id), SqlHelper.CreateParameter("@nodeId", node.Id)); + SqlHelper.CreateParameter("@userId", user.Id), SqlHelper.CreateParameter("@nodeId", node.Id)); + + if (raiseEvents) + { + OnDeleted(new UserPermission(user, node, null), new DeleteEventArgs()); + } } /// @@ -114,20 +147,23 @@ namespace umbraco.BusinessLogic { // delete all settings on the node for this user SqlHelper.ExecuteNonQuery("delete from umbracoUser2NodePermission where userId = @userId", - SqlHelper.CreateParameter("@userId", user.Id)); + SqlHelper.CreateParameter("@userId", user.Id)); + + OnDeleted(new UserPermission(user, Enumerable.Empty(), null), new DeleteEventArgs()); } public static void DeletePermissions(int iUserID, int[] iNodeIDs) { - string sql = "DELETE FROM umbracoUser2NodePermission WHERE nodeID IN ({0}) AND userID=@userID"; - string nodeIDs = string.Join(",", Array.ConvertAll(iNodeIDs, Converter)); + var sql = "DELETE FROM umbracoUser2NodePermission WHERE nodeID IN ({0}) AND userID=@userID"; + var nodeIDs = string.Join(",", Array.ConvertAll(iNodeIDs, Converter)); sql = string.Format(sql, nodeIDs); - SqlHelper.ExecuteNonQuery(sql, - new IParameter[] { SqlHelper.CreateParameter("@userID", iUserID) }); + SqlHelper.ExecuteNonQuery(sql, new[] { SqlHelper.CreateParameter("@userID", iUserID) }); + + OnDeleted(new UserPermission(iUserID, iNodeIDs), new DeleteEventArgs()); } public static void DeletePermissions(int iUserID, int iNodeID) { - DeletePermissions(iUserID, new int[] { iNodeID }); + DeletePermissions(iUserID, new[] { iNodeID }); } private static string Converter(int from) { @@ -140,20 +176,122 @@ namespace umbraco.BusinessLogic /// public static void DeletePermissions(CMSNode node) { - - SqlHelper.ExecuteNonQuery("delete from umbracoUser2NodePermission where nodeId = @nodeId", + SqlHelper.ExecuteNonQuery( + "delete from umbracoUser2NodePermission where nodeId = @nodeId", SqlHelper.CreateParameter("@nodeId", node.Id)); + + OnDeleted(new UserPermission(null, node, null), new DeleteEventArgs()); } [MethodImpl(MethodImplOptions.Synchronized)] - public static void UpdateCruds(User user, CMSNode node, string permissions) - { - // delete all settings on the node for this user - DeletePermissions(user, node); + public static void UpdateCruds(User user, CMSNode node, string permissions) + { + // delete all settings on the node for this user + //false = do not raise events + DeletePermissions(user, node, false); - // Loop through the permissions and create them - foreach (char c in permissions.ToCharArray()) - MakeNew(user, node, c); - } - } + // Loop through the permissions and create them + foreach (char c in permissions) + { + //false = don't raise events since we'll raise a custom event after + MakeNew(user, node, c, false); + } + OnUpdated(new UserPermission(user, node, permissions.ToCharArray()), new SaveEventArgs()); + } + + internal static event TypedEventHandler Deleted; + private static void OnDeleted(UserPermission permission, DeleteEventArgs args) + { + if (Deleted != null) + { + Deleted(permission, args); + } + } + + internal static event TypedEventHandler Updated; + private static void OnUpdated(UserPermission permission, SaveEventArgs args) + { + if (Updated != null) + { + Updated(permission, args); + } + } + + internal static event TypedEventHandler New; + private static void OnNew(UserPermission permission, NewEventArgs args) + { + if (New != null) + { + New(permission, args); + } + } + + } + + internal class UserPermission + { + private int? _userId; + private readonly int[] _nodeIds; + + internal UserPermission(int userId) + { + _userId = userId; + } + + internal UserPermission(int userId, IEnumerable nodeIds) + { + _userId = userId; + _nodeIds = nodeIds.ToArray(); + } + + internal UserPermission(User user, CMSNode node, char[] permissionKeys) + { + User = user; + Nodes = new[] { node }; + PermissionKeys = permissionKeys; + } + + internal UserPermission(User user, IEnumerable nodes, char[] permissionKeys) + { + User = user; + Nodes = nodes; + PermissionKeys = permissionKeys; + } + + internal int UserId + { + get + { + if (_userId.HasValue) + { + return _userId.Value; + } + if (User != null) + { + return User.Id; + } + return -1; + } + } + + internal IEnumerable NodeIds + { + get + { + if (_nodeIds != null) + { + return _nodeIds; + } + if (Nodes != null) + { + return Nodes.Select(x => x.Id); + } + return Enumerable.Empty(); + } + } + + internal User User { get; private set; } + internal IEnumerable Nodes { get; private set; } + internal char[] PermissionKeys { get; private set; } + } } \ No newline at end of file