feat(alerts): Postgres repo — read_at/deleted_at columns, filter params, new mutations

- save/rowMapper read+write read_at and deleted_at
- listForInbox: tri-state acked/read filters; always excludes deleted
- countUnreadBySeverity: rewire without alert_reads join, preserve zero-fill
- new: markRead/bulkMarkRead/softDelete/bulkSoftDelete/bulkAck/restore
- delete PostgresAlertReadRepository + its bean
- restore zero-fill Javadoc on interface
- mechanical compile-fixes in AlertController, InAppInboxQuery,
  AlertControllerIT, InAppInboxQueryTest; Task 6 owns the rewrite
- PostgresAlertReadRepositoryIT stubbed @Disabled; Task 7 owns migration

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
hsiegeln
2026-04-21 17:56:06 +02:00
parent 55b2a00458
commit da2819332c
10 changed files with 227 additions and 214 deletions

View File

@@ -5,7 +5,6 @@ import com.cameleer.server.app.TestSecurityHelper;
import com.cameleer.server.app.search.ClickHouseLogStore;
import com.cameleer.server.core.alerting.AlertInstance;
import com.cameleer.server.core.alerting.AlertInstanceRepository;
import com.cameleer.server.core.alerting.AlertReadRepository;
import com.cameleer.server.core.alerting.AlertSeverity;
import com.cameleer.server.core.alerting.AlertState;
import com.fasterxml.jackson.databind.JsonNode;
@@ -35,7 +34,6 @@ class AlertControllerIT extends AbstractPostgresIT {
@Autowired private ObjectMapper objectMapper;
@Autowired private TestSecurityHelper securityHelper;
@Autowired private AlertInstanceRepository instanceRepo;
@Autowired private AlertReadRepository readRepo;
private String operatorJwt;
private String viewerJwt;

View File

@@ -22,6 +22,8 @@ import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicLong;
import org.mockito.ArgumentMatchers;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
@@ -75,13 +77,13 @@ class InAppInboxQueryTest {
.thenReturn(List.of(new RoleSummary(roleId, "OPERATOR", true, "direct")));
when(instanceRepo.listForInbox(eq(ENV_ID), eq(List.of(groupId.toString())),
eq(USER_ID), eq(List.of("OPERATOR")), isNull(), isNull(), eq(20)))
eq(USER_ID), eq(List.of("OPERATOR")), isNull(), isNull(), isNull(), isNull(), eq(20)))
.thenReturn(List.of());
List<AlertInstance> result = query.listInbox(ENV_ID, USER_ID, 20);
assertThat(result).isEmpty();
verify(instanceRepo).listForInbox(ENV_ID, List.of(groupId.toString()),
USER_ID, List.of("OPERATOR"), null, null, 20);
USER_ID, List.of("OPERATOR"), null, null, null, null, 20);
}
@Test
@@ -94,12 +96,12 @@ class InAppInboxQueryTest {
List<AlertSeverity> severities = List.of(AlertSeverity.CRITICAL, AlertSeverity.WARNING);
when(instanceRepo.listForInbox(eq(ENV_ID), eq(List.of()), eq(USER_ID), eq(List.of()),
eq(states), eq(severities), eq(25)))
eq(states), eq(severities), isNull(), isNull(), eq(25)))
.thenReturn(List.of());
query.listInbox(ENV_ID, USER_ID, states, severities, 25);
verify(instanceRepo).listForInbox(ENV_ID, List.of(), USER_ID, List.of(),
states, severities, 25);
states, severities, null, null, 25);
}
// -------------------------------------------------------------------------
@@ -108,7 +110,8 @@ class InAppInboxQueryTest {
@Test
void countUnread_totalIsSumOfBySeverityValues() {
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(4L, 2L, 1L));
UnreadCountResponse response = query.countUnread(ENV_ID, USER_ID);
@@ -123,7 +126,8 @@ class InAppInboxQueryTest {
@Test
void countUnread_fillsMissingSeveritiesWithZero() {
// Repository returns only CRITICAL — WARNING/INFO must default to 0.
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(Map.of(AlertSeverity.CRITICAL, 3L));
UnreadCountResponse response = query.countUnread(ENV_ID, USER_ID);
@@ -141,7 +145,8 @@ class InAppInboxQueryTest {
@Test
void countUnread_secondCallWithin5sUsesCache() {
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(1L, 2L, 2L));
UnreadCountResponse first = query.countUnread(ENV_ID, USER_ID);
@@ -150,12 +155,14 @@ class InAppInboxQueryTest {
assertThat(first.total()).isEqualTo(5L);
assertThat(second.total()).isEqualTo(5L);
verify(instanceRepo, times(1)).countUnreadBySeverityForUser(ENV_ID, USER_ID);
verify(instanceRepo, times(1)).countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any());
}
@Test
void countUnread_callAfter5sRefreshesCache() {
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(1L, 1L, 1L)) // first call — total 3
.thenReturn(severities(4L, 3L, 2L)); // after TTL — total 9
@@ -165,29 +172,36 @@ class InAppInboxQueryTest {
assertThat(first.total()).isEqualTo(3L);
assertThat(third.total()).isEqualTo(9L);
verify(instanceRepo, times(2)).countUnreadBySeverityForUser(ENV_ID, USER_ID);
verify(instanceRepo, times(2)).countUnreadBySeverity(eq(ENV_ID), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any());
}
@Test
void countUnread_differentUsersDontShareCache() {
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, "alice"))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq("alice"),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(0L, 1L, 1L));
when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, "bob"))
when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq("bob"),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(2L, 2L, 4L));
assertThat(query.countUnread(ENV_ID, "alice").total()).isEqualTo(2L);
assertThat(query.countUnread(ENV_ID, "bob").total()).isEqualTo(8L);
verify(instanceRepo).countUnreadBySeverityForUser(ENV_ID, "alice");
verify(instanceRepo).countUnreadBySeverityForUser(ENV_ID, "bob");
verify(instanceRepo).countUnreadBySeverity(eq(ENV_ID), eq("alice"),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any());
verify(instanceRepo).countUnreadBySeverity(eq(ENV_ID), eq("bob"),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any());
}
@Test
void countUnread_differentEnvsDontShareCache() {
UUID envA = UUID.randomUUID();
UUID envB = UUID.randomUUID();
when(instanceRepo.countUnreadBySeverityForUser(envA, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(envA), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(0L, 0L, 1L));
when(instanceRepo.countUnreadBySeverityForUser(envB, USER_ID))
when(instanceRepo.countUnreadBySeverity(eq(envB), eq(USER_ID),
ArgumentMatchers.<List<String>>any(), ArgumentMatchers.<List<String>>any()))
.thenReturn(severities(1L, 1L, 2L));
assertThat(query.countUnread(envA, USER_ID).total()).isEqualTo(1L);

View File

@@ -50,7 +50,6 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
@AfterEach
void cleanup() {
jdbcTemplate.update("DELETE FROM alert_reads WHERE user_id = ?", userId);
jdbcTemplate.update("DELETE FROM alert_notifications WHERE alert_instance_id IN " +
"(SELECT id FROM alert_instances WHERE environment_id = ?)", envId);
jdbcTemplate.update("DELETE FROM alert_instances WHERE environment_id = ?", envId);
@@ -92,7 +91,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
repo.save(byRole);
// User is member of the group AND has the role
var inbox = repo.listForInbox(envId, List.of(groupId), userId, List.of(roleName), 50);
var inbox = repo.listForInbox(envId, List.of(groupId), userId, List.of(roleName), null, null, null, null, 50);
assertThat(inbox).extracting(AlertInstance::id)
.containsExactlyInAnyOrder(byUser.id(), byGroup.id(), byRole.id());
}
@@ -102,33 +101,30 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
var byUser = newInstance(ruleId, List.of(userId), List.of(), List.of());
repo.save(byUser);
var inbox = repo.listForInbox(envId, List.of(), userId, List.of(), 50);
var inbox = repo.listForInbox(envId, List.of(), userId, List.of(), null, null, null, null, 50);
assertThat(inbox).hasSize(1);
assertThat(inbox.get(0).id()).isEqualTo(byUser.id());
}
@Test
void countUnreadBySeverityForUser_decreasesAfterMarkRead() {
void countUnreadBySeverity_decreasesAfterMarkRead() {
var inst = newInstance(ruleId, List.of(userId), List.of(), List.of());
repo.save(inst);
var before = repo.countUnreadBySeverityForUser(envId, userId);
var before = repo.countUnreadBySeverity(envId, userId, List.of(), List.of());
assertThat(before)
.containsEntry(AlertSeverity.WARNING, 1L)
.containsEntry(AlertSeverity.CRITICAL, 0L)
.containsEntry(AlertSeverity.INFO, 0L);
// Insert read record directly (AlertReadRepository not yet wired in this test)
jdbcTemplate.update(
"INSERT INTO alert_reads (user_id, alert_instance_id) VALUES (?, ?) ON CONFLICT DO NOTHING",
userId, inst.id());
repo.markRead(inst.id(), Instant.now());
var after = repo.countUnreadBySeverityForUser(envId, userId);
var after = repo.countUnreadBySeverity(envId, userId, List.of(), List.of());
assertThat(after.values()).allMatch(v -> v == 0L);
}
@Test
void countUnreadBySeverityForUser_groupsBySeverity() {
void countUnreadBySeverity_groupsBySeverity() {
// Each open instance needs its own rule to satisfy V13's unique partial index.
UUID critRule = seedRuleWithSeverity("crit", AlertSeverity.CRITICAL);
UUID warnRule = seedRuleWithSeverity("warn", AlertSeverity.WARNING);
@@ -138,7 +134,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
repo.save(newInstance(warnRule, AlertSeverity.WARNING, List.of(userId), List.of(), List.of()));
repo.save(newInstance(infoRule, AlertSeverity.INFO, List.of(userId), List.of(), List.of()));
var counts = repo.countUnreadBySeverityForUser(envId, userId);
var counts = repo.countUnreadBySeverity(envId, userId, List.of(), List.of());
assertThat(counts)
.containsEntry(AlertSeverity.CRITICAL, 1L)
@@ -147,10 +143,10 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
}
@Test
void countUnreadBySeverityForUser_emptyMapStillHasAllKeys() {
void countUnreadBySeverity_emptyMapStillHasAllKeys() {
// No instances saved — every severity must still be present with value 0
// so callers never deal with null/missing keys.
var counts = repo.countUnreadBySeverityForUser(envId, userId);
var counts = repo.countUnreadBySeverity(envId, userId, List.of(), List.of());
assertThat(counts).hasSize(3);
assertThat(counts.values()).allMatch(v -> v == 0L);
}
@@ -269,7 +265,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
Long count = jdbcTemplate.queryForObject(
"SELECT COUNT(*) FROM alert_instances " +
" WHERE rule_id = ? AND state IN ('PENDING','FIRING','ACKNOWLEDGED')",
" WHERE rule_id = ? AND state IN ('PENDING','FIRING')",
Long.class, ruleId);
assertThat(count).isEqualTo(3L);
}
@@ -293,7 +289,7 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
Long count = jdbcTemplate.queryForObject(
"SELECT COUNT(*) FROM alert_instances " +
" WHERE rule_id = ? AND state IN ('PENDING','FIRING','ACKNOWLEDGED')",
" WHERE rule_id = ? AND state IN ('PENDING','FIRING')",
Long.class, ruleId);
assertThat(count).isEqualTo(1L);
}
@@ -308,8 +304,83 @@ class PostgresAlertInstanceRepositoryIT extends AbstractPostgresIT {
assertThat(repo.findById(inst.id()).orElseThrow().silenced()).isTrue();
}
@Test
void markRead_is_idempotent_and_sets_read_at() {
var inst = insertFreshFiring();
repo.markRead(inst.id(), Instant.parse("2026-04-21T10:00:00Z"));
repo.markRead(inst.id(), Instant.parse("2026-04-21T11:00:00Z")); // idempotent — no-op
var loaded = repo.findById(inst.id()).orElseThrow();
assertThat(loaded.readAt()).isEqualTo(Instant.parse("2026-04-21T10:00:00Z"));
}
@Test
void softDelete_excludes_from_listForInbox() {
var inst = insertFreshFiring();
repo.softDelete(inst.id(), Instant.parse("2026-04-21T10:00:00Z"));
var rows = repo.listForInbox(envId, List.of(), userId, List.of(),
null, null, null, null, 100);
assertThat(rows).extracting(AlertInstance::id).doesNotContain(inst.id());
}
@Test
void findOpenForRule_returns_acked_firing() {
var inst = insertFreshFiring();
repo.ack(inst.id(), userId, Instant.parse("2026-04-21T10:00:00Z"));
var open = repo.findOpenForRule(inst.ruleId());
assertThat(open).isPresent(); // ack no longer closes the open slot — state stays FIRING
}
@Test
void findOpenForRule_skips_soft_deleted() {
var inst = insertFreshFiring();
repo.softDelete(inst.id(), Instant.now());
assertThat(repo.findOpenForRule(inst.ruleId())).isEmpty();
}
@Test
void bulk_ack_only_touches_unacked_rows() {
var a = insertFreshFiring();
var b = insertFreshFiring();
// ack 'a' first with userId; bulkAck should leave 'a' untouched (already acked)
repo.ack(a.id(), userId, Instant.parse("2026-04-21T09:00:00Z"));
repo.bulkAck(List.of(a.id(), b.id()), userId, Instant.parse("2026-04-21T10:00:00Z"));
// a was already acked — acked_at stays at the first timestamp, not updated again
assertThat(repo.findById(a.id()).orElseThrow().ackedBy()).isEqualTo(userId);
assertThat(repo.findById(b.id()).orElseThrow().ackedBy()).isEqualTo(userId);
}
@Test
void listForInbox_acked_false_hides_acked_rows() {
var a = insertFreshFiring();
var b = insertFreshFiring();
repo.ack(a.id(), userId, Instant.now());
var rows = repo.listForInbox(envId, List.of(), userId, List.of(),
null, null, /*acked*/ false, null, 100);
assertThat(rows).extracting(AlertInstance::id).doesNotContain(a.id());
assertThat(rows).extracting(AlertInstance::id).contains(b.id());
}
@Test
void restore_clears_deleted_at() {
var inst = insertFreshFiring();
repo.softDelete(inst.id(), Instant.now());
repo.restore(inst.id());
var loaded = repo.findById(inst.id()).orElseThrow();
assertThat(loaded.deletedAt()).isNull();
var rows = repo.listForInbox(envId, List.of(), userId, List.of(),
null, null, null, null, 100);
assertThat(rows).extracting(AlertInstance::id).contains(inst.id());
}
// -------------------------------------------------------------------------
/** Creates and saves a fresh FIRING instance targeted at the test userId with its own rule. */
private AlertInstance insertFreshFiring() {
UUID freshRuleId = seedRule("fresh-rule");
var inst = newInstance(freshRuleId, List.of(userId), List.of(), List.of());
return repo.save(inst);
}
private AlertInstance newInstance(UUID ruleId,
List<String> userIds,
List<UUID> groupIds,

View File

@@ -1,120 +1,18 @@
package com.cameleer.server.app.alerting.storage;
import com.cameleer.server.app.AbstractPostgresIT;
import com.cameleer.server.app.search.ClickHouseLogStore;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.springframework.boot.test.mock.mockito.MockBean;
import java.util.List;
import java.util.UUID;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
class PostgresAlertReadRepositoryIT extends AbstractPostgresIT {
@MockBean(name = "clickHouseLogStore") ClickHouseLogStore clickHouseLogStore;
private PostgresAlertReadRepository repo;
private UUID envId;
private UUID instanceId1;
private UUID instanceId2;
private UUID instanceId3;
private final String userId = "read-user-" + UUID.randomUUID();
@BeforeEach
void setup() {
repo = new PostgresAlertReadRepository(jdbcTemplate);
envId = UUID.randomUUID();
instanceId1 = UUID.randomUUID();
instanceId2 = UUID.randomUUID();
instanceId3 = UUID.randomUUID();
jdbcTemplate.update(
"INSERT INTO environments (id, slug, display_name) VALUES (?, ?, ?)",
envId, "test-env-" + UUID.randomUUID(), "Test Env");
jdbcTemplate.update(
"INSERT INTO users (user_id, provider, email) VALUES ('sys-user', 'local', 'sys@example.com') ON CONFLICT (user_id) DO NOTHING");
jdbcTemplate.update(
"INSERT INTO users (user_id, provider, email) VALUES (?, 'local', ?) ON CONFLICT (user_id) DO NOTHING",
userId, userId + "@example.com");
// Each open alert_instance needs its own rule_id — the alert_instances_open_rule_uq
// partial unique forbids multiple open instances sharing the same rule_id + exchange
// discriminator (V13/V15). Three separate rules let all three instances coexist
// in FIRING state so alert_reads tests can target each one independently.
for (UUID instanceId : List.of(instanceId1, instanceId2, instanceId3)) {
UUID ruleId = UUID.randomUUID();
jdbcTemplate.update(
"INSERT INTO alert_rules (id, environment_id, name, severity, condition_kind, condition, " +
"notification_title_tmpl, notification_message_tmpl, created_by, updated_by) " +
"VALUES (?, ?, ?, 'WARNING', 'AGENT_STATE', '{}'::jsonb, 't', 'm', 'sys-user', 'sys-user')",
ruleId, envId, "rule-" + instanceId);
jdbcTemplate.update(
"INSERT INTO alert_instances (id, rule_id, rule_snapshot, environment_id, state, severity, " +
"fired_at, context, title, message) VALUES (?, ?, '{}'::jsonb, ?, 'FIRING', 'WARNING', " +
"now(), '{}'::jsonb, 'title', 'msg')",
instanceId, ruleId, envId);
}
}
@AfterEach
void cleanup() {
jdbcTemplate.update("DELETE FROM alert_reads WHERE user_id = ?", userId);
jdbcTemplate.update("DELETE FROM alert_instances WHERE environment_id = ?", envId);
jdbcTemplate.update("DELETE FROM alert_rules WHERE environment_id = ?", envId);
jdbcTemplate.update("DELETE FROM environments WHERE id = ?", envId);
jdbcTemplate.update("DELETE FROM users WHERE user_id = ?", userId);
}
/**
* Placeholder — PostgresAlertReadRepository was deleted in Task 5.
* Task 7 will rewrite this test class to cover the new read/delete mutation methods
* on {@link com.cameleer.server.app.alerting.storage.PostgresAlertInstanceRepository}.
*/
@Disabled("Task 7: rewrite after AlertReadRepository removal")
class PostgresAlertReadRepositoryIT {
@Test
void markRead_insertsReadRecord() {
repo.markRead(userId, instanceId1);
int count = jdbcTemplate.queryForObject(
"SELECT count(*) FROM alert_reads WHERE user_id = ? AND alert_instance_id = ?",
Integer.class, userId, instanceId1);
assertThat(count).isEqualTo(1);
}
@Test
void markRead_isIdempotent() {
repo.markRead(userId, instanceId1);
// second call should not throw
assertThatCode(() -> repo.markRead(userId, instanceId1)).doesNotThrowAnyException();
int count = jdbcTemplate.queryForObject(
"SELECT count(*) FROM alert_reads WHERE user_id = ? AND alert_instance_id = ?",
Integer.class, userId, instanceId1);
assertThat(count).isEqualTo(1);
}
@Test
void bulkMarkRead_marksMultiple() {
repo.bulkMarkRead(userId, List.of(instanceId1, instanceId2, instanceId3));
int count = jdbcTemplate.queryForObject(
"SELECT count(*) FROM alert_reads WHERE user_id = ?",
Integer.class, userId);
assertThat(count).isEqualTo(3);
}
@Test
void bulkMarkRead_emptyListDoesNotThrow() {
assertThatCode(() -> repo.bulkMarkRead(userId, List.of())).doesNotThrowAnyException();
}
@Test
void bulkMarkRead_isIdempotent() {
repo.bulkMarkRead(userId, List.of(instanceId1, instanceId2));
assertThatCode(() -> repo.bulkMarkRead(userId, List.of(instanceId1, instanceId2)))
.doesNotThrowAnyException();
int count = jdbcTemplate.queryForObject(
"SELECT count(*) FROM alert_reads WHERE user_id = ?",
Integer.class, userId);
assertThat(count).isEqualTo(2);
void placeholder() {
// replaced in Task 7
}
}