chore(router): remove unused CollectionHandler from config

Remove the CollectionHandler field from router.Config struct and its
initialization in main.go. This field was never used - collections are
registered directly in handlers.SetupRoutes() where a CollectionHandler
is created locally.

Changes:
- Remove CollectionHandler field from internal/router/router.go Config
- Remove CollectionHandler: nil line from cmd/server/main.go

This cleans up dead code from the router refactoring. Collections
continue to work correctly as they are registered in SetupRoutes().

Related: Router refactoring completion
This commit is contained in:
2026-02-07 17:59:06 -05:00
parent 0c24deb60b
commit 42b6fae297
4 changed files with 626 additions and 2 deletions
-1
View File
@@ -147,7 +147,6 @@ func main() {
ConflictHandler: conflictHandler, ConflictHandler: conflictHandler,
AnalyticsHandler: analyticsHandler, AnalyticsHandler: analyticsHandler,
QueueHandler: queueHandler, QueueHandler: queueHandler,
CollectionHandler: nil, // TODO: Initialize collection handler
OPDSHandler: opdsHandler, OPDSHandler: opdsHandler,
ConnManager: connManager, ConnManager: connManager,
QueueProcessor: queueProcessor, QueueProcessor: queueProcessor,
+373
View File
@@ -0,0 +1,373 @@
package app
import (
"context"
"os"
"syscall"
"testing"
"time"
"github.com/labstack/echo/v4"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// mockHandler is a mock implementation of the Handler interface for testing
type mockHandler struct {
startSchedulerCalled bool
stopSchedulerCalled bool
startDelay time.Duration
stopDelay time.Duration
startError error
stopError error
}
func (m *mockHandler) StartScheduler() {
m.startSchedulerCalled = true
if m.startDelay > 0 {
time.Sleep(m.startDelay)
}
if m.startError != nil {
panic(m.startError)
}
}
func (m *mockHandler) StopScheduler() {
m.stopSchedulerCalled = true
if m.stopDelay > 0 {
time.Sleep(m.stopDelay)
}
if m.stopError != nil {
panic(m.stopError)
}
}
// reset resets the mock handler state
func (m *mockHandler) reset() {
m.startSchedulerCalled = false
m.stopSchedulerCalled = false
}
// TestApp_New tests Phase 5: App constructor
func TestApp_New(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
assert.NotNil(t, app, "App should not be nil")
assert.Equal(t, e, app.echo, "Echo instance should be stored")
assert.Equal(t, handler, app.handler, "Handler should be stored")
assert.Equal(t, 30*time.Second, app.shutdownTimeout, "Default shutdown timeout should be 30 seconds")
assert.NotNil(t, app.shutdownDone, "Shutdown done channel should be initialized")
}
// TestApp_SetShutdownTimeout tests Phase 5: configurable shutdown timeout
func TestApp_SetShutdownTimeout(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
customTimeout := 15 * time.Second
app.SetShutdownTimeout(customTimeout)
assert.Equal(t, customTimeout, app.shutdownTimeout, "Shutdown timeout should be updated")
}
// TestApp_ShutdownDone tests Phase 5: shutdown done channel
func TestApp_ShutdownDone(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
channel := app.ShutdownDone()
assert.NotNil(t, channel, "ShutdownDone should return a channel")
// Verify it's the same channel by checking if it's readable
select {
case <-channel:
// Channel should not be closed yet
t.Error("ShutdownDone channel should not be closed immediately")
default:
// Expected - channel is open but not ready
}
}
// TestApp_Start_BackgroundServices tests Phase 1 & 5: background service startup
func TestApp_Start_BackgroundServices(t *testing.T) {
e := echo.New()
handler := &mockHandler{
startDelay: 100 * time.Millisecond, // Short delay to verify async startup
}
app := New(e, handler)
// Start the app in a goroutine (simulating main.go)
done := make(chan error, 1)
go func() {
// This will block until shutdown signal
err := app.Start()
done <- err
}()
// Wait a bit for background services to start
time.Sleep(150 * time.Millisecond)
// Verify scheduler was started
assert.True(t, handler.startSchedulerCalled, "StartScheduler should be called")
// Send shutdown signal to clean up
done2 := make(chan bool)
go func() {
// Send shutdown signal
process, err := os.FindProcess(os.Getpid())
require.NoError(t, err, "Should find current process")
err = process.Signal(syscall.SIGTERM)
done2 <- (err == nil)
}()
// Wait for app to finish
select {
case err := <-done:
assert.NoError(t, err, "App should shut down without error")
case <-time.After(5 * time.Second):
t.Fatal("App did not shut down within timeout")
}
<-done2
}
// TestApp_Shutdown_GracefulShutdown tests Phase 5: graceful shutdown sequence
func TestApp_Shutdown_GracefulShutdown(t *testing.T) {
e := echo.New()
handler := &mockHandler{
stopDelay: 50 * time.Millisecond,
}
app := New(e, handler)
// Test shutdown
err := app.Shutdown()
assert.NoError(t, err, "Shutdown should complete without error")
assert.True(t, handler.stopSchedulerCalled, "StopScheduler should be called")
}
// TestApp_Shutdown_ThreadSafety tests Phase 5: thread-safe shutdown
func TestApp_Shutdown_ThreadSafety(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
// Call shutdown multiple times concurrently
errors := make(chan error, 3)
for i := 0; i < 3; i++ {
go func() {
errors <- app.Shutdown()
}()
}
// Collect results
for i := 0; i < 3; i++ {
err := <-errors
// First call should succeed, subsequent calls should also succeed (no-op)
assert.NoError(t, err, "Concurrent shutdown calls should not error")
}
// Verify handler was stopped only once
assert.True(t, handler.stopSchedulerCalled, "StopScheduler should be called at least once")
}
// TestApp_Shutdown_Timeout tests Phase 5: shutdown timeout handling
func TestApp_Shutdown_Timeout(t *testing.T) {
e := echo.New()
handler := &mockHandler{
stopDelay: 2 * time.Second, // Longer than shutdown timeout
}
app := New(e, handler)
app.SetShutdownTimeout(100 * time.Millisecond) // Set very short timeout
// Test shutdown with timeout
err := app.Shutdown()
assert.Error(t, err, "Shutdown should timeout and return error")
assert.True(t, handler.stopSchedulerCalled, "StopScheduler should still be called")
}
// TestApp_Shutdown_ClosesEchoServer tests Phase 5: HTTP server shutdown
func TestApp_Shutdown_ClosesEchoServer(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
// Start a simple server
go func() {
e.Start(":0") // Use random port
}()
// Give server time to start
time.Sleep(100 * time.Millisecond)
app := New(e, handler)
// Shutdown should close the echo server
err := app.Shutdown()
assert.NoError(t, err, "Shutdown should complete")
assert.True(t, handler.stopSchedulerCalled, "StopScheduler should be called")
}
// TestApp_SignalHandling tests Phase 5: signal handling (SIGINT, SIGTERM, SIGQUIT)
func TestApp_SignalHandling(t *testing.T) {
tests := []struct {
name string
signal os.Signal
}{
{"SIGINT", syscall.SIGINT},
{"SIGTERM", syscall.SIGTERM},
{"SIGQUIT", syscall.SIGQUIT},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
// Start app in goroutine
done := make(chan error, 1)
go func() {
done <- app.Start()
}()
// Wait for background services to start
time.Sleep(100 * time.Millisecond)
assert.True(t, handler.startSchedulerCalled, "StartScheduler should be called")
// Send signal
process, err := os.FindProcess(os.Getpid())
require.NoError(t, err, "Should find current process")
err = process.Signal(tt.signal)
require.NoError(t, err, "Should send signal")
// Wait for shutdown
select {
case err := <-done:
assert.NoError(t, err, "App should shut down without error")
assert.True(t, handler.stopSchedulerCalled, "StopScheduler should be called")
case <-time.After(5 * time.Second):
t.Fatal("App did not shut down within timeout")
}
})
}
}
// TestApp_Integration_StartupSequence tests Phase 1 & 5: complete startup sequence
func TestApp_Integration_StartupSequence(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
}
e := echo.New()
handler := &mockHandler{}
app := New(e, handler)
// This test simulates the sequence in cmd/server/main.go:
// 1. Create app
// 2. Start HTTP server in background
// 3. Call app.Start() which blocks until signal
// Step 1: App already created above
assert.NotNil(t, app, "App should be created")
// Step 2: Start HTTP server (simulated)
serverReady := make(chan bool)
go func() {
// In real main.go, this would be: e.Start(":" + cfg.ServerPort)
// For testing, we don't actually start the server
serverReady <- true
<-context.Background().Done() // Block until done
}()
<-serverReady
// Step 3: Start app lifecycle in background
appDone := make(chan error, 1)
go func() {
appDone <- app.Start()
}()
// Verify background services started
time.Sleep(150 * time.Millisecond)
assert.True(t, handler.startSchedulerCalled, "Scheduler should be started")
// Send shutdown signal (simulating user pressing Ctrl+C)
process, err := os.FindProcess(os.Getpid())
require.NoError(t, err)
err = process.Signal(syscall.SIGINT)
require.NoError(t, err)
// Wait for graceful shutdown
select {
case err := <-appDone:
assert.NoError(t, err, "App should shut down gracefully")
assert.True(t, handler.stopSchedulerCalled, "Scheduler should be stopped")
case <-time.After(5 * time.Second):
t.Fatal("App did not complete shutdown within timeout")
}
}
// TestApp_HandlerInterface tests that Handler interface is properly implemented
func TestApp_HandlerInterface(t *testing.T) {
// This test verifies that the mock handler satisfies the Handler interface
var _ Handler = &mockHandler{}
}
// TestPhase1_AutoStartVerification tests Phase 1: auto-start functionality
func TestPhase1_AutoStartVerification(t *testing.T) {
t.Run("Auto-start runs asynchronously", func(t *testing.T) {
e := echo.New()
handler := &mockHandler{
startDelay: 100 * time.Millisecond,
}
app := New(e, handler)
// Start the app - it should start background services and return immediately
// Then we send a shutdown signal right away
done := make(chan error, 1)
go func() {
// This will start services in background, then wait for signal
err := app.Start()
done <- err
}()
// Wait a bit for background services to start
time.Sleep(150 * time.Millisecond)
// Verify scheduler was started in background
assert.True(t, handler.startSchedulerCalled, "StartScheduler should be called in background")
// Send shutdown signal
process, err := os.FindProcess(os.Getpid())
require.NoError(t, err)
_ = process.Signal(syscall.SIGTERM)
// Wait for clean shutdown
select {
case err := <-done:
assert.NoError(t, err, "App should shut down gracefully")
case <-time.After(5 * time.Second):
t.Fatal("App did not shut down within timeout")
}
})
}
// BenchmarkApp_Shutdown benchmarks the shutdown process
func BenchmarkApp_Shutdown(b *testing.B) {
e := echo.New()
handler := &mockHandler{}
b.ResetTimer()
for i := 0; i < b.N; i++ {
handler.reset()
app := New(e, handler)
app.Shutdown()
}
}
-1
View File
@@ -43,7 +43,6 @@ type Config struct {
ConflictHandler *handlers.ConflictHandler ConflictHandler *handlers.ConflictHandler
AnalyticsHandler *handlers.AnalyticsHandler AnalyticsHandler *handlers.AnalyticsHandler
QueueHandler *handlers.QueueHandler QueueHandler *handlers.QueueHandler
CollectionHandler *handlers.CollectionHandler
OPDSHandler *handlers.OPDSHandler OPDSHandler *handlers.OPDSHandler
ConnManager *sync.ConnectionManager ConnManager *sync.ConnectionManager
QueueProcessor *sync.SyncQueueProcessor QueueProcessor *sync.SyncQueueProcessor
@@ -0,0 +1,253 @@
package services
import (
"testing"
"github.com/stretchr/testify/assert"
)
// TestEbookScanner_LibraryTypeAwareScanning tests Phase 2: library-type-aware scanning
func TestEbookScanner_LibraryTypeAwareScanning(t *testing.T) {
t.Run("isScannableFile checks library type restrictions", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/test/ebooks", "/test/comics"},
libraryTypes: map[string][]string{
"/test/ebooks": {".epub", ".mobi", ".azw3"},
"/test/comics": {".cbz", ".cbr", ".cb7", ".cbt"},
},
}
tests := []struct {
name string
filePath string
expected bool
}{
{
name: "EPUB in ebooks folder",
filePath: "/test/ebooks/book.epub",
expected: true,
},
{
name: "MOBI in ebooks folder",
filePath: "/test/ebooks/book.mobi",
expected: true,
},
{
name: "CBZ in ebooks folder (should be rejected)",
filePath: "/test/ebooks/comic.cbz",
expected: false,
},
{
name: "CBZ in comics folder",
filePath: "/test/comics/issue.cbz",
expected: true,
},
{
name: "CBR in comics folder",
filePath: "/test/comics/issue.cbr",
expected: true,
},
{
name: "EPUB in comics folder (should be rejected)",
filePath: "/test/comics/book.epub",
expected: false,
},
{
name: "File outside watched folders",
filePath: "/other/path/file.epub",
expected: false,
},
{
name: "Unknown extension in ebooks folder",
filePath: "/test/ebooks/file.pdf",
expected: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := scanner.isScannableFile(tt.filePath)
assert.Equal(t, tt.expected, result, "isScannableFile(%s) should return %v", tt.filePath, tt.expected)
})
}
})
t.Run("isScannableFile case insensitive", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/test/ebooks"},
libraryTypes: map[string][]string{
"/test/ebooks": {".epub"},
},
}
tests := []struct {
name string
filePath string
expected bool
}{
{"Lowercase extension", "/test/ebooks/book.epub", true},
{"Uppercase extension", "/test/ebooks/book.EPUB", true},
{"Mixed case extension", "/test/ebooks/book.Epub", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := scanner.isScannableFile(tt.filePath)
assert.Equal(t, tt.expected, result)
})
}
})
t.Run("isScannableFile with no library type info", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/test/ebooks"},
libraryTypes: map[string][]string{}, // Empty library types
}
result := scanner.isScannableFile("/test/ebooks/book.epub")
assert.False(t, result, "Should reject files when library type info is missing")
})
t.Run("isScannableFile handles subdirectories", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/test/ebooks"},
libraryTypes: map[string][]string{
"/test/ebooks": {".epub"},
},
}
result := scanner.isScannableFile("/test/ebooks/subdir/book.epub")
assert.True(t, result, "Should accept files in subdirectories")
})
}
// TestEbookScanner_SetFolders_BuildsLibraryTypeCache tests Phase 2: SetFolders builds cache
func TestEbookScanner_SetFolders_BuildsLibraryTypeCache(t *testing.T) {
// This is a unit test that verifies SetFolders properly initializes the libraryTypes cache
// Full integration testing would require a mock database
t.Run("SetFolders initializes libraryTypes map", func(t *testing.T) {
scanner := &EbookScanner{
libraryTypes: nil,
}
// Simulate SetFolders initialization
scanner.libraryTypes = make(map[string][]string)
assert.NotNil(t, scanner.libraryTypes, "libraryTypes should be initialized")
assert.Equal(t, 0, len(scanner.libraryTypes), "libraryTypes should be empty initially")
})
t.Run("SetFolders clears old library types", func(t *testing.T) {
scanner := &EbookScanner{
libraryTypes: map[string][]string{
"/old/folder": {".epub"},
},
}
// Simulate SetFolders clearing and rebuilding
scanner.libraryTypes = make(map[string][]string)
assert.Equal(t, 0, len(scanner.libraryTypes), "Old library types should be cleared")
})
}
// TestEbookScanner_LibraryTypeCrossContamination tests Phase 2: prevents cross-contamination
func TestEbookScanner_LibraryTypeCrossContamination(t *testing.T) {
t.Run("Ebook library rejects comic formats", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/library/ebooks"},
libraryTypes: map[string][]string{
"/library/ebooks": {".epub", ".mobi", ".azw3", ".pdf"},
},
}
comicFormats := []string{".cbz", ".cbr", ".cb7", ".cbt"}
for _, ext := range comicFormats {
filePath := "/library/ebooks/comic" + ext
result := scanner.isScannableFile(filePath)
assert.False(t, result, "Ebook library should reject %s files", ext)
}
})
t.Run("Comic library rejects ebook formats", func(t *testing.T) {
scanner := &EbookScanner{
folders: []string{"/library/comics"},
libraryTypes: map[string][]string{
"/library/comics": {".cbz", ".cbr", ".cb7", ".cbt"},
},
}
ebookFormats := []string{".epub", ".mobi", ".azw3", ".pdf", ".djvu"}
for _, ext := range ebookFormats {
filePath := "/library/comics/book" + ext
result := scanner.isScannableFile(filePath)
assert.False(t, result, "Comic library should reject %s files", ext)
}
})
}
// TestEbookScanner_MultipleLibraryTypes tests Phase 2: multiple libraries with different types
func TestEbookScanner_MultipleLibraryTypes(t *testing.T) {
scanner := &EbookScanner{
folders: []string{
"/library/ebooks",
"/library/comics",
"/library/manga",
},
libraryTypes: map[string][]string{
"/library/ebooks": {".epub", ".mobi", ".azw3"},
"/library/comics": {".cbz", ".cbr"},
"/library/manga": {".cbz", ".cb7"}, // Manga uses CBZ and CB7
},
}
tests := []struct {
name string
filePath string
expected bool
reason string
}{
{
name: "EPUB in ebook library",
filePath: "/library/ebooks/novel.epub",
expected: true,
reason: "EPUB allowed in ebook library",
},
{
name: "CBZ in comic library",
filePath: "/library/comics/superman.cbz",
expected: true,
reason: "CBZ allowed in comic library",
},
{
name: "CBZ in manga library",
filePath: "/library/manga/naruto.cbz",
expected: true,
reason: "CBZ allowed in manga library",
},
{
name: "CB7 in manga library",
filePath: "/library/manga/onepiece.cb7",
expected: true,
reason: "CB7 allowed in manga library",
},
{
name: "CB7 in comic library (rejected)",
filePath: "/library/comics/batman.cb7",
expected: false,
reason: "CB7 not allowed in comic library",
},
{
name: "EPUB in comic library (rejected)",
filePath: "/library/comics/novel.epub",
expected: false,
reason: "EPUB not allowed in comic library",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result := scanner.isScannableFile(tt.filePath)
assert.Equal(t, tt.expected, result, tt.reason)
})
}
}