(Perf) Hooking: prevent repetitive hooking of the same entity/state/[pre|post] combination within a single request

muratcakir · muratcakir · commit 036ef2c65d9e · 2016-11-07T19:51:31.000+01:00
diff --git a/src/Libraries/SmartStore.Core/Data/Hooks/DefaultHookHandler.cs b/src/Libraries/SmartStore.Core/Data/Hooks/DefaultHookHandler.cs
@@ -15,6 +15,9 @@ public class DefaultHookHandler : IHookHandler
 		private readonly Multimap<Type, IPreActionHook> _preHooksRequestCache = new Multimap<Type, IPreActionHook>();
 		private readonly Multimap<Type, IPostActionHook> _postHooksRequestCache = new Multimap<Type, IPostActionHook>();
 
+		// Prevents repetitive hooking of the same entity/state/[pre|post] combination within a single request
+		private readonly HashSet<HookedEntityKey> _hookedEntities = new HashSet<HookedEntityKey>();
+
 		private static HashSet<Type> _importantPreHookTypes;
 		private static HashSet<Type> _importantPostHookTypes;
 		private readonly static object _lock = new object();
@@ -79,7 +82,13 @@ public bool TriggerPreActionHooks(IEnumerable<HookedEntityEntry> entries, bool r
 			foreach (var entry in entries)
 			{
 				var e = entry; // Prevents access to modified closure
-				var preHooks = GetPreHookInstancesFor(e.Entry.Entity as BaseEntity, importantHooksOnly);
+				var entity = e.Entry.Entity as BaseEntity;
+				if (HookedAlready(entity, e.PreSaveState, false))
+				{
+					// Prevent repetitive hooking of the same entity/state/pre combination within a single request
+					continue;
+				}
+				var preHooks = GetPreHookInstancesFor(entity, importantHooksOnly);
 				foreach (var hook in preHooks)
 				{
 					if (hook.CanProcess(e.PreSaveState) && hook.RequiresValidation == requiresValidation)
@@ -89,7 +98,7 @@ public bool TriggerPreActionHooks(IEnumerable<HookedEntityEntry> entries, bool r
 						// call hook
 						try
 						{
-							hook.HookObject(e.Entry.Entity, metadata);
+							hook.HookObject(entity, metadata);
 						}
 						catch (Exception ex)
 						{
@@ -153,7 +162,13 @@ public void TriggerPostActionHooks(IEnumerable<HookedEntityEntry> entries, bool
 			foreach (var entry in entries)
 			{
 				var e = entry; // Prevents access to modified closure
-				var postHooks = GetPostHookInstancesFor(e.Entry.Entity as BaseEntity, importantHooksOnly);
+				var entity = e.Entry.Entity as BaseEntity;
+				if (HookedAlready(entity, e.PreSaveState, true))
+				{
+					// Prevent repetitive hooking of the same entity/state/post combination within a single request
+					continue;
+				}
+				var postHooks = GetPostHookInstancesFor(entity, importantHooksOnly);
 				foreach (var hook in postHooks)
 				{
 					if (hook.CanProcess(e.PreSaveState))
@@ -163,7 +178,7 @@ public void TriggerPostActionHooks(IEnumerable<HookedEntityEntry> entries, bool
 						// call hook
 						try
 						{
-							hook.HookObject(e.Entry.Entity, metadata);
+							hook.HookObject(entity, metadata);
 						}
 						catch (Exception ex)
 						{
@@ -205,5 +220,28 @@ private IEnumerable<IPostActionHook> GetPostHookInstancesFor(BaseEntity entity,
 
 			return hooks;
 		}
+
+		private bool HookedAlready(BaseEntity entity, EntityState preSaveState, bool isPostActionHook)
+		{
+			if (entity.IsTransientRecord())
+				return false;
+
+			var key = new HookedEntityKey(entity.GetUnproxiedType(), entity.Id, preSaveState, isPostActionHook);
+			if (_hookedEntities.Contains(key))
+			{
+				return true;
+			}
+
+			_hookedEntities.Add(key);
+			return false;
+		}
+
+		class HookedEntityKey : Tuple<Type, int, EntityState, bool>
+		{
+			public HookedEntityKey(Type entityType, int entityId, EntityState preSaveState, bool isPostActionHook)
+				: base(entityType, entityId, preSaveState, isPostActionHook)
+			{
+			}
+		}
 	}
 }
