diff --git a/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/CPS.ComplexCases.API.HttpTelemetry.Tests.csproj b/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/CPS.ComplexCases.API.HttpTelemetry.Tests.csproj new file mode 100644 index 000000000..c3a6c5207 --- /dev/null +++ b/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/CPS.ComplexCases.API.HttpTelemetry.Tests.csproj @@ -0,0 +1,26 @@ + + + + net10.0 + enable + enable + false + true + + + + + + + + + + + + + + + + + + diff --git a/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/HealthCheckTelemetryHostRegistrationTests.cs b/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/HealthCheckTelemetryHostRegistrationTests.cs new file mode 100644 index 000000000..7c4ccb256 --- /dev/null +++ b/backend/CPS.ComplexCases.API.HttpTelemetry.Tests/HealthCheckTelemetryHostRegistrationTests.cs @@ -0,0 +1,97 @@ +using Microsoft.ApplicationInsights; +using Microsoft.ApplicationInsights.Channel; +using Microsoft.ApplicationInsights.DataContracts; +using Microsoft.ApplicationInsights.Extensibility; +using Microsoft.Extensions.DependencyInjection; + +namespace CPS.ComplexCases.API.HttpTelemetry.Tests; + +public class HealthCheckTelemetryHostRegistrationTests +{ + [Fact] + public void Register_WrapsTelemetryConfiguration_AndDropsStatusHostTelemetry() + { + var channel = new StubChannel(); + var services = new ServiceCollection(); + services.AddSingleton(_ => new TelemetryConfiguration { TelemetryChannel = channel }); + + HealthCheckTelemetryHostRegistration.Register(services); + + var config = services.BuildServiceProvider().GetRequiredService(); + var client = new TelemetryClient(config); + + var statusRequest = new RequestTelemetry + { + Name = "Status", + Url = new Uri("https://example.com/api/status") + }; + var invokeDependency = new DependencyTelemetry + { + Name = "Invoke", + Type = "InProc" + }; + invokeDependency.Context.Operation.Name = "Functions.Status"; + + client.TrackRequest(statusRequest); + client.Track(invokeDependency); + client.TrackRequest(new RequestTelemetry + { + Name = "GetCase", + Url = new Uri("https://example.com/api/v1/cases/1") + }); + client.Flush(); + + var remaining = Assert.Single(channel.Items); + var remainingRequest = Assert.IsType(remaining); + Assert.Equal("GetCase", remainingRequest.Name); + } + + [Fact] + public void Register_AddsTelemetryModule_WhenConfigurationIsNotRegistered() + { + var services = new ServiceCollection(); + + HealthCheckTelemetryHostRegistration.Register(services); + + Assert.Contains(services, descriptor => + descriptor.ServiceType == typeof(ITelemetryModule) + && descriptor.ImplementationType == typeof(HealthCheckTelemetryModule)); + } + + [Fact] + public void Module_Initialize_DropsStatusRequestsAfterBuild() + { + var channel = new StubChannel(); + var config = new TelemetryConfiguration { TelemetryChannel = channel }; + + new HealthCheckTelemetryModule().Initialize(config); + config.TelemetryProcessorChainBuilder.Build(); + + var client = new TelemetryClient(config); + client.TrackRequest(new RequestTelemetry + { + Name = "Status", + Url = new Uri("https://example.com/api/status") + }); + client.TrackRequest(new RequestTelemetry + { + Name = "GetCase", + Url = new Uri("https://example.com/api/v1/cases/1") + }); + client.Flush(); + + var remaining = Assert.Single(channel.Items); + var remainingRequest = Assert.IsType(remaining); + Assert.Equal("GetCase", remainingRequest.Name); + } + + private sealed class StubChannel : ITelemetryChannel + { + public List Items { get; } = []; + public bool? DeveloperMode { get; set; } + public string? EndpointAddress { get; set; } + public void Send(ITelemetry item) => Items.Add(item); + public void Flush() { } + public void Dispose() { } + } +} diff --git a/backend/CPS.ComplexCases.API.HttpTelemetry/CPS.ComplexCases.API.HttpTelemetry.csproj b/backend/CPS.ComplexCases.API.HttpTelemetry/CPS.ComplexCases.API.HttpTelemetry.csproj index 7f48d1917..c55658fde 100644 --- a/backend/CPS.ComplexCases.API.HttpTelemetry/CPS.ComplexCases.API.HttpTelemetry.csproj +++ b/backend/CPS.ComplexCases.API.HttpTelemetry/CPS.ComplexCases.API.HttpTelemetry.csproj @@ -2,11 +2,12 @@ - 1.0.41 + 1.0.42 net8.0;net10.0 enable enable + $(DefineConstants);HOST_TELEMETRY True Brian Dunnington @@ -26,5 +27,11 @@ + + + + <_Parameter1>CPS.ComplexCases.API.HttpTelemetry.Tests + + - \ No newline at end of file + diff --git a/backend/CPS.ComplexCases.API.HttpTelemetry/HealthCheckTelemetryHostRegistration.cs b/backend/CPS.ComplexCases.API.HttpTelemetry/HealthCheckTelemetryHostRegistration.cs new file mode 100644 index 000000000..48a4c5589 --- /dev/null +++ b/backend/CPS.ComplexCases.API.HttpTelemetry/HealthCheckTelemetryHostRegistration.cs @@ -0,0 +1,47 @@ +using Microsoft.ApplicationInsights.Extensibility; +using Microsoft.Extensions.DependencyInjection; + +namespace CPS.ComplexCases.API.HttpTelemetry; + +internal static class HealthCheckTelemetryHostRegistration +{ + public static void Register(IServiceCollection services) + { + var configDescriptor = services.FirstOrDefault(descriptor => descriptor.ServiceType == typeof(TelemetryConfiguration)); + if (configDescriptor?.ImplementationFactory is not null) + { + var implFactory = configDescriptor.ImplementationFactory; + services.Remove(configDescriptor); + services.AddSingleton(provider => + { + var config = (TelemetryConfiguration)implFactory(provider); + Attach(config); + return config; + }); + return; + } + + if (configDescriptor?.ImplementationInstance is TelemetryConfiguration instance) + { + Attach(instance); + return; + } + + services.AddSingleton(); + } + + internal static void Attach(TelemetryConfiguration config) + { + config.TelemetryProcessorChainBuilder + .Use(next => new HealthCheckTelemetryFilter(next)) + .Build(); + } +} + +internal sealed class HealthCheckTelemetryModule : ITelemetryModule +{ + public void Initialize(TelemetryConfiguration configuration) + { + configuration.TelemetryProcessorChainBuilder.Use(next => new HealthCheckTelemetryFilter(next)); + } +} diff --git a/backend/CPS.ComplexCases.API.HttpTelemetry/Startup.cs b/backend/CPS.ComplexCases.API.HttpTelemetry/Startup.cs index 77bbc3420..bf5339906 100644 --- a/backend/CPS.ComplexCases.API.HttpTelemetry/Startup.cs +++ b/backend/CPS.ComplexCases.API.HttpTelemetry/Startup.cs @@ -24,6 +24,8 @@ public void Configure(WebJobsBuilderContext context, IWebJobsBuilder builder) builder.AddExtension(); builder.Services.AddSingleton(); builder.Services.AddHttpContextAccessor(); + // Runs in the Functions host process. The worker ITelemetryProcessor cannot see host Request/Invoke telemetry. + HealthCheckTelemetryHostRegistration.Register(builder.Services); } } diff --git a/backend/CPS.ComplexCases.API/Attributes/HttpTelemetryAttribute.cs b/backend/CPS.ComplexCases.API/Attributes/HttpTelemetryAttribute.cs index 9feab45ad..872753133 100644 --- a/backend/CPS.ComplexCases.API/Attributes/HttpTelemetryAttribute.cs +++ b/backend/CPS.ComplexCases.API/Attributes/HttpTelemetryAttribute.cs @@ -1,6 +1,6 @@ using Microsoft.Azure.Functions.Worker.Extensions.Abstractions; -[assembly: ExtensionInformation("CPS.ComplexCases.API.HttpTelemetry", "1.0.41")] +[assembly: ExtensionInformation("CPS.ComplexCases.API.HttpTelemetry", "1.0.42")] namespace CPS.ComplexCases.API.Attributes { [AttributeUsage(AttributeTargets.Parameter)] diff --git a/backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj b/backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj index 63daceccd..2f9727258 100644 --- a/backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj +++ b/backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj @@ -16,8 +16,8 @@ - - + + @@ -33,7 +33,7 @@ - + @@ -68,4 +68,4 @@ - \ No newline at end of file + diff --git a/backend/CPS.ComplexCases.Common.Tests/Unit/Telemetry/HealthCheckTelemetryFilterTests.cs b/backend/CPS.ComplexCases.Common.Tests/Unit/Telemetry/HealthCheckTelemetryFilterTests.cs index 0107319ab..9d7e3f8d0 100644 --- a/backend/CPS.ComplexCases.Common.Tests/Unit/Telemetry/HealthCheckTelemetryFilterTests.cs +++ b/backend/CPS.ComplexCases.Common.Tests/Unit/Telemetry/HealthCheckTelemetryFilterTests.cs @@ -16,11 +16,15 @@ public HealthCheckTelemetryFilterTests() _sut = new HealthCheckTelemetryFilter(_next.Object); } - [Fact] - public void Process_FiltersRequest_WhenFunctionNameIsStatus() + [Theory] + [InlineData("Status")] + [InlineData("Functions.Status")] + [InlineData("functions.status")] + [InlineData(" Functions.Status ")] + public void Process_FiltersRequest_WhenFunctionNameIsStatus(string functionName) { var request = new RequestTelemetry { Name = "GetCase" }; - request.Properties["AzureFunctions_FunctionName"] = "Status"; + request.Properties["AzureFunctions_FunctionName"] = functionName; AssertFiltered(request); } @@ -58,6 +62,7 @@ public void Process_FiltersRequest_WhenUrlIsPeriodicHealthEndpoint(string url) [Theory] [InlineData("Status")] + [InlineData("Functions.Status")] [InlineData("GET /api/status")] [InlineData("GET /api/health")] public void Process_FiltersRequest_WhenNameIsPeriodicHealthEndpoint(string name) @@ -83,6 +88,8 @@ public void Process_FiltersDependency_WhenDataIsPeriodicHealthEndpoint(string da } [Theory] + [InlineData("Status")] + [InlineData("Functions.Status")] [InlineData("GET /api/status")] [InlineData("GET /api/health")] [InlineData("/api/status")] @@ -109,6 +116,7 @@ public void Process_FiltersDependency_WhenTargetIsPeriodicHealthEndpoint() [Theory] [InlineData("Status")] + [InlineData("Functions.Status")] [InlineData("GET /api/status")] [InlineData("GET /api/health")] public void Process_FiltersDependency_WhenOperationNameIsPeriodicHealthEndpoint(string operationName) @@ -139,6 +147,15 @@ public void Process_DoesNotFilterRequest_WhenPathIsTransferStatus() AssertPassedThrough(request); } + [Fact] + public void Process_DoesNotFilterRequest_WhenFunctionNameIsPrefixedTransferStatus() + { + var request = new RequestTelemetry { Name = "GetTransferStatus" }; + request.Properties["AzureFunctions_FunctionName"] = "Functions.GetTransferStatus"; + + AssertPassedThrough(request); + } + [Fact] public void Process_DoesNotFilterDependency_WhenDataIsTransferStatus() { @@ -154,6 +171,16 @@ public void Process_DoesNotFilterDependency_WhenDataIsTransferStatus() AssertPassedThrough(dependency); } + [Fact] + public void Process_DoesNotFilterDependency_WhenNameIsInvokeWithoutHealthOperation() + { + AssertPassedThrough(new DependencyTelemetry + { + Name = "Invoke", + Type = "InProc" + }); + } + [Fact] public void Process_DoesNotFilterDependency_WhenUnrelatedToHealthEndpoints() { diff --git a/backend/CPS.ComplexCases.Common/Telemetry/HealthCheckTelemetryFilter.cs b/backend/CPS.ComplexCases.Common/Telemetry/HealthCheckTelemetryFilter.cs index bd957e567..ccfa47d73 100644 --- a/backend/CPS.ComplexCases.Common/Telemetry/HealthCheckTelemetryFilter.cs +++ b/backend/CPS.ComplexCases.Common/Telemetry/HealthCheckTelemetryFilter.cs @@ -2,7 +2,11 @@ using Microsoft.ApplicationInsights.DataContracts; using Microsoft.ApplicationInsights.Extensibility; +#if HOST_TELEMETRY +namespace CPS.ComplexCases.API.HttpTelemetry; +#else namespace CPS.ComplexCases.Common.Telemetry; +#endif public class HealthCheckTelemetryFilter(ITelemetryProcessor next) : ITelemetryProcessor { @@ -13,6 +17,7 @@ public class HealthCheckTelemetryFilter(ITelemetryProcessor next) : ITelemetryPr ]; private const string StatusFunctionName = "Status"; + private const string FunctionsPrefix = "Functions."; private readonly ITelemetryProcessor _next = next; @@ -58,15 +63,28 @@ private static bool ShouldFilter(ITelemetry item) if (item is DependencyTelemetry dependency) { return IsPeriodicHealthProbeValue(dependency.Data) - || IsPeriodicHealthProbeValue(dependency.Name) + || IsPeriodicHealthOperation(dependency.Name) || IsPeriodicHealthProbeValue(dependency.Target); } return false; } - private static bool IsPeriodicHealthFunctionName(string? name) => - string.Equals(name, StatusFunctionName, StringComparison.OrdinalIgnoreCase); + private static bool IsPeriodicHealthFunctionName(string? name) + { + if (string.IsNullOrWhiteSpace(name)) + { + return false; + } + + var trimmed = name.Trim(); + if (trimmed.StartsWith(FunctionsPrefix, StringComparison.OrdinalIgnoreCase)) + { + trimmed = trimmed[FunctionsPrefix.Length..]; + } + + return string.Equals(trimmed, StatusFunctionName, StringComparison.OrdinalIgnoreCase); + } private static bool IsPeriodicHealthOperation(string? name) => IsPeriodicHealthFunctionName(name) || IsPeriodicHealthProbeValue(name); diff --git a/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs b/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs index c42ff5576..e7c0f05d1 100644 --- a/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs +++ b/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs @@ -367,13 +367,12 @@ public async Task DeleteFilesAsync_WithCodeZeroAndNullFileId_PopulatesDeletedFil var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); Assert.True(result.AllSuccessful); - Assert.Equal(2, result.DeletedFiles!.Count); Assert.Equal(["file1.txt", "file2.txt"], result.DeletedFiles); Assert.Empty(result.FailedFiles!); } [Fact] - public async Task DeleteFilesAsync_WithCodeZeroAndNullFileIdAndFilename_UsesDeletedFallback() + public async Task DeleteFilesAsync_WithCodeZeroAndNullFileIdAndFilename_UsesRequestedFileIds() { var workspaceId = _fixture.Create(); var token = _fixture.Create(); @@ -397,7 +396,8 @@ public async Task DeleteFilesAsync_WithCodeZeroAndNullFileIdAndFilename_UsesDele var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); - Assert.Equal(["deleted"], result.DeletedFiles); + Assert.True(result.AllSuccessful); + Assert.Equal(["file-1"], result.DeletedFiles); Assert.Empty(result.FailedFiles!); } @@ -741,6 +741,112 @@ public async Task DeleteFilesAsync_WhenEgressOmitsFiles_ReturnsNotAllSuccessful( Assert.Empty(result.FailedFiles!); } + [Fact] + public async Task DeleteFilesAsync_WhenEgressReturnsEmptyResultsForUnknownId_DoesNotTreatAsDeleted() + { + var workspaceId = _fixture.Create(); + var token = _fixture.Create(); + var filesToDelete = new List + { + new() { Path = "folder/missing.txt", FileId = "000000000000000000000000" } + }; + + SetupTokenRequest(token); + SetupDeleteFilesRequest(workspaceId, token); + SetupHttpMockResponses( + ("token", new GetWorkspaceTokenResponse { Token = token }), + ("delete", new { all_successful = true, results = Array.Empty() })); + + var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); + + Assert.False(result.AllSuccessful); + Assert.Empty(result.DeletedFiles!); + Assert.Empty(result.FailedFiles!); + } + + [Fact] + public async Task DeleteFilesAsync_WhenEgressReturnsEmptyFilesForUnknownId_DoesNotTreatAsDeleted() + { + var workspaceId = _fixture.Create(); + var token = _fixture.Create(); + var filesToDelete = new List + { + new() { Path = "folder/missing.txt", FileId = "000000000000000000000000" } + }; + + SetupTokenRequest(token); + SetupDeleteFilesRequest(workspaceId, token); + SetupHttpMockResponses( + ("token", new GetWorkspaceTokenResponse { Token = token }), + ("delete", new { all_successful = true, files = Array.Empty() })); + + var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); + + Assert.False(result.AllSuccessful); + Assert.Empty(result.DeletedFiles!); + Assert.Empty(result.FailedFiles!); + } + + [Fact] + public async Task DeleteFilesAsync_WhenEgressReturnsFilesWithId_PopulatesDeletedFiles() + { + var workspaceId = _fixture.Create(); + var token = _fixture.Create(); + var filesToDelete = new List + { + new() { Path = "1. ABEs for Transcript/Free_Test_Data_10.5MB_PDF.pdf", FileId = "6a7b09840b11b5e3185286b7" } + }; + + SetupTokenRequest(token); + SetupDeleteFilesRequest(workspaceId, token); + SetupHttpMockResponses( + ("token", new GetWorkspaceTokenResponse { Token = token }), + ("delete", new + { + all_successful = true, + files = new[] + { + new { code = 0, id = "6a7b09840b11b5e3185286b7", filename = "Free_Test_Data_10.5MB_PDF.pdf", is_folder = false } + } + })); + + var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); + + Assert.True(result.AllSuccessful); + Assert.Equal(["6a7b09840b11b5e3185286b7"], result.DeletedFiles); + Assert.Empty(result.FailedFiles!); + } + + [Fact] + public async Task DeleteFilesAsync_WhenEgressReturnsResultsWithId_PopulatesDeletedFiles() + { + var workspaceId = _fixture.Create(); + var token = _fixture.Create(); + var filesToDelete = new List + { + new() { Path = "4. Served Evidence/1000mb.txt", FileId = "6a7b09840b11b5e3185286b7" } + }; + + SetupTokenRequest(token); + SetupDeleteFilesRequest(workspaceId, token); + SetupHttpMockResponses( + ("token", new GetWorkspaceTokenResponse { Token = token }), + ("delete", new + { + all_successful = true, + results = new[] + { + new { code = 0, id = "6a7b09840b11b5e3185286b7", filename = "1000mb.txt", is_folder = false } + } + })); + + var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); + + Assert.True(result.AllSuccessful); + Assert.Equal(["6a7b09840b11b5e3185286b7"], result.DeletedFiles); + Assert.Empty(result.FailedFiles!); + } + [Fact] public async Task DeleteFilesAsync_WhenEgressReportsNotAllSuccessful_ReturnsNotAllSuccessful() { diff --git a/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs b/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs index 5475856c3..679673e5f 100644 --- a/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs +++ b/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs @@ -1,4 +1,5 @@ using System.Net; +using System.Text.Json; using CPS.ComplexCases.Common.Extensions; using CPS.ComplexCases.Common.Models.Domain; using CPS.ComplexCases.Common.Models.Domain.Dtos; @@ -272,21 +273,51 @@ public async Task DeleteFilesAsync(List fi FileIds = [.. chunk] }; - var result = await SendRequestAsync(_egressRequestFactory.DeleteFilesRequest(deleteArg, token)); + using var response = await SendRequestAsync(_egressRequestFactory.DeleteFilesRequest(deleteArg, token)); + var responseContent = await response.Content.ReadAsStringAsync(); + var result = JsonSerializer.Deserialize(responseContent) + ?? throw new InvalidOperationException("Deserialization returned null."); allSuccessful &= result.AllSuccessful; - var files = result.Files ?? []; + var files = result.Items ?? []; + if (files.Count == 0) + { + _logger.LogWarning( + "Egress bulk delete returned no per-file confirmations for workspace {WorkspaceId}. AllSuccessful={AllSuccessful}, RequestedCount={RequestedCount}, Response={Response}", + workspaceId, + result.AllSuccessful, + chunk.Length, + responseContent); + } - deletedFiles.AddRange(files - .Where(x => x.Code == 0) - .Select(x => x.FileId ?? x.Filename ?? "deleted")); + var failedResults = files.Where(x => x.Code > 0).ToList(); + var successfulResults = files.Where(x => x.Code == 0).ToList(); - failedFiles.AddRange(files.Where(x => x.Code > 0).Select(x => new FailedFileDeletion + failedFiles.AddRange(failedResults.Select(x => new FailedFileDeletion { - FileId = x.FileId ?? x.Filename ?? string.Empty, + FileId = x.ResolvedFileId ?? string.Empty, Filename = x.Filename ?? string.Empty, Reason = GetDeleteFailureReason(x) })); + + var identifiedDeleted = successfulResults + .Select(x => x.ResolvedFileId) + .Where(id => !string.IsNullOrEmpty(id)) + .Select(id => id!) + .ToList(); + deletedFiles.AddRange(identifiedDeleted); + + // A code-0 file/result entry with no id still counts as a confirmed delete. Pair leftover + // requested ids to those unidentified successes. An empty files/results list does not — + // AllSuccessful with no per-file rows must not be treated as deleted + var unidentifiedSuccessCount = successfulResults.Count - identifiedDeleted.Count; + if (unidentifiedSuccessCount > 0) + { + var remainingRequestedIds = chunk.Where(id => + !identifiedDeleted.Contains(id, StringComparer.OrdinalIgnoreCase) && + failedResults.All(f => !id.Equals(f.ResolvedFileId, StringComparison.OrdinalIgnoreCase))); + deletedFiles.AddRange(remainingRequestedIds.Take(unidentifiedSuccessCount)); + } } return new DeleteFilesResult diff --git a/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs b/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs index dfa7d158c..09239ab37 100644 --- a/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs +++ b/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs @@ -8,7 +8,11 @@ public class DeleteFilesResponse public bool AllSuccessful { get; set; } [JsonPropertyName("files")] public List Files { get; set; } = []; + [JsonPropertyName("results")] + public List Results { get; set; } = []; + [JsonIgnore] + public List Items => Results.Count > 0 ? Results : Files; } public class DeletedFileResult @@ -21,6 +25,16 @@ public class DeletedFileResult public string? Filename { get; set; } [JsonPropertyName("file_id")] public string? FileId { get; set; } + // Egress list/document APIs identify files as `id`. Bulk delete responses + // use the same field; `file_id` is kept for compatibility if it is sent. + [JsonPropertyName("id")] + public string? Id { get; set; } [JsonPropertyName("is_folder")] public bool IsFolder { get; set; } -} \ No newline at end of file + + [JsonIgnore] + public string? ResolvedFileId => + !string.IsNullOrWhiteSpace(FileId) ? FileId + : !string.IsNullOrWhiteSpace(Id) ? Id + : Filename; +} diff --git a/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/DeleteFilesTests.cs b/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/DeleteFilesTests.cs index b6d9847ce..6ce7aff88 100644 --- a/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/DeleteFilesTests.cs +++ b/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/DeleteFilesTests.cs @@ -389,6 +389,32 @@ public async Task Run_RecordsAllFilesAsDeletionErrors_WhenDeletedFilesIsEmpty() Times.Once); } + [Fact] + public async Task Run_RecordsAllFilesAsDeletionErrors_WhenDeletedFilesIsEmptyAndNotAllSuccessful() + { + var payload = CreateEgressToNetAppPayload(); + var items = CreateCompletedItems(("file1.txt", "f1"), ("file2.txt", "f2")); + SetupDeleteRun(payload, items, new DeleteFilesResult + { + AllSuccessful = false, + DeletedFiles = [], + FailedFiles = [] + }); + + await _activity.Run(payload, _durableTaskClientStub, CancellationToken.None); + + _transferEntityHelperMock.Verify( + c => c.DeleteMovedItemsCompleted( + It.IsAny(), + payload.TransferId, + It.Is>(errors => + errors.Count == 2 && + errors.Any(e => e.FileId == "f1") && + errors.Any(e => e.FileId == "f2")), + It.IsAny()), + Times.Once); + } + [Fact] public async Task Run_RecordsMissingFilesAsDeletionErrors_WhenDeletedFilesCountIsLessThanRequested() { diff --git a/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/UpdateActivityLogTests.cs b/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/UpdateActivityLogTests.cs index 3019f5e53..1e59019a8 100644 --- a/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/UpdateActivityLogTests.cs +++ b/backend/CPS.ComplexCases.FileTransfer.API.Tests/Unit/Durable/Activity/UpdateActivityLogTests.cs @@ -441,6 +441,69 @@ public async Task Run_IncludesDeletionErrors_WhenTransferTypeIsMoveAndDirectionI ), Times.Once); } + [Fact] + public async Task Run_MapsDeletionErrorFileIdToSourcePath_WhenSuccessfulItemExists() + { + var transferId = Guid.NewGuid(); + var userName = _fixture.Create(); + var caseId = _fixture.Create(); + const string fileId = "6a7b09840b11b5e3185286b7"; + const string sourcePath = "1. ABEs for Transcript/Free_Test_Data_10.5MB_PDF.pdf"; + + var entityState = new TransferEntity + { + Id = transferId, + CaseId = caseId, + Direction = TransferDirection.EgressToNetApp, + TransferType = TransferType.Move, + TotalFiles = 1, + BearerToken = _bearerToken, + SourcePaths = [new TransferSourcePath { FullFilePath = sourcePath, Path = sourcePath }], + SuccessfulItems = + [ + new TransferItem + { + SourcePath = sourcePath, + FileId = fileId, + Size = 11081517, + IsRenamed = false, + Status = TransferItemStatus.Completed + } + ], + DeletionErrors = + [ + new DeletionError { FileId = fileId, ErrorMessage = "File was not confirmed deleted by Egress." } + ], + DestinationPath = "/dest/path", + }; + + _durableEntityClientStub.OnGetEntityAsync = (_, _) => + Task.FromResult?>(new EntityMetadata( + new EntityInstanceId("TransferEntity", transferId.ToString()), + entityState + )); + + var payload = new UpdateActivityLogPayload + { + TransferId = transferId.ToString(), + ActionType = ActionType.TransferCompleted, + UserName = userName + }; + + await _activity.Run(payload, _durableTaskClientStub); + + _activityLogServiceMock.Verify(service => service.CreateActivityLogAsync( + payload.ActionType, + ResourceType.FileTransfer, + caseId, + entityState.Id.ToString(), + entityState.Direction.ToString(), + userName, + It.Is(doc => + doc.RootElement.GetProperty("errors")[0].GetProperty("path").GetString() == sourcePath) + ), Times.Once); + } + [Fact] public async Task Run_DoesNotIncludeDeletionErrors_WhenTransferTypeIsCopy() { diff --git a/backend/CPS.ComplexCases.FileTransfer.API/Attributes/HttpTelemetryExtension.cs b/backend/CPS.ComplexCases.FileTransfer.API/Attributes/HttpTelemetryExtension.cs new file mode 100644 index 000000000..36333686c --- /dev/null +++ b/backend/CPS.ComplexCases.FileTransfer.API/Attributes/HttpTelemetryExtension.cs @@ -0,0 +1,5 @@ +using Microsoft.Azure.Functions.Worker.Extensions.Abstractions; + +// Load the host extension so HealthCheckTelemetryFilter can drop /api/status telemetry +// emitted by the Functions host (worker-side processors never see those items). +[assembly: ExtensionInformation("CPS.ComplexCases.API.HttpTelemetry", "1.0.42")] diff --git a/backend/CPS.ComplexCases.FileTransfer.API/CPS.ComplexCases.FileTransfer.API.csproj b/backend/CPS.ComplexCases.FileTransfer.API/CPS.ComplexCases.FileTransfer.API.csproj index d7ae3d700..b67299351 100644 --- a/backend/CPS.ComplexCases.FileTransfer.API/CPS.ComplexCases.FileTransfer.API.csproj +++ b/backend/CPS.ComplexCases.FileTransfer.API/CPS.ComplexCases.FileTransfer.API.csproj @@ -11,6 +11,7 @@ + @@ -43,4 +44,4 @@ Never - \ No newline at end of file + diff --git a/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/DeleteFiles.cs b/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/DeleteFiles.cs index b2710d0b3..bc91514b7 100644 --- a/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/DeleteFiles.cs +++ b/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/DeleteFiles.cs @@ -79,10 +79,13 @@ public async Task Run([ActivityTrigger] DeleteFilesPayload? payload, [DurableCli if (deletionErrors.Count != 0) { _logger.LogWarning( - "Failed to delete {FailedCount} of {RequestedCount} files for transfer ID {TransferId}.", + "Failed to delete {FailedCount} of {RequestedCount} files for transfer ID {TransferId}. AllSuccessful={AllSuccessful}, DeletedIdentifiers={DeletedCount}, FailedIdentifiers={FailedApiCount}.", deletionErrors.Count, filesToDelete.Count, - payload.TransferId); + payload.TransferId, + result.AllSuccessful, + (result.DeletedFiles ?? []).Count, + (result.FailedFiles ?? []).Count); } else { @@ -153,7 +156,15 @@ private static List BuildDeletionErrors(List f } } - foreach (var file in filesToDelete.Where(file => !IsAccountedFor(file, accountedIdentifiers))) + var unaccountedFiles = filesToDelete.Where(file => !IsAccountedFor(file, accountedIdentifiers)).ToList(); + if (unaccountedFiles.Count == 0) + { + return deletionErrors; + } + + // compare confirmed deletes against the requested count. AllSuccessful with + // no DeletedFiles (for example an unknown file id) must still record DeletionErrors. + foreach (var file in unaccountedFiles) { deletionErrors.Add(new DeletionError { diff --git a/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/UpdateActivityLog.cs b/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/UpdateActivityLog.cs index c574a4947..277f9e5b7 100644 --- a/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/UpdateActivityLog.cs +++ b/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/UpdateActivityLog.cs @@ -116,7 +116,7 @@ public async Task Run([ActivityTrigger] UpdateActivityLogPayload payload, [Durab { deletionErrors = entity.State.DeletionErrors.Select(x => new FileTransferError { - Path = x.FileId, + Path = ResolveDeletionErrorPath(entity.State.SuccessfulItems, x.FileId), ErrorMessage = x.ErrorMessage }).ToList(); errorItems.AddRange(deletionErrors); @@ -170,4 +170,13 @@ private static string PrependNetappRootFolder(string netappRootFolderPath, strin return $"{root}/{relative}"; } + + private static string ResolveDeletionErrorPath(IEnumerable successfulItems, string fileId) + { + var matchingItem = successfulItems.FirstOrDefault(item => + !string.IsNullOrEmpty(item.FileId) && + item.FileId.Equals(fileId, StringComparison.OrdinalIgnoreCase)); + + return matchingItem?.SourcePath ?? fileId; + } } diff --git a/backend/CPS.ComplexCases.sln b/backend/CPS.ComplexCases.sln index b724ccd30..6d2e8bdb9 100644 --- a/backend/CPS.ComplexCases.sln +++ b/backend/CPS.ComplexCases.sln @@ -49,6 +49,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Cps.ComplexCases.Data.Tests EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CPS.ComplexCases.API.Integration.Tests", "CPS.ComplexCases.API.Integration.Tests\CPS.ComplexCases.API.Integration.Tests.csproj", "{CDF9B98D-75FB-4F39-9AAD-AE3F7281153B}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CPS.ComplexCases.API.HttpTelemetry.Tests", "CPS.ComplexCases.API.HttpTelemetry.Tests\CPS.ComplexCases.API.HttpTelemetry.Tests.csproj", "{ABB60364-BC8B-42DA-8DA5-734C6380501B}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -335,6 +337,18 @@ Global {CDF9B98D-75FB-4F39-9AAD-AE3F7281153B}.Release|x64.Build.0 = Release|Any CPU {CDF9B98D-75FB-4F39-9AAD-AE3F7281153B}.Release|x86.ActiveCfg = Release|Any CPU {CDF9B98D-75FB-4F39-9AAD-AE3F7281153B}.Release|x86.Build.0 = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|Any CPU.Build.0 = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|x64.ActiveCfg = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|x64.Build.0 = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|x86.ActiveCfg = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Debug|x86.Build.0 = Debug|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|Any CPU.ActiveCfg = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|Any CPU.Build.0 = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|x64.ActiveCfg = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|x64.Build.0 = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|x86.ActiveCfg = Release|Any CPU + {ABB60364-BC8B-42DA-8DA5-734C6380501B}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/devops-pipelines/backend/backend-pr-build-and-test.yml b/devops-pipelines/backend/backend-pr-build-and-test.yml index 060ce4342..1cbb3fdaa 100644 --- a/devops-pipelines/backend/backend-pr-build-and-test.yml +++ b/devops-pipelines/backend/backend-pr-build-and-test.yml @@ -75,63 +75,3 @@ stages: runTests: true checkFormatting: true testResultsDirectory: $(testResultsDirectory) - - - ${{ if or(eq(parameters.buildMainApi, true), eq(parameters.buildDB, true)) }}: - - job: Build_Main_API_And_DB - displayName: "Build Main API & Data Project" - dependsOn: Build_And_Test_Solution - steps: - # Build and Main API - also required for generating database migration script - - template: ../templates/dotnet-build-steps.yml - parameters: - projectPath: "backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj" - projectName: "lacc-main-api-drop" - buildConfiguration: $(buildConfiguration) - dotNetVersion: $(dotNetVersion) - useLocalNuGet: true - publishOutput: false - runTests: false - - - ${{ if eq(parameters.buildDB, true) }}: - # Build Data project with dependencies - - task: DotNetCoreCLI@2 - displayName: "Build Database Project" - inputs: - command: "build" - projects: "backend/CPS.ComplexCases.Data/CPS.ComplexCases.Data.csproj" - arguments: "--configuration $(buildConfiguration)" - - - task: Bash@3 - displayName: "Generate Migration Scripts" - inputs: - targetType: "inline" - script: | - export PATH="$PATH:$HOME/.dotnet/tools" - - if ! dotnet ef --version &> /dev/null; then - dotnet tool install --global dotnet-ef --version $(efVersion) - fi - - dotnet ef migrations script \ - --idempotent \ - --no-build \ - --configuration $(buildConfiguration) \ - --output $(Build.ArtifactStagingDirectory)/migration-script.sql \ - --project backend/CPS.ComplexCases.Data/CPS.ComplexCases.Data.csproj \ - --startup-project backend/CPS.ComplexCases.API/CPS.ComplexCases.API.csproj - - - ${{ if eq(parameters.buildFiletransfer, true) }}: - - job: Build_FileTransfer_API - displayName: "Build FileTransfer API" - dependsOn: Build_And_Test_Solution - steps: - # Build and publish FileTransfer API - - template: ../templates/dotnet-build-steps.yml - parameters: - projectPath: "backend/CPS.ComplexCases.FileTransfer.API/CPS.ComplexCases.FileTransfer.API.csproj" - projectName: "lacc-filetransfer-api-drop" - buildConfiguration: $(buildConfiguration) - dotNetVersion: $(dotNetVersion) - useLocalNuGet: false - publishOutput: false - runTests: false diff --git a/ui-spa/playwright/integration-tests/case-search.spec.ts b/ui-spa/playwright/integration-tests/case-search.spec.ts index f592b0f60..ab6949c68 100644 --- a/ui-spa/playwright/integration-tests/case-search.spec.ts +++ b/ui-spa/playwright/integration-tests/case-search.spec.ts @@ -316,11 +316,12 @@ test.describe("Case Search/Results", async () => { ).not.toBeVisible(); await expect(page.getByText("Loading...")).not.toBeVisible(); await expect(page).toHaveURL("/unauthorised"); - await expect(page.locator("h1")).toHaveText("Unauthorised"); - await expect(page.getByText("Your session has timed out.")).toBeVisible(); + await expect(page.locator("h1")).toHaveText( + "You cannot access this service", + ); await expect( page.getByText( - "Please close this tab and re-launch the homepage screen from CMS Classic.", + "Close this tab and open the homepage again from CMS Classic.", ), ).toBeVisible(); }); @@ -669,11 +670,12 @@ test.describe("Case Search/Results", async () => { await expect(page.getByText("Loading...")).toBeVisible(); await expect(page.getByText("Loading...")).not.toBeVisible(); await expect(page).toHaveURL("/unauthorised"); - await expect(page.locator("h1")).toHaveText("Unauthorised"); - await expect(page.getByText("Your session has timed out.")).toBeVisible(); + await expect(page.locator("h1")).toHaveText( + "You cannot access this service", + ); await expect( page.getByText( - "Please close this tab and re-launch the homepage screen from CMS Classic.", + "Close this tab and open the homepage again from CMS Classic.", ), ).toBeVisible(); }); diff --git a/ui-spa/src/components/unauthorised/index.tsx b/ui-spa/src/components/unauthorised/index.tsx index cdef74156..453f1385f 100644 --- a/ui-spa/src/components/unauthorised/index.tsx +++ b/ui-spa/src/components/unauthorised/index.tsx @@ -1,12 +1,8 @@ const UnAuthorisedPage = () => { return (
-

Unauthorised

-

Your session has timed out.

-

- Please close this tab and re-launch the homepage screen from CMS - Classic. -

+

You cannot access this service

+

Close this tab and open the homepage again from CMS Classic.

); };