diff --git a/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs b/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs index a22748094a..28e1de3996 100644 --- a/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs +++ b/src/Umbraco.Core/HealthCheck/Checks/Permissions/FolderAndFilePermissionsCheck.cs @@ -32,22 +32,16 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions /// /// Get the status for this health check /// - /// - public override IEnumerable GetStatus() - { - //return the statuses - return new[] { CheckFolderPermissions(), CheckFilePermissions() }; - } + // TODO: This should really just run the IFilePermissionHelper.RunFilePermissionTestSuite and then we'd have a + // IFilePermissions interface resolved as a collection within the IFilePermissionHelper that runs checks against all + // IFilePermissions registered. Then there's no hard coding things done here and the checks here will be consistent + // with the checks run in IFilePermissionHelper.RunFilePermissionTestSuite which occurs on install too. + public override IEnumerable GetStatus() => new[] { CheckFolderPermissions(), CheckFilePermissions() }; /// /// Executes the action and returns it's status /// - /// - /// - public override HealthCheckStatus ExecuteAction(HealthCheckAction action) - { - throw new InvalidOperationException("FolderAndFilePermissionsCheck has no executable actions"); - } + public override HealthCheckStatus ExecuteAction(HealthCheckAction action) => throw new InvalidOperationException("FolderAndFilePermissionsCheck has no executable actions"); private HealthCheckStatus CheckFolderPermissions() { @@ -67,8 +61,8 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions { Constants.SystemDirectories.MvcViews, PermissionCheckRequirement.Optional } }; - //These are special paths to check that will restart an app domain if a file is written to them, - //so these need to be tested differently + // These are special paths to check that will restart an app domain if a file is written to them, + // so these need to be tested differently var pathsToCheckWithRestarts = new Dictionary { { Constants.SystemDirectories.Bin, PermissionCheckRequirement.Optional } @@ -80,7 +74,7 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions var optionalPathCheckResult = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out var optionalFailedPaths); - //now check the special folders + // now check the special folders var requiredPathCheckResult2 = _filePermissionHelper.EnsureDirectories( GetPathsToCheck(pathsToCheckWithRestarts, PermissionCheckRequirement.Required), out var requiredFailedPaths2, writeCausesRestart: true); var optionalPathCheckResult2 = _filePermissionHelper.EnsureDirectories( @@ -89,7 +83,7 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions requiredPathCheckResult = requiredPathCheckResult && requiredPathCheckResult2; optionalPathCheckResult = optionalPathCheckResult && optionalPathCheckResult2; - //combine the paths + // combine the paths requiredFailedPaths = requiredFailedPaths.Concat(requiredFailedPaths2).ToList(); optionalFailedPaths = requiredFailedPaths.Concat(optionalFailedPaths2).ToList(); @@ -106,23 +100,19 @@ namespace Umbraco.Core.HealthCheck.Checks.Permissions }; // Run checks for required and optional paths for modify permission - IEnumerable requiredFailedPaths; - IEnumerable optionalFailedPaths; - var requiredPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out requiredFailedPaths); - var optionalPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out optionalFailedPaths); + var requiredPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Required), out IEnumerable requiredFailedPaths); + var optionalPathCheckResult = _filePermissionHelper.EnsureFiles(GetPathsToCheck(pathsToCheck, PermissionCheckRequirement.Optional), out IEnumerable optionalFailedPaths); return GetStatus(requiredPathCheckResult, requiredFailedPaths, optionalPathCheckResult, optionalFailedPaths, PermissionCheckFor.File); } - private string[] GetPathsToCheck(Dictionary pathsToCheck, - PermissionCheckRequirement requirement) - { - return pathsToCheck + private string[] GetPathsToCheck( + Dictionary pathsToCheck, + PermissionCheckRequirement requirement) => pathsToCheck .Where(x => x.Value == requirement) .Select(x => _hostingEnvironment.MapPathContentRoot(x.Key)) .OrderBy(x => x) .ToArray(); - } private HealthCheckStatus GetStatus(bool requiredPathCheckResult, IEnumerable requiredFailedPaths, bool optionalPathCheckResult, IEnumerable optionalFailedPaths, PermissionCheckFor checkingFor) { diff --git a/src/Umbraco.Core/Install/IFilePermissionHelper.cs b/src/Umbraco.Core/Install/IFilePermissionHelper.cs index b60839cb00..ab521d214e 100644 --- a/src/Umbraco.Core/Install/IFilePermissionHelper.cs +++ b/src/Umbraco.Core/Install/IFilePermissionHelper.cs @@ -1,11 +1,26 @@ -using System.Collections.Generic; +using System.Collections.Generic; namespace Umbraco.Core.Install { public interface IFilePermissionHelper { bool RunFilePermissionTestSuite(out Dictionary> report); + + /// + /// This will test the directories for write access + /// + /// The directories to check + /// The resulting errors, if any + /// + /// If this is false, the easiest way to test for write access is to write a temp file, however some folder will cause + /// an App Domain restart if a file is written to the folder, so in that case we need to use the ACL APIs which aren't as + /// reliable but we cannot write a file since it will cause an app domain restart. + /// + /// Returns true if test succeeds + // TODO: This shouldn't exist, see notes in FolderAndFilePermissionsCheck.GetStatus bool EnsureDirectories(string[] dirs, out IEnumerable errors, bool writeCausesRestart = false); + + // TODO: This shouldn't exist, see notes in FolderAndFilePermissionsCheck.GetStatus bool EnsureFiles(string[] files, out IEnumerable errors); } } diff --git a/src/Umbraco.Core/PublishedCache/IPublishedSnapshotService.cs b/src/Umbraco.Core/PublishedCache/IPublishedSnapshotService.cs index ce61dc4b8e..7307ba97f9 100644 --- a/src/Umbraco.Core/PublishedCache/IPublishedSnapshotService.cs +++ b/src/Umbraco.Core/PublishedCache/IPublishedSnapshotService.cs @@ -33,13 +33,6 @@ namespace Umbraco.Web.PublishedCache /// which is not specified and depends on the actual published snapshot service implementation. IPublishedSnapshot CreatePublishedSnapshot(string previewToken); - /// - /// Ensures that the published snapshot has the proper environment to run. - /// - /// The errors, if any. - /// A value indicating whether the published snapshot has the proper environment to run. - bool EnsureEnvironment(out IEnumerable errors); - /// /// Rebuilds internal database caches (but does not reload). /// diff --git a/src/Umbraco.Infrastructure/Install/FilePermissionHelper.cs b/src/Umbraco.Infrastructure/Install/FilePermissionHelper.cs index a12634f01d..00f7c80fe4 100644 --- a/src/Umbraco.Infrastructure/Install/FilePermissionHelper.cs +++ b/src/Umbraco.Infrastructure/Install/FilePermissionHelper.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.IO; @@ -20,7 +20,7 @@ namespace Umbraco.Web.Install private readonly string[] _packagesPermissionsDirs; // ensure Umbraco can write to these files (the directories must exist) - private readonly string[] _permissionFiles = { }; + private readonly string[] _permissionFiles = Array.Empty(); private readonly GlobalSettings _globalSettings; private readonly IIOHelper _ioHelper; private readonly IHostingEnvironment _hostingEnvironment; @@ -49,26 +49,13 @@ namespace Umbraco.Web.Install if (EnsureFiles(_permissionFiles, out errors) == false) report["File writing failed"] = errors.ToList(); - if (TestPublishedSnapshotService(out errors) == false) - report["Published snapshot environment check failed"] = errors.ToList(); - if (EnsureCanCreateSubDirectory(_globalSettings.UmbracoMediaPath, out errors) == false) report["Media folder creation failed"] = errors.ToList(); return report.Count == 0; } - /// - /// This will test the directories for write access - /// - /// - /// - /// - /// If this is false, the easiest way to test for write access is to write a temp file, however some folder will cause - /// an App Domain restart if a file is written to the folder, so in that case we need to use the ACL APIs which aren't as - /// reliable but we cannot write a file since it will cause an app domain restart. - /// - /// + /// public bool EnsureDirectories(string[] dirs, out IEnumerable errors, bool writeCausesRestart = false) { List temp = null; @@ -96,9 +83,16 @@ namespace Umbraco.Web.Install foreach (var file in files) { var canWrite = TryWriteFile(file); - if (canWrite) continue; + if (canWrite) + { + continue; + } + + if (temp == null) + { + temp = new List(); + } - if (temp == null) temp = new List(); temp.Add(file); success = false; } @@ -130,11 +124,6 @@ namespace Umbraco.Web.Install return success; } - public bool TestPublishedSnapshotService(out IEnumerable errors) - { - return _publishedSnapshotService.EnsureEnvironment(out errors); - } - // tries to create a sub-directory // if successful, the sub-directory is deleted // creates the directory if needed - does not delete it diff --git a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs index 1ddb1cef63..49283de276 100644 --- a/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs @@ -45,7 +45,6 @@ namespace Umbraco.Web.PublishedCache.NuCache private readonly IPublishedModelFactory _publishedModelFactory; private readonly IDefaultCultureAccessor _defaultCultureAccessor; private readonly IHostingEnvironment _hostingEnvironment; - private readonly IIOHelper _ioHelper; private readonly NuCacheSettings _config; private bool _isReady; @@ -90,7 +89,6 @@ namespace Umbraco.Web.PublishedCache.NuCache IEntityXmlSerializer entitySerializer, IPublishedModelFactory publishedModelFactory, IHostingEnvironment hostingEnvironment, - IIOHelper ioHelper, // TODO: Remove this, it is only needed for "EnsureEnvironment" which doesn't need to belong to this service IOptions config) { _serviceContext = serviceContext; @@ -105,7 +103,6 @@ namespace Umbraco.Web.PublishedCache.NuCache _defaultCultureAccessor = defaultCultureAccessor; _globalSettings = globalSettings.Value; _hostingEnvironment = hostingEnvironment; - _ioHelper = ioHelper; _config = config.Value; // we need an Xml serializer here so that the member cache can support XPath, @@ -253,14 +250,6 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - public bool EnsureEnvironment(out IEnumerable errors) - { - // must have app_data and be able to write files into it - var ok = _ioHelper.TryCreateDirectory(GetLocalFilesPath()); - errors = ok ? Enumerable.Empty() : new[] { "NuCache local files." }; - return ok; - } - /// /// Populates the stores /// diff --git a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs index 0111c9f088..1b65d6c70e 100644 --- a/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs +++ b/src/Umbraco.Tests/LegacyXmlPublishedCache/XmlPublishedSnapshotService.cs @@ -2,14 +2,10 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging; -using Umbraco.Core; using Umbraco.Core.Cache; -using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.Models; using Umbraco.Core.Hosting; -using Umbraco.Core.IO; using Umbraco.Core.Models; -using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.PublishedContent; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Runtime; @@ -136,30 +132,6 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache #endregion - #region Environment - - public bool EnsureEnvironment(out IEnumerable errors) - { - // Test creating/saving/deleting a file in the same location as the content xml file - // NOTE: We cannot modify the xml file directly because a background thread is responsible for - // that and we might get lock issues. - try - { - XmlStore.EnsureFilePermission(); - errors = Enumerable.Empty(); - return true; - } - catch - { - errors = new[] { SystemFiles.GetContentCacheXml(_hostingEnvironment) }; - return false; - } - } - - #endregion - - #region Caches - public IPublishedSnapshot CreatePublishedSnapshot(string previewToken) { // use _requestCache to store recursive properties lookup, etc. both in content @@ -176,8 +148,6 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache domainCache); } - #endregion - #region Xml specific /// diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs index 2297f5cf5b..cc3325377e 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheChildrenTests.cs @@ -162,7 +162,6 @@ namespace Umbraco.Tests.PublishedContent Mock.Of(), PublishedModelFactory, hostingEnvironment, - TestHelper.IOHelper, Options.Create(nuCacheSettings)); // invariant is the current default diff --git a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs index e11ccc9612..b56c0047ed 100644 --- a/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs +++ b/src/Umbraco.Tests/PublishedContent/NuCacheTests.cs @@ -202,7 +202,6 @@ namespace Umbraco.Tests.PublishedContent Mock.Of(), publishedModelFactory, TestHelper.GetHostingEnvironment(), - TestHelper.IOHelper, Microsoft.Extensions.Options.Options.Create(nuCacheSettings)); // invariant is the current default diff --git a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs index cd733abad2..76a947f63a 100644 --- a/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs +++ b/src/Umbraco.Tests/Scoping/ScopedNuCacheTests.cs @@ -103,7 +103,6 @@ namespace Umbraco.Tests.Scoping Factory.GetRequiredService(), new NoopPublishedModelFactory(), hostingEnvironment, - IOHelper, Microsoft.Extensions.Options.Options.Create(nuCacheSettings)); lifetime.Raise(e => e.ApplicationInit += null, EventArgs.Empty);