test: add comprehensive test coverage for API endpoints and services

internal/middleware/middleware_test.go

@@ -0,0 +1,319 @@
+package middleware
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPasswordValidator_ValidatePassword(t *testing.T) {
+	tests := []struct {
+		name      string
+		password  string
+		wantValid bool
+	}{
+		{
+			name:      "valid password with all requirements",
+			password:  "Test@Pass123!",
+			wantValid: true,
+		},
+		{
+			name:      "valid password with special chars",
+			password:  "MyP@ssw0rd#2024",
+			wantValid: true,
+		},
+		{
+			name:      "too short",
+			password:  "Test1!",
+			wantValid: false,
+		},
+		{
+			name:      "no uppercase",
+			password:  "test@pass123!",
+			wantValid: false,
+		},
+		{
+			name:      "no lowercase",
+			password:  "TEST@PASS123!",
+			wantValid: false,
+		},
+		{
+			name:      "no number",
+			password:  "Test@Password!",
+			wantValid: false,
+		},
+		{
+			name:      "no special character",
+			password:  "TestPassword123",
+			wantValid: false,
+		},
+		{
+			name:      "empty string",
+			password:  "",
+			wantValid: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidatePassword(tt.password)
+			if tt.wantValid {
+				assert.NoError(t, err)
+			} else {
+				assert.Error(t, err)
+			}
+		})
+	}
+}
+
+func TestPasswordValidator_ErrorMessages(t *testing.T) {
+	tests := []struct {
+		name            string
+		password        string
+		expectedInError string
+	}{
+		{
+			name:            "too short error",
+			password:        "Short1!",
+			expectedInError: "at least 8 characters",
+		},
+		{
+			name:            "no uppercase error",
+			password:        "alllower123!",
+			expectedInError: "uppercase letter",
+		},
+		{
+			name:            "no lowercase error",
+			password:        "ALLUPPER123!",
+			expectedInError: "lowercase letter",
+		},
+		{
+			name:            "no number error",
+			password:        "NoNumbers!",
+			expectedInError: "number",
+		},
+		{
+			name:            "no special char error",
+			password:        "NoSpecialChars123",
+			expectedInError: "special character",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidatePassword(tt.password)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), tt.expectedInError)
+		})
+	}
+}
+
+func TestGetPasswordRequirements(t *testing.T) {
+	requirements := GetPasswordRequirements()
+
+	assert.NotEmpty(t, requirements)
+	assert.Greater(t, len(requirements), 3)
+
+	// Check for common requirements
+	requirementText := ""
+	for _, req := range requirements {
+		requirementText += req + " "
+	}
+
+	assert.Contains(t, requirementText, "8")
+	assert.Contains(t, requirementText, "uppercase")
+	assert.Contains(t, requirementText, "lowercase")
+	assert.Contains(t, requirementText, "number")
+	assert.Contains(t, requirementText, "special")
+}
+
+func TestLoginAttemptTracker_RecordFailedAttempt(t *testing.T) {
+	tracker := NewLoginAttemptTracker(3, 5*time.Minute, 5*time.Minute)
+
+	username := "testuser"
+
+	// First failed attempt
+	locked, remainingTime := tracker.RecordFailedAttempt(username)
+	assert.False(t, locked)
+	assert.Equal(t, time.Duration(0), remainingTime)
+
+	// Second failed attempt
+	locked, remainingTime = tracker.RecordFailedAttempt(username)
+	assert.False(t, locked)
+	assert.Equal(t, time.Duration(0), remainingTime)
+
+	// Third failed attempt - should lock
+	locked, remainingTime = tracker.RecordFailedAttempt(username)
+	assert.True(t, locked)
+	assert.Greater(t, remainingTime, time.Duration(0))
+
+	// Verify user is locked
+	locked, _ = tracker.IsLocked(username)
+	assert.True(t, locked)
+
+	// Clear attempts
+	tracker.ClearAttempts(username)
+
+	// Should no longer be locked
+	locked, _ = tracker.IsLocked(username)
+	assert.False(t, locked)
+}
+
+func TestLoginAttemptTracker_IsLocked(t *testing.T) {
+	tracker := NewLoginAttemptTracker(3, 5*time.Minute, 5*time.Minute)
+
+	username := "lockeduser"
+
+	// Record failed attempts up to max
+	for i := 0; i < 3; i++ {
+		tracker.RecordFailedAttempt(username)
+	}
+
+	// Verify user is locked
+	locked, remainingTime := tracker.IsLocked(username)
+	assert.True(t, locked)
+	assert.Greater(t, remainingTime, time.Duration(0))
+
+	// Clear attempts
+	tracker.ClearAttempts(username)
+
+	// Should no longer be locked
+	locked, remainingTime = tracker.IsLocked(username)
+	assert.False(t, locked)
+	assert.Equal(t, time.Duration(0), remainingTime)
+}
+
+func TestLoginAttemptTracker_ConcurrentAccess(t *testing.T) {
+	tracker := NewLoginAttemptTracker(5, 5*time.Minute, 5*time.Minute)
+
+	done := make(chan bool, 10)
+
+	// Concurrent access from multiple goroutines
+	for i := 0; i < 10; i++ {
+		go func(index int) {
+			username := "user" + string(rune('0'+index))
+			tracker.RecordFailedAttempt(username)
+			tracker.IsLocked(username)
+			tracker.ClearAttempts(username)
+			done <- true
+		}(i)
+	}
+
+	// Wait for all goroutines
+	for i := 0; i < 10; i++ {
+		<-done
+	}
+
+	// Should complete without deadlock or race
+}
+
+func TestDeviceRateLimiter_CheckRateLimit(t *testing.T) {
+	limiter := NewDeviceRateLimiter()
+
+	config := DeviceRateLimitConfig{
+		SyncRequestsPerMinute: 5,
+	}
+
+	deviceID := "test-device-123"
+	requestType := "sync"
+
+	// First 5 requests should succeed
+	for i := 0; i < 5; i++ {
+		allowed := limiter.CheckRateLimit(deviceID, requestType, config)
+		assert.True(t, allowed, "Request %d should be allowed", i+1)
+	}
+
+	// 6th request should be rate limited
+	allowed := limiter.CheckRateLimit(deviceID, requestType, config)
+	assert.False(t, allowed, "Request 6 should be rate limited")
+
+	// Get remaining requests
+	remaining := limiter.GetRemainingRequests(deviceID, requestType, config)
+	assert.Equal(t, 0, remaining)
+
+	// Reset and verify
+	limiter.Reset(deviceID)
+
+	// Should be allowed again
+	allowed = limiter.CheckRateLimit(deviceID, requestType, config)
+	assert.True(t, allowed, "Request after reset should be allowed")
+}
+
+func TestDeviceRateLimiter_DifferentDevices(t *testing.T) {
+	limiter := NewDeviceRateLimiter()
+
+	config := DeviceRateLimitConfig{
+		SyncRequestsPerMinute: 2,
+	}
+
+	// Exhaust limit for device1
+	for i := 0; i < 2; i++ {
+		limiter.CheckRateLimit("device1", "sync", config)
+	}
+
+	// Device1 should be rate limited
+	allowed := limiter.CheckRateLimit("device1", "sync", config)
+	assert.False(t, allowed)
+
+	// Device2 should still work
+	allowed = limiter.CheckRateLimit("device2", "sync", config)
+	assert.True(t, allowed)
+}
+
+func TestDeviceRateLimiter_GetRemainingRequests(t *testing.T) {
+	limiter := NewDeviceRateLimiter()
+
+	config := DeviceRateLimitConfig{
+		SyncRequestsPerMinute: 10,
+	}
+
+	deviceID := "test-device-456"
+
+	// Initially should have all requests remaining
+	remaining := limiter.GetRemainingRequests(deviceID, "scan", config)
+	assert.Equal(t, 10, remaining)
+
+	// Use 3 requests
+	for i := 0; i < 3; i++ {
+		limiter.CheckRateLimit(deviceID, "scan", config)
+	}
+
+	// Should have 7 remaining
+	remaining = limiter.GetRemainingRequests(deviceID, "scan", config)
+	assert.Equal(t, 7, remaining)
+}
+
+func TestNewRateLimiter(t *testing.T) {
+	config := DefaultRateLimiterConfig()
+	limiter := NewRateLimiter(config)
+
+	assert.NotNil(t, limiter)
+	assert.NotNil(t, limiter.mu)
+}
+
+func TestHTTPError_Error(t *testing.T) {
+	err := NewHTTPError(404, "Not Found", nil)
+
+	assert.NotNil(t, err)
+	assert.Equal(t, 404, err.Code)
+	assert.Equal(t, "Not Found", err.Message)
+}
+
+func TestHTTPError_ErrorWithInternal(t *testing.T) {
+	internalErr := assert.AnError
+	err := NewHTTPError(500, "Internal Error", internalErr)
+
+	assert.Equal(t, "Internal Error", err.Error())
+	assert.Equal(t, 500, err.Code)
+	assert.Equal(t, "Internal Error", err.Message)
+	assert.Equal(t, internalErr, err.Err)
+}
+
+func TestNewHTTPError(t *testing.T) {
+	err := NewHTTPError(404, "Not Found", nil)
+
+	assert.NotNil(t, err)
+	assert.Equal(t, 404, err.Code)
+	assert.Equal(t, "Not Found", err.Message)
+}

