From da2819332c239ff04e83376b4e5032a16cca103e Mon Sep 17 00:00:00 2001 From: hsiegeln <37154749+hsiegeln@users.noreply.github.com> Date: Tue, 21 Apr 2026 17:56:06 +0200 Subject: [PATCH] =?UTF-8?q?feat(alerts):=20Postgres=20repo=20=E2=80=94=20r?= =?UTF-8?q?ead=5Fat/deleted=5Fat=20columns,=20filter=20params,=20new=20mut?= =?UTF-8?q?ations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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) --- .../alerting/config/AlertingBeanConfig.java | 10 +- .../alerting/controller/AlertController.java | 12 +- .../app/alerting/notify/InAppInboxQuery.java | 6 +- .../PostgresAlertInstanceRepository.java | 103 ++++++++++++--- .../storage/PostgresAlertReadRepository.java | 35 ----- .../controller/AlertControllerIT.java | 2 - .../alerting/notify/InAppInboxQueryTest.java | 46 ++++--- .../PostgresAlertInstanceRepositoryIT.java | 103 ++++++++++++--- .../PostgresAlertReadRepositoryIT.java | 122 ++---------------- .../alerting/AlertInstanceRepository.java | 2 + 10 files changed, 227 insertions(+), 214 deletions(-) delete mode 100644 cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepository.java diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/config/AlertingBeanConfig.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/config/AlertingBeanConfig.java index 2902f3ae..b4cc1e6a 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/config/AlertingBeanConfig.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/config/AlertingBeanConfig.java @@ -3,7 +3,10 @@ package com.cameleer.server.app.alerting.config; import com.cameleer.server.app.alerting.eval.PerKindCircuitBreaker; import com.cameleer.server.app.alerting.metrics.AlertingMetrics; import com.cameleer.server.app.alerting.storage.*; -import com.cameleer.server.core.alerting.*; +import com.cameleer.server.core.alerting.AlertInstanceRepository; +import com.cameleer.server.core.alerting.AlertNotificationRepository; +import com.cameleer.server.core.alerting.AlertRuleRepository; +import com.cameleer.server.core.alerting.AlertSilenceRepository; import com.fasterxml.jackson.databind.ObjectMapper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,11 +44,6 @@ public class AlertingBeanConfig { return new PostgresAlertNotificationRepository(jdbc, om); } - @Bean - public AlertReadRepository alertReadRepository(JdbcTemplate jdbc) { - return new PostgresAlertReadRepository(jdbc); - } - @Bean public Clock alertingClock() { return Clock.systemDefaultZone(); diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertController.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertController.java index eb65b0d5..69c4fb32 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertController.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/controller/AlertController.java @@ -7,7 +7,6 @@ import com.cameleer.server.app.alerting.notify.InAppInboxQuery; import com.cameleer.server.app.web.EnvPath; 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.cameleer.server.core.runtime.Environment; @@ -43,14 +42,11 @@ public class AlertController { private final InAppInboxQuery inboxQuery; private final AlertInstanceRepository instanceRepo; - private final AlertReadRepository readRepo; public AlertController(InAppInboxQuery inboxQuery, - AlertInstanceRepository instanceRepo, - AlertReadRepository readRepo) { + AlertInstanceRepository instanceRepo) { this.inboxQuery = inboxQuery; this.instanceRepo = instanceRepo; - this.readRepo = readRepo; } @GetMapping @@ -89,14 +85,12 @@ public class AlertController { @PostMapping("/{id}/read") public void read(@EnvPath Environment env, @PathVariable UUID id) { requireInstance(id, env.id()); - String userId = currentUserId(); - readRepo.markRead(userId, id); + instanceRepo.markRead(id, Instant.now()); } @PostMapping("/bulk-read") public void bulkRead(@EnvPath Environment env, @Valid @RequestBody BulkReadRequest req) { - String userId = currentUserId(); // filter to only instances in this env List filtered = req.instanceIds().stream() .filter(instanceId -> instanceRepo.findById(instanceId) @@ -104,7 +98,7 @@ public class AlertController { .orElse(false)) .toList(); if (!filtered.isEmpty()) { - readRepo.bulkMarkRead(userId, filtered); + instanceRepo.bulkMarkRead(filtered, Instant.now()); } } diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/notify/InAppInboxQuery.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/notify/InAppInboxQuery.java index 88f8906b..c3d0b497 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/notify/InAppInboxQuery.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/notify/InAppInboxQuery.java @@ -70,7 +70,7 @@ public class InAppInboxQuery { int limit) { List groupIds = resolveGroupIds(userId); List roleNames = resolveRoleNames(userId); - return instanceRepo.listForInbox(envId, groupIds, userId, roleNames, states, severities, limit); + return instanceRepo.listForInbox(envId, groupIds, userId, roleNames, states, severities, null, null, limit); } /** @@ -85,7 +85,9 @@ public class InAppInboxQuery { if (cached != null && now.isBefore(cached.expiresAt())) { return cached.response(); } - Map bySeverity = instanceRepo.countUnreadBySeverityForUser(envId, userId); + List groupIds = resolveGroupIds(userId); + List roleNames = resolveRoleNames(userId); + Map bySeverity = instanceRepo.countUnreadBySeverity(envId, userId, groupIds, roleNames); UnreadCountResponse response = UnreadCountResponse.from(bySeverity); memo.put(key, new Entry(response, now.plusMillis(MEMO_TTL_MS))); return response; diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepository.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepository.java index 63bbb142..e76fab97 100644 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepository.java +++ b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepository.java @@ -34,10 +34,12 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository INSERT INTO alert_instances ( id, rule_id, rule_snapshot, environment_id, state, severity, fired_at, acked_at, acked_by, resolved_at, last_notified_at, + read_at, deleted_at, silenced, current_value, threshold, context, title, message, target_user_ids, target_group_ids, target_role_names) VALUES (?, ?, ?::jsonb, ?, ?::alert_state_enum, ?::severity_enum, ?, ?, ?, ?, ?, + ?, ?, ?, ?, ?, ?::jsonb, ?, ?, ?, ?, ?) ON CONFLICT (id) DO UPDATE SET @@ -46,6 +48,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository acked_by = EXCLUDED.acked_by, resolved_at = EXCLUDED.resolved_at, last_notified_at = EXCLUDED.last_notified_at, + read_at = EXCLUDED.read_at, + deleted_at = EXCLUDED.deleted_at, silenced = EXCLUDED.silenced, current_value = EXCLUDED.current_value, threshold = EXCLUDED.threshold, @@ -66,6 +70,7 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository i.environmentId(), i.state().name(), i.severity().name(), ts(i.firedAt()), ts(i.ackedAt()), i.ackedBy(), ts(i.resolvedAt()), ts(i.lastNotifiedAt()), + ts(i.readAt()), ts(i.deletedAt()), i.silenced(), i.currentValue(), i.threshold(), writeJson(i.context()), i.title(), i.message(), userIds, groupIds, roleNames); @@ -101,8 +106,9 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository List userRoleNames, List states, List severities, + Boolean acked, + Boolean read, int limit) { - // Build arrays for group UUIDs and role names Array groupArray = toUuidArrayFromStrings(userGroupIdFilter); Array roleArray = toTextArray(userRoleNames); @@ -127,7 +133,13 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository sql.append(" AND severity::text = ANY(?)"); args.add(severityArray); } - + if (acked != null) { + sql.append(acked ? " AND acked_at IS NOT NULL" : " AND acked_at IS NULL"); + } + if (read != null) { + sql.append(read ? " AND read_at IS NOT NULL" : " AND read_at IS NULL"); + } + sql.append(" AND deleted_at IS NULL"); sql.append(" ORDER BY fired_at DESC LIMIT ?"); args.add(limit); @@ -135,23 +147,30 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository } @Override - public Map countUnreadBySeverityForUser(UUID environmentId, String userId) { + public Map countUnreadBySeverity(UUID environmentId, + String userId, + List groupIds, + List roleNames) { + Array groupArray = toUuidArrayFromStrings(groupIds); + Array roleArray = toTextArray(roleNames); String sql = """ - SELECT ai.severity::text AS severity, COUNT(*) AS cnt - FROM alert_instances ai - WHERE ai.environment_id = ? - AND ? = ANY(ai.target_user_ids) - AND NOT EXISTS ( - SELECT 1 FROM alert_reads ar - WHERE ar.user_id = ? AND ar.alert_instance_id = ai.id + SELECT severity::text AS severity, COUNT(*) AS cnt + FROM alert_instances + WHERE environment_id = ? + AND read_at IS NULL + AND deleted_at IS NULL + AND ( + ? = ANY(target_user_ids) + OR target_group_ids && ? + OR target_role_names && ? ) - GROUP BY ai.severity + GROUP BY severity """; EnumMap counts = new EnumMap<>(AlertSeverity.class); for (AlertSeverity s : AlertSeverity.values()) counts.put(s, 0L); - jdbc.query(sql, rs -> { - counts.put(AlertSeverity.valueOf(rs.getString("severity")), rs.getLong("cnt")); - }, environmentId, userId, userId); + jdbc.query(sql, (org.springframework.jdbc.core.RowCallbackHandler) rs -> counts.put( + AlertSeverity.valueOf(rs.getString("severity")), rs.getLong("cnt") + ), environmentId, userId, groupArray, roleArray); return counts; } @@ -164,6 +183,56 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository """, Timestamp.from(when), userId, id); } + @Override + public void markRead(UUID id, Instant when) { + jdbc.update("UPDATE alert_instances SET read_at = ? WHERE id = ? AND read_at IS NULL", + Timestamp.from(when), id); + } + + @Override + public void bulkMarkRead(List ids, Instant when) { + if (ids == null || ids.isEmpty()) return; + Array idArray = jdbc.execute((ConnectionCallback) c -> + c.createArrayOf("uuid", ids.toArray())); + jdbc.update(""" + UPDATE alert_instances SET read_at = ? + WHERE id = ANY(?) AND read_at IS NULL + """, Timestamp.from(when), idArray); + } + + @Override + public void softDelete(UUID id, Instant when) { + jdbc.update("UPDATE alert_instances SET deleted_at = ? WHERE id = ? AND deleted_at IS NULL", + Timestamp.from(when), id); + } + + @Override + public void bulkSoftDelete(List ids, Instant when) { + if (ids == null || ids.isEmpty()) return; + Array idArray = jdbc.execute((ConnectionCallback) c -> + c.createArrayOf("uuid", ids.toArray())); + jdbc.update(""" + UPDATE alert_instances SET deleted_at = ? + WHERE id = ANY(?) AND deleted_at IS NULL + """, Timestamp.from(when), idArray); + } + + @Override + public void restore(UUID id) { + jdbc.update("UPDATE alert_instances SET deleted_at = NULL WHERE id = ?", id); + } + + @Override + public void bulkAck(List ids, String userId, Instant when) { + if (ids == null || ids.isEmpty()) return; + Array idArray = jdbc.execute((ConnectionCallback) c -> + c.createArrayOf("uuid", ids.toArray())); + jdbc.update(""" + UPDATE alert_instances SET acked_at = ?, acked_by = ? + WHERE id = ANY(?) AND acked_at IS NULL AND deleted_at IS NULL + """, Timestamp.from(when), userId, idArray); + } + @Override public void resolve(UUID id, Instant when) { jdbc.update(""" @@ -215,6 +284,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository Timestamp ackedAt = rs.getTimestamp("acked_at"); Timestamp resolvedAt = rs.getTimestamp("resolved_at"); Timestamp lastNotifiedAt = rs.getTimestamp("last_notified_at"); + Timestamp readAt = rs.getTimestamp("read_at"); + Timestamp deletedAt = rs.getTimestamp("deleted_at"); Object cvObj = rs.getObject("current_value"); Double currentValue = cvObj == null ? null : ((Number) cvObj).doubleValue(); @@ -235,8 +306,8 @@ public class PostgresAlertInstanceRepository implements AlertInstanceRepository rs.getString("acked_by"), resolvedAt == null ? null : resolvedAt.toInstant(), lastNotifiedAt == null ? null : lastNotifiedAt.toInstant(), - null, - null, + readAt == null ? null : readAt.toInstant(), + deletedAt == null ? null : deletedAt.toInstant(), rs.getBoolean("silenced"), currentValue, threshold, diff --git a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepository.java b/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepository.java deleted file mode 100644 index fa6daab4..00000000 --- a/cameleer-server-app/src/main/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepository.java +++ /dev/null @@ -1,35 +0,0 @@ -package com.cameleer.server.app.alerting.storage; - -import com.cameleer.server.core.alerting.AlertReadRepository; -import org.springframework.jdbc.core.JdbcTemplate; - -import java.util.List; -import java.util.UUID; - -public class PostgresAlertReadRepository implements AlertReadRepository { - - private final JdbcTemplate jdbc; - - public PostgresAlertReadRepository(JdbcTemplate jdbc) { - this.jdbc = jdbc; - } - - @Override - public void markRead(String userId, UUID alertInstanceId) { - jdbc.update(""" - INSERT INTO alert_reads (user_id, alert_instance_id) - VALUES (?, ?) - ON CONFLICT (user_id, alert_instance_id) DO NOTHING - """, userId, alertInstanceId); - } - - @Override - public void bulkMarkRead(String userId, List alertInstanceIds) { - if (alertInstanceIds == null || alertInstanceIds.isEmpty()) { - return; - } - for (UUID id : alertInstanceIds) { - markRead(userId, id); - } - } -} diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertControllerIT.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertControllerIT.java index cdfcf478..5502d669 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertControllerIT.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/controller/AlertControllerIT.java @@ -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; diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/notify/InAppInboxQueryTest.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/notify/InAppInboxQueryTest.java index 24430e35..e0189c80 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/notify/InAppInboxQueryTest.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/notify/InAppInboxQueryTest.java @@ -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 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 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.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>any()); } @Test void countUnread_callAfter5sRefreshesCache() { - when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, USER_ID)) + when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq(USER_ID), + ArgumentMatchers.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>any()); } @Test void countUnread_differentUsersDontShareCache() { - when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, "alice")) + when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq("alice"), + ArgumentMatchers.>any(), ArgumentMatchers.>any())) .thenReturn(severities(0L, 1L, 1L)); - when(instanceRepo.countUnreadBySeverityForUser(ENV_ID, "bob")) + when(instanceRepo.countUnreadBySeverity(eq(ENV_ID), eq("bob"), + ArgumentMatchers.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>any()); + verify(instanceRepo).countUnreadBySeverity(eq(ENV_ID), eq("bob"), + ArgumentMatchers.>any(), ArgumentMatchers.>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.>any(), ArgumentMatchers.>any())) .thenReturn(severities(0L, 0L, 1L)); - when(instanceRepo.countUnreadBySeverityForUser(envB, USER_ID)) + when(instanceRepo.countUnreadBySeverity(eq(envB), eq(USER_ID), + ArgumentMatchers.>any(), ArgumentMatchers.>any())) .thenReturn(severities(1L, 1L, 2L)); assertThat(query.countUnread(envA, USER_ID).total()).isEqualTo(1L); diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepositoryIT.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepositoryIT.java index 7abcbc2c..5cf2d946 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepositoryIT.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertInstanceRepositoryIT.java @@ -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 userIds, List groupIds, diff --git a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepositoryIT.java b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepositoryIT.java index 6d8d372f..418c1fb7 100644 --- a/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepositoryIT.java +++ b/cameleer-server-app/src/test/java/com/cameleer/server/app/alerting/storage/PostgresAlertReadRepositoryIT.java @@ -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 } } diff --git a/cameleer-server-core/src/main/java/com/cameleer/server/core/alerting/AlertInstanceRepository.java b/cameleer-server-core/src/main/java/com/cameleer/server/core/alerting/AlertInstanceRepository.java index a01cbfec..964cffe6 100644 --- a/cameleer-server-core/src/main/java/com/cameleer/server/core/alerting/AlertInstanceRepository.java +++ b/cameleer-server-core/src/main/java/com/cameleer/server/core/alerting/AlertInstanceRepository.java @@ -43,6 +43,8 @@ public interface AlertInstanceRepository { * Count unread alert instances visible to the user, grouped by severity. * Visibility: targets user directly, or via one of the given groups/roles. * "Unread" = {@code read_at IS NULL AND deleted_at IS NULL}. + * Always returns a map with an entry for every {@link AlertSeverity} (value 0 if no rows), + * so callers never need null-checks. */ Map countUnreadBySeverity(UUID environmentId, String userId,