Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ public async Task DeleteFilesAsync_WithCodeZeroAndNullFileId_PopulatesDeletedFil
var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId);

Assert.True(result.AllSuccessful);
Assert.Equal(["file-1", "file-2"], result.DeletedFiles);
Assert.Equal(["file1.txt", "file2.txt"], result.DeletedFiles);
Assert.Empty(result.FailedFiles!);
}

Expand Down Expand Up @@ -711,7 +711,7 @@ public async Task DeleteFilesAsync_WithOnlyNullOrWhitespaceFileIds_ReturnsEmptyR
}

[Fact]
public async Task DeleteFilesAsync_WhenEgressOmitsFiles_TreatsRequestedIdsAsDeleted()
public async Task DeleteFilesAsync_WhenEgressOmitsFiles_ReturnsNotAllSuccessful()
{
var workspaceId = _fixture.Create<string>();
var token = _fixture.Create<string>();
Expand All @@ -736,36 +736,59 @@ public async Task DeleteFilesAsync_WhenEgressOmitsFiles_TreatsRequestedIdsAsDele

var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId);

Assert.True(result.AllSuccessful);
Assert.Equal(["file-1", "file-2"], result.DeletedFiles);
Assert.False(result.AllSuccessful);
Assert.Equal(["file-1"], result.DeletedFiles);
Assert.Empty(result.FailedFiles!);
}

[Fact]
public async Task DeleteFilesAsync_WhenEgressOmitsFilesArray_TreatsRequestedIdsAsDeleted()
public async Task DeleteFilesAsync_WhenEgressReturnsEmptyResultsForUnknownId_DoesNotTreatAsDeleted()
{
var workspaceId = _fixture.Create<string>();
var token = _fixture.Create<string>();
var filesToDelete = new List<DeletionEntityDto>
{
new() { Path = "folder/file1.txt", FileId = "6a7b09840b11b5e3185286b7" }
new() { Path = "folder/missing.txt", FileId = "000000000000000000000000" }
};

SetupTokenRequest(token);
SetupDeleteFilesRequest(workspaceId, token);
SetupHttpMockResponses(
("token", new GetWorkspaceTokenResponse { Token = token }),
("delete", new { all_successful = true }));
("delete", new { all_successful = true, results = Array.Empty<object>() }));

var result = await _client.DeleteFilesAsync(filesToDelete, workspaceId);

Assert.True(result.AllSuccessful);
Assert.Equal(["6a7b09840b11b5e3185286b7"], result.DeletedFiles);
Assert.False(result.AllSuccessful);
Assert.Empty(result.DeletedFiles!);
Assert.Empty(result.FailedFiles!);
}

[Fact]
public async Task DeleteFilesAsync_WhenEgressReturnsIdInsteadOfFileId_TreatsRequestedIdsAsDeleted()
public async Task DeleteFilesAsync_WhenEgressReturnsEmptyFilesForUnknownId_DoesNotTreatAsDeleted()
{
var workspaceId = _fixture.Create<string>();
var token = _fixture.Create<string>();
var filesToDelete = new List<DeletionEntityDto>
{
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<object>() }));

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<string>();
var token = _fixture.Create<string>();
Expand Down Expand Up @@ -794,6 +817,36 @@ public async Task DeleteFilesAsync_WhenEgressReturnsIdInsteadOfFileId_TreatsRequ
Assert.Empty(result.FailedFiles!);
}

