Skip to content

Commit 2e99c6d

Browse files
LEGLINK-820: Delete reporting plans as a set rather than a row at a time
DeleteAllAsync and DeleteForFacilityAsync loaded every matching plan into the change tracker and issued one DELETE per row. The table grows by a row per facility, measure mapping and month, so once the sync stories populate it those two endpoints would hold the whole table in memory and send a statement per row. The repository gains ExecuteDeleteAsync, which deletes by predicate in a single statement and returns the number of rows removed. It does not load or track the rows, so it takes no part in a pending SaveChanges - which suits these two actions, where the delete is the whole request. DeleteForFacilityAsync now logs the rows the database actually removed instead of a count taken before the delete. The manager tests assert that GetAllAsync, FindAsync and Remove are no longer called, so the load-then-delete pattern cannot return unnoticed, and that the by-facility predicate matches only that facility rather than clearing the table. Update also compares the two ids after the same normalization. The url id was sanitized and the body id was not, so a client sending the identical raw string in both places could be told they did not match. The module registers AddHttpClient itself. Its fallback facility lookup resolves IHttpClientFactory, which happened to work only because the Tenant host asks for one; a host that did not would fail to resolve the service. Claude-Session: https://claude.ai/code/session_0135KKpfA98BKzyQAsGS5kmg
1 parent 8f4ab51 commit 2e99c6d

6 files changed

Lines changed: 44 additions & 48 deletions

File tree

DotNet/DMRP/Business/Managers/FacilityReportingPlanManager.cs

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,11 @@ public async Task DeleteAsync(string id, CancellationToken cancellationToken = d
142142
await _repository.SaveChangesAsync(cancellationToken);
143143
}
144144

145-
public async Task<int> DeleteAllAsync(CancellationToken cancellationToken = default)
145+
public Task<int> DeleteAllAsync(CancellationToken cancellationToken = default)
146146
{
147147
using Activity? activity = ServiceActivitySource.Instance.StartActivity("Delete All Facility Reporting Plans");
148148

149-
var existing = await _repository.GetAllAsync(cancellationToken);
150-
151-
return await RemoveRangeAsync(existing, cancellationToken);
149+
return _repository.ExecuteDeleteAsync(_ => true, cancellationToken);
152150
}
153151

154152
public async Task<int> DeleteForFacilityAsync(string facilityId, CancellationToken cancellationToken = default)
@@ -160,29 +158,12 @@ public async Task<int> DeleteForFacilityAsync(string facilityId, CancellationTok
160158
throw new ApplicationException("FacilityId is required.");
161159
}
162160

163-
var existing = await _repository.FindAsync(p => p.FacilityId == facilityId, cancellationToken);
164-
165-
_logger.LogInformation("Removing {Count} reporting plan(s) for facility {FacilityId}",
166-
existing.Count, facilityId.SanitizeForLog());
167-
168-
return await RemoveRangeAsync(existing, cancellationToken);
169-
}
170-
171-
private async Task<int> RemoveRangeAsync(List<FacilityReportingPlan> plans, CancellationToken cancellationToken)
172-
{
173-
if (plans.Count == 0)
174-
{
175-
return 0;
176-
}
177-
178-
foreach (var plan in plans)
179-
{
180-
_repository.Remove(plan);
181-
}
161+
var removed = await _repository.ExecuteDeleteAsync(p => p.FacilityId == facilityId, cancellationToken);
182162

183-
await _repository.SaveChangesAsync(cancellationToken);
163+
_logger.LogInformation("Removed {Count} reporting plan(s) for facility {FacilityId}",
164+
removed, facilityId.SanitizeForLog());
184165

185-
return plans.Count;
166+
return removed;
186167
}
187168

188169
/// <summary>

DotNet/DMRP/Controllers/FacilityReportingPlansController.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,14 @@ public async Task<IActionResult> UpdateFacilityReportingPlan(string id, Facility
221221
return BadRequest("A facility reporting plan is required.");
222222
}
223223

224-
if (string.IsNullOrWhiteSpace(request.Id))
224+
var requestId = request.Id?.Sanitize();
225+
226+
if (string.IsNullOrWhiteSpace(requestId))
225227
{
226228
return BadRequest("Id is required in the request body.");
227229
}
228230

229-
if (request.Id != id)
231+
if (requestId != id)
230232
{
231233
return BadRequest("Id in the URL must match the Id in the request body.");
232234
}

DotNet/DMRP/DependencyInjection/DmrpModuleExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ private static void AddFacilityVerification(WebApplicationBuilder builder)
7070
}
7171
});
7272

73+
builder.Services.AddHttpClient();
7374
builder.Services.TryAddTransient<ITenantApiService, TenantApiService>();
7475
builder.Services.TryAddScoped<IFacilityExistence, TenantApiFacilityExistence>();
7576
}

