fix(alerting/I-1): retry endpoint resets attempts to 0 instead of incrementing
AlertNotificationRepository gains resetForRetry(UUID, Instant) which sets attempts=0, status=PENDING, next_attempt_at=now, and clears claim/response fields. AlertNotificationController calls resetForRetry instead of scheduleRetry so a manual retry always starts from a clean slate. AlertNotificationControllerIT adds retryResetsAttemptsToZero to verify attempts==0 and status==PENDING after three prior markFailed calls. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -69,10 +69,7 @@ public class AlertNotificationController {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Reset for retry: status -> PENDING, attempts -> 0, next_attempt_at -> now
|
// Reset for retry: status -> PENDING, attempts -> 0, next_attempt_at -> now
|
||||||
// We use scheduleRetry to reset attempt timing; then we need to reset attempts count.
|
notificationRepo.resetForRetry(id, Instant.now());
|
||||||
// The repository has scheduleRetry which sets next_attempt_at and records last status.
|
|
||||||
// We use a dedicated pattern: mark as pending by scheduling immediately.
|
|
||||||
notificationRepo.scheduleRetry(id, Instant.now(), 0, null);
|
|
||||||
|
|
||||||
return AlertNotificationDto.from(notificationRepo.findById(id)
|
return AlertNotificationDto.from(notificationRepo.findById(id)
|
||||||
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND)));
|
.orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND)));
|
||||||
|
|||||||
@@ -118,6 +118,21 @@ public class PostgresAlertNotificationRepository implements AlertNotificationRep
|
|||||||
""", Timestamp.from(nextAttemptAt), status, snippet, id);
|
""", Timestamp.from(nextAttemptAt), status, snippet, id);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public void resetForRetry(UUID id, Instant nextAttemptAt) {
|
||||||
|
jdbc.update("""
|
||||||
|
UPDATE alert_notifications
|
||||||
|
SET attempts = 0,
|
||||||
|
status = 'PENDING'::notification_status_enum,
|
||||||
|
next_attempt_at = ?,
|
||||||
|
claimed_by = NULL,
|
||||||
|
claimed_until = NULL,
|
||||||
|
last_response_status = NULL,
|
||||||
|
last_response_snippet = NULL
|
||||||
|
WHERE id = ?
|
||||||
|
""", Timestamp.from(nextAttemptAt), id);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void markFailed(UUID id, int status, String snippet) {
|
public void markFailed(UUID id, int status, String snippet) {
|
||||||
jdbc.update("""
|
jdbc.update("""
|
||||||
|
|||||||
@@ -113,6 +113,35 @@ class AlertNotificationControllerIT extends AbstractPostgresIT {
|
|||||||
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void retryResetsAttemptsToZero() throws Exception {
|
||||||
|
// Verify Fix I-1: retry endpoint resets attempts to 0, not attempts+1
|
||||||
|
AlertInstance instance = seedInstance();
|
||||||
|
AlertNotification notification = seedNotification(instance.id());
|
||||||
|
|
||||||
|
// Mark as failed with attempts at max (simulate exhausted retries)
|
||||||
|
notificationRepo.markFailed(notification.id(), 500, "server error");
|
||||||
|
notificationRepo.markFailed(notification.id(), 500, "server error");
|
||||||
|
notificationRepo.markFailed(notification.id(), 500, "server error");
|
||||||
|
|
||||||
|
// Verify attempts > 0 before retry
|
||||||
|
AlertNotification before = notificationRepo.findById(notification.id()).orElseThrow();
|
||||||
|
assertThat(before.attempts()).isGreaterThan(0);
|
||||||
|
|
||||||
|
// Operator retries
|
||||||
|
ResponseEntity<String> resp = restTemplate.exchange(
|
||||||
|
"/api/v1/alerts/notifications/" + notification.id() + "/retry",
|
||||||
|
HttpMethod.POST,
|
||||||
|
new HttpEntity<>(securityHelper.authHeaders(operatorJwt)),
|
||||||
|
String.class);
|
||||||
|
assertThat(resp.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||||
|
|
||||||
|
// After retry: attempts must be 0 and status PENDING (not attempts+1)
|
||||||
|
AlertNotification after = notificationRepo.findById(notification.id()).orElseThrow();
|
||||||
|
assertThat(after.attempts()).as("retry must reset attempts to 0").isEqualTo(0);
|
||||||
|
assertThat(after.status()).isEqualTo(NotificationStatus.PENDING);
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void viewerCannotRetry() throws Exception {
|
void viewerCannotRetry() throws Exception {
|
||||||
AlertInstance instance = seedInstance();
|
AlertInstance instance = seedInstance();
|
||||||
|
|||||||
@@ -13,5 +13,7 @@ public interface AlertNotificationRepository {
|
|||||||
void markDelivered(UUID id, int status, String snippet, Instant when);
|
void markDelivered(UUID id, int status, String snippet, Instant when);
|
||||||
void scheduleRetry(UUID id, Instant nextAttemptAt, int status, String snippet);
|
void scheduleRetry(UUID id, Instant nextAttemptAt, int status, String snippet);
|
||||||
void markFailed(UUID id, int status, String snippet);
|
void markFailed(UUID id, int status, String snippet);
|
||||||
|
/** Resets a FAILED notification for operator-triggered retry: attempts → 0, status → PENDING. */
|
||||||
|
void resetForRetry(UUID id, Instant nextAttemptAt);
|
||||||
void deleteSettledBefore(Instant cutoff);
|
void deleteSettledBefore(Instant cutoff);
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user