From 3f036da03d4a1881643f94b0d5628404ca4c7aa1 Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Mon, 20 Apr 2026 08:25:39 +0200 Subject: [PATCH] fix(alerting/B-1): PostgresAlertRuleRepository.save() now persists alert_rule_targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../storage/PostgresAlertRuleRepository.java | 53 +++++++++++++++++-- .../PostgresAlertRuleRepositoryIT.java | 46 +++++++++++++++- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepository.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepository.java index efbdd07e..9c13852f 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepository.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepository.java @@ -55,20 +55,36 @@ public class PostgresAlertRuleRepository implements AlertRuleRepository { writeJson(r.evalState()), Timestamp.from(r.createdAt()), r.createdBy(), Timestamp.from(r.updatedAt()), r.updatedBy()); + saveTargets(r.id(), r.targets()); return r; } + private void saveTargets(UUID ruleId, List 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 public Optional findById(UUID 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 public List listByEnvironment(UUID environmentId) { - return jdbc.query( + var list = jdbc.query( "SELECT * FROM alert_rules WHERE environment_id = ? ORDER BY created_at DESC", rowMapper(), environmentId); + return withTargets(list); } @Override @@ -113,7 +129,38 @@ public class PostgresAlertRuleRepository implements AlertRuleRepository { ) RETURNING * """; - return jdbc.query(sql, rowMapper(), instanceId, claimTtlSeconds, batchSize); + List 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 withTargets(List 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> 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 diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepositoryIT.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepositoryIT.java index 3cdae754..74b06c11 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepositoryIT.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertRuleRepositoryIT.java @@ -66,6 +66,44 @@ class PostgresAlertRuleRepositoryIT extends AbstractPostgresIT { 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 void claimDueRulesAtomicSkipLocked() { var rule = newRule(List.of()); @@ -80,11 +118,15 @@ class PostgresAlertRuleRepositoryIT extends AbstractPostgresIT { } private AlertRule newRule(List webhooks) { + return newRuleWithId(UUID.randomUUID(), webhooks, List.of()); + } + + private AlertRule newRuleWithId(UUID id, List webhooks, List targets) { return new AlertRule( - UUID.randomUUID(), envId, "rule-" + UUID.randomUUID(), "desc", + id, envId, "rule-" + id, "desc", AlertSeverity.WARNING, true, ConditionKind.AGENT_STATE, 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(), "test-user", Instant.now(), "test-user"); }