internal/services/scheduler_test.go

@@ -0,0 +1,143 @@
+package services
+
+import (
+	"testing"
+
+	"github.com/google/uuid"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestScheduler_NewScheduler(t *testing.T) {
+	worker := NewWorker(1)
+
+	// Use nil database interface for basic testing
+	scheduler := NewScheduler(worker, nil)
+
+	assert.NotNil(t, scheduler)
+	assert.NotNil(t, scheduler.worker)
+	assert.NotNil(t, scheduler.timers)
+	assert.NotNil(t, scheduler.scanSettings)
+	assert.NotNil(t, scheduler.ctx)
+	assert.NotNil(t, scheduler.cancel)
+}
+
+func TestScheduler_StartStop(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+
+	// Start should not panic
+	scheduler.Start()
+	assert.NotNil(t, scheduler.ctx)
+
+	// Stop should not panic
+	scheduler.Stop()
+}
+
+func TestScheduler_UpdateScanSettings(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+
+	userID := "test-user-123"
+
+	// Update scan settings
+	scheduler.UpdateScanSettings(userID, true, 30)
+
+	scheduler.mu.Lock()
+	settings, exists := scheduler.scanSettings[userID]
+	scheduler.mu.Unlock()
+
+	assert.True(t, exists)
+	assert.Equal(t, userID, settings.UserID)
+	assert.True(t, settings.Enabled)
+	assert.Equal(t, 30, settings.Frequency)
+}
+
+func TestScheduler_UpdateScanSettings_Disabled(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+
+	userID := "test-user-456"
+
+	// Update scan settings to disabled
+	scheduler.UpdateScanSettings(userID, false, 60)
+
+	scheduler.mu.Lock()
+	settings, exists := scheduler.scanSettings[userID]
+	scheduler.mu.Unlock()
+
+	assert.True(t, exists)
+	assert.False(t, settings.Enabled)
+	assert.Equal(t, 60, settings.Frequency)
+}
+
+func TestScheduler_UpdateScanSettings_Overwrite(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+
+	userID := "test-user-789"
+
+	// First update
+	scheduler.UpdateScanSettings(userID, true, 30)
+
+	// Overwrite with different settings
+	scheduler.UpdateScanSettings(userID, false, 45)
+
+	scheduler.mu.Lock()
+	settings, exists := scheduler.scanSettings[userID]
+	scheduler.mu.Unlock()
+
+	assert.True(t, exists)
+	assert.False(t, settings.Enabled)
+	assert.Equal(t, 45, settings.Frequency)
+}
+
+func TestScheduler_StopWithActiveTimers(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+
+	// Add some fake timers
+	scheduler.mu.Lock()
+	scheduler.timers["timer1"] = nil
+	scheduler.timers["timer2"] = nil
+	scheduler.timers["timer3"] = nil
+	scheduler.mu.Unlock()
+
+	// Stop should clear timers
+	scheduler.Stop()
+
+	scheduler.mu.Lock()
+	timerCount := len(scheduler.timers)
+	scheduler.mu.Unlock()
+
+	assert.Equal(t, 0, timerCount)
+}
+
+func TestScheduler_ConcurrentAccess(t *testing.T) {
+	worker := NewWorker(1)
+	scheduler := NewScheduler(worker, nil)
+	scheduler.Start()
+	defer scheduler.Stop()
+
+	// Concurrent updates should not cause race conditions
+	done := make(chan bool, 10)
+
+	for i := 0; i < 10; i++ {
+		go func(index int) {
+			userID := uuid.New().String()
+			scheduler.UpdateScanSettings(userID, true, 30)
+			done <- true
+		}(i)
+	}
+
+	// Wait for all goroutines
+	for i := 0; i < 10; i++ {
+		<-done
+	}
+
+	// Verify all settings were stored
+	scheduler.mu.Lock()
+	settingCount := len(scheduler.scanSettings)
+	scheduler.mu.Unlock()
+
+	assert.Equal(t, 10, settingCount)
+}

internal/services/worker_test.go

@@ -0,0 +1,347 @@
+package services
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestWorker_NewWorker(t *testing.T) {
+	worker := NewWorker(2)
+
+	assert.NotNil(t, worker)
+	assert.NotNil(t, worker.jobQueue)
+	assert.NotNil(t, worker.results)
+	assert.NotNil(t, worker.ctx)
+	assert.NotNil(t, worker.cancel)
+}
+
+func TestWorker_EnqueueJob_Success(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	job := &Job{
+		ID:     "test-job-1",
+		Type:   JobTypeScan,
+		Params: map[string]interface{}{},
+		Status: JobStatusPending,
+	}
+
+	err := worker.EnqueueJob(job)
+	assert.NoError(t, err)
+}
+
+func TestWorker_EnqueueJob_QueueFull(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	// Fill the queue (capacity is 100)
+	for i := 0; i < 100; i++ {
+		job := &Job{
+			ID:     fmt.Sprintf("job-%d", i),
+			Type:   JobTypeScan,
+			Params: map[string]interface{}{},
+			Status: JobStatusPending,
+		}
+		worker.jobQueue <- job
+	}
+
+	// Try to enqueue one more job
+	job := &Job{
+		ID:     "overflow-job",
+		Type:   JobTypeScan,
+		Params: map[string]interface{}{},
+		Status: JobStatusPending,
+	}
+
+	err := worker.EnqueueJob(job)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "job queue is full")
+}
+
+func TestWorker_EnqueueJob_WorkerShutdown(t *testing.T) {
+	worker := NewWorker(1)
+	worker.Shutdown()
+
+	job := &Job{
+		ID:     "test-job",
+		Type:   JobTypeScan,
+		Params: map[string]interface{}{},
+		Status: JobStatusPending,
+	}
+
+	err := worker.EnqueueJob(job)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "worker is shutting down")
+}
+
+func TestWorker_GetJobStatus_NotFound(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	result, exists := worker.GetJobStatus("non-existent-job")
+	assert.False(t, exists)
+	assert.Nil(t, result)
+}
+
+func TestWorker_GetJobStatus_Found(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	_ = &Job{
+		ID:     "test-job-2",
+		Type:   JobTypeScan,
+		Params: map[string]interface{}{},
+		Status: JobStatusPending,
+	}
+
+	worker.mu.Lock()
+	worker.results["test-job-2"] = &JobResult{
+		JobID:  "test-job-2",
+		Status: JobStatusPending,
+	}
+	worker.mu.Unlock()
+
+	result, exists := worker.GetJobStatus("test-job-2")
+	assert.True(t, exists)
+	assert.NotNil(t, result)
+	assert.Equal(t, "test-job-2", result.JobID)
+	assert.Equal(t, JobStatusPending, result.Status)
+}
+
+func TestWorker_CancelJob_Success(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	jobID := "test-job-3"
+
+	worker.mu.Lock()
+	worker.results[jobID] = &JobResult{
+		JobID:  jobID,
+		Status: JobStatusPending,
+	}
+	worker.mu.Unlock()
+
+	err := worker.CancelJob(jobID)
+	assert.NoError(t, err)
+
+	// Verify status was updated
+	result, exists := worker.GetJobStatus(jobID)
+	assert.True(t, exists)
+	assert.Equal(t, JobStatusCancelled, result.Status)
+}
+
+func TestWorker_CancelJob_AlreadyRunning(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	jobID := "test-job-4"
+
+	worker.mu.Lock()
+	worker.results[jobID] = &JobResult{
+		JobID:  jobID,
+		Status: JobStatusRunning,
+	}
+	worker.mu.Unlock()
+
+	err := worker.CancelJob(jobID)
+	assert.NoError(t, err)
+
+	// Verify status was updated
+	result, exists := worker.GetJobStatus(jobID)
+	assert.True(t, exists)
+	assert.Equal(t, JobStatusCancelled, result.Status)
+}
+
+func TestWorker_CancelJob_AlreadyCompleted(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	jobID := "test-job-5"
+
+	worker.mu.Lock()
+	worker.results[jobID] = &JobResult{
+		JobID:  jobID,
+		Status: JobStatusCompleted,
+	}
+	worker.mu.Unlock()
+
+	err := worker.CancelJob(jobID)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "job cannot be cancelled")
+}
+
+func TestWorker_CancelJob_NotFound(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	err := worker.CancelJob("non-existent-job")
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "job not found")
+}
+
+func TestWorker_ProcessJob_ScanJob_MissingParams(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	tests := []struct {
+		name    string
+		params  map[string]interface{}
+		wantErr string
+	}{
+		{
+			name:    "missing library_id",
+			params:  map[string]interface{}{},
+			wantErr: "library_id required",
+		},
+		{
+			name: "missing folders",
+			params: map[string]interface{}{
+				"library_id": "test-lib",
+			},
+			wantErr: "folders required",
+		},
+		{
+			name: "missing admin_id",
+			params: map[string]interface{}{
+				"library_id": "test-lib",
+				"folders":    []string{"/test"},
+			},
+			wantErr: "admin_id required",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			testJob := &Job{
+				ID:     "test-job",
+				Type:   JobTypeScan,
+				Params: tt.params,
+				Status: JobStatusPending,
+			}
+
+			result, err := worker.processScanJob(testJob)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), tt.wantErr)
+			assert.Nil(t, result)
+		})
+	}
+}
+
+func TestWorker_ProcessJob_UnknownJobType(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	job := &Job{
+		ID:     "test-job",
+		Type:   JobType("unknown"),
+		Params: map[string]interface{}{},
+		Status: JobStatusPending,
+	}
+
+	result, err := worker.processScanJob(job)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "unknown job type")
+	assert.Nil(t, result)
+}
+
+func TestWorker_JobLifecycle(t *testing.T) {
+	worker := NewWorker(1)
+	defer worker.Shutdown()
+
+	job := &Job{
+		ID:   "lifecycle-test",
+		Type: JobTypeScan,
+		Params: map[string]interface{}{
+			"library_id": "test-lib",
+			"folders":    []string{"/test"},
+			"admin_id":   "test-admin",
+			"db":         nil, // Will fail but tests the flow
+		},
+		Status: JobStatusPending,
+	}
+
+	// Enqueue the job
+	err := worker.EnqueueJob(job)
+	require.NoError(t, err)
+
+	// Give worker time to process
+	time.Sleep(100 * time.Millisecond)
+
+	// Check job status
+	result, exists := worker.GetJobStatus("lifecycle-test")
+	assert.True(t, exists)
+	assert.NotNil(t, result)
+
+	// Status should be failed (because we passed nil db)
+	assert.Equal(t, JobStatusFailed, result.Status)
+}
+
+func TestWorker_Shutdown(t *testing.T) {
+	worker := NewWorker(2)
+
+	// Enqueue some jobs
+	for i := 0; i < 5; i++ {
+		testJob := &Job{
+			ID:     fmt.Sprintf("shutdown-job-%d", i),
+			Type:   JobTypeScan,
+			Params: map[string]interface{}{},
+			Status: JobStatusPending,
+		}
+		worker.EnqueueJob(testJob)
+	}
+
+	// Shutdown should not block
+	shutdownDone := make(chan bool)
+	go func() {
+		worker.Shutdown()
+		shutdownDone <- true
+	}()
+
+	select {
+	case <-shutdownDone:
+		// Shutdown completed
+	case <-time.After(5 * time.Second):
+		t.Fatal("Shutdown took too long")
+	}
+}
+
+func TestWorker_ConcurrentJobProcessing(t *testing.T) {
+	worker := NewWorker(3) // 3 workers
+	defer worker.Shutdown()
+
+	jobCount := 10
+
+	// Enqueue multiple jobs
+	for i := 0; i < jobCount; i++ {
+		job := &Job{
+			ID:   fmt.Sprintf("concurrent-job-%d", i),
+			Type: JobTypeScan,
+			Params: map[string]interface{}{
+				"library_id": fmt.Sprintf("lib-%d", i),
+				"folders":    []string{"/test"},
+				"admin_id":   "admin",
+				"db":         nil,
+			},
+			Status: JobStatusPending,
+		}
+
+		go func(j *Job) {
+			err := worker.EnqueueJob(j)
+			assert.NoError(t, err)
+		}(job)
+	}
+
+	// Wait a bit for processing
+	time.Sleep(200 * time.Millisecond)
+
+	// Check that all jobs were processed
+	worker.mu.RLock()
+	resultCount := len(worker.results)
+	worker.mu.RUnlock()
+
+	assert.Equal(t, jobCount, resultCount)
+}