[Fact]
public async Task DeleteFilesAsync_WhenEgressReturnsResultsWithId_PopulatesDeletedFiles()
{
var workspaceId = _fixture.Create<string>();
var token = _fixture.Create<string>();
var filesToDelete = new List<DeletionEntityDto>
{
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()
{
Expand Down
52 changes: 35 additions & 17 deletions backend/CPS.ComplexCases.Egress/Client/EgressStorageClient.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -272,9 +273,25 @@ public async Task<DeleteFilesResult> DeleteFilesAsync(List<DeletionEntityDto> fi
FileIds = [.. chunk]
};

var result = await SendRequestAsync<DeleteFilesResponse>(_egressRequestFactory.DeleteFilesRequest(deleteArg, token));
var files = result.Files ?? [];
using var response = await SendRequestAsync(_egressRequestFactory.DeleteFilesRequest(deleteArg, token));
var responseContent = await response.Content.ReadAsStringAsync();
var result = JsonSerializer.Deserialize<DeleteFilesResponse>(responseContent)
?? throw new InvalidOperationException("Deserialization returned null.");
allSuccessful &= result.AllSuccessful;

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);
}

var failedResults = files.Where(x => x.Code > 0).ToList();
var successfulResults = files.Where(x => x.Code == 0).ToList();

failedFiles.AddRange(failedResults.Select(x => new FailedFileDeletion
{
Expand All @@ -283,23 +300,24 @@ public async Task<DeleteFilesResult> DeleteFilesAsync(List<DeletionEntityDto> fi
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)
var identifiedDeleted = successfulResults
.Select(x => x.ResolvedFileId)
.Where(id => !string.IsNullOrEmpty(id))
.Select(id => 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ public class DeleteFilesResponse
public bool AllSuccessful { get; set; }
[JsonPropertyName("files")]
public List<DeletedFileResult> Files { get; set; } = [];
[JsonPropertyName("results")]
public List<DeletedFileResult> Results { get; set; } = [];

[JsonIgnore]
public List<DeletedFileResult> Items => Results.Count > 0 ? Results : Files;
}

public class DeletedFileResult
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ public async Task Run_UsesDeletedFilesCount_WhenDeletedFilesAreReturned()
}

[Fact]
public async Task Run_DoesNotRecordDeletionErrors_WhenDeletedFilesIsEmptyAndAllSuccessful()
public async Task Run_RecordsAllFilesAsDeletionErrors_WhenDeletedFilesIsEmpty()
{
var payload = CreateEgressToNetAppPayload();
var items = CreateCompletedItems(("file1.txt", "f1"), ("file2.txt", "f2"));
Expand All @@ -373,16 +373,19 @@ public async Task Run_DoesNotRecordDeletionErrors_WhenDeletedFilesIsEmptyAndAllS
c => c.DeleteMovedItemsCompleted(
It.IsAny<DurableTaskClient>(),
payload.TransferId,
It.Is<List<DeletionError>>(errors => errors.Count == 0),
It.Is<List<DeletionError>>(errors =>
errors.Count == 2 &&
errors.Any(e => e.FileId == "f1") &&
errors.Any(e => e.FileId == "f2")),
It.IsAny<CancellationToken>()),
Times.Once);

_telemetryClientMock.Verify(
t => t.TrackEvent(It.Is<FilesDeletedEvent>(e =>
e.TotalFilesDeleted == 2 &&
e.TotalFilesFailedToDelete == 0 &&
e.IsSuccessful &&
string.IsNullOrEmpty(e.FailureReasons))),
e.TotalFilesDeleted == 0 &&
e.TotalFilesFailedToDelete == 2 &&
!e.IsSuccessful &&
e.FailureReasons == "File was not confirmed deleted by Egress. (2)")),
Times.Once);
}

Expand Down Expand Up @@ -429,7 +432,7 @@ public async Task Run_RecordsMissingFilesAsDeletionErrors_WhenDeletedFilesCountI

SetupDeleteRun(payload, items, new DeleteFilesResult
{
AllSuccessful = false,
AllSuccessful = true,
DeletedFiles = items.Take(10).Select(x => x.FileId!).ToList(),
FailedFiles = []
});
Expand Down Expand Up @@ -500,7 +503,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 = false,
AllSuccessful = true,
DeletedFiles = ["deleted", "f2"],
FailedFiles = []
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,8 @@ private static List<DeletionError> BuildDeletionErrors(List<DeletionEntityDto> f
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;
}

// 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
Expand Down
Loading