From e93624b10b1cd1cb54a350145dbaeb3b99e66f19 Mon Sep 17 00:00:00 2001 From: Deluan Date: Wed, 24 Dec 2025 08:11:06 -0500 Subject: [PATCH] refactor(scheduler): streamline scheduling logic and remove unused callback tracking Signed-off-by: Deluan --- plugins/host_scheduler.go | 201 ++++---------- plugins/host_scheduler_test.go | 354 +++++++++++++----------- plugins/testdata/fake-scheduler/main.go | 88 ++---- 3 files changed, 257 insertions(+), 386 deletions(-) diff --git a/plugins/host_scheduler.go b/plugins/host_scheduler.go index b23d9b622..6b9ccf9fb 100644 --- a/plugins/host_scheduler.go +++ b/plugins/host_scheduler.go @@ -14,20 +14,16 @@ import ( const FuncSchedulerCallback = "nd_scheduler_callback" +// timeAfterFunc is a variable for time.AfterFunc, allowing tests to override it. +var timeAfterFunc = time.AfterFunc + // scheduleEntry stores metadata about a scheduled task. type scheduleEntry struct { pluginName string payload string isRecurring bool - entryID int // Internal scheduler entry ID -} - -// callbackRecord stores information about a callback that was invoked (for testing). -type callbackRecord struct { - ScheduleID string - Payload string - IsRecurring bool - Count int + entryID int // Internal scheduler entry ID (for recurring tasks) + timer *time.Timer // Timer for one-time tasks (nil for recurring) } // schedulerServiceImpl implements host.SchedulerService. @@ -39,21 +35,15 @@ type schedulerServiceImpl struct { mu sync.Mutex schedules map[string]*scheduleEntry - - // Callback tracking (for testing) - tracks callbacks invoked on host side - callbackMu sync.Mutex - callbackRecords map[string]*callbackRecord - callbackCount int } // newSchedulerService creates a new SchedulerService for a plugin. func newSchedulerService(pluginName string, manager *Manager, sched scheduler.Scheduler) host.SchedulerService { return &schedulerServiceImpl{ - pluginName: pluginName, - manager: manager, - scheduler: sched, - schedules: make(map[string]*scheduleEntry), - callbackRecords: make(map[string]*callbackRecord), + pluginName: pluginName, + manager: manager, + scheduler: sched, + schedules: make(map[string]*scheduleEntry), } } @@ -63,43 +53,29 @@ func (s *schedulerServiceImpl) ScheduleOneTime(ctx context.Context, delaySeconds } s.mu.Lock() + defer s.mu.Unlock() + if _, exists := s.schedules[scheduleID]; exists { - s.mu.Unlock() return "", fmt.Errorf("schedule ID %q already exists", scheduleID) } - entry := &scheduleEntry{ + capturedID := scheduleID + timer := timeAfterFunc(time.Duration(delaySeconds)*time.Second, func() { + s.invokeCallback(capturedID) + // Clean up the entry after firing + s.mu.Lock() + delete(s.schedules, capturedID) + s.mu.Unlock() + }) + + s.schedules[scheduleID] = &scheduleEntry{ pluginName: s.pluginName, payload: payload, isRecurring: false, - } - s.schedules[scheduleID] = entry - s.mu.Unlock() - - // Use @every syntax for one-time delay - cronExpr := fmt.Sprintf("@every %ds", delaySeconds) - - // Create callback that will fire once and then cancel itself - schedID := scheduleID // capture for closure - callback := func() { - s.invokeCallback(schedID) - // One-time schedules cancel themselves after firing - _ = s.CancelSchedule(context.Background(), schedID) + timer: timer, } - entryID, err := s.scheduler.Add(cronExpr, callback) - if err != nil { - s.mu.Lock() - delete(s.schedules, scheduleID) - s.mu.Unlock() - return "", fmt.Errorf("failed to schedule one-time task: %w", err) - } - - s.mu.Lock() - entry.entryID = entryID - s.mu.Unlock() - - log.Debug(ctx, "Scheduled one-time task", "plugin", s.pluginName, "scheduleID", scheduleID, "delay", delaySeconds) + log.Debug(ctx, "Scheduled one-time task", "plugin", s.pluginName, "scheduleID", scheduleID, "delaySeconds", delaySeconds) return scheduleID, nil } @@ -108,36 +84,29 @@ func (s *schedulerServiceImpl) ScheduleRecurring(ctx context.Context, cronExpres scheduleID = uuid.New().String() } - s.mu.Lock() - if _, exists := s.schedules[scheduleID]; exists { - s.mu.Unlock() - return "", fmt.Errorf("schedule ID %q already exists", scheduleID) - } - - entry := &scheduleEntry{ - pluginName: s.pluginName, - payload: payload, - isRecurring: true, - } - s.schedules[scheduleID] = entry - s.mu.Unlock() - - schedID := scheduleID // capture for closure + capturedID := scheduleID callback := func() { - s.invokeCallback(schedID) + s.invokeCallback(capturedID) + } + + s.mu.Lock() + defer s.mu.Unlock() + + if _, exists := s.schedules[scheduleID]; exists { + return "", fmt.Errorf("schedule ID %q already exists", scheduleID) } entryID, err := s.scheduler.Add(cronExpression, callback) if err != nil { - s.mu.Lock() - delete(s.schedules, scheduleID) - s.mu.Unlock() - return "", fmt.Errorf("failed to schedule recurring task: %w", err) + return "", fmt.Errorf("failed to schedule task: %w", err) } - s.mu.Lock() - entry.entryID = entryID - s.mu.Unlock() + s.schedules[scheduleID] = &scheduleEntry{ + pluginName: s.pluginName, + payload: payload, + isRecurring: true, + entryID: entryID, + } log.Debug(ctx, "Scheduled recurring task", "plugin", s.pluginName, "scheduleID", scheduleID, "cron", cronExpression) return scheduleID, nil @@ -151,10 +120,13 @@ func (s *schedulerServiceImpl) CancelSchedule(ctx context.Context, scheduleID st return fmt.Errorf("schedule ID %q not found", scheduleID) } delete(s.schedules, scheduleID) - entryID := entry.entryID s.mu.Unlock() - s.scheduler.Remove(entryID) + if entry.timer != nil { + entry.timer.Stop() + } else { + s.scheduler.Remove(entry.entryID) + } log.Debug(ctx, "Cancelled schedule", "plugin", s.pluginName, "scheduleID", scheduleID) return nil } @@ -171,7 +143,11 @@ func (s *schedulerServiceImpl) CancelAllForPlugin() { s.mu.Unlock() for scheduleID, entry := range schedules { - s.scheduler.Remove(entry.entryID) + if entry.timer != nil { + entry.timer.Stop() + } else { + s.scheduler.Remove(entry.entryID) + } log.Debug(context.Background(), "Cancelled schedule on plugin unload", "plugin", s.pluginName, "scheduleID", scheduleID) } } @@ -239,73 +215,9 @@ func (s *schedulerServiceImpl) invokeCallback(scheduleID string) { return } - // Track callback invocation on host side (for testing) - s.trackCallback(scheduleID, payload, isRecurring) - log.Debug(ctx, "Scheduler callback completed", "plugin", s.pluginName, "scheduleID", scheduleID, "duration", time.Since(start)) } -// trackCallback records a callback invocation (for testing). -func (s *schedulerServiceImpl) trackCallback(scheduleID, payload string, isRecurring bool) { - s.callbackMu.Lock() - defer s.callbackMu.Unlock() - - s.callbackCount++ - if record, exists := s.callbackRecords[scheduleID]; exists { - record.Count++ - } else { - s.callbackRecords[scheduleID] = &callbackRecord{ - ScheduleID: scheduleID, - Payload: payload, - IsRecurring: isRecurring, - Count: 1, - } - } -} - -// GetCallbackCount returns the total number of callbacks invoked for this service. -// This is primarily used for testing. -func (s *schedulerServiceImpl) GetCallbackCount() int { - s.callbackMu.Lock() - defer s.callbackMu.Unlock() - return s.callbackCount -} - -// GetCallbackRecords returns the callback records for this service. -// This is primarily used for testing. -func (s *schedulerServiceImpl) GetCallbackRecords() map[string]*callbackRecord { - s.callbackMu.Lock() - defer s.callbackMu.Unlock() - // Return a copy - records := make(map[string]*callbackRecord, len(s.callbackRecords)) - for k, v := range s.callbackRecords { - records[k] = &callbackRecord{ - ScheduleID: v.ScheduleID, - Payload: v.Payload, - IsRecurring: v.IsRecurring, - Count: v.Count, - } - } - return records -} - -// ResetCallbackRecords clears the callback tracking state. -// This is primarily used for testing. -func (s *schedulerServiceImpl) ResetCallbackRecords() { - s.callbackMu.Lock() - defer s.callbackMu.Unlock() - s.callbackRecords = make(map[string]*callbackRecord) - s.callbackCount = 0 -} - -// GetScheduleCount returns the number of active schedules for this service. -// This is primarily used for testing. -func (s *schedulerServiceImpl) GetScheduleCount() int { - s.mu.Lock() - defer s.mu.Unlock() - return len(s.schedules) -} - // Verify interface implementation var _ host.SchedulerService = (*schedulerServiceImpl)(nil) @@ -340,22 +252,9 @@ func unregisterSchedulerService(pluginName string) { } } -// getSchedulerService returns the scheduler service for a plugin. +// getSchedulerService returns the scheduler service for a plugin (used by tests). func getSchedulerService(pluginName string) *schedulerServiceImpl { schedulerRegistry.mu.RLock() defer schedulerRegistry.mu.RUnlock() return schedulerRegistry.services[pluginName] } - -// CreateSchedulerHostFunctions creates scheduler host functions for a plugin. -// This should be called during plugin load if the plugin has the scheduler permission. -func CreateSchedulerHostFunctions(pluginName string, manager *Manager) []func() { - sched := scheduler.GetInstance() - service := newSchedulerService(pluginName, manager, sched).(*schedulerServiceImpl) - registerSchedulerService(pluginName, service) - - // Return a cleanup function - return []func(){ - func() { unregisterSchedulerService(pluginName) }, - } -} diff --git a/plugins/host_scheduler_test.go b/plugins/host_scheduler_test.go index 0e991b1ca..b4f56164c 100644 --- a/plugins/host_scheduler_test.go +++ b/plugins/host_scheduler_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "sync" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -17,13 +18,15 @@ import ( var _ = Describe("SchedulerService", Ordered, func() { var ( - manager *Manager - tmpDir string - mockSched *mockScheduler + manager *Manager + tmpDir string + mockSched *mockScheduler + mockTimers *mockTimerRegistry + testService *testableSchedulerService + origAfterFn func(time.Duration, func()) *time.Timer ) BeforeAll(func() { - // Create temp directory var err error tmpDir, err = os.MkdirTemp("", "scheduler-test-*") Expect(err).ToNot(HaveOccurred()) @@ -43,8 +46,13 @@ var _ = Describe("SchedulerService", Ordered, func() { conf.Server.Plugins.AutoReload = false conf.Server.CacheFolder = filepath.Join(tmpDir, "cache") - // Create mock scheduler + // Create mock scheduler and timer registry mockSched = newMockScheduler() + mockTimers = newMockTimerRegistry() + + // Replace timeAfterFunc with mock + origAfterFn = timeAfterFunc + timeAfterFunc = mockTimers.AfterFunc // Create and start manager manager = &Manager{ @@ -53,31 +61,23 @@ var _ = Describe("SchedulerService", Ordered, func() { err = manager.Start(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) - // Replace the scheduler in the service with our mock + // Wrap the scheduler service and replace the scheduler with our mock service := getSchedulerService("fake-scheduler") - if service != nil { - service.scheduler = mockSched - } + Expect(service).ToNot(BeNil()) + testService = &testableSchedulerService{schedulerServiceImpl: service} + testService.scheduler = mockSched DeferCleanup(func() { + timeAfterFunc = origAfterFn _ = manager.Stop() _ = os.RemoveAll(tmpDir) }) }) - // Reset state between tests BeforeEach(func() { mockSched.Reset() - service := getSchedulerService("fake-scheduler") - if service != nil { - service.ResetCallbackRecords() - // Clear any pending schedules - service.mu.Lock() - for id := range service.schedules { - delete(service.schedules, id) - } - service.mu.Unlock() - } + mockTimers.Reset() + testService.ClearSchedules() }) Describe("Plugin Loading", func() { @@ -93,170 +93,140 @@ var _ = Describe("SchedulerService", Ordered, func() { }) Describe("ScheduleOneTime", func() { - It("should schedule a one-time callback", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule a callback - scheduleID, err := service.ScheduleOneTime(GinkgoT().Context(), 1, "test-payload", "test-id") + It("should schedule a one-time task", func() { + scheduleID, err := testService.ScheduleOneTime(GinkgoT().Context(), 1, "test-payload", "test-id") Expect(err).ToNot(HaveOccurred()) Expect(scheduleID).To(Equal("test-id")) // Verify schedule was registered - Expect(service.GetScheduleCount()).To(Equal(1)) - Expect(mockSched.GetCallbackCount()).To(Equal(1)) - - // Manually trigger the callback - mockSched.TriggerAll() - - // Verify callback was invoked - Expect(service.GetCallbackCount()).To(Equal(1)) + Expect(testService.GetScheduleCount()).To(Equal(1)) + Expect(mockTimers.GetTimerCount()).To(Equal(1)) }) - It("should pass payload to callback", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule with specific payload - scheduleID, err := service.ScheduleOneTime(GinkgoT().Context(), 1, "my-test-data", "custom-id") + It("should invoke plugin callback and auto-cleanup after firing", func() { + _, err := testService.ScheduleOneTime(GinkgoT().Context(), 1, "data", "cleanup-id") Expect(err).ToNot(HaveOccurred()) - Expect(scheduleID).To(Equal("custom-id")) + Expect(testService.GetScheduleCount()).To(Equal(1)) - // Trigger callback - mockSched.TriggerAll() + // Trigger fires the callback which calls the plugin's nd_scheduler_callback + // One-time schedules clean up after the callback completes + mockTimers.TriggerAll() - // Verify payload was received - records := service.GetCallbackRecords() - Expect(records).To(HaveKey("custom-id")) - Expect(records["custom-id"].Payload).To(Equal("my-test-data")) - Expect(records["custom-id"].IsRecurring).To(BeFalse()) + // One-time schedules should self-cleanup + Expect(testService.GetScheduleCount()).To(Equal(0)) }) It("should reject duplicate schedule ID", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule first - _, err := service.ScheduleOneTime(GinkgoT().Context(), 60, "data", "dup-id") + _, err := testService.ScheduleOneTime(GinkgoT().Context(), 60, "data", "dup-id") Expect(err).ToNot(HaveOccurred()) - // Try to schedule with same ID - _, err = service.ScheduleOneTime(GinkgoT().Context(), 60, "data2", "dup-id") + _, err = testService.ScheduleOneTime(GinkgoT().Context(), 60, "data2", "dup-id") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("already exists")) }) - It("should clean up one-time schedule after firing", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule a callback - _, err := service.ScheduleOneTime(GinkgoT().Context(), 1, "cleanup-test", "cleanup-id") - Expect(err).ToNot(HaveOccurred()) - - // Verify schedule exists - Expect(service.GetScheduleCount()).To(Equal(1)) - - // Trigger callback (one-time schedules self-cancel) - mockSched.TriggerAll() - - // Schedule should be cleaned up - Expect(service.GetScheduleCount()).To(Equal(0)) - }) - It("should auto-generate schedule ID when empty", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule without providing ID - scheduleID, err := service.ScheduleOneTime(GinkgoT().Context(), 1, "data", "") + scheduleID, err := testService.ScheduleOneTime(GinkgoT().Context(), 1, "data", "") Expect(err).ToNot(HaveOccurred()) - Expect(scheduleID).ToNot(BeEmpty()) - // UUID format - Expect(scheduleID).To(HaveLen(36)) + Expect(scheduleID).To(HaveLen(36)) // UUID format }) }) Describe("ScheduleRecurring", func() { - It("should schedule recurring callbacks", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule recurring task - scheduleID, err := service.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "recurring", "recurring-id") + It("should schedule recurring tasks", func() { + scheduleID, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "recurring-data", "recurring-id") Expect(err).ToNot(HaveOccurred()) Expect(scheduleID).To(Equal("recurring-id")) - // Trigger multiple times - mockSched.TriggerAll() - mockSched.TriggerAll() - - // Verify callback count - Expect(service.GetCallbackCount()).To(Equal(2)) - - // Verify records show recurring - records := service.GetCallbackRecords() - Expect(records).To(HaveKey("recurring-id")) - Expect(records["recurring-id"].IsRecurring).To(BeTrue()) - Expect(records["recurring-id"].Count).To(Equal(2)) + // Verify schedule was registered + Expect(testService.GetScheduleCount()).To(Equal(1)) + entry := testService.GetSchedule("recurring-id") + Expect(entry).ToNot(BeNil()) + Expect(entry.isRecurring).To(BeTrue()) }) - It("should not self-cancel recurring schedules", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule recurring task - _, err := service.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "data", "persist-id") + It("should invoke plugin callback multiple times without self-canceling", func() { + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "data", "persist-id") Expect(err).ToNot(HaveOccurred()) - // Trigger multiple times + // Trigger multiple times - recurring schedules should persist mockSched.TriggerAll() mockSched.TriggerAll() - // Schedule should still exist (recurring doesn't self-cancel) - Expect(service.GetScheduleCount()).To(Equal(1)) + // Recurring schedules should persist + Expect(testService.GetScheduleCount()).To(Equal(1)) + }) + }) + + Describe("Plugin Calling Host Functions", func() { + It("should allow plugin to schedule a one-time task from callback", func() { + // Schedule with magic payload that triggers plugin to call SchedulerScheduleOneTime + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "schedule-followup", "trigger-id") + Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(1)) + + // Trigger - plugin callback will schedule a follow-up task + mockSched.TriggerAll() + + // Verify the plugin created a new schedule via host function + Expect(testService.GetScheduleCount()).To(Equal(2)) // original + followup + + // Verify the follow-up schedule was created with correct ID and properties + followup := testService.GetSchedule("followup-id") + Expect(followup).ToNot(BeNil()) + Expect(followup.payload).To(Equal("followup-created")) + Expect(followup.isRecurring).To(BeFalse()) + Expect(followup.timer).ToNot(BeNil()) // One-time tasks use timers }) - It("should reject invalid cron expression", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Note: The mock scheduler doesn't validate cron expressions, - // but the real scheduler would. This test verifies behavior - // when the scheduler returns an error. - // For now, just verify the method works with a valid expression - _, err := service.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "data", "") + It("should allow plugin to schedule a recurring task from callback", func() { + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "schedule-recurring", "trigger-id") Expect(err).ToNot(HaveOccurred()) + + mockSched.TriggerAll() + + // Verify the plugin created a recurring schedule + entry := testService.GetSchedule("recurring-from-plugin") + Expect(entry).ToNot(BeNil()) + Expect(entry.isRecurring).To(BeTrue()) + Expect(entry.payload).To(Equal("recurring-created")) }) }) Describe("CancelSchedule", func() { - It("should cancel a scheduled task", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule recurring task - _, err := service.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "cancel-test", "cancel-id") + It("should cancel a recurring task", func() { + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "data", "cancel-id") Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(1)) - Expect(service.GetScheduleCount()).To(Equal(1)) - - // Cancel - err = service.CancelSchedule(GinkgoT().Context(), "cancel-id") + err = testService.CancelSchedule(GinkgoT().Context(), "cancel-id") Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(0)) + }) - Expect(service.GetScheduleCount()).To(Equal(0)) + It("should cancel a one-time task", func() { + _, err := testService.ScheduleOneTime(GinkgoT().Context(), 60, "data", "cancel-onetime-id") + Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(1)) + Expect(mockTimers.GetTimerCount()).To(Equal(1)) - // Trigger should not invoke callback - mockSched.TriggerAll() - Expect(service.GetCallbackCount()).To(Equal(0)) + err = testService.CancelSchedule(GinkgoT().Context(), "cancel-onetime-id") + Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(0)) + }) + + It("should remove callback from scheduler for recurring tasks", func() { + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 1s", "data", "cancel-id") + Expect(err).ToNot(HaveOccurred()) + Expect(mockSched.GetCallbackCount()).To(Equal(1)) + + err = testService.CancelSchedule(GinkgoT().Context(), "cancel-id") + Expect(err).ToNot(HaveOccurred()) + Expect(mockSched.GetCallbackCount()).To(Equal(0)) }) It("should return error for non-existent schedule", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - err := service.CancelSchedule(GinkgoT().Context(), "non-existent") + err := testService.CancelSchedule(GinkgoT().Context(), "non-existent") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("not found")) }) @@ -264,29 +234,47 @@ var _ = Describe("SchedulerService", Ordered, func() { Describe("Plugin Unload", func() { It("should cancel all schedules when plugin is unloaded", func() { - service := getSchedulerService("fake-scheduler") - Expect(service).ToNot(BeNil()) - - // Schedule multiple tasks - _, err := service.ScheduleRecurring(GinkgoT().Context(), "@every 10s", "data1", "unload-1") + _, err := testService.ScheduleRecurring(GinkgoT().Context(), "@every 10s", "data1", "unload-1") Expect(err).ToNot(HaveOccurred()) - _, err = service.ScheduleRecurring(GinkgoT().Context(), "@every 10s", "data2", "unload-2") + _, err = testService.ScheduleOneTime(GinkgoT().Context(), 60, "data2", "unload-2") Expect(err).ToNot(HaveOccurred()) + Expect(testService.GetScheduleCount()).To(Equal(2)) + Expect(mockSched.GetCallbackCount()).To(Equal(1)) // Only recurring task uses scheduler + Expect(mockTimers.GetTimerCount()).To(Equal(1)) // Only one-time task uses timer - Expect(service.GetScheduleCount()).To(Equal(2)) - - // Unload plugin err = manager.UnloadPlugin("fake-scheduler") Expect(err).ToNot(HaveOccurred()) - // Verify scheduler service was cleaned up Expect(getSchedulerService("fake-scheduler")).To(BeNil()) + Expect(mockSched.GetCallbackCount()).To(Equal(0)) // Recurring task removed }) }) }) +// testableSchedulerService wraps schedulerServiceImpl with test helpers. +type testableSchedulerService struct { + *schedulerServiceImpl +} + +func (t *testableSchedulerService) GetScheduleCount() int { + t.mu.Lock() + defer t.mu.Unlock() + return len(t.schedules) +} + +func (t *testableSchedulerService) GetSchedule(id string) *scheduleEntry { + t.mu.Lock() + defer t.mu.Unlock() + return t.schedules[id] +} + +func (t *testableSchedulerService) ClearSchedules() { + t.mu.Lock() + defer t.mu.Unlock() + t.schedules = make(map[string]*scheduleEntry) +} + // mockScheduler implements scheduler.Scheduler for testing without timing dependencies. -// It allows tests to manually trigger callbacks. type mockScheduler struct { mu sync.Mutex callbacks map[int]func() @@ -300,9 +288,7 @@ func newMockScheduler() *mockScheduler { } } -func (s *mockScheduler) Run(_ context.Context) { - // No-op for mock - we trigger callbacks manually -} +func (s *mockScheduler) Run(_ context.Context) {} func (s *mockScheduler) Add(_ string, cmd func()) (int, error) { s.mu.Lock() @@ -319,19 +305,6 @@ func (s *mockScheduler) Remove(id int) { delete(s.callbacks, id) } -// TriggerCallback manually triggers a callback by its entry ID. -func (s *mockScheduler) TriggerCallback(id int) bool { - s.mu.Lock() - cb, exists := s.callbacks[id] - s.mu.Unlock() - if exists && cb != nil { - cb() - return true - } - return false -} - -// TriggerAll triggers all registered callbacks. func (s *mockScheduler) TriggerAll() { s.mu.Lock() callbacks := make([]func(), 0, len(s.callbacks)) @@ -344,14 +317,12 @@ func (s *mockScheduler) TriggerAll() { } } -// GetCallbackCount returns the number of registered callbacks. func (s *mockScheduler) GetCallbackCount() int { s.mu.Lock() defer s.mu.Unlock() return len(s.callbacks) } -// Reset clears all callbacks and resets the ID counter. func (s *mockScheduler) Reset() { s.mu.Lock() defer s.mu.Unlock() @@ -360,3 +331,58 @@ func (s *mockScheduler) Reset() { } var _ scheduler.Scheduler = (*mockScheduler)(nil) + +// mockTimerRegistry tracks mock timers created during tests. +type mockTimerRegistry struct { + mu sync.Mutex + callbacks []func() + timers []*time.Timer +} + +func newMockTimerRegistry() *mockTimerRegistry { + return &mockTimerRegistry{ + callbacks: make([]func(), 0), + timers: make([]*time.Timer, 0), + } +} + +// AfterFunc creates a timer that we control for testing. +func (r *mockTimerRegistry) AfterFunc(_ time.Duration, f func()) *time.Timer { + r.mu.Lock() + defer r.mu.Unlock() + + // Store callback for TriggerAll + r.callbacks = append(r.callbacks, f) + + // Create a real timer that won't fire (very long duration, immediately stopped) + t := time.NewTimer(time.Hour * 24 * 365) + t.Stop() + r.timers = append(r.timers, t) + + return t +} + +// TriggerAll fires all pending timer callbacks. +func (r *mockTimerRegistry) TriggerAll() { + r.mu.Lock() + callbacks := make([]func(), len(r.callbacks)) + copy(callbacks, r.callbacks) + r.mu.Unlock() + + for _, cb := range callbacks { + cb() + } +} + +func (r *mockTimerRegistry) GetTimerCount() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.callbacks) +} + +func (r *mockTimerRegistry) Reset() { + r.mu.Lock() + defer r.mu.Unlock() + r.callbacks = make([]func(), 0) + r.timers = make([]*time.Timer, 0) +} diff --git a/plugins/testdata/fake-scheduler/main.go b/plugins/testdata/fake-scheduler/main.go index ef9d41863..7d31fa0db 100644 --- a/plugins/testdata/fake-scheduler/main.go +++ b/plugins/testdata/fake-scheduler/main.go @@ -1,13 +1,9 @@ // Fake scheduler plugin for Navidrome plugin system integration tests. -// This plugin was created based on the scheduler_callback.yaml XTP schema. // Build with: tinygo build -o ../fake-scheduler.wasm -target wasip1 -buildmode=c-shared . -// -// Note: pdk.gen.go contains the domain types from the XTP schema where your plugin will run. package main import ( "encoding/json" - "strconv" pdk "github.com/extism/go-pdk" ) @@ -29,21 +25,8 @@ type SchedulerPermission struct { Reason string `json:"reason,omitempty"` } -// CallRecord stores information about a callback that was received (for testing) -type CallRecord struct { - ScheduleID string `json:"schedule_id"` - Payload string `json:"payload"` - IsRecurring bool `json:"is_recurring"` - CallCount int `json:"call_count"` -} - -// Global state for tracking callbacks (var stores persist in wasm memory between calls) -var callRecords = make(map[string]*CallRecord) -var totalCallCount = 0 - //go:wasmexport nd_manifest func ndManifest() int32 { - reason := "For testing scheduler callbacks" manifest := Manifest{ Name: "Fake Scheduler", Author: "Navidrome Test", @@ -51,7 +34,7 @@ func ndManifest() int32 { Description: "A fake scheduler plugin for integration testing", Permissions: &Permissions{ Scheduler: &SchedulerPermission{ - Reason: reason, + Reason: "For testing scheduler callbacks", }, }, } @@ -64,63 +47,26 @@ func ndManifest() int32 { return 0 } -// NdSchedulerCallback implements the scheduler callback logic. -// Called when a scheduled task fires. -// This function is called by the generated wrapper in pdk.gen.go. +// NdSchedulerCallback is called when a scheduled task fires. +// Magic payloads trigger specific behaviors to test host functions: +// - "schedule-followup": schedules a one-time task via host function +// - "schedule-recurring": schedules a recurring task via host function func NdSchedulerCallback(input SchedulerCallbackInput) (SchedulerCallbackOutput, error) { - // Check for configured error response - errCfg, hasErr := pdk.GetConfig("callback_error") - if hasErr && errCfg != "" { - return SchedulerCallbackOutput{Error: &errCfg}, nil - } - - // Track the callback - totalCallCount++ - if record, exists := callRecords[input.ScheduleId]; exists { - record.CallCount++ - } else { - callRecords[input.ScheduleId] = &CallRecord{ - ScheduleID: input.ScheduleId, - Payload: input.Payload, - IsRecurring: input.IsRecurring, - CallCount: 1, + switch input.Payload { + case "schedule-followup": + _, err := SchedulerScheduleOneTime(1, "followup-created", "followup-id") + if err != nil { + errStr := err.Error() + return SchedulerCallbackOutput{Error: &errStr}, nil + } + case "schedule-recurring": + _, err := SchedulerScheduleRecurring("@every 1s", "recurring-created", "recurring-from-plugin") + if err != nil { + errStr := err.Error() + return SchedulerCallbackOutput{Error: &errStr}, nil } } - - // Log the callback for debugging - pdk.Log(pdk.LogInfo, "Scheduler callback received: "+input.ScheduleId+" payload="+input.Payload) - return SchedulerCallbackOutput{}, nil } -// Helper function to get call records (for testing) -// -//go:wasmexport nd_get_call_records -func ndGetCallRecords() int32 { - out, err := json.Marshal(callRecords) - if err != nil { - pdk.SetError(err) - return 1 - } - pdk.Output(out) - return 0 -} - -// Helper function to get total call count (for testing) -// -//go:wasmexport nd_get_total_call_count -func ndGetTotalCallCount() int32 { - pdk.Output([]byte(strconv.Itoa(totalCallCount))) - return 0 -} - -// Helper function to reset call records (for testing) -// -//go:wasmexport nd_reset_call_records -func ndResetCallRecords() int32 { - callRecords = make(map[string]*CallRecord) - totalCallCount = 0 - return 0 -} - func main() {}