internal/utils/isbn_test.go

@@ -0,0 +1,248 @@
+package utils
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNormalizeISBN_ValidISBNs(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "ISBN-10 with hyphens",
+			input:    "0-306-40615-2",
+			expected: "0306406152",
+		},
+		{
+			name:     "ISBN-13 with hyphens",
+			input:    "978-0-306-40615-7",
+			expected: "9780306406157",
+		},
+		{
+			name:     "ISBN-10 with spaces",
+			input:    "0 306 40615 2",
+			expected: "0306406152",
+		},
+		{
+			name:     "ISBN-13 with spaces",
+			input:    "978 0 306 40615 7",
+			expected: "9780306406157",
+		},
+		{
+			name:     "ISBN-10 with hyphens and spaces",
+			input:    "0-306-40615 2",
+			expected: "0306406152",
+		},
+		{
+			name:     "ISBN-13 with hyphens and spaces",
+			input:    "978-0-306 40615-7",
+			expected: "9780306406157",
+		},
+		{
+			name:     "ISBN-10 clean",
+			input:    "0306406152",
+			expected: "0306406152",
+		},
+		{
+			name:     "ISBN-13 clean",
+			input:    "9780306406157",
+			expected: "9780306406157",
+		},
+		{
+			name:     "ISBN-10 with X",
+			input:    "0-8044-2957-X",
+			expected: "080442957X",
+		},
+		{
+			name:     "ISBN-10 with X and hyphens",
+			input:    "0-8044-2957-X",
+			expected: "080442957X",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeISBN(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestNormalizeISBN_EdgeCases(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "empty string",
+			input:    "",
+			expected: "",
+		},
+		{
+			name:     "only hyphens",
+			input:    "---",
+			expected: "",
+		},
+		{
+			name:     "only spaces",
+			input:    "   ",
+			expected: "",
+		},
+		{
+			name:     "mixed hyphens and spaces",
+			input:    "- - -",
+			expected: "",
+		},
+		{
+			name:     "multiple consecutive hyphens",
+			input:    "0--306--40615--2",
+			expected: "0306406152",
+		},
+		{
+			name:     "multiple consecutive spaces",
+			input:    "0  306  40615  2",
+			expected: "0306406152",
+		},
+		{
+			name:     "tabs and newlines (treated as spaces)",
+			input:    "0\t306\n40615\r2",
+			expected: "0306406152",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeISBN(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestNormalizeISBN_SpecialCharacters(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "with dots (not removed, only hyphens/spaces)",
+			input:    "978.0.306.40615.7",
+			expected: "978.0.306.40615.7",
+		},
+		{
+			name:     "mixed dots and hyphens",
+			input:    "978-0.306-40615.7",
+			expected: "978.0.306-40615.7",
+		},
+		{
+			name:     "with underscores (preserved)",
+			input:    "978_0_306_40615_7",
+			expected: "978_0_306_40615_7",
+		},
+		{
+			name:     "with slashes (preserved)",
+			input:    "978/0/306/40615/7",
+			expected: "978/0/306/40615/7",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeISBN(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestNormalizeISBN_RealWorldExamples(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "real book ISBN-10",
+			input:    "0-596-00965-X",
+			expected: "059600965X",
+		},
+		{
+			name:     "real book ISBN-13",
+			input:    "978-0-596-00965-2",
+			expected: "9780596009652",
+		},
+		{
+			name:     "another real ISBN-10",
+			input:    "1-4028-9462-7",
+			expected: "1402894627",
+		},
+		{
+			name:     "another real ISBN-13",
+			input:    "978-1-4028-9462-6",
+			expected: "9781402894626",
+		},
+		{
+			name:     "popular programming book",
+			input:    "978-0-13-595705-9",
+			expected: "9780135957059",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeISBN(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestNormalizeISBN_DoesNotModifyValidISBNs(t *testing.T) {
+	validISBNs := []string{
+		"0306406152",
+		"9780306406157",
+		"080442957X",
+		"1234567890123",
+	}
+
+	for _, isbn := range validISBNs {
+		t.Run(isbn, func(t *testing.T) {
+			result := NormalizeISBN(isbn)
+			assert.Equal(t, isbn, result, "Valid ISBN should not be modified")
+		})
+	}
+}
+
+func TestNormalizeISBN_PreservesX(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "X at end",
+			input:    "0-8044-2957-X",
+			expected: "080442957X",
+		},
+		{
+			name:     "lowercase x",
+			input:    "0-8044-2957-x",
+			expected: "080442957x",
+		},
+		{
+			name:     "X in middle (invalid but preserved)",
+			input:    "0-8044-X-2957",
+			expected: "08044X2957",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := NormalizeISBN(tt.input)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}