DotNet/ServiceTests/UnitTests/DMRP/FacilityReportingPlanManagerTests.cs

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -321,46 +321,43 @@ public async Task DeleteAsync_ExistingId_RemovesTheRow()
321321
[Fact]
322322
public async Task DeleteAllAsync_RemovesEveryRowAndReportsHowMany()
323323
{
324-
var plans = new List<FacilityReportingPlan> { ValidPlan(), ValidPlan(), ValidPlan() };
325-
326-
_mockRepository.Setup(r => r.GetAllAsync(It.IsAny<CancellationToken>()))
327-
.ReturnsAsync(plans);
324+
_mockRepository.Setup(r => r.ExecuteDeleteAsync(It.IsAny<Expression<Func<FacilityReportingPlan, bool>>>(), It.IsAny<CancellationToken>()))
325+
.ReturnsAsync(3);
328326

329327
var removed = await _manager.DeleteAllAsync();
330328

331329
Assert.Equal(3, removed);
332-
foreach (var plan in plans)
333-
{
334-
_mockRepository.Verify(r => r.Remove(plan), Times.Once);
335-
}
336-
337-
_mockRepository.Verify(r => r.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once);
330+
_mockRepository.Verify(r => r.GetAllAsync(It.IsAny<CancellationToken>()), Times.Never);
331+
_mockRepository.Verify(r => r.Remove(It.IsAny<FacilityReportingPlan>()), Times.Never);
338332
}
339333

340334
[Fact]
341-
public async Task DeleteAllAsync_EmptyTable_TouchesNothing()
335+
public async Task DeleteAllAsync_EmptyTable_ReportsNothingRemoved()
342336
{
343-
_mockRepository.Setup(r => r.GetAllAsync(It.IsAny<CancellationToken>()))
344-
.ReturnsAsync(new List<FacilityReportingPlan>());
337+
_mockRepository.Setup(r => r.ExecuteDeleteAsync(It.IsAny<Expression<Func<FacilityReportingPlan, bool>>>(), It.IsAny<CancellationToken>()))
338+
.ReturnsAsync(0);
345339

346-
var removed = await _manager.DeleteAllAsync();
347-
348-
Assert.Equal(0, removed);
349-
_mockRepository.Verify(r => r.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Never);
340+
Assert.Equal(0, await _manager.DeleteAllAsync());
350341
}
351342

352343
[Fact]
353344
public async Task DeleteForFacilityAsync_RemovesThatFacilitysRowsAndReportsHowMany()
354345
{
355-
var plans = new List<FacilityReportingPlan> { ValidPlan(), ValidPlan() };
346+
Expression<Func<FacilityReportingPlan, bool>>? captured = null;
356347

357-
_mockRepository.Setup(r => r.FindAsync(It.IsAny<Expression<Func<FacilityReportingPlan, bool>>>(), It.IsAny<CancellationToken>()))
358-
.ReturnsAsync(plans);
348+
_mockRepository.Setup(r => r.ExecuteDeleteAsync(It.IsAny<Expression<Func<FacilityReportingPlan, bool>>>(), It.IsAny<CancellationToken>()))
349+
.Callback((Expression<Func<FacilityReportingPlan, bool>> p, CancellationToken _) => captured = p)
350+
.ReturnsAsync(2);
359351

360352
var removed = await _manager.DeleteForFacilityAsync(FacilityId);
361353

362354
Assert.Equal(2, removed);
363-
_mockRepository.Verify(r => r.SaveChangesAsync(It.IsAny<CancellationToken>()), Times.Once);
355+
_mockRepository.Verify(r => r.FindAsync(It.IsAny<Expression<Func<FacilityReportingPlan, bool>>>(), It.IsAny<CancellationToken>()), Times.Never);
356+
357+
Assert.NotNull(captured);
358+
var matches = captured!.Compile();
359+
Assert.True(matches(new FacilityReportingPlan { FacilityId = FacilityId }));
360+
Assert.False(matches(new FacilityReportingPlan { FacilityId = "another-facility" }));
364361
}
365362

366363
[Fact]

DotNet/Shared/Domain/Repositories/Implementations/EntityRepository.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,17 @@ public void Remove(T entity)
5656
_dbContext.Set<T>().Remove(entity);
5757
}
5858

59+
public Task<int> ExecuteDeleteAsync(Expression<Func<T, bool>> predicate)
60+
{
61+
return ExecuteDeleteAsync(predicate, CancellationToken.None);
62+
}
63+
64+
public async Task<int> ExecuteDeleteAsync(Expression<Func<T, bool>> predicate, CancellationToken cancellationToken)
65+
{
66+
cancellationToken.ThrowIfCancellationRequested();
67+
return await _dbContext.Set<T>().Where(predicate).ExecuteDeleteAsync(cancellationToken);
68+
}
69+
5970
// Query Methods
6071
public Task<bool> AnyAsync(Expression<Func<T, bool>> predicate)
6172
{

DotNet/Shared/Domain/Repositories/Interfaces/IEntityRepository.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ public interface IEntityRepository<T>
2525
Task<T> SingleAsync(Expression<Func<T, bool>> predicate);
2626
Task<T> SingleAsync(Expression<Func<T, bool>> predicate, CancellationToken cancellationToken);
2727
void Remove(T entity);
28+
29+
Task<int> ExecuteDeleteAsync(Expression<Func<T, bool>> predicate);
30+
Task<int> ExecuteDeleteAsync(Expression<Func<T, bool>> predicate, CancellationToken cancellationToken);
31+
2832
void Update(T entity);
2933
Task<(List<T>, PaginationMetadata)> SearchAsync(Expression<Func<T, bool>> predicate, string? sortBy, SortOrder? sortOrder, int pageSize, int pageNumber);
3034
Task<(List<T>, PaginationMetadata)> SearchAsync(Expression<Func<T, bool>> predicate, string? sortBy, SortOrder? sortOrder, int pageSize, int pageNumber, CancellationToken cancellationToken);

0 commit comments

Comments
 (0)