diff --git a/docker-compose.yml b/docker-compose.yml index eb3c72d74..9bbb02658 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -56,7 +56,6 @@ services: - COSMOS_TOKEN=${COSMOS_TOKEN:-C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==} - COSMOS_IGNORE_SSL_CERT=true - APP_CONFIG_CONNECTION=${APP_CONFIG_CONNECTION:-local} - - LEASE_MANAGER_CONNECTION=local - Auth__Providers__0__Name=nhs-mail - Auth__Providers__0__Issuer=http://oidc-server - Auth__Providers__0__AuthorizeUri=http://localhost:8020/connect/authorize diff --git a/docs/concurrency.md b/docs/concurrency.md index d2c99bc55..b4bba8011 100644 --- a/docs/concurrency.md +++ b/docs/concurrency.md @@ -2,4 +2,4 @@ ## Make Booking -In order to prevent concurrent calls to make booking from booking the same time slot, and causing and overbooked block of time the system implements a leasing mechanism. Leases are acquired at a site level, meaning that only one booking can be processed for a given site at a given instance in time. The leasing mechanism using Azure Blob Leases, with a blob existing for each site (note the blob will be created on demand). The connection string to the blob storage account is configured with the LEASE_MANAGER_CONNECTION application setting. This can be set to `local` to allow local testing using an in memory lease manager. Consideration should be given to this setting when deploying into multiple regions, as this could introduce a loop whole if the function app in the separate regions are not using the same storage account for leasing. This will not be an issue during the alpha development as it will only be deployed to a single region. +In order to prevent concurrent calls to make booking from booking the same time slot, and causing and overbooked block of time the system implements a leasing mechanism. Leases are acquired at a site level, meaning that only one booking can be processed for a given site at a given instance in time. The leasing mechanism using Azure Blob Leases, with a blob existing for each site (note the blob will be created on demand). The connection string to the blob storage account is configured with the LEASE_MANAGER_CONNECTION application setting. Consideration should be given to this setting when deploying into multiple regions, as this could introduce a loop whole if the function app in the separate regions are not using the same storage account for leasing. This will not be an issue during the alpha development as it will only be deployed to a single region. diff --git a/src/api/Nhs.Appointments.Api/FunctionConfigurationExtensions.cs b/src/api/Nhs.Appointments.Api/FunctionConfigurationExtensions.cs index 33bc73bee..73c567a2c 100644 --- a/src/api/Nhs.Appointments.Api/FunctionConfigurationExtensions.cs +++ b/src/api/Nhs.Appointments.Api/FunctionConfigurationExtensions.cs @@ -123,15 +123,7 @@ public static IFunctionsWorkerApplicationBuilder ConfigureFunctionDependencies( .AddScoped() .AddTransient(); - var leaseManagerConnection = Environment.GetEnvironmentVariable("LEASE_MANAGER_CONNECTION"); - if (leaseManagerConnection == "local") - { - builder.Services.AddInMemoryLeasing(); - } - else - { - builder.Services.AddAzureBlobStoreLeasing(leaseManagerConnection, "leases"); - } + builder.Services.AddConcurrency(configuration); builder.Services.AddHttpClient(); diff --git a/src/api/Nhs.Appointments.Api/local.settings.json b/src/api/Nhs.Appointments.Api/local.settings.json index 9df75ebaa..14567fd59 100644 --- a/src/api/Nhs.Appointments.Api/local.settings.json +++ b/src/api/Nhs.Appointments.Api/local.settings.json @@ -19,7 +19,6 @@ "COSMOS_TOKEN": "C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==", "APP_CONFIG_CONNECTION": "local", "COSMOS_IGNORE_SSL_CERT": true, - "LEASE_MANAGER_CONNECTION": "local", "Auth__Providers__0__Name": "nhs-mail", "Auth__Providers__0__Issuer": "http://localhost:8020", "Auth__Providers__0__AuthorizeUri": "http://localhost:8020/connect/authorize", diff --git a/src/api/Nhs.Appointments.Core/Blob/AzureBlobStorage.cs b/src/api/Nhs.Appointments.Core/Blob/AzureBlobStorage.cs index 94788b6e1..3c5c14d19 100644 --- a/src/api/Nhs.Appointments.Core/Blob/AzureBlobStorage.cs +++ b/src/api/Nhs.Appointments.Core/Blob/AzureBlobStorage.cs @@ -6,7 +6,7 @@ public class AzureBlobStorage(BlobServiceClient blobServiceClient) : IAzureBlobS { public async Task GetBlobUploadStream(string containerName, string blobName) { - var blobClient = GetBlobClientFromContainerAndBlobName(containerName, blobName); + var blobClient = await GetBlobClientFromContainerAndBlobNameAsync(containerName, blobName); return await blobClient.OpenWriteAsync(true); } @@ -18,6 +18,13 @@ public BlobClient GetBlobClientFromContainerAndBlobName(string containerName, st return containerClient.GetBlobClient(blobName); } + public async Task GetBlobClientFromContainerAndBlobNameAsync(string containerName, string blobName) + { + var containerClient = await ResolveContainerClientAsync(containerName); + + return containerClient.GetBlobClient(blobName); + } + private BlobContainerClient ResolveContainerClient(string containerName) { var containers = blobServiceClient.GetBlobContainers(); @@ -26,4 +33,13 @@ private BlobContainerClient ResolveContainerClient(string containerName) ? blobServiceClient.GetBlobContainerClient(containerName) : blobServiceClient.CreateBlobContainer(containerName); } + + private async Task ResolveContainerClientAsync(string containerName) + { + var exists = await blobServiceClient.GetBlobContainersAsync().AnyAsync(x => x.Name.Equals(containerName)); + + return exists + ? blobServiceClient.GetBlobContainerClient(containerName) + : await blobServiceClient.CreateBlobContainerAsync(containerName); + } } diff --git a/src/api/Nhs.Appointments.Core/Blob/IAzureBlobStorage.cs b/src/api/Nhs.Appointments.Core/Blob/IAzureBlobStorage.cs index cdc55fa17..f34b53a15 100644 --- a/src/api/Nhs.Appointments.Core/Blob/IAzureBlobStorage.cs +++ b/src/api/Nhs.Appointments.Core/Blob/IAzureBlobStorage.cs @@ -7,4 +7,5 @@ public interface IAzureBlobStorage Task GetBlobUploadStream(string containerName, string blobName); BlobClient GetBlobClientFromContainerAndBlobName(string containerName, string blobName); + Task GetBlobClientFromContainerAndBlobNameAsync(string containerName, string blobName); } diff --git a/src/api/Nhs.Appointments.Core/Bookings/BookingWriteService.cs b/src/api/Nhs.Appointments.Core/Bookings/BookingWriteService.cs index ebbe5506b..93d130a61 100644 --- a/src/api/Nhs.Appointments.Core/Bookings/BookingWriteService.cs +++ b/src/api/Nhs.Appointments.Core/Bookings/BookingWriteService.cs @@ -48,7 +48,7 @@ public class BookingWriteService( IBookingsDocumentStore bookingDocumentStore, IBookingQueryService bookingQueryService, IReferenceNumberProvider referenceNumberProvider, - ILeaseManager leaseManager, + ILeaseManagerFactory leaseManagerFactory, IBookingAvailabilityStateService bookingAvailabilityStateService, IBookingEventFactory eventFactory, IMessageBus bus, @@ -64,7 +64,7 @@ public class BookingWriteService( var leaseKey = LeaseKeys.SiteKeyFactory.Create(booking.Site, booking.Date); - using var leaseContent = leaseManager.Acquire(leaseKey); + using var leaseContent = leaseManagerFactory.Create().Acquire(leaseKey); var availableSlots = await bookingAvailabilityStateService.GetAvailableSlots(booking.Site, from, to); @@ -358,7 +358,7 @@ private async Task> RecalculateAppointmen var leaseKey = LeaseKeys.SiteKeyFactory.Create(site, day); - using var leaseContent = leaseManager.Acquire(leaseKey); + using var leaseContent = leaseManagerFactory.Create().Acquire(leaseKey); var recalculations = (await bookingAvailabilityStateService.BuildRecalculations(site, bookingDayRange.Start, bookingDayRange.End, diff --git a/src/api/Nhs.Appointments.Core/Concurrency/AzureStorageLeaseManager.cs b/src/api/Nhs.Appointments.Core/Concurrency/AzureStorageLeaseManager.cs index 198a14484..a91013b6f 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/AzureStorageLeaseManager.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/AzureStorageLeaseManager.cs @@ -9,38 +9,46 @@ namespace Nhs.Appointments.Core.Concurrency; internal class AzureStorageLeaseManager : ILeaseManager { private readonly IAzureBlobStorage _azureBlobStorage; - private readonly LeaseManagerOptions _options; - private readonly int _acquireTimeInSeconds; + private readonly LeaseManagerOptions _defaultOptions; private readonly int _delayRetryTimeInMilliseconds; public AzureStorageLeaseManager( IOptions options, IAzureBlobStorage azureBlobStorage, - int acquireTimeInSeconds = 20, int delayRetryTimeInMilliseconds = 100 ) { - ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(acquireTimeInSeconds, 0, nameof(acquireTimeInSeconds)); ArgumentOutOfRangeException.ThrowIfLessThanOrEqual(delayRetryTimeInMilliseconds, 0, nameof(delayRetryTimeInMilliseconds)); _azureBlobStorage = azureBlobStorage ?? throw new ArgumentNullException(nameof(azureBlobStorage)); - _options = options.Value; - _acquireTimeInSeconds = acquireTimeInSeconds; _delayRetryTimeInMilliseconds = delayRetryTimeInMilliseconds; + _defaultOptions = options.Value; } - public ILeaseContext Acquire(string leaseKey) + public LeaseManagerMode Mode => LeaseManagerMode.DistributedAzureBlob; + + public ILeaseContext Acquire(string leaseKey, LeaseManagerOptions options = null) { - var leaseClient = GetLeaseClient(leaseKey); - var leasePipeline = CreateResiliencePipeline(); - leasePipeline.Execute(() => leaseClient.Acquire(TimeSpan.FromSeconds(_acquireTimeInSeconds))); + var leaseClient = GetLeaseClient(ResolveContainerName(options),leaseKey); + CreateResiliencePipeline().Execute(() => leaseClient.Acquire(ResolveTimeout(options))); return new LeaseContext(leaseKey, () => leaseClient.Release()); } + + public async Task AcquireAsync(string leaseKey, LeaseManagerOptions options = null) + { + var leaseClient = await GetLeaseClientAsync(ResolveContainerName(options),leaseKey); + await CreateResiliencePipeline().ExecuteAsync( + async (cancellationToken) => await leaseClient.AcquireAsync( + ResolveTimeout(options), + cancellationToken: cancellationToken)); - private BlobLeaseClient GetLeaseClient(string blobName) + return new LeaseContext(leaseKey, () => leaseClient.Release()); + } + + private BlobLeaseClient GetLeaseClient(string containerName, string blobName) { - var blobClient = _azureBlobStorage.GetBlobClientFromContainerAndBlobName(_options.ContainerName, blobName); + var blobClient = _azureBlobStorage.GetBlobClientFromContainerAndBlobName(containerName, blobName); if (blobClient.Exists() == false) { blobClient.Upload(BinaryData.FromString("")); @@ -48,6 +56,17 @@ private BlobLeaseClient GetLeaseClient(string blobName) return blobClient.GetBlobLeaseClient(); } + + private async Task GetLeaseClientAsync(string containerName, string blobName) + { + var blobClient = await _azureBlobStorage.GetBlobClientFromContainerAndBlobNameAsync(containerName, blobName); + if (await blobClient.ExistsAsync() == false) + { + await blobClient.UploadAsync(BinaryData.FromString("")); + } + + return blobClient.GetBlobLeaseClient(); + } private ResiliencePipeline> CreateResiliencePipeline() { @@ -63,5 +82,9 @@ private BlobLeaseClient GetLeaseClient(string blobName) Delay = TimeSpan.FromMilliseconds(_delayRetryTimeInMilliseconds) }) .Build(); - } + } + + private string ResolveContainerName(LeaseManagerOptions options = null) => options?.Realm ?? _defaultOptions.Realm; + private TimeSpan ResolveTimeout(LeaseManagerOptions options = null) => + options?.Timeout ?? _defaultOptions.Timeout; } diff --git a/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManager.cs b/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManager.cs index 5cf13e2b6..6015d2235 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManager.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManager.cs @@ -3,5 +3,7 @@ namespace Nhs.Appointments.Core.Concurrency; public interface ILeaseManager { - ILeaseContext Acquire(string leaseKey); + LeaseManagerMode Mode { get; } + ILeaseContext Acquire(string leaseKey, LeaseManagerOptions options = null); + Task AcquireAsync(string leaseKey, LeaseManagerOptions options = null); } diff --git a/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManagerFactory.cs b/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManagerFactory.cs new file mode 100644 index 000000000..e917305bb --- /dev/null +++ b/src/api/Nhs.Appointments.Core/Concurrency/ILeaseManagerFactory.cs @@ -0,0 +1,6 @@ +namespace Nhs.Appointments.Core.Concurrency; + +public interface ILeaseManagerFactory +{ + ILeaseManager Create(LeaseManagerMode? mode = null); +} diff --git a/src/api/Nhs.Appointments.Core/Concurrency/InMemoryLeaseManager.cs b/src/api/Nhs.Appointments.Core/Concurrency/InMemoryLeaseManager.cs index dfa8940b6..8bba74baf 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/InMemoryLeaseManager.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/InMemoryLeaseManager.cs @@ -5,18 +5,45 @@ namespace Nhs.Appointments.Core.Concurrency; internal class InMemoryLeaseManager : ILeaseManager { private readonly Dictionary _locks; - private readonly LeaseManagerOptions _options; + private readonly LeaseManagerOptions _defaultOptions; public InMemoryLeaseManager(IOptions options) { _locks = new Dictionary(); - _options = options.Value; + _defaultOptions = options.Value; } - public ILeaseContext Acquire(string leaseKey) + public LeaseManagerMode Mode => LeaseManagerMode.InMemory; + + public ILeaseContext Acquire(string leaseKey, LeaseManagerOptions options = null) { - SemaphoreSlim mutex; + var inMemoryLeaseKey = BuildInMemoryKey(leaseKey, options); + var mutex = ResolveMutex(inMemoryLeaseKey, options); + if (!mutex.Wait(ResolveTimeout(options))) + { + throw new AbandonedMutexException($"Abandoned attempt to acquire lock for lease key {inMemoryLeaseKey}"); + } + + return new LeaseContext(inMemoryLeaseKey, () => mutex.Release()); + } + + public async Task AcquireAsync(string leaseKey, LeaseManagerOptions options = null) + { + var inMemoryLeaseKey = BuildInMemoryKey(leaseKey, options); + var mutex = ResolveMutex(inMemoryLeaseKey, options); + + if (!(await mutex.WaitAsync(ResolveTimeout(options)))) + { + throw new AbandonedMutexException($"Abandoned attempt to acquire lock for lease key {inMemoryLeaseKey}"); + } + + return new LeaseContext(inMemoryLeaseKey, () => mutex.Release()); + } + private SemaphoreSlim ResolveMutex(string leaseKey, LeaseManagerOptions options = null) + { + SemaphoreSlim mutex; + lock (_locks) { if (!_locks.ContainsKey(leaseKey)) @@ -25,12 +52,13 @@ public ILeaseContext Acquire(string leaseKey) } mutex = _locks[leaseKey]; } - - if (!mutex.Wait(_options.Timeout)) - { - throw new AbandonedMutexException($"Abandoned attempt to acquire lock for lease key {leaseKey}"); - } - - return new LeaseContext(leaseKey, () => mutex.Release()); + + return mutex; } + + private string BuildInMemoryKey(string leaseKey, LeaseManagerOptions options = null) => + LeaseKeys.InMemoryKeyFactory.Create(options?.Realm ?? _defaultOptions.Realm, leaseKey); + + private TimeSpan ResolveTimeout(LeaseManagerOptions options = null) => + options?.Timeout ?? _defaultOptions.Timeout; } diff --git a/src/api/Nhs.Appointments.Core/Concurrency/LeaseKeys.cs b/src/api/Nhs.Appointments.Core/Concurrency/LeaseKeys.cs index fbe80a65b..adc0c6171 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/LeaseKeys.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/LeaseKeys.cs @@ -23,4 +23,21 @@ public static string Create(string siteId, DateOnly date) return $"{siteId}_{date:yyyyMMdd}"; } } + + public static class InMemoryKeyFactory + { + /// + /// This factory method creates a In Memory Key that logically separates leases and values. + /// + /// The logical separator + /// The lease key requested + /// A key for in memory leases + public static string Create(string realm, string leaseKey) + { + ArgumentException.ThrowIfNullOrEmpty(realm, nameof(realm)); + ArgumentException.ThrowIfNullOrEmpty(leaseKey, nameof(leaseKey)); + + return $"{realm}_{leaseKey}"; + } + } } diff --git a/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerFactory.cs b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerFactory.cs new file mode 100644 index 000000000..39bda83f8 --- /dev/null +++ b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerFactory.cs @@ -0,0 +1,32 @@ +namespace Nhs.Appointments.Core.Concurrency; + +public class LeaseManagerFactory : ILeaseManagerFactory +{ + private readonly ILeaseManager[] _leaseManagers; + + public LeaseManagerFactory(IEnumerable leaseManagers) + { + var manager = leaseManagers.ToArray(); + if (manager.Length == 0) + { + throw new ArgumentException("No lease managers have been registered"); + } + + _leaseManagers = manager; + } + + public ILeaseManager Create(LeaseManagerMode? mode = null) + { + if (mode is null) + { + return ResolveDefault(); + } + + return _leaseManagers.SingleOrDefault(x => x.Mode.Equals(mode)) ?? throw new ArgumentException($"No lease manager found for mode: {mode}"); + } + + private ILeaseManager ResolveDefault() => + _leaseManagers.SingleOrDefault(x => x.Mode.Equals(LeaseManagerMode.DistributedAzureBlob)) + ?? _leaseManagers.SingleOrDefault(x => x.Mode.Equals(LeaseManagerMode.InMemory)) + ?? throw new ArgumentException("No default lease manager found"); +} diff --git a/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerMode.cs b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerMode.cs new file mode 100644 index 000000000..57f1c6877 --- /dev/null +++ b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerMode.cs @@ -0,0 +1,7 @@ +namespace Nhs.Appointments.Core.Concurrency; + +public enum LeaseManagerMode +{ + InMemory, + DistributedAzureBlob +} diff --git a/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerOptions.cs b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerOptions.cs index f8930ab90..3ec8d3724 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerOptions.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/LeaseManagerOptions.cs @@ -3,5 +3,5 @@ namespace Nhs.Appointments.Core.Concurrency; public class LeaseManagerOptions { public TimeSpan Timeout { get; set; } - public string ContainerName { get; set; } + public string Realm { get; set; } } diff --git a/src/api/Nhs.Appointments.Core/Concurrency/ServiceRegistration.cs b/src/api/Nhs.Appointments.Core/Concurrency/ServiceRegistration.cs index 65585814f..bea841d5a 100644 --- a/src/api/Nhs.Appointments.Core/Concurrency/ServiceRegistration.cs +++ b/src/api/Nhs.Appointments.Core/Concurrency/ServiceRegistration.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.Azure; +using Microsoft.Extensions.Configuration; using Nhs.Appointments.Core.Blob; using Nhs.Appointments.Core.Concurrency; @@ -6,26 +7,37 @@ namespace Microsoft.Extensions.DependencyInjection; public static class ServiceRegistration { - public static IServiceCollection AddInMemoryLeasing(this IServiceCollection services) + public static IServiceCollection AddConcurrency(this IServiceCollection services, IConfiguration configuration) { - services.Configure(opts => opts.Timeout = TimeSpan.FromSeconds(15)); - return services.AddSingleton(); + var leaseManagerConnection = configuration.GetValue("LEASE_MANAGER_CONNECTION"); + + services + .AddTransient() + .Configure(opts => + { + opts.Timeout = + TimeSpan.FromSeconds(configuration.GetValue("LEASE_MANAGER_DEFAULT_TIME_OUT_SECONDS", 15)); + opts.Realm = configuration.GetValue("LEASE_MANAGER_DEFAULT_REALM", "leases"); + }) + .AddSingleton(); + + if (!string.IsNullOrEmpty(leaseManagerConnection)) + { + services.AddAzureBlobStoreLeasing(leaseManagerConnection); + } + + return services; } - public static IServiceCollection AddAzureBlobStoreLeasing(this IServiceCollection services, string connectionString, string containerName) + private static IServiceCollection AddAzureBlobStoreLeasing(this IServiceCollection services, string connectionString) { - services.Configure(opts => { - opts.Timeout = TimeSpan.FromSeconds(30); - opts.ContainerName = containerName; - }); - services.AddAzureClients(x => { x.AddBlobServiceClient(connectionString); }); return services - .AddSingleton() - .AddSingleton(); + .AddTransient() + .AddTransient(); } } diff --git a/tests/Nhs.Appointments.Core.UnitTests/BookingWriteServiceTests.cs b/tests/Nhs.Appointments.Core.UnitTests/BookingWriteServiceTests.cs index c8fed3644..b899afe81 100644 --- a/tests/Nhs.Appointments.Core.UnitTests/BookingWriteServiceTests.cs +++ b/tests/Nhs.Appointments.Core.UnitTests/BookingWriteServiceTests.cs @@ -19,16 +19,18 @@ public class BookingWriteServiceTests private readonly Mock _bookingsDocumentStore = new(); private readonly Mock _messageBus = new(); private readonly Mock _referenceNumberProvider = new(); + private readonly Mock _leaseManagerFactory = new(); private readonly Mock _leaseManager = new(); private BookingWriteService _sut; public BookingWriteServiceTests() { + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); _sut = new BookingWriteService( _bookingsDocumentStore.Object, _bookingQueryService.Object, _referenceNumberProvider.Object, - _leaseManager.Object, + _leaseManagerFactory.Object, _bookingAvailabilityStateService.Object, new EventFactory(), _messageBus.Object, @@ -70,18 +72,19 @@ public async Task MakeBooking_AcquiresLockForTheSiteIdAndLocalDayOfTheBooking_Wh var availabilityQueryService = new AvailabilityQueryService(_availabilityStore.Object, _availabilityCreatedEventStore.Object); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay))).Returns(new FakeLeaseContext()); - + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); + var bookingService = new BookingWriteService(_bookingsDocumentStore.Object, bookingQueryService, _referenceNumberProvider.Object, - _leaseManager.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), + _leaseManagerFactory.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), new EventFactory(), _messageBus.Object, TimeProvider.System); // Act. await bookingService.MakeBooking(booking); // Assert. - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay)), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay), It.IsAny()), Times.Once()); _leaseManager.VerifyNoOtherCalls(); } @@ -126,12 +129,13 @@ public async Task MakeBookingForTwoDifferentDaysForTheSameSite_AcquiresLockWithD var availabilityQueryService = new AvailabilityQueryService(_availabilityStore.Object, _availabilityCreatedEventStore.Object); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom1))).Returns(new FakeLeaseContext()); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom2))).Returns(new FakeLeaseContext()); - + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom1), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom2), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); + var bookingService = new BookingWriteService(_bookingsDocumentStore.Object, bookingQueryService, _referenceNumberProvider.Object, - _leaseManager.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), + _leaseManagerFactory.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), new EventFactory(), _messageBus.Object, TimeProvider.System); // Act. @@ -139,8 +143,8 @@ public async Task MakeBookingForTwoDifferentDaysForTheSameSite_AcquiresLockWithD await bookingService.MakeBooking(booking2); // Assert. - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom1)), Times.Once()); - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom2)), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom1), It.IsAny()), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom2), It.IsAny()), Times.Once()); _leaseManager.VerifyNoOtherCalls(); } @@ -181,12 +185,13 @@ public async Task MakeBookingForTheSameDayForTwoDifferentSites_AcquiresLockWithD var availabilityQueryService = new AvailabilityQueryService(_availabilityStore.Object, _availabilityCreatedEventStore.Object); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId1, expectedFrom))).Returns(new FakeLeaseContext()); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId2, expectedFrom))).Returns(new FakeLeaseContext()); - + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId1, expectedFrom), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId2, expectedFrom), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); + var bookingService = new BookingWriteService(_bookingsDocumentStore.Object, bookingQueryService, _referenceNumberProvider.Object, - _leaseManager.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), + _leaseManagerFactory.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), new EventFactory(), _messageBus.Object, TimeProvider.System); // Act. @@ -194,8 +199,8 @@ public async Task MakeBookingForTheSameDayForTwoDifferentSites_AcquiresLockWithD await bookingService.MakeBooking(booking2); // Assert. - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId1, expectedFrom)), Times.Once()); - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId2, expectedFrom)), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId1, expectedFrom), It.IsAny()), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId2, expectedFrom), It.IsAny()), Times.Once()); _leaseManager.VerifyNoOtherCalls(); } @@ -235,11 +240,12 @@ public async Task MakeBookingTwiceForTheSameDayForTheSameSite_AcquiresLockWithTh var availabilityQueryService = new AvailabilityQueryService(_availabilityStore.Object, _availabilityCreatedEventStore.Object); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom))).Returns(new FakeLeaseContext()); - + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); + var bookingService = new BookingWriteService(_bookingsDocumentStore.Object, bookingQueryService, _referenceNumberProvider.Object, - _leaseManager.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), + _leaseManagerFactory.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), new EventFactory(), _messageBus.Object, TimeProvider.System); // Act. @@ -247,7 +253,7 @@ public async Task MakeBookingTwiceForTheSameDayForTheSameSite_AcquiresLockWithTh await bookingService.MakeBooking(booking2); // Assert. - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom)), Times.Exactly(2)); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedFrom), It.IsAny()), Times.Exactly(2)); _leaseManager.VerifyNoOtherCalls(); } @@ -294,18 +300,19 @@ public async Task MakeBookingOnAndAroundDayClocksChange_AcquiresLockForTheCorrec var availabilityQueryService = new AvailabilityQueryService(_availabilityStore.Object, _availabilityCreatedEventStore.Object); - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay))).Returns(new FakeLeaseContext()); - + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay), It.IsAny())).Returns(new FakeLeaseContext()); + _leaseManagerFactory.Setup(x => x.Create(null)).Returns(_leaseManager.Object); + var bookingService = new BookingWriteService(_bookingsDocumentStore.Object, bookingQueryService, _referenceNumberProvider.Object, - _leaseManager.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), + _leaseManagerFactory.Object, new BookingAvailabilityStateService(availabilityQueryService, bookingQueryService), new EventFactory(), _messageBus.Object, TimeProvider.System); // Act. await bookingService.MakeBooking(booking); // Assert. - _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay)), Times.Once()); + _leaseManager.Verify(slm => slm.Acquire(LeaseKeys.SiteKeyFactory.Create(expectedSiteId, expectedDay), It.IsAny()), Times.Once()); _leaseManager.VerifyNoOtherCalls(); } @@ -331,7 +338,7 @@ public async Task MakeBooking_RaisesNotificationEvent_WhenBooking() Status = AppointmentStatus.Booked }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); MockAvailability(availability); @@ -371,7 +378,7 @@ public async Task MakeBooking_DoesNotRaiseNotificationEvent_WhenProvisional() Status = AppointmentStatus.Provisional }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); MockAvailability(availability); @@ -410,7 +417,7 @@ public async Task RescheduleBooking_IsSuccessful() Status = AppointmentStatus.Provisional }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); _bookingAvailabilityStateService.Setup(x => x.GetAvailableSlots(MockSite, new DateTime(2077, 1, 1, 10, 0, 0, 0), @@ -499,7 +506,7 @@ public async Task RescheduleBooking_RaisesNotificationEvent() Status = AppointmentStatus.Provisional }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); _bookingAvailabilityStateService.Setup(x => x.GetAvailableSlots(MockSite, new DateTime(2077, 1, 1, 10, 0, 0, 0), @@ -597,7 +604,7 @@ public async Task MakeBooking_ReturnsSuccess_WhenSlotIsAvailable() Status = AppointmentStatus.Booked }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); MockAvailability(availability); @@ -630,7 +637,7 @@ public async Task MakeBooking_FlagsBookingForReminder_WhenBooking() Status = AppointmentStatus.Booked }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); MockAvailability(availability); @@ -655,7 +662,7 @@ public async Task MakeBooking_ReturnsNonSuccess_WhenSlotIsNotAvailable() Status = AppointmentStatus.Booked }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(MockSite, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); _bookingAvailabilityStateService.Setup(x => x.GetAvailableSlots(MockSite, new DateTime(2077, 1, 1, 13, 0, 0, 0), @@ -784,7 +791,7 @@ public async Task CancelBooking_NotRunningAppointmentStatusesRecalculation() await _sut.CancelBooking(bookingRef, site, CancellationReason.CancelledByCitizen, runRecalculation: false); - _leaseManager.Verify(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(site, It.IsAny())), Times.Never); + _leaseManager.Verify(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(site, It.IsAny()), It.IsAny()), Times.Never); } [Fact] @@ -1204,7 +1211,7 @@ public async Task MakeBooking_CallsAllocationStateService_WhenBooking() var siteId = "TEST"; var booking = new Booking { Site = siteId, Service = "TSERV", From = new DateTime(2077, 1, 1) }; - _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(siteId, new DateOnly(2077, 1, 1)))).Returns(new FakeLeaseContext()); + _leaseManager.Setup(x => x.Acquire(LeaseKeys.SiteKeyFactory.Create(siteId, new DateOnly(2077, 1, 1)), It.IsAny())).Returns(new FakeLeaseContext()); _bookingAvailabilityStateService .Setup(x => x.GetAvailableSlots(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync(new List()); @@ -1538,11 +1545,15 @@ public FakeLeaseManager() public AutoResetEvent WaitHandle { get; } - public ILeaseContext Acquire(string leaseKey) + public LeaseManagerMode Mode => LeaseManagerMode.InMemory; + + public ILeaseContext Acquire(string leaseKey, LeaseManagerOptions options = null) { WaitHandle.WaitOne(); return new FakeLeaseContext(); } + + public Task AcquireAsync(string leaseKey, LeaseManagerOptions options = null) => throw new NotImplementedException(); } public class FakeLeaseContext : ILeaseContext diff --git a/tests/Nhs.Appointments.Core.UnitTests/Concurrency/AzureStorageLeaseManagerTests.cs b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/AzureStorageLeaseManagerTests.cs index b8bcd9189..ac39134da 100644 --- a/tests/Nhs.Appointments.Core.UnitTests/Concurrency/AzureStorageLeaseManagerTests.cs +++ b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/AzureStorageLeaseManagerTests.cs @@ -1,8 +1,5 @@ -using System.Diagnostics.CodeAnalysis; using Azure; -using Azure.Core; using Azure.Storage.Blobs; -using Azure.Storage.Blobs.Models; using Azure.Storage.Blobs.Specialized; using Microsoft.Extensions.Options; using Nhs.Appointments.Core.Blob; @@ -12,25 +9,18 @@ namespace Nhs.Appointments.Core.UnitTests.Concurrency; public class AzureStorageLeaseManagerTests { - [Theory] - [InlineData(0)] - [InlineData(-1)] - [InlineData(-10)] - public void CannotConstructWithInvalidAcquireTime(int acquireTimeInSeconds) + [Fact] + public void ModeIsCorrect() { - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0) }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "Test" }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); - var blobClient = new TestableBlobClient(); var abs = new Mock(); - Action action = () => new AzureStorageLeaseManager(options.Object, abs.Object, acquireTimeInSeconds); + var sut = new AzureStorageLeaseManager(options.Object, abs.Object); - // Assert. - action.Should() - .Throw() - .WithMessage("acquireTimeInSeconds * be greater than '0'*"); + sut.Mode.Should().Be(LeaseManagerMode.DistributedAzureBlob); } [Theory] @@ -42,11 +32,10 @@ public void CannotConstructWithInvalidDelayTime(int delayRetryTimeInMilliseconds var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0) }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); - - var blobClient = new TestableBlobClient(); + var abs = new Mock(); - Action action = () => new AzureStorageLeaseManager(options.Object, abs.Object, 10, delayRetryTimeInMilliseconds); + Action action = () => new AzureStorageLeaseManager(options.Object, abs.Object, delayRetryTimeInMilliseconds); // Assert. action.Should() @@ -60,13 +49,13 @@ public void SiteAndDateSupplied_Acquire_AcquiresLeaseContextWithExpectedKey() // Arrange. var siteId = Guid.NewGuid().ToString(); var date = DateOnly.FromDateTime(DateTime.UtcNow); - var expectedSiteKey = LeaseKeys.SiteKeyFactory.Create(siteId, date); var containerName = Guid.NewGuid().ToString(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), ContainerName = containerName }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); + var expectedSiteKey = LeaseKeys.SiteKeyFactory.Create(siteId, date); var blobClient = new TestableBlobClient(); var abs = new Mock(); @@ -82,6 +71,35 @@ public void SiteAndDateSupplied_Acquire_AcquiresLeaseContextWithExpectedKey() abs.Verify(); blobClient.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } + + [Fact] + public async Task SiteAndDateSupplied_AcquireAsync_AcquiresLeaseContextWithExpectedKey() + { + // Arrange. + var siteId = Guid.NewGuid().ToString(); + var date = DateOnly.FromDateTime(DateTime.UtcNow); + + var containerName = Guid.NewGuid().ToString(); + + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; + var options = new Mock>(); + options.Setup(o => o.Value).Returns(slmo); + var expectedSiteKey = LeaseKeys.SiteKeyFactory.Create(siteId, date); + + var blobClient = new TestableBlobClient(); + var abs = new Mock(); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey)).ReturnsAsync(blobClient); + + var sut = new AzureStorageLeaseManager(options.Object, abs.Object); + + // Act. + var slc = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId, date)); + + // Assert. + slc.LeaseKey.Should().Be(expectedSiteKey); + abs.Verify(); + blobClient.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + } [Fact] public void TwoLocksAttemptedForSameSiteAndDifferentDate_Acquire_AcquiresSeparateLeaseContextsWithExpectedKeys() @@ -90,14 +108,14 @@ public void TwoLocksAttemptedForSameSiteAndDifferentDate_Acquire_AcquiresSeparat var siteId = Guid.NewGuid().ToString(); var date1 = DateOnly.FromDateTime(DateTime.UtcNow); var date2 = date1.AddDays(1); - var expectedSiteKey1 = LeaseKeys.SiteKeyFactory.Create(siteId, date1); - var expectedSiteKey2 = LeaseKeys.SiteKeyFactory.Create(siteId, date2); var containerName = Guid.NewGuid().ToString(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), ContainerName = containerName }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); + var expectedSiteKey1 = LeaseKeys.SiteKeyFactory.Create(siteId, date1); + var expectedSiteKey2 = LeaseKeys.SiteKeyFactory.Create(siteId, date2); var blobClient1 = new TestableBlobClient(); var blobClient2 = new TestableBlobClient(); @@ -118,6 +136,42 @@ public void TwoLocksAttemptedForSameSiteAndDifferentDate_Acquire_AcquiresSeparat blobClient1.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); blobClient2.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } + + [Fact] + public async Task TwoLocksAttemptedForSameSiteAndDifferentDate_AcquireAsync_AcquiresSeparateLeaseContextsWithExpectedKeys() + { + // Arrange. + var siteId = Guid.NewGuid().ToString(); + var date1 = DateOnly.FromDateTime(DateTime.UtcNow); + var date2 = date1.AddDays(1); + + var containerName = Guid.NewGuid().ToString(); + + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; + var options = new Mock>(); + options.Setup(o => o.Value).Returns(slmo); + var expectedSiteKey1 = LeaseKeys.SiteKeyFactory.Create(siteId, date1); + var expectedSiteKey2 = LeaseKeys.SiteKeyFactory.Create(siteId, date2); + + var blobClient1 = new TestableBlobClient(); + var blobClient2 = new TestableBlobClient(); + var abs = new Mock(); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey1)).ReturnsAsync(blobClient1); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey2)).ReturnsAsync(blobClient2); + + var sut = new AzureStorageLeaseManager(options.Object, abs.Object); + + // Act. + var slc1 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId, date1)); + var slc2 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId, date2)); + + // Assert. + slc1.LeaseKey.Should().Be(expectedSiteKey1); + slc2.LeaseKey.Should().Be(expectedSiteKey2); + abs.Verify(); + blobClient1.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + blobClient2.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + } [Fact] public void TwoLocksAttemptedForDifferentSitesAndSameDate_Acquire_AcquiresSeparateLeaseContextsWithExpectedKeys() @@ -131,7 +185,7 @@ public void TwoLocksAttemptedForDifferentSitesAndSameDate_Acquire_AcquiresSepara var containerName = Guid.NewGuid().ToString(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), ContainerName = containerName }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); @@ -154,6 +208,42 @@ public void TwoLocksAttemptedForDifferentSitesAndSameDate_Acquire_AcquiresSepara blobClient1.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); blobClient2.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } + + [Fact] + public async Task TwoLocksAttemptedForDifferentSitesAndSameDate_AcquireAsync_AcquiresSeparateLeaseContextsWithExpectedKeys() + { + // Arrange. + var siteId1 = Guid.NewGuid().ToString(); + var siteId2 = Guid.NewGuid().ToString(); + var date = DateOnly.FromDateTime(DateTime.UtcNow); + var expectedSiteKey1 = LeaseKeys.SiteKeyFactory.Create(siteId1, date); + var expectedSiteKey2 = LeaseKeys.SiteKeyFactory.Create(siteId2, date); + + var containerName = Guid.NewGuid().ToString(); + + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; + var options = new Mock>(); + options.Setup(o => o.Value).Returns(slmo); + + var blobClient1 = new TestableBlobClient(); + var blobClient2 = new TestableBlobClient(); + var abs = new Mock(); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey1)).ReturnsAsync(blobClient1); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey2)).ReturnsAsync(blobClient2); + + var sut = new AzureStorageLeaseManager(options.Object, abs.Object); + + // Act. + var slc1 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId1, date)); + var slc2 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId2, date)); + + // Assert. + slc1.LeaseKey.Should().Be(expectedSiteKey1); + slc2.LeaseKey.Should().Be(expectedSiteKey2); + abs.Verify(); + blobClient1.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + blobClient2.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + } [Fact] public void TwoLocksAttemptedForSameSiteAndSameDate_Acquire_AttemptToAcquireSameLeaseContextsWithExpectedKey() @@ -165,7 +255,7 @@ public void TwoLocksAttemptedForSameSiteAndSameDate_Acquire_AttemptToAcquireSame var containerName = Guid.NewGuid().ToString(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), ContainerName = containerName }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; var options = new Mock>(); options.Setup(o => o.Value).Returns(slmo); @@ -185,6 +275,37 @@ public void TwoLocksAttemptedForSameSiteAndSameDate_Acquire_AttemptToAcquireSame abs.Verify(); blobClient.RetainedBlobLeaseClient.Verify(blc => blc.Acquire(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); } + + [Fact] + public async Task TwoLocksAttemptedForSameSiteAndSameDate_AcquireAsync_AttemptToAcquireSameLeaseContextsWithExpectedKey() + { + // Arrange. + var siteId = Guid.NewGuid().ToString(); + var date = DateOnly.FromDateTime(DateTime.UtcNow); + var expectedSiteKey = LeaseKeys.SiteKeyFactory.Create(siteId, date); + + var containerName = Guid.NewGuid().ToString(); + + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = containerName }; + var options = new Mock>(); + options.Setup(o => o.Value).Returns(slmo); + + var blobClient = new TestableBlobClient(); + var abs = new Mock(); + abs.Setup(a => a.GetBlobClientFromContainerAndBlobNameAsync(containerName, expectedSiteKey)).ReturnsAsync(blobClient); + + var sut = new AzureStorageLeaseManager(options.Object, abs.Object); + + // Act. + var slc1 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId, date)); + var slc2 = await sut.AcquireAsync(LeaseKeys.SiteKeyFactory.Create(siteId, date)); + + // Assert. + slc1.LeaseKey.Should().Be(expectedSiteKey); + slc2.LeaseKey.Should().Be(expectedSiteKey); + abs.Verify(); + blobClient.RetainedBlobLeaseClient.Verify(blc => blc.AcquireAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + } private class TestableBlobClient : BlobClient { @@ -207,5 +328,13 @@ public override Response Exists(CancellationToken cancellationToken = defa return response.Object; } + + public override Task> ExistsAsync(CancellationToken cancellationToken = default) + { + var response = new Mock>(); + response.Setup(r => r.Value).Returns(true); + + return Task.FromResult(response.Object); + } } } diff --git a/tests/Nhs.Appointments.Core.UnitTests/Concurrency/InMemoryLeaseManagerTests.cs b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/InMemoryLeaseManagerTests.cs index 770f85166..e4eee809c 100644 --- a/tests/Nhs.Appointments.Core.UnitTests/Concurrency/InMemoryLeaseManagerTests.cs +++ b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/InMemoryLeaseManagerTests.cs @@ -5,20 +5,55 @@ namespace Nhs.Appointments.Core.UnitTests.Concurrency; public class InMemoryLeaseManagerTests { + [Fact] + public void ModeIsCorrect() + { + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "Test" }; + var options = new Mock>(); + options.Setup(o => o.Value).Returns(slmo); + + var sut = new InMemoryLeaseManager(options.Object); + + sut.Mode.Should().Be(LeaseManagerMode.InMemory); + } + [Fact] public void LeaseKeySupplied_Acquire_ReturnsLeaseContext() { // Arrange. var options = new Mock>(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0) }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "test"}; options.Setup(o => o.Value).Returns(slmo); - var expectedLeaseKey = Guid.NewGuid().ToString(); + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; var sut = new InMemoryLeaseManager(options.Object); // Act. - var slc = sut.Acquire(expectedLeaseKey); + var slc = sut.Acquire(leaseKey); + + // Assert. + slc.Should().NotBeNull(); + slc.LeaseKey.Should() + .Be(expectedLeaseKey); + } + + [Fact] + public async Task LeaseKeySupplied_AcquireAsync_ReturnsLeaseContext() + { + // Arrange. + var options = new Mock>(); + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "test"}; + options.Setup(o => o.Value).Returns(slmo); + + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; + + var sut = new InMemoryLeaseManager(options.Object); + + // Act. + var slc = await sut.AcquireAsync(leaseKey); // Assert. slc.Should().NotBeNull(); @@ -31,16 +66,17 @@ public void SameLeaseKeyCallsConcurrent_Acquire_AttemptToAcquireTheSameLock() { // Arrange. var options = new Mock>(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1) }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1), Realm = "test" }; options.Setup(o => o.Value).Returns(slmo); - var expectedLeaseKey = Guid.NewGuid().ToString(); + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; var sut = new InMemoryLeaseManager(options.Object); // Act. - var slc1 = sut.Acquire(expectedLeaseKey); - Action action = () => sut.Acquire(expectedLeaseKey); + var slc1 = sut.Acquire(leaseKey); + Action action = () => sut.Acquire(leaseKey); // Assert. slc1.LeaseKey.Should() @@ -49,26 +85,51 @@ public void SameLeaseKeyCallsConcurrent_Acquire_AttemptToAcquireTheSameLock() .Throw() .WithMessage($"Abandoned attempt to acquire lock for lease key {expectedLeaseKey}"); } + + [Fact] + public async Task SameLeaseKeyCallsConcurrent_AcquireAsync_AttemptToAcquireTheSameLock() + { + // Arrange. + var options = new Mock>(); + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1), Realm = "test" }; + options.Setup(o => o.Value).Returns(slmo); + + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; + + var sut = new InMemoryLeaseManager(options.Object); + + // Act. + var slc1 = await sut.AcquireAsync(leaseKey); + Task action = sut.AcquireAsync(leaseKey); + + // Assert. + slc1.LeaseKey.Should() + .Be(expectedLeaseKey); + var exception = await Assert.ThrowsAsync(() => action); + exception.Message.Should().BeEquivalentTo($"Abandoned attempt to acquire lock for lease key {expectedLeaseKey}"); + } [Fact] public void SameLeaseKeyCallsButNotConcurrent_Acquire_ReturnsMatchingLeaseContexts() { // Arrange. var options = new Mock>(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1) }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1), Realm = "test" }; options.Setup(o => o.Value).Returns(slmo); - var expectedLeaseKey = Guid.NewGuid().ToString(); + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; var sut = new InMemoryLeaseManager(options.Object); // Act. var originalLeaseKey = ""; - using (var slc1 = sut.Acquire(expectedLeaseKey)) + using (var slc1 = sut.Acquire(leaseKey)) { originalLeaseKey = slc1.LeaseKey; } - var slc2 = sut.Acquire(expectedLeaseKey); + var slc2 = sut.Acquire(leaseKey); // Assert. originalLeaseKey.Should() @@ -76,23 +137,83 @@ public void SameLeaseKeyCallsButNotConcurrent_Acquire_ReturnsMatchingLeaseContex slc2.LeaseKey.Should() .Be(expectedLeaseKey); } + + [Fact] + public async Task SameLeaseKeyCallsButNotConcurrent_AcquireAsync_ReturnsMatchingLeaseContexts() + { + // Arrange. + var options = new Mock>(); + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(0, 0, 0, 0, 0, 1), Realm = "test" }; + options.Setup(o => o.Value).Returns(slmo); + + var leaseKey = Guid.NewGuid().ToString(); + var expectedLeaseKey = $"{slmo.Realm}_{leaseKey}"; + + var sut = new InMemoryLeaseManager(options.Object); + + // Act. + var originalLeaseKey = ""; + using (var slc1 = await sut.AcquireAsync(leaseKey)) + { + originalLeaseKey = slc1.LeaseKey; + } + var slc2 = await sut.AcquireAsync(leaseKey); + // Assert. + originalLeaseKey.Should() + .Be(expectedLeaseKey); + slc2.LeaseKey.Should() + .Be(expectedLeaseKey); + } + [Fact] public void DifferentLeaseKeyCalls_Acquire_ReturnsSeparateLeaseContexts() { // Arrange. var options = new Mock>(); - var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0) }; + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "test"}; + options.Setup(o => o.Value).Returns(slmo); + + var leaseKey1 = Guid.NewGuid().ToString(); + var leaseKey2 = Guid.NewGuid().ToString(); + + var expectedLeaseKey1 = $"{slmo.Realm}_{leaseKey1}"; + var expectedLeaseKey2 = $"{slmo.Realm}_{leaseKey2}"; + + var sut = new InMemoryLeaseManager(options.Object); + + // Act. + var slc1 = sut.Acquire(leaseKey1); + var slc2 = sut.Acquire(leaseKey2); + + // Assert. + slc1.LeaseKey.Should() + .Be(expectedLeaseKey1); + slc2.LeaseKey.Should() + .Be(expectedLeaseKey2); + expectedLeaseKey1.Should() + .NotBe(expectedLeaseKey2); + } + + [Fact] + public async Task DifferentLeaseKeyCalls_AcquireAsync_ReturnsSeparateLeaseContexts() + { + // Arrange. + var options = new Mock>(); + var slmo = new LeaseManagerOptions { Timeout = new TimeSpan(1, 0, 0), Realm = "test"}; options.Setup(o => o.Value).Returns(slmo); - var expectedLeaseKey1 = Guid.NewGuid().ToString(); - var expectedLeaseKey2 = Guid.NewGuid().ToString(); + var leaseKey1 = Guid.NewGuid().ToString(); + var leaseKey2 = Guid.NewGuid().ToString(); + + var expectedLeaseKey1 = $"{slmo.Realm}_{leaseKey1}"; + var expectedLeaseKey2 = $"{slmo.Realm}_{leaseKey2}"; var sut = new InMemoryLeaseManager(options.Object); // Act. - var slc1 = sut.Acquire(expectedLeaseKey1); - var slc2 = sut.Acquire(expectedLeaseKey2); + var slc1 = await sut.AcquireAsync(leaseKey1); + var slc2 = await sut.AcquireAsync(leaseKey2); // Assert. slc1.LeaseKey.Should() diff --git a/tests/Nhs.Appointments.Core.UnitTests/Concurrency/LeaseManagerFactoryTests.cs b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/LeaseManagerFactoryTests.cs new file mode 100644 index 000000000..3185bd18f --- /dev/null +++ b/tests/Nhs.Appointments.Core.UnitTests/Concurrency/LeaseManagerFactoryTests.cs @@ -0,0 +1,82 @@ +using Nhs.Appointments.Core.Concurrency; + +namespace Nhs.Appointments.Core.UnitTests.Concurrency; + +public class LeaseManagerFactoryTests +{ + private readonly Mock _leaseManagerInMemory = new(); + private readonly Mock _leaseManagerDistributed = new(); + + public LeaseManagerFactoryTests() + { + _leaseManagerInMemory.Setup(x => x.Mode).Returns(LeaseManagerMode.InMemory); + _leaseManagerDistributed.Setup(x => x.Mode).Returns(LeaseManagerMode.DistributedAzureBlob); + } + + [Fact] + public void Default_Create_InMemoryLeaseManager_WhenNoDistributed() + { + var sut = new LeaseManagerFactory( + [ + _leaseManagerInMemory.Object + ]); + + sut.Create().Mode.Should().Be(LeaseManagerMode.InMemory); + } + + [Fact] + public void Default_Create_Distributed_WhenAllRegistered() + { + var sut = new LeaseManagerFactory( + [ + _leaseManagerInMemory.Object, + _leaseManagerDistributed.Object, + ]); + + sut.Create().Mode.Should().Be(LeaseManagerMode.DistributedAzureBlob); + } + + [Fact] + public void Default_Create_Distributed_WhenJustDistributed() + { + var sut = new LeaseManagerFactory( + [ + _leaseManagerDistributed.Object, + ]); + + sut.Create().Mode.Should().Be(LeaseManagerMode.DistributedAzureBlob); + } + + [Fact] + public void Default_Create_Throws_WhenEmpty() + { + var exception = Assert.Throws(() => new LeaseManagerFactory([])); + exception.Message.Should().Be("No lease managers have been registered"); + } + + [Theory] + [InlineData(LeaseManagerMode.InMemory)] + [InlineData(LeaseManagerMode.DistributedAzureBlob)] + public void Mode_Create_BringsBackRequest(LeaseManagerMode mode) + { + var sut = new LeaseManagerFactory( + [ + _leaseManagerDistributed.Object, + _leaseManagerInMemory.Object, + ]); + + sut.Create(mode).Mode.Should().Be(mode); + } + + [Fact] + public void Mode_Create_Throws_WhenNotFound() + { + var sut = new LeaseManagerFactory( + [ + _leaseManagerInMemory.Object, + ]); + + var exception = Assert.Throws(() => sut.Create(LeaseManagerMode.DistributedAzureBlob)); + exception.Message.Should().Be($"No lease manager found for mode: {LeaseManagerMode.DistributedAzureBlob}"); + } +}