Skip to content

Commit 4141d6a

Browse files
committed
PM-31923 fixing code based on PR comments
1 parent a070867 commit 4141d6a

4 files changed

Lines changed: 63 additions & 13 deletions

File tree

src/Api/Dirt/Controllers/OrganizationReportsController.cs

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Bit.Core.Utilities;
1515
using Microsoft.AspNetCore.Authorization;
1616
using Microsoft.AspNetCore.Mvc;
17+
using ZiggyCreatures.Caching.Fusion;
1718

1819
namespace Bit.Api.Dirt.Controllers;
1920

@@ -38,6 +39,7 @@ public class OrganizationReportsController : Controller
3839
private readonly IUpdateOrganizationReportV2Command _updateReportV2Command;
3940
private readonly IValidateOrganizationReportFileCommand _validateCommand;
4041
private readonly ILogger<OrganizationReportsController> _logger;
42+
private readonly IFusionCache _cache;
4143

4244
public OrganizationReportsController(
4345
ICurrentContext currentContext,
@@ -56,7 +58,8 @@ public OrganizationReportsController(
5658
IOrganizationReportRepository organizationReportRepo,
5759
IUpdateOrganizationReportV2Command updateReportV2Command,
5860
IValidateOrganizationReportFileCommand validateCommand,
59-
ILogger<OrganizationReportsController> logger)
61+
ILogger<OrganizationReportsController> logger,
62+
[FromKeyedServices(OrganizationReportCacheConstants.CacheName)] IFusionCache cache)
6063
{
6164
_currentContext = currentContext;
6265
_getOrganizationReportQuery = getOrganizationReportQuery;
@@ -75,6 +78,7 @@ public OrganizationReportsController(
7578
_updateReportV2Command = updateReportV2Command;
7679
_validateCommand = validateCommand;
7780
_logger = logger;
81+
_cache = cache;
7882
}
7983

8084
/// <summary>
@@ -133,7 +137,6 @@ public async Task<IActionResult> CreateOrganizationReportAsync(
133137
/// <summary>
134138
/// Gets the most recent organization report for the specified organization.
135139
/// Includes a presigned download URL for the report file if one has been validated.
136-
/// If no file is associated, the response contains inline ReportData.
137140
/// </summary>
138141
/// <param name="organizationId">The unique identifier of the organization.</param>
139142
/// <returns>An <see cref="OrganizationReportResponseModel"/> for the most recent report.</returns>
@@ -156,15 +159,13 @@ public async Task<IActionResult> GetLatestOrganizationReportAsync(Guid organizat
156159

157160
var response = new OrganizationReportResponseModel(latestReport);
158161

159-
var fileData = latestReport.GetReportFile();
160-
if (fileData == null)
161-
{
162-
return Ok(response);
163-
}
164-
165-
if (fileData.Validated)
162+
if (_featureService.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2))
166163
{
167-
response.ReportFileDownloadUrl = await _storageService.GetReportDataDownloadUrlAsync(latestReport, fileData);
164+
var fileData = latestReport.GetReportFile();
165+
if (fileData is { Validated: true })
166+
{
167+
response.ReportFileDownloadUrl = await _storageService.GetReportDataDownloadUrlAsync(latestReport, fileData);
168+
}
168169
}
169170

170171
return Ok(response);
@@ -174,7 +175,6 @@ public async Task<IActionResult> GetLatestOrganizationReportAsync(Guid organizat
174175
/// Gets a specific organization report by its report ID.
175176
/// Validates that the report belongs to the specified organization.
176177
/// Includes a presigned download URL for the report file if one has been validated.
177-
/// If no file is associated, the response contains inline ReportData.
178178
/// </summary>
179179
/// <param name="organizationId">The unique identifier of the organization.</param>
180180
/// <param name="reportId">The unique identifier of the report to retrieve.</param>
@@ -227,7 +227,6 @@ public async Task<IActionResult> GetOrganizationReportAsync(Guid organizationId,
227227
/// <param name="request">The request model containing updated report data.</param>
228228
/// <returns>An <see cref="OrganizationReportResponseModel"/> with the updated report.</returns>
229229
[HttpPatch("{organizationId}/{reportId}")]
230-
[RequestSizeLimit(Constants.FileSize501mb)]
231230
public async Task<IActionResult> UpdateOrganizationReportAsync(
232231
Guid organizationId,
233232
Guid reportId,
@@ -333,6 +332,9 @@ public async Task DeleteOrganizationReportAsync(Guid organizationId, Guid report
333332

334333
await _organizationReportRepo.DeleteAsync(report);
335334

335+
await _cache.RemoveByTagAsync(
336+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(organizationId));
337+
336338
if (fileData != null && !string.IsNullOrEmpty(fileData.Id))
337339
{
338340
await _storageService.DeleteReportFilesAsync(report, fileData.Id);
@@ -497,6 +499,10 @@ await Request.GetFileAsync(async (stream) =>
497499
var (valid, length) = await _storageService.ValidateFileAsync(report, fileData, minimum, maximum);
498500
if (!valid)
499501
{
502+
await _storageService.DeleteReportFilesAsync(report, fileData.Id!);
503+
await _organizationReportRepo.DeleteAsync(report);
504+
await _cache.RemoveByTagAsync(
505+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(organizationId));
500506
throw new BadRequestException("File received does not match expected constraints.");
501507
}
502508

@@ -505,6 +511,8 @@ await Request.GetFileAsync(async (stream) =>
505511
report.SetReportFile(fileData);
506512
report.RevisionDate = DateTime.UtcNow;
507513
await _organizationReportRepo.ReplaceAsync(report);
514+
await _cache.RemoveByTagAsync(
515+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(organizationId));
508516
}
509517

510518
/// <summary>

src/Core/Dirt/Reports/ReportFeatures/ValidateOrganizationReportFileCommand.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22
using Bit.Core.Dirt.Reports.ReportFeatures.Interfaces;
33
using Bit.Core.Dirt.Reports.Services;
44
using Bit.Core.Dirt.Repositories;
5+
using Bit.Core.Utilities;
6+
using Microsoft.Extensions.DependencyInjection;
57
using Microsoft.Extensions.Logging;
8+
using ZiggyCreatures.Caching.Fusion;
69

710
namespace Bit.Core.Dirt.Reports.ReportFeatures;
811

@@ -11,15 +14,18 @@ public class ValidateOrganizationReportFileCommand : IValidateOrganizationReport
1114
private readonly IOrganizationReportRepository _organizationReportRepo;
1215
private readonly IOrganizationReportStorageService _storageService;
1316
private readonly ILogger<ValidateOrganizationReportFileCommand> _logger;
17+
private readonly IFusionCache _cache;
1418

1519
public ValidateOrganizationReportFileCommand(
1620
IOrganizationReportRepository organizationReportRepo,
1721
IOrganizationReportStorageService storageService,
18-
ILogger<ValidateOrganizationReportFileCommand> logger)
22+
ILogger<ValidateOrganizationReportFileCommand> logger,
23+
[FromKeyedServices(OrganizationReportCacheConstants.CacheName)] IFusionCache cache)
1924
{
2025
_organizationReportRepo = organizationReportRepo;
2126
_storageService = storageService;
2227
_logger = logger;
28+
_cache = cache;
2329
}
2430

2531
public async Task<bool> ValidateAsync(OrganizationReport report, string reportFileId)
@@ -45,6 +51,8 @@ public async Task<bool> ValidateAsync(OrganizationReport report, string reportFi
4551
report.Id, length);
4652
await _storageService.DeleteReportFilesAsync(report, reportFileId);
4753
await _organizationReportRepo.DeleteAsync(report);
54+
await _cache.RemoveByTagAsync(
55+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(report.OrganizationId));
4856
return false;
4957
}
5058

@@ -53,6 +61,8 @@ public async Task<bool> ValidateAsync(OrganizationReport report, string reportFi
5361
report.SetReportFile(fileData);
5462
report.RevisionDate = DateTime.UtcNow;
5563
await _organizationReportRepo.ReplaceAsync(report);
64+
await _cache.RemoveByTagAsync(
65+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(report.OrganizationId));
5666
return true;
5767
}
5868
}

test/Api.Test/Dirt/OrganizationReportsControllerTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313
using Bit.Core.Exceptions;
1414
using Bit.Core.Models.Data.Organizations;
1515
using Bit.Core.Services;
16+
using Bit.Core.Utilities;
1617
using Bit.Test.Common.AutoFixture;
1718
using Bit.Test.Common.AutoFixture.Attributes;
1819
using Microsoft.AspNetCore.Mvc;
1920
using NSubstitute;
2021
using Xunit;
22+
using ZiggyCreatures.Caching.Fusion;
2123

2224
namespace Bit.Api.Test.Dirt;
2325

@@ -40,6 +42,10 @@ public async Task GetLatestOrganizationReportAsync_WithValidatedFile_ReturnsOkWi
4042

4143
SetupAuthorization(sutProvider, orgId);
4244

45+
sutProvider.GetDependency<IFeatureService>()
46+
.IsEnabled(FeatureFlagKeys.AccessIntelligenceVersion2)
47+
.Returns(true);
48+
4349
sutProvider.GetDependency<IGetOrganizationReportQuery>()
4450
.GetLatestOrganizationReportAsync(orgId)
4551
.Returns(expectedReport);
@@ -415,6 +421,11 @@ await sutProvider.GetDependency<IOrganizationReportRepository>()
415421
await sutProvider.GetDependency<IOrganizationReportStorageService>()
416422
.Received(1)
417423
.DeleteReportFilesAsync(report, "file-id");
424+
425+
await sutProvider.GetDependency<IFusionCache>()
426+
.Received(1)
427+
.RemoveByTagAsync(
428+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(orgId));
418429
}
419430

420431
[Theory, BitAutoData]
@@ -444,6 +455,11 @@ await sutProvider.GetDependency<IOrganizationReportRepository>()
444455
await sutProvider.GetDependency<IOrganizationReportStorageService>()
445456
.DidNotReceive()
446457
.DeleteReportFilesAsync(Arg.Any<OrganizationReport>(), Arg.Any<string>());
458+
459+
await sutProvider.GetDependency<IFusionCache>()
460+
.Received(1)
461+
.RemoveByTagAsync(
462+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(orgId));
447463
}
448464

449465
[Theory, BitAutoData]

test/Core.Test/Dirt/ReportFeatures/ValidateOrganizationReportFileCommandTests.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@
33
using Bit.Core.Dirt.Reports.ReportFeatures;
44
using Bit.Core.Dirt.Reports.Services;
55
using Bit.Core.Dirt.Repositories;
6+
using Bit.Core.Utilities;
67
using Bit.Test.Common.AutoFixture;
78
using Bit.Test.Common.AutoFixture.Attributes;
89
using NSubstitute;
910
using Xunit;
11+
using ZiggyCreatures.Caching.Fusion;
1012

1113
namespace Bit.Core.Test.Dirt.ReportFeatures;
1214

@@ -71,6 +73,11 @@ await sutProvider.GetDependency<IOrganizationReportStorageService>()
7173
await sutProvider.GetDependency<IOrganizationReportRepository>()
7274
.DidNotReceive()
7375
.DeleteAsync(Arg.Any<OrganizationReport>());
76+
77+
await sutProvider.GetDependency<IFusionCache>()
78+
.Received(1)
79+
.RemoveByTagAsync(
80+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(organizationId));
7481
}
7582

7683
[Theory]
@@ -105,6 +112,11 @@ await sutProvider.GetDependency<IOrganizationReportRepository>()
105112
await sutProvider.GetDependency<IOrganizationReportRepository>()
106113
.DidNotReceive()
107114
.ReplaceAsync(Arg.Any<OrganizationReport>());
115+
116+
await sutProvider.GetDependency<IFusionCache>()
117+
.Received(1)
118+
.RemoveByTagAsync(
119+
OrganizationReportCacheConstants.BuildCacheTagForOrganizationReports(organizationId));
108120
}
109121

110122
[Theory]
@@ -139,6 +151,10 @@ await sutProvider.GetDependency<IOrganizationReportRepository>()
139151
await sutProvider.GetDependency<IOrganizationReportRepository>()
140152
.DidNotReceive()
141153
.ReplaceAsync(Arg.Any<OrganizationReport>());
154+
155+
await sutProvider.GetDependency<IFusionCache>()
156+
.DidNotReceive()
157+
.RemoveByTagAsync(Arg.Any<string>());
142158
}
143159

144160
[Theory]

0 commit comments

Comments
 (0)