From 8a7b5bd83200d5c6c3b001e4c2f1b12bf5d9b8b5 Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 16:54:20 +0100 Subject: [PATCH 01/10] Removing TODO - has been done --- .../NbssAppointmentEvents/Validation/IRecordValidator.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ServiceLayer.Mesh/FileTypes/NbssAppointmentEvents/Validation/IRecordValidator.cs b/src/ServiceLayer.Mesh/FileTypes/NbssAppointmentEvents/Validation/IRecordValidator.cs index dba4575..5bbad7e 100644 --- a/src/ServiceLayer.Mesh/FileTypes/NbssAppointmentEvents/Validation/IRecordValidator.cs +++ b/src/ServiceLayer.Mesh/FileTypes/NbssAppointmentEvents/Validation/IRecordValidator.cs @@ -2,7 +2,6 @@ namespace ServiceLayer.Mesh.FileTypes.NbssAppointmentEvents.Validation; -// TODO - create a whole bunch of implementations of this to perform the validation against NBSS Appointment events records public interface IRecordValidator { IEnumerable Validate(FileDataRecord fileDataRecord); From c1130426c71e91cc6e8ea8a18a1fbe2e88be613d Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:11:29 +0100 Subject: [PATCH 02/10] Avoid string interpolation in logging message templates --- src/ServiceLayer.Mesh/Functions/FileRetryFunction.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ServiceLayer.Mesh/Functions/FileRetryFunction.cs b/src/ServiceLayer.Mesh/Functions/FileRetryFunction.cs index 36ca269..afd893c 100644 --- a/src/ServiceLayer.Mesh/Functions/FileRetryFunction.cs +++ b/src/ServiceLayer.Mesh/Functions/FileRetryFunction.cs @@ -34,14 +34,14 @@ private async Task RetryStaleExtractions(DateTime staleDateTimeUtc) && f.LastUpdatedUtc <= staleDateTimeUtc) .ToListAsync(); - logger.LogInformation($"FileRetryFunction: {staleFiles.Count} stale files found for extraction retry"); + logger.LogInformation("FileRetryFunction: {StaleFilesCount} stale files found for extraction retry", staleFiles.Count); foreach (var file in staleFiles) { await fileExtractQueueClient.EnqueueFileExtractAsync(file); file.LastUpdatedUtc = DateTime.UtcNow; await serviceLayerDbContext.SaveChangesAsync(); - logger.LogInformation($"FileRetryFunction: File {file.FileId} enqueued to Extract queue"); + logger.LogInformation("FileRetryFunction: File {FileFileId} enqueued to Extract queue", file.FileId); } } @@ -53,14 +53,14 @@ private async Task RetryStaleTransformations(DateTime staleDateTimeUtc) && f.LastUpdatedUtc <= staleDateTimeUtc) .ToListAsync(); - logger.LogInformation($"FileRetryFunction: {staleFiles.Count} stale files found for transforming retry"); + logger.LogInformation("FileRetryFunction: {StaleFilesCount} stale files found for transforming retry", staleFiles.Count); foreach (var file in staleFiles) { await fileTransformQueueClient.EnqueueFileTransformAsync(file); file.LastUpdatedUtc = DateTime.UtcNow; await serviceLayerDbContext.SaveChangesAsync(); - logger.LogInformation($"FileRetryFunction: File {file.FileId} enqueued to Transform queue"); + logger.LogInformation("FileRetryFunction: File {FileFileId} enqueued to Transform queue", file.FileId); } } } From d603228a3ea2bc849b49dcc50fe0c57897c33f9b Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:12:16 +0100 Subject: [PATCH 03/10] Removing commented-out code --- src/ServiceLayer.Mesh/Program.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/ServiceLayer.Mesh/Program.cs b/src/ServiceLayer.Mesh/Program.cs index e252f52..85227f4 100644 --- a/src/ServiceLayer.Mesh/Program.cs +++ b/src/ServiceLayer.Mesh/Program.cs @@ -26,12 +26,6 @@ services.AddNbssAppointmentEventServices(); }); - -// Application Insights isn't enabled by default. See https://aka.ms/AAt8mw4. -// builder.Services -// .AddApplicationInsightsTelemetryWorkerService() -// .ConfigureFunctionsApplicationInsights(); - var app = host.Build(); await app.RunAsync(); return; From 34d3d51185f6d1386f2b375efe5e83d8bfb20d44 Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:13:01 +0100 Subject: [PATCH 04/10] Removing TODO, updating comment --- src/ServiceLayer.Mesh/Storage/IMeshFilesBlobStore.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ServiceLayer.Mesh/Storage/IMeshFilesBlobStore.cs b/src/ServiceLayer.Mesh/Storage/IMeshFilesBlobStore.cs index d0545a2..f7637d6 100644 --- a/src/ServiceLayer.Mesh/Storage/IMeshFilesBlobStore.cs +++ b/src/ServiceLayer.Mesh/Storage/IMeshFilesBlobStore.cs @@ -4,9 +4,8 @@ namespace ServiceLayer.Mesh.Storage; public interface IMeshFilesBlobStore { - // TODO - return a Stream or a byte array? public Task DownloadAsync(MeshFile file); - // Mesh client gives us a byte array, hence this not taking a stream. + // Mesh client gives us a byte array, hence this is not taking a stream. public Task UploadAsync(MeshFile file, byte[] data); } From 12c23c52e66f40fad3c42c0eeb47c678b5bbd8c5 Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:13:25 +0100 Subject: [PATCH 05/10] Making local field readonly --- src/ServiceLayer.Mesh/Storage/MeshFilesBlobStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ServiceLayer.Mesh/Storage/MeshFilesBlobStore.cs b/src/ServiceLayer.Mesh/Storage/MeshFilesBlobStore.cs index ee707a2..aa13f5e 100644 --- a/src/ServiceLayer.Mesh/Storage/MeshFilesBlobStore.cs +++ b/src/ServiceLayer.Mesh/Storage/MeshFilesBlobStore.cs @@ -5,7 +5,7 @@ namespace ServiceLayer.Mesh.Storage; public class MeshFilesBlobStore : IMeshFilesBlobStore { - private BlobContainerClient _blobContainerClient; + private readonly BlobContainerClient _blobContainerClient; public MeshFilesBlobStore(BlobContainerClient blobContainerClient) { From ef42d6147c7bc96a3ef3deb38698b3d900c5ae3a Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:14:28 +0100 Subject: [PATCH 06/10] Using GeneratedRegexAttribute to generate regex implementation at compile-time --- .../Validation/RegexValidatorTests.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/ServiceLayer.Mesh.Tests/FileTypes/NbssAppointmentEvents/Validation/RegexValidatorTests.cs b/tests/ServiceLayer.Mesh.Tests/FileTypes/NbssAppointmentEvents/Validation/RegexValidatorTests.cs index d7c81b8..46429bc 100644 --- a/tests/ServiceLayer.Mesh.Tests/FileTypes/NbssAppointmentEvents/Validation/RegexValidatorTests.cs +++ b/tests/ServiceLayer.Mesh.Tests/FileTypes/NbssAppointmentEvents/Validation/RegexValidatorTests.cs @@ -4,12 +4,12 @@ namespace ServiceLayer.Mesh.Tests.FileTypes.NbssAppointmentEvents.Validation; -public class RegexValidatorTests +public partial class RegexValidatorTests { private const string FieldName = "TestField"; private const string MissingCode = "ERR001"; private const string InvalidFormatCode = "ERR002"; - private readonly Regex _pattern = new(@"^[A-Z]{2}\d{2}$", RegexOptions.Compiled); + private readonly Regex _pattern = TestRegex(); [Fact] public void Validate_NullValue_ShouldReturnMissingError() @@ -68,4 +68,7 @@ public void Validate_ValueMatchingPattern_ShouldReturnNoErrors(string validValue Assert.Empty(errors); } + + [GeneratedRegex(@"^[A-Z]{2}\d{2}$", RegexOptions.Compiled)] + private static partial Regex TestRegex(); } From 502e74dddc2aaabaea366a2afc9c9902aa26f9cc Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:21:30 +0100 Subject: [PATCH 07/10] Avoid passing null literal to non-nullable reference type --- .../Functions/FileDiscoveryFunctionTests.cs | 11 ++++++----- .../Functions/FileRetryFunctionTests.cs | 13 +++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/ServiceLayer.Mesh.Tests/Functions/FileDiscoveryFunctionTests.cs b/tests/ServiceLayer.Mesh.Tests/Functions/FileDiscoveryFunctionTests.cs index a80995a..61aa477 100644 --- a/tests/ServiceLayer.Mesh.Tests/Functions/FileDiscoveryFunctionTests.cs +++ b/tests/ServiceLayer.Mesh.Tests/Functions/FileDiscoveryFunctionTests.cs @@ -1,4 +1,5 @@ -using Moq; +using Microsoft.Azure.Functions.Worker; +using Moq; using NHS.MESH.Client.Contracts.Services; using NHS.MESH.Client.Models; using ServiceLayer.Data.Models; @@ -44,7 +45,7 @@ public async Task Run_AddsNewMessageToDbAndQueue() }); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert var meshFile = AssertFileUpdated(testMessageId, MeshFileStatus.Discovered); @@ -67,7 +68,7 @@ public async Task Run_DoesNotAddDuplicateMessageOrQueueIt() }); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert var count = DbContext.MeshFiles.Count(f => f.FileId == existingFile.FileId); @@ -87,7 +88,7 @@ public async Task Run_NoMessagesInInbox_DoesNothing() }); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert Assert.Empty(DbContext.MeshFiles); @@ -107,7 +108,7 @@ public async Task Run_MultipleMessagesInInbox_AllAreProcessed() }); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert foreach (var id in messageIds) diff --git a/tests/ServiceLayer.Mesh.Tests/Functions/FileRetryFunctionTests.cs b/tests/ServiceLayer.Mesh.Tests/Functions/FileRetryFunctionTests.cs index 862d423..d989a37 100644 --- a/tests/ServiceLayer.Mesh.Tests/Functions/FileRetryFunctionTests.cs +++ b/tests/ServiceLayer.Mesh.Tests/Functions/FileRetryFunctionTests.cs @@ -1,3 +1,4 @@ +using Microsoft.Azure.Functions.Worker; using Moq; using ServiceLayer.Data.Models; using ServiceLayer.Mesh.Functions; @@ -38,7 +39,7 @@ public async Task Run_EnqueuesDiscoveredOrExtractingFilesOlderThan12Hours(MeshFi var file = SaveMeshFile(testStatus, 13); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileExtractQueueClientMock.Verify(q => q.EnqueueFileExtractAsync(It.Is(f => f.FileId == file.FileId)), Times.Once); @@ -56,7 +57,7 @@ public async Task Run_EnqueuesExtractedOrTransformingFilesOlderThan12Hours(MeshF var file = SaveMeshFile(testStatus, 13); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileTransformQueueClientMock.Verify(q => q.EnqueueFileTransformAsync(It.Is(f => f.FileId == file.FileId)), Times.Once); @@ -76,7 +77,7 @@ public async Task Run_SkipsFreshFiles(MeshFileStatus testStatus) var file = SaveMeshFile(testStatus); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileExtractQueueClientMock.Verify(q => q.EnqueueFileExtractAsync(It.IsAny()), Times.Never); @@ -95,7 +96,7 @@ public async Task Run_IgnoresFilesInOtherStatuses(MeshFileStatus ignoredStatus) SaveMeshFile(ignoredStatus, 20); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileTransformQueueClientMock.Verify(q => q.EnqueueFileTransformAsync(It.IsAny()), Times.Never); @@ -106,7 +107,7 @@ public async Task Run_IgnoresFilesInOtherStatuses(MeshFileStatus ignoredStatus) public async Task Run_IfNoFilesFoundDoNothing() { // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileExtractQueueClientMock.Verify(q => q.EnqueueFileExtractAsync(It.IsAny()), Times.Never); @@ -122,7 +123,7 @@ public async Task Run_ProcessesMultipleEligibleFiles() var file3 = SaveMeshFile(MeshFileStatus.Transforming, 13); // Act - await _function.Run(null); + await _function.Run(new TimerInfo()); // Assert _fileExtractQueueClientMock.Verify(q => q.EnqueueFileExtractAsync(It.Is(f => f.FileId == file1.FileId)), Times.Once); From 1fa909181c1bda23f5b223c69e009ae5dc07e5d8 Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:24:01 +0100 Subject: [PATCH 08/10] Remove unhelpful catch blocks --- .../Messaging/QueueClientBase.cs | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs b/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs index b443497..360304b 100644 --- a/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs +++ b/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs @@ -37,30 +37,13 @@ private QueueClient CreatePoisonClient() protected async Task SendJsonMessageAsync(T message) { - try - { - var json = JsonSerializer.Serialize(message, QueueJsonOptions); - await Client.SendMessageAsync(json); - } - catch (Exception e) - { - // TODO - consider including file ID or correlation ID in error logs - logger.LogError(e, "Error sending message to queue {QueueName}", QueueName); - throw; - } + var json = JsonSerializer.Serialize(message, QueueJsonOptions); + await Client.SendMessageAsync(json); } protected async Task SendToPoisonQueueAsync(T message) { - try - { - var json = JsonSerializer.Serialize(message, QueueJsonOptions); - await PoisonClient.SendMessageAsync(json); - } - catch (Exception e) - { - logger.LogError(e, "Error sending message to poison queue {PoisonQueueName}", $"{QueueName}-poison"); - throw; - } + var json = JsonSerializer.Serialize(message, QueueJsonOptions); + await PoisonClient.SendMessageAsync(json); } } From 6c0a9bcb5b39c0123fc6d44c211543e28ad1ee12 Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:40:33 +0100 Subject: [PATCH 09/10] Remove unused logger --- src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs b/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs index 360304b..84956ce 100644 --- a/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs +++ b/src/ServiceLayer.Mesh/Messaging/QueueClientBase.cs @@ -1,10 +1,9 @@ using System.Text.Json; using Azure.Storage.Queues; -using Microsoft.Extensions.Logging; namespace ServiceLayer.Mesh.Messaging; -public abstract class QueueClientBase(ILogger logger, QueueServiceClient queueServiceClient) +public abstract class QueueClientBase(QueueServiceClient queueServiceClient) { private QueueClient? _queueClient; private QueueClient? _poisonQueueClient; From 8c06d4d5f53d6f312d055335d84805224e31e7da Mon Sep 17 00:00:00 2001 From: Ian Nelson Date: Thu, 29 May 2025 19:43:29 +0100 Subject: [PATCH 10/10] Remove unused logger --- src/ServiceLayer.Mesh/Messaging/FileExtractQueueClient.cs | 4 +--- src/ServiceLayer.Mesh/Messaging/FileTransformQueueClient.cs | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ServiceLayer.Mesh/Messaging/FileExtractQueueClient.cs b/src/ServiceLayer.Mesh/Messaging/FileExtractQueueClient.cs index 8cb8a74..76031c9 100644 --- a/src/ServiceLayer.Mesh/Messaging/FileExtractQueueClient.cs +++ b/src/ServiceLayer.Mesh/Messaging/FileExtractQueueClient.cs @@ -1,15 +1,13 @@ using Azure.Storage.Queues; -using Microsoft.Extensions.Logging; using ServiceLayer.Data.Models; using ServiceLayer.Mesh.Configuration; namespace ServiceLayer.Mesh.Messaging; public class FileExtractQueueClient( - ILogger logger, IFileExtractQueueClientConfiguration configuration, QueueServiceClient queueServiceClient) - : QueueClientBase(logger, queueServiceClient), IFileExtractQueueClient + : QueueClientBase(queueServiceClient), IFileExtractQueueClient { public async Task EnqueueFileExtractAsync(MeshFile file) { diff --git a/src/ServiceLayer.Mesh/Messaging/FileTransformQueueClient.cs b/src/ServiceLayer.Mesh/Messaging/FileTransformQueueClient.cs index 64e2004..abf739e 100644 --- a/src/ServiceLayer.Mesh/Messaging/FileTransformQueueClient.cs +++ b/src/ServiceLayer.Mesh/Messaging/FileTransformQueueClient.cs @@ -1,15 +1,13 @@ using Azure.Storage.Queues; -using Microsoft.Extensions.Logging; using ServiceLayer.Data.Models; using ServiceLayer.Mesh.Configuration; namespace ServiceLayer.Mesh.Messaging; public class FileTransformQueueClient( - ILogger logger, IFileTransformQueueClientConfiguration configuration, QueueServiceClient queueServiceClient) - : QueueClientBase(logger, queueServiceClient), IFileTransformQueueClient + : QueueClientBase(queueServiceClient), IFileTransformQueueClient { public async Task EnqueueFileTransformAsync(MeshFile file) {