From 79c2eb50201568b47f6ebff4d9c4665347d4eba4 Mon Sep 17 00:00:00 2001 From: wucm667 Date: Mon, 17 Aug 2026 10:59:43 +0800 Subject: [PATCH] fix: skip expiry reminders without SMTP config --- backend/internal/service/leader_lock_test.go | 12 ++-- .../service/subscription_expiry_service.go | 29 +++++++++ .../subscription_expiry_service_test.go | 61 +++++++++++++++++-- 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/backend/internal/service/leader_lock_test.go b/backend/internal/service/leader_lock_test.go index aba1c62c3e..a2adf01267 100644 --- a/backend/internal/service/leader_lock_test.go +++ b/backend/internal/service/leader_lock_test.go @@ -92,10 +92,10 @@ func TestSubscriptionExpiryService_ReminderSkipsScanWhenNotLeader(t *testing.T) _, _ = cache.TryAcquireLeaderLock(context.Background(), subscriptionExpiryReminderLeaderLockKey, "peer", time.Minute) repo := &subscriptionExpiryRepoStub{} - settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{}} + settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{SettingKeySMTPHost: "smtp.example.com"}} svc := NewSubscriptionExpiryService(repo, time.Minute) svc.SetSettingRepository(settingRepo) - svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, nil)) + svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, NewEmailService(settingRepo, nil))) svc.SetLeaderLock(cache, nil) svc.sendExpiryReminders(context.Background()) @@ -105,10 +105,10 @@ func TestSubscriptionExpiryService_ReminderSkipsScanWhenNotLeader(t *testing.T) func TestSubscriptionExpiryService_ReminderScansWhenLeader(t *testing.T) { repo := &subscriptionExpiryRepoStub{} - settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{}} + settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{SettingKeySMTPHost: "smtp.example.com"}} svc := NewSubscriptionExpiryService(repo, time.Minute) svc.SetSettingRepository(settingRepo) - svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, nil)) + svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, NewEmailService(settingRepo, nil))) svc.SetLeaderLock(&fakeLeaderLockCache{}, nil) svc.sendExpiryReminders(context.Background()) @@ -127,10 +127,10 @@ func TestSubscriptionExpiryService_ReminderRunsEveryCycleSingleInstance(t *testi for name, cache := range cases { t.Run(name, func(t *testing.T) { repo := &subscriptionExpiryRepoStub{} - settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{}} + settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{SettingKeySMTPHost: "smtp.example.com"}} svc := NewSubscriptionExpiryService(repo, time.Minute) svc.SetSettingRepository(settingRepo) - svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, nil)) + svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, NewEmailService(settingRepo, nil))) svc.SetLeaderLock(cache, nil) // Three consecutive cycles, mimicking the ticker loop. diff --git a/backend/internal/service/subscription_expiry_service.go b/backend/internal/service/subscription_expiry_service.go index c93c763f02..e27da3661d 100644 --- a/backend/internal/service/subscription_expiry_service.go +++ b/backend/internal/service/subscription_expiry_service.go @@ -15,6 +15,7 @@ import ( ) const ( + subscriptionExpiryReminderSMTPWarningInterval = time.Minute // subscriptionExpiryReminderLeaderLockKey gates the per-cycle reminder scan so // that only one instance walks all active subscriptions and sends reminder // emails, avoiding redundant full scans and duplicate emails. @@ -37,6 +38,9 @@ type SubscriptionExpiryService struct { lockCache LeaderLockCache db *sql.DB instanceID string + + smtpWarningMu sync.Mutex + lastSMTPWarning time.Time } func NewSubscriptionExpiryService(userSubRepo UserSubscriptionRepository, interval time.Duration) *SubscriptionExpiryService { @@ -121,6 +125,9 @@ func (s *SubscriptionExpiryService) sendExpiryReminders(ctx context.Context) { if !s.expiryReminderEnabled(ctx) { return } + if !s.smtpConfigured(ctx) { + return + } // Multi-instance guard: only the leader walks every active subscription and // sends reminders, avoiding N× full scans and duplicate reminder emails. @@ -159,6 +166,28 @@ func (s *SubscriptionExpiryService) expiryReminderEnabled(ctx context.Context) b return !isFalseSettingValue(value) } +func (s *SubscriptionExpiryService) smtpConfigured(ctx context.Context) bool { + if s == nil || s.notificationEmailService == nil || s.notificationEmailService.emailService == nil { + return false + } + _, err := s.notificationEmailService.emailService.GetSMTPConfig(ctx) + if err == nil { + return true + } + if errors.Is(err, ErrEmailNotConfigured) { + s.smtpWarningMu.Lock() + defer s.smtpWarningMu.Unlock() + now := time.Now() + if s.lastSMTPWarning.IsZero() || now.Sub(s.lastSMTPWarning) >= subscriptionExpiryReminderSMTPWarningInterval { + log.Printf("[SubscriptionExpiry] SMTP is not configured; skipping expiry reminders") + s.lastSMTPWarning = now + } + return false + } + log.Printf("[SubscriptionExpiry] Read SMTP configuration failed; skipping expiry reminders: %v", err) + return false +} + func (s *SubscriptionExpiryService) sendExpiryReminderIfDue(ctx context.Context, sub *UserSubscription) { if sub == nil || sub.User == nil || sub.Group == nil || sub.User.Email == "" { return diff --git a/backend/internal/service/subscription_expiry_service_test.go b/backend/internal/service/subscription_expiry_service_test.go index 74b82e56c3..decb816e84 100644 --- a/backend/internal/service/subscription_expiry_service_test.go +++ b/backend/internal/service/subscription_expiry_service_test.go @@ -1,8 +1,10 @@ package service import ( + "bytes" "context" "errors" + "log" "testing" "time" @@ -116,8 +118,9 @@ func (r *subscriptionExpiryRepoStub) BatchUpdateExpiredStatus(context.Context) ( } type subscriptionExpirySettingRepoStub struct { - values map[string]string - err error + values map[string]string + err error + multiErr error } func (r *subscriptionExpirySettingRepoStub) Get(context.Context, string) (*Setting, error) { @@ -139,8 +142,17 @@ func (r *subscriptionExpirySettingRepoStub) Set(context.Context, string, string) return nil } -func (r *subscriptionExpirySettingRepoStub) GetMultiple(context.Context, []string) (map[string]string, error) { - return nil, nil +func (r *subscriptionExpirySettingRepoStub) GetMultiple(_ context.Context, keys []string) (map[string]string, error) { + if r.multiErr != nil { + return nil, r.multiErr + } + values := make(map[string]string, len(keys)) + for _, key := range keys { + if value, ok := r.values[key]; ok { + values[key] = value + } + } + return values, nil } func (r *subscriptionExpirySettingRepoStub) SetMultiple(context.Context, map[string]string) error { @@ -182,3 +194,44 @@ func TestSubscriptionExpiryService_ExpiryReminderSettingReadErrorFailsClosed(t * require.False(t, svc.expiryReminderEnabled(context.Background())) } + +func TestSubscriptionExpiryService_MissingSMTPSkipsReminderScanAndLogsOncePerInterval(t *testing.T) { + repo := &subscriptionExpiryRepoStub{} + settingRepo := &subscriptionExpirySettingRepoStub{values: map[string]string{}} + emailService := NewEmailService(settingRepo, nil) + svc := NewSubscriptionExpiryService(repo, time.Minute) + svc.SetSettingRepository(settingRepo) + svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, emailService)) + + var logs bytes.Buffer + previousWriter := log.Writer() + previousFlags := log.Flags() + log.SetOutput(&logs) + log.SetFlags(0) + t.Cleanup(func() { + log.SetOutput(previousWriter) + log.SetFlags(previousFlags) + }) + + svc.sendExpiryReminders(context.Background()) + svc.sendExpiryReminders(context.Background()) + + require.Zero(t, repo.listCalls) + require.Equal(t, 1, bytes.Count(logs.Bytes(), []byte("SMTP is not configured"))) +} + +func TestSubscriptionExpiryService_SMTPConfigReadErrorSkipsReminderScan(t *testing.T) { + repo := &subscriptionExpiryRepoStub{} + settingRepo := &subscriptionExpirySettingRepoStub{ + values: map[string]string{}, + multiErr: errors.New("db down"), + } + emailService := NewEmailService(settingRepo, nil) + svc := NewSubscriptionExpiryService(repo, time.Minute) + svc.SetSettingRepository(settingRepo) + svc.SetNotificationEmailService(NewNotificationEmailService(settingRepo, emailService)) + + svc.sendExpiryReminders(context.Background()) + + require.Zero(t, repo.listCalls) +}