From eee7222e0b0b162f6a412f7f34e6bb02a24acee4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 26 Oct 2018 15:06:53 +0200 Subject: [PATCH] Refactor filesystems --- .../Composers/FileSystemsComposer.cs | 99 ++++++++- src/Umbraco.Core/ContentExtensions.cs | 4 +- .../Events/QueuingEventDispatcher.cs | 4 +- src/Umbraco.Core/IO/FileSystemAttribute.cs | 25 +++ .../IO/FileSystemProviderAttribute.cs | 19 -- src/Umbraco.Core/IO/FileSystemWrapper.cs | 44 ++-- src/Umbraco.Core/IO/FileSystems.cs | 191 +++--------------- src/Umbraco.Core/IO/IFileSystem.cs | 2 +- src/Umbraco.Core/IO/IFileSystems.cs | 43 ++++ src/Umbraco.Core/IO/IMediaFileSystem.cs | 66 ++++++ src/Umbraco.Core/IO/MediaFileSystem.cs | 113 ++--------- src/Umbraco.Core/IO/ShadowFileSystems.cs | 95 +-------- .../Services/Implement/ContentService.cs | 8 +- .../Services/Implement/MediaService.cs | 7 +- .../Services/Implement/MemberService.cs | 9 +- src/Umbraco.Core/Umbraco.Core.csproj | 4 +- src/Umbraco.Tests/IO/FileSystemsTests.cs | 84 ++++---- .../Scoping/ScopeEventDispatcherTests.cs | 3 +- src/Umbraco.Tests/TestHelpers/TestObjects.cs | 2 +- src/Umbraco.Web/Editors/ImagesController.cs | 4 +- .../Media/UploadAutoFillProperties.cs | 4 +- .../FileUploadPropertyEditor.cs | 4 +- .../FileUploadPropertyValueEditor.cs | 4 +- .../ImageCropperPropertyEditor.cs | 4 +- .../ImageCropperPropertyValueEditor.cs | 4 +- 25 files changed, 380 insertions(+), 466 deletions(-) create mode 100644 src/Umbraco.Core/IO/FileSystemAttribute.cs delete mode 100644 src/Umbraco.Core/IO/FileSystemProviderAttribute.cs create mode 100644 src/Umbraco.Core/IO/IFileSystems.cs create mode 100644 src/Umbraco.Core/IO/IMediaFileSystem.cs diff --git a/src/Umbraco.Core/Composing/Composers/FileSystemsComposer.cs b/src/Umbraco.Core/Composing/Composers/FileSystemsComposer.cs index 15c4bd0eea..56a0ba316c 100644 --- a/src/Umbraco.Core/Composing/Composers/FileSystemsComposer.cs +++ b/src/Umbraco.Core/Composing/Composers/FileSystemsComposer.cs @@ -1,28 +1,105 @@ -using Umbraco.Core.IO; +using System; +using System.Configuration; +using Umbraco.Core.Configuration; +using Umbraco.Core.IO; namespace Umbraco.Core.Composing.Composers { public static class FileSystemsComposer { + /* + * HOW TO REPLACE THE MEDIA UNDERLYING FILESYSTEM + * ---------------------------------------------- + * + * Create a component and use it to modify the composition by adding something like: + * + * composition.Container.RegisterSingleton("media", factory => ...); + * + * where the ... part returns the new underlying filesystem, as an IFileSystem. + * + * + * HOW TO IMPLEMENT MY OWN FILESYSTEM + * ---------------------------------- + * + * Declare your filesystem interface: + * + * public interface IMyFileSystem : IFileSystem + * { } + * + * Create your filesystem class: + * + * [FileSystem("my")] + * public class MyFileSystem : FileSystemWrapper, IFormsFileSystem + * { + * public FormsFileSystem(IFileSystem innerFileSystem) + * : base(innerFileSystem) + * { } + * } + * + * Register both the underlying filesystem, and your filesystem, in a component: + * + * composition.Container.RegisterSingleton("my", factory => ...); + * composition.Container.RegisterSingleton(factory => + * factory.GetInstance().GetFileSystem(); + * + * And that's it, you can inject IMyFileSystem wherever it's needed. + * + * + * WHAT IS SHADOWING + * ----------------- + * + * Shadowing is the technology used for Deploy to implement some sort of + * transaction-management on top of filesystems. The plumbing explained above, + * compared to creating your own physical filesystem, ensures that your filesystem + * would participate into such transactions. + * + * Also note that in order for things to work correctly, all filesystems should + * be instantiated before shadowing - so if registering a new filesystem in a + * component, it's a good idea to initialize it. This would be enough (in the + * component): + * + * public void Initialize(IMyFileSystem fs) + * { } + * + * + */ + public static IContainer ComposeFileSystems(this IContainer container) { // register FileSystems, which manages all filesystems - container.RegisterSingleton(); + // it needs to be registered (not only the interface) because it provides additional + // functionality eg for scoping, and is injected in the scope provider - whereas the + // interface is really for end-users to get access to filesystems. + //container.RegisterSingleton(); + container.RegisterSingleton(factory => factory.CreateInstance(new { container} )); // register IFileSystems, which gives access too all filesystems container.RegisterSingleton(factory => factory.GetInstance()); - - // fixme - review registering mediafilesystem. it seems to create cyclic dependencies for castle. - // let's try naming it so the default is overwritten... - - // register MediaFileSystem, which can be injected directly - // note: the actual MediaFileSystem implementation is created by FileSystems directly, - // without being registered in the container - this just gives access to it - container.Register(f => f.GetInstance().MediaFileSystem); - + // register IMediaFileSystem + var virtualRoot = GetMediaFileSystemVirtualRoot(); + container.RegisterSingleton("media", factory => new PhysicalFileSystem(virtualRoot)); + container.RegisterSingleton(factory => factory.GetInstance().GetFileSystem()); return container; } + + private static string GetMediaFileSystemVirtualRoot() + { + // for the time being, we still use the FileSystemProvider config file + // but, detect if ppl are trying to use it to change the "provider" + + var virtualRoot = "~/media"; + + var config = (FileSystemProvidersSection)ConfigurationManager.GetSection("umbracoConfiguration/FileSystemProviders"); + var p = config?.Providers["media"]; + if (p == null) return virtualRoot; + + if (!string.IsNullOrWhiteSpace(p.Type) && p.Type != "Umbraco.Core.IO.PhysicalFileSystem, Umbraco.Core") + throw new InvalidOperationException("Setting a provider type in FileSystemProviders.config is not supported anymore, see FileSystemsComposer for help."); + + virtualRoot = p?.Parameters["virtualRoot"]?.Value ?? "~/media"; + return virtualRoot; + } } } diff --git a/src/Umbraco.Core/ContentExtensions.cs b/src/Umbraco.Core/ContentExtensions.cs index 4f88c2b803..ee6602e9aa 100644 --- a/src/Umbraco.Core/ContentExtensions.cs +++ b/src/Umbraco.Core/ContentExtensions.cs @@ -18,8 +18,8 @@ namespace Umbraco.Core public static class ContentExtensions { // this ain't pretty - private static MediaFileSystem _mediaFileSystem; - private static MediaFileSystem MediaFileSystem => _mediaFileSystem ?? (_mediaFileSystem = Current.FileSystems.MediaFileSystem); + private static IMediaFileSystem _mediaFileSystem; + private static IMediaFileSystem MediaFileSystem => _mediaFileSystem ?? (_mediaFileSystem = Current.FileSystems.MediaFileSystem); #region IContent diff --git a/src/Umbraco.Core/Events/QueuingEventDispatcher.cs b/src/Umbraco.Core/Events/QueuingEventDispatcher.cs index e0d3847c17..54ed8580a0 100644 --- a/src/Umbraco.Core/Events/QueuingEventDispatcher.cs +++ b/src/Umbraco.Core/Events/QueuingEventDispatcher.cs @@ -32,9 +32,9 @@ namespace Umbraco.Core.Events } } - private MediaFileSystem _mediaFileSystem; + private IMediaFileSystem _mediaFileSystem; // fixme inject - private MediaFileSystem MediaFileSystem => _mediaFileSystem ?? (_mediaFileSystem = Current.FileSystems.MediaFileSystem); + private IMediaFileSystem MediaFileSystem => _mediaFileSystem ?? (_mediaFileSystem = Current.FileSystems.MediaFileSystem); } } diff --git a/src/Umbraco.Core/IO/FileSystemAttribute.cs b/src/Umbraco.Core/IO/FileSystemAttribute.cs new file mode 100644 index 0000000000..cf59d4e145 --- /dev/null +++ b/src/Umbraco.Core/IO/FileSystemAttribute.cs @@ -0,0 +1,25 @@ +using System; + +namespace Umbraco.Core.IO +{ + /// + /// Decorates a filesystem. + /// + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)] + public class FileSystemAttribute : Attribute + { + /// + /// Initializes a new instance of the class. + /// + /// + public FileSystemAttribute(string alias) + { + Alias = alias; + } + + /// + /// Gets the alias of the filesystem. + /// + public string Alias { get; } + } +} diff --git a/src/Umbraco.Core/IO/FileSystemProviderAttribute.cs b/src/Umbraco.Core/IO/FileSystemProviderAttribute.cs deleted file mode 100644 index b3b6cb6b79..0000000000 --- a/src/Umbraco.Core/IO/FileSystemProviderAttribute.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using Umbraco.Core.CodeAnnotations; - -namespace Umbraco.Core.IO -{ - [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = false)] - public class FileSystemProviderAttribute : Attribute - { - public string Alias { get; private set; } - - public FileSystemProviderAttribute(string alias) - { - Alias = alias; - } - } -} diff --git a/src/Umbraco.Core/IO/FileSystemWrapper.cs b/src/Umbraco.Core/IO/FileSystemWrapper.cs index 2c4377b89b..14d028c16d 100644 --- a/src/Umbraco.Core/IO/FileSystemWrapper.cs +++ b/src/Umbraco.Core/IO/FileSystemWrapper.cs @@ -16,103 +16,103 @@ namespace Umbraco.Core.IO /// public abstract class FileSystemWrapper : IFileSystem { - protected FileSystemWrapper(IFileSystem wrapped) + protected FileSystemWrapper(IFileSystem innerFileSystem) { - Wrapped = wrapped; + InnerFileSystem = innerFileSystem; } - internal IFileSystem Wrapped { get; set; } + internal IFileSystem InnerFileSystem { get; set; } public IEnumerable GetDirectories(string path) { - return Wrapped.GetDirectories(path); + return InnerFileSystem.GetDirectories(path); } public void DeleteDirectory(string path) { - Wrapped.DeleteDirectory(path); + InnerFileSystem.DeleteDirectory(path); } public void DeleteDirectory(string path, bool recursive) { - Wrapped.DeleteDirectory(path, recursive); + InnerFileSystem.DeleteDirectory(path, recursive); } public bool DirectoryExists(string path) { - return Wrapped.DirectoryExists(path); + return InnerFileSystem.DirectoryExists(path); } public void AddFile(string path, Stream stream) { - Wrapped.AddFile(path, stream); + InnerFileSystem.AddFile(path, stream); } public void AddFile(string path, Stream stream, bool overrideExisting) { - Wrapped.AddFile(path, stream, overrideExisting); + InnerFileSystem.AddFile(path, stream, overrideExisting); } public IEnumerable GetFiles(string path) { - return Wrapped.GetFiles(path); + return InnerFileSystem.GetFiles(path); } public IEnumerable GetFiles(string path, string filter) { - return Wrapped.GetFiles(path, filter); + return InnerFileSystem.GetFiles(path, filter); } public Stream OpenFile(string path) { - return Wrapped.OpenFile(path); + return InnerFileSystem.OpenFile(path); } public void DeleteFile(string path) { - Wrapped.DeleteFile(path); + InnerFileSystem.DeleteFile(path); } public bool FileExists(string path) { - return Wrapped.FileExists(path); + return InnerFileSystem.FileExists(path); } public string GetRelativePath(string fullPathOrUrl) { - return Wrapped.GetRelativePath(fullPathOrUrl); + return InnerFileSystem.GetRelativePath(fullPathOrUrl); } public string GetFullPath(string path) { - return Wrapped.GetFullPath(path); + return InnerFileSystem.GetFullPath(path); } public string GetUrl(string path) { - return Wrapped.GetUrl(path); + return InnerFileSystem.GetUrl(path); } public DateTimeOffset GetLastModified(string path) { - return Wrapped.GetLastModified(path); + return InnerFileSystem.GetLastModified(path); } public DateTimeOffset GetCreated(string path) { - return Wrapped.GetCreated(path); + return InnerFileSystem.GetCreated(path); } public long GetSize(string path) { - return Wrapped.GetSize(path); + return InnerFileSystem.GetSize(path); } - public bool CanAddPhysical => Wrapped.CanAddPhysical; + public bool CanAddPhysical => InnerFileSystem.CanAddPhysical; public void AddFile(string path, string physicalPath, bool overrideIfExists = true, bool copy = false) { - Wrapped.AddFile(path, physicalPath, overrideIfExists, copy); + InnerFileSystem.AddFile(path, physicalPath, overrideIfExists, copy); } } } diff --git a/src/Umbraco.Core/IO/FileSystems.cs b/src/Umbraco.Core/IO/FileSystems.cs index cc52487018..24e7a73172 100644 --- a/src/Umbraco.Core/IO/FileSystems.cs +++ b/src/Umbraco.Core/IO/FileSystems.cs @@ -1,36 +1,20 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Configuration; -using System.IO; using System.Linq; -using System.Reflection; using System.Threading; -using Umbraco.Core.Configuration; using Umbraco.Core.Logging; using Umbraco.Core.Composing; namespace Umbraco.Core.IO { - public interface IFileSystems // fixme move! - { - IFileSystem MacroPartialsFileSystem { get; } - IFileSystem PartialViewsFileSystem { get; } - IFileSystem StylesheetsFileSystem { get; } - IFileSystem ScriptsFileSystem { get; } - IFileSystem MasterPagesFileSystem { get; } - IFileSystem MvcViewsFileSystem { get; } - MediaFileSystem MediaFileSystem { get; } - } - public class FileSystems : IFileSystems { - private readonly IFileSystemProvidersSection _config; private readonly ConcurrentSet _wrappers = new ConcurrentSet(); + private readonly IContainer _container; private readonly ILogger _logger; - private readonly ConcurrentDictionary _providerLookup = new ConcurrentDictionary(); - private readonly ConcurrentDictionary _filesystems = new ConcurrentDictionary(); + private readonly ConcurrentDictionary> _filesystems = new ConcurrentDictionary>(); // wrappers for shadow support private ShadowWrapper _macroPartialFileSystem; @@ -50,11 +34,9 @@ namespace Umbraco.Core.IO #region Constructor // DI wants a public ctor - // but IScopeProviderInternal is not public - public FileSystems(ILogger logger) + public FileSystems(IContainer container, ILogger logger) { - // fixme inject config section => can be used by tests - _config = (FileSystemProvidersSection) ConfigurationManager.GetSection("umbracoConfiguration/FileSystemProviders"); + _container = container; _logger = logger; } @@ -62,7 +44,6 @@ namespace Umbraco.Core.IO internal void Reset() { _wrappers.Clear(); - _providerLookup.Clear(); _filesystems.Clear(); Volatile.Write(ref _wkfsInitialized, false); } @@ -73,6 +54,7 @@ namespace Umbraco.Core.IO #region Well-Known FileSystems + /// public IFileSystem MacroPartialsFileSystem { get @@ -82,6 +64,7 @@ namespace Umbraco.Core.IO } } + /// public IFileSystem PartialViewsFileSystem { get @@ -91,6 +74,7 @@ namespace Umbraco.Core.IO } } + /// public IFileSystem StylesheetsFileSystem { get @@ -100,6 +84,7 @@ namespace Umbraco.Core.IO } } + /// public IFileSystem ScriptsFileSystem { get @@ -108,16 +93,18 @@ namespace Umbraco.Core.IO return _scriptsFileSystem; } } - + + /// public IFileSystem MasterPagesFileSystem { get { if (Volatile.Read(ref _wkfsInitialized) == false) EnsureWellKnownFileSystems(); - return _masterPagesFileSystem;// fixme - see 7.6?! + return _masterPagesFileSystem; } } + /// public IFileSystem MvcViewsFileSystem { get @@ -127,7 +114,8 @@ namespace Umbraco.Core.IO } } - public MediaFileSystem MediaFileSystem + /// + public IMediaFileSystem MediaFileSystem { get { @@ -160,7 +148,7 @@ namespace Umbraco.Core.IO _mvcViewsFileSystem = new ShadowWrapper(mvcViewsFileSystem, "Views", () => IsScoped()); // filesystems obtained from GetFileSystemProvider are already wrapped and do not need to be wrapped again - _mediaFileSystem = GetFileSystemProvider(); + _mediaFileSystem = GetFileSystem(); return null; } @@ -169,155 +157,42 @@ namespace Umbraco.Core.IO #region Providers - /// - /// used to cache the lookup of how to construct this object so we don't have to reflect each time. - /// - private class ProviderConstructionInfo - { - public object[] Parameters { get; set; } - public ConstructorInfo Constructor { get; set; } - //public string ProviderAlias { get; set; } - } - - /// - /// Gets an underlying (non-typed) filesystem supporting a strongly-typed filesystem. - /// - /// The alias of the strongly-typed filesystem. - /// The non-typed filesystem supporting the strongly-typed filesystem with the specified alias. - /// This method should not be used directly, used instead. - internal IFileSystem GetUnderlyingFileSystemProvider(string alias) - { - return GetUnderlyingFileSystemProvider(alias, null); - } - - /// - /// Gets an underlying (non-typed) filesystem supporting a strongly-typed filesystem. - /// - /// The alias of the strongly-typed filesystem. - /// A fallback creator for the filesystem. - /// The non-typed filesystem supporting the strongly-typed filesystem with the specified alias. - /// This method should not be used directly, used instead. - internal IFileSystem GetUnderlyingFileSystemProvider(string alias, Func fallback) - { - // either get the constructor info from cache or create it and add to cache - var ctorInfo = _providerLookup.GetOrAdd(alias, _ => GetUnderlyingFileSystemCtor(alias, fallback)); - return ctorInfo == null ? fallback() : (IFileSystem) ctorInfo.Constructor.Invoke(ctorInfo.Parameters); - } - - private IFileSystem GetUnderlyingFileSystemNoCache(string alias, Func fallback) - { - var ctorInfo = GetUnderlyingFileSystemCtor(alias, fallback); - return ctorInfo == null ? fallback() : (IFileSystem) ctorInfo.Constructor.Invoke(ctorInfo.Parameters); - } - - private ProviderConstructionInfo GetUnderlyingFileSystemCtor(string alias, Func fallback) - { - // get config - if (_config.Providers.TryGetValue(alias, out var providerConfig) == false) - { - if (fallback != null) return null; - throw new ArgumentException($"No provider found with alias {alias}."); - } - - // get the filesystem type - var providerType = Type.GetType(providerConfig.Type); - if (providerType == null) - throw new InvalidOperationException($"Could not find type {providerConfig.Type}."); - - // ensure it implements IFileSystem - if (providerType.IsAssignableFrom(typeof (IFileSystem))) - throw new InvalidOperationException($"Type {providerType.FullName} does not implement IFileSystem."); - - // find a ctor matching the config parameters - var paramCount = providerConfig.Parameters?.Count ?? 0; - var constructor = providerType.GetConstructors().SingleOrDefault(x - => x.GetParameters().Length == paramCount && x.GetParameters().All(y => providerConfig.Parameters.Keys.Contains(y.Name))); - if (constructor == null) - throw new InvalidOperationException($"Type {providerType.FullName} has no ctor matching the {paramCount} configuration parameter(s)."); - - var parameters = new object[paramCount]; - if (providerConfig.Parameters != null) // keeps ReSharper happy - { - var allKeys = providerConfig.Parameters.Keys.ToArray(); - for (var i = 0; i < paramCount; i++) - parameters[i] = providerConfig.Parameters[allKeys[i]]; - } - - return new ProviderConstructionInfo - { - Constructor = constructor, - Parameters = parameters, - //ProviderAlias = s - }; - } - /// /// Gets a strongly-typed filesystem. /// /// The type of the filesystem. /// A strongly-typed filesystem of the specified type. /// - /// Ideally, this should cache the instances, but that would break backward compatibility, so we - /// only do it for our own MediaFileSystem - for everything else, it's the responsibility of the caller - /// to ensure that they maintain singletons. This is important for singletons, as each filesystem maintains - /// its own shadow and having multiple instances would lead to inconsistencies. /// Note that any filesystem created by this method *after* shadowing begins, will *not* be /// shadowing (and an exception will be thrown by the ShadowWrapper). /// - // fixme - should it change for v8? - public TFileSystem GetFileSystemProvider() - where TFileSystem : FileSystemWrapper - { - return GetFileSystemProvider(null); - } - - /// - /// Gets a strongly-typed filesystem. - /// - /// The type of the filesystem. - /// A fallback creator for the inner filesystem. - /// A strongly-typed filesystem of the specified type. - /// - /// The fallback creator is used only if nothing is configured. - /// Ideally, this should cache the instances, but that would break backward compatibility, so we - /// only do it for our own MediaFileSystem - for everything else, it's the responsibility of the caller - /// to ensure that they maintain singletons. This is important for singletons, as each filesystem maintains - /// its own shadow and having multiple instances would lead to inconsistencies. - /// Note that any filesystem created by this method *after* shadowing begins, will *not* be - /// shadowing (and an exception will be thrown by the ShadowWrapper). - /// - public TFileSystem GetFileSystemProvider(Func fallback) + public TFileSystem GetFileSystem() where TFileSystem : FileSystemWrapper { var alias = GetFileSystemAlias(); - return (TFileSystem)_filesystems.GetOrAdd(alias, _ => - { - // gets the inner fs, create the strongly-typed fs wrapping the inner fs, register & return - // so we are double-wrapping here - // could be optimized by having FileSystemWrapper inherit from ShadowWrapper, maybe - var innerFs = GetUnderlyingFileSystemNoCache(alias, fallback); - var shadowWrapper = new ShadowWrapper(innerFs, "typed/" + alias, () => IsScoped()); - var fs = (IFileSystem)Activator.CreateInstance(typeof(TFileSystem), shadowWrapper); - _wrappers.Add(shadowWrapper); // keeping a reference to the wrapper - return fs; - }); + // note: GetOrAdd can run multiple times - and here, since we have side effects + // (adding to _wrappers) we want to be sure the factory runs only once, hence the + // additional Lazy. + return (TFileSystem) _filesystems.GetOrAdd(alias, _ => new Lazy(() => + { + var supportingFileSystem = _container.GetInstance(alias); + var shadowWrapper = new ShadowWrapper(supportingFileSystem, "typed/" + alias, () => IsScoped()); + + _wrappers.Add(shadowWrapper); // _wrappers is a concurrent set - this is safe + + return _container.CreateInstance(new { innerFileSystem = shadowWrapper}); + })).Value; } private string GetFileSystemAlias() { - var fsType = typeof(TFileSystem); - - // validate the ctor - var constructor = fsType.GetConstructors().SingleOrDefault(x - => x.GetParameters().Length >= 1 && TypeHelper.IsTypeAssignableFrom(x.GetParameters().First().ParameterType)); - if (constructor == null) - throw new InvalidOperationException("Type " + fsType.FullName + " must inherit from FileSystemWrapper and have a constructor that accepts one parameter of type " + typeof(IFileSystem).FullName + "."); + var fileSystemType = typeof(TFileSystem); // find the attribute and get the alias - var attr = (FileSystemProviderAttribute)fsType.GetCustomAttributes(typeof(FileSystemProviderAttribute), false).SingleOrDefault(); + var attr = (FileSystemAttribute) fileSystemType.GetCustomAttributes(typeof(FileSystemAttribute), false).SingleOrDefault(); if (attr == null) - throw new InvalidOperationException("Type " + fsType.FullName + "is missing the required FileSystemProviderAttribute."); + throw new InvalidOperationException("Type " + fileSystemType.FullName + "is missing the required FileSystemProviderAttribute."); return attr.Alias; } @@ -334,9 +209,9 @@ namespace Umbraco.Core.IO // be created directly (via ctor) or via GetFileSystem is *not* shadowed. // shadow must be enabled in an app event handler before anything else ie before any filesystem - // is actually created and used - after, it is too late - enabling shadow has a neglictible perfs + // is actually created and used - after, it is too late - enabling shadow has a negligible perfs // impact. - // NO! by the time an app event handler is instanciated it is already too late, see note in ctor. + // NO! by the time an app event handler is instantiated it is already too late, see note in ctor. //internal void EnableShadow() //{ // if (_mvcViewsFileSystem != null) // test one of the fs... diff --git a/src/Umbraco.Core/IO/IFileSystem.cs b/src/Umbraco.Core/IO/IFileSystem.cs index 115cb8a5c1..14b015a468 100644 --- a/src/Umbraco.Core/IO/IFileSystem.cs +++ b/src/Umbraco.Core/IO/IFileSystem.cs @@ -5,7 +5,7 @@ using System.IO; namespace Umbraco.Core.IO { /// - /// Provides methods allowing the manipulation of files within an Umbraco application. + /// Provides methods allowing the manipulation of files. /// public interface IFileSystem { diff --git a/src/Umbraco.Core/IO/IFileSystems.cs b/src/Umbraco.Core/IO/IFileSystems.cs new file mode 100644 index 0000000000..1af5377c43 --- /dev/null +++ b/src/Umbraco.Core/IO/IFileSystems.cs @@ -0,0 +1,43 @@ +namespace Umbraco.Core.IO +{ + /// + /// Provides the system filesystems. + /// + public interface IFileSystems + { + /// + /// Gets the macro partials filesystem. + /// + IFileSystem MacroPartialsFileSystem { get; } + + /// + /// Gets the partial views filesystem. + /// + IFileSystem PartialViewsFileSystem { get; } + + /// + /// Gets the stylesheets filesystem. + /// + IFileSystem StylesheetsFileSystem { get; } + + /// + /// Gets the scripts filesystem. + /// + IFileSystem ScriptsFileSystem { get; } + + /// + /// Gets the masterpages filesystem. + /// + IFileSystem MasterPagesFileSystem { get; } + + /// + /// Gets the MVC views filesystem. + /// + IFileSystem MvcViewsFileSystem { get; } + + /// + /// Gets the media filesystem. + /// + IMediaFileSystem MediaFileSystem { get; } + } +} diff --git a/src/Umbraco.Core/IO/IMediaFileSystem.cs b/src/Umbraco.Core/IO/IMediaFileSystem.cs new file mode 100644 index 0000000000..e1957857bf --- /dev/null +++ b/src/Umbraco.Core/IO/IMediaFileSystem.cs @@ -0,0 +1,66 @@ +using System; +using System.Collections.Generic; +using System.IO; +using Umbraco.Core.Models; + +namespace Umbraco.Core.IO +{ + /// + /// Provides methods allowing the manipulation of media files. + /// + public interface IMediaFileSystem : IFileSystem + { + /// + /// Delete media files. + /// + /// Files to delete (filesystem-relative paths). + void DeleteMediaFiles(IEnumerable files); + + /// + /// Gets the file path of a media file. + /// + /// The file name. + /// The unique identifier of the content/media owning the file. + /// The unique identifier of the property type owning the file. + /// The filesystem-relative path to the media file. + /// With the old media path scheme, this CREATES a new media path each time it is invoked. + string GetMediaPath(string filename, Guid cuid, Guid puid); + + /// + /// Gets the file path of a media file. + /// + /// The file name. + /// A previous file path. + /// The unique identifier of the content/media owning the file. + /// The unique identifier of the property type owning the file. + /// The filesystem-relative path to the media file. + /// In the old, legacy, number-based scheme, we try to re-use the media folder + /// specified by . Else, we CREATE a new one. Each time we are invoked. + string GetMediaPath(string filename, string prevpath, Guid cuid, Guid puid); + + /// + /// Stores a media file associated to a property of a content item. + /// + /// The content item owning the media file. + /// The property type owning the media file. + /// The media file name. + /// A stream containing the media bytes. + /// An optional filesystem-relative filepath to the previous media file. + /// The filesystem-relative filepath to the media file. + /// + /// The file is considered "owned" by the content/propertyType. + /// If an is provided then that file (and associated thumbnails if any) is deleted + /// before the new file is saved, and depending on the media path scheme, the folder may be reused for the new file. + /// + string StoreFile(IContentBase content, PropertyType propertyType, string filename, Stream filestream, string oldpath); + + /// + /// Copies a media file as a new media file, associated to a property of a content item. + /// + /// The content item owning the copy of the media file. + /// The property type owning the copy of the media file. + /// The filesystem-relative path to the source media file. + /// The filesystem-relative path to the copy of the media file. + string CopyFile(IContentBase content, PropertyType propertyType, string sourcepath); + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/IO/MediaFileSystem.cs b/src/Umbraco.Core/IO/MediaFileSystem.cs index 1b0c8079f0..c36a086bf8 100644 --- a/src/Umbraco.Core/IO/MediaFileSystem.cs +++ b/src/Umbraco.Core/IO/MediaFileSystem.cs @@ -15,11 +15,14 @@ namespace Umbraco.Core.IO /// /// A custom file system provider for media /// - [FileSystemProvider("media")] - public class MediaFileSystem : FileSystemWrapper + [FileSystem("media")] + public class MediaFileSystem : FileSystemWrapper, IMediaFileSystem { - public MediaFileSystem(IFileSystem wrapped) - : base(wrapped) + /// + /// Initializes a new instance of the class. + /// + public MediaFileSystem(IFileSystem innerFileSystem) + : base(innerFileSystem) { ContentConfig = Current.Container.GetInstance(); Logger = Current.Container.GetInstance(); @@ -32,58 +35,16 @@ namespace Umbraco.Core.IO private IContentSection ContentConfig { get; } private ILogger Logger { get; } - - /// - /// Deletes all files passed in. - /// - /// - /// - /// - internal bool DeleteFiles(IEnumerable files, Action onError = null) - { - //ensure duplicates are removed - files = files.Distinct(); - - var allsuccess = true; - var rootRelativePath = GetRelativePath("/"); - - Parallel.ForEach(files, file => - { - try - { - if (file.IsNullOrWhiteSpace()) return; - - var relativeFilePath = GetRelativePath(file); - if (FileExists(relativeFilePath) == false) return; - - var parentDirectory = Path.GetDirectoryName(relativeFilePath); - - // don't want to delete the media folder if not using directories. - if (ContentConfig.UploadAllowDirectories && parentDirectory != rootRelativePath) - { - //issue U4-771: if there is a parent directory the recursive parameter should be true - DeleteDirectory(parentDirectory, string.IsNullOrEmpty(parentDirectory) == false); - } - else - { - DeleteFile(file); - } - } - catch (Exception e) - { - onError?.Invoke(file, e); - allsuccess = false; - } - }); - - return allsuccess; - } + /// public void DeleteMediaFiles(IEnumerable files) { files = files.Distinct(); - Parallel.ForEach(files, file => + // kinda try to keep things under control + var options = new ParallelOptions { MaxDegreeOfParallelism = 20 }; + + Parallel.ForEach(files, options, file => { try { @@ -97,21 +58,14 @@ namespace Umbraco.Core.IO } catch (Exception e) { - Logger.Error(e, "Failed to delete attached file '{File}'", file); + Logger.Error(e, "Failed to delete media file '{File}'.", file); } }); } #region Media Path - /// - /// Gets the file path of a media file. - /// - /// The file name. - /// The unique identifier of the content/media owning the file. - /// The unique identifier of the property type owning the file. - /// The filesystem-relative path to the media file. - /// With the old media path scheme, this CREATES a new media path each time it is invoked. + /// public string GetMediaPath(string filename, Guid cuid, Guid puid) { filename = Path.GetFileName(filename); @@ -121,16 +75,7 @@ namespace Umbraco.Core.IO return MediaPathScheme.GetFilePath(cuid, puid, filename); } - /// - /// Gets the file path of a media file. - /// - /// The file name. - /// A previous file path. - /// The unique identifier of the content/media owning the file. - /// The unique identifier of the property type owning the file. - /// The filesystem-relative path to the media file. - /// In the old, legacy, number-based scheme, we try to re-use the media folder - /// specified by . Else, we CREATE a new one. Each time we are invoked. + /// public string GetMediaPath(string filename, string prevpath, Guid cuid, Guid puid) { filename = Path.GetFileName(filename); @@ -144,20 +89,7 @@ namespace Umbraco.Core.IO #region Associated Media Files - /// - /// Stores a media file associated to a property of a content item. - /// - /// The content item owning the media file. - /// The property type owning the media file. - /// The media file name. - /// A stream containing the media bytes. - /// An optional filesystem-relative filepath to the previous media file. - /// The filesystem-relative filepath to the media file. - /// - /// The file is considered "owned" by the content/propertyType. - /// If an is provided then that file (and associated thumbnails if any) is deleted - /// before the new file is saved, and depending on the media path scheme, the folder may be reused for the new file. - /// + /// public string StoreFile(IContentBase content, PropertyType propertyType, string filename, Stream filestream, string oldpath) { if (content == null) throw new ArgumentNullException(nameof(content)); @@ -176,13 +108,7 @@ namespace Umbraco.Core.IO return filepath; } - /// - /// Copies a media file as a new media file, associated to a property of a content item. - /// - /// The content item owning the copy of the media file. - /// The property type owning the copy of the media file. - /// The filesystem-relative path to the source media file. - /// The filesystem-relative path to the copy of the media file. + /// public string CopyFile(IContentBase content, PropertyType propertyType, string sourcepath) { if (content == null) throw new ArgumentNullException(nameof(content)); @@ -199,9 +125,6 @@ namespace Umbraco.Core.IO return filepath; } - - - #endregion - + #endregion } } diff --git a/src/Umbraco.Core/IO/ShadowFileSystems.cs b/src/Umbraco.Core/IO/ShadowFileSystems.cs index 289667b0db..80aa1791fd 100644 --- a/src/Umbraco.Core/IO/ShadowFileSystems.cs +++ b/src/Umbraco.Core/IO/ShadowFileSystems.cs @@ -6,19 +6,6 @@ namespace Umbraco.Core.IO { internal class ShadowFileSystems : ICompletable { - // note: taking a reference to the _manager instead of using manager.Current - // to avoid using Current everywhere but really, we support only 1 scope at - // a time, not multiple scopes in case of multiple managers (not supported) - - // fixme - why are we managing logical call context here? should be bound - // to the current scope, always => REFACTOR! but there should be something in - // place (static?) to ensure we only have one concurrent shadow FS? - // - // => yes, that's _currentId - need to cleanup this entirely - // and, we probably need a way to stop shadowing entirely without cycling the app - - private const string ItemKey = "Umbraco.Core.IO.ShadowFileSystems"; - private static readonly object Locker = new object(); private static Guid _currentId = Guid.Empty; @@ -28,26 +15,13 @@ namespace Umbraco.Core.IO private bool _completed; - //static ShadowFileSystems() - //{ - // SafeCallContext.Register( - // () => - // { - // var scope = CallContext.LogicalGetData(ItemKey); - // CallContext.FreeNamedDataSlot(ItemKey); - // return scope; - // }, - // o => - // { - // if (CallContext.LogicalGetData(ItemKey) != null) throw new InvalidOperationException(); - // if (o != null) CallContext.LogicalSetData(ItemKey, o); - // }); - //} - + // invoked by the filesystems when shadowing + // can only be 1 shadow at a time (static) public ShadowFileSystems(Guid id, ShadowWrapper[] wrappers, ILogger logger) { lock (Locker) { + // if we throw here, it means that something very wrong happened. if (_currentId != Guid.Empty) throw new InvalidOperationException("Already shadowing."); _currentId = id; @@ -62,56 +36,23 @@ namespace Umbraco.Core.IO wrapper.Shadow(id); } - // fixme - remove - //// internal for tests + FileSystems - //// do NOT use otherwise - //internal static ShadowFileSystems CreateScope(Guid id, ShadowWrapper[] wrappers, ILogger logger) - //{ - // lock (Locker) - // { - // if (CallContext.LogicalGetData(ItemKey) != null) throw new InvalidOperationException("Already shadowing."); - // CallContext.LogicalSetData(ItemKey, ItemKey); // value does not matter - // } - // return new ShadowFileSystems(id, wrappers, logger); - //} - - //internal static bool InScope => NoScope == false; - - //internal static bool NoScope => CallContext.LogicalGetData(ItemKey) == null; - + // invoked by the scope when exiting, if completed public void Complete() { _completed = true; - //lock (Locker) - //{ - // _logger.Debug("UnShadow " + _id + " (complete)."); - - // var exceptions = new List(); - // foreach (var wrapper in _wrappers) - // { - // try - // { - // // this may throw an AggregateException if some of the changes could not be applied - // wrapper.UnShadow(true); - // } - // catch (AggregateException ae) - // { - // exceptions.Add(ae); - // } - // } - - // if (exceptions.Count > 0) - // throw new AggregateException("Failed to apply all changes (see exceptions).", exceptions); - - // // last, & *only* if successful (otherwise we'll unshadow & cleanup as best as we can) - // CallContext.FreeNamedDataSlot(ItemKey); - //} } + // invoked by the scope when exiting public void Dispose() { lock (Locker) { + // if we throw here, it means that something very wrong happened. + if (_currentId == Guid.Empty) + throw new InvalidOperationException("Not shadowing."); + if (_id != _currentId) + throw new InvalidOperationException("Not the current shadow."); + _logger.Debug("UnShadow '{ShadowId}' {Status}", _id, _completed ? "complete" : "abort"); var exceptions = new List(); @@ -132,20 +73,6 @@ namespace Umbraco.Core.IO if (exceptions.Count > 0) throw new AggregateException(_completed ? "Failed to apply all changes (see exceptions)." : "Failed to abort (see exceptions).", exceptions); - - //if (CallContext.LogicalGetData(ItemKey) == null) return; - - //try - //{ - // _logger.Debug("UnShadow " + _id + " (abort)"); - // foreach (var wrapper in _wrappers) - // wrapper.UnShadow(false); // should not throw - //} - //finally - //{ - // // last, & always - // CallContext.FreeNamedDataSlot(ItemKey); - //} } } diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index f14747cda3..999bd56646 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -28,13 +28,13 @@ namespace Umbraco.Core.Services.Implement private readonly IContentTypeRepository _contentTypeRepository; private readonly IDocumentBlueprintRepository _documentBlueprintRepository; private readonly ILanguageRepository _languageRepository; - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; private IQuery _queryNotTrashed; #region Constructors public ContentService(IScopeProvider provider, ILogger logger, - IEventMessagesFactory eventMessagesFactory, MediaFileSystem mediaFileSystem, + IEventMessagesFactory eventMessagesFactory, IMediaFileSystem mediaFileSystem, IDocumentRepository documentRepository, IEntityRepository entityRepository, IAuditRepository auditRepository, IContentTypeRepository contentTypeRepository, IDocumentBlueprintRepository documentBlueprintRepository, ILanguageRepository languageRepository) : base(provider, logger, eventMessagesFactory) @@ -1471,9 +1471,7 @@ namespace Umbraco.Core.Services.Implement var args = new DeleteEventArgs(c, false); // raise event & get flagged files scope.Events.Dispatch(Deleted, this, args, nameof(Deleted)); - // fixme not going to work, do it differently - _mediaFileSystem.DeleteFiles(args.MediaFilesToDelete, // remove flagged files - (file, e) => Logger.Error(e, "An error occurred while deleting file attached to nodes: {File}", file)); + // media files deleted by QueuingEventDispatcher } } diff --git a/src/Umbraco.Core/Services/Implement/MediaService.cs b/src/Umbraco.Core/Services/Implement/MediaService.cs index da04f41e18..8363935adb 100644 --- a/src/Umbraco.Core/Services/Implement/MediaService.cs +++ b/src/Umbraco.Core/Services/Implement/MediaService.cs @@ -26,11 +26,11 @@ namespace Umbraco.Core.Services.Implement private readonly IAuditRepository _auditRepository; private readonly IEntityRepository _entityRepository; - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; #region Constructors - public MediaService(IScopeProvider provider, MediaFileSystem mediaFileSystem, ILogger logger, IEventMessagesFactory eventMessagesFactory, + public MediaService(IScopeProvider provider, IMediaFileSystem mediaFileSystem, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMediaRepository mediaRepository, IAuditRepository auditRepository, IMediaTypeRepository mediaTypeRepository, IEntityRepository entityRepository) : base(provider, logger, eventMessagesFactory) @@ -888,8 +888,7 @@ namespace Umbraco.Core.Services.Implement var args = new DeleteEventArgs(c, false); // raise event & get flagged files scope.Events.Dispatch(Deleted, this, args); - _mediaFileSystem.DeleteFiles(args.MediaFilesToDelete, // remove flagged files - (file, e) => Logger.Error(e, "An error occurred while deleting file attached to nodes: {File}", file)); + // media files deleted by QueuingEventDispatcher } } diff --git a/src/Umbraco.Core/Services/Implement/MemberService.cs b/src/Umbraco.Core/Services/Implement/MemberService.cs index 3fd714f974..82be6d3dc6 100644 --- a/src/Umbraco.Core/Services/Implement/MemberService.cs +++ b/src/Umbraco.Core/Services/Implement/MemberService.cs @@ -28,14 +28,14 @@ namespace Umbraco.Core.Services.Implement private readonly IAuditRepository _auditRepository; private readonly IMemberGroupService _memberGroupService; - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; //only for unit tests! internal MembershipProviderBase MembershipProvider { get; set; } #region Constructor - public MemberService(IScopeProvider provider, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMemberGroupService memberGroupService, MediaFileSystem mediaFileSystem, + public MemberService(IScopeProvider provider, ILogger logger, IEventMessagesFactory eventMessagesFactory, IMemberGroupService memberGroupService, IMediaFileSystem mediaFileSystem, IMemberRepository memberRepository, IMemberTypeRepository memberTypeRepository, IMemberGroupRepository memberGroupRepository, IAuditRepository auditRepository) : base(provider, logger, eventMessagesFactory) { @@ -927,10 +927,7 @@ namespace Umbraco.Core.Services.Implement args.CanCancel = false; scope.Events.Dispatch(Deleted, this, args); - // fixme - this is MOOT because the event will not trigger immediately - // it's been refactored already (think it's the dispatcher that deals with it?) - _mediaFileSystem.DeleteFiles(args.MediaFilesToDelete, // remove flagged files - (file, e) => Logger.Error(e, "An error occurred while deleting file attached to nodes: {File}", file)); + // media files deleted by QueuingEventDispatcher } #endregion diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 59f353e1e9..e7d85472fb 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -329,6 +329,8 @@ + + @@ -574,7 +576,7 @@ - + diff --git a/src/Umbraco.Tests/IO/FileSystemsTests.cs b/src/Umbraco.Tests/IO/FileSystemsTests.cs index f97bdc4189..fe27229c72 100644 --- a/src/Umbraco.Tests/IO/FileSystemsTests.cs +++ b/src/Umbraco.Tests/IO/FileSystemsTests.cs @@ -6,6 +6,7 @@ using NUnit.Framework; using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Composing; +using Umbraco.Core.Composing.Composers; using Umbraco.Core.IO; using Umbraco.Core.IO.MediaPathSchemes; using Umbraco.Core.Logging; @@ -29,11 +30,12 @@ namespace Umbraco.Tests.IO _container = Current.Container = ContainerFactory.Create(); _container.Register(_ => Mock.Of()); - _container.Register(); _container.Register(_ => Mock.Of()); _container.Register(_ => Mock.Of()); _container.RegisterSingleton(); + _container.ComposeFileSystems(); + // make sure we start clean // because some tests will create corrupt or weird filesystems FileSystems.Reset(); @@ -52,25 +54,39 @@ namespace Umbraco.Tests.IO private FileSystems FileSystems => _container.GetInstance(); [Test] - public void Can_Get_Base_File_System() + public void Can_Get_MediaFileSystem() { - var fileSystem = FileSystems.GetUnderlyingFileSystemProvider("media"); - + var fileSystem = FileSystems.GetFileSystem(); Assert.NotNull(fileSystem); } [Test] - public void Can_Get_Typed_File_System() + public void Can_Get_IMediaFileSystem() { - var fileSystem = FileSystems.GetFileSystemProvider(); - + var fileSystem = _container.GetInstance(); Assert.NotNull(fileSystem); } [Test] - public void Media_Fs_Safe_Delete() + public void MediaFileSystem_Is_Singleton() { - var fs = FileSystems.GetFileSystemProvider(); + var fileSystem1 = FileSystems.GetFileSystem(); + var fileSystem2 = FileSystems.GetFileSystem(); + Assert.AreSame(fileSystem1, fileSystem2); + } + + [Test] + public void IMediaFileSystem_Is_Singleton() + { + var fileSystem1 = _container.GetInstance(); + var fileSystem2 = _container.GetInstance(); + Assert.AreSame(fileSystem1, fileSystem2); + } + + [Test] + public void Can_Delete_MediaFiles() + { + var fs = FileSystems.GetFileSystem(); var ms = new MemoryStream(Encoding.UTF8.GetBytes("test")); var virtPath = fs.GetMediaPath("file.txt", Guid.NewGuid(), Guid.NewGuid()); fs.AddFile(virtPath, ms); @@ -92,51 +108,37 @@ namespace Umbraco.Tests.IO Assert.IsTrue(Directory.Exists(physPath)); } - public void Singleton_Typed_File_System() + [Test] + public void Cannot_Get_InvalidFileSystem() { - var fs1 = FileSystems.GetFileSystemProvider(); - var fs2 = FileSystems.GetFileSystemProvider(); - - Assert.AreSame(fs1, fs2); + // throws because InvalidTypedFileSystem does not have the proper attribute with an alias + Assert.Throws(() => FileSystems.GetFileSystem()); } [Test] - public void Exception_Thrown_On_Invalid_Typed_File_System() - { - Assert.Throws(() => FileSystems.GetFileSystemProvider()); - } - - [Test] - public void Exception_Thrown_On_NonConfigured_Typed_File_System() + public void Cannot_Get_NonConfiguredFileSystem() { // note: we need to reset the manager between tests else the Accept_Fallback test would corrupt that one - Assert.Throws(() => FileSystems.GetFileSystemProvider()); + // throws because NonConfiguredFileSystem has the proper attribute with an alias, + // but then the container cannot find an IFileSystem implementation for that alias + Assert.Throws(() => FileSystems.GetFileSystem()); + + // all we'd need to pass is to register something like: + //_container.Register("noconfig", factory => new PhysicalFileSystem("~/foo")); } - [Test] - public void Accept_Fallback_On_NonConfigured_Typed_File_System() + internal class InvalidFileSystem : FileSystemWrapper { - var fs = FileSystems.GetFileSystemProvider(() => new PhysicalFileSystem("~/App_Data/foo")); - - Assert.NotNull(fs); - } - - /// - /// Used in unit tests, for a typed file system we need to inherit from FileSystemWrapper and they MUST have a ctor - /// that only accepts a base IFileSystem object - /// - internal class InvalidTypedFileSystem : FileSystemWrapper - { - public InvalidTypedFileSystem(IFileSystem wrapped, string invalidParam) - : base(wrapped) + public InvalidFileSystem(IFileSystem innerFileSystem) + : base(innerFileSystem) { } } - [FileSystemProvider("noconfig")] - internal class NonConfiguredTypeFileSystem : FileSystemWrapper + [FileSystem("noconfig")] + internal class NonConfiguredFileSystem : FileSystemWrapper { - public NonConfiguredTypeFileSystem(IFileSystem wrapped) - : base(wrapped) + public NonConfiguredFileSystem(IFileSystem innerFileSystem) + : base(innerFileSystem) { } } } diff --git a/src/Umbraco.Tests/Scoping/ScopeEventDispatcherTests.cs b/src/Umbraco.Tests/Scoping/ScopeEventDispatcherTests.cs index f652205147..56987416a4 100644 --- a/src/Umbraco.Tests/Scoping/ScopeEventDispatcherTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopeEventDispatcherTests.cs @@ -33,8 +33,7 @@ namespace Umbraco.Tests.Scoping _testObjects = new TestObjects(container); - Current.Container.RegisterSingleton(f => Current.Container); - Current.Container.RegisterSingleton(factory => new FileSystems(factory.TryGetInstance())); + Current.Container.RegisterSingleton(factory => new FileSystems(container, factory.TryGetInstance())); Current.Container.RegisterCollectionBuilder(); SettingsForTests.Reset(); // ensure we have configuration diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index fb4eed6180..b2ac7b4431 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -240,7 +240,7 @@ namespace Umbraco.Tests.TestHelpers databaseFactory = new UmbracoDatabaseFactory(Constants.System.UmbracoConnectionName, GetDefaultSqlSyntaxProviders(logger), logger, mappers); } - fileSystems = fileSystems ?? new FileSystems(logger); + fileSystems = fileSystems ?? new FileSystems(Current.Container, logger); var scopeProvider = new ScopeProvider(databaseFactory, fileSystems, logger); return scopeProvider; } diff --git a/src/Umbraco.Web/Editors/ImagesController.cs b/src/Umbraco.Web/Editors/ImagesController.cs index e9b89a0ab4..0eb0f54882 100644 --- a/src/Umbraco.Web/Editors/ImagesController.cs +++ b/src/Umbraco.Web/Editors/ImagesController.cs @@ -17,10 +17,10 @@ namespace Umbraco.Web.Editors [PluginController("UmbracoApi")] public class ImagesController : UmbracoAuthorizedApiController { - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; private readonly IContentSection _contentSection; - public ImagesController(MediaFileSystem mediaFileSystem, IContentSection contentSection) + public ImagesController(IMediaFileSystem mediaFileSystem, IContentSection contentSection) { _mediaFileSystem = mediaFileSystem; _contentSection = contentSection; diff --git a/src/Umbraco.Web/Media/UploadAutoFillProperties.cs b/src/Umbraco.Web/Media/UploadAutoFillProperties.cs index 6a56dec918..01ced179d6 100644 --- a/src/Umbraco.Web/Media/UploadAutoFillProperties.cs +++ b/src/Umbraco.Web/Media/UploadAutoFillProperties.cs @@ -15,11 +15,11 @@ namespace Umbraco.Web.Media /// internal class UploadAutoFillProperties { - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; private readonly ILogger _logger; private readonly IContentSection _contentSection; - public UploadAutoFillProperties(MediaFileSystem mediaFileSystem, ILogger logger, IContentSection contentSection) + public UploadAutoFillProperties(IMediaFileSystem mediaFileSystem, ILogger logger, IContentSection contentSection) { _mediaFileSystem = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); diff --git a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs index 412b8c9766..2b32e9d774 100644 --- a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyEditor.cs @@ -15,11 +15,11 @@ namespace Umbraco.Web.PropertyEditors [DataEditor(Constants.PropertyEditors.Aliases.UploadField, "File upload", "fileupload", Icon = "icon-download-alt", Group = "media")] public class FileUploadPropertyEditor : DataEditor { - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; private readonly IContentSection _contentSection; private readonly UploadAutoFillProperties _uploadAutoFillProperties; - public FileUploadPropertyEditor(ILogger logger, MediaFileSystem mediaFileSystem, IContentSection contentSection) + public FileUploadPropertyEditor(ILogger logger, IMediaFileSystem mediaFileSystem, IContentSection contentSection) : base(logger) { _mediaFileSystem = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem)); diff --git a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyValueEditor.cs b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyValueEditor.cs index 47711507b2..f36dd6bfa8 100644 --- a/src/Umbraco.Web/PropertyEditors/FileUploadPropertyValueEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/FileUploadPropertyValueEditor.cs @@ -15,9 +15,9 @@ namespace Umbraco.Web.PropertyEditors /// internal class FileUploadPropertyValueEditor : DataValueEditor { - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; - public FileUploadPropertyValueEditor(DataEditorAttribute attribute, MediaFileSystem mediaFileSystem) + public FileUploadPropertyValueEditor(DataEditorAttribute attribute, IMediaFileSystem mediaFileSystem) : base(attribute) { _mediaFileSystem = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem)); diff --git a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs index c8a7bfc80c..70b705f397 100644 --- a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyEditor.cs @@ -22,7 +22,7 @@ namespace Umbraco.Web.PropertyEditors [DataEditor(Constants.PropertyEditors.Aliases.ImageCropper, "Image Cropper", "imagecropper", ValueType = ValueTypes.Json, HideLabel = false, Group="media", Icon="icon-crop")] public class ImageCropperPropertyEditor : DataEditor { - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; private readonly IContentSection _contentSettings; private readonly IDataTypeService _dataTypeService; private readonly UploadAutoFillProperties _autoFillProperties; @@ -30,7 +30,7 @@ namespace Umbraco.Web.PropertyEditors /// /// Initializes a new instance of the class. /// - public ImageCropperPropertyEditor(ILogger logger, MediaFileSystem mediaFileSystem, IContentSection contentSettings, IDataTypeService dataTypeService) + public ImageCropperPropertyEditor(ILogger logger, IMediaFileSystem mediaFileSystem, IContentSection contentSettings, IDataTypeService dataTypeService) : base(logger) { _mediaFileSystem = mediaFileSystem ?? throw new ArgumentNullException(nameof(mediaFileSystem)); diff --git a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs index 4f44352d34..88da8982c7 100644 --- a/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/ImageCropperPropertyValueEditor.cs @@ -19,9 +19,9 @@ namespace Umbraco.Web.PropertyEditors internal class ImageCropperPropertyValueEditor : DataValueEditor // fixme core vs web? { private readonly ILogger _logger; - private readonly MediaFileSystem _mediaFileSystem; + private readonly IMediaFileSystem _mediaFileSystem; - public ImageCropperPropertyValueEditor(DataEditorAttribute attribute, ILogger logger, MediaFileSystem mediaFileSystem) + public ImageCropperPropertyValueEditor(DataEditorAttribute attribute, ILogger logger, IMediaFileSystem mediaFileSystem) : base(attribute) { _logger = logger ?? throw new ArgumentNullException(nameof(logger));