Skip to content

Commit 58f68ce

Browse files
committed
PM-31923 fixing delete scenario with orphaned db record
1 parent 8126bc3 commit 58f68ce

File tree

2 files changed

+87
-1
lines changed

2 files changed

+87
-1
lines changed

src/Api/Dirt/Controllers/OrganizationReportsController.cs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,16 @@ await _cache.RemoveByTagAsync(
337337

338338
if (fileData != null && !string.IsNullOrEmpty(fileData.Id))
339339
{
340-
await _storageService.DeleteReportFilesAsync(report, fileData.Id);
340+
try
341+
{
342+
await _storageService.DeleteReportFilesAsync(report, fileData.Id);
343+
}
344+
catch (Exception ex)
345+
{
346+
_logger.LogWarning(ex,
347+
"Failed to delete storage files for report {ReportId}, file {FileId}. Manual cleanup may be required.",
348+
reportId, fileData.Id);
349+
}
341350
}
342351
}
343352

@@ -365,6 +374,11 @@ public async Task<OrganizationReportFileResponseModel> RenewFileUploadUrlAsync(
365374
throw new BadRequestException("ReportId is required.");
366375
}
367376

377+
if (string.IsNullOrEmpty(reportFileId))
378+
{
379+
throw new BadRequestException("ReportFileId is required.");
380+
}
381+
368382
await AuthorizeAsync(organizationId);
369383

370384
var report = await _organizationReportRepo.GetByIdAsync(reportId);

test/Api.Test/Dirt/OrganizationReportsControllerTests.cs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
using Bit.Test.Common.AutoFixture;
1818
using Bit.Test.Common.AutoFixture.Attributes;
1919
using Microsoft.AspNetCore.Mvc;
20+
using Microsoft.Extensions.Logging;
2021
using NSubstitute;
22+
using NSubstitute.ExceptionExtensions;
2123
using Xunit;
2224
using ZiggyCreatures.Caching.Fusion;
2325

@@ -672,6 +674,76 @@ await Assert.ThrowsAsync<NotFoundException>(() =>
672674
sutProvider.Sut.RenewFileUploadUrlAsync(orgId, report.Id, "wrong-file-id"));
673675
}
674676

677+
[Theory, BitAutoData]
678+
public async Task RenewFileUploadUrlAsync_NullReportFileId_ThrowsBadRequestException(
679+
SutProvider<OrganizationReportsController> sutProvider,
680+
Guid orgId,
681+
Guid reportId)
682+
{
683+
// Act & Assert
684+
var exception = await Assert.ThrowsAsync<BadRequestException>(() =>
685+
sutProvider.Sut.RenewFileUploadUrlAsync(orgId, reportId, null));
686+
687+
Assert.Equal("ReportFileId is required.", exception.Message);
688+
}
689+
690+
[Theory, BitAutoData]
691+
public async Task RenewFileUploadUrlAsync_EmptyReportFileId_ThrowsBadRequestException(
692+
SutProvider<OrganizationReportsController> sutProvider,
693+
Guid orgId,
694+
Guid reportId)
695+
{
696+
// Act & Assert
697+
var exception = await Assert.ThrowsAsync<BadRequestException>(() =>
698+
sutProvider.Sut.RenewFileUploadUrlAsync(orgId, reportId, string.Empty));
699+
700+
Assert.Equal("ReportFileId is required.", exception.Message);
701+
}
702+
703+
[Theory, BitAutoData]
704+
public async Task DeleteOrganizationReportAsync_StorageFailure_StillCompletesWithoutThrowing(
705+
SutProvider<OrganizationReportsController> sutProvider,
706+
Guid orgId,
707+
OrganizationReport report)
708+
{
709+
// Arrange
710+
var reportFile = new ReportFile { Id = "file-id", FileName = "report.json", Size = 1024, Validated = true };
711+
report.OrganizationId = orgId;
712+
report.SetReportFile(reportFile);
713+
714+
SetupAuthorization(sutProvider, orgId);
715+
716+
sutProvider.GetDependency<IOrganizationReportRepository>()
717+
.GetByIdAsync(report.Id)
718+
.Returns(report);
719+
720+
sutProvider.GetDependency<IOrganizationReportStorageService>()
721+
.DeleteReportFilesAsync(report, "file-id")
722+
.ThrowsAsync(new Exception("Azure storage unavailable"));
723+
724+
// Act — should not throw despite storage failure
725+
await sutProvider.Sut.DeleteOrganizationReportAsync(orgId, report.Id);
726+
727+
// Assert — DB delete and cache invalidation still happened
728+
await sutProvider.GetDependency<IOrganizationReportRepository>()
729+
.Received(1)
730+
.DeleteAsync(report);
731+
732+
await sutProvider.GetDependency<IFusionCache>()
733+
.Received(1)
734+
.RemoveByTagAsync(
735+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(orgId));
736+
737+
sutProvider.GetDependency<ILogger<OrganizationReportsController>>()
738+
.Received(1)
739+
.Log(
740+
LogLevel.Warning,
741+
Arg.Any<EventId>(),
742+
Arg.Any<object>(),
743+
Arg.Any<Exception>(),
744+
Arg.Any<Func<object, Exception?, string>>());
745+
}
746+
675747
// UpdateOrganizationReportAsync - V1 (flag off)
676748

677749
[Theory, BitAutoData]

0 commit comments

Comments
 (0)