From 95d3a87bb2638ece9841b5f5693ba116d2ecf4cc Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 20 Dec 2019 16:19:29 +1100 Subject: [PATCH] Simplifies ScopeProvider now that we are using AsyncLocal we don't need to worry about the weird constraints of CallContext which was being worked around with the static object dictionary --- .../Scoping/IScopeProvider.cs | 2 +- .../Scoping/ScopeProvider.cs | 125 +++++------------- .../Migrations/MigrationTests.cs | 1 - 3 files changed, 36 insertions(+), 92 deletions(-) diff --git a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs index 6c9eb63ba0..4a7ccae481 100644 --- a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs @@ -88,7 +88,7 @@ namespace Umbraco.Core.Scoping ISqlContext SqlContext { get; } #if DEBUG_SCOPES - Dictionary CallContextObjects { get; } + IEnumerable ScopeInfos { get; } ScopeInfo GetScopeInfo(IScope scope); #endif diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index 10130e820e..d4bd224eae 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -49,23 +49,23 @@ namespace Umbraco.Core.Scoping SafeCallContext.Register( () => { - var scope = GetCallContextObject(ScopeItemKey); - var context = GetCallContextObject(ContextItemKey); - SetCallContextObject(ScopeItemKey, null); - SetCallContextObject(ContextItemKey, null); + var scope = GetCallContextObject(ScopeItemKey); + var context = GetCallContextObject(ContextItemKey); + SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ContextItemKey, null); return Tuple.Create(scope, context); }, o => { // cannot re-attached over leaked scope/context - if (GetCallContextObject(ScopeItemKey) != null) + if (GetCallContextObject(ScopeItemKey) != null) throw new Exception("Found leaked scope when restoring call context."); - if (GetCallContextObject(ContextItemKey) != null) + if (GetCallContextObject(ContextItemKey) != null) throw new Exception("Found leaked context when restoring call context."); - var t = (Tuple) o; - SetCallContextObject(ScopeItemKey, t.Item1); - SetCallContextObject(ContextItemKey, t.Item2); + var t = (Tuple) o; + SetCallContextObject(ScopeItemKey, t.Item1); + SetCallContextObject(ContextItemKey, t.Item2); }); } @@ -93,51 +93,16 @@ namespace Umbraco.Core.Scoping // care of de-registering themselves from context. // see https://www.zpqrtbnk.net/posts/putting-things-in-contexts/ - private static readonly object StaticCallContextObjectsLock = new object(); - private static readonly Dictionary StaticCallContextObjects - = new Dictionary(); - -#if DEBUG_SCOPES - public Dictionary CallContextObjects - { - get - { - lock (StaticCallContextObjectsLock) - { - // capture in a dictionary - return StaticCallContextObjects.ToDictionary(x => x.Key, x => x.Value); - } - } - } -#endif - private static T GetCallContextObject(string key) - where T : class + where T : class, IInstanceIdentifiable { - var objectKey = CallContext.GetData(key); - if (objectKey == Guid.Empty) return null; - - lock (StaticCallContextObjectsLock) - { - if (StaticCallContextObjects.TryGetValue(objectKey, out object callContextObject)) - { -#if DEBUG_SCOPES - Current.Logger.Debug("Got " + typeof(T).Name + " Object " + objectKey.ToString("N").Substring(0, 8)); - //_logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - return (T)callContextObject; - } - - // hard to inject into a static method :( - Current.Logger.Warn("Missed {TypeName} Object {ObjectKey}", typeof(T).Name, objectKey.ToString("N").Substring(0, 8)); -#if DEBUG_SCOPES - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - return null; - } + var obj = CallContext.GetData(key); + if (obj == default(T)) return null; + return obj; } - private static void SetCallContextObject(string key, IInstanceIdentifiable value) + private static void SetCallContextObject(string key, T value) + where T: class, IInstanceIdentifiable { #if DEBUG_SCOPES // manage the 'context' that contains the scope (null, "http" or "call") @@ -145,14 +110,8 @@ namespace Umbraco.Core.Scoping if (key == ScopeItemKey) { // first, null-register the existing value - var ambientKey = CallContext.GetData(ScopeItemKey); - object o = null; - lock (StaticCallContextObjectsLock) - { - if (ambientKey != default) - StaticCallContextObjects.TryGetValue(ambientKey, out o); - } - var ambientScope = o as IScope; + var ambientScope = CallContext.GetData(ScopeItemKey); + if (ambientScope != null) RegisterContext(ambientScope, null); // then register the new value var scope = value as IScope; @@ -161,33 +120,18 @@ namespace Umbraco.Core.Scoping #endif if (value == null) { - var objectKey = CallContext.GetData(key); - CallContext.SetData(key, default); // aka remove - if (objectKey == Guid.Empty) return; - lock (StaticCallContextObjectsLock) - { -#if DEBUG_SCOPES - Current.Logger.Debug("Remove Object " + objectKey.ToString("N").Substring(0, 8)); - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); -#endif - StaticCallContextObjects.Remove(objectKey); - } + var obj = CallContext.GetData(key); + CallContext.SetData(key, default); // aka remove + if (obj == null) return; } else { - // note - we are *not* detecting an already-existing value - // because our code in this class *always* sets to null before - // setting to a real value - var objectKey = value.InstanceId; - lock (StaticCallContextObjectsLock) - { + #if DEBUG_SCOPES - Current.Logger.Debug("AddObject " + objectKey.ToString("N").Substring(0, 8)); - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 24)); + Current.Logger.Debug("AddObject " + value.InstanceId.ToString("N").Substring(0, 8)); #endif - StaticCallContextObjects.Add(objectKey, value); - } - CallContext.SetData(key, objectKey); + + CallContext.SetData(key, value); } } @@ -249,12 +193,12 @@ namespace Umbraco.Core.Scoping { // clear both SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); + SetCallContextObject(ContextItemKey, null); if (value == null) return; // set http/call context if (SetHttpContextObject(ContextItemKey, value, false) == false) - SetCallContextObject(ContextItemKey, value); + SetCallContextObject(ContextItemKey, value); } } @@ -274,21 +218,22 @@ namespace Umbraco.Core.Scoping public Scope AmbientScope { // try http context, fallback onto call context - get => GetHttpContextObject(ScopeItemKey, false) - ?? GetCallContextObject(ScopeItemKey); + // we are casting here because we know its a concrete type + get => (Scope)GetHttpContextObject(ScopeItemKey, false) + ?? (Scope)GetCallContextObject(ScopeItemKey); set { // clear both SetHttpContextObject(ScopeItemKey, null, false); SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ScopeItemKey, null); if (value == null) return; // set http/call context if (value.CallContext == false && SetHttpContextObject(ScopeItemKey, value, false)) SetHttpContextObject(ScopeRefItemKey, _scopeReference); else - SetCallContextObject(ScopeItemKey, value); + SetCallContextObject(ScopeItemKey, value); } } @@ -299,9 +244,9 @@ namespace Umbraco.Core.Scoping // clear all SetHttpContextObject(ScopeItemKey, null, false); SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); + SetCallContextObject(ScopeItemKey, null); SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); + SetCallContextObject(ContextItemKey, null); if (scope == null) { if (context != null) @@ -316,8 +261,8 @@ namespace Umbraco.Core.Scoping } else { - SetCallContextObject(ScopeItemKey, scope); - SetCallContextObject(ContextItemKey, context); + SetCallContextObject(ScopeItemKey, scope); + SetCallContextObject(ContextItemKey, context); } } diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index 64318bc193..bfadd45b0d 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -56,7 +56,6 @@ namespace Umbraco.Tests.Migrations { throw new NotImplementedException(); } - public Dictionary CallContextObjects => throw new NotImplementedException(); public IEnumerable ScopeInfos => throw new NotImplementedException(); #endif }