From b21e02a1765025b985cb9c0701edd2a050f6652b Mon Sep 17 00:00:00 2001 From: warren Date: Tue, 28 Apr 2026 13:17:01 +0100 Subject: [PATCH 1/8] feat: ReferenceData Function and Handler --- .../CohortManager/compose.data-services.yaml | 17 +++ .../DataServices.Migrations.csproj | 2 +- .../Model/ReferenceDataUpdateMessage.cs | 11 ++ .../ReferenceDataUpdater/Dockerfile | 16 ++ .../IReferenceDataInsertHandler.cs | 8 + .../ReferenceDataUpdater/Program.cs | 22 +++ .../ReferenceDataInsertHandler.cs | 143 ++++++++++++++++++ .../ReferenceDataUpdater.csproj | 31 ++++ .../ReferenceDataUpdaterFunction.cs | 86 +++++++++++ 9 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/IReferenceDataInsertHandler.cs create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Program.cs create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdater.csproj create mode 100644 application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdaterFunction.cs diff --git a/application/CohortManager/compose.data-services.yaml b/application/CohortManager/compose.data-services.yaml index fcb7d533db..9c2baacf4e 100644 --- a/application/CohortManager/compose.data-services.yaml +++ b/application/CohortManager/compose.data-services.yaml @@ -155,6 +155,23 @@ services: - DtOsDatabaseConnectionString=Server=db,1433;Database=${DB_NAME};User Id=SA;Password=${PASSWORD};TrustServerCertificate=True - AcceptableLatencyThresholdMs=500 + reference-data-updater: + container_name: reference-data-updater + image: cohort-manager-reference-data-updater + networks: [cohman-network] + build: + context: ./src/Functions/ + dockerfile: screeningDataServices/ReferenceDataUpdater/Dockerfile + args: + BASE_IMAGE: ${FUNCTION_BASE_IMAGE} + environment: + - DtOsDatabaseConnectionString=Server=db,1433;Database=${DB_NAME};User Id=SA;Password=${PASSWORD};TrustServerCertificate=True + - ServiceBusConnectionString=${SERVICE_BUS_CONNECTION_STRING} + - ReferenceDataTopicName=reference-data-updates + - ReferenceDataSubscription=reference-data-updater-sub + - AzureWebJobsStorage=${AZURE_WEB_JOBS_STORAGE} + - SeedDataBlobContainer=seed-data + servicenow-cases-data-service: container_name: servicenow-cases-data-service image: cohort-manager-servicenow-cases-data-service diff --git a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/DataServices.Migrations.csproj b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/DataServices.Migrations.csproj index 0fb55d3e4e..f7c1029d39 100644 --- a/application/CohortManager/src/Functions/Shared/DataServices.Migrations/DataServices.Migrations.csproj +++ b/application/CohortManager/src/Functions/Shared/DataServices.Migrations/DataServices.Migrations.csproj @@ -1,4 +1,4 @@ - + net8.0 diff --git a/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs b/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs new file mode 100644 index 0000000000..04b880b6f5 --- /dev/null +++ b/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs @@ -0,0 +1,11 @@ +namespace Model; + +using System.Text.Json; + +public class ReferenceDataUpdateMessage +{ + public string DataType { get; set; } + public JsonElement Data { get; set; } + public string CorrelationId { get; set; } + public DateTime Timestamp { get; set; } +} diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile new file mode 100644 index 0000000000..4024c99139 --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile @@ -0,0 +1,16 @@ +ARG BASE_IMAGE +FROM ${BASE_IMAGE} AS function + +COPY ./screeningDataServices/ReferenceDataUpdater /app/src/dotnet-function-app +WORKDIR /app/src/dotnet-function-app + +RUN --mount=type=cache,target=/root/.nuget/packages \ + dotnet publish *.csproj --output /home/site/wwwroot + +# To enable ssh & remote debugging on app service change the base image to the one below +# FROM mcr.microsoft.com/azure-functions/dotnet-isolated:4-dotnet-isolated8.0-appservice +FROM mcr.microsoft.com/azure-functions/dotnet-isolated:4-dotnet-isolated8.0 +ENV AzureWebJobsScriptRoot=/home/site/wwwroot \ + AzureFunctionsJobHost__Logging__Console__IsEnabled=true + +COPY --from=function ["/home/site/wwwroot", "/home/site/wwwroot"] diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/IReferenceDataInsertHandler.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/IReferenceDataInsertHandler.cs new file mode 100644 index 0000000000..87e2785848 --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/IReferenceDataInsertHandler.cs @@ -0,0 +1,8 @@ +namespace ReferenceDataUpdater; + +using System.Text.Json; + +public interface IReferenceDataInsertHandler +{ + Task ProcessRecord(string dataType, JsonElement data); +} diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Program.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Program.cs new file mode 100644 index 0000000000..8dcd727d3a --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Program.cs @@ -0,0 +1,22 @@ +using Common; +using DataServices.Core; +using DataServices.Database; +using HealthChecks.Extensions; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using ReferenceDataUpdater; + +var host = new HostBuilder() + .ConfigureFunctionsWorkerDefaults() + .AddDataServicesHandler() + .ConfigureServices(services => + { + services.AddSingleton(); + services.AddScoped(); + services.AddDatabaseHealthCheck("ReferenceDataUpdater"); + }) + .AddTelemetry() + .Build(); + +await host.RunAsync(); diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs new file mode 100644 index 0000000000..91341dfc43 --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs @@ -0,0 +1,143 @@ +namespace ReferenceDataUpdater; + +using System.Text; +using System.Text.Json; +using Common; +using DataServices.Core; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Model; + +public class ReferenceDataInsertHandler : IReferenceDataInsertHandler +{ + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNameCaseInsensitive = true, + WriteIndented = true + }; + + private static readonly Dictionary TypeRegistry = new(StringComparer.OrdinalIgnoreCase) + { + ["BsSelectGpPractice"] = (typeof(BsSelectGpPractice), "BsSelectGpPractice.json"), + ["BsSelectOutCode"] = (typeof(BsSelectOutCode), "BsSelectOutCode.json"), + ["CurrentPosting"] = (typeof(CurrentPosting), "CurrentPosting.json"), + ["ExcludedSMULookup"] = (typeof(ExcludedSMULookup), "ExcludedSMULookup.json"), + ["LanguageCode"] = (typeof(LanguageCode), "LanguageCode.json"), + ["ScreeningLkp"] = (typeof(ScreeningLkp), "ScreeningLkp.json"), + ["GeneCodeLkp"] = (typeof(GeneCodeLkp), "GeneCodeLkp.json"), + ["HigherRiskReferralReasonLkp"] = (typeof(HigherRiskReferralReasonLkp), "HigherRiskReferralReasonLkp.json"), + ["BsoOrganisation"] = (typeof(BsoOrganisation), "BsoOrganisation.json"), + ["GenderMaster"] = (typeof(GenderMaster), "GenderMaster.json"), + }; + + private readonly IServiceProvider _serviceProvider; + private readonly IBlobStorageHelper _blobStorageHelper; + private readonly ILogger _logger; + + public ReferenceDataInsertHandler( + IServiceProvider serviceProvider, + IBlobStorageHelper blobStorageHelper, + ILogger logger) + { + _serviceProvider = serviceProvider; + _blobStorageHelper = blobStorageHelper; + _logger = logger; + } + + public async Task ProcessRecord(string dataType, JsonElement data) + { + if (!TypeRegistry.TryGetValue(dataType, out var registration)) + { + _logger.LogError("Unknown reference data type: {DataType}", dataType); + return false; + } + + var entity = JsonSerializer.Deserialize(data.GetRawText(), registration.EntityType, JsonOptions); + if (entity is null) + { + _logger.LogError("Failed to deserialise payload for type {DataType}.", dataType); + return false; + } + + var dbInserted = await InsertIntoDatabase(dataType, registration.EntityType, entity); + await AppendToBlob(dataType, registration.BlobFileName, data); + + return dbInserted; + } + + private async Task InsertIntoDatabase(string dataType, Type entityType, object entity) + { + try + { + var accessorType = typeof(IDataServiceAccessor<>).MakeGenericType(entityType); + var accessor = _serviceProvider.GetRequiredService(accessorType); + + var insertMethod = accessorType.GetMethod("InsertSingle")!; + var task = (Task)insertMethod.Invoke(accessor, new[] { entity })!; + var result = await task; + + if (!result) + { + _logger.LogWarning("InsertSingle returned false for type {DataType}. Record may be a duplicate.", dataType); + } + + return true; + } + catch (DbUpdateException ex) when (IsPrimaryKeyViolation(ex)) + { + _logger.LogWarning("Duplicate record detected for type {DataType}. Skipping insert.", dataType); + return true; + } + catch (Exception ex) + { + _logger.LogError(ex, "Failed to insert record into database for type {DataType}.", dataType); + return false; + } + } + + private async Task AppendToBlob(string dataType, string blobFileName, JsonElement newRecord) + { + var connectionString = Environment.GetEnvironmentVariable("AzureWebJobsStorage"); + var containerName = Environment.GetEnvironmentVariable("SeedDataBlobContainer") ?? "seed-data"; + + try + { + var existingRecords = new List(); + + var existingBlob = await _blobStorageHelper.GetFileFromBlobStorage(connectionString!, containerName, blobFileName); + if (existingBlob?.Data != null) + { + existingBlob.Data.Position = 0; + using var reader = new StreamReader(existingBlob.Data); + var existingJson = await reader.ReadToEndAsync(); + existingRecords = JsonSerializer.Deserialize>(existingJson, JsonOptions) ?? new List(); + } + + existingRecords.Add(newRecord); + + var updatedJson = JsonSerializer.Serialize(existingRecords, JsonOptions); + var bytes = Encoding.UTF8.GetBytes(updatedJson); + var blobFile = new BlobFile(bytes, blobFileName); + + await _blobStorageHelper.UploadFileToBlobStorage(connectionString!, containerName, blobFile, overwrite: true); + + _logger.LogInformation( + "Appended record to blob {BlobFileName} for type {DataType}. Total records: {Count}", + blobFileName, dataType, existingRecords.Count); + } + catch (Exception ex) + { + _logger.LogWarning(ex, + "Failed to append record to blob for type {DataType}. DB insert was successful; blob is out of sync.", + dataType); + } + } + + private static bool IsPrimaryKeyViolation(DbUpdateException ex) + { + return ex.InnerException?.Message?.Contains("duplicate key", StringComparison.OrdinalIgnoreCase) == true + || ex.InnerException?.Message?.Contains("violation of primary key", StringComparison.OrdinalIgnoreCase) == true + || ex.InnerException?.Message?.Contains("unique constraint", StringComparison.OrdinalIgnoreCase) == true; + } +} diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdater.csproj b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdater.csproj new file mode 100644 index 0000000000..20e9433760 --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdater.csproj @@ -0,0 +1,31 @@ + + + net8.0 + v4 + Exe + enable + enable + + + + + + + + + + PreserveNewest + + + PreserveNewest + Never + + + + + + + + + + diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdaterFunction.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdaterFunction.cs new file mode 100644 index 0000000000..d006f4a2de --- /dev/null +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataUpdaterFunction.cs @@ -0,0 +1,86 @@ +namespace ReferenceDataUpdater; + +using System.Text.Json; +using Azure.Messaging.ServiceBus; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Extensions.Logging; +using Model; + +public class ReferenceDataUpdaterFunction +{ + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNameCaseInsensitive = true + }; + + private readonly IReferenceDataInsertHandler _insertHandler; + private readonly ILogger _logger; + + public ReferenceDataUpdaterFunction( + IReferenceDataInsertHandler insertHandler, + ILogger logger) + { + _insertHandler = insertHandler; + _logger = logger; + } + + [Function(nameof(ReferenceDataUpdaterFunction))] + public async Task Run( + [ServiceBusTrigger( + topicName: "%ReferenceDataTopicName%", + subscriptionName: "%ReferenceDataSubscription%", + Connection = "ServiceBusConnectionString", + AutoCompleteMessages = false)] + ServiceBusReceivedMessage message, + ServiceBusMessageActions messageActions) + { + ReferenceDataUpdateMessage? updateMessage; + try + { + updateMessage = JsonSerializer.Deserialize(message.Body, JsonOptions); + } + catch (JsonException ex) + { + _logger.LogError(ex, "Failed to deserialise reference data update message."); + await messageActions.DeadLetterMessageAsync(message); + return; + } + + if (updateMessage is null || string.IsNullOrWhiteSpace(updateMessage.DataType)) + { + _logger.LogError("Reference data update message was null or missing DataType."); + await messageActions.DeadLetterMessageAsync(message); + return; + } + + _logger.LogInformation( + "Processing reference data update | Type: {DataType} | CorrelationId: {CorrelationId}", + updateMessage.DataType, updateMessage.CorrelationId); + + try + { + var success = await _insertHandler.ProcessRecord(updateMessage.DataType, updateMessage.Data); + + if (!success) + { + _logger.LogError( + "Failed to process reference data update for type {DataType}.", + updateMessage.DataType); + await messageActions.DeadLetterMessageAsync(message); + return; + } + + await messageActions.CompleteMessageAsync(message); + _logger.LogInformation( + "Reference data update completed | Type: {DataType} | CorrelationId: {CorrelationId}", + updateMessage.DataType, updateMessage.CorrelationId); + } + catch (Exception ex) + { + _logger.LogError(ex, + "Unexpected error processing reference data update for type {DataType}. CorrelationId: {CorrelationId}", + updateMessage.DataType, updateMessage.CorrelationId); + await messageActions.DeadLetterMessageAsync(message); + } + } +} From 425084eede476c08ee7b6c06ceafa04310ec8453 Mon Sep 17 00:00:00 2001 From: warren Date: Tue, 28 Apr 2026 13:17:42 +0100 Subject: [PATCH 2/8] test: unit tests --- .../ReferenceDataInsertHandlerTests.cs | 301 ++++++++++++++++++ .../ReferenceDataUpdaterFunctionTests.cs | 215 +++++++++++++ .../ReferenceDataUpdaterTests.csproj | 33 ++ 3 files changed, 549 insertions(+) create mode 100644 tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs create mode 100644 tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs create mode 100644 tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterTests.csproj diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs new file mode 100644 index 0000000000..a4729b2fae --- /dev/null +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs @@ -0,0 +1,301 @@ +namespace NHS.CohortManager.Tests.UnitTests.ScreeningDataServicesTests; + +using System.Text; +using System.Text.Json; +using Common; +using DataServices.Core; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Model; +using Moq; +using ReferenceDataUpdater; + +[TestClass] +public class ReferenceDataInsertHandlerTests +{ + private readonly Mock _serviceProviderMock = new(); + private readonly Mock _blobStorageHelperMock = new(); + private readonly Mock> _loggerMock = new(); + private readonly ReferenceDataInsertHandler _handler; + + public ReferenceDataInsertHandlerTests() + { + Environment.SetEnvironmentVariable("AzureWebJobsStorage", "UseDevelopmentStorage=true"); + Environment.SetEnvironmentVariable("SeedDataBlobContainer", "seed-data"); + + _handler = new ReferenceDataInsertHandler( + _serviceProviderMock.Object, + _blobStorageHelperMock.Object, + _loggerMock.Object); + } + + [TestMethod] + public async Task ProcessRecord_UnknownDataType_ReturnsFalse() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { Id = 1 }); + + // Act + var result = await _handler.ProcessRecord("NonExistentType", data); + + // Assert + Assert.IsFalse(result); + } + + [TestMethod] + public async Task ProcessRecord_ValidDataType_InsertsIntoDatabaseAndAppendsToBlob_ReturnsTrue() + { + // Arrange + var gpPractice = new { GpPracticeCode = "Y12345", GpPracticeName = "Test Practice" }; + var data = JsonSerializer.SerializeToElement(gpPractice); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "BsSelectGpPractice.json")) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("BsSelectGpPractice", data); + + // Assert + Assert.IsTrue(result); + accessorMock.Verify(a => a.InsertSingle(It.IsAny()), Times.Once); + _blobStorageHelperMock.Verify(b => b.UploadFileToBlobStorage(It.IsAny(), "seed-data", It.IsAny(), true), Times.Once); + } + + [TestMethod] + public async Task ProcessRecord_DatabaseInsertFails_ReturnsFalse() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { ScreeningLkpId = 1 }); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())) + .ThrowsAsync(new Exception("Database connection failed")); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + // Act + var result = await _handler.ProcessRecord("ScreeningLkp", data); + + // Assert + Assert.IsFalse(result); + } + + [TestMethod] + public async Task ProcessRecord_DuplicateKeyViolation_ReturnsTrue() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { LanguageCodeId = "EN", LanguageDescription = "English" }); + + var innerException = new Exception("Violation of PRIMARY KEY constraint"); + var dbUpdateException = new DbUpdateException("An error occurred", innerException); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())) + .ThrowsAsync(dbUpdateException); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "LanguageCode.json")) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("LanguageCode", data); + + // Assert + Assert.IsTrue(result); + } + + [TestMethod] + public async Task ProcessRecord_DuplicateKey_UniqueConstraintMessage_ReturnsTrue() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { GenderCode = "M" }); + + var innerException = new Exception("unique constraint violation on table"); + var dbUpdateException = new DbUpdateException("An error occurred", innerException); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())) + .ThrowsAsync(dbUpdateException); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "GenderMaster.json")) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("GenderMaster", data); + + // Assert + Assert.IsTrue(result); + } + + [TestMethod] + public async Task ProcessRecord_BlobAppendFails_StillReturnsTrue() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { OutCode = "AB1" }); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "BsSelectOutCode.json")) + .ThrowsAsync(new Exception("Blob storage unavailable")); + + // Act + var result = await _handler.ProcessRecord("BsSelectOutCode", data); + + // Assert + Assert.IsTrue(result); + } + + [TestMethod] + public async Task ProcessRecord_ExistingBlobData_AppendsNewRecord() + { + // Arrange + var existingRecords = new[] { new { PostingId = 1 } }; + var existingJson = JsonSerializer.Serialize(existingRecords); + var existingBlob = new BlobFile(Encoding.UTF8.GetBytes(existingJson), "CurrentPosting.json"); + + var data = JsonSerializer.SerializeToElement(new { PostingId = 2 }); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "CurrentPosting.json")) + .ReturnsAsync(existingBlob); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + BlobFile? uploadedBlob = null; + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .Callback((_, _, blob, _) => uploadedBlob = blob) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("CurrentPosting", data); + + // Assert + Assert.IsTrue(result); + Assert.IsNotNull(uploadedBlob); + + uploadedBlob.Data.Position = 0; + using var reader = new StreamReader(uploadedBlob.Data); + var uploadedJson = await reader.ReadToEndAsync(); + var records = JsonSerializer.Deserialize>(uploadedJson); + + Assert.AreEqual(2, records!.Count); + } + + [TestMethod] + public async Task ProcessRecord_NullDeserialisation_ReturnsFalse() + { + // Arrange — a payload that deserialises to something but is "null" in a JSON sense + var data = JsonSerializer.SerializeToElement(null!); + + // Act + var result = await _handler.ProcessRecord("BsSelectGpPractice", data); + + // Assert + Assert.IsFalse(result); + } + + [TestMethod] + [DataRow("BsSelectGpPractice")] + [DataRow("BsSelectOutCode")] + [DataRow("CurrentPosting")] + [DataRow("ExcludedSMULookup")] + [DataRow("LanguageCode")] + [DataRow("ScreeningLkp")] + [DataRow("GeneCodeLkp")] + [DataRow("HigherRiskReferralReasonLkp")] + [DataRow("BsoOrganisation")] + [DataRow("GenderMaster")] + public async Task ProcessRecord_AllRegisteredTypes_AreRecognised(string dataType) + { + // Arrange — just verify the type is recognised (will fail on deserialise but not on lookup) + var data = JsonSerializer.SerializeToElement(new { Id = 1 }); + + // We need a mock accessor for whatever type this maps to. + // Since we can't easily set up all 10, verify it doesn't return false for "unknown type". + // The handler will attempt to resolve from DI and fail, but that's a different error path. + _serviceProviderMock.Setup(sp => sp.GetService(It.IsAny())).Returns(null!); + + // Act + var result = await _handler.ProcessRecord(dataType, data); + + // Assert — will be false because DI can't resolve the accessor, but it should NOT be + // false because the type was unknown. We verify no "Unknown reference data type" log. + _loggerMock.Verify( + x => x.Log( + LogLevel.Error, + It.IsAny(), + It.Is((v, t) => v.ToString()!.Contains("Unknown reference data type")), + It.IsAny(), + It.IsAny>() + ), Times.Never); + } + + [TestMethod] + public async Task ProcessRecord_DataTypeLookup_IsCaseInsensitive() + { + // Arrange + var data = JsonSerializer.SerializeToElement(new { GpPracticeCode = "Y12345" }); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("bsselectgppractice", data); + + // Assert + Assert.IsTrue(result); + accessorMock.Verify(a => a.InsertSingle(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task ProcessRecord_InsertSingleReturnsFalse_StillReturnsTrue() + { + // Arrange — InsertSingle returns false (e.g. no rows affected) but no exception + var data = JsonSerializer.SerializeToElement(new { GpPracticeCode = "Y99999" }); + + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(false); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + + // Act + var result = await _handler.ProcessRecord("BsSelectGpPractice", data); + + // Assert — returns true because InsertSingle returning false is treated as "maybe duplicate" not failure + Assert.IsTrue(result); + } +} diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs new file mode 100644 index 0000000000..dacda26722 --- /dev/null +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs @@ -0,0 +1,215 @@ +namespace NHS.CohortManager.Tests.UnitTests.ScreeningDataServicesTests; + +using System.Text.Json; +using Azure.Messaging.ServiceBus; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Extensions.Logging; +using Moq; +using Model; +using ReferenceDataUpdater; +using NHS.CohortManager.Tests.TestUtils; + +[TestClass] +public class ReferenceDataUpdaterFunctionTests +{ + private readonly Mock _insertHandlerMock = new(); + private readonly Mock> _loggerMock = new(); + private readonly Mock _messageActionsMock = new(); + private readonly ReferenceDataUpdaterFunction _function; + + public ReferenceDataUpdaterFunctionTests() + { + _function = new ReferenceDataUpdaterFunction(_insertHandlerMock.Object, _loggerMock.Object); + } + + private static ServiceBusReceivedMessage CreateServiceBusMessage(object body) + { + var json = JsonSerializer.Serialize(body); + return ServiceBusModelFactory.ServiceBusReceivedMessage( + body: new BinaryData(json), + messageId: "test-message-id", + correlationId: "test-correlation-id" + ); + } + + private static ServiceBusReceivedMessage CreateServiceBusMessage(string rawBody) + { + return ServiceBusModelFactory.ServiceBusReceivedMessage( + body: new BinaryData(rawBody), + messageId: "test-message-id", + correlationId: "test-correlation-id" + ); + } + + [TestMethod] + public async Task Run_ValidMessage_ProcessRecordSucceeds_CompletesMessage() + { + // Arrange + var updateMessage = new ReferenceDataUpdateMessage + { + DataType = "BsSelectGpPractice", + Data = JsonSerializer.SerializeToElement(new { Code = "Y12345", Name = "Test Practice" }), + CorrelationId = "corr-001", + Timestamp = DateTime.UtcNow + }; + + var message = CreateServiceBusMessage(updateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord("BsSelectGpPractice", It.IsAny())) + .ReturnsAsync(true); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Never); + } + + [TestMethod] + public async Task Run_ValidMessage_ProcessRecordFails_DeadLettersMessage() + { + // Arrange + var updateMessage = new ReferenceDataUpdateMessage + { + DataType = "UnknownType", + Data = JsonSerializer.SerializeToElement(new { Id = 1 }), + CorrelationId = "corr-002", + Timestamp = DateTime.UtcNow + }; + + var message = CreateServiceBusMessage(updateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord("UnknownType", It.IsAny())) + .ReturnsAsync(false); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Never); + } + + [TestMethod] + public async Task Run_InvalidJson_DeadLettersMessage() + { + // Arrange + var message = CreateServiceBusMessage("not valid json {{{"); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Never); + } + + [TestMethod] + public async Task Run_NullDataType_DeadLettersMessage() + { + // Arrange + var updateMessage = new ReferenceDataUpdateMessage + { + DataType = null!, + Data = JsonSerializer.SerializeToElement(new { Id = 1 }), + CorrelationId = "corr-003", + Timestamp = DateTime.UtcNow + }; + + var message = CreateServiceBusMessage(updateMessage); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _insertHandlerMock.Verify( + h => h.ProcessRecord(It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task Run_EmptyDataType_DeadLettersMessage() + { + // Arrange + var updateMessage = new ReferenceDataUpdateMessage + { + DataType = " ", + Data = JsonSerializer.SerializeToElement(new { Id = 1 }), + CorrelationId = "corr-004", + Timestamp = DateTime.UtcNow + }; + + var message = CreateServiceBusMessage(updateMessage); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _insertHandlerMock.Verify( + h => h.ProcessRecord(It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task Run_ProcessRecordThrowsException_DeadLettersMessage() + { + // Arrange + var updateMessage = new ReferenceDataUpdateMessage + { + DataType = "BsSelectGpPractice", + Data = JsonSerializer.SerializeToElement(new { Code = "Y12345" }), + CorrelationId = "corr-005", + Timestamp = DateTime.UtcNow + }; + + var message = CreateServiceBusMessage(updateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord("BsSelectGpPractice", It.IsAny())) + .ThrowsAsync(new InvalidOperationException("Something went wrong")); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Never); + } + + [TestMethod] + public async Task Run_EmptyMessageBody_DeadLettersMessage() + { + // Arrange + var message = ServiceBusModelFactory.ServiceBusReceivedMessage( + body: new BinaryData("null"), + messageId: "test-message-id" + ); + + // Act + await _function.Run(message, _messageActionsMock.Object); + + // Assert + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + } +} diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterTests.csproj b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterTests.csproj new file mode 100644 index 0000000000..b8a01a7cb9 --- /dev/null +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterTests.csproj @@ -0,0 +1,33 @@ + + + + net8.0 + enable + enable + false + true + + + + + + + + + + + + + + + + + + + + + + + + + From cab88c1b60794906aa04f610e6bca6c21c56186a Mon Sep 17 00:00:00 2001 From: warren Date: Tue, 28 Apr 2026 15:22:28 +0100 Subject: [PATCH 3/8] test: unit tests Co-authored-by: Copilot --- .../ReferenceDataInsertHandlerTests.cs | 184 +++++++----------- .../ReferenceDataUpdaterFunctionTests.cs | 147 ++++++-------- 2 files changed, 131 insertions(+), 200 deletions(-) diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs index a4729b2fae..e3d4ec83da 100644 --- a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs @@ -5,7 +5,6 @@ namespace NHS.CohortManager.Tests.UnitTests.ScreeningDataServicesTests; using Common; using DataServices.Core; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Model; using Moq; @@ -30,11 +29,33 @@ public ReferenceDataInsertHandlerTests() _loggerMock.Object); } + private Mock> SetupAccessor(bool insertResult = true) where T : class + { + var accessorMock = new Mock>(); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(insertResult); + _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) + .Returns(accessorMock.Object); + return accessorMock; + } + + private void SetupBlobStorageDefaults(string blobFileName) + { + _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), blobFileName)) + .ReturnsAsync((BlobFile)null!); + _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) + .ReturnsAsync(true); + } + + private static JsonElement CreatePayload(object data) + { + return JsonSerializer.SerializeToElement(data); + } + [TestMethod] public async Task ProcessRecord_UnknownDataType_ReturnsFalse() { // Arrange - var data = JsonSerializer.SerializeToElement(new { Id = 1 }); + var data = CreatePayload(new { Id = 1 }); // Act var result = await _handler.ProcessRecord("NonExistentType", data); @@ -44,21 +65,12 @@ public async Task ProcessRecord_UnknownDataType_ReturnsFalse() } [TestMethod] - public async Task ProcessRecord_ValidDataType_InsertsIntoDatabaseAndAppendsToBlob_ReturnsTrue() + public async Task ProcessRecord_ValidRecord_InsertsAndAppendsToBlob_ReturnsTrue() { // Arrange - var gpPractice = new { GpPracticeCode = "Y12345", GpPracticeName = "Test Practice" }; - var data = JsonSerializer.SerializeToElement(gpPractice); - - var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); - _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) - .Returns(accessorMock.Object); - - _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "BsSelectGpPractice.json")) - .ReturnsAsync((BlobFile)null!); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); + var data = CreatePayload(new { GpPracticeCode = "Y12345" }); + var accessorMock = SetupAccessor(); + SetupBlobStorageDefaults("BsSelectGpPractice.json"); // Act var result = await _handler.ProcessRecord("BsSelectGpPractice", data); @@ -70,11 +82,10 @@ public async Task ProcessRecord_ValidDataType_InsertsIntoDatabaseAndAppendsToBlo } [TestMethod] - public async Task ProcessRecord_DatabaseInsertFails_ReturnsFalse() + public async Task ProcessRecord_DatabaseInsertThrows_ReturnsFalse() { // Arrange - var data = JsonSerializer.SerializeToElement(new { ScreeningLkpId = 1 }); - + var data = CreatePayload(new { ScreeningLkpId = 1 }); var accessorMock = new Mock>(); accessorMock.Setup(a => a.InsertSingle(It.IsAny())) .ThrowsAsync(new Exception("Database connection failed")); @@ -89,24 +100,17 @@ public async Task ProcessRecord_DatabaseInsertFails_ReturnsFalse() } [TestMethod] - public async Task ProcessRecord_DuplicateKeyViolation_ReturnsTrue() + public async Task ProcessRecord_PrimaryKeyViolation_ReturnsTrue() { // Arrange - var data = JsonSerializer.SerializeToElement(new { LanguageCodeId = "EN", LanguageDescription = "English" }); - - var innerException = new Exception("Violation of PRIMARY KEY constraint"); - var dbUpdateException = new DbUpdateException("An error occurred", innerException); + var data = CreatePayload(new { LanguageCodeId = "EN", LanguageDescription = "English" }); + var dbUpdateException = new DbUpdateException("An error occurred", new Exception("Violation of PRIMARY KEY constraint")); var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())) - .ThrowsAsync(dbUpdateException); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ThrowsAsync(dbUpdateException); _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) .Returns(accessorMock.Object); - - _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "LanguageCode.json")) - .ReturnsAsync((BlobFile)null!); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); + SetupBlobStorageDefaults("LanguageCode.json"); // Act var result = await _handler.ProcessRecord("LanguageCode", data); @@ -116,24 +120,17 @@ public async Task ProcessRecord_DuplicateKeyViolation_ReturnsTrue() } [TestMethod] - public async Task ProcessRecord_DuplicateKey_UniqueConstraintMessage_ReturnsTrue() + public async Task ProcessRecord_UniqueConstraintViolation_ReturnsTrue() { // Arrange - var data = JsonSerializer.SerializeToElement(new { GenderCode = "M" }); - - var innerException = new Exception("unique constraint violation on table"); - var dbUpdateException = new DbUpdateException("An error occurred", innerException); + var data = CreatePayload(new { GenderCode = "M" }); + var dbUpdateException = new DbUpdateException("An error occurred", new Exception("unique constraint violation on table")); var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())) - .ThrowsAsync(dbUpdateException); + accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ThrowsAsync(dbUpdateException); _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) .Returns(accessorMock.Object); - - _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "GenderMaster.json")) - .ReturnsAsync((BlobFile)null!); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); + SetupBlobStorageDefaults("GenderMaster.json"); // Act var result = await _handler.ProcessRecord("GenderMaster", data); @@ -143,16 +140,11 @@ public async Task ProcessRecord_DuplicateKey_UniqueConstraintMessage_ReturnsTrue } [TestMethod] - public async Task ProcessRecord_BlobAppendFails_StillReturnsTrue() + public async Task ProcessRecord_BlobAppendFails_ReturnsTrue() { // Arrange - var data = JsonSerializer.SerializeToElement(new { OutCode = "AB1" }); - - var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); - _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) - .Returns(accessorMock.Object); - + var data = CreatePayload(new { OutCode = "AB1" }); + SetupAccessor(); _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "BsSelectOutCode.json")) .ThrowsAsync(new Exception("Blob storage unavailable")); @@ -164,24 +156,16 @@ public async Task ProcessRecord_BlobAppendFails_StillReturnsTrue() } [TestMethod] - public async Task ProcessRecord_ExistingBlobData_AppendsNewRecord() + public async Task ProcessRecord_ExistingBlobRecords_AppendsNewRecord() { // Arrange - var existingRecords = new[] { new { PostingId = 1 } }; - var existingJson = JsonSerializer.Serialize(existingRecords); + var existingJson = JsonSerializer.Serialize(new[] { new { PostingId = 1 } }); var existingBlob = new BlobFile(Encoding.UTF8.GetBytes(existingJson), "CurrentPosting.json"); + var data = CreatePayload(new { PostingId = 2 }); - var data = JsonSerializer.SerializeToElement(new { PostingId = 2 }); - - var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); - _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) - .Returns(accessorMock.Object); - + SetupAccessor(); _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), "CurrentPosting.json")) .ReturnsAsync(existingBlob); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); BlobFile? uploadedBlob = null; _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) @@ -199,14 +183,13 @@ public async Task ProcessRecord_ExistingBlobData_AppendsNewRecord() using var reader = new StreamReader(uploadedBlob.Data); var uploadedJson = await reader.ReadToEndAsync(); var records = JsonSerializer.Deserialize>(uploadedJson); - Assert.AreEqual(2, records!.Count); } [TestMethod] - public async Task ProcessRecord_NullDeserialisation_ReturnsFalse() + public async Task ProcessRecord_NullJsonPayload_ReturnsFalse() { - // Arrange — a payload that deserialises to something but is "null" in a JSON sense + // Arrange var data = JsonSerializer.SerializeToElement(null!); // Act @@ -216,32 +199,27 @@ public async Task ProcessRecord_NullDeserialisation_ReturnsFalse() Assert.IsFalse(result); } + [DataRow("BsSelectGpPractice", DisplayName = "BsSelectGpPractice is a registered type")] + [DataRow("BsSelectOutCode", DisplayName = "BsSelectOutCode is a registered type")] + [DataRow("CurrentPosting", DisplayName = "CurrentPosting is a registered type")] + [DataRow("ExcludedSMULookup", DisplayName = "ExcludedSMULookup is a registered type")] + [DataRow("LanguageCode", DisplayName = "LanguageCode is a registered type")] + [DataRow("ScreeningLkp", DisplayName = "ScreeningLkp is a registered type")] + [DataRow("GeneCodeLkp", DisplayName = "GeneCodeLkp is a registered type")] + [DataRow("HigherRiskReferralReasonLkp", DisplayName = "HigherRiskReferralReasonLkp is a registered type")] + [DataRow("BsoOrganisation", DisplayName = "BsoOrganisation is a registered type")] + [DataRow("GenderMaster", DisplayName = "GenderMaster is a registered type")] [TestMethod] - [DataRow("BsSelectGpPractice")] - [DataRow("BsSelectOutCode")] - [DataRow("CurrentPosting")] - [DataRow("ExcludedSMULookup")] - [DataRow("LanguageCode")] - [DataRow("ScreeningLkp")] - [DataRow("GeneCodeLkp")] - [DataRow("HigherRiskReferralReasonLkp")] - [DataRow("BsoOrganisation")] - [DataRow("GenderMaster")] - public async Task ProcessRecord_AllRegisteredTypes_AreRecognised(string dataType) + public async Task ProcessRecord_RegisteredDataType_DoesNotLogUnknownType(string dataType) { - // Arrange — just verify the type is recognised (will fail on deserialise but not on lookup) - var data = JsonSerializer.SerializeToElement(new { Id = 1 }); - - // We need a mock accessor for whatever type this maps to. - // Since we can't easily set up all 10, verify it doesn't return false for "unknown type". - // The handler will attempt to resolve from DI and fail, but that's a different error path. + // Arrange + var data = CreatePayload(new { Id = 1 }); _serviceProviderMock.Setup(sp => sp.GetService(It.IsAny())).Returns(null!); // Act - var result = await _handler.ProcessRecord(dataType, data); + await _handler.ProcessRecord(dataType, data); - // Assert — will be false because DI can't resolve the accessor, but it should NOT be - // false because the type was unknown. We verify no "Unknown reference data type" log. + // Assert _loggerMock.Verify( x => x.Log( LogLevel.Error, @@ -253,20 +231,12 @@ public async Task ProcessRecord_AllRegisteredTypes_AreRecognised(string dataType } [TestMethod] - public async Task ProcessRecord_DataTypeLookup_IsCaseInsensitive() + public async Task ProcessRecord_CaseInsensitiveDataType_ReturnsTrue() { // Arrange - var data = JsonSerializer.SerializeToElement(new { GpPracticeCode = "Y12345" }); - - var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(true); - _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) - .Returns(accessorMock.Object); - - _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((BlobFile)null!); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); + var data = CreatePayload(new { GpPracticeCode = "Y12345" }); + var accessorMock = SetupAccessor(); + SetupBlobStorageDefaults("BsSelectGpPractice.json"); // Act var result = await _handler.ProcessRecord("bsselectgppractice", data); @@ -277,25 +247,17 @@ public async Task ProcessRecord_DataTypeLookup_IsCaseInsensitive() } [TestMethod] - public async Task ProcessRecord_InsertSingleReturnsFalse_StillReturnsTrue() + public async Task ProcessRecord_InsertSingleReturnsFalse_ReturnsTrue() { - // Arrange — InsertSingle returns false (e.g. no rows affected) but no exception - var data = JsonSerializer.SerializeToElement(new { GpPracticeCode = "Y99999" }); - - var accessorMock = new Mock>(); - accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ReturnsAsync(false); - _serviceProviderMock.Setup(sp => sp.GetService(typeof(IDataServiceAccessor))) - .Returns(accessorMock.Object); - - _blobStorageHelperMock.Setup(b => b.GetFileFromBlobStorage(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((BlobFile)null!); - _blobStorageHelperMock.Setup(b => b.UploadFileToBlobStorage(It.IsAny(), It.IsAny(), It.IsAny(), true)) - .ReturnsAsync(true); + // Arrange + var data = CreatePayload(new { GpPracticeCode = "Y99999" }); + SetupAccessor(insertResult: false); + SetupBlobStorageDefaults("BsSelectGpPractice.json"); // Act var result = await _handler.ProcessRecord("BsSelectGpPractice", data); - // Assert — returns true because InsertSingle returning false is treated as "maybe duplicate" not failure + // Assert Assert.IsTrue(result); } } diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs index dacda26722..164b32f74e 100644 --- a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataUpdaterFunctionTests.cs @@ -7,7 +7,6 @@ namespace NHS.CohortManager.Tests.UnitTests.ScreeningDataServicesTests; using Moq; using Model; using ReferenceDataUpdater; -using NHS.CohortManager.Tests.TestUtils; [TestClass] public class ReferenceDataUpdaterFunctionTests @@ -17,22 +16,29 @@ public class ReferenceDataUpdaterFunctionTests private readonly Mock _messageActionsMock = new(); private readonly ReferenceDataUpdaterFunction _function; + private readonly ReferenceDataUpdateMessage _validUpdateMessage = new() + { + DataType = "BsSelectGpPractice", + Data = JsonSerializer.SerializeToElement(new { GpPracticeCode = "Y12345" }), + CorrelationId = "test-correlation-id", + Timestamp = DateTime.UtcNow + }; + public ReferenceDataUpdaterFunctionTests() { _function = new ReferenceDataUpdaterFunction(_insertHandlerMock.Object, _loggerMock.Object); } - private static ServiceBusReceivedMessage CreateServiceBusMessage(object body) + private static ServiceBusReceivedMessage CreateMessage(object body) { - var json = JsonSerializer.Serialize(body); return ServiceBusModelFactory.ServiceBusReceivedMessage( - body: new BinaryData(json), + body: new BinaryData(JsonSerializer.Serialize(body)), messageId: "test-message-id", correlationId: "test-correlation-id" ); } - private static ServiceBusReceivedMessage CreateServiceBusMessage(string rawBody) + private static ServiceBusReceivedMessage CreateMessage(string rawBody) { return ServiceBusModelFactory.ServiceBusReceivedMessage( body: new BinaryData(rawBody), @@ -41,78 +47,67 @@ private static ServiceBusReceivedMessage CreateServiceBusMessage(string rawBody) ); } + private void VerifyMessageCompleted() + { + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Never); + } + + private void VerifyMessageDeadLettered() + { + _messageActionsMock.Verify( + x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), + Times.Once); + _messageActionsMock.Verify( + x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), + Times.Never); + } + [TestMethod] - public async Task Run_ValidMessage_ProcessRecordSucceeds_CompletesMessage() + public async Task Run_ProcessRecordSucceeds_CompletesMessage() { // Arrange - var updateMessage = new ReferenceDataUpdateMessage - { - DataType = "BsSelectGpPractice", - Data = JsonSerializer.SerializeToElement(new { Code = "Y12345", Name = "Test Practice" }), - CorrelationId = "corr-001", - Timestamp = DateTime.UtcNow - }; - - var message = CreateServiceBusMessage(updateMessage); - _insertHandlerMock.Setup(h => h.ProcessRecord("BsSelectGpPractice", It.IsAny())) + var message = CreateMessage(_validUpdateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord(_validUpdateMessage.DataType, It.IsAny())) .ReturnsAsync(true); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), - Times.Once); - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Never); + VerifyMessageCompleted(); } [TestMethod] - public async Task Run_ValidMessage_ProcessRecordFails_DeadLettersMessage() + public async Task Run_ProcessRecordReturnsFalse_DeadLettersMessage() { // Arrange - var updateMessage = new ReferenceDataUpdateMessage - { - DataType = "UnknownType", - Data = JsonSerializer.SerializeToElement(new { Id = 1 }), - CorrelationId = "corr-002", - Timestamp = DateTime.UtcNow - }; - - var message = CreateServiceBusMessage(updateMessage); - _insertHandlerMock.Setup(h => h.ProcessRecord("UnknownType", It.IsAny())) + var message = CreateMessage(_validUpdateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord(_validUpdateMessage.DataType, It.IsAny())) .ReturnsAsync(false); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Once); - _messageActionsMock.Verify( - x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), - Times.Never); + VerifyMessageDeadLettered(); } [TestMethod] - public async Task Run_InvalidJson_DeadLettersMessage() + public async Task Run_InvalidJsonBody_DeadLettersMessage() { // Arrange - var message = CreateServiceBusMessage("not valid json {{{"); + var message = CreateMessage("not valid json {{{"); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Once); - _messageActionsMock.Verify( - x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), - Times.Never); + VerifyMessageDeadLettered(); } [TestMethod] @@ -122,87 +117,61 @@ public async Task Run_NullDataType_DeadLettersMessage() var updateMessage = new ReferenceDataUpdateMessage { DataType = null!, - Data = JsonSerializer.SerializeToElement(new { Id = 1 }), - CorrelationId = "corr-003", + Data = _validUpdateMessage.Data, + CorrelationId = "test-correlation-id", Timestamp = DateTime.UtcNow }; - - var message = CreateServiceBusMessage(updateMessage); + var message = CreateMessage(updateMessage); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Once); - _insertHandlerMock.Verify( - h => h.ProcessRecord(It.IsAny(), It.IsAny()), - Times.Never); + VerifyMessageDeadLettered(); + _insertHandlerMock.Verify(h => h.ProcessRecord(It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] - public async Task Run_EmptyDataType_DeadLettersMessage() + public async Task Run_WhitespaceDataType_DeadLettersMessage() { // Arrange var updateMessage = new ReferenceDataUpdateMessage { DataType = " ", - Data = JsonSerializer.SerializeToElement(new { Id = 1 }), - CorrelationId = "corr-004", + Data = _validUpdateMessage.Data, + CorrelationId = "test-correlation-id", Timestamp = DateTime.UtcNow }; - - var message = CreateServiceBusMessage(updateMessage); + var message = CreateMessage(updateMessage); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Once); - _insertHandlerMock.Verify( - h => h.ProcessRecord(It.IsAny(), It.IsAny()), - Times.Never); + VerifyMessageDeadLettered(); + _insertHandlerMock.Verify(h => h.ProcessRecord(It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] public async Task Run_ProcessRecordThrowsException_DeadLettersMessage() { // Arrange - var updateMessage = new ReferenceDataUpdateMessage - { - DataType = "BsSelectGpPractice", - Data = JsonSerializer.SerializeToElement(new { Code = "Y12345" }), - CorrelationId = "corr-005", - Timestamp = DateTime.UtcNow - }; - - var message = CreateServiceBusMessage(updateMessage); - _insertHandlerMock.Setup(h => h.ProcessRecord("BsSelectGpPractice", It.IsAny())) + var message = CreateMessage(_validUpdateMessage); + _insertHandlerMock.Setup(h => h.ProcessRecord(_validUpdateMessage.DataType, It.IsAny())) .ThrowsAsync(new InvalidOperationException("Something went wrong")); // Act await _function.Run(message, _messageActionsMock.Object); // Assert - _messageActionsMock.Verify( - x => x.DeadLetterMessageAsync(It.IsAny(), null, null, null, CancellationToken.None), - Times.Once); - _messageActionsMock.Verify( - x => x.CompleteMessageAsync(It.IsAny(), CancellationToken.None), - Times.Never); + VerifyMessageDeadLettered(); } [TestMethod] - public async Task Run_EmptyMessageBody_DeadLettersMessage() + public async Task Run_NullMessageBody_DeadLettersMessage() { // Arrange - var message = ServiceBusModelFactory.ServiceBusReceivedMessage( - body: new BinaryData("null"), - messageId: "test-message-id" - ); + var message = CreateMessage("null"); // Act await _function.Run(message, _messageActionsMock.Object); From 8bc44089075d6865246876d42f7fc2d49154f4cf Mon Sep 17 00:00:00 2001 From: warren Date: Wed, 29 Apr 2026 10:12:37 +0100 Subject: [PATCH 4/8] feat: terraform --- .../tf-core/environments/development.tfvars | 26 +++++++++++++++++++ .../tf-core/environments/integration.tfvars | 26 +++++++++++++++++++ .../tf-core/environments/preprod.tfvars | 26 +++++++++++++++++++ .../tf-core/environments/production.tfvars | 26 +++++++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/infrastructure/tf-core/environments/development.tfvars b/infrastructure/tf-core/environments/development.tfvars index 3ae6c6fcd1..8a7db32e0c 100644 --- a/infrastructure/tf-core/environments/development.tfvars +++ b/infrastructure/tf-core/environments/development.tfvars @@ -1158,6 +1158,19 @@ function_apps = { } } + ReferenceDataUpdater = { + name_suffix = "reference-data-updater" + function_endpoint_name = "ReferenceDataUpdater" + app_service_plan_key = "NonScaling" + db_connection_string = "DtOsDatabaseConnectionString" + service_bus_connections = ["external"] + env_vars_static = { + ReferenceDataTopicName = "reference-data-updates" + ReferenceDataSubscription = "ReferenceDataUpdater" + SeedDataBlobContainer = "seed-data" + } + } + NemsSubscribe = { name_suffix = "nems-subscribe" function_endpoint_name = "NemsSubscribe" @@ -1343,6 +1356,19 @@ service_bus = { } } } + + external = { + capacity = 1 + sku_tier = "Premium" + max_payload_size = "100mb" + topics = { + reference-data-updates = { + batched_operations_enabled = true + max_delivery_count = 10 + subscribers = ["ReferenceDataUpdater"] + } + } + } } sqlserver = { diff --git a/infrastructure/tf-core/environments/integration.tfvars b/infrastructure/tf-core/environments/integration.tfvars index bb051878f6..78cf3bf0db 100644 --- a/infrastructure/tf-core/environments/integration.tfvars +++ b/infrastructure/tf-core/environments/integration.tfvars @@ -1131,6 +1131,19 @@ function_apps = { } } + ReferenceDataUpdater = { + name_suffix = "reference-data-updater" + function_endpoint_name = "ReferenceDataUpdater" + app_service_plan_key = "NonScaling" + db_connection_string = "DtOsDatabaseConnectionString" + service_bus_connections = ["external"] + env_vars_static = { + ReferenceDataTopicName = "reference-data-updates" + ReferenceDataSubscription = "ReferenceDataUpdater" + SeedDataBlobContainer = "seed-data" + } + } + NemsSubscribe = { name_suffix = "nems-subscribe" function_endpoint_name = "NemsSubscribe" @@ -1315,6 +1328,19 @@ service_bus = { } } } + + external = { + capacity = 1 + sku_tier = "Premium" + max_payload_size = "100mb" + topics = { + reference-data-updates = { + batched_operations_enabled = true + max_delivery_count = 10 + subscribers = ["ReferenceDataUpdater"] + } + } + } } sqlserver = { diff --git a/infrastructure/tf-core/environments/preprod.tfvars b/infrastructure/tf-core/environments/preprod.tfvars index 8e8509d600..97f5e0c8e9 100644 --- a/infrastructure/tf-core/environments/preprod.tfvars +++ b/infrastructure/tf-core/environments/preprod.tfvars @@ -1141,6 +1141,19 @@ function_apps = { } } + ReferenceDataUpdater = { + name_suffix = "reference-data-updater" + function_endpoint_name = "ReferenceDataUpdater" + app_service_plan_key = "NonScaling" + db_connection_string = "DtOsDatabaseConnectionString" + service_bus_connections = ["external"] + env_vars_static = { + ReferenceDataTopicName = "reference-data-updates" + ReferenceDataSubscription = "ReferenceDataUpdater" + SeedDataBlobContainer = "seed-data" + } + } + NemsSubscribe = { name_suffix = "nems-subscribe" function_endpoint_name = "NemsSubscribe" @@ -1326,6 +1339,19 @@ service_bus = { } } } + + external = { + capacity = 1 + sku_tier = "Premium" + max_payload_size = "100mb" + topics = { + reference-data-updates = { + batched_operations_enabled = true + max_delivery_count = 10 + subscribers = ["ReferenceDataUpdater"] + } + } + } } sqlserver = { diff --git a/infrastructure/tf-core/environments/production.tfvars b/infrastructure/tf-core/environments/production.tfvars index 366a1f8b4b..fe501538cf 100644 --- a/infrastructure/tf-core/environments/production.tfvars +++ b/infrastructure/tf-core/environments/production.tfvars @@ -1162,6 +1162,19 @@ function_apps = { } } + ReferenceDataUpdater = { + name_suffix = "reference-data-updater" + function_endpoint_name = "ReferenceDataUpdater" + app_service_plan_key = "NonScaling" + db_connection_string = "DtOsDatabaseConnectionString" + service_bus_connections = ["external"] + env_vars_static = { + ReferenceDataTopicName = "reference-data-updates" + ReferenceDataSubscription = "ReferenceDataUpdater" + SeedDataBlobContainer = "seed-data" + } + } + NemsSubscribe = { name_suffix = "nems-subscribe" function_endpoint_name = "NemsSubscribe" @@ -1347,6 +1360,19 @@ service_bus = { } } } + + external = { + capacity = 1 + sku_tier = "Premium" + max_payload_size = "100mb" + topics = { + reference-data-updates = { + batched_operations_enabled = true + max_delivery_count = 10 + subscribers = ["ReferenceDataUpdater"] + } + } + } } sqlserver = { From 12c9c87688ecb0a6f8f6662f895d0aaa12f2ec58 Mon Sep 17 00:00:00 2001 From: warren Date: Thu, 30 Apr 2026 12:13:33 +0100 Subject: [PATCH 5/8] refactor: improve ReferenceDataInsertHandler reliability and testability - Cache compiled delegates for InsertSingle to avoid per-call reflection - Move environment variable access to constructor fields for fail-fast behaviour - Use SQL error numbers for PK violation detection instead of string matching - Add required/nullable modifiers to ReferenceDataUpdateMessage properties - Add TestCleanup to restore environment variables after tests Co-authored-by: Copilot --- .../Model/ReferenceDataUpdateMessage.cs | 8 +-- .../ReferenceDataInsertHandler.cs | 60 ++++++++++++++----- .../ReferenceDataInsertHandlerTests.cs | 12 ++++ 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs b/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs index 04b880b6f5..404ea39503 100644 --- a/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs +++ b/application/CohortManager/src/Functions/Shared/Model/ReferenceDataUpdateMessage.cs @@ -4,8 +4,8 @@ namespace Model; public class ReferenceDataUpdateMessage { - public string DataType { get; set; } - public JsonElement Data { get; set; } - public string CorrelationId { get; set; } - public DateTime Timestamp { get; set; } + public required string DataType { get; set; } + public required JsonElement Data { get; set; } + public string? CorrelationId { get; set; } + public DateTime Timestamp { get; set; } = DateTime.UtcNow; } diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs index 91341dfc43..11da64f6f2 100644 --- a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs @@ -34,6 +34,9 @@ public class ReferenceDataInsertHandler : IReferenceDataInsertHandler private readonly IServiceProvider _serviceProvider; private readonly IBlobStorageHelper _blobStorageHelper; private readonly ILogger _logger; + private readonly string _storageConnectionString; + private readonly string _seedDataContainerName; + private static readonly ConcurrentDictionary>> _insertDelegates = new(); public ReferenceDataInsertHandler( IServiceProvider serviceProvider, @@ -43,6 +46,9 @@ public ReferenceDataInsertHandler( _serviceProvider = serviceProvider; _blobStorageHelper = blobStorageHelper; _logger = logger; + _storageConnectionString = Environment.GetEnvironmentVariable("AzureWebJobsStorage") + ?? throw new InvalidOperationException("AzureWebJobsStorage environment variable is not set."); + _seedDataContainerName = Environment.GetEnvironmentVariable("SeedDataBlobContainer") ?? "seed-data"; } public async Task ProcessRecord(string dataType, JsonElement data) @@ -73,21 +79,36 @@ private async Task InsertIntoDatabase(string dataType, Type entityType, ob var accessorType = typeof(IDataServiceAccessor<>).MakeGenericType(entityType); var accessor = _serviceProvider.GetRequiredService(accessorType); - var insertMethod = accessorType.GetMethod("InsertSingle")!; - var task = (Task)insertMethod.Invoke(accessor, new[] { entity })!; - var result = await task; + var invoker = _insertDelegates.GetOrAdd(entityType, t => + { + var aType = typeof(IDataServiceAccessor<>).MakeGenericType(t); + var method = aType.GetMethod("InsertSingle")!; + + var accessorParam = System.Linq.Expressions.Expression.Parameter(typeof(object), "a"); + var entityParam = System.Linq.Expressions.Expression.Parameter(typeof(object), "e"); + + var call = System.Linq.Expressions.Expression.Call( + System.Linq.Expressions.Expression.Convert(accessorParam, aType), + method, + System.Linq.Expressions.Expression.Convert(entityParam, t)); + + return System.Linq.Expressions.Expression.Lambda>>( + call, accessorParam, entityParam).Compile(); + }); + + var result = await invoker(accessor, entity); if (!result) { - _logger.LogWarning("InsertSingle returned false for type {DataType}. Record may be a duplicate.", dataType); + _logger.LogWarning("InsertSingle returned false for type {DataType}. Record may not have been inserted, possibly because it already exists.", dataType); } - return true; + return result; } catch (DbUpdateException ex) when (IsPrimaryKeyViolation(ex)) { _logger.LogWarning("Duplicate record detected for type {DataType}. Skipping insert.", dataType); - return true; + return false; } catch (Exception ex) { @@ -98,14 +119,11 @@ private async Task InsertIntoDatabase(string dataType, Type entityType, ob private async Task AppendToBlob(string dataType, string blobFileName, JsonElement newRecord) { - var connectionString = Environment.GetEnvironmentVariable("AzureWebJobsStorage"); - var containerName = Environment.GetEnvironmentVariable("SeedDataBlobContainer") ?? "seed-data"; - try { var existingRecords = new List(); - var existingBlob = await _blobStorageHelper.GetFileFromBlobStorage(connectionString!, containerName, blobFileName); + var existingBlob = await _blobStorageHelper.GetFileFromBlobStorage(_storageConnectionString, _seedDataContainerName, blobFileName); if (existingBlob?.Data != null) { existingBlob.Data.Position = 0; @@ -120,7 +138,7 @@ private async Task AppendToBlob(string dataType, string blobFileName, JsonElemen var bytes = Encoding.UTF8.GetBytes(updatedJson); var blobFile = new BlobFile(bytes, blobFileName); - await _blobStorageHelper.UploadFileToBlobStorage(connectionString!, containerName, blobFile, overwrite: true); + await _blobStorageHelper.UploadFileToBlobStorage(_storageConnectionString, _seedDataContainerName, blobFile, overwrite: true); _logger.LogInformation( "Appended record to blob {BlobFileName} for type {DataType}. Total records: {Count}", @@ -136,8 +154,22 @@ private async Task AppendToBlob(string dataType, string blobFileName, JsonElemen private static bool IsPrimaryKeyViolation(DbUpdateException ex) { - return ex.InnerException?.Message?.Contains("duplicate key", StringComparison.OrdinalIgnoreCase) == true - || ex.InnerException?.Message?.Contains("violation of primary key", StringComparison.OrdinalIgnoreCase) == true - || ex.InnerException?.Message?.Contains("unique constraint", StringComparison.OrdinalIgnoreCase) == true; + const int SqlServerPrimaryKeyViolation = 2627; + const int SqlServerUniqueConstraintViolation = 2601; + + for (Exception? current = ex.InnerException; current is not null; current = current.InnerException) + { + var numberProperty = current.GetType().GetProperty("Number"); + if (numberProperty?.PropertyType == typeof(int)) + { + var errorNumber = (int)numberProperty.GetValue(current)!; + if (errorNumber == SqlServerPrimaryKeyViolation || errorNumber == SqlServerUniqueConstraintViolation) + { + return true; + } + } + } + + return false; } } diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs index e3d4ec83da..170b96f254 100644 --- a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs @@ -17,9 +17,14 @@ public class ReferenceDataInsertHandlerTests private readonly Mock _blobStorageHelperMock = new(); private readonly Mock> _loggerMock = new(); private readonly ReferenceDataInsertHandler _handler; + private readonly string? _originalAzureWebJobsStorage; + private readonly string? _originalSeedDataBlobContainer; public ReferenceDataInsertHandlerTests() { + _originalAzureWebJobsStorage = Environment.GetEnvironmentVariable("AzureWebJobsStorage"); + _originalSeedDataBlobContainer = Environment.GetEnvironmentVariable("SeedDataBlobContainer"); + Environment.SetEnvironmentVariable("AzureWebJobsStorage", "UseDevelopmentStorage=true"); Environment.SetEnvironmentVariable("SeedDataBlobContainer", "seed-data"); @@ -29,6 +34,13 @@ public ReferenceDataInsertHandlerTests() _loggerMock.Object); } + [TestCleanup] + public void Cleanup() + { + Environment.SetEnvironmentVariable("AzureWebJobsStorage", _originalAzureWebJobsStorage); + Environment.SetEnvironmentVariable("SeedDataBlobContainer", _originalSeedDataBlobContainer); + } + private Mock> SetupAccessor(bool insertResult = true) where T : class { var accessorMock = new Mock>(); From 26ee5df99a92dd42bdd44a49f7b752be8ffe3731 Mon Sep 17 00:00:00 2001 From: warren Date: Thu, 30 Apr 2026 12:29:08 +0100 Subject: [PATCH 6/8] chore: pass exception --- .../ReferenceDataUpdater/ReferenceDataInsertHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs index 11da64f6f2..9eadfa5eec 100644 --- a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs @@ -107,7 +107,7 @@ private async Task InsertIntoDatabase(string dataType, Type entityType, ob } catch (DbUpdateException ex) when (IsPrimaryKeyViolation(ex)) { - _logger.LogWarning("Duplicate record detected for type {DataType}. Skipping insert.", dataType); + _logger.LogWarning(ex, "Duplicate record detected for type {DataType}. Skipping insert.", dataType); return false; } catch (Exception ex) From 8357491967770c1d54ea36bc5eeb892c5b3d48b8 Mon Sep 17 00:00:00 2001 From: warren Date: Thu, 30 Apr 2026 12:34:17 +0100 Subject: [PATCH 7/8] fix: sonarqube Co-authored-by: Copilot --- .../screeningDataServices/ReferenceDataUpdater/Dockerfile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile index 4024c99139..ec25afc8a9 100644 --- a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/Dockerfile @@ -5,7 +5,7 @@ COPY ./screeningDataServices/ReferenceDataUpdater /app/src/dotnet-function-app WORKDIR /app/src/dotnet-function-app RUN --mount=type=cache,target=/root/.nuget/packages \ - dotnet publish *.csproj --output /home/site/wwwroot + dotnet publish ./*.csproj --output /home/site/wwwroot # To enable ssh & remote debugging on app service change the base image to the one below # FROM mcr.microsoft.com/azure-functions/dotnet-isolated:4-dotnet-isolated8.0-appservice @@ -14,3 +14,4 @@ ENV AzureWebJobsScriptRoot=/home/site/wwwroot \ AzureFunctionsJobHost__Logging__Console__IsEnabled=true COPY --from=function ["/home/site/wwwroot", "/home/site/wwwroot"] + From 052d0204fc434dd57c42344f4146095ddfa1e0ed Mon Sep 17 00:00:00 2001 From: warren Date: Thu, 30 Apr 2026 12:38:31 +0100 Subject: [PATCH 8/8] feat: missing import nad updated test Co-authored-by: Copilot --- .../ReferenceDataInsertHandler.cs | 1 + .../ReferenceDataInsertHandlerTests.cs | 23 ++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs index 9eadfa5eec..7956288ad9 100644 --- a/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs +++ b/application/CohortManager/src/Functions/screeningDataServices/ReferenceDataUpdater/ReferenceDataInsertHandler.cs @@ -1,5 +1,6 @@ namespace ReferenceDataUpdater; +using System.Collections.Concurrent; using System.Text; using System.Text.Json; using Common; diff --git a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs index 170b96f254..6e2919596e 100644 --- a/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs +++ b/tests/UnitTests/ScreeningDataServicesTests/ReferenceDataUpdaterTests/ReferenceDataInsertHandlerTests.cs @@ -112,11 +112,12 @@ public async Task ProcessRecord_DatabaseInsertThrows_ReturnsFalse() } [TestMethod] - public async Task ProcessRecord_PrimaryKeyViolation_ReturnsTrue() + public async Task ProcessRecord_PrimaryKeyViolation_ReturnsFalse() { // Arrange var data = CreatePayload(new { LanguageCodeId = "EN", LanguageDescription = "English" }); - var dbUpdateException = new DbUpdateException("An error occurred", new Exception("Violation of PRIMARY KEY constraint")); + var sqlException = new SqlExceptionWithNumber(2627); + var dbUpdateException = new DbUpdateException("An error occurred", sqlException); var accessorMock = new Mock>(); accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ThrowsAsync(dbUpdateException); @@ -128,15 +129,16 @@ public async Task ProcessRecord_PrimaryKeyViolation_ReturnsTrue() var result = await _handler.ProcessRecord("LanguageCode", data); // Assert - Assert.IsTrue(result); + Assert.IsFalse(result); } [TestMethod] - public async Task ProcessRecord_UniqueConstraintViolation_ReturnsTrue() + public async Task ProcessRecord_UniqueConstraintViolation_ReturnsFalse() { // Arrange var data = CreatePayload(new { GenderCode = "M" }); - var dbUpdateException = new DbUpdateException("An error occurred", new Exception("unique constraint violation on table")); + var sqlException = new SqlExceptionWithNumber(2601); + var dbUpdateException = new DbUpdateException("An error occurred", sqlException); var accessorMock = new Mock>(); accessorMock.Setup(a => a.InsertSingle(It.IsAny())).ThrowsAsync(dbUpdateException); @@ -148,7 +150,7 @@ public async Task ProcessRecord_UniqueConstraintViolation_ReturnsTrue() var result = await _handler.ProcessRecord("GenderMaster", data); // Assert - Assert.IsTrue(result); + Assert.IsFalse(result); } [TestMethod] @@ -259,7 +261,7 @@ public async Task ProcessRecord_CaseInsensitiveDataType_ReturnsTrue() } [TestMethod] - public async Task ProcessRecord_InsertSingleReturnsFalse_ReturnsTrue() + public async Task ProcessRecord_InsertSingleReturnsFalse_ReturnsFalse() { // Arrange var data = CreatePayload(new { GpPracticeCode = "Y99999" }); @@ -270,6 +272,11 @@ public async Task ProcessRecord_InsertSingleReturnsFalse_ReturnsTrue() var result = await _handler.ProcessRecord("BsSelectGpPractice", data); // Assert - Assert.IsTrue(result); + Assert.IsFalse(result); } } + +internal class SqlExceptionWithNumber(int number) : Exception($"SQL error {number}") +{ + public int Number { get; } = number; +}