Merge pull request #7234 from umbraco/netcore/feature/AB3980-replace-httpcontext-items-in-scopeprovider

NetCore: Replaces the usages of HttpContext.Items ScopeProvider

src/Umbraco.Abstractions/Cache/AppCaches.cs

@@ -12,7 +12,7 @@ namespace Umbraco.Core.Cache
         /// </summary>
         public AppCaches(
             IAppPolicyCache runtimeCache,
-            IAppCache requestCache,
+            IRequestCache requestCache,
             IsolatedCaches isolatedCaches)
         {
             RuntimeCache = runtimeCache ?? throw new ArgumentNullException(nameof(runtimeCache));
@@ -45,7 +45,7 @@ namespace Umbraco.Core.Cache
         /// <para>The per-request caches works on top of the current HttpContext items.</para>
         /// <para>Outside a web environment, the behavior of that cache is unspecified.</para>
         /// </remarks>
-        public IAppCache RequestCache { get; }
+        public IRequestCache RequestCache { get; }
 
         /// <summary>
         /// Gets the runtime cache.

src/Umbraco.Abstractions/Cache/DictionaryAppCache.cs

@@ -8,7 +8,7 @@ namespace Umbraco.Core.Cache
     /// <summary>
     /// Implements <see cref="IAppCache"/> on top of a concurrent dictionary.
     /// </summary>
-    public class DictionaryAppCache : IAppCache
+    public class DictionaryAppCache : IRequestCache
     {
         /// <summary>
         /// Gets the internal items dictionary, for tests only!
@@ -17,6 +17,9 @@ namespace Umbraco.Core.Cache
 
         public int Count => _items.Count;
 
+        /// <inheritdoc />
+        public bool IsAvailable => true;
+
         /// <inheritdoc />
         public virtual object Get(string key)
         {
@@ -29,6 +32,10 @@ namespace Umbraco.Core.Cache
             return _items.GetOrAdd(key, _ => factory());
         }
 
+        public bool Set(string key, object value) => _items.TryAdd(key, value);
+
+        public bool Remove(string key) => _items.TryRemove(key, out _);
+
         /// <inheritdoc />
         public virtual IEnumerable<object> SearchByKey(string keyStartsWith)
         {

src/Umbraco.Abstractions/Cache/FastDictionaryAppCacheBase.cs

@@ -275,7 +275,7 @@ namespace Umbraco.Core.Cache
             return $"{CacheItemPrefix}-{key}";
         }
 
-       
+
 
         #endregion
     }

src/Umbraco.Abstractions/Cache/HttpRequestAppCache.cs

@@ -15,7 +15,7 @@ namespace Umbraco.Core.Cache
     /// or no Items...) then this cache acts as a pass-through and does not cache
     /// anything.</para>
     /// </remarks>
-    public class HttpRequestAppCache : FastDictionaryAppCacheBase
+    public class HttpRequestAppCache : FastDictionaryAppCacheBase, IRequestCache
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="HttpRequestAppCache"/> class with a context, for unit tests!
@@ -27,6 +27,8 @@ namespace Umbraco.Core.Cache
 
         private Func<IDictionary> ContextItems { get; }
 
+        public bool IsAvailable => TryGetContextItems(out _);
+
         private bool TryGetContextItems(out IDictionary items)
         {
             items = ContextItems?.Invoke();
@@ -75,6 +77,42 @@ namespace Umbraco.Core.Cache
             return value;
         }
 
+        public bool Set(string key, object value)
+        {
+            //no place to cache so just return the callback result
+            if (!TryGetContextItems(out var items)) return false;
+            key = GetCacheKey(key);
+            try
+            {
+
+                EnterWriteLock();
+                items[key] = SafeLazy.GetSafeLazy(() => value);
+            }
+            finally
+            {
+                ExitWriteLock();
+            }
+            return true;
+        }
+
+        public bool Remove(string key)
+        {
+            //no place to cache so just return the callback result
+            if (!TryGetContextItems(out var items)) return false;
+            key = GetCacheKey(key);
+            try
+            {
+
+                EnterWriteLock();
+                items.Remove(key);
+            }
+            finally
+            {
+                ExitWriteLock();
+            }
+            return true;
+        }
+
         #region Entries
 
         protected override IEnumerable<DictionaryEntry> GetDictionaryEntries()

src/Umbraco.Abstractions/Cache/IRequestCache.cs

@@ -0,0 +1,13 @@
+namespace Umbraco.Core.Cache
+{
+    public interface IRequestCache : IAppCache
+    {
+        bool Set(string key, object value);
+        bool Remove(string key);
+
+        /// <summary>
+        /// Returns true if the request cache is available otherwise false
+        /// </summary>
+        bool IsAvailable { get; }
+    }
+}

src/Umbraco.Abstractions/Cache/NoAppCache.cs

@@ -7,15 +7,18 @@ namespace Umbraco.Core.Cache
     /// <summary>
     /// Implements <see cref="IAppPolicyCache"/> and do not cache.
     /// </summary>
-    public class NoAppCache : IAppPolicyCache
+    public class NoAppCache : IAppPolicyCache, IRequestCache
     {
-        private NoAppCache() { }
+        protected NoAppCache() { }
 
         /// <summary>
         /// Gets the singleton instance.
         /// </summary>
         public static NoAppCache Instance { get; } = new NoAppCache();
 
+        /// <inheritdoc />
+        public bool IsAvailable => false;
+
         /// <inheritdoc />
         public virtual object Get(string cacheKey)
         {
@@ -28,6 +31,10 @@ namespace Umbraco.Core.Cache
             return factory();
         }
 
+        public bool Set(string key, object value) => false;
+
+        public bool Remove(string key) => false;
+
         /// <inheritdoc />
         public virtual IEnumerable<object> SearchByKey(string keyStartsWith)
         {

src/Umbraco.Core/CompositionExtensions_Essentials.cs

@@ -31,6 +31,7 @@ namespace Umbraco.Core
             composition.RegisterUnique(profilingLogger);
             composition.RegisterUnique(mainDom);
             composition.RegisterUnique(appCaches);
+            composition.RegisterUnique(appCaches.RequestCache);
             composition.RegisterUnique(databaseFactory);
             composition.RegisterUnique(factory => factory.GetInstance<IUmbracoDatabaseFactory>().SqlContext);
             composition.RegisterUnique(typeLoader);

src/Umbraco.Core/Scoping/ScopeProvider.cs

@@ -1,17 +1,16 @@
 using System;
-using System.Collections;
 using System.Collections.Generic;
 using System.Data;
 using System.Runtime.Remoting.Messaging;
-using System.Web;
+using Umbraco.Core.Cache;
 using Umbraco.Core.Composing;
 using Umbraco.Core.Events;
 using Umbraco.Core.IO;
 using Umbraco.Core.Logging;
 using Umbraco.Core.Persistence;
-
 #if DEBUG_SCOPES
 using System.Linq;
+using System.Text;
 #endif
 
 namespace Umbraco.Core.Scoping
@@ -23,14 +22,16 @@ namespace Umbraco.Core.Scoping
     {
         private readonly ILogger _logger;
         private readonly ITypeFinder _typeFinder;
+        private readonly IRequestCache _requestCache;
         private readonly FileSystems _fileSystems;
 
-        public ScopeProvider(IUmbracoDatabaseFactory databaseFactory, FileSystems fileSystems, ILogger logger, ITypeFinder typeFinder)
+        public ScopeProvider(IUmbracoDatabaseFactory databaseFactory, FileSystems fileSystems, ILogger logger, ITypeFinder typeFinder, IRequestCache requestCache)
         {
             DatabaseFactory = databaseFactory;
             _fileSystems = fileSystems;
             _logger = logger;
             _typeFinder = typeFinder;
+            _requestCache = requestCache;
 
             // take control of the FileSystems
             _fileSystems.IsScoped = () => AmbientScope != null && AmbientScope.ScopedFileSystems;
@@ -181,40 +182,33 @@ namespace Umbraco.Core.Scoping
             }
         }
 
-        // this is for tests exclusively until we have a proper accessor in v8
-        internal static Func<IDictionary> HttpContextItemsGetter { get; set; }
 
-        private static IDictionary HttpContextItems => HttpContextItemsGetter == null
-            ? HttpContext.Current?.Items
-            : HttpContextItemsGetter();
-
-        public static T GetHttpContextObject<T>(string key, bool required = true)
+        private T GetHttpContextObject<T>(string key, bool required = true)
             where T : class
         {
-            var httpContextItems = HttpContextItems;
-            if (httpContextItems != null)
-                return (T)httpContextItems[key];
-            if (required)
-                throw new Exception("HttpContext.Current is null.");
-            return null;
+
+            if (!_requestCache.IsAvailable && required)
+                throw new Exception("Request cache is unavailable.");
+
+            return (T)_requestCache.Get(key);
         }
 
-        private static bool SetHttpContextObject(string key, object value, bool required = true)
+        private bool SetHttpContextObject(string key, object value, bool required = true)
         {
-            var httpContextItems = HttpContextItems;
-            if (httpContextItems == null)
+            if (!_requestCache.IsAvailable)
             {
                 if (required)
-                    throw new Exception("HttpContext.Current is null.");
+                    throw new Exception("Request cache is unavailable.");
                 return false;
             }
+
 #if DEBUG_SCOPES
             // manage the 'context' that contains the scope (null, "http" or "call")
             // only for scopes of course!
             if (key == ScopeItemKey)
             {
                 // first, null-register the existing value
-                var ambientScope = (IScope)httpContextItems[ScopeItemKey];
+                var ambientScope = (IScope)_requestCache.Get(ScopeItemKey);
                 if (ambientScope != null) RegisterContext(ambientScope, null);
                 // then register the new value
                 var scope = value as IScope;
@@ -222,9 +216,9 @@ namespace Umbraco.Core.Scoping
             }
 #endif
             if (value == null)
-                httpContextItems.Remove(key);
+                _requestCache.Remove(key);
             else
-                httpContextItems[key] = value;
+                _requestCache.Set(key, value);
             return true;
         }
 

src/Umbraco.Tests/Components/ComponentTests.cs

@@ -38,7 +38,7 @@ namespace Umbraco.Tests.Components
             var typeFinder = new TypeFinder(logger);
             var f = new UmbracoDatabaseFactory(logger, new Lazy<IMapperCollection>(() => new MapperCollection(Enumerable.Empty<BaseMapper>())), TestHelper.GetConfigs());
             var fs = new FileSystems(mock.Object, logger, IOHelper.Default, SettingsForTests.GenerateMockGlobalSettings());
-            var p = new ScopeProvider(f, fs, logger, typeFinder);
+            var p = new ScopeProvider(f, fs, logger, typeFinder, TestHelper.GetRequestCache());
 
             mock.Setup(x => x.GetInstance(typeof (ILogger))).Returns(logger);
             mock.Setup(x => x.GetInstance(typeof (IProfilingLogger))).Returns(new ProfilingLogger(Mock.Of<ILogger>(), Mock.Of<IProfiler>()));

src/Umbraco.Tests/Scoping/ScopeTests.cs

@@ -107,46 +107,33 @@ namespace Umbraco.Tests.Scoping
             var scopeProvider = ScopeProvider;
             Assert.IsNull(scopeProvider.AmbientScope);
 
-            var httpContextItems = new Hashtable();
-            ScopeProviderStatic.HttpContextItemsGetter = () => httpContextItems;
-            try
+            using (var scope = scopeProvider.CreateScope())
             {
-                using (var scope = scopeProvider.CreateScope())
+                Assert.IsInstanceOf<Scope>(scope);
+                Assert.IsNotNull(scopeProvider.AmbientScope);
+                Assert.AreSame(scope, scopeProvider.AmbientScope);
+                // only if Core.DEBUG_SCOPES are defined
+                //Assert.IsEmpty(scopeProvider.CallContextObjects);
+
+                using (var nested = scopeProvider.CreateScope(callContext: true))
                 {
-                    Assert.IsInstanceOf<Scope>(scope);
+                    Assert.IsInstanceOf<Scope>(nested);
                     Assert.IsNotNull(scopeProvider.AmbientScope);
-                    Assert.AreSame(scope, scopeProvider.AmbientScope);
-                    Assert.AreSame(scope, httpContextItems[ScopeProviderStatic.ScopeItemKey]);
+                    Assert.AreSame(nested, scopeProvider.AmbientScope);
+                    Assert.AreSame(scope, ((Scope) nested).ParentScope);
+
+                    // it's moved over to call context
+                    var callContextKey = CallContext.LogicalGetData(ScopeProviderStatic.ScopeItemKey).AsGuid();
+                    Assert.AreNotEqual(Guid.Empty, callContextKey);
 
                     // only if Core.DEBUG_SCOPES are defined
-                    //Assert.IsEmpty(scopeProvider.CallContextObjects);
-
-                    using (var nested = scopeProvider.CreateScope(callContext: true))
-                    {
-                        Assert.IsInstanceOf<Scope>(nested);
-                        Assert.IsNotNull(scopeProvider.AmbientScope);
-                        Assert.AreSame(nested, scopeProvider.AmbientScope);
-                        Assert.AreSame(scope, ((Scope) nested).ParentScope);
-
-                        // it's moved over to call context
-                        Assert.IsNull(httpContextItems[ScopeProviderStatic.ScopeItemKey]);
-                        var callContextKey = CallContext.LogicalGetData(ScopeProviderStatic.ScopeItemKey).AsGuid();
-                        Assert.AreNotEqual(Guid.Empty, callContextKey);
-
-                        // only if Core.DEBUG_SCOPES are defined
-                        //var ccnested = scopeProvider.CallContextObjects[callContextKey];
-                        //Assert.AreSame(nested, ccnested);
-                    }
-
-                    // it's naturally back in http context
-                    Assert.AreSame(scope, httpContextItems[ScopeProviderStatic.ScopeItemKey]);
+                    //var ccnested = scopeProvider.CallContextObjects[callContextKey];
+                    //Assert.AreSame(nested, ccnested);
                 }
-                Assert.IsNull(scopeProvider.AmbientScope);
-            }
-            finally
-            {
-                ScopeProviderStatic.HttpContextItemsGetter = null;
+
+                // it's naturally back in http context
             }
+            Assert.IsNull(scopeProvider.AmbientScope);
         }
 
         [Test]
@@ -441,46 +428,33 @@ namespace Umbraco.Tests.Scoping
             var scopeProvider = ScopeProvider;
             Assert.IsNull(scopeProvider.AmbientScope);
 
-            var httpContextItems = new Hashtable();
-            ScopeProviderStatic.HttpContextItemsGetter = () => httpContextItems;
-            try
+            using (var scope = scopeProvider.CreateScope())
             {
-                using (var scope = scopeProvider.CreateScope())
+                Assert.IsNotNull(scopeProvider.AmbientScope);
+                Assert.IsNotNull(scopeProvider.AmbientContext);
+                using (new SafeCallContext())
                 {
-                    Assert.IsNotNull(scopeProvider.AmbientScope);
-                    Assert.IsNotNull(scopeProvider.AmbientContext);
-                    using (new SafeCallContext())
+                    Assert.IsNull(scopeProvider.AmbientScope);
+                    Assert.IsNull(scopeProvider.AmbientContext);
+
+                    using (var newScope = scopeProvider.CreateScope())
                     {
-                        // pretend it's another thread
-                        ScopeProviderStatic.HttpContextItemsGetter = null;
-
-                        Assert.IsNull(scopeProvider.AmbientScope);
-                        Assert.IsNull(scopeProvider.AmbientContext);
-
-                        using (var newScope = scopeProvider.CreateScope())
-                        {
-                            Assert.IsNotNull(scopeProvider.AmbientScope);
-                            Assert.IsNull(scopeProvider.AmbientScope.ParentScope);
-                            Assert.IsNotNull(scopeProvider.AmbientContext);
-                        }
-
-                        Assert.IsNull(scopeProvider.AmbientScope);
-                        Assert.IsNull(scopeProvider.AmbientContext);
-
-                        // back to original thread
-                        ScopeProviderStatic.HttpContextItemsGetter = () => httpContextItems;
+                        Assert.IsNotNull(scopeProvider.AmbientScope);
+                        Assert.IsNull(scopeProvider.AmbientScope.ParentScope);
+                        Assert.IsNotNull(scopeProvider.AmbientContext);
                     }
-                    Assert.IsNotNull(scopeProvider.AmbientScope);
-                    Assert.AreSame(scope, scopeProvider.AmbientScope);
-                }
 
-                Assert.IsNull(scopeProvider.AmbientScope);
-                Assert.IsNull(scopeProvider.AmbientContext);
-            }
-            finally
-            {
-                ScopeProviderStatic.HttpContextItemsGetter = null;
+                    Assert.IsNull(scopeProvider.AmbientScope);
+                    Assert.IsNull(scopeProvider.AmbientContext);
+                }
+                Assert.IsNotNull(scopeProvider.AmbientScope);
+                Assert.AreSame(scope, scopeProvider.AmbientScope);
             }
+
+            Assert.IsNull(scopeProvider.AmbientScope);
+            Assert.IsNull(scopeProvider.AmbientContext);
+
+
         }
 
         [Test]

