From 860c8e8ae2c4790e0c16e8e5f0427a436352a338 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Thu, 24 Feb 2022 10:17:34 +0000 Subject: [PATCH] Filesystem based MainDomLock & extract interface for MainDomKey generation (#12037) * Extract MainDomKey generation to its own class to ease customization. Also add discriminator config value to GlobalSettings for advanced users. Prevents a mandatory custom implementation, should be good enough for the vast majority of use cases. * Prevent duplicate runs of ScheduledPublishing during slot swap. * Add filesystem based MainDomLock --- .../Configuration/Models/GlobalSettings.cs | 8 ++ .../Persistence/Constants-Locks.cs | 5 + .../Runtime/IMainDomKeyGenerator.cs | 13 ++ src/Umbraco.Core/Runtime/MainDom.cs | 4 +- .../UmbracoBuilder.CoreServices.cs | 10 +- .../HostedServices/ScheduledPublishing.cs | 51 +++++-- .../Migrations/Install/DatabaseDataCreator.cs | 1 + .../Migrations/Upgrade/UmbracoPlan.cs | 3 + .../V_9_4_0/AddScheduledPublishingLock.cs | 15 ++ .../Runtime/DefaultMainDomKeyGenerator.cs | 34 +++++ .../Runtime/FileSystemMainDomLock.cs | 131 ++++++++++++++++++ .../Runtime/SqlMainDomLock.cs | 63 ++++++--- .../Runtime/FileSystemMainDomLockTests.cs | 97 +++++++++++++ .../ScheduledPublishingTests.cs | 11 +- .../DefaultMainDomKeyGeneratorTests.cs | 47 +++++++ 15 files changed, 457 insertions(+), 36 deletions(-) create mode 100644 src/Umbraco.Core/Runtime/IMainDomKeyGenerator.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_4_0/AddScheduledPublishingLock.cs create mode 100644 src/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGenerator.cs create mode 100644 src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs create mode 100644 tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs create mode 100644 tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGeneratorTests.cs diff --git a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs index 7e3e1a2700..53584af1d1 100644 --- a/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs +++ b/src/Umbraco.Core/Configuration/Models/GlobalSettings.cs @@ -137,6 +137,14 @@ namespace Umbraco.Cms.Core.Configuration.Models /// public string MainDomLock { get; set; } = string.Empty; + /// + /// Gets or sets a value to discriminate MainDom boundaries. + /// + /// Generally the default should suffice but useful for advanced scenarios e.g. azure deployment slot based zero downtime deployments. + /// + /// + public string MainDomKeyDiscriminator { get; set; } = string.Empty; + /// /// Gets or sets the telemetry ID. /// diff --git a/src/Umbraco.Core/Persistence/Constants-Locks.cs b/src/Umbraco.Core/Persistence/Constants-Locks.cs index 5312bf6886..3c0b2c4d28 100644 --- a/src/Umbraco.Core/Persistence/Constants-Locks.cs +++ b/src/Umbraco.Core/Persistence/Constants-Locks.cs @@ -65,6 +65,11 @@ namespace Umbraco.Cms.Core /// All languages. /// public const int Languages = -340; + + /// + /// ScheduledPublishing job. + /// + public const int ScheduledPublishing = -341; } } } diff --git a/src/Umbraco.Core/Runtime/IMainDomKeyGenerator.cs b/src/Umbraco.Core/Runtime/IMainDomKeyGenerator.cs new file mode 100644 index 0000000000..5b8fb819e6 --- /dev/null +++ b/src/Umbraco.Core/Runtime/IMainDomKeyGenerator.cs @@ -0,0 +1,13 @@ +namespace Umbraco.Cms.Core.Runtime +{ + /// + /// Defines a class which can generate a distinct key for a MainDom boundary. + /// + public interface IMainDomKeyGenerator + { + /// + /// Returns a key that signifies a MainDom boundary. + /// + string GenerateKey(); + } +} diff --git a/src/Umbraco.Core/Runtime/MainDom.cs b/src/Umbraco.Core/Runtime/MainDom.cs index 08d11db5cd..d22176d9cf 100644 --- a/src/Umbraco.Core/Runtime/MainDom.cs +++ b/src/Umbraco.Core/Runtime/MainDom.cs @@ -87,7 +87,7 @@ namespace Umbraco.Cms.Core.Runtime if (_isMainDom.HasValue == false) { - throw new InvalidOperationException("Register called when MainDom has not been acquired"); + throw new InvalidOperationException("Register called before IsMainDom has been established"); } else if (_isMainDom == false) { @@ -225,7 +225,7 @@ namespace Umbraco.Cms.Core.Runtime { if (!_isMainDom.HasValue) { - throw new InvalidOperationException("MainDom has not been acquired yet"); + throw new InvalidOperationException("IsMainDom has not been established yet"); } return _isMainDom.Value; } diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index f4a4866beb..1c8b3ec0de 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -218,6 +218,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection private static IUmbracoBuilder AddMainDom(this IUmbracoBuilder builder) { + builder.Services.AddSingleton(); builder.Services.AddSingleton(factory => { var globalSettings = factory.GetRequiredService>(); @@ -229,15 +230,20 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection var isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); var loggerFactory = factory.GetRequiredService(); var npocoMappers = factory.GetRequiredService(); + var mainDomKeyGenerator = factory.GetRequiredService(); + + if (globalSettings.Value.MainDomLock == "FileSystemMainDomLock") + { + return new FileSystemMainDomLock(loggerFactory.CreateLogger(), mainDomKeyGenerator, hostingEnvironment); + } return globalSettings.Value.MainDomLock.Equals("SqlMainDomLock") || isWindows == false ? (IMainDomLock)new SqlMainDomLock( - loggerFactory.CreateLogger(), loggerFactory, globalSettings, connectionStrings, dbCreator, - hostingEnvironment, + mainDomKeyGenerator, databaseSchemaCreatorFactory, npocoMappers) : new MainDomSemaphoreLock(loggerFactory.CreateLogger(), hostingEnvironment); diff --git a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs index d59ea4fad3..429389273f 100644 --- a/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs +++ b/src/Umbraco.Infrastructure/HostedServices/ScheduledPublishing.cs @@ -5,13 +5,15 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Runtime; -using Umbraco.Cms.Core.Security; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Core.Web; +using Umbraco.Cms.Web.Common.DependencyInjection; namespace Umbraco.Cms.Infrastructure.HostedServices { @@ -27,20 +29,16 @@ namespace Umbraco.Cms.Infrastructure.HostedServices private readonly IMainDom _mainDom; private readonly IRuntimeState _runtimeState; private readonly IServerMessenger _serverMessenger; + private readonly IScopeProvider _scopeProvider; private readonly IServerRoleAccessor _serverRegistrar; private readonly IUmbracoContextFactory _umbracoContextFactory; /// /// Initializes a new instance of the class. /// - /// Representation of the state of the Umbraco runtime. - /// Representation of the main application domain. - /// Provider of server registrations to the distributed cache. - /// Service for handling content operations. - /// Service for creating and managing Umbraco context. - /// The typed logger. - /// Service broadcasting cache notifications to registered servers. - /// Creates and manages instances. + // Note: Ignoring the two version notice rule as this class should probably be internal. + // We don't expect anyone downstream to be instantiating a HostedService + [Obsolete("This constructor will be removed in version 10, please use an alternative constructor.")] public ScheduledPublishing( IRuntimeState runtimeState, IMainDom mainDom, @@ -49,6 +47,30 @@ namespace Umbraco.Cms.Infrastructure.HostedServices IUmbracoContextFactory umbracoContextFactory, ILogger logger, IServerMessenger serverMessenger) + : this( + runtimeState, + mainDom, + serverRegistrar, + contentService, + umbracoContextFactory, + logger, + serverMessenger, + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + /// + /// Initializes a new instance of the class. + /// + public ScheduledPublishing( + IRuntimeState runtimeState, + IMainDom mainDom, + IServerRoleAccessor serverRegistrar, + IContentService contentService, + IUmbracoContextFactory umbracoContextFactory, + ILogger logger, + IServerMessenger serverMessenger, + IScopeProvider scopeProvider) : base(TimeSpan.FromMinutes(1), DefaultDelay) { _runtimeState = runtimeState; @@ -58,6 +80,7 @@ namespace Umbraco.Cms.Infrastructure.HostedServices _umbracoContextFactory = umbracoContextFactory; _logger = logger; _serverMessenger = serverMessenger; + _scopeProvider = scopeProvider; } public override Task PerformExecuteAsync(object state) @@ -93,8 +116,6 @@ namespace Umbraco.Cms.Infrastructure.HostedServices try { - // We don't need an explicit scope here because PerformScheduledPublish creates it's own scope - // so it's safe as it will create it's own ambient scope. // Ensure we run with an UmbracoContext, because this will run in a background task, // and developers may be using the UmbracoContext in the event handlers. @@ -105,6 +126,14 @@ namespace Umbraco.Cms.Infrastructure.HostedServices // - and we should definitively *not* have to flush it here (should be auto) using UmbracoContextReference contextReference = _umbracoContextFactory.EnsureUmbracoContext(); + using IScope scope = _scopeProvider.CreateScope(autoComplete: true); + + /* We used to assume that there will never be two instances running concurrently where (IsMainDom && ServerRole == SchedulingPublisher) + * However this is possible during an azure deployment slot swap for the SchedulingPublisher instance when trying to achieve zero downtime deployments. + * If we take a distributed write lock, we are certain that the multiple instances of the job will not run in parallel. + * It's possible that during the swapping process we may run this job more frequently than intended but this is not of great concern and it's + * only until the old SchedulingPublisher shuts down. */ + scope.EagerWriteLock(Constants.Locks.ScheduledPublishing); try { // Run diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs index 4f2ef1f2e9..25dadb0c85 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseDataCreator.cs @@ -175,6 +175,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.Domains, Name = "Domains" }); _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.KeyValues, Name = "KeyValues" }); _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.Languages, Name = "Languages" }); + _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.ScheduledPublishing, Name = "ScheduledPublishing" }); _database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.MainDom, Name = "MainDom" }); } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 39d7d886b3..11de1209b3 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -16,6 +16,7 @@ using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_1_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_2_0; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_3_0; +using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_4_0; using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade @@ -280,6 +281,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade To("{CA7A1D9D-C9D4-4914-BC0A-459E7B9C3C8C}"); To("{0828F206-DCF7-4F73-ABBB-6792275532EB}"); + // TO 9.4.0 + To("{DBBA1EA0-25A1-4863-90FB-5D306FB6F1E1}"); } } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_4_0/AddScheduledPublishingLock.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_4_0/AddScheduledPublishingLock.cs new file mode 100644 index 0000000000..01cfb22a3d --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_4_0/AddScheduledPublishingLock.cs @@ -0,0 +1,15 @@ +using Umbraco.Cms.Infrastructure.Persistence.Dtos; + +namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_4_0 +{ + internal class AddScheduledPublishingLock : MigrationBase + { + public AddScheduledPublishingLock(IMigrationContext context) + : base(context) + { + } + + protected override void Migrate() => + Database.Insert(Cms.Core.Constants.DatabaseSchema.Tables.Lock, "id", false, new LockDto { Id = Cms.Core.Constants.Locks.ScheduledPublishing, Name = "ScheduledPublishing" }); + } +} diff --git a/src/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGenerator.cs b/src/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGenerator.cs new file mode 100644 index 0000000000..11944e776c --- /dev/null +++ b/src/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGenerator.cs @@ -0,0 +1,34 @@ +using System; +using System.Security.Cryptography; +using Microsoft.Extensions.Options; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Runtime; +using Umbraco.Extensions; + +namespace Umbraco.Cms.Infrastructure.Runtime +{ + + internal class DefaultMainDomKeyGenerator : IMainDomKeyGenerator + { + private readonly IHostingEnvironment _hostingEnvironment; + private readonly IOptionsMonitor _globalSettings; + + public DefaultMainDomKeyGenerator(IHostingEnvironment hostingEnvironment, IOptionsMonitor globalSettings) + { + _hostingEnvironment = hostingEnvironment; + _globalSettings = globalSettings; + } + + public string GenerateKey() + { + var machineName = Environment.MachineName; + var mainDomId = MainDom.GetMainDomId(_hostingEnvironment); + var discriminator = _globalSettings.CurrentValue.MainDomKeyDiscriminator; + + var rawKey = $"{machineName}{mainDomId}{discriminator}"; + + return rawKey.GenerateHash(); + } + } +} diff --git a/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs new file mode 100644 index 0000000000..d18e4085a0 --- /dev/null +++ b/src/Umbraco.Infrastructure/Runtime/FileSystemMainDomLock.cs @@ -0,0 +1,131 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Runtime; + +namespace Umbraco.Cms.Infrastructure.Runtime +{ + internal class FileSystemMainDomLock : IMainDomLock + { + private readonly ILogger _log; + + private readonly CancellationTokenSource _cancellationTokenSource = new(); + + private readonly string _lockFilePath; + private readonly string _releaseSignalFilePath; + + private FileStream _lockFileStream; + + public FileSystemMainDomLock( + ILogger log, + IMainDomKeyGenerator mainDomKeyGenerator, + IHostingEnvironment hostingEnvironment) + { + _log = log; + + var lockFileName = $"MainDom_{mainDomKeyGenerator.GenerateKey()}.lock"; + _lockFilePath = Path.Combine(hostingEnvironment.LocalTempPath, lockFileName); + _releaseSignalFilePath = $"{_lockFilePath}_release"; + } + + public Task AcquireLockAsync(int millisecondsTimeout) + { + var stopwatch = new Stopwatch(); + stopwatch.Start(); + + do + { + try + { + _log.LogDebug("Attempting to obtain MainDom lock file handle {lockFilePath}", _lockFilePath); + _lockFileStream = File.Open(_lockFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + DeleteLockReleaseFile(); + return Task.FromResult(true); + } + catch (IOException) + { + _log.LogDebug("Couldn't obtain MainDom lock file handle, signalling for release of {lockFilePath}", _lockFilePath); + CreateLockReleaseFile(); + Thread.Sleep(500); + } + catch (Exception ex) + { + _log.LogError(ex, "Unexpected exception attempting to obtain MainDom lock file handle {lockFilePath}, giving up", _lockFilePath); + return Task.FromResult(false); + } + } + while (stopwatch.ElapsedMilliseconds < millisecondsTimeout); + + return Task.FromResult(false); + } + + // Create a long running task to poll to check if anyone has created a lock release file. + public Task ListenAsync() => + Task.Factory.StartNew( + ListeningLoop, + _cancellationTokenSource.Token, + TaskCreationOptions.LongRunning, + TaskScheduler.Default); + + public void Dispose() + { + _lockFileStream?.Close(); + _lockFileStream = null; + } + + private void CreateLockReleaseFile() + { + try + { + // Dispose immediately to release the file handle so it's easier to cleanup in any process. + using FileStream releaseFileStream = File.Open(_releaseSignalFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite); + } + catch (Exception ex) + { + _log.LogError(ex, "Unexpected exception attempting to create lock release signal file {file}", _releaseSignalFilePath); + } + } + + private void DeleteLockReleaseFile() + { + while (File.Exists(_releaseSignalFilePath)) + { + try + { + File.Delete(_releaseSignalFilePath); + } + catch (Exception ex) + { + _log.LogError(ex, "Unexpected exception attempting to delete release signal file {file}", _releaseSignalFilePath); + Thread.Sleep(500); + } + } + } + + private void ListeningLoop() + { + while (true) + { + if (_cancellationTokenSource.IsCancellationRequested) + { + _log.LogDebug("ListenAsync Task canceled, exiting loop"); + return; + } + + if (File.Exists(_releaseSignalFilePath)) + { + _log.LogDebug("Found lock release signal file, releasing lock on {lockFilePath}", _lockFilePath); + _lockFileStream?.Close(); + _lockFileStream = null; + break; + } + + Thread.Sleep(2000); + } + } + } +} diff --git a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs index 8d1c74b619..635b0b9c28 100644 --- a/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Infrastructure/Runtime/SqlMainDomLock.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Security.Cryptography; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NPoco; @@ -18,6 +19,7 @@ using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Dtos; using Umbraco.Cms.Infrastructure.Persistence.Mappers; using Umbraco.Cms.Infrastructure.Persistence.SqlSyntax; +using Umbraco.Cms.Web.Common.DependencyInjection; using Umbraco.Extensions; using MapperCollection = Umbraco.Cms.Infrastructure.Persistence.Mappers.MapperCollection; @@ -30,7 +32,6 @@ namespace Umbraco.Cms.Infrastructure.Runtime private const string UpdatedSuffix = "_updated"; private readonly ILogger _logger; private readonly IOptions _globalSettings; - private readonly IHostingEnvironment _hostingEnvironment; private readonly IUmbracoDatabase _db; private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); private SqlServerSyntaxProvider _sqlServerSyntax; @@ -41,6 +42,9 @@ namespace Umbraco.Cms.Infrastructure.Runtime private bool _hasTable = false; private bool _acquireWhenTablesNotAvailable = false; + // Note: Ignoring the two version notice rule as this class should probably be internal. + // We don't expect anyone downstream to be instantiating a SqlMainDomLock, only resolving IMainDomLock + [Obsolete("This constructor will be removed in version 10, please use an alternative constructor.")] public SqlMainDomLock( ILogger logger, ILoggerFactory loggerFactory, @@ -51,25 +55,20 @@ namespace Umbraco.Cms.Infrastructure.Runtime DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory, NPocoMapperCollection npocoMappers, string connectionStringName) - { - // unique id for our appdomain, this is more unique than the appdomain id which is just an INT counter to its safer - _lockId = Guid.NewGuid().ToString(); - _logger = logger; - _globalSettings = globalSettings; - _sqlServerSyntax = new SqlServerSyntaxProvider(_globalSettings); - _hostingEnvironment = hostingEnvironment; - _dbFactory = new UmbracoDatabaseFactory( - loggerFactory.CreateLogger(), + : this( loggerFactory, - _globalSettings, - new MapperCollection(() => Enumerable.Empty()), + globalSettings, + connectionStrings, dbProviderFactoryCreator, + StaticServiceProvider.Instance.GetRequiredService(), databaseSchemaCreatorFactory, - npocoMappers, - connectionStringName); - MainDomKey = MainDomKeyPrefix + "-" + (Environment.MachineName + MainDom.GetMainDomId(_hostingEnvironment)).GenerateHash(); + npocoMappers) + { } + // Note: Ignoring the two version notice rule as this class should probably be internal. + // We don't expect anyone downstream to be instantiating a SqlMainDomLock, only resolving IMainDomLock + [Obsolete("This constructor will be removed in version 10, please use an alternative constructor.")] public SqlMainDomLock( ILogger logger, ILoggerFactory loggerFactory, @@ -80,18 +79,42 @@ namespace Umbraco.Cms.Infrastructure.Runtime DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory, NPocoMapperCollection npocoMappers) : this( - logger, loggerFactory, globalSettings, connectionStrings, dbProviderFactoryCreator, - hostingEnvironment, + StaticServiceProvider.Instance.GetRequiredService(), databaseSchemaCreatorFactory, - npocoMappers, - connectionStrings.CurrentValue.UmbracoConnectionString.ConnectionString - ) + npocoMappers) { + } + public SqlMainDomLock( + ILoggerFactory loggerFactory, + IOptions globalSettings, + IOptionsMonitor connectionStrings, + IDbProviderFactoryCreator dbProviderFactoryCreator, + IMainDomKeyGenerator mainDomKeyGenerator, + DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory, + NPocoMapperCollection npocoMappers) + { + // unique id for our appdomain, this is more unique than the appdomain id which is just an INT counter to its safer + _lockId = Guid.NewGuid().ToString(); + _logger = loggerFactory.CreateLogger(); + _globalSettings = globalSettings; + _sqlServerSyntax = new SqlServerSyntaxProvider(_globalSettings); + + _dbFactory = new UmbracoDatabaseFactory( + loggerFactory.CreateLogger(), + loggerFactory, + _globalSettings, + new MapperCollection(() => Enumerable.Empty()), + dbProviderFactoryCreator, + databaseSchemaCreatorFactory, + npocoMappers, + connectionStrings.CurrentValue.UmbracoConnectionString.ConnectionString); + + MainDomKey = MainDomKeyPrefix + "-" + mainDomKeyGenerator.GenerateKey(); } public async Task AcquireLockAsync(int millisecondsTimeout) diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs new file mode 100644 index 0000000000..59442a683a --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Runtime/FileSystemMainDomLockTests.cs @@ -0,0 +1,97 @@ +using System.IO; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using NUnit.Framework; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Runtime; +using Umbraco.Cms.Infrastructure.Runtime; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Runtime +{ + [TestFixture] + internal class FileSystemMainDomLockTests : UmbracoIntegrationTest + { + private IMainDomKeyGenerator MainDomKeyGenerator { get; set; } + + private IHostingEnvironment HostingEnvironment { get; set; } + + private FileSystemMainDomLock FileSystemMainDomLock { get; set; } + + private string LockFilePath { get; set; } + private string LockReleaseFilePath { get; set; } + + [SetUp] + public void SetUp() + { + MainDomKeyGenerator = GetRequiredService(); + HostingEnvironment = GetRequiredService(); + + var lockFileName = $"MainDom_{MainDomKeyGenerator.GenerateKey()}.lock"; + LockFilePath = Path.Combine(HostingEnvironment.LocalTempPath, lockFileName); + LockReleaseFilePath = LockFilePath + "_release"; + + var log = GetRequiredService>(); + FileSystemMainDomLock = new FileSystemMainDomLock(log, MainDomKeyGenerator, HostingEnvironment); + } + + [TearDown] + public void TearDown() + { + while (File.Exists(LockFilePath)) + { + File.Delete(LockFilePath); + } + while (File.Exists(LockReleaseFilePath)) + { + File.Delete(LockReleaseFilePath); + } + } + + [Test] + public async Task AcquireLockAsync_WhenNoOtherHoldsLockFileHandle_ReturnsTrue() + { + using var sut = FileSystemMainDomLock; + + var result = await sut.AcquireLockAsync(1000); + + Assert.True(result); + } + + [Test] + public async Task AcquireLockAsync_WhenTimeoutExceeded_ReturnsFalse() + { + await using var lockFile = File.Open(LockFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None); + + using var sut = FileSystemMainDomLock; + + var result = await sut.AcquireLockAsync(1000); + + Assert.False(result); + } + + [Test] + public async Task ListenAsync_WhenLockReleaseSignalFileFound_DropsLockFileHandle() + { + using var sut = FileSystemMainDomLock; + + await sut.AcquireLockAsync(1000); + + var before = await sut.AcquireLockAsync(1000); + + await using (_ = File.Open(LockReleaseFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite)) + { + } + + await sut.ListenAsync(); + + var after = await sut.AcquireLockAsync(1000); + + Assert.Multiple(() => + { + Assert.False(before); + Assert.True(after); + }); + } + } +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs index 2a8f006ab6..08999affe2 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/HostedServices/ScheduledPublishingTests.cs @@ -2,12 +2,15 @@ // See LICENSE for more details. using System; +using System.Data; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Runtime; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Security; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Sync; @@ -108,6 +111,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices var mockServerMessenger = new Mock(); + var mockScopeProvider = new Mock(); + mockScopeProvider + .Setup(x => x.CreateScope(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Mock.Of()); + return new ScheduledPublishing( mockRunTimeState.Object, mockMainDom.Object, @@ -115,7 +123,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.HostedServices _mockContentService.Object, mockUmbracoContextFactory.Object, _mockLogger.Object, - mockServerMessenger.Object); + mockServerMessenger.Object, + mockScopeProvider.Object); } private void VerifyScheduledPublishingNotPerformed() => VerifyScheduledPublishingPerformed(Times.Never()); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGeneratorTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGeneratorTests.cs new file mode 100644 index 0000000000..9b013aa38f --- /dev/null +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Runtime/DefaultMainDomKeyGeneratorTests.cs @@ -0,0 +1,47 @@ +using AutoFixture.NUnit3; +using Microsoft.Extensions.Options; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Infrastructure.Runtime; +using Umbraco.Cms.Tests.UnitTests.AutoFixture; + +namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Runtime +{ + [TestFixture] + internal class DefaultMainDomKeyGeneratorTests + { + [Test] + [AutoMoqData] + public void GenerateKey_WithConfiguredDiscriminatorValue_AltersHash( + [Frozen] IHostingEnvironment hostingEnvironment, + [Frozen] GlobalSettings globalSettings, + [Frozen] IOptionsMonitor globalSettingsMonitor, + DefaultMainDomKeyGenerator sut, + string aDiscriminator) + { + var withoutDiscriminator = sut.GenerateKey(); + globalSettings.MainDomKeyDiscriminator = aDiscriminator; + var withDiscriminator = sut.GenerateKey(); + + Assert.AreNotEqual(withoutDiscriminator, withDiscriminator); + } + + [Test] + [AutoMoqData] + public void GenerateKey_WithUnchangedDiscriminatorValue_ReturnsSameValue( + [Frozen] IHostingEnvironment hostingEnvironment, + [Frozen] GlobalSettings globalSettings, + [Frozen] IOptionsMonitor globalSettingsMonitor, + DefaultMainDomKeyGenerator sut, + string aDiscriminator) + { + globalSettings.MainDomKeyDiscriminator = aDiscriminator; + + var a = sut.GenerateKey(); + var b = sut.GenerateKey(); + + Assert.AreEqual(a, b); + } + } +}