21990f2533
- Copy discipline for notifications: the store, the queue, workers, and API callers each own clones; no notification object is shared across goroutines (races previously flagged by -race between workers mutating Status/RetryCount and handlers JSON-encoding the same pointer). - Retry progress derives from QueueMessage.Attempt so it survives requeues; exponential backoff (1s base, 30s cap) honors the documented queue.retry_backoff setting instead of hammering failing providers in a tight loop; shutdown abandons pending backoff waits cleanly. - Stop() cancels a service-lifetime context so idle workers blocked in Dequeue exit immediately instead of waiting out their poll timeout. - LocalQueue no longer holds its mutex while sending on the queue channel (Enqueue/Nack) — with a full buffer this deadlocked the entire worker pool, since draining requires the same mutex. - Tenant scoping: notifications are stamped with the caller's ClientID; non-admin clients can only read/cancel/retry their own (cross-tenant access reports not-found to avoid leaking existence). - Sentinel errors ErrNotificationNotFound/ErrNotificationAlreadySent. - New race, backoff, and tenant-scoping test suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
248 lines
7.5 KiB
Go
248 lines
7.5 KiB
Go
package service
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/igodwin/notifier/internal/domain"
|
|
"github.com/igodwin/notifier/internal/logging"
|
|
"github.com/igodwin/notifier/internal/notifier"
|
|
"github.com/igodwin/notifier/internal/queue"
|
|
)
|
|
|
|
// alwaysFailNotifier is a fake domain.Notifier that always reports failure
|
|
// while recording the wall-clock time of each Send call, so tests can assert
|
|
// on the spacing between retry attempts.
|
|
type alwaysFailNotifier struct {
|
|
mu sync.Mutex
|
|
calls []time.Time
|
|
}
|
|
|
|
func (n *alwaysFailNotifier) Send(ctx context.Context, notification *domain.Notification) (*domain.NotificationResult, error) {
|
|
n.mu.Lock()
|
|
n.calls = append(n.calls, time.Now())
|
|
n.mu.Unlock()
|
|
|
|
return &domain.NotificationResult{
|
|
NotificationID: notification.ID,
|
|
Success: false,
|
|
Error: "simulated failure",
|
|
SentAt: time.Now(),
|
|
}, nil
|
|
}
|
|
|
|
func (n *alwaysFailNotifier) Type() domain.NotificationType { return domain.TypeStdout }
|
|
|
|
func (n *alwaysFailNotifier) Validate(notification *domain.Notification) error { return nil }
|
|
|
|
func (n *alwaysFailNotifier) Close() error { return nil }
|
|
|
|
func (n *alwaysFailNotifier) callTimes() []time.Time {
|
|
n.mu.Lock()
|
|
defer n.mu.Unlock()
|
|
return append([]time.Time(nil), n.calls...)
|
|
}
|
|
|
|
// createFailingTestService builds a NotificationService wired to the given
|
|
// (always failing) notifier instead of the stdout notifier used by
|
|
// createTestService.
|
|
func createFailingTestService(t *testing.T, fail domain.Notifier) *NotificationService {
|
|
t.Helper()
|
|
|
|
factory := notifier.NewFactory()
|
|
if err := factory.RegisterNotifier(domain.TypeStdout, "", fail); err != nil {
|
|
t.Fatalf("Failed to register notifier: %v", err)
|
|
}
|
|
|
|
q, err := queue.NewLocalQueue(&domain.LocalQueueConfig{BufferSize: 100})
|
|
if err != nil {
|
|
t.Fatalf("Failed to create queue: %v", err)
|
|
}
|
|
|
|
logger, err := logging.NewFromConfig("error", "stdout")
|
|
if err != nil {
|
|
t.Fatalf("Failed to create logger: %v", err)
|
|
}
|
|
|
|
return NewNotificationService(factory, q, 2, nil, nil, logger)
|
|
}
|
|
|
|
// waitForStatus polls GetNotification until it observes the notification in
|
|
// the given status, or fails the test after timeout.
|
|
func waitForStatus(t *testing.T, svc *NotificationService, ctx context.Context, id string, status domain.NotificationStatus, timeout time.Duration) *domain.Notification {
|
|
t.Helper()
|
|
|
|
deadline := time.Now().Add(timeout)
|
|
for time.Now().Before(deadline) {
|
|
n, err := svc.GetNotification(ctx, id)
|
|
if err == nil && n.Status == status {
|
|
return n
|
|
}
|
|
time.Sleep(10 * time.Millisecond)
|
|
}
|
|
|
|
t.Fatalf("notification %s did not reach status %s within %v", id, status, timeout)
|
|
return nil
|
|
}
|
|
|
|
// TestRetryBackoffExponentialDelaysRequeue verifies that, with the default
|
|
// (exponential) backoff mode, the delay between retry attempts roughly
|
|
// doubles each time rather than hammering the failing notifier in a tight
|
|
// loop.
|
|
func TestRetryBackoffExponentialDelaysRequeue(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.retryBaseDelay = 40 * time.Millisecond // tiny base delay for a fast test
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
defer svc.Stop()
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-exponential-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
waitForStatus(t, svc, ctx, notification.ID, domain.StatusFailed, 5*time.Second)
|
|
|
|
calls := fail.callTimes()
|
|
if len(calls) != 3 {
|
|
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
|
|
}
|
|
|
|
gap1 := calls[1].Sub(calls[0])
|
|
gap2 := calls[2].Sub(calls[1])
|
|
|
|
// Expected gaps are ~40ms then ~80ms; use generous tolerances to absorb
|
|
// scheduler jitter while still proving a real, growing delay was applied.
|
|
if gap1 < 25*time.Millisecond {
|
|
t.Errorf("expected delay before 2nd attempt >= ~40ms, got %v", gap1)
|
|
}
|
|
if gap2 < gap1 {
|
|
t.Errorf("expected delay before 3rd attempt (%v) to be larger than before 2nd (%v)", gap2, gap1)
|
|
}
|
|
|
|
t.Logf("attempt gaps: %v, %v", gap1, gap2)
|
|
}
|
|
|
|
// TestRetryBackoffNoneIsImmediate verifies that retryBackoff "none" requeues
|
|
// immediately, ignoring the configured base delay entirely.
|
|
func TestRetryBackoffNoneIsImmediate(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.WithRetryBackoff("none")
|
|
svc.retryBaseDelay = 5 * time.Second // large, to prove "none" ignores it
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
defer svc.Stop()
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-none-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
start := time.Now()
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
waitForStatus(t, svc, ctx, notification.ID, domain.StatusFailed, 2*time.Second)
|
|
elapsed := time.Since(start)
|
|
|
|
if elapsed > 1*time.Second {
|
|
t.Errorf("expected immediate retries with backoff=none, took %v", elapsed)
|
|
}
|
|
|
|
calls := fail.callTimes()
|
|
if len(calls) != 3 {
|
|
t.Fatalf("expected 3 send attempts (MaxRetries=3), got %d", len(calls))
|
|
}
|
|
}
|
|
|
|
// TestRetryBackoffShutdownAbandonsCleanly verifies that Stop() does not block
|
|
// on (or leak) a goroutine that is waiting out a backoff delay: it should
|
|
// abandon the pending retry and mark the notification Failed instead.
|
|
func TestRetryBackoffShutdownAbandonsCleanly(t *testing.T) {
|
|
fail := &alwaysFailNotifier{}
|
|
svc := createFailingTestService(t, fail)
|
|
svc.retryBaseDelay = 5 * time.Second // long enough that shutdown lands mid-wait
|
|
|
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
|
defer cancel()
|
|
|
|
if err := svc.Start(ctx); err != nil {
|
|
t.Fatalf("Failed to start service: %v", err)
|
|
}
|
|
|
|
notification := &domain.Notification{
|
|
ID: "backoff-shutdown-1",
|
|
Type: domain.TypeStdout,
|
|
Status: domain.StatusPending,
|
|
Recipients: []string{"test@example.com"},
|
|
MaxRetries: 3,
|
|
}
|
|
|
|
if _, err := svc.Send(ctx, notification); err != nil {
|
|
t.Fatalf("Send failed: %v", err)
|
|
}
|
|
|
|
// Give the worker time to make its first (failing) attempt and enter the
|
|
// backoff wait.
|
|
deadline := time.Now().Add(2 * time.Second)
|
|
for time.Now().Before(deadline) && len(fail.callTimes()) == 0 {
|
|
time.Sleep(10 * time.Millisecond)
|
|
}
|
|
if len(fail.callTimes()) == 0 {
|
|
t.Fatal("notifier was never called; cannot test shutdown mid-backoff")
|
|
}
|
|
|
|
stopped := make(chan error, 1)
|
|
go func() {
|
|
stopped <- svc.Stop()
|
|
}()
|
|
|
|
select {
|
|
case err := <-stopped:
|
|
if err != nil {
|
|
t.Errorf("Stop returned error: %v", err)
|
|
}
|
|
case <-time.After(3 * time.Second):
|
|
t.Fatal("Stop did not return promptly - possible goroutine leak in retry backoff")
|
|
}
|
|
|
|
n, err := svc.GetNotification(context.Background(), notification.ID)
|
|
if err != nil {
|
|
t.Fatalf("GetNotification failed after shutdown: %v", err)
|
|
}
|
|
if n.Status != domain.StatusFailed {
|
|
t.Errorf("expected notification to be marked Failed after shutdown abandon, got %s", n.Status)
|
|
}
|
|
|
|
// Only the one attempt made before shutdown should have happened; the
|
|
// pending retry must not have fired.
|
|
if calls := len(fail.callTimes()); calls != 1 {
|
|
t.Errorf("expected exactly 1 send attempt before shutdown abandoned the retry, got %d", calls)
|
|
}
|
|
}
|