src/Umbraco.Tests/TestHelpers/TestHelper.cs

@@ -296,5 +296,10 @@ namespace Umbraco.Tests.TestHelpers
         {
             return RegisterFactory.Create(GetConfigs().Global());
         }
+
+        public static IRequestCache GetRequestCache()
+        {
+            return new DictionaryAppCache();
+        }
     }
 }

src/Umbraco.Tests/TestHelpers/TestObjects.cs

@@ -173,7 +173,7 @@ namespace Umbraco.Tests.TestHelpers
 
             var macroService = GetLazyService<IMacroService>(factory, c => new MacroService(scopeProvider, logger, eventMessagesFactory, GetRepo<IMacroRepository>(c), GetRepo<IAuditRepository>(c)));
             var packagingService = GetLazyService<IPackagingService>(factory, c =>
-            {                
+            {
                 var compiledPackageXmlParser = new CompiledPackageXmlParser(new ConflictingPackageData(macroService.Value, fileService.Value), globalSettings);
                 return new PackagingService(
                     auditService.Value,
@@ -247,7 +247,7 @@ namespace Umbraco.Tests.TestHelpers
 
             typeFinder = typeFinder ?? new TypeFinder(logger);
             fileSystems = fileSystems ?? new FileSystems(Current.Factory, logger, IOHelper.Default, SettingsForTests.GenerateMockGlobalSettings());
-            var scopeProvider = new ScopeProvider(databaseFactory, fileSystems, logger, typeFinder);
+            var scopeProvider = new ScopeProvider(databaseFactory, fileSystems, logger, typeFinder, NoAppCache.Instance);
             return scopeProvider;
         }