diff --git a/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs b/backend/CPS.ComplexCases.Egress.Tests/Unit/EgressStorageClientTests.cs index c42ff5576..8b6720287 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.Equal(["file-1", "file-2"], 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!); } @@ -711,7 +711,7 @@ public async Task DeleteFilesAsync_WithOnlyNullOrWhitespaceFileIds_ReturnsEmptyR } [Fact] - public async Task DeleteFilesAsync_WhenEgressOmitsFiles_ReturnsNotAllSuccessful() + public async Task DeleteFilesAsync_WhenEgressOmitsFiles_TreatsRequestedIdsAsDeleted() { var workspaceId = _fixture.Create(); var token = _fixture.Create(); @@ -736,8 +736,61 @@ public async Task DeleteFilesAsync_WhenEgressOmitsFiles_ReturnsNotAllSuccessful( var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId); - Assert.False(result.AllSuccessful); - Assert.Equal(["file-1"], result.DeletedFiles); + Assert.True(result.AllSuccessful); + Assert.Equal(["file-1", "file-2"], result.DeletedFiles); + Assert.Empty(result.FailedFiles!); + } + + [Fact] + public async Task DeleteFilesAsync_WhenEgressOmitsFilesArray_TreatsRequestedIdsAsDeleted() + { + var workspaceId = _fixture.Create(); + var token = _fixture.Create(); + var filesToDelete = new List + { + new() { Path = "folder/file1.txt", FileId = "6a7b09840b11b5e3185286b7" } + }; + + SetupTokenRequest(token); + SetupDeleteFilesRequest(workspaceId, token); + SetupHttpMockResponses( + ("token", new GetWorkspaceTokenResponse { Token = token }), + ("delete", new { all_successful = true })); + + 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_WhenEgressReturnsIdInsteadOfFileId_TreatsRequestedIdsAsDeleted() + { + 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!); } diff --git a/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs b/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs index 5475856c3..2184924ad 100644 --- a/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs +++ b/backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs @@ -273,20 +273,33 @@ public async Task DeleteFilesAsync(List fi }; var result = await SendRequestAsync(_egressRequestFactory.DeleteFilesRequest(deleteArg, token)); - allSuccessful &= result.AllSuccessful; - var files = result.Files ?? []; + var failedResults = files.Where(x => x.Code > 0).ToList(); - deletedFiles.AddRange(files - .Where(x => x.Code == 0) - .Select(x => x.FileId ?? x.Filename ?? "deleted")); - - 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) })); + + if (result.AllSuccessful && failedResults.Count == 0) + { + // Egress often returns HTTP 200 / all_successful without per-file + // `file_id` values (it uses `id`, or omits the files array). Trust + // that overall success and treat the requested IDs as deleted so + // move transfers are not marked PartiallyCompleted. + deletedFiles.AddRange(chunk); + continue; + } + + allSuccessful &= result.AllSuccessful; + + deletedFiles.AddRange(files + .Where(x => x.Code == 0) + .Select(x => x.ResolvedFileId) + .Where(id => !string.IsNullOrEmpty(id)) + .Select(id => id!)); } 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..5bcac2bbd 100644 --- a/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs +++ b/backend/CPS.ComplexCases.Egress/Models/Response/DeleteFilesResponse.cs @@ -21,6 +21,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..e8e81bcfd 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 @@ -356,7 +356,7 @@ public async Task Run_UsesDeletedFilesCount_WhenDeletedFilesAreReturned() } [Fact] - public async Task Run_RecordsAllFilesAsDeletionErrors_WhenDeletedFilesIsEmpty() + public async Task Run_DoesNotRecordDeletionErrors_WhenDeletedFilesIsEmptyAndAllSuccessful() { var payload = CreateEgressToNetAppPayload(); var items = CreateCompletedItems(("file1.txt", "f1"), ("file2.txt", "f2")); @@ -373,19 +373,42 @@ public async Task Run_RecordsAllFilesAsDeletionErrors_WhenDeletedFilesIsEmpty() 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.Is>(errors => errors.Count == 0), It.IsAny()), Times.Once); _telemetryClientMock.Verify( t => t.TrackEvent(It.Is(e => - e.TotalFilesDeleted == 0 && - e.TotalFilesFailedToDelete == 2 && - !e.IsSuccessful && - e.FailureReasons == "File was not confirmed deleted by Egress. (2)")), + e.TotalFilesDeleted == 2 && + e.TotalFilesFailedToDelete == 0 && + e.IsSuccessful && + string.IsNullOrEmpty(e.FailureReasons))), + 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); } @@ -406,7 +429,7 @@ public async Task Run_RecordsMissingFilesAsDeletionErrors_WhenDeletedFilesCountI SetupDeleteRun(payload, items, new DeleteFilesResult { - AllSuccessful = true, + AllSuccessful = false, DeletedFiles = items.Take(10).Select(x => x.FileId!).ToList(), FailedFiles = [] }); @@ -477,7 +500,7 @@ public async Task Run_RecordsEveryUnmatchedFile_WhenDeletedIdentifiersDoNotMatch var items = CreateCompletedItems(("file1.txt", "f1"), ("file2.txt", "f2"), ("file3.txt", "f3")); SetupDeleteRun(payload, items, new DeleteFilesResult { - AllSuccessful = true, + AllSuccessful = false, DeletedFiles = ["deleted", "f2"], FailedFiles = [] }); 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/Durable/Activity/DeleteFiles.cs b/backend/CPS.ComplexCases.FileTransfer.API/Durable/Activity/DeleteFiles.cs index b2710d0b3..9f3c4361e 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,21 @@ 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; + } + + // Egress confirms overall success (all_successful / no failed files) without + // returning identifiers we can match. Do not treat that as a source-delete + // failure — that incorrectly marks a successful move as PartiallyCompleted. + if (failedFiles.Count == 0 && result.AllSuccessful) + { + return 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; + } }