From 68f1120b4fce85f946cb2d531fdd88c816663cd2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Sep 2017 13:18:34 +1000 Subject: [PATCH 1/3] U4-10118 Umbraco.Core.IO.FileSystemProviderManager is simply not testable and cannot be properly used or setup in tests --- .../FileSystemProviderElement.cs | 27 ++++++++- .../FileSystemProviderElementCollection.cs | 17 +++++- .../FileSystemProvidersSection.cs | 14 ++++- .../IFileSystemProviderElement.cs | 11 ++++ .../IFileSystemProvidersSection.cs | 9 +++ .../IO/FileSystemProviderManager.cs | 56 +++++++++++++++---- src/Umbraco.Core/Umbraco.Core.csproj | 2 + 7 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 src/Umbraco.Core/Configuration/IFileSystemProviderElement.cs create mode 100644 src/Umbraco.Core/Configuration/IFileSystemProvidersSection.cs diff --git a/src/Umbraco.Core/Configuration/FileSystemProviderElement.cs b/src/Umbraco.Core/Configuration/FileSystemProviderElement.cs index c0773e64e3..a1221bc0d4 100644 --- a/src/Umbraco.Core/Configuration/FileSystemProviderElement.cs +++ b/src/Umbraco.Core/Configuration/FileSystemProviderElement.cs @@ -6,7 +6,7 @@ using System.Text; namespace Umbraco.Core.Configuration { - public class FileSystemProviderElement : ConfigurationElement + public class FileSystemProviderElement : ConfigurationElement, IFileSystemProviderElement { private const string ALIAS_KEY = "alias"; private const string TYPE_KEY = "type"; @@ -38,5 +38,30 @@ namespace Umbraco.Core.Configuration return ((KeyValueConfigurationCollection)(base[PARAMETERS_KEY])); } } + + string IFileSystemProviderElement.Alias + { + get { return Alias; } + } + + string IFileSystemProviderElement.Type + { + get { return Type; } + } + + private IDictionary _params; + IDictionary IFileSystemProviderElement.Parameters + { + get + { + if (_params != null) return _params; + _params = new Dictionary(); + foreach (KeyValueConfigurationElement element in Parameters) + { + _params.Add(element.Key, element.Value); + } + return _params; + } + } } } diff --git a/src/Umbraco.Core/Configuration/FileSystemProviderElementCollection.cs b/src/Umbraco.Core/Configuration/FileSystemProviderElementCollection.cs index 0604560939..26957dd68e 100644 --- a/src/Umbraco.Core/Configuration/FileSystemProviderElementCollection.cs +++ b/src/Umbraco.Core/Configuration/FileSystemProviderElementCollection.cs @@ -7,7 +7,7 @@ using System.Text; namespace Umbraco.Core.Configuration { [ConfigurationCollection(typeof(FileSystemProviderElement), AddItemName = "Provider")] - public class FileSystemProviderElementCollection : ConfigurationElementCollection + public class FileSystemProviderElementCollection : ConfigurationElementCollection, IEnumerable { protected override ConfigurationElement CreateNewElement() { @@ -19,12 +19,25 @@ namespace Umbraco.Core.Configuration return ((FileSystemProviderElement)(element)).Alias; } - new public FileSystemProviderElement this[string key] + public new FileSystemProviderElement this[string key] { get { return (FileSystemProviderElement)BaseGet(key); } } + + IEnumerator IEnumerable.GetEnumerator() + { + for (var i = 0; i < Count; i++) + { + yield return BaseGet(i) as IFileSystemProviderElement; + } + } + + System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } } } diff --git a/src/Umbraco.Core/Configuration/FileSystemProvidersSection.cs b/src/Umbraco.Core/Configuration/FileSystemProvidersSection.cs index 0da1cb802a..fa32fe0885 100644 --- a/src/Umbraco.Core/Configuration/FileSystemProvidersSection.cs +++ b/src/Umbraco.Core/Configuration/FileSystemProvidersSection.cs @@ -6,7 +6,7 @@ using System.Text; namespace Umbraco.Core.Configuration { - public class FileSystemProvidersSection : ConfigurationSection + public class FileSystemProvidersSection : ConfigurationSection, IFileSystemProvidersSection { [ConfigurationProperty("", IsDefaultCollection = true, IsRequired = true)] @@ -14,5 +14,17 @@ namespace Umbraco.Core.Configuration { get { return ((FileSystemProviderElementCollection)(base[""])); } } + + private IDictionary _providers; + + IDictionary IFileSystemProvidersSection.Providers + { + get + { + if (_providers != null) return _providers; + _providers = Providers.ToDictionary(x => x.Alias, x => x); + return _providers; + } + } } } diff --git a/src/Umbraco.Core/Configuration/IFileSystemProviderElement.cs b/src/Umbraco.Core/Configuration/IFileSystemProviderElement.cs new file mode 100644 index 0000000000..9427f42b68 --- /dev/null +++ b/src/Umbraco.Core/Configuration/IFileSystemProviderElement.cs @@ -0,0 +1,11 @@ +using System.Collections.Generic; + +namespace Umbraco.Core.Configuration +{ + public interface IFileSystemProviderElement + { + string Alias { get; } + string Type { get; } + IDictionary Parameters { get; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Configuration/IFileSystemProvidersSection.cs b/src/Umbraco.Core/Configuration/IFileSystemProvidersSection.cs new file mode 100644 index 0000000000..108d5a87de --- /dev/null +++ b/src/Umbraco.Core/Configuration/IFileSystemProvidersSection.cs @@ -0,0 +1,9 @@ +using System.Collections.Generic; + +namespace Umbraco.Core.Configuration +{ + public interface IFileSystemProvidersSection + { + IDictionary Providers { get; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/IO/FileSystemProviderManager.cs b/src/Umbraco.Core/IO/FileSystemProviderManager.cs index d2ae8a0612..abfb1d1a33 100644 --- a/src/Umbraco.Core/IO/FileSystemProviderManager.cs +++ b/src/Umbraco.Core/IO/FileSystemProviderManager.cs @@ -11,7 +11,7 @@ namespace Umbraco.Core.IO { public class FileSystemProviderManager { - private readonly FileSystemProvidersSection _config; + private readonly IFileSystemProvidersSection _config; private readonly ConcurrentSet _wrappers = new ConcurrentSet(); private readonly ConcurrentDictionary _providerLookup = new ConcurrentDictionary(); @@ -28,13 +28,31 @@ namespace Umbraco.Core.IO private ShadowWrapper _mvcViewsFileSystem; #region Singleton & Constructor + + //ensures that the construction of the singleton only happens when Current is accessed, otherwise any reference to this class would cause + //the ctor to be invoked which can lead to problems outside of a web app. + private static readonly Lazy Instance = new Lazy(() => new FileSystemProviderManager()); - private static readonly FileSystemProviderManager Instance = new FileSystemProviderManager(); + private static FileSystemProviderManager _current; public static FileSystemProviderManager Current { - get { return Instance; } + get + { + if (_current != null) return _current; + _current = Instance.Value; + return _current; + } } + + /// + /// For tests only, allows setting the value of the singleton "Current" property + /// + /// + public static void SetCurrent(FileSystemProviderManager instance) + { + _current = instance; + } // for tests only, totally unsafe internal void Reset() @@ -52,10 +70,24 @@ namespace Umbraco.Core.IO // beware: means that we capture the "current" scope provider - take care in tests! get { return ApplicationContext.Current == null ? null : ApplicationContext.Current.ScopeProvider as IScopeProviderInternal; } } - - internal FileSystemProviderManager() + + /// + /// Constructor that can be used for tests + /// + /// + public FileSystemProviderManager(IFileSystemProvidersSection configSection) { - _config = (FileSystemProvidersSection) ConfigurationManager.GetSection("umbracoConfiguration/FileSystemProviders"); + if (configSection == null) throw new ArgumentNullException("configSection"); + _config = configSection; + CreateWellKnownFileSystems(); + } + + /// + /// Default constructor that will read the config from the locally found config section + /// + public FileSystemProviderManager() + { + _config = (FileSystemProvidersSection)ConfigurationManager.GetSection("umbracoConfiguration/FileSystemProviders"); CreateWellKnownFileSystems(); } @@ -166,17 +198,21 @@ namespace Umbraco.Core.IO if (providerType.IsAssignableFrom(typeof(IFileSystem))) throw new InvalidOperationException(string.Format("Type {0} does not implement IFileSystem.", providerType.FullName)); - // find a ctor matching the config parameters + // find a ctor matching the config parameters var paramCount = providerConfig.Parameters != null ? providerConfig.Parameters.Count : 0; var constructor = providerType.GetConstructors().SingleOrDefault(x - => x.GetParameters().Length == paramCount && x.GetParameters().All(y => providerConfig.Parameters.AllKeys.Contains(y.Name))); + => x.GetParameters().Length == paramCount && x.GetParameters().All(y => providerConfig.Parameters.Keys.Contains(y.Name))); if (constructor == null) throw new InvalidOperationException(string.Format("Type {0} has no ctor matching the {1} configuration parameter(s).", providerType.FullName, paramCount)); var parameters = new object[paramCount]; - if (providerConfig.Parameters != null) // keeps ReSharper happy + + if (providerConfig.Parameters != null) + { + var allKeys = providerConfig.Parameters.Keys.ToArray(); for (var i = 0; i < paramCount; i++) - parameters[i] = providerConfig.Parameters[providerConfig.Parameters.AllKeys[i]].Value; + parameters[i] = providerConfig.Parameters[allKeys[i]]; + } return new ProviderConstructionInfo { diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c6e7692cee..d2ffe06b01 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -238,6 +238,8 @@ + + From 68a30c39065220107175a7178232d79736a25006 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Sep 2017 16:25:12 +1000 Subject: [PATCH 2/3] fixes issue when referencing a key that doesn't exist --- src/Umbraco.Core/IO/FileSystemProviderManager.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/IO/FileSystemProviderManager.cs b/src/Umbraco.Core/IO/FileSystemProviderManager.cs index abfb1d1a33..da8768cd99 100644 --- a/src/Umbraco.Core/IO/FileSystemProviderManager.cs +++ b/src/Umbraco.Core/IO/FileSystemProviderManager.cs @@ -182,8 +182,9 @@ namespace Umbraco.Core.IO private ProviderConstructionInfo GetUnderlyingFileSystemCtor(string alias, Func fallback) { // get config - var providerConfig = _config.Providers[alias]; - if (providerConfig == null) + IFileSystemProviderElement providerConfig; + + if (_config.Providers.TryGetValue(alias, out providerConfig) == false) { if (fallback != null) return null; throw new ArgumentException(string.Format("No provider found with alias {0}.", alias)); From 8548ee7761e40e8f192273125e9e01e412d7880d Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 12 Sep 2017 16:05:35 +0200 Subject: [PATCH 3/3] u4-10118 - don't create a FS provider manager just to reset it --- .../IO/FileSystemProviderManager.cs | 31 +++++++++++++------ .../IO/FileSystemProviderManagerTests.cs | 4 +-- .../TestHelpers/BaseUmbracoApplicationTest.cs | 2 +- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Core/IO/FileSystemProviderManager.cs b/src/Umbraco.Core/IO/FileSystemProviderManager.cs index da8768cd99..e50322d95c 100644 --- a/src/Umbraco.Core/IO/FileSystemProviderManager.cs +++ b/src/Umbraco.Core/IO/FileSystemProviderManager.cs @@ -29,19 +29,18 @@ namespace Umbraco.Core.IO #region Singleton & Constructor - //ensures that the construction of the singleton only happens when Current is accessed, otherwise any reference to this class would cause - //the ctor to be invoked which can lead to problems outside of a web app. - private static readonly Lazy Instance = new Lazy(() => new FileSystemProviderManager()); - - private static FileSystemProviderManager _current; + private static volatile FileSystemProviderManager _instance; + private static readonly object _instanceLocker = new object(); public static FileSystemProviderManager Current { get { - if (_current != null) return _current; - _current = Instance.Value; - return _current; + if (_instance != null) return _instance; + lock (_instanceLocker) + { + return _instance ?? (_instance = new FileSystemProviderManager()); + } } } @@ -51,11 +50,23 @@ namespace Umbraco.Core.IO /// public static void SetCurrent(FileSystemProviderManager instance) { - _current = instance; + lock (_instanceLocker) + { + _instance = instance; + } + } + + internal static void ResetCurrent() + { + lock (_instanceLocker) + { + if (_instance != null) + _instance.Reset(); + } } // for tests only, totally unsafe - internal void Reset() + private void Reset() { _wrappers.Clear(); _providerLookup.Clear(); diff --git a/src/Umbraco.Tests/IO/FileSystemProviderManagerTests.cs b/src/Umbraco.Tests/IO/FileSystemProviderManagerTests.cs index 416151fcc5..a080fda87c 100644 --- a/src/Umbraco.Tests/IO/FileSystemProviderManagerTests.cs +++ b/src/Umbraco.Tests/IO/FileSystemProviderManagerTests.cs @@ -26,14 +26,14 @@ namespace Umbraco.Tests.IO // start clean // because some tests will create corrupt or weird filesystems - FileSystemProviderManager.Current.Reset(); + FileSystemProviderManager.ResetCurrent(); } [TearDown] public void TearDown() { // stay clean (see note in SetUp) - FileSystemProviderManager.Current.Reset(); + FileSystemProviderManager.ResetCurrent(); } [Test] diff --git a/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs b/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs index fa4e851b5e..49895ed453 100644 --- a/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs +++ b/src/Umbraco.Tests/TestHelpers/BaseUmbracoApplicationTest.cs @@ -155,7 +155,7 @@ namespace Umbraco.Tests.TestHelpers // FileSystemProviderManager captures the current ApplicationContext ScopeProvider // in its current static instance (yea...) so we need to reset it here to ensure // it is using the proper ScopeProvider - FileSystemProviderManager.Current.Reset(); + FileSystemProviderManager.ResetCurrent(); } protected virtual ApplicationContext CreateApplicationContext()