diff --git a/build/NuSpecs/UmbracoCms.Web.nuspec b/build/NuSpecs/UmbracoCms.Web.nuspec index 92cb0f065e..f12ada7e64 100644 --- a/build/NuSpecs/UmbracoCms.Web.nuspec +++ b/build/NuSpecs/UmbracoCms.Web.nuspec @@ -25,12 +25,12 @@ --> - + - + - + diff --git a/src/Umbraco.Core/Cache/FastDictionaryAppCacheBase.cs b/src/Umbraco.Core/Cache/FastDictionaryAppCacheBase.cs index 7ebbcc8b63..9eaf5fb833 100644 --- a/src/Umbraco.Core/Cache/FastDictionaryAppCacheBase.cs +++ b/src/Umbraco.Core/Cache/FastDictionaryAppCacheBase.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Text.RegularExpressions; @@ -87,9 +87,10 @@ namespace Umbraco.Cms.Core.Cache try { EnterWriteLock(); - foreach (var entry in GetDictionaryEntries() - .ToArray()) + foreach (var entry in GetDictionaryEntries().ToArray()) + { RemoveEntry((string) entry.Key); + } } finally { @@ -133,7 +134,9 @@ namespace Umbraco.Cms.Core.Cache return value == null || (isInterface ? (type.IsInstanceOfType(value)) : (value.GetType() == type)); }) .ToArray()) + { RemoveEntry((string) entry.Key); + } } finally { @@ -163,7 +166,9 @@ namespace Umbraco.Cms.Core.Cache return value == null || (isInterface ? (value is T) : (value.GetType() == typeOfT)); }) .ToArray()) + { RemoveEntry((string) entry.Key); + } } finally { @@ -196,7 +201,9 @@ namespace Umbraco.Cms.Core.Cache // run predicate on the 'public key' part only, ie without prefix && predicate(((string) x.Key).Substring(plen), (T) value); })) + { RemoveEntry((string) entry.Key); + } } finally { @@ -214,7 +221,9 @@ namespace Umbraco.Cms.Core.Cache foreach (var entry in GetDictionaryEntries() .Where(x => ((string)x.Key).Substring(plen).InvariantStartsWith(keyStartsWith)) .ToArray()) + { RemoveEntry((string) entry.Key); + } } finally { @@ -233,7 +242,9 @@ namespace Umbraco.Cms.Core.Cache foreach (var entry in GetDictionaryEntries() .Where(x => compiled.IsMatch(((string)x.Key).Substring(plen))) .ToArray()) + { RemoveEntry((string) entry.Key); + } } finally { @@ -261,10 +272,7 @@ namespace Umbraco.Cms.Core.Cache protected abstract void EnterWriteLock(); protected abstract void ExitWriteLock(); - protected string GetCacheKey(string key) - { - return $"{CacheItemPrefix}-{key}"; - } + protected string GetCacheKey(string key) => $"{CacheItemPrefix}-{key}"; diff --git a/src/Umbraco.Core/Cache/GenericDictionaryRequestAppCache.cs b/src/Umbraco.Core/Cache/GenericDictionaryRequestAppCache.cs deleted file mode 100644 index 17558a78d4..0000000000 --- a/src/Umbraco.Core/Cache/GenericDictionaryRequestAppCache.cs +++ /dev/null @@ -1,189 +0,0 @@ -using System; -using System.Collections; -using System.Collections.Generic; -using System.Linq; -using System.Threading; - -namespace Umbraco.Cms.Core.Cache -{ - /// - /// Implements a fast on top of HttpContext.Items. - /// - /// - /// If no current HttpContext items can be found (no current HttpContext, - /// or no Items...) then this cache acts as a pass-through and does not cache - /// anything. - /// - public class GenericDictionaryRequestAppCache : FastDictionaryAppCacheBase, IRequestCache - { - /// - /// Initializes a new instance of the class with a context, for unit tests! - /// - public GenericDictionaryRequestAppCache(Func> requestItems) : base() - { - ContextItems = requestItems; - } - - private Func> ContextItems { get; } - - public bool IsAvailable => TryGetContextItems(out _); - - private bool TryGetContextItems(out IDictionary items) - { - items = ContextItems?.Invoke(); - return items != null; - } - - /// - public override object Get(string key, Func factory) - { - //no place to cache so just return the callback result - if (!TryGetContextItems(out var items)) return factory(); - - key = GetCacheKey(key); - - Lazy result; - - try - { - EnterWriteLock(); - result = items[key] as Lazy; // null if key not found - - // cannot create value within the lock, so if result.IsValueCreated is false, just - // do nothing here - means that if creation throws, a race condition could cause - // more than one thread to reach the return statement below and throw - accepted. - - if (result == null || SafeLazy.GetSafeLazyValue(result, true) == null) // get non-created as NonCreatedValue & exceptions as null - { - result = SafeLazy.GetSafeLazy(factory); - items[key] = result; - } - } - finally - { - ExitWriteLock(); - } - - // using GetSafeLazy and GetSafeLazyValue ensures that we don't cache - // exceptions (but try again and again) and silently eat them - however at - // some point we have to report them - so need to re-throw here - - // this does not throw anymore - //return result.Value; - - var value = result.Value; // will not throw (safe lazy) - if (value is SafeLazy.ExceptionHolder eh) eh.Exception.Throw(); // throw once! - 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> GetDictionaryEntries() - { - const string prefix = CacheItemPrefix + "-"; - - if (!TryGetContextItems(out var items)) return Enumerable.Empty>(); - - return items.Cast>() - .Where(x => x.Key is string s && s.StartsWith(prefix)); - } - - protected override void RemoveEntry(string key) - { - if (!TryGetContextItems(out var items)) return; - - items.Remove(key); - } - - protected override object GetEntry(string key) - { - return !TryGetContextItems(out var items) ? null : items[key]; - } - - #endregion - - #region Lock - - private const string ContextItemsLockKey = "Umbraco.Core.Cache.HttpRequestCache::LockEntered"; - - protected override void EnterReadLock() => EnterWriteLock(); - - protected override void EnterWriteLock() - { - if (!TryGetContextItems(out var items)) return; - - // note: cannot keep 'entered' as a class variable here, - // since there is one per request - so storing it within - // ContextItems - which is locked, so this should be safe - - var entered = false; - Monitor.Enter(items, ref entered); - items[ContextItemsLockKey] = entered; - } - - protected override void ExitReadLock() => ExitWriteLock(); - - protected override void ExitWriteLock() - { - if (!TryGetContextItems(out var items)) return; - - var entered = (bool?)items[ContextItemsLockKey] ?? false; - if (entered) - Monitor.Exit(items); - items.Remove(ContextItemsLockKey); - } - - #endregion - - public IEnumerator> GetEnumerator() - { - if (!TryGetContextItems(out var items)) - { - yield break; - } - - foreach (var item in items) - { - yield return new KeyValuePair(item.Key.ToString(), item.Value); - } - } - - IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - } -} diff --git a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs index 45abc39268..f8cc97acb8 100644 --- a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs @@ -84,16 +84,6 @@ namespace Umbraco.Cms.Core.Configuration.Models /// public bool InstallMissingDatabase { get; set; } = false; - /// - /// Gets or sets a value indicating whether unattended installs are enabled. - /// - /// - /// By default, when a database connection string is configured and it is possible to connect to - /// the database, but the database is empty, the runtime enters the Install level. - /// If this option is set to true an unattended install will be performed and the runtime enters - /// the Run level. - /// - public bool InstallUnattended { get; set; } = false; /// /// Gets or sets a value indicating whether to disable the election for a single server. /// diff --git a/src/Umbraco.Core/Configuration/Models/UnattendedSettings.cs b/src/Umbraco.Core/Configuration/Models/UnattendedSettings.cs new file mode 100644 index 0000000000..f8779d817c --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/UnattendedSettings.cs @@ -0,0 +1,38 @@ +using System.ComponentModel.DataAnnotations; + +namespace Umbraco.Cms.Core.Configuration.Models +{ + + /// + /// Typed configuration options for unattended settings. + /// + public class UnattendedSettings + { + /// + /// Gets or sets a value indicating whether unattended installs are enabled. + /// + /// + /// By default, when a database connection string is configured and it is possible to connect to + /// the database, but the database is empty, the runtime enters the Install level. + /// If this option is set to true an unattended install will be performed and the runtime enters + /// the Run level. + /// + public bool InstallUnattended { get; set; } = false; + + /// + /// Gets or sets a value to use for creating a user with a name for Unattended Installs + /// + public string UnattendedUserName { get; set; } = null; + + /// + /// Gets or sets a value to use for creating a user with an email for Unattended Installs + /// + [EmailAddress] + public string UnattendedUserEmail { get; set; } = null; + + /// + /// Gets or sets a value to use for creating a user with a password for Unattended Installs + /// + public string UnattendedUserPassword { get; set; } = null; + } +} diff --git a/src/Umbraco.Core/Configuration/Models/Validation/UnattendedSettingsValidator.cs b/src/Umbraco.Core/Configuration/Models/Validation/UnattendedSettingsValidator.cs new file mode 100644 index 0000000000..3c073ac100 --- /dev/null +++ b/src/Umbraco.Core/Configuration/Models/Validation/UnattendedSettingsValidator.cs @@ -0,0 +1,44 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Microsoft.Extensions.Options; + +namespace Umbraco.Cms.Core.Configuration.Models.Validation +{ + /// + /// Validator for configuration representated as . + /// + public class UnattendedSettingsValidator + : IValidateOptions + { + /// + public ValidateOptionsResult Validate(string name, UnattendedSettings options) + { + if (options.InstallUnattended) + { + int setValues = 0; + if (!string.IsNullOrEmpty(options.UnattendedUserName)) + { + setValues++; + } + + if (!string.IsNullOrEmpty(options.UnattendedUserEmail)) + { + setValues++; + } + + if (!string.IsNullOrEmpty(options.UnattendedUserPassword)) + { + setValues++; + } + + if (0 < setValues && setValues < 3) + { + return ValidateOptionsResult.Fail($"Configuration entry {Constants.Configuration.ConfigUnattended} contains invalid values.\nIf any of the {nameof(options.UnattendedUserName)}, {nameof(options.UnattendedUserEmail)}, {nameof(options.UnattendedUserPassword)} are set, all of them are required."); + } + } + + return ValidateOptionsResult.Success; + } + } +} diff --git a/src/Umbraco.Core/Constants-Configuration.cs b/src/Umbraco.Core/Constants-Configuration.cs index 6ad3e0fda0..0d62094dad 100644 --- a/src/Umbraco.Core/Constants-Configuration.cs +++ b/src/Umbraco.Core/Constants-Configuration.cs @@ -30,6 +30,7 @@ public const string ConfigCoreDebug = ConfigCorePrefix + "Debug"; public const string ConfigExceptionFilter = ConfigPrefix + "ExceptionFilter"; public const string ConfigGlobal = ConfigPrefix + "Global"; + public const string ConfigUnattended = ConfigPrefix + "Unattended"; public const string ConfigHealthChecks = ConfigPrefix + "HealthChecks"; public const string ConfigHosting = ConfigPrefix + "Hosting"; public const string ConfigImaging = ConfigPrefix + "Imaging"; diff --git a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs index f987e29eac..47a98ea9e1 100644 --- a/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs +++ b/src/Umbraco.Core/DependencyInjection/UmbracoBuilder.Configuration.cs @@ -10,6 +10,14 @@ namespace Umbraco.Cms.Core.DependencyInjection /// public static partial class UmbracoBuilderExtensions { + + private static OptionsBuilder AddOptions(IUmbracoBuilder builder, string key) + where TOptions : class + { + return builder.Services.AddOptions() + .Bind(builder.Config.GetSection(key)) + .ValidateDataAnnotations(); + } /// /// Add Umbraco configuration services and options /// @@ -20,31 +28,34 @@ namespace Umbraco.Cms.Core.DependencyInjection builder.Services.AddSingleton, GlobalSettingsValidator>(); builder.Services.AddSingleton, HealthChecksSettingsValidator>(); builder.Services.AddSingleton, RequestHandlerSettingsValidator>(); + builder.Services.AddSingleton, UnattendedSettingsValidator>(); // Register configuration sections. - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigActiveDirectory)); + builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigModelsBuilder), o => o.BindNonPublicProperties = true); builder.Services.Configure(builder.Config.GetSection("ConnectionStrings"), o => o.BindNonPublicProperties = true); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigContent)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigCoreDebug)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigExceptionFilter)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigGlobal)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigHealthChecks)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigHosting)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigImaging)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigExamine)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigKeepAlive)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigLogging)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigMemberPassword)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigModelsBuilder), o => o.BindNonPublicProperties = true); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigNuCache)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigRequestHandler)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigRuntime)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigSecurity)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigTours)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigTypeFinder)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigUserPassword)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigWebRouting)); - builder.Services.Configure(builder.Config.GetSection(Constants.Configuration.ConfigPlugins)); + + AddOptions(builder, Constants.Configuration.ConfigActiveDirectory); + AddOptions(builder, Constants.Configuration.ConfigContent); + AddOptions(builder, Constants.Configuration.ConfigCoreDebug); + AddOptions(builder, Constants.Configuration.ConfigExceptionFilter); + AddOptions(builder, Constants.Configuration.ConfigGlobal); + AddOptions(builder, Constants.Configuration.ConfigHealthChecks); + AddOptions(builder, Constants.Configuration.ConfigHosting); + AddOptions(builder, Constants.Configuration.ConfigImaging); + AddOptions(builder, Constants.Configuration.ConfigExamine); + AddOptions(builder, Constants.Configuration.ConfigKeepAlive); + AddOptions(builder, Constants.Configuration.ConfigLogging); + AddOptions(builder, Constants.Configuration.ConfigMemberPassword); + AddOptions(builder, Constants.Configuration.ConfigNuCache); + AddOptions(builder, Constants.Configuration.ConfigRequestHandler); + AddOptions(builder, Constants.Configuration.ConfigRuntime); + AddOptions(builder, Constants.Configuration.ConfigSecurity); + AddOptions(builder, Constants.Configuration.ConfigTours); + AddOptions(builder, Constants.Configuration.ConfigTypeFinder); + AddOptions(builder, Constants.Configuration.ConfigUserPassword); + AddOptions(builder, Constants.Configuration.ConfigWebRouting); + AddOptions(builder, Constants.Configuration.ConfigPlugins); + AddOptions(builder, Constants.Configuration.ConfigUnattended); return builder; } diff --git a/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs b/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs deleted file mode 100644 index dd2b4d0d58..0000000000 --- a/src/Umbraco.Core/Events/UmbracoRoutedRequest.cs +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System; -using Umbraco.Cms.Core.Web; -using Umbraco.Extensions; - -namespace Umbraco.Cms.Core.Events -{ - /// - /// Notification raised when Umbraco routes a front-end request. - /// - public class UmbracoRoutedRequest : INotification - { - /// - /// Initializes a new instance of the class. - /// - public UmbracoRoutedRequest(IUmbracoContext umbracoContext) - { - if (!umbracoContext.IsFrontEndUmbracoRequest()) - { - throw new InvalidOperationException($"{nameof(UmbracoRoutedRequest)} is only valid for Umbraco front-end requests"); - } - - UmbracoContext = umbracoContext; - } - - /// - /// Gets the - /// - public IUmbracoContext UmbracoContext { get; } - } -} diff --git a/src/Umbraco.Core/Events/UnattendedInstallNotification.cs b/src/Umbraco.Core/Events/UnattendedInstallNotification.cs new file mode 100644 index 0000000000..5bfb64e08f --- /dev/null +++ b/src/Umbraco.Core/Events/UnattendedInstallNotification.cs @@ -0,0 +1,12 @@ +using System; +using Umbraco.Cms.Core.Events; + +namespace Umbraco.Core.Events +{ + /// + /// Used to notify that an Unattended install has completed + /// + public class UnattendedInstallNotification : INotification + { + } +} diff --git a/src/Umbraco.Core/HybridAccessorBase.cs b/src/Umbraco.Core/HybridAccessorBase.cs index 51a51e0d01..530d1d5689 100644 --- a/src/Umbraco.Core/HybridAccessorBase.cs +++ b/src/Umbraco.Core/HybridAccessorBase.cs @@ -17,10 +17,6 @@ namespace Umbraco.Cms.Core where T : class { private readonly IRequestCache _requestCache; - - private readonly object _locker = new object(); - private readonly bool _registered; - private string _itemKey; protected string ItemKey => _itemKey ??= GetType().FullName; @@ -47,41 +43,7 @@ namespace Umbraco.Cms.Core } protected HybridAccessorBase(IRequestCache requestCache) - { - _requestCache = requestCache ?? throw new ArgumentNullException(nameof(requestCache)); - - lock (_locker) - { - // register the itemKey once with SafeCallContext - if (_registered) - { - return; - } - - _registered = true; - } - - // ReSharper disable once VirtualMemberCallInConstructor - var itemKey = ItemKey; // virtual - SafeCallContext.Register(() => - { - T value = CallContext.GetData(itemKey); - return value; - }, o => - { - if (o == null) - { - return; - } - - if (o is not T value) - { - throw new ArgumentException($"Expected type {typeof(T).FullName}, got {o.GetType().FullName}", nameof(o)); - } - - CallContext.SetData(itemKey, value); - }); - } + => _requestCache = requestCache ?? throw new ArgumentNullException(nameof(requestCache)); protected T Value { diff --git a/src/Umbraco.Core/IDisposeOnRequestEnd.cs b/src/Umbraco.Core/IDisposeOnRequestEnd.cs deleted file mode 100644 index 97df5793b9..0000000000 --- a/src/Umbraco.Core/IDisposeOnRequestEnd.cs +++ /dev/null @@ -1,11 +0,0 @@ -using System; - -namespace Umbraco.Cms.Core -{ - /// - /// Any class implementing this interface that is added to the httpcontext.items keys or values will be disposed of at the end of the request. - /// - public interface IDisposeOnRequestEnd : IDisposable - { - } -} diff --git a/src/Umbraco.Core/SafeCallContext.cs b/src/Umbraco.Core/SafeCallContext.cs deleted file mode 100644 index e15ee36e33..0000000000 --- a/src/Umbraco.Core/SafeCallContext.cs +++ /dev/null @@ -1,104 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; - -namespace Umbraco.Cms.Core -{ - /// - /// Provides a way to stop the data flow of a logical call context (i.e. CallContext or AsyncLocal) from within - /// a SafeCallContext and then have the original data restored to the current logical call context. - /// - /// - /// Some usages of this might be when spawning async thread or background threads in which the current logical call context will be flowed but - /// you don't want it to flow there yet you don't want to clear it either since you want the data to remain on the current thread. - /// - public class SafeCallContext : IDisposable - { - private static readonly List> EnterFuncs = new List>(); - private static readonly List> ExitActions = new List>(); - private static int _count; - private readonly List _objects; - private bool _disposed; - - public static void Register(Func enterFunc, Action exitAction) - { - if (enterFunc == null) throw new ArgumentNullException(nameof(enterFunc)); - if (exitAction == null) throw new ArgumentNullException(nameof(exitAction)); - - lock (EnterFuncs) - { - if (_count > 0) throw new InvalidOperationException("Cannot register while some SafeCallContext instances exist."); - EnterFuncs.Add(enterFunc); - ExitActions.Add(exitAction); - } - } - - // tried to make the UmbracoDatabase serializable but then it leaks to weird places - // in ReSharper and so on, where Umbraco.Core is not available. Tried to serialize - // as an object instead but then it comes *back* deserialized into the original context - // as an object and of course it breaks everything. Cannot prevent this from flowing, - // and ExecutionContext.SuppressFlow() works for threads but not domains. and we'll - // have the same issue with anything that toys with logical call context... - // - // so this class lets anything that uses the logical call context register itself, - // providing two methods: - // - an enter func that removes and returns whatever is in the logical call context - // - an exit action that restores the value into the logical call context - // whenever a SafeCallContext instance is created, it uses these methods to capture - // and clear the logical call context, and restore it when disposed. - // - // in addition, a static Clear method is provided - which uses the enter funcs to - // remove everything from logical call context - not to be used when the app runs, - // but can be useful during tests - // - // note - // see System.Transactions - // pre 4.5.1, the TransactionScope would not flow in async, and then introduced - // an option to store in the LLC so that it flows - // they are using a conditional weak table to store the data, and what they store in - // LLC is the key - which is just an empty MarshalByRefObject that is created with - // the transaction scope - that way, they can "clear current data" provided that - // they have the key - but they need to hold onto a ref to the scope... not ok for us - - public static void Clear() - { - lock (EnterFuncs) - { - foreach (var enter in EnterFuncs) - enter(); - } - } - - public SafeCallContext() - { - lock (EnterFuncs) - { - _count++; - _objects = EnterFuncs.Select(x => x()).ToList(); - } - } - - public void Dispose() - { - if (_disposed) throw new ObjectDisposedException("this"); - _disposed = true; - lock (EnterFuncs) - { - for (var i = 0; i < ExitActions.Count; i++) - ExitActions[i](_objects[i]); - _count--; - } - } - - // for unit tests ONLY - internal static void Reset() - { - lock (EnterFuncs) - { - if (_count > 0) throw new InvalidOperationException("Cannot reset while some SafeCallContext instances exist."); - EnterFuncs.Clear(); - ExitActions.Clear(); - } - } - } -} diff --git a/src/Umbraco.Core/Scoping/CallContext.cs b/src/Umbraco.Core/Scoping/CallContext.cs index d975d0e695..b77414ddd6 100644 --- a/src/Umbraco.Core/Scoping/CallContext.cs +++ b/src/Umbraco.Core/Scoping/CallContext.cs @@ -11,14 +11,16 @@ namespace Umbraco.Cms.Core.Scoping /// public static class CallContext { - private static ConcurrentDictionary> _state = new ConcurrentDictionary>(); + // TODO: Kill this. Wherever we need AsyncLocal, we should just use it there. + + private static readonly ConcurrentDictionary> s_state = new ConcurrentDictionary>(); /// /// Stores a given object and associates it with the specified name. /// /// The name with which to associate the new item in the call context. /// The object to store in the call context. - public static void SetData(string name, T data) => _state.GetOrAdd(name, _ => new AsyncLocal()).Value = data; + public static void SetData(string name, T data) => s_state.GetOrAdd(name, _ => new AsyncLocal()).Value = data; //Replace the SetData with the following when you need to debug AsyncLocal. The args.ThreadContextChanged can be usefull //public static void SetData(string name, T data) => _state.GetOrAdd(name, _ => new AsyncLocal(OnValueChanged)).Value = data; @@ -34,7 +36,7 @@ namespace Umbraco.Cms.Core.Scoping /// The type of the data being retrieved. Must match the type used when the was set via . /// The name of the item in the call context. /// The object in the call context associated with the specified name, or a default value for if none is found. - public static T GetData(string name) => _state.TryGetValue(name, out var data) ? data.Value : default; + public static T GetData(string name) => s_state.TryGetValue(name, out AsyncLocal data) ? data.Value : default; // NOTE: If you have used the old CallContext in the past you might be thinking you need to clean this up but that is not the case. // With CallContext you had to call FreeNamedDataSlot to prevent leaks but with AsyncLocal this is not the case, there is no way to clean this up. diff --git a/src/Umbraco.Core/Scoping/IInstanceIdentifiable.cs b/src/Umbraco.Core/Scoping/IInstanceIdentifiable.cs index a8d9f92f4a..9d0bc9ceef 100644 --- a/src/Umbraco.Core/Scoping/IInstanceIdentifiable.cs +++ b/src/Umbraco.Core/Scoping/IInstanceIdentifiable.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace Umbraco.Cms.Core.Scoping { @@ -11,5 +11,6 @@ namespace Umbraco.Cms.Core.Scoping /// Gets the instance unique identifier. /// Guid InstanceId { get; } + int CreatedThreadId { get; } } } diff --git a/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs b/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs deleted file mode 100644 index 924f0a31a6..0000000000 --- a/src/Umbraco.Core/Security/HybridBackofficeSecurityAccessor.cs +++ /dev/null @@ -1,23 +0,0 @@ -using Umbraco.Cms.Core.Cache; - -namespace Umbraco.Cms.Core.Security -{ - public class HybridBackofficeSecurityAccessor : HybridAccessorBase, IBackOfficeSecurityAccessor - { - /// - /// Initializes a new instance of the class. - /// - public HybridBackofficeSecurityAccessor(IRequestCache requestCache) - : base(requestCache) - { } - - /// - /// Gets or sets the object. - /// - public IBackOfficeSecurity BackOfficeSecurity - { - get => Value; - set => Value = value; - } - } -} diff --git a/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs deleted file mode 100644 index 3145f400d1..0000000000 --- a/src/Umbraco.Core/Security/HybridUmbracoWebsiteSecurityAccessor.cs +++ /dev/null @@ -1,24 +0,0 @@ -using Umbraco.Cms.Core.Cache; - -namespace Umbraco.Cms.Core.Security -{ - - public class HybridUmbracoWebsiteSecurityAccessor : HybridAccessorBase, IUmbracoWebsiteSecurityAccessor - { - /// - /// Initializes a new instance of the class. - /// - public HybridUmbracoWebsiteSecurityAccessor(IRequestCache requestCache) - : base(requestCache) - { } - - /// - /// Gets or sets the object. - /// - public IUmbracoWebsiteSecurity WebsiteSecurity - { - get => Value; - set => Value = value; - } - } -} diff --git a/src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs b/src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs deleted file mode 100644 index ee553e85e6..0000000000 --- a/src/Umbraco.Core/Security/IBackOfficeSecurityFactory.cs +++ /dev/null @@ -1,13 +0,0 @@ -namespace Umbraco.Cms.Core.Security -{ - /// - /// Creates and manages instances. - /// - public interface IBackOfficeSecurityFactory - { - /// - /// Ensures that a current exists. - /// - void EnsureBackOfficeSecurity(); - } -} diff --git a/src/Umbraco.Core/Security/IBackofficeSecurityAccessor.cs b/src/Umbraco.Core/Security/IBackofficeSecurityAccessor.cs index dbc64f40c7..2999ceacf4 100644 --- a/src/Umbraco.Core/Security/IBackofficeSecurityAccessor.cs +++ b/src/Umbraco.Core/Security/IBackofficeSecurityAccessor.cs @@ -1,7 +1,7 @@ -namespace Umbraco.Cms.Core.Security +namespace Umbraco.Cms.Core.Security { public interface IBackOfficeSecurityAccessor { - IBackOfficeSecurity BackOfficeSecurity { get; set; } + IBackOfficeSecurity BackOfficeSecurity { get; } } } diff --git a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs index d05d84476c..3d20a7656f 100644 --- a/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs +++ b/src/Umbraco.Core/Security/IUmbracoWebsiteSecurityAccessor.cs @@ -1,7 +1,7 @@ -namespace Umbraco.Cms.Core.Security +namespace Umbraco.Cms.Core.Security { public interface IUmbracoWebsiteSecurityAccessor { - IUmbracoWebsiteSecurity WebsiteSecurity { get; set; } + IUmbracoWebsiteSecurity WebsiteSecurity { get; } } } diff --git a/src/Umbraco.Core/TaskHelper.cs b/src/Umbraco.Core/TaskHelper.cs index 113327ed88..ba9f865eba 100644 --- a/src/Umbraco.Core/TaskHelper.cs +++ b/src/Umbraco.Core/TaskHelper.cs @@ -1,7 +1,9 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; +using System.Runtime.CompilerServices; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -10,32 +12,55 @@ namespace Umbraco.Cms.Core /// /// Helper class to not repeat common patterns with Task. /// - public class TaskHelper + public sealed class TaskHelper { private readonly ILogger _logger; - public TaskHelper(ILogger logger) + public TaskHelper(ILogger logger) => _logger = logger; + + /// + /// Executes a fire and forget task outside of the current execution flow. + /// + public void RunBackgroundTask(Func fn) => ExecuteBackgroundTask(fn); + + // for tests, returning the Task as a public API indicates it can be awaited that is not what we want to do + internal Task ExecuteBackgroundTask(Func fn) { - _logger = logger; + // it is also possible to use UnsafeQueueUserWorkItem which does not flow the execution context, + // however that seems more difficult to use for async operations. + + // Do not flow AsyncLocal to the child thread + using (ExecutionContext.SuppressFlow()) + { + // NOTE: ConfigureAwait(false) is irrelevant here, it is not needed because this is not being + // awaited. ConfigureAwait(false) is only relevant when awaiting to prevent the SynchronizationContext + // (very different from the ExecutionContext!) from running the continuation on the calling thread. + return Task.Run(LoggingWrapper(fn)); + } } /// - /// Runs a TPL Task fire-and-forget style, the right way - in the - /// background, separate from the current thread, with no risk - /// of it trying to rejoin the current thread. + /// Executes a fire and forget task outside of the current execution flow on a dedicated (non thread-pool) thread. /// - public void RunBackgroundTask(Func fn) => Task.Run(LoggingWrapper(fn)).ConfigureAwait(false); + public void RunLongRunningBackgroundTask(Func fn) => ExecuteLongRunningBackgroundTask(fn); - /// - /// Runs a task fire-and-forget style and notifies the TPL that this - /// will not need a Thread to resume on for a long time, or that there - /// are multiple gaps in thread use that may be long. - /// Use for example when talking to a slow webservice. - /// - public void RunLongRunningBackgroundTask(Func fn) => - Task.Factory.StartNew(LoggingWrapper(fn), TaskCreationOptions.LongRunning) - .ConfigureAwait(false); + // for tests, returning the Task as a public API indicates it can be awaited that is not what we want to do + internal Task ExecuteLongRunningBackgroundTask(Func fn) + { + // it is also possible to use UnsafeQueueUserWorkItem which does not flow the execution context, + // however that seems more difficult to use for async operations. + // Do not flow AsyncLocal to the child thread + using (ExecutionContext.SuppressFlow()) + { + // NOTE: ConfigureAwait(false) is irrelevant here, it is not needed because this is not being + // awaited. ConfigureAwait(false) is only relevant when awaiting to prevent the SynchronizationContext + // (very different from the ExecutionContext!) from running the continuation on the calling thread. + return Task.Factory.StartNew(LoggingWrapper(fn), TaskCreationOptions.LongRunning); + } + } + + // ensure any exceptions are handled and do not take down the app pool private Func LoggingWrapper(Func fn) => async () => { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 1166bc1270..be36173981 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -16,6 +16,7 @@ + diff --git a/src/Umbraco.Examine.Lucene/Umbraco.Examine.Lucene.csproj b/src/Umbraco.Examine.Lucene/Umbraco.Examine.Lucene.csproj index d0419abe6b..43f06ceef3 100644 --- a/src/Umbraco.Examine.Lucene/Umbraco.Examine.Lucene.csproj +++ b/src/Umbraco.Examine.Lucene/Umbraco.Examine.Lucene.csproj @@ -39,7 +39,7 @@ - 3.5.3 + 3.5.4 runtime; build; native; contentfiles; analyzers all diff --git a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs index 16806c530a..18b9945a6e 100644 --- a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs +++ b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs @@ -1,4 +1,4 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; diff --git a/src/Umbraco.Examine.Lucene/UmbracoExamineIndex.cs b/src/Umbraco.Examine.Lucene/UmbracoExamineIndex.cs index 3dc3176c11..a5c98cdeba 100644 --- a/src/Umbraco.Examine.Lucene/UmbracoExamineIndex.cs +++ b/src/Umbraco.Examine.Lucene/UmbracoExamineIndex.cs @@ -1,9 +1,10 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using Examine; using Examine.LuceneEngine; using Examine.LuceneEngine.Providers; @@ -30,12 +31,12 @@ namespace Umbraco.Cms.Infrastructure.Examine private readonly ILoggerFactory _loggerFactory; private readonly IRuntimeState _runtimeState; + // note // wrapping all operations that end up calling base.SafelyProcessQueueItems in a safe call // context because they will fork a thread/task/whatever which should *not* capture our - // call context (and the database it can contain)! ideally we should be able to override - // SafelyProcessQueueItems but that's not possible in the current version of Examine. - + // call context (and the database it can contain)! + // TODO: FIX Examine to not flow the ExecutionContext so callers don't need to worry about this! /// /// Create a new @@ -92,7 +93,7 @@ namespace Umbraco.Cms.Infrastructure.Examine public IEnumerable GetFields() { //we know this is a LuceneSearcher - var searcher = (LuceneSearcher) GetSearcher(); + var searcher = (LuceneSearcher)GetSearcher(); return searcher.GetAllIndexedFields(); } @@ -106,13 +107,30 @@ namespace Umbraco.Cms.Infrastructure.Examine { if (CanInitialize()) { - using (new SafeCallContext()) + // Use ExecutionContext.SuppressFlow to prevent the current Execution Context (AsyncLocal) flow to child + // tasks executed in the base class so we don't leak Scopes. + // TODO: See notes at the top of this class + using (ExecutionContext.SuppressFlow()) { base.PerformDeleteFromIndex(itemIds, onComplete); } } } + protected override void PerformIndexItems(IEnumerable values, Action onComplete) + { + if (CanInitialize()) + { + // Use ExecutionContext.SuppressFlow to prevent the current Execution Context (AsyncLocal) flow to child + // tasks executed in the base class so we don't leak Scopes. + // TODO: See notes at the top of this class + using (ExecutionContext.SuppressFlow()) + { + base.PerformIndexItems(values, onComplete); + } + } + } + /// /// Returns true if the Umbraco application is in a state that we can initialize the examine indexes /// @@ -167,9 +185,9 @@ namespace Umbraco.Cms.Infrastructure.Examine protected override void AddDocument(Document doc, ValueSet valueSet, IndexWriter writer) { _logger.LogDebug("Write lucene doc id:{DocumentId}, category:{DocumentCategory}, type:{DocumentItemType}", - valueSet.Id, - valueSet.Category, - valueSet.ItemType); + valueSet.Id, + valueSet.Category, + valueSet.ItemType); base.AddDocument(doc, valueSet, writer); } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index f42e88b7df..e97367b804 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -77,6 +77,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection builder.Services.AddUnique(); // implements both IScopeProvider and IScopeAccessor builder.Services.AddUnique(f => f.GetRequiredService()); builder.Services.AddUnique(f => f.GetRequiredService()); + builder.Services.AddScoped(); builder.Services.AddUnique(); builder.Services.AddUnique(); diff --git a/src/Umbraco.Infrastructure/Extensions/InstanceIdentifiableExtensions.cs b/src/Umbraco.Infrastructure/Extensions/InstanceIdentifiableExtensions.cs new file mode 100644 index 0000000000..10f919589a --- /dev/null +++ b/src/Umbraco.Infrastructure/Extensions/InstanceIdentifiableExtensions.cs @@ -0,0 +1,20 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Umbraco.Cms.Core.Scoping; + +namespace Umbraco.Extensions +{ + internal static class InstanceIdentifiableExtensions + { + public static string GetDebugInfo(this IInstanceIdentifiable instance) + { + if (instance == null) + { + return "(NULL)"; + } + + return $"(id: {instance.InstanceId.ToString("N").Substring(0, 8)} from thread: {instance.CreatedThreadId})"; + } + } +} diff --git a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs index 0fc1809250..ff93940c61 100644 --- a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs +++ b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs @@ -27,7 +27,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices private readonly IMainDom _mainDom; private readonly IRuntimeState _runtimeState; private readonly IServerMessenger _serverMessenger; - private readonly IBackOfficeSecurityFactory _backofficeSecurityFactory; private readonly IServerRoleAccessor _serverRegistrar; private readonly IUmbracoContextFactory _umbracoContextFactory; @@ -49,8 +48,7 @@ namespace Umbraco.Cms.Infrastructure.HostedServices IContentService contentService, IUmbracoContextFactory umbracoContextFactory, ILogger logger, - IServerMessenger serverMessenger, - IBackOfficeSecurityFactory backofficeSecurityFactory) + IServerMessenger serverMessenger) : base(TimeSpan.FromMinutes(1), DefaultDelay) { _runtimeState = runtimeState; @@ -60,7 +58,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices _umbracoContextFactory = umbracoContextFactory; _logger = logger; _serverMessenger = serverMessenger; - _backofficeSecurityFactory = backofficeSecurityFactory; } internal override Task PerformExecuteAsync(object state) @@ -107,11 +104,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices // but then what should be its "scope"? could we attach it to scopes? // - and we should definitively *not* have to flush it here (should be auto) - // TODO: This dependency chain is broken and needs to be fixed. - // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null - // This is a very ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the - // way it was intended. - _backofficeSecurityFactory.EnsureBackOfficeSecurity(); using UmbracoContextReference contextReference = _umbracoContextFactory.EnsureUmbracoContext(); try { diff --git a/src/Umbraco.Infrastructure/Persistence/DatabaseDebugHelper.cs b/src/Umbraco.Infrastructure/Persistence/DatabaseDebugHelper.cs index f6f3d8d802..f54691994e 100644 --- a/src/Umbraco.Infrastructure/Persistence/DatabaseDebugHelper.cs +++ b/src/Umbraco.Infrastructure/Persistence/DatabaseDebugHelper.cs @@ -1,4 +1,4 @@ -#if DEBUG_DATABASES +#if DEBUG_DATABASES using System; using System.Collections.Generic; using System.Data; @@ -9,7 +9,7 @@ using System.Reflection; using System.Text; using Umbraco.Core.Persistence.FaultHandling; -namespace Umbraco.Core.Persistence +namespace Umbraco.Cms.Core.Persistence { internal static class DatabaseDebugHelper { diff --git a/src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs b/src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs index 58a283a142..279ab1215f 100644 --- a/src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs +++ b/src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlServerSyntaxProvider.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data; using System.Data.SqlClient; @@ -263,10 +263,22 @@ where tbl.[name]=@0 and col.[name]=@1;", tableName, columnName) public void WriteLock(IDatabase db, TimeSpan timeout, params int[] lockIds) { + if (db is null) + { + throw new ArgumentNullException(nameof(db)); + } + + if (db.Transaction is null) + { + throw new ArgumentException(nameof(db) + "." + nameof(db.Transaction) + " is null"); + } + // soon as we get Database, a transaction is started if (db.Transaction.IsolationLevel < IsolationLevel.ReadCommitted) + { throw new InvalidOperationException("A transaction with minimum ReadCommitted isolation level is required."); + } // *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks @@ -275,24 +287,40 @@ where tbl.[name]=@0 and col.[name]=@1;", tableName, columnName) db.Execute($"SET LOCK_TIMEOUT {timeout.TotalMilliseconds};"); var i = db.Execute(@"UPDATE umbracoLock WITH (REPEATABLEREAD) SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { id = lockId }); if (i == 0) // ensure we are actually locking! + { throw new ArgumentException($"LockObject with id={lockId} does not exist."); + } } } public override void ReadLock(IDatabase db, params int[] lockIds) { + if (db is null) + { + throw new ArgumentNullException(nameof(db)); + } + + if (db.Transaction is null) + { + throw new ArgumentException(nameof(db) + "." + nameof(db.Transaction) + " is null"); + } + // soon as we get Database, a transaction is started if (db.Transaction.IsolationLevel < IsolationLevel.ReadCommitted) + { throw new InvalidOperationException("A transaction with minimum ReadCommitted isolation level is required."); + } // *not* using a unique 'WHERE IN' query here because the *order* of lockIds is important to avoid deadlocks foreach (var lockId in lockIds) { var i = db.ExecuteScalar("SELECT value FROM umbracoLock WITH (REPEATABLEREAD) WHERE id=@id", new { id = lockId }); if (i == null) // ensure we are actually locking! + { throw new ArgumentException($"LockObject with id={lockId} does not exist.", nameof(lockIds)); + } } } diff --git a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs index e8f5072e18..63828715fd 100644 --- a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Data; using System.Data.SqlClient; using System.Diagnostics; @@ -254,37 +254,40 @@ _hostingEnvironment = hostingEnvironment; { var updatedTempId = tempId + UpdatedSuffix; - return Task.Run(() => + using (ExecutionContext.SuppressFlow()) { - try + return Task.Run(() => { - using var db = _dbFactory.CreateDatabase(); - - var watch = new Stopwatch(); - watch.Start(); - while (true) + try { - // poll very often, we need to take over as fast as we can - // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO - Thread.Sleep(1000); + using var db = _dbFactory.CreateDatabase(); - var acquired = TryAcquire(db, tempId, updatedTempId); - if (acquired.HasValue) - return acquired.Value; - - if (watch.ElapsedMilliseconds >= millisecondsTimeout) + var watch = new Stopwatch(); + watch.Start(); + while (true) { - return AcquireWhenMaxWaitTimeElapsed(db); + // poll very often, we need to take over as fast as we can + // local testing shows the actual query to be executed from client/server is approx 300ms but would change depending on environment/IO + Thread.Sleep(1000); + + var acquired = TryAcquire(db, tempId, updatedTempId); + if (acquired.HasValue) + return acquired.Value; + + if (watch.ElapsedMilliseconds >= millisecondsTimeout) + { + return AcquireWhenMaxWaitTimeElapsed(db); + } } } - } - catch (Exception ex) - { - _logger.LogError(ex, "An error occurred trying to acquire and waiting for existing SqlMainDomLock to shutdown"); - return false; - } + catch (Exception ex) + { + _logger.LogError(ex, "An error occurred trying to acquire and waiting for existing SqlMainDomLock to shutdown"); + return false; + } - }, _cancellationTokenSource.Token); + }, _cancellationTokenSource.Token); + } } private bool? TryAcquire(IUmbracoDatabase db, string tempId, string updatedTempId) diff --git a/src/Umbraco.Infrastructure/RuntimeState.cs b/src/Umbraco.Infrastructure/RuntimeState.cs index b62c30e4d2..fc8f5f3912 100644 --- a/src/Umbraco.Infrastructure/RuntimeState.cs +++ b/src/Umbraco.Infrastructure/RuntimeState.cs @@ -2,15 +2,16 @@ using System; using System.Threading; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; -using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Exceptions; using Umbraco.Cms.Core.Semver; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Migrations.Install; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Core.Events; namespace Umbraco.Cms.Core { @@ -19,11 +20,13 @@ namespace Umbraco.Cms.Core /// public class RuntimeState : IRuntimeState { - private readonly GlobalSettings _globalSettings; + private readonly IOptions _globalSettings; + private readonly IOptions _unattendedSettings; private readonly IUmbracoVersion _umbracoVersion; private readonly IUmbracoDatabaseFactory _databaseFactory; private readonly ILogger _logger; private readonly DatabaseSchemaCreatorFactory _databaseSchemaCreatorFactory; + private readonly IEventAggregator _eventAggregator; /// /// The initial @@ -39,16 +42,20 @@ namespace Umbraco.Cms.Core /// public RuntimeState( IOptions globalSettings, + IOptions unattendedSettings, IUmbracoVersion umbracoVersion, IUmbracoDatabaseFactory databaseFactory, ILogger logger, - DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory) + DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory, + IEventAggregator eventAggregator) { - _globalSettings = globalSettings.Value; + _globalSettings = globalSettings; + _unattendedSettings = unattendedSettings; _umbracoVersion = umbracoVersion; _databaseFactory = databaseFactory; _logger = logger; _databaseSchemaCreatorFactory = databaseSchemaCreatorFactory; + _eventAggregator = eventAggregator; } @@ -98,7 +105,7 @@ namespace Umbraco.Cms.Core // cannot connect to configured database, this is bad, fail _logger.LogDebug("Could not connect to database."); - if (_globalSettings.InstallMissingDatabase) + if (_globalSettings.Value.InstallMissingDatabase) { // ok to install on a configured but missing database Level = RuntimeLevel.Install; @@ -197,13 +204,13 @@ namespace Umbraco.Cms.Core public void DoUnattendedInstall() { // unattended install is not enabled - if (_globalSettings.InstallUnattended == false) return; + if (_unattendedSettings.Value.InstallUnattended == false) return; // no connection string set if (_databaseFactory.Configured == false) return; var connect = false; - var tries = _globalSettings.InstallMissingDatabase ? 2 : 5; + var tries = _globalSettings.Value.InstallMissingDatabase ? 2 : 5; for (var i = 0;;) { connect = _databaseFactory.CanConnect; @@ -232,6 +239,11 @@ namespace Umbraco.Cms.Core creator.InitializeDatabaseSchema(); database.CompleteTransaction(); _logger.LogInformation("Unattended install completed."); + + // Emit an event with EventAggregator that unattended install completed + // Then this event can be listened for and create an unattended user + _eventAggregator.Publish(new UnattendedInstallNotification()); + } catch (Exception ex) { @@ -279,7 +291,7 @@ namespace Umbraco.Cms.Core // anything other than install wants a database - see if we can connect // (since this is an already existing database, assume localdb is ready) bool canConnect; - var tries = _globalSettings.InstallMissingDatabase ? 2 : 5; + var tries = _globalSettings.Value.InstallMissingDatabase ? 2 : 5; for (var i = 0; ;) { canConnect = databaseFactory.CanConnect; diff --git a/src/Umbraco.Infrastructure/Scoping/HttpScopeReference.cs b/src/Umbraco.Infrastructure/Scoping/HttpScopeReference.cs new file mode 100644 index 0000000000..1e7185a961 --- /dev/null +++ b/src/Umbraco.Infrastructure/Scoping/HttpScopeReference.cs @@ -0,0 +1,48 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +namespace Umbraco.Cms.Core.Scoping +{ + + /// + /// Disposed at the end of the request to cleanup any orphaned Scopes. + /// + /// Registered as Scoped in DI (per request) + internal class HttpScopeReference : IHttpScopeReference + { + private readonly ScopeProvider _scopeProvider; + private bool _disposedValue; + private bool _registered = false; + + public HttpScopeReference(ScopeProvider scopeProvider) => _scopeProvider = scopeProvider; + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + if (_registered) + { + // dispose the entire chain (if any) + // reset (don't commit by default) + Scope scope; + while ((scope = _scopeProvider.AmbientScope) != null) + { + scope.Reset(); + scope.Dispose(); + } + } + } + + _disposedValue = true; + } + } + + public void Dispose() => + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + + public void Register() => _registered = true; + } +} diff --git a/src/Umbraco.Infrastructure/Scoping/IHttpScopeReference.cs b/src/Umbraco.Infrastructure/Scoping/IHttpScopeReference.cs new file mode 100644 index 0000000000..bd98834ca2 --- /dev/null +++ b/src/Umbraco.Infrastructure/Scoping/IHttpScopeReference.cs @@ -0,0 +1,18 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using System; + +namespace Umbraco.Cms.Core.Scoping +{ + /// + /// Cleans up orphaned references at the end of a request + /// + public interface IHttpScopeReference : IDisposable + { + /// + /// Register for cleanup in the request + /// + void Register(); + } +} diff --git a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs index 06cf16d221..12c081ef81 100644 --- a/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/IScopeProvider.cs @@ -51,6 +51,7 @@ namespace Umbraco.Cms.Core.Scoping /// A detached scope is not ambient and has no parent. /// It is meant to be attached by . /// + // TODO: This is not actually used apart from unit tests - I'm assuming it's maybe used by Deploy? IScope CreateDetachedScope( IsolationLevel isolationLevel = IsolationLevel.Unspecified, RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified, diff --git a/src/Umbraco.Infrastructure/Scoping/Scope.cs b/src/Umbraco.Infrastructure/Scoping/Scope.cs index 7d50f5e55a..603a83c197 100644 --- a/src/Umbraco.Infrastructure/Scoping/Scope.cs +++ b/src/Umbraco.Infrastructure/Scoping/Scope.cs @@ -1,11 +1,12 @@ using System; using System.Data; +using System.Threading; using Microsoft.Extensions.Logging; +using Umbraco.Extensions; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Infrastructure.Persistence; -using Umbraco.Extensions; using CoreDebugSettings = Umbraco.Cms.Core.Configuration.Models.CoreDebugSettings; namespace Umbraco.Cms.Core.Scoping @@ -71,21 +72,34 @@ namespace Umbraco.Cms.Core.Scoping #if DEBUG_SCOPES _scopeProvider.RegisterScope(this); - Console.WriteLine("create " + InstanceId.ToString("N").Substring(0, 8)); #endif + logger.LogTrace("Create {InstanceId} on thread {ThreadId}", InstanceId.ToString("N").Substring(0, 8), Thread.CurrentThread.ManagedThreadId); if (detachable) { - if (parent != null) throw new ArgumentException("Cannot set parent on detachable scope.", nameof(parent)); - if (scopeContext != null) throw new ArgumentException("Cannot set context on detachable scope.", nameof(scopeContext)); - if (autoComplete) throw new ArgumentException("Cannot auto-complete a detachable scope.", nameof(autoComplete)); + if (parent != null) + { + throw new ArgumentException("Cannot set parent on detachable scope.", nameof(parent)); + } + + if (scopeContext != null) + { + throw new ArgumentException("Cannot set context on detachable scope.", nameof(scopeContext)); + } + + if (autoComplete) + { + throw new ArgumentException("Cannot auto-complete a detachable scope.", nameof(autoComplete)); + } // detachable creates its own scope context Context = new ScopeContext(); // see note below if (scopeFileSystems == true) + { _fscope = fileSystems.Shadow(); + } return; } @@ -98,16 +112,22 @@ namespace Umbraco.Cms.Core.Scoping // TODO: means that it's OK to go from L2 to None for reading purposes, but writing would be BAD! // this is for XmlStore that wants to bypass caches when rebuilding XML (same for NuCache) if (repositoryCacheMode != RepositoryCacheMode.Unspecified && parent.RepositoryCacheMode > repositoryCacheMode) + { throw new ArgumentException($"Value '{repositoryCacheMode}' cannot be lower than parent value '{parent.RepositoryCacheMode}'.", nameof(repositoryCacheMode)); + } // cannot specify a dispatcher! if (_eventDispatcher != null) + { throw new ArgumentException("Value cannot be specified on nested scope.", nameof(eventDispatcher)); + } // cannot specify a different fs scope! // can be 'true' only on outer scope (and false does not make much sense) if (scopeFileSystems != null && parent._scopeFileSystem != scopeFileSystems) + { throw new ArgumentException($"Value '{scopeFileSystems.Value}' be different from parent value '{parent._scopeFileSystem}'.", nameof(scopeFileSystems)); + } } else { @@ -115,7 +135,9 @@ namespace Umbraco.Cms.Core.Scoping // every scoped FS to trigger the creation of shadow FS "on demand", and that would be // pretty pointless since if scopeFileSystems is true, we *know* we want to shadow if (scopeFileSystems == true) + { _fscope = fileSystems.Shadow(); + } } } @@ -156,6 +178,8 @@ namespace Umbraco.Cms.Core.Scoping public Guid InstanceId { get; } = Guid.NewGuid(); + public int CreatedThreadId { get; } = Thread.CurrentThread.ManagedThreadId; + public ISqlContext SqlContext => _scopeProvider.SqlContext; // a value indicating whether to force call-context @@ -163,8 +187,16 @@ namespace Umbraco.Cms.Core.Scoping { get { - if (_callContext) return true; - if (ParentScope != null) return ParentScope.CallContext; + if (_callContext) + { + return true; + } + + if (ParentScope != null) + { + return ParentScope.CallContext; + } + return false; } set => _callContext = value; @@ -174,7 +206,11 @@ namespace Umbraco.Cms.Core.Scoping { get { - if (ParentScope != null) return ParentScope.ScopedFileSystems; + if (ParentScope != null) + { + return ParentScope.ScopedFileSystems; + } + return _fscope != null; } } @@ -184,8 +220,16 @@ namespace Umbraco.Cms.Core.Scoping { get { - if (_repositoryCacheMode != RepositoryCacheMode.Unspecified) return _repositoryCacheMode; - if (ParentScope != null) return ParentScope.RepositoryCacheMode; + if (_repositoryCacheMode != RepositoryCacheMode.Unspecified) + { + return _repositoryCacheMode; + } + + if (ParentScope != null) + { + return ParentScope.RepositoryCacheMode; + } + return RepositoryCacheMode.Default; } } @@ -195,10 +239,12 @@ namespace Umbraco.Cms.Core.Scoping { get { - if (ParentScope != null) return ParentScope.IsolatedCaches; + if (ParentScope != null) + { + return ParentScope.IsolatedCaches; + } - return _isolatedCaches ?? (_isolatedCaches - = new IsolatedCaches(type => new DeepCloneAppCache(new ObjectCacheAppCache()))); + return _isolatedCaches ??= new IsolatedCaches(type => new DeepCloneAppCache(new ObjectCacheAppCache())); } } @@ -224,8 +270,16 @@ namespace Umbraco.Cms.Core.Scoping { get { - if (_isolationLevel != IsolationLevel.Unspecified) return _isolationLevel; - if (ParentScope != null) return ParentScope.IsolationLevel; + if (_isolationLevel != IsolationLevel.Unspecified) + { + return _isolationLevel; + } + + if (ParentScope != null) + { + return ParentScope.IsolationLevel; + } + return Database.SqlContext.SqlSyntax.DefaultIsolationLevel; } } @@ -238,14 +292,19 @@ namespace Umbraco.Cms.Core.Scoping EnsureNotDisposed(); if (_database != null) + { return _database; + } if (ParentScope != null) { - var database = ParentScope.Database; - var currentLevel = database.GetCurrentTransactionIsolationLevel(); + IUmbracoDatabase database = ParentScope.Database; + IsolationLevel currentLevel = database.GetCurrentTransactionIsolationLevel(); if (_isolationLevel > IsolationLevel.Unspecified && currentLevel < _isolationLevel) + { throw new Exception("Scope requires isolation level " + _isolationLevel + ", but got " + currentLevel + " from parent."); + } + return _database = database; } @@ -282,8 +341,12 @@ namespace Umbraco.Cms.Core.Scoping get { EnsureNotDisposed(); - if (ParentScope != null) return ParentScope.Messages; - return _messages ?? (_messages = new EventMessages()); + if (ParentScope != null) + { + return ParentScope.Messages; + } + + return _messages ??= new EventMessages(); // TODO: event messages? // this may be a problem: the messages collection will be cleared at the end of the scope @@ -309,8 +372,12 @@ namespace Umbraco.Cms.Core.Scoping get { EnsureNotDisposed(); - if (ParentScope != null) return ParentScope.Events; - return _eventDispatcher ?? (_eventDispatcher = new QueuingEventDispatcher(_mediaFileSystem)); + if (ParentScope != null) + { + return ParentScope.Events; + } + + return _eventDispatcher ??= new QueuingEventDispatcher(_mediaFileSystem); } } @@ -318,14 +385,14 @@ namespace Umbraco.Cms.Core.Scoping public bool Complete() { if (_completed.HasValue == false) + { _completed = true; + } + return _completed.Value; } - public void Reset() - { - _completed = null; - } + public void Reset() => _completed = null; public void ChildCompleted(bool? completed) { @@ -333,7 +400,9 @@ namespace Umbraco.Cms.Core.Scoping if (completed.HasValue == false || completed.Value == false) { if (LogUncompletedScopes) - _logger.LogDebug("Uncompleted Child Scope at\r\n {StackTrace}", Environment.StackTrace); + { + _logger.LogWarning("Uncompleted Child Scope at\r\n {StackTrace}", Environment.StackTrace); + } _completed = false; } @@ -341,8 +410,17 @@ namespace Umbraco.Cms.Core.Scoping private void EnsureNotDisposed() { + // We can't be disposed if (_disposed) - throw new ObjectDisposedException(GetType().FullName); + { + throw new ObjectDisposedException($"The {nameof(Scope)} ({this.GetDebugInfo()}) is already disposed"); + } + + // And neither can our ancestors if we're trying to be disposed since + // a child must always be disposed before it's parent. + // This is a safety check, it's actually not entirely possible that a parent can be + // disposed before the child since that will end up with a "not the Ambient" exception. + ParentScope?.EnsureNotDisposed(); // TODO: safer? //if (Interlocked.CompareExchange(ref _disposed, 1, 0) != 0) @@ -355,38 +433,49 @@ namespace Umbraco.Cms.Core.Scoping if (this != _scopeProvider.AmbientScope) { + var failedMessage = $"The {nameof(Scope)} {this.GetDebugInfo()} being disposed is not the Ambient {nameof(Scope)} {_scopeProvider.AmbientScope.GetDebugInfo()}. This typically indicates that a child {nameof(Scope)} was not disposed, or flowed to a child thread that was not awaited, or concurrent threads are accessing the same {nameof(Scope)} (Ambient context) which is not supported. If using Task.Run (or similar) as a fire and forget tasks or to run threads in parallel you must suppress execution context flow with ExecutionContext.SuppressFlow() and ExecutionContext.RestoreFlow()."; + #if DEBUG_SCOPES - var ambient = _scopeProvider.AmbientScope; - _logger.Debug("Dispose error (" + (ambient == null ? "no" : "other") + " ambient)"); + Scope ambient = _scopeProvider.AmbientScope; + _logger.LogWarning("Dispose error (" + (ambient == null ? "no" : "other") + " ambient)"); if (ambient == null) + { throw new InvalidOperationException("Not the ambient scope (no ambient scope)."); - var ambientInfos = _scopeProvider.GetScopeInfo(ambient); - var disposeInfos = _scopeProvider.GetScopeInfo(this); - throw new InvalidOperationException("Not the ambient scope (see ctor stack traces).\r\n" - + "- ambient ctor ->\r\n" + ambientInfos.CtorStack + "\r\n" - + "- dispose ctor ->\r\n" + disposeInfos.CtorStack + "\r\n"); + } + + ScopeInfo ambientInfos = _scopeProvider.GetScopeInfo(ambient); + ScopeInfo disposeInfos = _scopeProvider.GetScopeInfo(this); + throw new InvalidOperationException($"{failedMessage} (see ctor stack traces).\r\n" + + "- ambient ->\r\n" + ambientInfos.ToString() + "\r\n" + + "- dispose ->\r\n" + disposeInfos.ToString() + "\r\n"); #else - throw new InvalidOperationException("Not the ambient scope."); + throw new InvalidOperationException(failedMessage); #endif } - var parent = ParentScope; - _scopeProvider.AmbientScope = parent; // might be null = this is how scopes are removed from context objects + // Replace the Ambient scope with the parent + Scope parent = ParentScope; + _scopeProvider.PopAmbientScope(this); // pop, the parent is on the stack so is now current #if DEBUG_SCOPES _scopeProvider.Disposed(this); #endif if (_autoComplete && _completed == null) + { _completed = true; + } if (parent != null) + { parent.ChildCompleted(_completed); + } else + { DisposeLastScope(); + } _disposed = true; - GC.SuppressFinalize(this); } private void DisposeLastScope() @@ -401,9 +490,13 @@ namespace Umbraco.Cms.Core.Scoping try { if (completed) + { _database.CompleteTransaction(); + } else + { _database.AbortTransaction(); + } } catch { @@ -416,7 +509,9 @@ namespace Umbraco.Cms.Core.Scoping _database = null; if (databaseException) + { RobustExit(false, true); + } } } @@ -438,14 +533,20 @@ namespace Umbraco.Cms.Core.Scoping // to ensure we don't leave a scope around, etc private void RobustExit(bool completed, bool onException) { - if (onException) completed = false; + if (onException) + { + completed = false; + } TryFinally(() => { if (_scopeFileSystem == true) { if (completed) + { _fscope.Complete(); + } + _fscope.Dispose(); _fscope = null; } @@ -453,7 +554,9 @@ namespace Umbraco.Cms.Core.Scoping { // deal with events if (onException == false) + { _eventDispatcher?.ScopeExit(completed); + } }, () => { // if *we* created it, then get rid of it @@ -466,7 +569,7 @@ namespace Umbraco.Cms.Core.Scoping finally { // removes the ambient context (ambient scope already gone) - _scopeProvider.SetAmbient(null); + _scopeProvider.PopAmbientScopeContext(); } } }, () => @@ -474,7 +577,18 @@ namespace Umbraco.Cms.Core.Scoping if (Detachable) { // get out of the way, restore original - _scopeProvider.SetAmbient(OrigScope, OrigContext); + + // TODO: Difficult to know if this is correct since this is all required + // by Deploy which I don't fully understand since there is limited tests on this in the CMS + if (OrigScope != _scopeProvider.AmbientScope) + { + _scopeProvider.PopAmbientScope(_scopeProvider.AmbientScope); + } + if (OrigContext != _scopeProvider.AmbientContext) + { + _scopeProvider.PopAmbientScopeContext(); + } + Attached = false; OrigScope = null; OrigContext = null; @@ -482,14 +596,15 @@ namespace Umbraco.Cms.Core.Scoping }); } - private static void TryFinally(params Action[] actions) - { - TryFinally(0, actions); - } + private static void TryFinally(params Action[] actions) => TryFinally(0, actions); private static void TryFinally(int index, Action[] actions) { - if (index == actions.Length) return; + if (index == actions.Length) + { + return; + } + try { actions[index](); @@ -500,13 +615,8 @@ namespace Umbraco.Cms.Core.Scoping } } - // backing field for LogUncompletedScopes - private static bool? _logUncompletedScopes; - - // caching config // true if Umbraco.CoreDebugSettings.LogUncompletedScope appSetting is set to "true" - private bool LogUncompletedScopes => (_logUncompletedScopes - ?? (_logUncompletedScopes = _coreDebugSettings.LogIncompletedScopes)).Value; + private bool LogUncompletedScopes => _coreDebugSettings.LogIncompletedScopes; /// public void ReadLock(params int[] lockIds) => Database.SqlContext.SqlSyntax.ReadLock(Database, lockIds); diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeContext.cs b/src/Umbraco.Infrastructure/Scoping/ScopeContext.cs index 0ec966d18f..75a9ecb6ad 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeContext.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeContext.cs @@ -1,6 +1,7 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using Umbraco.Cms.Core.Scoping; namespace Umbraco.Cms.Core.Scoping @@ -41,6 +42,8 @@ namespace Umbraco.Cms.Core.Scoping public Guid InstanceId { get; } = Guid.NewGuid(); + public int CreatedThreadId { get; } = Thread.CurrentThread.ManagedThreadId; + private IDictionary Enlisted => _enlisted ?? (_enlisted = new Dictionary()); diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs index b0b1868a0d..ca5b96e0db 100644 --- a/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs +++ b/src/Umbraco.Infrastructure/Scoping/ScopeProvider.cs @@ -7,6 +7,10 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Infrastructure.Persistence; using CoreDebugSettings = Umbraco.Cms.Core.Configuration.Models.CoreDebugSettings; +using Umbraco.Extensions; +using System.Collections.Generic; +using System.Collections.Concurrent; +using System.Threading; #if DEBUG_SCOPES using System.Linq; @@ -26,6 +30,10 @@ namespace Umbraco.Cms.Core.Scoping private readonly FileSystems _fileSystems; private readonly CoreDebugSettings _coreDebugSettings; private readonly IMediaFileSystem _mediaFileSystem; + private static readonly AsyncLocal> s_scopeStack = new AsyncLocal>(); + private static readonly AsyncLocal> s_scopeContextStack = new AsyncLocal>(); + private static readonly string s_scopeItemKey = typeof(Scope).FullName; + private static readonly string s_contextItemKey = typeof(ScopeProvider).FullName; public ScopeProvider(IUmbracoDatabaseFactory databaseFactory, FileSystems fileSystems, IOptions coreDebugSettings, IMediaFileSystem mediaFileSystem, ILogger logger, ILoggerFactory loggerFactory, IRequestCache requestCache) { @@ -38,33 +46,6 @@ namespace Umbraco.Cms.Core.Scoping _requestCache = requestCache; // take control of the FileSystems _fileSystems.IsScoped = () => AmbientScope != null && AmbientScope.ScopedFileSystems; - - _scopeReference = new ScopeReference(this); - } - - static ScopeProvider() - { - SafeCallContext.Register( - () => - { - 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) - throw new Exception("Found leaked scope when restoring call context."); - 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); - }); } public IUmbracoDatabaseFactory DatabaseFactory { get; } @@ -73,45 +54,123 @@ namespace Umbraco.Cms.Core.Scoping #region Context - private static T GetCallContextObject(string key) - where T : class, IInstanceIdentifiable + private void MoveHttpContextScopeToCallContext() { - var obj = CallContext.GetData(key); - if (obj == default(T)) return null; - return obj; + var source = (ConcurrentStack)_requestCache.Get(s_scopeItemKey); + ConcurrentStack stack = s_scopeStack.Value; + MoveContexts(s_scopeItemKey, source, stack, (_, v) => s_scopeStack.Value = v); } - private static void SetCallContextObject(string key, T value) - where T: class, IInstanceIdentifiable + private void MoveHttpContextScopeContextToCallContext() { -#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 = CallContext.GetData(ScopeItemKey); + var source = (ConcurrentStack)_requestCache.Get(s_contextItemKey); + ConcurrentStack stack = s_scopeContextStack.Value; + MoveContexts(s_contextItemKey, source, stack, (_, v) => s_scopeContextStack.Value = v); + } - if (ambientScope != null) RegisterContext(ambientScope, null); - // then register the new value - var scope = value as IScope; - if (scope != null) RegisterContext(scope, "call"); + private void MoveCallContextScopeToHttpContext() + { + ConcurrentStack source = s_scopeStack.Value; + var stack = (ConcurrentStack)_requestCache.Get(s_scopeItemKey); + MoveContexts(s_scopeItemKey, source, stack, (k, v) => _requestCache.Set(k, v)); + } + + private void MoveCallContextScopeContextToHttpContext() + { + ConcurrentStack source = s_scopeContextStack.Value; + var stack = (ConcurrentStack)_requestCache.Get(s_contextItemKey); + MoveContexts(s_contextItemKey, source, stack, (k, v) => _requestCache.Set(k, v)); + } + + private void MoveContexts(string key, ConcurrentStack source, ConcurrentStack stack, Action> setter) + where T : class, IInstanceIdentifiable + { + if (source == null) + { + return; + } + + if (stack != null) + { + stack.Clear(); + } + else + { + // TODO: This isn't going to copy it back up the execution context chain + stack = new ConcurrentStack(); + setter(key, stack); + } + + var arr = new T[source.Count]; + source.CopyTo(arr, 0); + Array.Reverse(arr); + foreach (T a in arr) + { + stack.Push(a); + } + + source.Clear(); + } + + private void SetCallContextScope(IScope value) + { + ConcurrentStack stack = s_scopeStack.Value; + +#if DEBUG_SCOPES + // first, null-register the existing value + if (stack != null && stack.TryPeek(out IScope ambientScope)) + { + RegisterContext(ambientScope, null); + } + + // then register the new value + if (value != null) + { + RegisterContext(value, "call"); } #endif + if (value == null) { - var obj = CallContext.GetData(key); - CallContext.SetData(key, default); // aka remove - if (obj == null) return; + if (stack != null) + { + stack.TryPop(out _); + } } else { #if DEBUG_SCOPES - Current.Logger.Debug("AddObject " + value.InstanceId.ToString("N").Substring(0, 8)); + _logger.LogDebug("AddObject " + value.InstanceId.ToString("N").Substring(0, 8)); #endif + if (stack == null) + { + stack = new ConcurrentStack(); + } + stack.Push(value); + s_scopeStack.Value = stack; + } + } - CallContext.SetData(key, value); + private void SetCallContextScopeContext(IScopeContext value) + { + ConcurrentStack stack = s_scopeContextStack.Value; + + if (value == null) + { + if (stack != null) + { + stack.TryPop(out _); + } + } + else + { + if (stack == null) + { + stack = new ConcurrentStack(); + } + stack.Push(value); + s_scopeContextStack.Value = stack; } } @@ -119,131 +178,199 @@ namespace Umbraco.Cms.Core.Scoping private T GetHttpContextObject(string key, bool required = true) where T : class { - if (!_requestCache.IsAvailable && required) + { throw new Exception("Request cache is unavailable."); + } - return (T)_requestCache.Get(key); + var stack = (ConcurrentStack)_requestCache.Get(key); + return stack != null && stack.TryPeek(out T peek) ? peek : null; } - private bool SetHttpContextObject(string key, object value, bool required = true) + private bool SetHttpContextObject(string key, T value, bool required = true) { if (!_requestCache.IsAvailable) { if (required) + { 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) + if (key == s_scopeItemKey) { // first, null-register the existing value - var ambientScope = (IScope)_requestCache.Get(ScopeItemKey); - if (ambientScope != null) RegisterContext(ambientScope, null); + var ambientScope = (IScope)_requestCache.Get(s_scopeItemKey); + if (ambientScope != null) + { + RegisterContext(ambientScope, null); + } + // then register the new value - var scope = value as IScope; - if (scope != null) RegisterContext(scope, "http"); + if (value is IScope scope) + { + RegisterContext(scope, "http"); + } } #endif + var stack = (ConcurrentStack)_requestCache.Get(key); + if (value == null) - _requestCache.Remove(key); + { + if (stack != null) + { + stack.TryPop(out _); + } + } else - _requestCache.Set(key, value); + { + if (stack == null) + { + stack = new ConcurrentStack(); + } + stack.Push(value); + _requestCache.Set(key, stack); + } + return true; } -#endregion + #endregion #region Ambient Context - internal const string ContextItemKey = "Umbraco.Core.Scoping.ScopeContext"; - + /// + /// Get the Ambient (Current) for the current execution context. + /// + /// + /// The current execution context may be request based (HttpContext) or on a background thread (AsyncLocal) + /// public IScopeContext AmbientContext { get { // try http context, fallback onto call context - var value = GetHttpContextObject(ContextItemKey, false); - return value ?? GetCallContextObject(ContextItemKey); - } - set - { - // clear both - SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); - if (value == null) return; + IScopeContext value = GetHttpContextObject(s_contextItemKey, false); + if (value != null) + { + return value; + } - // set http/call context - if (SetHttpContextObject(ContextItemKey, value, false) == false) - SetCallContextObject(ContextItemKey, value); + ConcurrentStack stack = s_scopeContextStack.Value; + if (stack == null || !stack.TryPeek(out IScopeContext peek)) + { + return null; + } + + return peek; } } #endregion #region Ambient Scope - - internal const string ScopeItemKey = "Umbraco.Core.Scoping.Scope"; - internal const string ScopeRefItemKey = "Umbraco.Core.Scoping.ScopeReference"; - - // only 1 instance which can be disposed and disposed again - private readonly ScopeReference _scopeReference; - + IScope IScopeAccessor.AmbientScope => AmbientScope; - // null if there is none + /// + /// Get or set the Ambient (Current) for the current execution context. + /// + /// + /// The current execution context may be request based (HttpContext) or on a background thread (AsyncLocal) + /// public Scope AmbientScope { - // try http context, fallback onto call context - // we are casting here because we know its a concrete type - get => (Scope)GetHttpContextObject(ScopeItemKey, false) - ?? (Scope)GetCallContextObject(ScopeItemKey); - set + get { - // clear both - SetHttpContextObject(ScopeItemKey, null, false); - SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); - if (value == null) return; + // try http context, fallback onto call context + IScope value = GetHttpContextObject(s_scopeItemKey, false); + if (value != null) + { + return (Scope)value; + } - // set http/call context - if (value.CallContext == false && SetHttpContextObject(ScopeItemKey, value, false)) - SetHttpContextObject(ScopeRefItemKey, _scopeReference); - else - SetCallContextObject(ScopeItemKey, value); + ConcurrentStack stack = s_scopeStack.Value; + if (stack == null || !stack.TryPeek(out IScope peek)) + { + return null; + } + + return (Scope)peek; } } + public void PopAmbientScope(Scope scope) + { + // pop the stack from all contexts + SetHttpContextObject(s_scopeItemKey, null, false); + SetCallContextScope(null); + + // We need to move the stack to a different context if the parent scope + // is flagged with a different CallContext flag. This is required + // if creating a child scope with callContext: true (thus forcing CallContext) + // when there is actually a current HttpContext available. + // It's weird but is required for Deploy somehow. + bool parentScopeCallContext = (scope.ParentScope?.CallContext ?? false); + if (scope.CallContext && !parentScopeCallContext) + { + MoveCallContextScopeToHttpContext(); + MoveCallContextScopeContextToHttpContext(); + } + else if (!scope.CallContext && parentScopeCallContext) + { + MoveHttpContextScopeToCallContext(); + MoveHttpContextScopeContextToCallContext(); + } + } + #endregion - public void SetAmbient(Scope scope, IScopeContext context = null) + public void PushAmbientScope(Scope scope) { - // clear all - SetHttpContextObject(ScopeItemKey, null, false); - SetHttpContextObject(ScopeRefItemKey, null, false); - SetCallContextObject(ScopeItemKey, null); - SetHttpContextObject(ContextItemKey, null, false); - SetCallContextObject(ContextItemKey, null); - if (scope == null) + if (scope is null) { - if (context != null) - throw new ArgumentException("Must be null if scope is null.", nameof(context)); - return; + throw new ArgumentNullException(nameof(scope)); } - if (scope.CallContext == false && SetHttpContextObject(ScopeItemKey, scope, false)) + if (scope.CallContext != false || !SetHttpContextObject(s_scopeItemKey, scope, false)) { - SetHttpContextObject(ScopeRefItemKey, _scopeReference); - SetHttpContextObject(ContextItemKey, context); + // In this case, always ensure that the HttpContext items + // is transfered to CallContext and then cleared since we + // may be migrating context with the callContext = true flag. + // This is a weird case when forcing callContext when HttpContext + // is available. Required by Deploy. + + if (_requestCache.IsAvailable) + { + MoveHttpContextScopeToCallContext(); + MoveHttpContextScopeContextToCallContext(); + } + + SetCallContextScope(scope); } - else + } + + public void PushAmbientScopeContext(IScopeContext scopeContext) + { + if (scopeContext is null) { - SetCallContextObject(ScopeItemKey, scope); - SetCallContextObject(ContextItemKey, context); + throw new ArgumentNullException(nameof(scopeContext)); } + + SetHttpContextObject(s_contextItemKey, scopeContext, false); + SetCallContextScopeContext(scopeContext); + } + + public void PopAmbientScopeContext() + { + // pop stack from all contexts + SetHttpContextObject(s_contextItemKey, null, false); + SetCallContextScopeContext(null); } /// @@ -252,43 +379,65 @@ namespace Umbraco.Cms.Core.Scoping RepositoryCacheMode repositoryCacheMode = RepositoryCacheMode.Unspecified, IEventDispatcher eventDispatcher = null, bool? scopeFileSystems = null) - { - return new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger(), _fileSystems, true, null, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems); - } + => new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger(), _fileSystems, true, null, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems); /// public void AttachScope(IScope other, bool callContext = false) { // IScopeProvider.AttachScope works with an IScope // but here we can only deal with our own Scope class - if (!(other is Scope otherScope)) + if (other is not Scope otherScope) + { throw new ArgumentException("Not a Scope instance."); + } if (otherScope.Detachable == false) + { throw new ArgumentException("Not a detachable scope."); + } if (otherScope.Attached) + { throw new InvalidOperationException("Already attached."); + } otherScope.Attached = true; otherScope.OrigScope = AmbientScope; otherScope.OrigContext = AmbientContext; - otherScope.CallContext = callContext; - SetAmbient(otherScope, otherScope.Context); + + PushAmbientScopeContext(otherScope.Context); + PushAmbientScope(otherScope); } /// public IScope DetachScope() { - var ambientScope = AmbientScope; + Scope ambientScope = AmbientScope; if (ambientScope == null) + { throw new InvalidOperationException("There is no ambient scope."); + } if (ambientScope.Detachable == false) + { throw new InvalidOperationException("Ambient scope is not detachable."); + } - SetAmbient(ambientScope.OrigScope, ambientScope.OrigContext); + PopAmbientScope(ambientScope); + PopAmbientScopeContext(); + + Scope originalScope = AmbientScope; + if (originalScope != ambientScope.OrigScope) + { + throw new InvalidOperationException($"The detatched scope ({ambientScope.GetDebugInfo()}) does not match the original ({originalScope.GetDebugInfo()})"); + } + IScopeContext originalScopeContext = AmbientContext; + if (originalScopeContext != ambientScope.OrigContext) + { + throw new InvalidOperationException($"The detatched scope context does not match the original"); + } + ambientScope.OrigScope = null; ambientScope.OrigContext = null; ambientScope.Attached = false; @@ -304,33 +453,32 @@ namespace Umbraco.Cms.Core.Scoping bool callContext = false, bool autoComplete = false) { - var ambientScope = AmbientScope; + Scope ambientScope = AmbientScope; if (ambientScope == null) { - var ambientContext = AmbientContext; - var newContext = ambientContext == null ? new ScopeContext() : null; + IScopeContext ambientContext = AmbientContext; + ScopeContext newContext = ambientContext == null ? new ScopeContext() : null; var scope = new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger(), _fileSystems, false, newContext, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems, callContext, autoComplete); // assign only if scope creation did not throw! - SetAmbient(scope, newContext ?? ambientContext); + PushAmbientScope(scope); + if (newContext != null) + { + PushAmbientScopeContext(newContext); + } return scope; } var nested = new Scope(this, _coreDebugSettings, _mediaFileSystem, _loggerFactory.CreateLogger(), _fileSystems, ambientScope, isolationLevel, repositoryCacheMode, eventDispatcher, scopeFileSystems, callContext, autoComplete); - SetAmbient(nested, AmbientContext); + PushAmbientScope(nested); return nested; } - public void Reset() - { - var scope = AmbientScope; - scope?.Reset(); - - _scopeReference.Dispose(); - } - /// public IScopeContext Context => AmbientContext; + // for testing + internal ConcurrentStack GetCallContextScopeValue() => s_scopeStack.Value; + #if DEBUG_SCOPES // this code needs TLC // @@ -356,70 +504,96 @@ namespace Umbraco.Cms.Core.Scoping //} // all scope instances that are currently being tracked - private static readonly object StaticScopeInfosLock = new object(); - private static readonly Dictionary StaticScopeInfos = new Dictionary(); + private static readonly object s_staticScopeInfosLock = new object(); + private static readonly Dictionary s_staticScopeInfos = new Dictionary(); public IEnumerable ScopeInfos { get { - lock (StaticScopeInfosLock) + lock (s_staticScopeInfosLock) { - return StaticScopeInfos.Values.ToArray(); // capture in an array + return s_staticScopeInfos.Values.ToArray(); // capture in an array } } } public ScopeInfo GetScopeInfo(IScope scope) { - lock (StaticScopeInfosLock) + lock (s_staticScopeInfosLock) { - ScopeInfo scopeInfo; - return StaticScopeInfos.TryGetValue(scope, out scopeInfo) ? scopeInfo : null; + return s_staticScopeInfos.TryGetValue(scope, out ScopeInfo scopeInfo) ? scopeInfo : null; } } - //private static void Log(string message, UmbracoDatabase database) - //{ - // LogHelper.Debug(message + " (" + (database == null ? "" : database.InstanceSid) + ")."); - //} - // register a scope and capture its ctor stacktrace public void RegisterScope(IScope scope) { - lock (StaticScopeInfosLock) + if (scope is null) { - if (StaticScopeInfos.ContainsKey(scope)) throw new Exception("oops: already registered."); - _logger.Debug("Register " + scope.InstanceId.ToString("N").Substring(0, 8)); - StaticScopeInfos[scope] = new ScopeInfo(scope, Environment.StackTrace); + throw new ArgumentNullException(nameof(scope)); + } + + lock (s_staticScopeInfosLock) + { + if (s_staticScopeInfos.ContainsKey(scope)) + { + throw new Exception("oops: already registered."); + } + + _logger.LogDebug("Register {ScopeId} on Thread {ThreadId}", scope.InstanceId.ToString("N").Substring(0, 8), Thread.CurrentThread.ManagedThreadId); + s_staticScopeInfos[scope] = new ScopeInfo(scope, Environment.StackTrace); } } // register that a scope is in a 'context' // 'context' that contains the scope (null, "http" or "call") - public static void RegisterContext(IScope scope, string context) + public void RegisterContext(IScope scope, string context) { - lock (StaticScopeInfosLock) + if (scope is null) { - ScopeInfo info; - if (StaticScopeInfos.TryGetValue(scope, out info) == false) info = null; + throw new ArgumentNullException(nameof(scope)); + } + + lock (s_staticScopeInfosLock) + { + if (s_staticScopeInfos.TryGetValue(scope, out ScopeInfo info) == false) + { + info = null; + } + if (info == null) { - if (context == null) return; + if (context == null) + { + return; + } + throw new Exception("oops: unregistered scope."); } var sb = new StringBuilder(); - var s = scope; + IScope s = scope; while (s != null) { - if (sb.Length > 0) sb.Append(" < "); + if (sb.Length > 0) + { + sb.Append(" < "); + } + sb.Append(s.InstanceId.ToString("N").Substring(0, 8)); var ss = s as Scope; s = ss?.ParentScope; } - Current.Logger.Debug("Register " + (context ?? "null") + " context " + sb); - if (context == null) info.NullStack = Environment.StackTrace; - //Current.Logger.Debug("At:\r\n" + Head(Environment.StackTrace, 16)); + + _logger.LogTrace("Register " + (context ?? "null") + " context " + sb); + + if (context == null) + { + info.NullStack = Environment.StackTrace; + } + + _logger.LogTrace("At:\r\n" + Head(Environment.StackTrace, 16)); + info.Context = context; } } @@ -433,20 +607,25 @@ namespace Umbraco.Cms.Core.Scoping pos = s.IndexOf("\r\n", pos + 1, StringComparison.OrdinalIgnoreCase); i++; } - if (pos < 0) return s; + + if (pos < 0) + { + return s; + } + return s.Substring(0, pos); } public void Disposed(IScope scope) { - lock (StaticScopeInfosLock) + lock (s_staticScopeInfosLock) { - if (StaticScopeInfos.ContainsKey(scope)) + if (s_staticScopeInfos.ContainsKey(scope)) { // enable this by default //Console.WriteLine("unregister " + scope.InstanceId.ToString("N").Substring(0, 8)); - StaticScopeInfos.Remove(scope); - _logger.Debug("Remove " + scope.InstanceId.ToString("N").Substring(0, 8)); + s_staticScopeInfos.Remove(scope); + _logger.LogDebug("Remove " + scope.InstanceId.ToString("N").Substring(0, 8)); // instead, enable this to keep *all* scopes // beware, there can be a lot of scopes! @@ -471,15 +650,31 @@ namespace Umbraco.Cms.Core.Scoping public IScope Scope { get; } // the scope itself // the scope's parent identifier - public Guid Parent => ((Scope) Scope).ParentScope == null ? Guid.Empty : ((Scope) Scope).ParentScope.InstanceId; + public Guid Parent => ((Scope)Scope).ParentScope == null ? Guid.Empty : ((Scope)Scope).ParentScope.InstanceId; public DateTime Created { get; } // the date time the scope was created public bool Disposed { get; set; } // whether the scope has been disposed already public string Context { get; set; } // the current 'context' that contains the scope (null, "http" or "lcc") public string CtorStack { get; } // the stacktrace of the scope ctor - public string DisposedStack { get; set; } // the stacktrace when disposed + //public string DisposedStack { get; set; } // the stacktrace when disposed public string NullStack { get; set; } // the stacktrace when the 'context' that contains the scope went null + + public override string ToString() => new StringBuilder() + .AppendLine("ScopeInfo:") + .Append("Instance Id: ") + .AppendLine(Scope.InstanceId.ToString()) + .Append("Parent Id: ") + .AppendLine(Parent.ToString()) + .Append("Created Thread Id: ") + .AppendLine(Scope.CreatedThreadId.ToInvariantString()) + .Append("Created At: ") + .AppendLine(Created.ToString("O")) + .Append("Disposed: ") + .AppendLine(Disposed.ToString()) + .Append("CTOR stack: ") + .AppendLine(CtorStack) + .ToString(); } #endif } diff --git a/src/Umbraco.Infrastructure/Scoping/ScopeReference.cs b/src/Umbraco.Infrastructure/Scoping/ScopeReference.cs deleted file mode 100644 index 54d41d1efa..0000000000 --- a/src/Umbraco.Infrastructure/Scoping/ScopeReference.cs +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -namespace Umbraco.Cms.Core.Scoping -{ - /// - /// References a scope. - /// - /// Should go into HttpContext to indicate there is also an IScope in context - /// that needs to be disposed at the end of the request (the scope, and the entire scopes - /// chain). - internal class ScopeReference : IDisposeOnRequestEnd // implies IDisposable - { - private readonly ScopeProvider _scopeProvider; - - public ScopeReference(ScopeProvider scopeProvider) - { - _scopeProvider = scopeProvider; - } - - public void Dispose() - { - // dispose the entire chain (if any) - // reset (don't commit by default) - Scope scope; - while ((scope = _scopeProvider.AmbientScope) != null) - { - scope.Reset(); - scope.Dispose(); - } - } - } -} diff --git a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs index 1eb1d3bc29..4535bebd8b 100644 --- a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs +++ b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Threading.Tasks; using Examine; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; @@ -41,9 +42,11 @@ namespace Umbraco.Cms.Infrastructure.Search private const int EnlistPriority = 80; public ExamineComponent(IMainDom mainDom, - IExamineManager examineManager, IProfilingLogger profilingLogger, + IExamineManager examineManager, + IProfilingLogger profilingLogger, ILoggerFactory loggerFactory, - IScopeProvider scopeProvider, IUmbracoIndexesCreator indexCreator, + IScopeProvider scopeProvider, + IUmbracoIndexesCreator indexCreator, ServiceContext services, IContentValueSetBuilder contentValueSetBuilder, IPublishedContentValueSetBuilder publishedContentValueSetBuilder, @@ -88,8 +91,10 @@ namespace Umbraco.Cms.Infrastructure.Search } //create the indexes and register them with the manager - foreach(var index in _indexCreator.Create()) + foreach (IIndex index in _indexCreator.Create()) + { _examineManager.AddIndex(index); + } _logger.LogDebug("Examine shutdown registered with MainDom"); @@ -99,7 +104,9 @@ namespace Umbraco.Cms.Infrastructure.Search // don't bind event handlers if we're not suppose to listen if (registeredIndexers == 0) + { return; + } // bind to distributed cache events - this ensures that this logic occurs on ALL servers // that are taking part in a load balanced environment. @@ -129,10 +136,14 @@ namespace Umbraco.Cms.Infrastructure.Search private void ContentCacheRefresherUpdated(ContentCacheRefresher sender, CacheRefresherEventArgs args) { if (Suspendable.ExamineEvents.CanIndex == false) + { return; + } if (args.MessageType != MessageType.RefreshByPayload) + { throw new NotSupportedException(); + } var contentService = _services.ContentService; @@ -167,10 +178,14 @@ namespace Umbraco.Cms.Infrastructure.Search IContent published = null; if (content.Published && contentService.IsPathPublished(content)) + { published = content; + } if (published == null) + { DeleteIndexForEntity(payload.Id, true); + } // just that content ReIndexForContent(content, published != null); @@ -194,9 +209,13 @@ namespace Umbraco.Cms.Infrastructure.Search if (masked != null) // else everything is masked { if (masked.Contains(descendant.ParentId) || !descendant.Published) + { masked.Add(descendant.Id); + } else + { published = descendant; + } } ReIndexForContent(descendant, published != null); @@ -221,7 +240,9 @@ namespace Umbraco.Cms.Infrastructure.Search private void MemberCacheRefresherUpdated(MemberCacheRefresher sender, CacheRefresherEventArgs args) { if (Suspendable.ExamineEvents.CanIndex == false) + { return; + } switch (args.MessageType) { @@ -256,7 +277,7 @@ namespace Umbraco.Cms.Infrastructure.Search case MessageType.RefreshByPayload: var payload = (MemberCacheRefresher.JsonPayload[])args.MessageObject; var members = payload.Select(x => _services.MemberService.GetById(x.Id)); - foreach(var m in members) + foreach (var m in members) { ReIndexForMember(m); } @@ -272,10 +293,14 @@ namespace Umbraco.Cms.Infrastructure.Search private void MediaCacheRefresherUpdated(MediaCacheRefresher sender, CacheRefresherEventArgs args) { if (Suspendable.ExamineEvents.CanIndex == false) + { return; + } if (args.MessageType != MessageType.RefreshByPayload) + { throw new NotSupportedException(); + } var mediaService = _services.MediaService; @@ -303,7 +328,9 @@ namespace Umbraco.Cms.Infrastructure.Search } if (media.Trashed) + { DeleteIndexForEntity(payload.Id, true); + } // just that media ReIndexForMedia(media, !media.Trashed); @@ -330,9 +357,14 @@ namespace Umbraco.Cms.Infrastructure.Search private void LanguageCacheRefresherUpdated(LanguageCacheRefresher sender, CacheRefresherEventArgs e) { if (!(e.MessageObject is LanguageCacheRefresher.JsonPayload[] payloads)) + { return; + } - if (payloads.Length == 0) return; + if (payloads.Length == 0) + { + return; + } var removedOrCultureChanged = payloads.Any(x => x.ChangeType == LanguageCacheRefresher.JsonPayload.LanguageChangeType.ChangeCulture @@ -354,10 +386,14 @@ namespace Umbraco.Cms.Infrastructure.Search private void ContentTypeCacheRefresherUpdated(ContentTypeCacheRefresher sender, CacheRefresherEventArgs args) { if (Suspendable.ExamineEvents.CanIndex == false) + { return; + } if (args.MessageType != MessageType.RefreshByPayload) + { throw new NotSupportedException(); + } var changedIds = new Dictionary removedIds, List refreshedIds, List otherIds)>(); @@ -370,11 +406,17 @@ namespace Umbraco.Cms.Infrastructure.Search } if (payload.ChangeTypes.HasType(ContentTypeChangeTypes.Remove)) + { idLists.removedIds.Add(payload.Id); + } else if (payload.ChangeTypes.HasType(ContentTypeChangeTypes.RefreshMain)) + { idLists.refreshedIds.Add(payload.Id); + } else if (payload.ChangeTypes.HasType(ContentTypeChangeTypes.RefreshOther)) + { idLists.otherIds.Add(payload.Id); + } } const int pageSize = 500; @@ -413,9 +455,14 @@ namespace Umbraco.Cms.Infrastructure.Search total = results.TotalItemCount; var paged = results.Skip(page * pageSize); - foreach (var item in paged) - if (int.TryParse(item.Id, out var contentId)) + foreach (ISearchResult item in paged) + { + if (int.TryParse(item.Id, out int contentId)) + { DeleteIndexForEntity(contentId, false); + } + } + page++; } } @@ -427,18 +474,18 @@ namespace Umbraco.Cms.Infrastructure.Search { const int pageSize = 500; - var memberTypes = _services.MemberTypeService.GetAll(memberTypeIds); - foreach (var memberType in memberTypes) + IEnumerable memberTypes = _services.MemberTypeService.GetAll(memberTypeIds); + foreach (IMemberType memberType in memberTypes) { var page = 0; var total = long.MaxValue; while (page * pageSize < total) { - var memberToRefresh = _services.MemberService.GetAll( + IEnumerable memberToRefresh = _services.MemberService.GetAll( page++, pageSize, out total, "LoginName", Direction.Ascending, memberType.Alias); - foreach (var c in memberToRefresh) + foreach (IMember c in memberToRefresh) { ReIndexForMember(c); } @@ -453,13 +500,13 @@ namespace Umbraco.Cms.Infrastructure.Search var total = long.MaxValue; while (page * pageSize < total) { - var mediaToRefresh = _services.MediaService.GetPagedOfTypes( + IEnumerable mediaToRefresh = _services.MediaService.GetPagedOfTypes( //Re-index all content of these types mediaTypeIds, page++, pageSize, out total, null, Ordering.By("Path", Direction.Ascending)); - foreach (var c in mediaToRefresh) + foreach (IMedia c in mediaToRefresh) { ReIndexForMedia(c, c.Trashed == false); } @@ -473,7 +520,7 @@ namespace Umbraco.Cms.Infrastructure.Search var total = long.MaxValue; while (page * pageSize < total) { - var contentToRefresh = _services.ContentService.GetPagedOfTypes( + IEnumerable contentToRefresh = _services.ContentService.GetPagedOfTypes( //Re-index all content of these types contentTypeIds, page++, pageSize, out total, null, @@ -483,7 +530,7 @@ namespace Umbraco.Cms.Infrastructure.Search //track which Ids have their paths are published var publishChecked = new Dictionary(); - foreach (var c in contentToRefresh) + foreach (IContent c in contentToRefresh) { var isPublished = false; if (c.Published) @@ -508,27 +555,39 @@ namespace Umbraco.Cms.Infrastructure.Search { var actions = DeferedActions.Get(_scopeProvider); if (actions != null) + { actions.Add(new DeferedReIndexForContent(_taskHelper, this, sender, isPublished)); + } else + { DeferedReIndexForContent.Execute(_taskHelper, this, sender, isPublished); + } } private void ReIndexForMember(IMember member) { var actions = DeferedActions.Get(_scopeProvider); if (actions != null) + { actions.Add(new DeferedReIndexForMember(_taskHelper, this, member)); + } else + { DeferedReIndexForMember.Execute(_taskHelper, this, member); + } } private void ReIndexForMedia(IMedia sender, bool isPublished) { var actions = DeferedActions.Get(_scopeProvider); if (actions != null) + { actions.Add(new DeferedReIndexForMedia(_taskHelper, this, sender, isPublished)); + } else + { DeferedReIndexForMedia.Execute(_taskHelper, this, sender, isPublished); + } } /// @@ -543,9 +602,13 @@ namespace Umbraco.Cms.Infrastructure.Search { var actions = DeferedActions.Get(_scopeProvider); if (actions != null) + { actions.Add(new DeferedDeleteIndex(this, entityId, keepIfUnpublished)); + } else + { DeferedDeleteIndex.Execute(this, entityId, keepIfUnpublished); + } } #endregion @@ -556,25 +619,27 @@ namespace Umbraco.Cms.Infrastructure.Search public static DeferedActions Get(IScopeProvider scopeProvider) { - var scopeContext = scopeProvider.Context; + IScopeContext scopeContext = scopeProvider.Context; return scopeContext?.Enlist("examineEvents", () => new DeferedActions(), // creator (completed, actions) => // action { - if (completed) actions.Execute(); + if (completed) + { + actions.Execute(); + } }, EnlistPriority); } - public void Add(DeferedAction action) - { - _actions.Add(action); - } + public void Add(DeferedAction action) => _actions.Add(action); private void Execute() { - foreach (var action in _actions) + foreach (DeferedAction action in _actions) + { action.Execute(); + } } } @@ -605,15 +670,13 @@ namespace Umbraco.Cms.Infrastructure.Search _isPublished = isPublished; } - public override void Execute() - { - Execute(_taskHelper, _examineComponent, _content, _isPublished); - } + public override void Execute() => Execute(_taskHelper, _examineComponent, _content, _isPublished); public static void Execute(TaskHelper taskHelper, ExamineComponent examineComponent, IContent content, bool isPublished) - { - taskHelper.RunBackgroundTask(async () => + => taskHelper.RunBackgroundTask(() => { + using IScope scope = examineComponent._scopeProvider.CreateScope(autoComplete: true); + // for content we have a different builder for published vs unpublished // we don't want to build more value sets than is needed so we'll lazily build 2 one for published one for non-published var builders = new Dictionary>> @@ -622,17 +685,16 @@ namespace Umbraco.Cms.Infrastructure.Search [false] = new Lazy>(() => examineComponent._contentValueSetBuilder.GetValueSets(content).ToList()) }; - foreach (var index in examineComponent._examineManager.Indexes.OfType() + foreach (IUmbracoIndex index in examineComponent._examineManager.Indexes.OfType() //filter the indexers .Where(x => isPublished || !x.PublishedValuesOnly) .Where(x => x.EnableDefaultEventHandler)) { - var valueSet = builders[index.PublishedValuesOnly].Value; + List valueSet = builders[index.PublishedValuesOnly].Value; index.IndexItems(valueSet); } + return Task.CompletedTask; }); - - } } /// @@ -653,27 +715,26 @@ namespace Umbraco.Cms.Infrastructure.Search _isPublished = isPublished; } - public override void Execute() - { - Execute(_taskHelper, _examineComponent, _media, _isPublished); - } + public override void Execute() => Execute(_taskHelper, _examineComponent, _media, _isPublished); - public static void Execute(TaskHelper taskHelper, ExamineComponent examineComponent, IMedia media, bool isPublished) - { + public static void Execute(TaskHelper taskHelper, ExamineComponent examineComponent, IMedia media, bool isPublished) => // perform the ValueSet lookup on a background thread - taskHelper.RunBackgroundTask(async () => + taskHelper.RunBackgroundTask(() => { + using IScope scope = examineComponent._scopeProvider.CreateScope(autoComplete: true); + var valueSet = examineComponent._mediaValueSetBuilder.GetValueSets(media).ToList(); - foreach (var index in examineComponent._examineManager.Indexes.OfType() + foreach (IUmbracoIndex index in examineComponent._examineManager.Indexes.OfType() //filter the indexers .Where(x => isPublished || !x.PublishedValuesOnly) .Where(x => x.EnableDefaultEventHandler)) { index.IndexItems(valueSet); } + + return Task.CompletedTask; }); - } } /// @@ -692,25 +753,24 @@ namespace Umbraco.Cms.Infrastructure.Search _taskHelper = taskHelper; } - public override void Execute() - { - Execute(_taskHelper, _examineComponent, _member); - } + public override void Execute() => Execute(_taskHelper, _examineComponent, _member); - public static void Execute(TaskHelper taskHelper, ExamineComponent examineComponent, IMember member) - { + public static void Execute(TaskHelper taskHelper, ExamineComponent examineComponent, IMember member) => // perform the ValueSet lookup on a background thread - taskHelper.RunBackgroundTask(async () => + taskHelper.RunBackgroundTask(() => { + using IScope scope = examineComponent._scopeProvider.CreateScope(autoComplete: true); + var valueSet = examineComponent._memberValueSetBuilder.GetValueSets(member).ToList(); - foreach (var index in examineComponent._examineManager.Indexes.OfType() + foreach (IUmbracoIndex index in examineComponent._examineManager.Indexes.OfType() //filter the indexers .Where(x => x.EnableDefaultEventHandler)) { index.IndexItems(valueSet); } + + return Task.CompletedTask; }); - } } private class DeferedDeleteIndex : DeferedAction @@ -726,10 +786,7 @@ namespace Umbraco.Cms.Infrastructure.Search _keepIfUnpublished = keepIfUnpublished; } - public override void Execute() - { - Execute(_examineComponent, _id, _keepIfUnpublished); - } + public override void Execute() => Execute(_examineComponent, _id, _keepIfUnpublished); public static void Execute(ExamineComponent examineComponent, int id, bool keepIfUnpublished) { diff --git a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj index 61bd171336..23c1af2c5d 100644 --- a/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj +++ b/src/Umbraco.Infrastructure/Umbraco.Infrastructure.csproj @@ -9,8 +9,9 @@ bin\Release\Umbraco.Infrastructure.xml + - + @@ -19,7 +20,7 @@ - + @@ -38,7 +39,7 @@ - + diff --git a/src/Umbraco.TestData/Umbraco.TestData.csproj b/src/Umbraco.TestData/Umbraco.TestData.csproj index 3ddd067cfb..7980df2205 100644 --- a/src/Umbraco.TestData/Umbraco.TestData.csproj +++ b/src/Umbraco.TestData/Umbraco.TestData.csproj @@ -66,7 +66,7 @@ - 32.0.2 + 33.0.2 5.2.7 diff --git a/src/Umbraco.Tests.Benchmarks/CombineGuidBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/CombineGuidBenchmarks.cs index 67b6f42250..a27437c6f8 100644 --- a/src/Umbraco.Tests.Benchmarks/CombineGuidBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/CombineGuidBenchmarks.cs @@ -1,7 +1,6 @@ -using System; +using System; using BenchmarkDotNet.Attributes; using Umbraco.Cms.Core; -using Umbraco.Core; using Umbraco.Tests.Benchmarks.Config; namespace Umbraco.Tests.Benchmarks diff --git a/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs index 34d885a27d..33d8ba371b 100644 --- a/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/CtorInvokeBenchmarks.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Linq.Expressions; using System.Reflection; using System.Reflection.Emit; @@ -8,7 +8,6 @@ using BenchmarkDotNet.Diagnosers; using BenchmarkDotNet.Jobs; using Perfolizer.Horology; using Umbraco.Cms.Core; -using Umbraco.Core; namespace Umbraco.Tests.Benchmarks { diff --git a/src/Umbraco.Tests.Benchmarks/HexStringBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/HexStringBenchmarks.cs index d5d079f318..7f6f766aed 100644 --- a/src/Umbraco.Tests.Benchmarks/HexStringBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/HexStringBenchmarks.cs @@ -1,8 +1,7 @@ -using System; +using System; using System.Text; using BenchmarkDotNet.Attributes; using Umbraco.Cms.Core; -using Umbraco.Core; using Umbraco.Tests.Benchmarks.Config; namespace Umbraco.Tests.Benchmarks diff --git a/src/Umbraco.Tests.Common/Umbraco.Tests.Common.csproj b/src/Umbraco.Tests.Common/Umbraco.Tests.Common.csproj index 870e33e3cb..1040df225d 100644 --- a/src/Umbraco.Tests.Common/Umbraco.Tests.Common.csproj +++ b/src/Umbraco.Tests.Common/Umbraco.Tests.Common.csproj @@ -1,4 +1,4 @@ - + netstandard2.0 @@ -10,8 +10,8 @@ - - + + diff --git a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs index dbb2adef89..94d8be6b4d 100644 --- a/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs +++ b/src/Umbraco.Tests.Integration/TestServerTest/UmbracoTestServerTestBase.cs @@ -107,7 +107,6 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest /// The string URL of the controller action. protected string PrepareUrl(string url) { - IBackOfficeSecurityFactory backofficeSecurityFactory = GetRequiredService(); IUmbracoContextFactory umbracoContextFactory = GetRequiredService(); IHttpContextAccessor httpContextAccessor = GetRequiredService(); @@ -122,11 +121,6 @@ namespace Umbraco.Cms.Tests.Integration.TestServerTest } }; - // TODO: This dependency chain is broken and needs to be fixed. - // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null - // This is a very ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the - // way it was intended. - backofficeSecurityFactory.EnsureBackOfficeSecurity(); umbracoContextFactory.EnsureUmbracoContext(); return url; diff --git a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs index 133320b853..e73c0a5c5f 100644 --- a/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs +++ b/src/Umbraco.Tests.Integration/Testing/UmbracoIntegrationTest.cs @@ -87,8 +87,8 @@ namespace Umbraco.Cms.Tests.Integration.Testing } _testTeardown = null; - FirstTestInFixture = false; - FirstTestInSession = false; + _firstTestInFixture = false; + s_firstTestInSession = false; // Ensure CoreRuntime stopped (now it's a HostedService) IHost host = Services.GetRequiredService(); @@ -102,12 +102,12 @@ namespace Umbraco.Cms.Tests.Integration.Testing [SetUp] public virtual void SetUp_Logging() => - TestContext.Progress.Write($"Start test {TestCount++}: {TestContext.CurrentContext.Test.Name}"); + TestContext.Progress.Write($"Start test {s_testCount++}: {TestContext.CurrentContext.Test.Name}"); [SetUp] public virtual void Setup() { - InMemoryConfiguration[Constants.Configuration.ConfigGlobal + ":" + nameof(GlobalSettings.InstallUnattended)] = "true"; + InMemoryConfiguration[Constants.Configuration.ConfigUnattended + ":" + nameof(UnattendedSettings.InstallUnattended)] = "true"; IHostBuilder hostBuilder = CreateHostBuilder(); IHost host = hostBuilder.Build(); @@ -244,7 +244,6 @@ namespace Umbraco.Cms.Tests.Integration.Testing { if (TestOptions.Boot) { - Services.GetRequiredService().EnsureBackOfficeSecurity(); Services.GetRequiredService().EnsureUmbracoContext(); } @@ -347,7 +346,7 @@ namespace Umbraco.Cms.Tests.Integration.Testing // Only attach schema once per fixture // Doing it more than once will block the process since the old db hasn't been detached // and it would be the same as NewSchemaPerTest even if it didn't block - if (FirstTestInFixture) + if (_firstTestInFixture) { // New DB + Schema TestDbMeta newSchemaFixtureDbMeta = db.AttachSchema(); @@ -364,7 +363,7 @@ namespace Umbraco.Cms.Tests.Integration.Testing // Only attach schema once per fixture // Doing it more than once will block the process since the old db hasn't been detached // and it would be the same as NewSchemaPerTest even if it didn't block - if (FirstTestInFixture) + if (_firstTestInFixture) { // New DB + Schema TestDbMeta newEmptyFixtureDbMeta = db.AttachEmpty(); @@ -437,12 +436,11 @@ namespace Umbraco.Cms.Tests.Integration.Testing protected IMapperCollection Mappers => Services.GetRequiredService(); - protected UserBuilder UserBuilderInstance = new UserBuilder(); - protected UserGroupBuilder UserGroupBuilderInstance = new UserGroupBuilder(); + protected UserBuilder UserBuilderInstance { get; } = new UserBuilder(); + protected UserGroupBuilder UserGroupBuilderInstance { get; } = new UserGroupBuilder(); - protected static bool FirstTestInSession = true; - - protected bool FirstTestInFixture = true; - protected static int TestCount = 1; + private static bool s_firstTestInSession = true; + private bool _firstTestInFixture = true; + private static int s_testCount = 1; } } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Core/IO/ShadowFileSystemTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Core/IO/ShadowFileSystemTests.cs index 54f5e04dd2..276d7a267e 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Core/IO/ShadowFileSystemTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Core/IO/ShadowFileSystemTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.IO; using System.Linq; using System.Text; @@ -34,7 +34,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.IO [SetUp] public void SetUp() { - SafeCallContext.Clear(); ClearFiles(HostingEnvironment); FileSystems.ResetShadowId(); } @@ -42,7 +41,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Core.IO [TearDown] public void TearDown() { - SafeCallContext.Clear(); ClearFiles(HostingEnvironment); FileSystems.ResetShadowId(); } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs index 592de6a89c..e42e681a58 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/LocksTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Data.SqlClient; using System.Linq; @@ -85,10 +85,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence }); } - // safe call context ensures that current scope does not leak into starting threads - using (new SafeCallContext()) + // ensure that current scope does not leak into starting threads + using (ExecutionContext.SuppressFlow()) { - foreach (var thread in threads) thread.Start(); + foreach (var thread in threads) + { + thread.Start(); + } } m2.Wait(); @@ -96,13 +99,18 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence var maxAcquired = acquired; m1.Set(); - foreach (var thread in threads) thread.Join(); + foreach (var thread in threads) + { + thread.Join(); + } Assert.AreEqual(threadCount, maxAcquired); Assert.AreEqual(0, acquired); for (var i = 0; i < threadCount; i++) + { Assert.IsNull(exceptions[i]); + } } [Test] @@ -115,7 +123,11 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence var acquired = 0; var entered = 0; var ms = new AutoResetEvent[threadCount]; - for (var i = 0; i < threadCount; i++) ms[i] = new AutoResetEvent(false); + for (var i = 0; i < threadCount; i++) + { + ms[i] = new AutoResetEvent(false); + } + var m1 = new ManualResetEventSlim(false); for (var i = 0; i < threadCount; i++) @@ -153,28 +165,43 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence }); } - // safe call context ensures that current scope does not leak into starting threads - using (new SafeCallContext()) + // ensure that current scope does not leak into starting threads + using (ExecutionContext.SuppressFlow()) { - foreach (var thread in threads) thread.Start(); + foreach (var thread in threads) + { + thread.Start(); + } } m1.Wait(); // all threads have entered ms[0].Set(); // let 0 go Thread.Sleep(100); - for (var i = 1; i < threadCount; i++) ms[i].Set(); // let others go + for (var i = 1; i < threadCount; i++) + { + ms[i].Set(); // let others go + } + Thread.Sleep(500); // only 1 thread has locked Assert.AreEqual(1, acquired); - for (var i = 0; i < threadCount; i++) ms[i].Set(); // let all go + for (var i = 0; i < threadCount; i++) + { + ms[i].Set(); // let all go + } - foreach (var thread in threads) thread.Join(); + foreach (var thread in threads) + { + thread.Join(); + } Assert.AreEqual(0, acquired); for (var i = 0; i < threadCount; i++) + { Assert.IsNull(exceptions[i]); + } } [Retry(10)] // TODO make this test non-flaky. @@ -191,8 +218,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence var thread1 = new Thread(() => DeadLockTestThread(1, 2, ev1, ev2, ref e1)); var thread2 = new Thread(() => DeadLockTestThread(2, 1, ev2, ev1, ref e2)); - // safe call context ensures that current scope does not leak into starting threads - using (new SafeCallContext()) + // ensure that current scope does not leak into starting threads + using (ExecutionContext.SuppressFlow()) { thread1.Start(); thread2.Start(); @@ -242,9 +269,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence myEv.Set(); if (id1 == 1) + { otherEv.WaitOne(); + } else + { Thread.Sleep(5200); // wait for deadlock... + } Console.WriteLine($"[{id1}] WAIT {id2}"); scope.WriteLock(id2); @@ -275,8 +306,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Persistence var thread1 = new Thread(() => NoDeadLockTestThread(1, ev1, ev2, ref e1)); var thread2 = new Thread(() => NoDeadLockTestThread(2, ev2, ev1, ref e1)); - // need safe call context else the current one leaks into *both* threads - using (new SafeCallContext()) + // ensure that current scope does not leak into starting threads + using (ExecutionContext.SuppressFlow()) { thread1.Start(); thread2.Start(); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeFileSystemsTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeFileSystemsTests.cs index 7529409032..f96852faeb 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeFileSystemsTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeFileSystemsTests.cs @@ -4,7 +4,10 @@ using System; using System.IO; using System.Text; +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Moq; using NUnit.Framework; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; @@ -27,16 +30,11 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping private IHostingEnvironment HostingEnvironment => GetRequiredService(); [SetUp] - public void SetUp() - { - SafeCallContext.Clear(); - ClearFiles(IOHelper); - } + public void SetUp() => ClearFiles(IOHelper); [TearDown] public void Teardown() { - SafeCallContext.Clear(); FileSystems.ResetShadowId(); ClearFiles(IOHelper); } @@ -115,6 +113,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping string rootUrl = HostingEnvironment.ToAbsolute(GlobalSettings.UmbracoMediaPath); var physMediaFileSystem = new PhysicalFileSystem(IOHelper, HostingEnvironment, GetRequiredService>(), rootPath, rootUrl); IMediaFileSystem mediaFileSystem = MediaFileSystem; + var taskHelper = new TaskHelper(Mock.Of>()); IScopeProvider scopeProvider = ScopeProvider; using (IScope scope = scopeProvider.CreateScope(scopeFileSystems: true)) @@ -127,7 +126,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.IsTrue(mediaFileSystem.FileExists("f1.txt")); Assert.IsFalse(physMediaFileSystem.FileExists("f1.txt")); - using (new SafeCallContext()) + // execute on another disconnected thread (execution context will not flow) + Task t = taskHelper.ExecuteBackgroundTask(() => { Assert.IsFalse(mediaFileSystem.FileExists("f1.txt")); @@ -138,7 +138,11 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.IsTrue(mediaFileSystem.FileExists("f2.txt")); Assert.IsTrue(physMediaFileSystem.FileExists("f2.txt")); - } + + return Task.CompletedTask; + }); + + Task.WaitAll(t); Assert.IsTrue(mediaFileSystem.FileExists("f2.txt")); Assert.IsTrue(physMediaFileSystem.FileExists("f2.txt")); @@ -148,10 +152,14 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping [Test] public void SingleShadow() { + var taskHelper = new TaskHelper(Mock.Of>()); IScopeProvider scopeProvider = ScopeProvider; + bool isThrown = false; using (IScope scope = scopeProvider.CreateScope(scopeFileSystems: true)) { - using (new SafeCallContext()) // not nesting! + // This is testing when another thread concurrently tries to create a scoped file system + // because at the moment we don't support concurrent scoped filesystems. + Task t = taskHelper.ExecuteBackgroundTask(() => { // ok to create a 'normal' other scope using (IScope other = scopeProvider.CreateScope()) @@ -160,31 +168,47 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping } // not ok to create a 'scoped filesystems' other scope - // because at the moment we don't support concurrent scoped filesystems + // we will get a "Already shadowing." exception. Assert.Throws(() => - { - IScope other = scopeProvider.CreateScope(scopeFileSystems: true); + { + using IScope other = scopeProvider.CreateScope(scopeFileSystems: true); }); - } + + isThrown = true; + + return Task.CompletedTask; + }); + + Task.WaitAll(t); } + + Assert.IsTrue(isThrown); } [Test] public void SingleShadowEvenDetached() { + var taskHelper = new TaskHelper(Mock.Of>()); var scopeProvider = (ScopeProvider)ScopeProvider; using (IScope scope = scopeProvider.CreateScope(scopeFileSystems: true)) { - using (new SafeCallContext()) // not nesting! + // This is testing when another thread concurrently tries to create a scoped file system + // because at the moment we don't support concurrent scoped filesystems. + Task t = taskHelper.ExecuteBackgroundTask(() => { // not ok to create a 'scoped filesystems' other scope // because at the moment we don't support concurrent scoped filesystems // even a detached one + // we will get a "Already shadowing." exception. Assert.Throws(() => { - IScope other = scopeProvider.CreateDetachedScope(scopeFileSystems: true); + using IScope other = scopeProvider.CreateDetachedScope(scopeFileSystems: true); }); - } + + return Task.CompletedTask; + }); + + Task.WaitAll(t); } IScope detached = scopeProvider.CreateDetachedScope(scopeFileSystems: true); @@ -194,9 +218,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.Throws(() => { // even if there is no ambient scope, there's a single shadow - using (IScope other = scopeProvider.CreateScope(scopeFileSystems: true)) - { - } + using IScope other = scopeProvider.CreateScope(scopeFileSystems: true); }); scopeProvider.AttachScope(detached); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeTests.cs index 86e717620a..7b3075eeb9 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Scoping/ScopeTests.cs @@ -2,8 +2,16 @@ // See LICENSE for more details. using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Castle.Core.Logging; +using Microsoft.Extensions.Logging; +using Moq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Tests.Common.Testing; @@ -20,6 +28,116 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping [SetUp] public void SetUp() => Assert.IsNull(ScopeProvider.AmbientScope); // gone + protected override AppCaches GetAppCaches() + { + // Need to have a mockable request cache for tests + var appCaches = new AppCaches( + NoAppCache.Instance, + Mock.Of(x => x.IsAvailable == false), + new IsolatedCaches(_ => NoAppCache.Instance)); + return appCaches; + } + + [Test] + public void GivenUncompletedScopeOnChildThread_WhenTheParentCompletes_TheTransactionIsRolledBack() + { + ScopeProvider scopeProvider = ScopeProvider; + + Assert.IsNull(ScopeProvider.AmbientScope); + IScope mainScope = scopeProvider.CreateScope(); + + var t = Task.Run(() => + { + IScope nested = scopeProvider.CreateScope(); + Thread.Sleep(2000); + nested.Dispose(); + }); + + Thread.Sleep(1000); // mimic some long running operation that is shorter than the other thread + mainScope.Complete(); + Assert.Throws(() => mainScope.Dispose()); + + Task.WaitAll(t); + } + + [Test] + public void GivenNonDisposedChildScope_WhenTheParentDisposes_ThenInvalidOperationExceptionThrows() + { + // this all runs in the same execution context so the AmbientScope reference isn't a copy + + ScopeProvider scopeProvider = ScopeProvider; + + Assert.IsNull(ScopeProvider.AmbientScope); + IScope mainScope = scopeProvider.CreateScope(); + + IScope nested = scopeProvider.CreateScope(); // not disposing + + InvalidOperationException ex = Assert.Throws(() => mainScope.Dispose()); + Console.WriteLine(ex); + } + + [Test] + public void GivenChildThread_WhenParentDisposedBeforeChild_ParentScopeThrows() + { + ScopeProvider scopeProvider = ScopeProvider; + + Assert.IsNull(ScopeProvider.AmbientScope); + IScope mainScope = scopeProvider.CreateScope(); + + var t = Task.Run(() => + { + Console.WriteLine("Child Task start: " + scopeProvider.AmbientScope.InstanceId); + // This will push the child scope to the top of the Stack + IScope nested = scopeProvider.CreateScope(); + Console.WriteLine("Child Task scope created: " + scopeProvider.AmbientScope.InstanceId); + Thread.Sleep(5000); // block for a bit to ensure the parent task is disposed first + Console.WriteLine("Child Task before dispose: " + scopeProvider.AmbientScope.InstanceId); + nested.Dispose(); + Console.WriteLine("Child Task after dispose: " + scopeProvider.AmbientScope.InstanceId); + }); + + // provide some time for the child thread to start so the ambient context is copied in AsyncLocal + Thread.Sleep(2000); + // now dispose the main without waiting for the child thread to join + Console.WriteLine("Parent Task disposing: " + scopeProvider.AmbientScope.InstanceId); + // This will throw because at this stage a child scope has been created which means + // it is the Ambient (top) scope but here we're trying to dispose the non top scope. + Assert.Throws(() => mainScope.Dispose()); + Task.WaitAll(t); // wait for the child to dispose + mainScope.Dispose(); // now it's ok + Console.WriteLine("Parent Task disposed: " + scopeProvider.AmbientScope?.InstanceId); + } + + [Test] + public void GivenChildThread_WhenChildDisposedBeforeParent_OK() + { + ScopeProvider scopeProvider = ScopeProvider; + + Assert.IsNull(ScopeProvider.AmbientScope); + IScope mainScope = scopeProvider.CreateScope(); + + // Task.Run will flow the execution context unless ExecutionContext.SuppressFlow() is explicitly called. + // This is what occurs in normal async behavior since it is expected to await (and join) the main thread, + // but if Task.Run is used as a fire and forget thread without being done correctly then the Scope will + // flow to that thread. + var t = Task.Run(() => + { + Console.WriteLine("Child Task start: " + scopeProvider.AmbientScope.InstanceId); + IScope nested = scopeProvider.CreateScope(); + Console.WriteLine("Child Task before dispose: " + scopeProvider.AmbientScope.InstanceId); + nested.Dispose(); + Console.WriteLine("Child Task after disposed: " + scopeProvider.AmbientScope.InstanceId); + }); + + Console.WriteLine("Parent Task waiting: " + scopeProvider.AmbientScope?.InstanceId); + Task.WaitAll(t); + Console.WriteLine("Parent Task disposing: " + scopeProvider.AmbientScope.InstanceId); + mainScope.Dispose(); + Console.WriteLine("Parent Task disposed: " + scopeProvider.AmbientScope?.InstanceId); + + Assert.Pass(); + } + [Test] public void SimpleCreateScope() { @@ -104,6 +222,24 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping [Test] public void NestedMigrateScope() { + // Get the request cache mock and re-configure it to be available and used + var requestCacheDictionary = new Dictionary(); + IRequestCache requestCache = AppCaches.RequestCache; + var requestCacheMock = Mock.Get(requestCache); + requestCacheMock + .Setup(x => x.IsAvailable) + .Returns(true); + requestCacheMock + .Setup(x => x.Set(It.IsAny(), It.IsAny())) + .Returns((string key, object val) => + { + requestCacheDictionary.Add(key, val); + return true; + }); + requestCacheMock + .Setup(x => x.Get(It.IsAny())) + .Returns((string key) => requestCacheDictionary.TryGetValue(key, out var val) ? val : null); + ScopeProvider scopeProvider = ScopeProvider; Assert.IsNull(scopeProvider.AmbientScope); @@ -113,9 +249,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.IsNotNull(scopeProvider.AmbientScope); Assert.AreSame(scope, scopeProvider.AmbientScope); - // only if Core.DEBUG_SCOPES are defined - //// Assert.IsEmpty(scopeProvider.CallContextObjects); - using (IScope nested = scopeProvider.CreateScope(callContext: true)) { Assert.IsInstanceOf(nested); @@ -124,12 +257,10 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.AreSame(scope, ((Scope)nested).ParentScope); // it's moved over to call context - IScope callContextScope = CallContext.GetData(ScopeProvider.ScopeItemKey); - Assert.IsNotNull(callContextScope); + ConcurrentStack callContextScope = scopeProvider.GetCallContextScopeValue(); - // only if Core.DEBUG_SCOPES are defined - // var ccnested = scopeProvider.CallContextObjects[callContextKey]; - // Assert.AreSame(nested, ccnested); + Assert.IsNotNull(callContextScope); + Assert.AreEqual(2, callContextScope.Count); } // it's naturally back in http context @@ -404,12 +535,15 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping [Test] public void CallContextScope1() { + var taskHelper = new TaskHelper(Mock.Of>()); ScopeProvider scopeProvider = ScopeProvider; using (IScope scope = scopeProvider.CreateScope()) { Assert.IsNotNull(scopeProvider.AmbientScope); Assert.IsNotNull(scopeProvider.AmbientContext); - using (new SafeCallContext()) + + // Run on another thread without a flowed context + Task t = taskHelper.ExecuteBackgroundTask(() => { Assert.IsNull(scopeProvider.AmbientScope); Assert.IsNull(scopeProvider.AmbientContext); @@ -423,7 +557,11 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.IsNull(scopeProvider.AmbientScope); Assert.IsNull(scopeProvider.AmbientContext); - } + + return Task.CompletedTask; + }); + + Task.WaitAll(t); Assert.IsNotNull(scopeProvider.AmbientScope); Assert.AreSame(scope, scopeProvider.AmbientScope); @@ -436,6 +574,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping [Test] public void CallContextScope2() { + var taskHelper = new TaskHelper(Mock.Of>()); ScopeProvider scopeProvider = ScopeProvider; Assert.IsNull(scopeProvider.AmbientScope); @@ -443,7 +582,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping { Assert.IsNotNull(scopeProvider.AmbientScope); Assert.IsNotNull(scopeProvider.AmbientContext); - using (new SafeCallContext()) + + // Run on another thread without a flowed context + Task t = taskHelper.ExecuteBackgroundTask(() => { Assert.IsNull(scopeProvider.AmbientScope); Assert.IsNull(scopeProvider.AmbientContext); @@ -457,7 +598,10 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping Assert.IsNull(scopeProvider.AmbientScope); Assert.IsNull(scopeProvider.AmbientContext); - } + return Task.CompletedTask; + }); + + Task.WaitAll(t); Assert.IsNotNull(scopeProvider.AmbientScope); Assert.AreSame(scope, scopeProvider.AmbientScope); @@ -474,7 +618,8 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Scoping IScope scope = scopeProvider.CreateScope(); IScope nested = scopeProvider.CreateScope(); Assert.IsNotNull(scopeProvider.AmbientScope); - var scopeRef = new ScopeReference(scopeProvider); + var scopeRef = new HttpScopeReference(scopeProvider); + scopeRef.Register(); scopeRef.Dispose(); Assert.IsNull(scopeProvider.AmbientScope); Assert.Throws(() => diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ThreadSafetyServiceTest.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ThreadSafetyServiceTest.cs index cc061c0a18..5611c7cb07 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ThreadSafetyServiceTest.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/ThreadSafetyServiceTest.cs @@ -2,6 +2,7 @@ // See LICENSE for more details. using System; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -14,6 +15,7 @@ using Umbraco.Cms.Core.Services.Implement; using Umbraco.Cms.Tests.Common.Builders; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; +using Umbraco.Extensions; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services @@ -120,12 +122,27 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services { try { - log.LogInformation("[{0}] Running...", Thread.CurrentThread.ManagedThreadId); + ConcurrentStack currentStack = ((ScopeProvider)ScopeProvider).GetCallContextScopeValue(); + log.LogInformation("[{ThreadId}] Current Stack? {CurrentStack}", Thread.CurrentThread.ManagedThreadId, currentStack?.Count); + + // NOTE: This is NULL because we have supressed the execution context flow. + // If we don't do that we will get various exceptions because we're trying to run concurrent threads + // against an ambient context which cannot be done due to the rules of scope creation and completion. + // But this works in v8 without the supression!? Why? + // In v8 the value of the AmbientScope is simply the current CallContext (i.e. AsyncLocal) Value which + // is not a mutable Stack like we are maintaining now. This means that for each child thread + // in v8, that thread will see it's own CallContext Scope value that it set and not the 'true' + // ambient Scope like we do now. + // So although the test passes in v8, there's actually some strange things occuring because Scopes + // are being created and disposed concurrently and out of order. + var currentScope = (Scope)ScopeAccessor.AmbientScope; + log.LogInformation("[{ThreadId}] Current Scope? {CurrentScope}", Thread.CurrentThread.ManagedThreadId, currentScope?.GetDebugInfo()); + Assert.IsNull(currentScope); string name1 = "test-" + Guid.NewGuid(); IContent content1 = contentService.Create(name1, -1, "umbTextpage"); - log.LogInformation("[{0}] Saving content #1.", Thread.CurrentThread.ManagedThreadId); + log.LogInformation("[{ThreadId}] Saving content #1.", Thread.CurrentThread.ManagedThreadId); Save(contentService, content1); Thread.Sleep(100); // quick pause for maximum overlap! @@ -133,11 +150,12 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services string name2 = "test-" + Guid.NewGuid(); IContent content2 = contentService.Create(name2, -1, "umbTextpage"); - log.LogInformation("[{0}] Saving content #2.", Thread.CurrentThread.ManagedThreadId); + log.LogInformation("[{ThreadId}] Saving content #2.", Thread.CurrentThread.ManagedThreadId); Save(contentService, content2); } catch (Exception e) { + //throw; lock (exceptions) { exceptions.Add(e); @@ -147,9 +165,14 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services threads.Add(t); } - // start all threads - log.LogInformation("Starting threads"); - threads.ForEach(x => x.Start()); + // See NOTE above, we must supress flow here to be able to run concurrent threads, + // else the AsyncLocal value from this current context will flow to the child threads. + using (ExecutionContext.SuppressFlow()) + { + // start all threads + log.LogInformation("Starting threads"); + threads.ForEach(x => x.Start()); + } // wait for all to complete log.LogInformation("Joining threads"); @@ -196,7 +219,22 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services { try { - log.LogInformation("[{0}] Running...", Thread.CurrentThread.ManagedThreadId); + ConcurrentStack currentStack = ((ScopeProvider)ScopeProvider).GetCallContextScopeValue(); + log.LogInformation("[{ThreadId}] Current Stack? {CurrentStack}", Thread.CurrentThread.ManagedThreadId, currentStack?.Count); + + // NOTE: This is NULL because we have supressed the execution context flow. + // If we don't do that we will get various exceptions because we're trying to run concurrent threads + // against an ambient context which cannot be done due to the rules of scope creation and completion. + // But this works in v8 without the supression!? Why? + // In v8 the value of the AmbientScope is simply the current CallContext (i.e. AsyncLocal) Value which + // is not a mutable Stack like we are maintaining now. This means that for each child thread + // in v8, that thread will see it's own CallContext Scope value that it set and not the 'true' + // ambient Scope like we do now. + // So although the test passes in v8, there's actually some strange things occuring because Scopes + // are being created and disposed concurrently and out of order. + var currentScope = (Scope)ScopeAccessor.AmbientScope; + log.LogInformation("[{ThreadId}] Current Scope? {CurrentScope}", Thread.CurrentThread.ManagedThreadId, currentScope?.GetDebugInfo()); + Assert.IsNull(currentScope); string name1 = "test-" + Guid.NewGuid(); IMedia media1 = mediaService.CreateMedia(name1, -1, Constants.Conventions.MediaTypes.Folder); @@ -221,8 +259,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services threads.Add(t); } - // start all threads - threads.ForEach(x => x.Start()); + // See NOTE above, we must supress flow here to be able to run concurrent threads, + // else the AsyncLocal value from this current context will flow to the child threads. + using (ExecutionContext.SuppressFlow()) + { + // start all threads + threads.ForEach(x => x.Start()); + } // wait for all to complete threads.ForEach(x => x.Join()); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj b/src/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj index f45a7bc444..adec339a08 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj +++ b/src/Umbraco.Tests.Integration/Umbraco.Tests.Integration.csproj @@ -57,10 +57,10 @@ - - - - + + + + all runtime; build; native; contentfiles; analyzers; buildtransitive diff --git a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs index 91432f142e..99d1c7d1fd 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Web.BackOffice/Filters/ContentModelValidatorTests.cs @@ -138,8 +138,6 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Web.BackOffice.Filters public void Validating_ContentItemSave() { ILogger logger = Services.GetRequiredService>(); - IBackOfficeSecurityFactory backofficeSecurityFactory = Services.GetRequiredService(); - backofficeSecurityFactory.EnsureBackOfficeSecurity(); IPropertyValidationService propertyValidationService = Services.GetRequiredService(); UmbracoMapper umbracoMapper = Services.GetRequiredService(); diff --git a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs index 15c2e91bc5..73a429753c 100644 --- a/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs +++ b/src/Umbraco.Tests.UnitTests/TestHelpers/Objects/TestUmbracoContextFactory.cs @@ -8,7 +8,6 @@ using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Tests.Common; using Umbraco.Cms.Web.Common.AspNetCore; @@ -57,9 +56,6 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects IHostingEnvironment hostingEnvironment = TestHelper.GetHostingEnvironment(); - var backofficeSecurityAccessorMock = new Mock(); - backofficeSecurityAccessorMock.Setup(x => x.BackOfficeSecurity).Returns(Mock.Of()); - var umbracoContextFactory = new UmbracoContextFactory( umbracoContextAccessor, snapshotService.Object, @@ -69,8 +65,7 @@ namespace Umbraco.Cms.Tests.UnitTests.TestHelpers.Objects hostingEnvironment, new UriUtility(hostingEnvironment), new AspNetCoreCookieManager(httpContextAccessor), - Mock.Of(), - backofficeSecurityAccessorMock.Object); + httpContextAccessor); return umbracoContextFactory; } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpContextRequestAppCacheTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpContextRequestAppCacheTests.cs new file mode 100644 index 0000000000..30134c1b1a --- /dev/null +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpContextRequestAppCacheTests.cs @@ -0,0 +1,30 @@ +// Copyright (c) Umbraco. +// See LICENSE for more details. + +using Microsoft.AspNetCore.Http; +using Moq; +using NUnit.Framework; +using Umbraco.Cms.Core.Cache; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache +{ + [TestFixture] + public class HttpContextRequestAppCacheTests : AppCacheTests + { + private HttpContextRequestAppCache _appCache; + private IHttpContextAccessor _httpContextAccessor; + + public override void Setup() + { + base.Setup(); + var httpContext = new DefaultHttpContext(); + + _httpContextAccessor = Mock.Of(x => x.HttpContext == httpContext); + _appCache = new HttpContextRequestAppCache(_httpContextAccessor); + } + + internal override IAppCache AppCache => _appCache; + + protected override int GetTotalItemCount => _httpContextAccessor.HttpContext.Items.Count; + } +} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpRequestAppCacheTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpRequestAppCacheTests.cs deleted file mode 100644 index e2d0dfeb07..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/Cache/HttpRequestAppCacheTests.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using Microsoft.AspNetCore.Http; -using NUnit.Framework; -using Umbraco.Cms.Core.Cache; - -namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.Cache -{ - [TestFixture] - public class HttpRequestAppCacheTests : AppCacheTests - { - private HttpRequestAppCache _appCache; - private HttpContext _httpContext; - - public override void Setup() - { - base.Setup(); - _httpContext = new DefaultHttpContext(); - _appCache = new HttpRequestAppCache(() => _httpContext.Items); - } - - internal override IAppCache AppCache => _appCache; - - protected override int GetTotalItemCount => _httpContext.Items.Count; - } -} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/CoreThings/CallContextTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/CoreThings/CallContextTests.cs deleted file mode 100644 index ac54e72cce..0000000000 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/CoreThings/CallContextTests.cs +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using NUnit.Framework; -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Scoping; - -namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core.CoreThings -{ - [TestFixture] - public class CallContextTests - { - private static bool s_first; - - static CallContextTests() => SafeCallContext.Register( - () => - { - CallContext.SetData("test1", null); - CallContext.SetData("test2", null); - return null; - }, o => { }); - - [OneTimeSetUp] - public void SetUpFixture() => s_first = true; - - // logical call context leaks between tests - // is is required to clear it before tests begin - // (don't trust other tests properly tearing down) - [SetUp] - public void Setup() => SafeCallContext.Clear(); - - [TearDown] - public void TearDown() => SafeCallContext.Clear(); - - [Test] - public void Test1() - { - CallContext.SetData("test1", "test1"); - Assert.IsNull(CallContext.GetData("test2")); - - CallContext.SetData("test3b", "test3b"); - - if (s_first) - { - s_first = false; - } - else - { - Assert.IsNotNull(CallContext.GetData("test3a")); // leak! - } - } - - [Test] - public void Test2() - { - CallContext.SetData("test2", "test2"); - Assert.IsNull(CallContext.GetData("test1")); - } - - [Test] - public void Test3() - { - CallContext.SetData("test3a", "test3a"); - - if (s_first) - { - s_first = false; - } - else - { - Assert.IsNotNull(CallContext.GetData("test3b")); // leak! - } - } - } -} diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Core/TaskHelperTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Core/TaskHelperTests.cs index 48c9b984ca..a4680387ee 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Core/TaskHelperTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Core/TaskHelperTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; @@ -19,27 +19,57 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Core { [Test] [AutoMoqData] - public void RunBackgroundTask__must_run_func([Frozen] ILogger logger, TaskHelper sut) + public void RunBackgroundTask__Suppress_Execution_Context( + [Frozen] ILogger logger, + TaskHelper sut) + { + var local = new AsyncLocal + { + Value = "hello" + }; + + string taskResult = null; + + Task t = sut.ExecuteBackgroundTask(() => + { + // FireAndForgetTasks ensure that flow is suppressed therefore this value will be null + taskResult = local.Value; + return Task.CompletedTask; + }); + + Task.WaitAll(t); + + Assert.IsNull(taskResult); + } + + [Test] + [AutoMoqData] + public void RunBackgroundTask__Must_Run_Func( + [Frozen] ILogger logger, + TaskHelper sut) { var i = 0; - sut.RunBackgroundTask(() => + Task t = sut.ExecuteBackgroundTask(() => { Interlocked.Increment(ref i); return Task.CompletedTask; }); - Thread.Sleep(5); // Wait for background task to execute + Task.WaitAll(t); Assert.AreEqual(1, i); } [Test] [AutoMoqData] - public void RunBackgroundTask__Log_error_when_exception_happen_in_background_task([Frozen] ILogger logger, Exception exception, TaskHelper sut) + public void RunBackgroundTask__Log_Error_When_Exception_Happen_In_Background_Task( + [Frozen] ILogger logger, + Exception exception, + TaskHelper sut) { - sut.RunBackgroundTask(() => throw exception); + Task t = sut.ExecuteBackgroundTask(() => throw exception); - Thread.Sleep(5); // Wait for background task to execute + Task.WaitAll(t); Mock.Get(logger).VerifyLogError(exception, "Exception thrown in a background thread", Times.Once()); } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs index 3ef434edab..efbd0c9e98 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs @@ -108,8 +108,6 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices var mockServerMessenger = new Mock(); - var mockBackOfficeSecurityFactory = new Mock(); - return new ScheduledPublishing( mockRunTimeState.Object, mockMainDom.Object, @@ -117,8 +115,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices _mockContentService.Object, mockUmbracoContextFactory.Object, _mockLogger.Object, - mockServerMessenger.Object, - mockBackOfficeSecurityFactory.Object); + mockServerMessenger.Object); } private void VerifyScheduledPublishingNotPerformed() => VerifyScheduledPublishingPerformed(Times.Never()); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationTests.cs index 58614443b5..97af07d1ed 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationTests.cs @@ -1,7 +1,10 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; +#if DEBUG_SCOPES +using System.Collections.Generic; +#endif using System.Data; using Microsoft.Extensions.Logging; using Moq; diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj b/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj index a85b158810..0839047be4 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Tests.UnitTests.csproj @@ -1,42 +1,42 @@ - + - - Exe - net5.0 - false - Umbraco.Cms.Tests.UnitTests - + + Exe + net5.0 + false + Umbraco.Cms.Tests.UnitTests + - - - - - - - - + + + + + + + + - - - - - - - - + + + + + + + + - - - - + + + + - - - + + + - - - + + + diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index 52722f75d1..aa6677c994 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -436,13 +436,15 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IOptions globalSettings, IUser user) { + var httpContextAccessor = new HttpContextAccessor(); + var mockShortStringHelper = new MockShortStringHelper(); var textService = new Mock(); var contentTypeBaseServiceProvider = new Mock(); contentTypeBaseServiceProvider.Setup(x => x.GetContentTypeOf(It.IsAny())).Returns(new ContentType(mockShortStringHelper, 123)); var contentAppFactories = new Mock>(); var mockContentAppFactoryCollection = new Mock>(); - var hybridBackOfficeSecurityAccessor = new HybridBackofficeSecurityAccessor(new DictionaryAppCache()); + var hybridBackOfficeSecurityAccessor = new BackOfficeSecurityAccessor(httpContextAccessor); var contentAppFactoryCollection = new ContentAppFactoryCollection( contentAppFactories.Object, mockContentAppFactoryCollection.Object, @@ -463,7 +465,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Mock.Get(dataEditor).Setup(x => x.GetValueEditor()).Returns(new TextOnlyValueEditor(Mock.Of(), Mock.Of(), new DataEditorAttribute(Constants.PropertyEditors.Aliases.TextBox, "Test Textbox", "textbox"), textService.Object, Mock.Of(), Mock.Of())); var propertyEditorCollection = new PropertyEditorCollection(new DataEditorCollection(new[] { dataEditor })); - + IMapDefinition memberMapDefinition = new MemberMapDefinition( commonMapper, new CommonTreeNodeMapper(Mock.Of()), @@ -477,7 +479,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers mockPasswordConfig.Object, contentTypeBaseServiceProvider.Object, propertyEditorCollection), - new HttpContextAccessor()); + httpContextAccessor); var map = new MapDefinitionCollection(new List() { @@ -501,7 +503,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers return new MemberController( new DefaultCultureDictionary( new Mock().Object, - new HttpRequestAppCache(() => null)), + NoAppCache.Instance), new LoggerFactory(), mockShortStringHelper, new DefaultEventMessagesFactory( diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/BackgroundTaskRunner.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/BackgroundTaskRunner.cs index a249185c0d..c1555a95a6 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/BackgroundTaskRunner.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/BackgroundTaskRunner.cs @@ -327,8 +327,10 @@ namespace Umbraco.Web.Scheduling // create a new token source since this is a new process _shutdownTokenSource = new CancellationTokenSource(); _shutdownToken = _shutdownTokenSource.Token; - _runningTask = Task.Run(async () => await Pump().ConfigureAwait(false), _shutdownToken); - + using (ExecutionContext.SuppressFlow()) // Do not flow AsyncLocal to the child thread + { + _runningTask = Task.Run(async () => await Pump().ConfigureAwait(false), _shutdownToken); + } _logger.LogDebug("{LogPrefix} Starting", _logPrefix); } @@ -350,7 +352,9 @@ namespace Umbraco.Web.Scheduling var hasTasks = TaskCount > 0; if (!force && hasTasks) + { _logger.LogInformation("{LogPrefix} Waiting for tasks to complete", _logPrefix); + } // complete the queue // will stop waiting on the queue or on a latch @@ -552,16 +556,21 @@ namespace Umbraco.Web.Scheduling try { if (bgTask.IsAsync) + { // configure await = false since we don't care about the context, we're on a background thread. await bgTask.RunAsync(token).ConfigureAwait(false); + } else + { bgTask.Run(); + } } finally // ensure we disposed - unless latched again ie wants to re-run { - var lbgTask = bgTask as ILatchedBackgroundTask; - if (lbgTask == null || lbgTask.IsLatched == false) + if (!(bgTask is ILatchedBackgroundTask lbgTask) || lbgTask.IsLatched == false) + { bgTask.Dispose(); + } } } catch (Exception e) diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/IBackgroundTaskRunner.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/IBackgroundTaskRunner.cs index 32a488fb44..52dc75f3fb 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/IBackgroundTaskRunner.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/IBackgroundTaskRunner.cs @@ -1,6 +1,5 @@ -using System; +using System; using Umbraco.Cms.Core; -using Umbraco.Core; namespace Umbraco.Web.Scheduling { diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/LatchedBackgroundTaskBase.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/LatchedBackgroundTaskBase.cs index 46d80dee70..738bed9b5b 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/LatchedBackgroundTaskBase.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/LegacyBackgroundTask/LatchedBackgroundTaskBase.cs @@ -1,8 +1,7 @@ -using System; +using System; using System.Threading; using System.Threading.Tasks; using Umbraco.Cms.Core; -using Umbraco.Core; namespace Umbraco.Web.Scheduling { diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs index 58900dea5f..6029a069cb 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlStoreFilePersister.cs @@ -1,8 +1,7 @@ -using System; +using System; using System.Threading; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; -using Umbraco.Core; using Umbraco.Web.Scheduling; namespace Umbraco.Tests.LegacyXmlPublishedCache diff --git a/src/Umbraco.Tests/TestHelpers/TestHelper.cs b/src/Umbraco.Tests/TestHelpers/TestHelper.cs index cd9a9a78ce..1a0e9a03d2 100644 --- a/src/Umbraco.Tests/TestHelpers/TestHelper.cs +++ b/src/Umbraco.Tests/TestHelpers/TestHelper.cs @@ -38,7 +38,6 @@ using Umbraco.Cms.Infrastructure.Migrations.Install; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Persistence.SqlCe; using Umbraco.Cms.Tests.Common; -using Umbraco.Core; using Umbraco.Extensions; using Umbraco.Web; using Umbraco.Web.Hosting; diff --git a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs index 47ba5f78ce..fa94cae4fb 100644 --- a/src/Umbraco.Tests/Testing/UmbracoTestBase.cs +++ b/src/Umbraco.Tests/Testing/UmbracoTestBase.cs @@ -300,11 +300,11 @@ namespace Umbraco.Tests.Testing { // imported from TestWithSettingsBase // which was inherited by TestWithApplicationBase so pretty much used everywhere - Umbraco.Web.Composing.Current.UmbracoContextAccessor = new TestUmbracoContextAccessor(); + Current.UmbracoContextAccessor = new TestUmbracoContextAccessor(); // web Builder.Services.AddUnique(Current.UmbracoContextAccessor); - Builder.Services.AddUnique(new HybridBackofficeSecurityAccessor(AppCaches.NoCache.RequestCache)); + Builder.Services.AddUnique(Mock.Of()); Builder.Services.AddUnique(); Builder.WithCollectionBuilder(); diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 930db50828..74ae7604aa 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -86,7 +86,7 @@ 2.0.0-alpha.20200128.15 - 1.11.30 + 1.11.31 @@ -111,17 +111,15 @@ - - + + - + - - diff --git a/src/Umbraco.Tests/Web/HttpCookieExtensionsTests.cs b/src/Umbraco.Tests/Web/HttpCookieExtensionsTests.cs index 73b5987407..3964017e57 100644 --- a/src/Umbraco.Tests/Web/HttpCookieExtensionsTests.cs +++ b/src/Umbraco.Tests/Web/HttpCookieExtensionsTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Net.Http; @@ -6,7 +6,6 @@ using System.Net.Http.Headers; using System.Text; using System.Threading.Tasks; using NUnit.Framework; -using Umbraco.Core; using Umbraco.Web; namespace Umbraco.Tests.Web diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs index 6be3008a32..c11a13e1a1 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs @@ -50,7 +50,8 @@ namespace Umbraco.Extensions .AddPreviewSupport() .AddHostedServices() .AddDistributedCache() - .AddModelsBuilderDashboard(); + .AddModelsBuilderDashboard() + .AddUnattedInstallCreateUser(); /// /// Adds Umbraco back office authentication requirements diff --git a/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs b/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs index 6836931e00..f2ef6b6807 100644 --- a/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs +++ b/src/Umbraco.Web.BackOffice/Filters/CheckIfUserTicketDataIsStaleAttribute.cs @@ -111,8 +111,7 @@ namespace Umbraco.Cms.Web.BackOffice.Filters return; } - var identity = actionContext.HttpContext.User.Identity as ClaimsIdentity; - if (identity == null) + if (actionContext.HttpContext.User.Identity is not ClaimsIdentity identity) { return; } diff --git a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj index e66bfb1577..22799eaa63 100644 --- a/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj +++ b/src/Umbraco.Web.BackOffice/Umbraco.Web.BackOffice.csproj @@ -16,7 +16,7 @@ - + diff --git a/src/Umbraco.Core/Cache/HttpRequestAppCache.cs b/src/Umbraco.Web.Common/Cache/HttpContextRequestAppCache.cs similarity index 53% rename from src/Umbraco.Core/Cache/HttpRequestAppCache.cs rename to src/Umbraco.Web.Common/Cache/HttpContextRequestAppCache.cs index 2e053c3486..f43f9a9a24 100644 --- a/src/Umbraco.Core/Cache/HttpRequestAppCache.cs +++ b/src/Umbraco.Web.Common/Cache/HttpContextRequestAppCache.cs @@ -1,40 +1,38 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Umbraco.Cms.Core.Events; +using Umbraco.Extensions; namespace Umbraco.Cms.Core.Cache { /// - /// Implements a fast on top of HttpContext.Items. + /// Implements a on top of /// /// - /// If no current HttpContext items can be found (no current HttpContext, - /// or no Items...) then this cache acts as a pass-through and does not cache - /// anything. + /// The HttpContext is not thread safe and no part of it is which means we need to include our own thread + /// safety mechanisms. This relies on notifications: and + /// in order to facilitate the correct locking and releasing allocations. + /// /// - public class HttpRequestAppCache : FastDictionaryAppCacheBase, IRequestCache + public class HttpContextRequestAppCache : FastDictionaryAppCacheBase, IRequestCache { - private static object _syncRoot = new object(); // Using this for locking as the SyncRoot property is not available to us - // on the provided collection provided from .NET Core's HttpContext.Items dictionary, - // as it doesn't implement ICollection where SyncRoot is defined. + private readonly IHttpContextAccessor _httpContextAccessor; /// /// Initializes a new instance of the class with a context, for unit tests! /// - public HttpRequestAppCache(Func> requestItems) : base() - { - ContextItems = requestItems; - } - - private Func> ContextItems { get; } + public HttpContextRequestAppCache(IHttpContextAccessor httpContextAccessor) => _httpContextAccessor = httpContextAccessor; public bool IsAvailable => TryGetContextItems(out _); private bool TryGetContextItems(out IDictionary items) { - items = ContextItems?.Invoke(); + items = _httpContextAccessor.HttpContext?.Items; return items != null; } @@ -42,7 +40,10 @@ namespace Umbraco.Cms.Core.Cache public override object Get(string key, Func factory) { //no place to cache so just return the callback result - if (!TryGetContextItems(out var items)) return factory(); + if (!TryGetContextItems(out var items)) + { + return factory(); + } key = GetCacheKey(key); @@ -76,14 +77,22 @@ namespace Umbraco.Cms.Core.Cache //return result.Value; var value = result.Value; // will not throw (safe lazy) - if (value is SafeLazy.ExceptionHolder eh) eh.Exception.Throw(); // throw once! + if (value is SafeLazy.ExceptionHolder eh) + { + eh.Exception.Throw(); // throw once! + } + 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; + if (!TryGetContextItems(out var items)) + { + return false; + } + key = GetCacheKey(key); try { @@ -101,7 +110,11 @@ namespace Umbraco.Cms.Core.Cache public bool Remove(string key) { //no place to cache so just return the callback result - if (!TryGetContextItems(out var items)) return false; + if (!TryGetContextItems(out var items)) + { + return false; + } + key = GetCacheKey(key); try { @@ -122,7 +135,8 @@ namespace Umbraco.Cms.Core.Cache { const string prefix = CacheItemPrefix + "-"; - if (!TryGetContextItems(out var items)) return Enumerable.Empty>(); + if (!TryGetContextItems(out var items)) + return Enumerable.Empty>(); return items.Cast>() .Where(x => x.Key is string s && s.StartsWith(prefix)); @@ -130,7 +144,8 @@ namespace Umbraco.Cms.Core.Cache protected override void RemoveEntry(string key) { - if (!TryGetContextItems(out var items)) return; + if (!TryGetContextItems(out var items)) + return; items.Remove(key); } @@ -144,50 +159,103 @@ namespace Umbraco.Cms.Core.Cache #region Lock - private const string ContextItemsLockKey = "Umbraco.Core.Cache.HttpRequestCache::LockEntered"; - - protected override void EnterReadLock() => EnterWriteLock(); + protected override void EnterReadLock() + { + object locker = GetLock(); + if (locker == null) + { + return; + } + Monitor.Enter(locker); + } protected override void EnterWriteLock() { - if (!TryGetContextItems(out var items)) return; - - // note: cannot keep 'entered' as a class variable here, - // since there is one per request - so storing it within - // ContextItems - which is locked, so this should be safe - - var entered = false; - Monitor.Enter(_syncRoot, ref entered); - items[ContextItemsLockKey] = entered; + object locker = GetLock(); + if (locker == null) + { + return; + } + Monitor.Enter(locker); } - protected override void ExitReadLock() => ExitWriteLock(); + protected override void ExitReadLock() + { + object locker = GetLock(); + if (locker == null) + { + return; + } + if (Monitor.IsEntered(locker)) + { + Monitor.Exit(locker); + } + } protected override void ExitWriteLock() { - if (!TryGetContextItems(out var items)) return; - - var entered = (bool?)items[ContextItemsLockKey] ?? false; - if (entered) - Monitor.Exit(_syncRoot); - items.Remove(ContextItemsLockKey); + object locker = GetLock(); + if (locker == null) + { + return; + } + if (Monitor.IsEntered(locker)) + { + Monitor.Exit(locker); + } } #endregion public IEnumerator> GetEnumerator() { - if (!TryGetContextItems(out var items)) + if (!TryGetContextItems(out IDictionary items)) { yield break; } - foreach (var item in items) + foreach (KeyValuePair item in items) { yield return new KeyValuePair(item.Key.ToString(), item.Value); } } IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + /// + /// Ensures and returns the current lock + /// + /// + private object GetLock() + { + HttpContext httpContext = _httpContextAccessor.HttpContext; + if (httpContext == null) + { + return null; + } + + RequestLock requestLock = httpContext.Features.Get(); + if (requestLock != null) + { + return requestLock.SyncRoot; + } + + IFeatureCollection features = httpContext.Features; + + lock (httpContext) + { + requestLock = new RequestLock(); + features.Set(requestLock); + return requestLock.SyncRoot; + } + } + + /// + /// Used as Scoped instance to allow locking within a request + /// + private class RequestLock + { + public object SyncRoot { get; } = new object(); + } } } diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs index 6c11b91a95..0126d443c9 100644 --- a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs @@ -53,6 +53,8 @@ using Umbraco.Cms.Web.Common.Routing; using Umbraco.Cms.Web.Common.Security; using Umbraco.Cms.Web.Common.Templates; using Umbraco.Cms.Web.Common.UmbracoContext; +using Umbraco.Core.Events; +using static Umbraco.Cms.Core.Cache.HttpContextRequestAppCache; using IHostingEnvironment = Umbraco.Cms.Core.Hosting.IHostingEnvironment; namespace Umbraco.Extensions @@ -85,15 +87,18 @@ namespace Umbraco.Extensions IHostingEnvironment tempHostingEnvironment = GetTemporaryHostingEnvironment(webHostEnvironment, config); - var loggingDir = tempHostingEnvironment.MapPathContentRoot(Cms.Core.Constants.SystemDirectories.LogFiles); + var loggingDir = tempHostingEnvironment.MapPathContentRoot(Constants.SystemDirectories.LogFiles); var loggingConfig = new LoggingConfiguration(loggingDir); services.AddLogger(tempHostingEnvironment, loggingConfig, config); + // Manually create and register the HttpContextAccessor. In theory this should not be registered + // again by the user but if that is the case it's not the end of the world since HttpContextAccessor + // is just based on AsyncLocal, see https://github.com/dotnet/aspnetcore/blob/main/src/Http/Http/src/HttpContextAccessor.cs IHttpContextAccessor httpContextAccessor = new HttpContextAccessor(); services.AddSingleton(httpContextAccessor); - var requestCache = new GenericDictionaryRequestAppCache(() => httpContextAccessor.HttpContext?.Items); + var requestCache = new HttpContextRequestAppCache(httpContextAccessor); var appCaches = AppCaches.Create(requestCache); services.AddUnique(appCaches); @@ -237,7 +242,6 @@ namespace Umbraco.Extensions builder.Services.AddUmbracoImageSharp(builder.Config); // AspNetCore specific services - builder.Services.AddUnique(); builder.Services.AddUnique(); builder.AddNotificationHandler(); @@ -261,10 +265,8 @@ namespace Umbraco.Extensions // register the umbraco context factory builder.Services.AddUnique(); - builder.Services.AddUnique(); - builder.Services.AddUnique(); - builder.AddNotificationHandler(); - builder.Services.AddUnique(); + builder.Services.AddUnique(); + builder.Services.AddUnique(); var umbracoApiControllerTypes = builder.TypeLoader.GetUmbracoApiControllers().ToList(); builder.WithCollectionBuilder() @@ -284,6 +286,8 @@ namespace Umbraco.Extensions builder.Services.AddSingleton(); builder.Services.AddScoped(); + builder.Services.AddScoped(); + builder.Services.AddScoped(); builder.AddHttpClients(); @@ -293,6 +297,12 @@ namespace Umbraco.Extensions return builder; } + public static IUmbracoBuilder AddUnattedInstallCreateUser(this IUmbracoBuilder builder) + { + builder.AddNotificationAsyncHandler(); + return builder; + } + // TODO: Does this need to exist and/or be public? public static IUmbracoBuilder AddWebServer(this IUmbracoBuilder builder) { diff --git a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs index 9513eb2ec2..fbb9e77770 100644 --- a/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs +++ b/src/Umbraco.Web.Common/Extensions/HttpContextExtensions.cs @@ -1,13 +1,27 @@ -using System; +using System; using System.Security.Claims; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; -using Umbraco.Cms.Core.Security; namespace Umbraco.Extensions { public static class HttpContextExtensions { + /// + /// Get the value in the request form or query string for the key + /// + public static string GetRequestValue(this HttpContext context, string key) + { + HttpRequest request = context.Request; + if (!request.HasFormContentType) + { + return request.Query[key]; + } + + string value = request.Form[key]; + return value ?? request.Query[key]; + } + public static void SetPrincipalForRequest(this HttpContext context, ClaimsPrincipal principal) { context.User = principal; diff --git a/src/Umbraco.Web.Common/Install/CreateUnattendedUserNotificationHandler.cs b/src/Umbraco.Web.Common/Install/CreateUnattendedUserNotificationHandler.cs new file mode 100644 index 0000000000..7017298a8e --- /dev/null +++ b/src/Umbraco.Web.Common/Install/CreateUnattendedUserNotificationHandler.cs @@ -0,0 +1,100 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Models.Membership; +using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Services; +using Umbraco.Core.Events; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Web.Common.Install +{ + public class CreateUnattendedUserNotificationHandler : INotificationAsyncHandler + { + private readonly IOptions _unattendedSettings; + private readonly IUserService _userService; + private readonly IServiceScopeFactory _serviceScopeFactory; + + public CreateUnattendedUserNotificationHandler(IOptions unattendedSettings, IUserService userService, IServiceScopeFactory serviceScopeFactory) + { + _unattendedSettings = unattendedSettings; + _userService = userService; + _serviceScopeFactory = serviceScopeFactory; + } + + /// + /// Listening for when the UnattendedInstallNotification fired after a sucessfulk + /// + /// + public async Task HandleAsync(UnattendedInstallNotification notification, CancellationToken cancellationToken) + { + + var unattendedSettings = _unattendedSettings.Value; + // Ensure we have the setting enabled (Sanity check) + // In theory this should always be true as the event only fired when a sucessfull + if (_unattendedSettings.Value.InstallUnattended == false) + { + return; + } + + var unattendedName = unattendedSettings.UnattendedUserName; + var unattendedEmail = unattendedSettings.UnattendedUserEmail; + var unattendedPassword = unattendedSettings.UnattendedUserPassword; + + // Missing configuration values (json, env variables etc) + if (unattendedName.IsNullOrWhiteSpace() + || unattendedEmail.IsNullOrWhiteSpace() + || unattendedPassword.IsNullOrWhiteSpace()) + { + return; + } + + IUser admin = _userService.GetUserById(Core.Constants.Security.SuperUserId); + if (admin == null) + { + throw new InvalidOperationException("Could not find the super user!"); + } + + // User email/login has already been modified + if (admin.Email == unattendedEmail) + { + return; + } + + // Update name, email & login & save user + admin.Name = unattendedName.Trim(); + admin.Email = unattendedEmail.Trim(); + admin.Username = unattendedEmail.Trim(); + _userService.Save(admin); + + // Change Password for the default user we ship out of the box + // Uses same approach as NewInstall Step + using IServiceScope scope = _serviceScopeFactory.CreateScope(); + IBackOfficeUserManager backOfficeUserManager = scope.ServiceProvider.GetRequiredService(); + BackOfficeIdentityUser membershipUser = await backOfficeUserManager.FindByIdAsync(Core.Constants.Security.SuperUserId.ToString()); + if (membershipUser == null) + { + throw new InvalidOperationException($"No user found in membership provider with id of {Core.Constants.Security.SuperUserId}."); + } + + //To change the password here we actually need to reset it since we don't have an old one to use to change + var resetToken = await backOfficeUserManager.GeneratePasswordResetTokenAsync(membershipUser); + if (string.IsNullOrWhiteSpace(resetToken)) + { + throw new InvalidOperationException("Could not reset password: unable to generate internal reset token"); + } + + IdentityResult resetResult = await backOfficeUserManager.ChangePasswordWithResetAsync(membershipUser.Id, resetToken, unattendedPassword.Trim()); + if (!resetResult.Succeeded) + { + throw new InvalidOperationException("Could not reset password: " + string.Join(", ", resetResult.Errors.ToErrorMessage())); + } + } + + } +} diff --git a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs index 467ec29451..02976f9e1d 100644 --- a/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs +++ b/src/Umbraco.Web.Common/Middleware/UmbracoRequestMiddleware.cs @@ -4,12 +4,14 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Logging; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Cms.Infrastructure.PublishedCache; @@ -36,7 +38,6 @@ namespace Umbraco.Cms.Web.Common.Middleware private readonly IUmbracoContextFactory _umbracoContextFactory; private readonly IRequestCache _requestCache; - private readonly IBackOfficeSecurityFactory _backofficeSecurityFactory; private readonly PublishedSnapshotServiceEventHandler _publishedSnapshotServiceEventHandler; private readonly IEventAggregator _eventAggregator; private readonly IHostingEnvironment _hostingEnvironment; @@ -52,7 +53,6 @@ namespace Umbraco.Cms.Web.Common.Middleware ILogger logger, IUmbracoContextFactory umbracoContextFactory, IRequestCache requestCache, - IBackOfficeSecurityFactory backofficeSecurityFactory, PublishedSnapshotServiceEventHandler publishedSnapshotServiceEventHandler, IEventAggregator eventAggregator, IProfiler profiler, @@ -61,7 +61,6 @@ namespace Umbraco.Cms.Web.Common.Middleware _logger = logger; _umbracoContextFactory = umbracoContextFactory; _requestCache = requestCache; - _backofficeSecurityFactory = backofficeSecurityFactory; _publishedSnapshotServiceEventHandler = publishedSnapshotServiceEventHandler; _eventAggregator = eventAggregator; _hostingEnvironment = hostingEnvironment; @@ -84,11 +83,6 @@ namespace Umbraco.Cms.Web.Common.Middleware EnsureContentCacheInitialized(); - // TODO: This dependency chain is broken and needs to be fixed. - // This is required to be called before EnsureUmbracoContext else the UmbracoContext's IBackOfficeSecurity instance is null - // This is ugly Temporal Coupling which also means that developers can no longer just use IUmbracoContextFactory the - // way it was intended. - _backofficeSecurityFactory.EnsureBackOfficeSecurity(); UmbracoContextReference umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext(); Uri currentApplicationUrl = GetApplicationUrlFromCurrentRequest(context.Request); @@ -133,10 +127,11 @@ namespace Umbraco.Cms.Web.Common.Middleware try { - DisposeRequestCacheItems(_logger, _requestCache, context.Request); + DisposeHttpContextItems(context.Request); } finally { + // Dispose the umbraco context reference which will in turn dispose the UmbracoContext itself. umbracoContextReference.Dispose(); } } @@ -159,9 +154,9 @@ namespace Umbraco.Cms.Web.Common.Middleware } /// - /// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request + /// Dispose some request scoped objects that we are maintaining the lifecycle for. /// - private static void DisposeRequestCacheItems(ILogger logger, IRequestCache requestCache, HttpRequest request) + private void DisposeHttpContextItems(HttpRequest request) { // do not process if client-side request if (request.IsClientSideRequest()) @@ -169,37 +164,9 @@ namespace Umbraco.Cms.Web.Common.Middleware return; } - // get a list of keys to dispose - var keys = new HashSet(); - foreach (var i in requestCache) - { - if (i.Value is IDisposeOnRequestEnd || i.Key is IDisposeOnRequestEnd) - { - keys.Add(i.Key); - } - } - - // dispose each item and key that was found as disposable. - foreach (var k in keys) - { - try - { - requestCache.Get(k).DisposeIfDisposable(); - } - catch (Exception ex) - { - logger.LogError("Could not dispose item with key " + k, ex); - } - - try - { - k.DisposeIfDisposable(); - } - catch (Exception ex) - { - logger.LogError("Could not dispose item key " + k, ex); - } - } + // ensure this is disposed by DI at the end of the request + IHttpScopeReference httpScopeReference = request.HttpContext.RequestServices.GetRequiredService(); + httpScopeReference.Register(); } /// diff --git a/src/Umbraco.Web.Common/ModelsBuilder/PureLiveModelFactory.cs b/src/Umbraco.Web.Common/ModelsBuilder/PureLiveModelFactory.cs index 8a17419964..bd4ccf3d60 100644 --- a/src/Umbraco.Web.Common/ModelsBuilder/PureLiveModelFactory.cs +++ b/src/Umbraco.Web.Common/ModelsBuilder/PureLiveModelFactory.cs @@ -774,9 +774,10 @@ namespace Umbraco.Cms.Web.Common.ModelsBuilder } public void Stop(bool immediate) - { + { _watcher.EnableRaisingEvents = false; _watcher.Dispose(); + _locker.Dispose(); _hostingLifetime.UnregisterObject(this); } diff --git a/src/Umbraco.Web.Common/ModelsBuilder/RefreshingRazorViewEngine.cs b/src/Umbraco.Web.Common/ModelsBuilder/RefreshingRazorViewEngine.cs index 048a6e2965..c49668451a 100644 --- a/src/Umbraco.Web.Common/ModelsBuilder/RefreshingRazorViewEngine.cs +++ b/src/Umbraco.Web.Common/ModelsBuilder/RefreshingRazorViewEngine.cs @@ -69,9 +69,10 @@ namespace Umbraco.Cms.Web.Common.ModelsBuilder /// This is used so that when new PureLive models are built, the entire razor stack is re-constructed so all razor /// caches and assembly references, etc... are cleared. /// - internal class RefreshingRazorViewEngine : IRazorViewEngine + internal class RefreshingRazorViewEngine : IRazorViewEngine, IDisposable { private IRazorViewEngine _current; + private bool _disposedValue; private readonly PureLiveModelFactory _pureLiveModelFactory; private readonly Func _defaultRazorViewEngineFactory; private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(); @@ -172,5 +173,24 @@ namespace Umbraco.Cms.Web.Common.ModelsBuilder _locker.ExitReadLock(); } } + + protected virtual void Dispose(bool disposing) + { + if (!_disposedValue) + { + if (disposing) + { + _locker.Dispose(); + } + + _disposedValue = true; + } + } + + public void Dispose() + { + // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method + Dispose(disposing: true); + } } } diff --git a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs index 34326083d3..899373f35e 100644 --- a/src/Umbraco.Web.Common/Profiler/WebProfiler.cs +++ b/src/Umbraco.Web.Common/Profiler/WebProfiler.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Linq; using System.Threading; using Microsoft.AspNetCore.Http; diff --git a/src/Umbraco.Web.Common/Security/BackOfficeSecurityAccessor.cs b/src/Umbraco.Web.Common/Security/BackOfficeSecurityAccessor.cs new file mode 100644 index 0000000000..d98877e1c7 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/BackOfficeSecurityAccessor.cs @@ -0,0 +1,22 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.Security; + +namespace Umbraco.Cms.Web.Common.Security +{ + public class BackOfficeSecurityAccessor : IBackOfficeSecurityAccessor + { + private readonly IHttpContextAccessor _httpContextAccessor; + + /// + /// Initializes a new instance of the class. + /// + public BackOfficeSecurityAccessor(IHttpContextAccessor httpContextAccessor) => _httpContextAccessor = httpContextAccessor; + + /// + /// Gets the current object. + /// + public IBackOfficeSecurity BackOfficeSecurity + => _httpContextAccessor.HttpContext?.RequestServices?.GetService(); + } +} diff --git a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs index d7a0aeb043..24a5b01832 100644 --- a/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs +++ b/src/Umbraco.Web.Common/Security/BackofficeSecurity.cs @@ -1,4 +1,4 @@ -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models.Membership; using Umbraco.Cms.Core.Security; diff --git a/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs b/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs deleted file mode 100644 index ad0731f790..0000000000 --- a/src/Umbraco.Web.Common/Security/BackofficeSecurityFactory.cs +++ /dev/null @@ -1,34 +0,0 @@ -using Microsoft.AspNetCore.Http; -using Umbraco.Cms.Core.Security; -using Umbraco.Cms.Core.Services; - -namespace Umbraco.Cms.Web.Common.Security -{ - // TODO: This is only for the back office, does it need to be in common? YES currently UmbracoContext has an transitive dependency on this which needs to be fixed/reviewed. - - public class BackOfficeSecurityFactory: IBackOfficeSecurityFactory - { - private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; - private readonly IUserService _userService; - private readonly IHttpContextAccessor _httpContextAccessor; - - public BackOfficeSecurityFactory( - IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IUserService userService, - IHttpContextAccessor httpContextAccessor) - { - _backOfficeSecurityAccessor = backOfficeSecurityAccessor; - _userService = userService; - _httpContextAccessor = httpContextAccessor; - } - - public void EnsureBackOfficeSecurity() - { - if (_backOfficeSecurityAccessor.BackOfficeSecurity is null) - { - _backOfficeSecurityAccessor.BackOfficeSecurity = new BackOfficeSecurity(_userService, _httpContextAccessor); - } - - } - } -} diff --git a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs new file mode 100644 index 0000000000..2b90b5ad14 --- /dev/null +++ b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityAccessor.cs @@ -0,0 +1,23 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Umbraco.Cms.Core.Security; + +namespace Umbraco.Cms.Web.Common.Security +{ + + public class UmbracoWebsiteSecurityAccessor : IUmbracoWebsiteSecurityAccessor + { + private readonly IHttpContextAccessor _httpContextAccessor; + + /// + /// Initializes a new instance of the class. + /// + public UmbracoWebsiteSecurityAccessor(IHttpContextAccessor httpContextAccessor) => _httpContextAccessor = httpContextAccessor; + + /// + /// Gets or sets the object. + /// + public IUmbracoWebsiteSecurity WebsiteSecurity + => _httpContextAccessor.HttpContext?.RequestServices?.GetService(); + } +} diff --git a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs b/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs deleted file mode 100644 index ec256a86cb..0000000000 --- a/src/Umbraco.Web.Common/Security/UmbracoWebsiteSecurityFactory.cs +++ /dev/null @@ -1,46 +0,0 @@ -using Microsoft.AspNetCore.Http; -using Umbraco.Cms.Core.Events; -using Umbraco.Cms.Core.Security; -using Umbraco.Cms.Core.Services; -using Umbraco.Cms.Core.Strings; - -namespace Umbraco.Cms.Web.Common.Security -{ - /// - /// Ensures that the is populated on a front-end request - /// - internal sealed class UmbracoWebsiteSecurityFactory : INotificationHandler - { - private readonly IUmbracoWebsiteSecurityAccessor _umbracoWebsiteSecurityAccessor; - private readonly IHttpContextAccessor _httpContextAccessor; - private readonly IMemberService _memberService; - private readonly IMemberTypeService _memberTypeService; - private readonly IShortStringHelper _shortStringHelper; - - public UmbracoWebsiteSecurityFactory( - IUmbracoWebsiteSecurityAccessor umbracoWebsiteSecurityAccessor, - IHttpContextAccessor httpContextAccessor, - IMemberService memberService, - IMemberTypeService memberTypeService, - IShortStringHelper shortStringHelper) - { - _umbracoWebsiteSecurityAccessor = umbracoWebsiteSecurityAccessor; - _httpContextAccessor = httpContextAccessor; - _memberService = memberService; - _memberTypeService = memberTypeService; - _shortStringHelper = shortStringHelper; - } - - public void Handle(UmbracoRoutedRequest notification) - { - if (_umbracoWebsiteSecurityAccessor.WebsiteSecurity is null) - { - _umbracoWebsiteSecurityAccessor.WebsiteSecurity = new UmbracoWebsiteSecurity( - _httpContextAccessor, - _memberService, - _memberTypeService, - _shortStringHelper); - } - } - } -} diff --git a/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj b/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj index fdf14a3408..34a3155d21 100644 --- a/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj +++ b/src/Umbraco.Web.Common/Umbraco.Web.Common.csproj @@ -24,10 +24,12 @@ + + - + diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs index c31fe4dd3e..50dcb4182c 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContext.cs @@ -1,10 +1,11 @@ using System; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; using Umbraco.Extensions; @@ -13,17 +14,17 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext /// /// Class that encapsulates Umbraco information of a specific HTTP request /// - public class UmbracoContext : DisposableObjectSlim, IDisposeOnRequestEnd, IUmbracoContext + public class UmbracoContext : DisposableObjectSlim, IUmbracoContext { private readonly IHostingEnvironment _hostingEnvironment; private readonly UriUtility _uriUtility; private readonly ICookieManager _cookieManager; - private readonly IRequestAccessor _requestAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly Lazy _publishedSnapshot; private string _previewToken; private bool? _previewing; - private readonly IBackOfficeSecurity _backofficeSecurity; private readonly UmbracoRequestPaths _umbracoRequestPaths; + private Uri _requestUrl; private Uri _originalRequestUrl; private Uri _cleanedUmbracoUrl; @@ -33,13 +34,12 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext // warn: does *not* manage setting any IUmbracoContextAccessor internal UmbracoContext( IPublishedSnapshotService publishedSnapshotService, - IBackOfficeSecurity backofficeSecurity, UmbracoRequestPaths umbracoRequestPaths, IHostingEnvironment hostingEnvironment, IVariationContextAccessor variationContextAccessor, UriUtility uriUtility, ICookieManager cookieManager, - IRequestAccessor requestAccessor) + IHttpContextAccessor httpContextAccessor) { if (publishedSnapshotService == null) { @@ -50,11 +50,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _uriUtility = uriUtility; _hostingEnvironment = hostingEnvironment; _cookieManager = cookieManager; - _requestAccessor = requestAccessor; - + _httpContextAccessor = httpContextAccessor; ObjectCreated = DateTime.Now; UmbracoRequestId = Guid.NewGuid(); - _backofficeSecurity = backofficeSecurity ?? throw new ArgumentNullException(nameof(backofficeSecurity)); _umbracoRequestPaths = umbracoRequestPaths; // beware - we cannot expect a current user here, so detecting preview mode must be a lazy thing @@ -72,6 +70,11 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext /// internal Guid UmbracoRequestId { get; } + // lazily get/create a Uri for the current request + private Uri RequestUrl => _requestUrl ??= _httpContextAccessor.HttpContext is null + ? null + : new Uri(_httpContextAccessor.HttpContext.Request.GetEncodedUrl()); + /// // set the urls lazily, no need to allocate until they are needed... // NOTE: The request will not be available during app startup so we can only set this to an absolute URL of localhost, this @@ -79,7 +82,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext // 'could' still generate URLs during startup BUT any domain driven URL generation will not work because it is NOT possible to get // the current domain during application startup. // see: http://issues.umbraco.org/issue/U4-1890 - public Uri OriginalRequestUrl => _originalRequestUrl ?? (_originalRequestUrl = _requestAccessor.GetRequestUrl() ?? new Uri("http://localhost")); + public Uri OriginalRequestUrl => _originalRequestUrl ?? (_originalRequestUrl = RequestUrl ?? new Uri("http://localhost")); /// // set the urls lazily, no need to allocate until they are needed... @@ -106,8 +109,8 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext /// public bool IsDebug => // NOTE: the request can be null during app startup! _hostingEnvironment.IsDebugMode - && (string.IsNullOrEmpty(_requestAccessor.GetRequestValue("umbdebugshowtrace")) == false - || string.IsNullOrEmpty(_requestAccessor.GetRequestValue("umbdebug")) == false + && (string.IsNullOrEmpty(_httpContextAccessor.HttpContext.GetRequestValue("umbdebugshowtrace")) == false + || string.IsNullOrEmpty(_httpContextAccessor.HttpContext.GetRequestValue("umbdebug")) == false || string.IsNullOrEmpty(_cookieManager.GetCookieValue("UMB-DEBUG")) == false); /// @@ -140,10 +143,9 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private void DetectPreviewMode() { - Uri requestUrl = _requestAccessor.GetRequestUrl(); - if (requestUrl != null - && _umbracoRequestPaths.IsBackOfficeRequest(requestUrl.AbsolutePath) == false - && _backofficeSecurity.CurrentUser != null) + if (RequestUrl != null + && _umbracoRequestPaths.IsBackOfficeRequest(RequestUrl.AbsolutePath) == false + && _httpContextAccessor.HttpContext?.GetCurrentIdentity() != null) { var previewToken = _cookieManager.GetCookieValue(Core.Constants.Web.PreviewCookieName); // may be null or empty _previewToken = previewToken.IsNullOrWhiteSpace() ? null : previewToken; diff --git a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs index 8d199febd0..b41d96e0d0 100644 --- a/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs +++ b/src/Umbraco.Web.Common/UmbracoContext/UmbracoContextFactory.cs @@ -1,10 +1,10 @@ using System; +using Microsoft.AspNetCore.Http; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Hosting; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Routing; -using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Web; namespace Umbraco.Cms.Web.Common.UmbracoContext @@ -18,12 +18,10 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext private readonly IPublishedSnapshotService _publishedSnapshotService; private readonly IVariationContextAccessor _variationContextAccessor; private readonly IDefaultCultureAccessor _defaultCultureAccessor; - private readonly UmbracoRequestPaths _umbracoRequestPaths; private readonly IHostingEnvironment _hostingEnvironment; private readonly ICookieManager _cookieManager; - private readonly IRequestAccessor _requestAccessor; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IHttpContextAccessor _httpContextAccessor; private readonly UriUtility _uriUtility; /// @@ -38,8 +36,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext IHostingEnvironment hostingEnvironment, UriUtility uriUtility, ICookieManager cookieManager, - IRequestAccessor requestAccessor, - IBackOfficeSecurityAccessor backofficeSecurityAccessor) + IHttpContextAccessor httpContextAccessor) { _umbracoContextAccessor = umbracoContextAccessor ?? throw new ArgumentNullException(nameof(umbracoContextAccessor)); _publishedSnapshotService = publishedSnapshotService ?? throw new ArgumentNullException(nameof(publishedSnapshotService)); @@ -49,8 +46,7 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext _hostingEnvironment = hostingEnvironment ?? throw new ArgumentNullException(nameof(hostingEnvironment)); _uriUtility = uriUtility ?? throw new ArgumentNullException(nameof(uriUtility)); _cookieManager = cookieManager ?? throw new ArgumentNullException(nameof(cookieManager)); - _requestAccessor = requestAccessor ?? throw new ArgumentNullException(nameof(requestAccessor)); - _backofficeSecurityAccessor = backofficeSecurityAccessor ?? throw new ArgumentNullException(nameof(backofficeSecurityAccessor)); + _httpContextAccessor = httpContextAccessor ?? throw new ArgumentNullException(nameof(httpContextAccessor)); } private IUmbracoContext CreateUmbracoContext() @@ -75,13 +71,12 @@ namespace Umbraco.Cms.Web.Common.UmbracoContext return new UmbracoContext( _publishedSnapshotService, - _backofficeSecurityAccessor.BackOfficeSecurity, _umbracoRequestPaths, _hostingEnvironment, _variationContextAccessor, _uriUtility, _cookieManager, - _requestAccessor); + _httpContextAccessor); } /// diff --git a/src/Umbraco.Web.UI.NetCore/Umbraco.Web.UI.NetCore.csproj b/src/Umbraco.Web.UI.NetCore/Umbraco.Web.UI.NetCore.csproj index 066a27ccee..5aeb0e8bb1 100644 --- a/src/Umbraco.Web.UI.NetCore/Umbraco.Web.UI.NetCore.csproj +++ b/src/Umbraco.Web.UI.NetCore/Umbraco.Web.UI.NetCore.csproj @@ -1,10 +1,9 @@ - - net5.0 - Umbraco.Cms.Web.UI.NetCore - Umbraco.Cms.Web.UI.NetCore - + + net5.0 + Umbraco.Cms.Web.UI.NetCore + bin\Release\Umbraco.Web.UI.NetCore.xml @@ -74,11 +73,11 @@ - - - - - + + + + + false diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 6205dc67a3..02a514e1ea 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -83,7 +83,7 @@ - + @@ -97,7 +97,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all - + 3.5.3 @@ -312,4 +312,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs index c5d13b721a..b8446ce718 100644 --- a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs +++ b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs @@ -121,9 +121,6 @@ namespace Umbraco.Cms.Web.Website.Routing // Store the route values as a httpcontext feature httpContext.Features.Set(umbracoRouteValues); - // publish an event that we've routed a request - await _eventAggregator.PublishAsync(new UmbracoRoutedRequest(_umbracoContextAccessor.UmbracoContext)); - // Need to check if there is form data being posted back to an Umbraco URL PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values); if (postedInfo != null) diff --git a/src/Umbraco.Web/AspNet/AspNetIpResolver.cs b/src/Umbraco.Web/AspNet/AspNetIpResolver.cs index 5a14592aaf..6a5766e5c4 100644 --- a/src/Umbraco.Web/AspNet/AspNetIpResolver.cs +++ b/src/Umbraco.Web/AspNet/AspNetIpResolver.cs @@ -1,6 +1,5 @@ using System.Web; using Umbraco.Cms.Core.Net; -using Umbraco.Core; namespace Umbraco.Web { diff --git a/src/Umbraco.Web/HttpContextExtensions.cs b/src/Umbraco.Web/HttpContextExtensions.cs index 76cfc0aff1..1e29f76a1a 100644 --- a/src/Umbraco.Web/HttpContextExtensions.cs +++ b/src/Umbraco.Web/HttpContextExtensions.cs @@ -1,6 +1,6 @@ using System.Web; -namespace Umbraco.Core +namespace Umbraco.Web { public static class HttpContextExtensions { diff --git a/src/Umbraco.Web/Mvc/UmbracoVirtualNodeByUdiRouteHandler.cs b/src/Umbraco.Web/Mvc/UmbracoVirtualNodeByUdiRouteHandler.cs index c74045c0d2..3ba24945b7 100644 --- a/src/Umbraco.Web/Mvc/UmbracoVirtualNodeByUdiRouteHandler.cs +++ b/src/Umbraco.Web/Mvc/UmbracoVirtualNodeByUdiRouteHandler.cs @@ -1,8 +1,7 @@ -using System.Web.Routing; +using System.Web.Routing; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Web; -using Umbraco.Core; namespace Umbraco.Web.Mvc { diff --git a/src/Umbraco.Web/Runtime/WebFinalComposer.cs b/src/Umbraco.Web/Runtime/WebFinalComposer.cs index 2c2a2423e6..818bff521a 100644 --- a/src/Umbraco.Web/Runtime/WebFinalComposer.cs +++ b/src/Umbraco.Web/Runtime/WebFinalComposer.cs @@ -1,5 +1,4 @@ -using Umbraco.Cms.Core.Composing; -using Umbraco.Core; +using Umbraco.Cms.Core.Composing; namespace Umbraco.Web.Runtime { diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index fbe21702f9..09a9ee4384 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -65,12 +65,12 @@ 2.0.0-alpha.20200128.15 - + 5.0.376 - 2.7.0.100 + 2.9.1 @@ -78,8 +78,8 @@ - - + + @@ -91,21 +91,18 @@ runtime; build; native; contentfiles; analyzers; buildtransitive all - + - 3.5.3 + 3.5.4 runtime; build; native; contentfiles; analyzers all - - 1.0.5 - @@ -159,7 +156,6 @@ - diff --git a/src/Umbraco.Web/UmbracoApplication.cs b/src/Umbraco.Web/UmbracoApplication.cs index 8bc0720606..327537ee00 100644 --- a/src/Umbraco.Web/UmbracoApplication.cs +++ b/src/Umbraco.Web/UmbracoApplication.cs @@ -1,8 +1,7 @@ -using System.Runtime.InteropServices; +using System.Runtime.InteropServices; using System.Web; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core.Logging; -using Umbraco.Core; using Umbraco.Web.Runtime; using ConnectionStrings = Umbraco.Cms.Core.Configuration.Models.ConnectionStrings; diff --git a/src/Umbraco.Web/UmbracoContext.cs b/src/Umbraco.Web/UmbracoContext.cs index 3b2af1e0ff..4aae46e44c 100644 --- a/src/Umbraco.Web/UmbracoContext.cs +++ b/src/Umbraco.Web/UmbracoContext.cs @@ -13,7 +13,7 @@ namespace Umbraco.Web { // NOTE: has all been ported to netcore but exists here just to keep the build working for tests - public class UmbracoContext : DisposableObjectSlim, IDisposeOnRequestEnd, IUmbracoContext + public class UmbracoContext : DisposableObjectSlim, IUmbracoContext { private readonly IHttpContextAccessor _httpContextAccessor; private readonly Lazy _publishedSnapshot; diff --git a/src/Umbraco.Web/UmbracoDbProviderFactoryCreator.cs b/src/Umbraco.Web/UmbracoDbProviderFactoryCreator.cs index d0f63f5242..04dba05a90 100644 --- a/src/Umbraco.Web/UmbracoDbProviderFactoryCreator.cs +++ b/src/Umbraco.Web/UmbracoDbProviderFactoryCreator.cs @@ -5,7 +5,6 @@ using Umbraco.Cms.Infrastructure.Migrations.Install; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.SqlSyntax; using Umbraco.Cms.Persistence.SqlCe; -using Umbraco.Core; using Constants = Umbraco.Cms.Core.Constants; namespace Umbraco.Web diff --git a/src/Umbraco.Web/WebApi/Filters/FeatureAuthorizeAttribute.cs b/src/Umbraco.Web/WebApi/Filters/FeatureAuthorizeAttribute.cs index af5987757d..bae3ad05c2 100644 --- a/src/Umbraco.Web/WebApi/Filters/FeatureAuthorizeAttribute.cs +++ b/src/Umbraco.Web/WebApi/Filters/FeatureAuthorizeAttribute.cs @@ -1,7 +1,6 @@ -using System.Web.Http; +using System.Web.Http; using System.Web.Http.Controllers; using Umbraco.Web.Composing; -using Umbraco.Core; using Microsoft.Extensions.DependencyInjection; using Umbraco.Cms.Core.Features; diff --git a/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs b/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs deleted file mode 100644 index 21c783177d..0000000000 --- a/src/Umbraco.Web/WebApi/HttpActionContextExtensions.cs +++ /dev/null @@ -1,105 +0,0 @@ -using System; -using System.IO; -using System.Net; -using System.Net.Http; -using System.Threading.Tasks; -using System.Web.Http; -using System.Web.Http.Controllers; -using Microsoft.Extensions.DependencyInjection; -using Newtonsoft.Json; -using Umbraco.Web.Composing; - -namespace Umbraco.Web.WebApi -{ - internal static class HttpActionContextExtensions - { - /// - /// Helper method to get a model from a multipart request and ensure that the model is validated - /// - /// - /// - /// - /// - /// - /// - //public static T GetModelFromMultipartRequest(this HttpActionContext actionContext, MultipartFormDataStreamProvider result, string requestKey, string validationKeyPrefix = "") - //{ - // if (result.FormData[requestKey/*"contentItem"*/] == null) - // { - // var response = actionContext.Request.CreateResponse(HttpStatusCode.BadRequest); - // response.ReasonPhrase = $"The request was not formatted correctly and is missing the '{requestKey}' parameter"; - // throw new HttpResponseException(response); - // } - - // //get the string json from the request - // var contentItem = result.FormData[requestKey]; - - // //deserialize into our model - // var model = JsonConvert.DeserializeObject(contentItem); - - // //get the default body validator and validate the object - // var bodyValidator = actionContext.ControllerContext.Configuration.Services.GetBodyModelValidator(); - // var metadataProvider = actionContext.ControllerContext.Configuration.Services.GetModelMetadataProvider(); - // //by default all validation errors will not contain a prefix (empty string) unless specified - // bodyValidator.Validate(model, typeof(T), metadataProvider, actionContext, validationKeyPrefix); - - // return model; - //} - - /// - /// Helper method to get the from the request in a non-async manner - /// - /// - /// - /// - //public static MultipartFormDataStreamProvider ReadAsMultipart(this HttpActionContext actionContext, string rootVirtualPath) - //{ - // if (actionContext.Request.Content.IsMimeMultipartContent() == false) - // { - // throw new HttpResponseException(HttpStatusCode.UnsupportedMediaType); - // } - - // var hostingEnvironment = Current.Factory.GetRequiredService(); - // var root = hostingEnvironment.MapPathContentRoot(rootVirtualPath); - // //ensure it exists - // Directory.CreateDirectory(root); - // var provider = new MultipartFormDataStreamProvider(root); - - // var request = actionContext.Request; - // var content = request.Content; - - // // Note: YES this is super strange, ugly, and weird. - // // One would think that you could just do: - // // - // //var result = content.ReadAsMultipartAsync(provider).Result; - // // - // // But it deadlocks. See https://stackoverflow.com/questions/15201255 for details, which - // // points to https://msdn.microsoft.com/en-us/magazine/jj991977.aspx which contains more - // // details under "Async All the Way" - see also https://olitee.com/2015/01/c-async-await-common-deadlock-scenario/ - // // which contains a simplified explanation: ReadAsMultipartAsync is meant to be awaited, - // // not used in the non-async .Result way, and there is nothing we can do about it. - // // - // // Alas, model binders cannot be async "all the way", so we have to wrap in a task, to - // // force proper threading, and then it works. - - // MultipartFormDataStreamProvider result = null; - // var task = Task.Run(() => content.ReadAsMultipartAsync(provider)) - // .ContinueWith(x => - // { - // if (x.IsFaulted && x.Exception != null) - // { - // throw x.Exception; - // } - // result = x.ConfigureAwait(false).GetAwaiter().GetResult(); - // }, - // // Must explicitly specify this, see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html - // TaskScheduler.Default); - // task.Wait(); - - // if (result == null) - // throw new InvalidOperationException("Could not read multi-part message"); - - // return result; - //} - } -} diff --git a/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs b/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs index 0c55db10a4..2d53393d21 100644 --- a/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs +++ b/src/Umbraco.Web/WebApi/HttpRequestMessageExtensions.cs @@ -4,7 +4,6 @@ using System.Net.Http; using System.Web; using Microsoft.Owin; using Umbraco.Cms.Core; -using Umbraco.Core; namespace Umbraco.Web.WebApi {