Skip to content

Commit 5ff39dd

Browse files
hhvrcCopilot
andauthored
test(cron): cover the control-log retention trim end-to-end against real Postgres (#330)
* test(cron): cover the control-log retention trim end-to-end against real Postgres Extract ClearOldShockerControlLogs' inline CTE DELETE into ShockerControlLogQueries.DeleteControlLogsBeyondPerUserLimitAsync (Common) as a single source of truth, parameterized by the per-user cap so tests can drive it with a small limit. The job now calls it with HardLimits.MaxShockerControlLogsPerUser. Add a Cron integration test (Cron.IntegrationTests, alongside the delivery tests) that seeds a user -> device -> shocker -> logs graph and runs the actual statement against a Testcontainers Postgres: it exercises the real table/column names, the shocker -> device -> owner join, the per-user window function, and newest-first ordering, asserting the newest N survive and the rest are deleted. A schema rename now fails the test rather than only the Cron host at runtime. Mirrors the outbox claim-query extraction (#328). No behavior change to the job. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * test(cron): make control-log trim test prove per-owner partitioning Per Copilot review: the single-owner setup (where the log's controller equalled the device owner) couldn't tell PARTITION BY d.owner_id apart from a buggy partition on controlled_by_user_id or a dropped PARTITION BY. Strengthen it: seed a second owner under the limit whose (older) logs must all survive, and attribute the over-limit owner's logs to two distinct controller users (neither over the limit). Now the assertions fail if the query ranks globally (would delete 10 and wipe the under-limit owner's logs) or partitions by controller (would delete 0) - only a correct per-owner trim deletes exactly the owner's oldest 5. Keeps the await-using async scope from the prior autofix commit. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent bb177cc commit 5ff39dd

3 files changed

Lines changed: 180 additions & 17 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
using Microsoft.EntityFrameworkCore;
2+
3+
namespace OpenShock.Common.OpenShockDb;
4+
5+
/// <summary>
6+
/// Reusable commands over the <c>shocker_control_logs</c> table. Kept here (rather than inline in the
7+
/// Cron cleanup job) so the raw SQL is a single source of truth that integration tests can execute
8+
/// directly - a table or column rename then breaks the test rather than surfacing only in production.
9+
/// </summary>
10+
public static class ShockerControlLogQueries
11+
{
12+
/// <summary>
13+
/// Retention trim: per owning user, keeps the newest <paramref name="maxPerUser"/> control logs (by
14+
/// <c>created_at</c>) and deletes the rest. Ownership is resolved through
15+
/// <c>shocker -&gt; device -&gt; owner</c>, so the cap is per user, not per shocker or device. Runs as a
16+
/// single set-based statement (a window function ranks each user's logs newest-first; anything past the
17+
/// cap is deleted). Returns the number of rows deleted.
18+
/// </summary>
19+
public static Task<int> DeleteControlLogsBeyondPerUserLimitAsync(this OpenShockContext db, int maxPerUser,
20+
CancellationToken cancellationToken = default)
21+
{
22+
if (maxPerUser < 0) throw new ArgumentOutOfRangeException(nameof(maxPerUser));
23+
24+
return db.Database.ExecuteSqlAsync(
25+
$"""
26+
WITH ranked_logs AS (
27+
SELECT
28+
l.id,
29+
ROW_NUMBER() OVER (PARTITION BY d.owner_id ORDER BY l.created_at DESC) AS rn
30+
FROM shocker_control_logs l
31+
JOIN shockers s ON s.id = l.shocker_id
32+
JOIN devices d ON d.id = s.device_id
33+
)
34+
DELETE FROM shocker_control_logs l
35+
USING ranked_logs rl
36+
WHERE l.id = rl.id
37+
AND rl.rn > {maxPerUser}
38+
""", cancellationToken);
39+
}
40+
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
using Microsoft.EntityFrameworkCore;
2+
using Microsoft.Extensions.DependencyInjection;
3+
using OpenShock.Common.Models;
4+
using OpenShock.Common.OpenShockDb;
5+
using OpenShock.Common.Utils;
6+
7+
namespace OpenShock.Cron.IntegrationTests.Tests;
8+
9+
/// <summary>
10+
/// End-to-end coverage for the Cron retention trim against real Postgres. Runs the cleanup job's *actual*
11+
/// statement (<see cref="ShockerControlLogQueries.DeleteControlLogsBeyondPerUserLimitAsync"/>, the single
12+
/// source of truth the job uses) over a seeded graph, so the real table/column names, the
13+
/// shocker -> device -> owner join, the per-user window function, and newest-first ordering are exercised.
14+
/// A schema rename breaks this test rather than only the Cron host at runtime.
15+
/// </summary>
16+
public sealed class ControlLogRetentionTests
17+
{
18+
[ClassDataSource<CronApplicationFactory>(Shared = SharedType.PerTestSession)]
19+
public required CronApplicationFactory Factory { get; init; }
20+
21+
[Test]
22+
public async Task DeleteControlLogsBeyondPerUserLimit_TrimsPerOwner_KeepsNewest_AndSparesOwnersUnderTheLimit()
23+
{
24+
const int keep = 10;
25+
26+
// Owner A is over the limit; owner B is under it. A's logs are attributed to two *different*
27+
// controller users (each under the limit on its own) and B's logs to B itself - so the trim is only
28+
// correct if it partitions by the shocker's OWNER (d.owner_id). If it instead partitioned by the
29+
// controller (l.controlled_by_user_id) it would delete nothing (no controller exceeds the limit); if
30+
// it dropped PARTITION BY entirely it would rank globally and wrongly delete B's (older) logs too.
31+
var ownerA = Guid.CreateVersion7();
32+
var ownerB = Guid.CreateVersion7();
33+
var controllerC = Guid.CreateVersion7();
34+
var controllerD = Guid.CreateVersion7();
35+
36+
var shockerA = Guid.CreateVersion7();
37+
var shockerB = Guid.CreateVersion7();
38+
39+
var baseTime = DateTime.UtcNow - TimeSpan.FromDays(1);
40+
41+
// A: 15 logs (5 over). Newest 10 survive, oldest 5 go. Split across controllers C (first 8) and D.
42+
var aOldestToNewest = new List<Guid>(15);
43+
// B: 5 logs, all older than A's - a global (unpartitioned) trim would wrongly delete them.
44+
var bIds = new List<Guid>(5);
45+
46+
await using (var db = await Factory.DbContextFactory.CreateDbContextAsync())
47+
{
48+
db.Users.Add(NewUser(ownerA, "own-a"));
49+
db.Users.Add(NewUser(ownerB, "own-b"));
50+
db.Users.Add(NewUser(controllerC, "ctl-c"));
51+
db.Users.Add(NewUser(controllerD, "ctl-d"));
52+
53+
AddShocker(db, shockerB, ownerB, rfId: 2000);
54+
AddShocker(db, shockerA, ownerA, rfId: 1000);
55+
56+
// B's logs sit at minutes 0..4 (oldest overall) so a global newest-10 trim would drop them.
57+
for (var i = 0; i < 5; i++)
58+
{
59+
var logId = Guid.CreateVersion7();
60+
bIds.Add(logId);
61+
db.ShockerControlLogs.Add(NewLog(logId, shockerB, ownerB, baseTime + TimeSpan.FromMinutes(i)));
62+
}
63+
64+
// A's logs sit at minutes 100..114 (newest overall), the first 8 by C and the last 7 by D.
65+
for (var i = 0; i < 15; i++)
66+
{
67+
var logId = Guid.CreateVersion7();
68+
aOldestToNewest.Add(logId);
69+
var controller = i < 8 ? controllerC : controllerD;
70+
db.ShockerControlLogs.Add(NewLog(logId, shockerA, controller, baseTime + TimeSpan.FromMinutes(100 + i)));
71+
}
72+
73+
await db.SaveChangesAsync();
74+
}
75+
76+
// Resolve the pooled OpenShockContext exactly as the Cron cleanup job does (DI-injected) so this
77+
// covers the same registration path that runs in production.
78+
await using var scope = Factory.Services.CreateAsyncScope();
79+
var db2 = scope.ServiceProvider.GetRequiredService<OpenShockContext>();
80+
81+
var deleted = await db2.DeleteControlLogsBeyondPerUserLimitAsync(keep);
82+
83+
// Only owner A is over the limit, so exactly its oldest 5 are removed. No other integration test
84+
// writes control logs, so the global count equals A's deletion: 5. (Dropping PARTITION BY would
85+
// delete 10; partitioning by controller instead of owner would delete 0.)
86+
await Assert.That(deleted).IsEqualTo(5);
87+
88+
var survivingA = (await db2.ShockerControlLogs.AsNoTracking()
89+
.Where(l => l.ShockerId == shockerA).Select(l => l.Id).ToListAsync()).ToHashSet();
90+
var survivingB = await db2.ShockerControlLogs.AsNoTracking()
91+
.Where(l => l.ShockerId == shockerB).Select(l => l.Id).ToListAsync();
92+
93+
// A keeps its newest 10; its oldest 5 are gone.
94+
await Assert.That(survivingA.SetEquals(aOldestToNewest.Skip(5).ToHashSet())).IsTrue();
95+
await Assert.That(aOldestToNewest.Take(5).Any(survivingA.Contains)).IsFalse();
96+
97+
// B is under the limit, so every one of its (older) logs survives - a global trim would have deleted them.
98+
await Assert.That(survivingB.Count).IsEqualTo(5);
99+
}
100+
101+
private static User NewUser(Guid id, string prefix) => new()
102+
{
103+
Id = id,
104+
Name = $"{prefix}{id:N}"[..16],
105+
Email = $"{prefix}-{id:N}@test.org",
106+
SecurityStamp = Guid.CreateVersion7(),
107+
CreatedAt = DateTime.UtcNow,
108+
ActivatedAt = DateTime.UtcNow
109+
};
110+
111+
private static void AddShocker(OpenShockContext db, Guid shockerId, Guid ownerId, ushort rfId)
112+
{
113+
var deviceId = Guid.CreateVersion7();
114+
db.Devices.Add(new Device
115+
{
116+
Id = deviceId, OwnerId = ownerId, Name = "LogTrimHub",
117+
Token = CryptoUtils.RandomAlphaNumericString(256), CreatedAt = DateTime.UtcNow
118+
});
119+
db.Shockers.Add(new Shocker
120+
{
121+
Id = shockerId, Name = "LogTrimShocker", RfId = rfId,
122+
DeviceId = deviceId, Model = ShockerModelType.CaiXianlin
123+
});
124+
}
125+
126+
private static ShockerControlLog NewLog(Guid id, Guid shockerId, Guid controllerId, DateTime createdAt) => new()
127+
{
128+
Id = id,
129+
ShockerId = shockerId,
130+
ControlledByUserId = controllerId,
131+
Intensity = 50,
132+
Duration = 1000,
133+
Type = ControlType.Shock,
134+
CustomName = null,
135+
CreatedAt = createdAt
136+
};
137+
}

‎Cron/Jobs/ClearOldShockerControlLogs.cs‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
using Microsoft.EntityFrameworkCore;
2-
using OpenShock.Common.Constants;
1+
using OpenShock.Common.Constants;
32
using OpenShock.Common.OpenShockDb;
43
using OpenShock.Cron.Attributes;
54

@@ -27,21 +26,8 @@ public ClearOldShockerControlLogs(OpenShockContext db, ILogger<ClearOldShockerCo
2726

2827
public async Task<int> Execute()
2928
{
30-
var deletedUserLimits = await _db.Database.ExecuteSqlAsync(
31-
$"""
32-
WITH ranked_logs AS (
33-
SELECT
34-
l.id,
35-
ROW_NUMBER() OVER (PARTITION BY d.owner_id ORDER BY l.created_at DESC) AS rn
36-
FROM shocker_control_logs l
37-
JOIN shockers s ON s.id = l.shocker_id
38-
JOIN devices d ON d.id = s.device_id
39-
)
40-
DELETE FROM shocker_control_logs l
41-
USING ranked_logs rl
42-
WHERE l.id = rl.id
43-
AND rl.rn > {HardLimits.MaxShockerControlLogsPerUser}
44-
""");
29+
var deletedUserLimits =
30+
await _db.DeleteControlLogsBeyondPerUserLimitAsync(HardLimits.MaxShockerControlLogsPerUser);
4531

4632
_logger.LogInformation("Deleted {deletedUserLimits} shocker control logs exceeding the per-user limit",
4733
deletedUserLimits);

0 commit comments

Comments
 (0)