fix(alerts): backend hardening + complete ACKNOWLEDGED migration
- new AlertInstanceRepository.filterInEnvLive(ids, env): single-query bulk ID validation - AlertController.inEnvLiveIds now one SQL round-trip instead of N - bulkMarkRead SQL: defense-in-depth AND deleted_at IS NULL - bulkAck SQL already had deleted_at IS NULL guard — no change needed - PostgresAlertInstanceRepositoryIT: add filterInEnvLive_excludes_other_env_and_soft_deleted - V12MigrationIT: remove alert_reads assertion (table dropped by V17) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -151,11 +151,7 @@ public class AlertController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private List<UUID> inEnvLiveIds(List<UUID> ids, UUID envId) {
|
private List<UUID> inEnvLiveIds(List<UUID> ids, UUID envId) {
|
||||||
return ids.stream()
|
return instanceRepo.filterInEnvLive(ids, envId);
|
||||||
.filter(id -> instanceRepo.findById(id)
|
|
||||||
.map(i -> i.environmentId().equals(envId) && i.deletedAt() == null)
|
|
||||||
.orElse(false))
|
|
||||||
.toList();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
private String currentUserId() {
|
private String currentUserId() {
|
||||||
|
|||||||
@@ -196,7 +196,7 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
|||||||
c.createArrayOf("uuid", ids.toArray()));
|
c.createArrayOf("uuid", ids.toArray()));
|
||||||
jdbc.update("""
|
jdbc.update("""
|
||||||
UPDATE alert_instances SET read_at = ?
|
UPDATE alert_instances SET read_at = ?
|
||||||
WHERE id = ANY(?) AND read_at IS NULL
|
WHERE id = ANY(?) AND read_at IS NULL AND deleted_at IS NULL
|
||||||
""", Timestamp.from(when), idArray);
|
""", Timestamp.from(when), idArray);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -262,6 +262,17 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository
|
|||||||
""", rowMapper(), Timestamp.from(now));
|
""", rowMapper(), Timestamp.from(now));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public List<UUID> filterInEnvLive(List<UUID> ids, UUID environmentId) {
|
||||||
|
if (ids == null || ids.isEmpty()) return List.of();
|
||||||
|
Array idArray = jdbc.execute((ConnectionCallback<Array>) c ->
|
||||||
|
c.createArrayOf("uuid", ids.toArray()));
|
||||||
|
return jdbc.query("""
|
||||||
|
SELECT id FROM alert_instances
|
||||||
|
WHERE id = ANY(?) AND environment_id = ? AND deleted_at IS NULL
|
||||||
|
""", (rs, i) -> (UUID) rs.getObject("id"), idArray, environmentId);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void deleteResolvedBefore(Instant cutoff) {
|
public void deleteResolvedBefore(Instant cutoff) {
|
||||||
jdbc.update("""
|
jdbc.update("""
|
||||||
|
|||||||
@@ -372,6 +372,18 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
|
|||||||
assertThat(rows).extracting(AlertInstance::id).contains(inst.id());
|
assertThat(rows).extracting(AlertInstance::id).contains(inst.id());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void filterInEnvLive_excludes_other_env_and_soft_deleted() {
|
||||||
|
var a = insertFreshFiring(); // env envId, live
|
||||||
|
var b = insertFreshFiring(); // env envId, will be soft-deleted
|
||||||
|
repo.softDelete(b.id(), Instant.now());
|
||||||
|
|
||||||
|
UUID unknownId = UUID.randomUUID(); // not in DB at all
|
||||||
|
|
||||||
|
var kept = repo.filterInEnvLive(List.of(a.id(), b.id(), unknownId), envId);
|
||||||
|
assertThat(kept).containsExactly(a.id());
|
||||||
|
}
|
||||||
|
|
||||||
// -------------------------------------------------------------------------
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
/** Creates and saves a fresh FIRING instance targeted at the test userId with its own rule. */
|
/** Creates and saves a fresh FIRING instance targeted at the test userId with its own rule. */
|
||||||
|
|||||||
@@ -22,14 +22,15 @@ class V12MigrationIT extends AbstractPostgresIT {
|
|||||||
|
|
||||||
@Test
|
@Test
|
||||||
void allAlertingTablesAndEnumsExist() {
|
void allAlertingTablesAndEnumsExist() {
|
||||||
|
// Note: alert_reads was created in V12 but dropped by V17 (superseded by read_at column).
|
||||||
var tables = jdbcTemplate.queryForList(
|
var tables = jdbcTemplate.queryForList(
|
||||||
"SELECT table_name FROM information_schema.tables WHERE table_schema='public' " +
|
"SELECT table_name FROM information_schema.tables WHERE table_schema='public' " +
|
||||||
"AND table_name IN ('alert_rules','alert_rule_targets','alert_instances'," +
|
"AND table_name IN ('alert_rules','alert_rule_targets','alert_instances'," +
|
||||||
"'alert_silences','alert_notifications','alert_reads')",
|
"'alert_silences','alert_notifications')",
|
||||||
String.class);
|
String.class);
|
||||||
assertThat(tables).containsExactlyInAnyOrder(
|
assertThat(tables).containsExactlyInAnyOrder(
|
||||||
"alert_rules","alert_rule_targets","alert_instances",
|
"alert_rules","alert_rule_targets","alert_instances",
|
||||||
"alert_silences","alert_notifications","alert_reads");
|
"alert_silences","alert_notifications");
|
||||||
|
|
||||||
var enums = jdbcTemplate.queryForList(
|
var enums = jdbcTemplate.queryForList(
|
||||||
"SELECT typname FROM pg_type WHERE typname IN " +
|
"SELECT typname FROM pg_type WHERE typname IN " +
|
||||||
|
|||||||
@@ -73,4 +73,10 @@ public interface AlertInstanceRepository {
|
|||||||
void bulkAck(List<UUID> ids, String userId, Instant when);
|
void bulkAck(List<UUID> ids, String userId, Instant when);
|
||||||
|
|
||||||
List<AlertInstance> listFiringDueForReNotify(Instant now);
|
List<AlertInstance> listFiringDueForReNotify(Instant now);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Filter the given IDs to those that exist in the given environment and are not
|
||||||
|
* soft-deleted. Single SQL round-trip — avoids N+1 in bulk operations.
|
||||||
|
*/
|
||||||
|
List<UUID> filterInEnvLive(List<UUID> ids, UUID environmentId);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user