fix(alerting/B-1): PostgresAlertRuleRepository.save() now persists alert_rule_targets
saveTargets() is called unconditionally at the end of save() — it deletes existing targets and re-inserts from the current targets list. findById() and listByEnvironment() already call withTargets() so reads are consistent. PostgresAlertRuleRepositoryIT adds saveTargets_roundtrip and saveTargets_updateReplacesExistingTargets to cover the new write path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -55,20 +55,36 @@ public class PostgresAlertRuleRepository implements AlertRuleRepository {
|
|||||||
writeJson(r.evalState()),
|
writeJson(r.evalState()),
|
||||||
Timestamp.from(r.createdAt()), r.createdBy(),
|
Timestamp.from(r.createdAt()), r.createdBy(),
|
||||||
Timestamp.from(r.updatedAt()), r.updatedBy());
|
Timestamp.from(r.updatedAt()), r.updatedBy());
|
||||||
|
saveTargets(r.id(), r.targets());
|
||||||
return r;
|
return r;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private void saveTargets(UUID ruleId, List<AlertRuleTarget> targets) {
|
||||||
|
jdbc.update("DELETE FROM alert_rule_targets WHERE rule_id = ?", ruleId);
|
||||||
|
if (targets == null || targets.isEmpty()) return;
|
||||||
|
jdbc.batchUpdate(
|
||||||
|
"INSERT INTO alert_rule_targets (id, rule_id, target_kind, target_id) VALUES (?, ?, ?::target_kind_enum, ?)",
|
||||||
|
targets, targets.size(), (ps, t) -> {
|
||||||
|
ps.setObject(1, t.id() != null ? t.id() : UUID.randomUUID());
|
||||||
|
ps.setObject(2, ruleId);
|
||||||
|
ps.setString(3, t.kind().name());
|
||||||
|
ps.setString(4, t.targetId());
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Optional<AlertRule> findById(UUID id) {
|
public Optional<AlertRule> findById(UUID id) {
|
||||||
var list = jdbc.query("SELECT * FROM alert_rules WHERE id = ?", rowMapper(), id);
|
var list = jdbc.query("SELECT * FROM alert_rules WHERE id = ?", rowMapper(), id);
|
||||||
return list.isEmpty() ? Optional.empty() : Optional.of(list.get(0));
|
if (list.isEmpty()) return Optional.empty();
|
||||||
|
return Optional.of(withTargets(list).get(0));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public List<AlertRule> listByEnvironment(UUID environmentId) {
|
public List<AlertRule> listByEnvironment(UUID environmentId) {
|
||||||
return jdbc.query(
|
var list = jdbc.query(
|
||||||
"SELECT * FROM alert_rules WHERE environment_id = ? ORDER BY created_at DESC",
|
"SELECT * FROM alert_rules WHERE environment_id = ? ORDER BY created_at DESC",
|
||||||
rowMapper(), environmentId);
|
rowMapper(), environmentId);
|
||||||
|
return withTargets(list);
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -113,7 +129,38 @@ public class PostgresAlertRuleRepository implements AlertRuleRepository {
|
|||||||
)
|
)
|
||||||
RETURNING *
|
RETURNING *
|
||||||
""";
|
""";
|
||||||
return jdbc.query(sql, rowMapper(), instanceId, claimTtlSeconds, batchSize);
|
List<AlertRule> rules = jdbc.query(sql, rowMapper(), instanceId, claimTtlSeconds, batchSize);
|
||||||
|
return withTargets(rules);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Batch-loads targets for the given rules and returns new rule instances with targets populated. */
|
||||||
|
private List<AlertRule> withTargets(List<AlertRule> rules) {
|
||||||
|
if (rules.isEmpty()) return rules;
|
||||||
|
// Build IN clause
|
||||||
|
String inClause = rules.stream()
|
||||||
|
.map(r -> "'" + r.id() + "'")
|
||||||
|
.collect(java.util.stream.Collectors.joining(","));
|
||||||
|
String sql = "SELECT * FROM alert_rule_targets WHERE rule_id IN (" + inClause + ")";
|
||||||
|
Map<UUID, List<AlertRuleTarget>> byRuleId = new HashMap<>();
|
||||||
|
jdbc.query(sql, rs -> {
|
||||||
|
UUID ruleId = (UUID) rs.getObject("rule_id");
|
||||||
|
AlertRuleTarget t = new AlertRuleTarget(
|
||||||
|
(UUID) rs.getObject("id"),
|
||||||
|
ruleId,
|
||||||
|
TargetKind.valueOf(rs.getString("target_kind")),
|
||||||
|
rs.getString("target_id"));
|
||||||
|
byRuleId.computeIfAbsent(ruleId, k -> new ArrayList<>()).add(t);
|
||||||
|
});
|
||||||
|
return rules.stream()
|
||||||
|
.map(r -> new AlertRule(
|
||||||
|
r.id(), r.environmentId(), r.name(), r.description(),
|
||||||
|
r.severity(), r.enabled(), r.conditionKind(), r.condition(),
|
||||||
|
r.evaluationIntervalSeconds(), r.forDurationSeconds(), r.reNotifyMinutes(),
|
||||||
|
r.notificationTitleTmpl(), r.notificationMessageTmpl(),
|
||||||
|
r.webhooks(), byRuleId.getOrDefault(r.id(), List.of()),
|
||||||
|
r.nextEvaluationAt(), r.claimedBy(), r.claimedUntil(), r.evalState(),
|
||||||
|
r.createdAt(), r.createdBy(), r.updatedAt(), r.updatedBy()))
|
||||||
|
.toList();
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
|
|||||||
@@ -66,6 +66,44 @@ class PostgresAlertRuleRepositoryIT extends AbstractPostgresIT {
|
|||||||
assertThat(repo.findRuleIdsByOutboundConnectionId(UUID.randomUUID())).isEmpty();
|
assertThat(repo.findRuleIdsByOutboundConnectionId(UUID.randomUUID())).isEmpty();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void saveTargets_roundtrip() {
|
||||||
|
// Rule saved with a USER target and a ROLE target
|
||||||
|
UUID ruleId = UUID.randomUUID();
|
||||||
|
AlertRuleTarget userTarget = new AlertRuleTarget(UUID.randomUUID(), ruleId, TargetKind.USER, "alice");
|
||||||
|
AlertRuleTarget roleTarget = new AlertRuleTarget(UUID.randomUUID(), ruleId, TargetKind.ROLE, "OPERATOR");
|
||||||
|
var rule = newRuleWithId(ruleId, List.of(), List.of(userTarget, roleTarget));
|
||||||
|
|
||||||
|
repo.save(rule);
|
||||||
|
|
||||||
|
// findById must return the targets that were persisted by saveTargets()
|
||||||
|
var found = repo.findById(ruleId).orElseThrow();
|
||||||
|
assertThat(found.targets()).hasSize(2);
|
||||||
|
assertThat(found.targets()).extracting(AlertRuleTarget::targetId)
|
||||||
|
.containsExactlyInAnyOrder("alice", "OPERATOR");
|
||||||
|
assertThat(found.targets()).extracting(t -> t.kind().name())
|
||||||
|
.containsExactlyInAnyOrder("USER", "ROLE");
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void saveTargets_updateReplacesExistingTargets() {
|
||||||
|
// Save rule with one target
|
||||||
|
UUID ruleId = UUID.randomUUID();
|
||||||
|
AlertRuleTarget initial = new AlertRuleTarget(UUID.randomUUID(), ruleId, TargetKind.USER, "bob");
|
||||||
|
var rule = newRuleWithId(ruleId, List.of(), List.of(initial));
|
||||||
|
repo.save(rule);
|
||||||
|
|
||||||
|
// Update: replace with a different target
|
||||||
|
AlertRuleTarget updated = new AlertRuleTarget(UUID.randomUUID(), ruleId, TargetKind.GROUP, "team-ops");
|
||||||
|
var updated_rule = newRuleWithId(ruleId, List.of(), List.of(updated));
|
||||||
|
repo.save(updated_rule);
|
||||||
|
|
||||||
|
var found = repo.findById(ruleId).orElseThrow();
|
||||||
|
assertThat(found.targets()).hasSize(1);
|
||||||
|
assertThat(found.targets().get(0).targetId()).isEqualTo("team-ops");
|
||||||
|
assertThat(found.targets().get(0).kind()).isEqualTo(TargetKind.GROUP);
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void claimDueRulesAtomicSkipLocked() {
|
void claimDueRulesAtomicSkipLocked() {
|
||||||
var rule = newRule(List.of());
|
var rule = newRule(List.of());
|
||||||
@@ -80,11 +118,15 @@ class PostgresAlertRuleRepositoryIT extends AbstractPostgresIT {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private AlertRule newRule(List<WebhookBinding> webhooks) {
|
private AlertRule newRule(List<WebhookBinding> webhooks) {
|
||||||
|
return newRuleWithId(UUID.randomUUID(), webhooks, List.of());
|
||||||
|
}
|
||||||
|
|
||||||
|
private AlertRule newRuleWithId(UUID id, List<WebhookBinding> webhooks, List<AlertRuleTarget> targets) {
|
||||||
return new AlertRule(
|
return new AlertRule(
|
||||||
UUID.randomUUID(), envId, "rule-" + UUID.randomUUID(), "desc",
|
id, envId, "rule-" + id, "desc",
|
||||||
AlertSeverity.WARNING, true, ConditionKind.AGENT_STATE,
|
AlertSeverity.WARNING, true, ConditionKind.AGENT_STATE,
|
||||||
new AgentStateCondition(new AlertScope(null, null, null), "DEAD", 60),
|
new AgentStateCondition(new AlertScope(null, null, null), "DEAD", 60),
|
||||||
60, 0, 60, "t", "m", webhooks, List.of(),
|
60, 0, 60, "t", "m", webhooks, targets,
|
||||||
Instant.now().minusSeconds(10), null, null, Map.of(),
|
Instant.now().minusSeconds(10), null, null, Map.of(),
|
||||||
Instant.now(), "test-user", Instant.now(), "test-user");
|
Instant.now(), "test-user", Instant.now(), "test-user");
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user