From d802236874e54d87f63630bb00639d48367df34d Mon Sep 17 00:00:00 2001 From: John O'Keefe Date: Thu, 26 Feb 2026 16:39:42 -0500 Subject: [PATCH] scanner: fix library isolation, file mtime, force rescan, and deletion handling Fix 1 - File modification time for created_at: - Get file.ModTime() in processMediaFile and pass to CreateMediaItem - Modified SQL INSERT to include created_at column Fix 2 - Force rescan UPDATE instead of DELETE+INSERT: - Changed force rescan logic to call updateMediaItem instead of delete + create - Preserves created_at timestamp on force rescan Fix 3 - GetMediaItemByFilePath filters by library_id: - Added library_id to WHERE clause in SQL query - Created GetMediaItemByFilePathAnyLibrary for cross-library lookups (KOReader) - Added SetLibraryID method to MediaScanner - Updated handler to call SetLibraryID for watch mode Fix 4 - File deletion handling with persistent logging: - Added fsnotify.Remove handler in WatchChanges - Added orphan cleanup in ScanFolders after scan completes - Created scanner_logger.go with daily log rotation (7 days) - Logs to /app/logs/scanner-deletes-YYYY-MM-DD.log and scanner-errors-YYYY-MM-DD.log - Individual deletes with enhanced safety logging Note: Integration tests can now safely scan /app/uploads because GetMediaItemByFilePath now filters by library_id, preventing cross-library interference. --- IMPLEMENTATION_PLAN.md | 863 +++++++++++++++++++++ SUMMARY.md | 117 +++ bruno/collections/Get Book Collections.yml | 15 +- bruno/environments/Bookhoard.yml | 6 +- bruno/library/Get Scan Settings.yml | 17 +- bruno/library/Update Scan Settings.yml | 24 +- bruno/scanner/Scan Media Items.yml | 23 + internal/database/querier.go | 3 +- internal/database/queries.sql.go | 127 ++- internal/database/queries/queries.sql | 9 +- internal/handlers/koreader.go | 4 +- internal/handlers/scanner.go | 3 +- internal/services/media_scanner.go | 197 ++++- internal/services/scanner_logger.go | 107 +++ 14 files changed, 1432 insertions(+), 83 deletions(-) create mode 100644 IMPLEMENTATION_PLAN.md create mode 100644 SUMMARY.md create mode 100644 internal/services/scanner_logger.go diff --git a/IMPLEMENTATION_PLAN.md b/IMPLEMENTATION_PLAN.md new file mode 100644 index 0000000..e06864c --- /dev/null +++ b/IMPLEMENTATION_PLAN.md @@ -0,0 +1,863 @@ +# Scanner Implementation Plan - Detailed Steps + +This document provides precise, line-by-line steps to implement the scanner fixes. Follow in order. + +--- + +## Fix 1: Use File Modification Time for `created_at` on INSERT + +**Goal:** When inserting a new media item, use the file's actual modification time instead of database default (which uses scan time). + +### Step 1.1: Modify `processMediaFile` to get file mtime + +**File:** `internal/services/media_scanner.go` + +**Current code (around line 348-360):** +```go +func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, error) { + // Get file info + info, err := os.Stat(path) + if err != nil { + fmt.Printf("Failed to get file info for %s: %v\n", path, err) + return false, fmt.Errorf("failed to get file info: %v", err) + } + + // Check if media item already exists in database + existingItem, err := s.getMediaItemByFilePath(ctx, path) +``` + +**Add AFTER line 357 (after getting file info, before existingItem check):** +```go + // Get file modification time for created_at + fileModTime := info.ModTime() +``` + +### Step 1.2: Modify SQL INSERT to include created_at column + +**File:** `internal/database/queries/queries.sql` + +**Current code (line 131-133):** +```sql +-- name: CreateMediaItem :one +INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27) +RETURNING *; +``` + +**Change TO:** +```sql +-- name: CreateMediaItem :one +INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28) +RETURNING *; +``` + +### Step 1.3: Regenerate Go code from SQL OR manually update queries.sql.go + +**Option A - Run SQL generation (recommended):** +```bash +cd internal/database && go generate ./... +``` + +**Option B - Manual update (if Option A fails):** + +**File:** `internal/database/queries.sql.go` + +**Find `CreateMediaItemParams` struct (around line 557):** + +**Add to struct (after AddedByAdminID):** +```go +CreatedAt pgtype.Timestamp `db:"created_at" json:"created_at"` +``` + +**Find `CreateMediaItem` function (around line 588):** + +**Add to the query parameters (after AddedByAdminID in the VALUES):** +```go +arg.CreatedAt, +``` + +### Step 1.4: Pass `fileModTime` to CreateMediaItem in scanner + +**File:** `internal/services/media_scanner.go` + +**Find the CreateMediaItem call** - around line 512. + +**Current code (line 512-532):** +```go +createdItem, err := s.db.CreateMediaItem(ctx, database.CreateMediaItemParams{ + LibraryID: libraryID, + Title: metadata.Title, + // ... other fields +}) +``` + +**Add to the params (after AddedByAdminID):** +```go + CreatedAt: pgtype.Timestamp{Time: fileModTime, Valid: true}, +``` + +**Note:** You'll need to import `"github.com/jackc/pgx/v5/pgtype"` if not already present. +```go + CreatedAt: pgtype.Timestamp{Time: fileModTime, Valid: true}, +``` + +**Note:** Verify `CreatedAt` field exists in `CreateMediaItemParams`. If not, the schema uses `DEFAULT NOW()` and the column is `created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW()`. The INSERT may need to explicitly include the column name and value. + +--- + +## Fix 2: Force Rescan Uses UPDATE Instead of DELETE+INSERT + +**Goal:** Preserve `created_at` when force rescan runs by using UPDATE instead of delete + insert. + +### Step 2.1: Find the force rescan delete logic + +**File:** `internal/services/media_scanner.go` + +**Current code (around lines 365-372):** +```go +if s.forceRescan { + fmt.Printf("Force rescan enabled, re-processing existing media item: %s\n", path) + // Force update: delete existing and re-create + if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { + fmt.Printf("Warning: failed to delete existing media item: %v\n", err) + } + // Continue to create new entry below +} +``` + +### Step 2.2: Replace DELETE+INSERT with UPDATE + +**Replace lines 365-372 with:** +```go +if s.forceRescan { + fmt.Printf("Force rescan enabled, updating existing media item: %s\n", path) + // Use UPDATE instead of DELETE+INSERT to preserve created_at + // Re-extract metadata and update + metadata, err := s.extractMetadata(path) + if err != nil { + fmt.Printf("Warning: failed to extract metadata for force rescan %s: %v\n", path, err) + metadata = &MediaMetadata{} + } + if err := s.updateMediaItem(ctx, existingItem.ID, path, info); err != nil { + fmt.Printf("Warning: failed to update existing media item: %v\n", err) + } + return false, nil +} +``` + +**Note:** The existing `updateMediaItem` function (line 1390) handles updating most fields. Verify it doesn't overwrite `created_at`. + +--- + +## Fix 3: `GetMediaItemByFilePath` Filters by `library_id` + +**Goal:** Prevent cross-library interference - scanning one library shouldn't affect another library's entries. + +**Important Context:** The `libraryID` is already available in `StartWatchModeForLibrary` at `scanner.go:385`: +```go +h.StartWatchModeForLibrary(ctx, library.ID, library.CreatedByAdminID) +``` + +The function receives `libraryID` but doesn't store it in the scanner. We need to store it. + +### Step 3.1: Add SetLibraryID method to MediaScanner + +**File:** `internal/services/media_scanner.go` + +**Find the SetAdminID function (around line 104):** + +**Add AFTER SetAdminID:** +```go +func (s *MediaScanner) SetLibraryID(libraryID pgtype.UUID) { + s.defaultLibraryID = libraryID +} +``` + +### Step 3.2: Store libraryID in StartWatchModeForLibrary + +**File:** `internal/handlers/scanner.go` + +**Current code (around line 268):** +```go +scanner.SetAdminID(adminID) +scanner.WatchChanges(h.watchModeCtx) +``` + +**Add AFTER line 268:** +```go +scanner.SetLibraryID(libraryID) +``` + +### Step 3.3: Modify the SQL query + +**File:** `internal/database/queries/queries.sql` + +**Current code (line 302-303):** +```sql +-- name: GetMediaItemByFilePath :one +SELECT * FROM media_items WHERE file_path = $1; +``` + +**Change TO:** +```sql +-- name: GetMediaItemByFilePath :one +SELECT * FROM media_items WHERE file_path = $1 AND library_id = $2; +``` + +### Step 3.4: Update the Go code (generated or manual) + +**File:** `internal/database/queries.sql.go` + +Find `GetMediaItemByFilePath` function and update: +1. Add `LibraryID pgtype.UUID` parameter to the function and params struct +2. Add the parameter to the query call + +**OR run SQL generation:** +```bash +cd internal/database && go generate ./... +``` + +### Step 3.5: Update getMediaItemByFilePath in scanner + +**File:** `internal/services/media_scanner.go` + +**Current code (lines 1395-1397):** +```go +func (s *MediaScanner) getMediaItemByFilePath(ctx context.Context, filePath string) (database.MediaItems, error) { + return s.db.GetMediaItemByFilePath(ctx, filePath) +} +``` + +**Change TO:** +```go +func (s *MediaScanner) getMediaItemByFilePath(ctx context.Context, filePath string) (database.MediaItems, error) { + return s.db.GetMediaItemByFilePath(ctx, filePath, s.defaultLibraryID) +} +``` + +### Step 3.6: Update all callers + +**File:** `internal/services/media_scanner.go` + +Update all places that call `getMediaItemByFilePath` to pass the libraryID: +- Line 361: In `processMediaFile` - already has access to libraryID via folder lookup + +**Note:** The `processMediaFile` function already determines libraryID from the folder path (lines 487-501). Use that libraryID instead of `s.defaultLibraryID` for better accuracy. + +--- + +## Fix 4: Add Deletion Handling + +**Goal:** Delete media items when files are removed from the filesystem. + +**Safety Note:** Individual deletes are used (not bulk) to minimize risk of accidental mass deletion. Each deletion is logged before execution for traceability. + +### Step 4.0: Add File Logging Infrastructure + +**New File:** `internal/services/scanner_logger.go` + +Create a new file with the following structure: + +```go +package services + +import ( + "fmt" + "os" + "path/filepath" + "time" +) + +const ( + logDir = "/app/logs" + maxLogAgeDays = 7 +) + +type ScannerLogger struct { + deletesFile *os.File + errorsFile *os.File + currentDate string +} + +// NewScannerLogger creates a new scanner logger instance +func NewScannerLogger() *ScannerLogger { + return &ScannerLogger{} +} + +// ensureLogFiles creates/opens log files for today +func (l *ScannerLogger) ensureLogFiles() error { + today := time.Now().Format("2006-01-02") + + // Check if we need to rotate (new day) + if l.currentDate == today && l.deletesFile != nil { + return nil // Already have today's files open + } + + // Close existing files + if l.deletesFile != nil { + l.deletesFile.Close() + } + if l.errorsFile != nil { + l.errorsFile.Close() + } + + // Create log directory if it doesn't exist + if err := os.MkdirAll(logDir, 0755); err != nil { + return fmt.Errorf("failed to create log directory: %v", err) + } + + // Open new files for today + deletesPath := filepath.Join(logDir, fmt.Sprintf("scanner-deletes-%s.log", today)) + errorsPath := filepath.Join(logDir, fmt.Sprintf("scanner-errors-%s.log", today)) + + deletesFile, err := os.OpenFile(deletesPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return fmt.Errorf("failed to open deletes log file: %v", err) + } + + errorsFile, err := os.OpenFile(errorsPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + deletesFile.Close() + return fmt.Errorf("failed to open errors log file: %v", err) + } + + l.deletesFile = deletesFile + l.errorsFile = errorsFile + l.currentDate = today + + // Clean up old log files + l.cleanupOldLogs() + + return nil +} + +// cleanupOldLogs removes log files older than maxLogAgeDays +func (l *ScannerLogger) cleanupOldLogs() { + cutoff := time.Now().AddDate(0, 0, -maxLogAgeDays) + + filepath.Walk(logDir, func(path string, info os.FileInfo) error { + if err != nil { + return err + } + if !info.IsDir() && info.ModTime().Before(cutoff) { + os.Remove(path) + } + return nil + }) +} + +// LogDelete logs a deletion event +func (l *ScannerLogger) LogDelete(message string) { + if err := l.ensureLogFiles(); err != nil { + fmt.Printf("ERROR: Failed to ensure log files: %v\n", err) + return + } + timestamp := time.Now().Format("2006-01-02 15:04:05") + logLine := fmt.Sprintf("[%s] %s\n", timestamp, message) + l.deletesFile.WriteString(logLine) +} + +// LogError logs an error event +func (l *ScannerLogger) LogError(message string) { + if err := l.ensureLogFiles(); err != nil { + fmt.Printf("ERROR: Failed to ensure log files: %v\n", err) + return + } + timestamp := time.Now().Format("2006-01-02 15:04:05") + logLine := fmt.Sprintf("[%s] %s\n", timestamp, message) + l.errorsFile.WriteString(logLine) +} + +// Close closes the log files +func (l *ScannerLogger) Close() { + if l.deletesFile != nil { + l.deletesFile.Close() + } + if l.errorsFile != nil { + l.errorsFile.Close() + } +} +``` + +**Add to MediaScanner struct:** +- Add `logger *ScannerLogger` field to track logger instance + +**Update NewMediaScanner function:** +- Initialize logger: `logger: NewScannerLogger()` + +### Step 4.1: Ensure libraryID is available in scanner + +**Verify:** Steps 3.1-3.2 are complete - scanner has `SetLibraryID` and handler calls it. + +### Step 4.2: Add deletion logic in Watch mode + +**File:** `internal/services/media_scanner.go` + +**Find:** `WatchChanges` function (around line 1427). + +**Find the event handling section (around lines 1449-1455):** +```go +// Handle file modifications and creations +if (event.Has(fsnotify.Create) || event.Has(fsnotify.Write)) && s.isScannableFile(event.Name) { + fmt.Printf("New/modified media file detected: %s\n", event.Name) + if _, err := s.processMediaFile(ctx, event.Name); err != nil { + fmt.Printf("Error processing modified media file %s: %v\n", event.Name, err) + } +} +``` + +**Add AFTER that block (before line 1457):** +```go +// Handle file deletions +if event.Has(fsnotify.Remove) && s.isScannableFile(event.Name) { + // Use file logger for persistence + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] File removed from filesystem: %s", event.Name)) + + // Determine libraryID for this file + var libraryID pgtype.UUID + for _, folder := range s.folders { + if strings.HasPrefix(event.Name, folder) { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err == nil { + libraryID = lib.LibraryID + break + } + } + } + if !libraryID.Valid { + msg := fmt.Sprintf("[WATCH-DELETE] WARNING: could not determine library for deleted file: %s", event.Name) + s.logger.LogDelete(msg) + s.logger.LogError(msg) + return + } + + // Look up media item BEFORE deleting - log for safety + existingItem, err := s.db.GetMediaItemByFilePath(ctx, event.Name, libraryID) + if err == nil { + msg := fmt.Sprintf("[WATCH-DELETE] Found media item to delete: ID=%s, Title=%s, Path=%s", + existingItem.ID, existingItem.Title.String, existingItem.FilePath.String) + s.logger.LogDelete(msg) + + if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { + errMsg := fmt.Sprintf("[WATCH-DELETE] ERROR: failed to delete media item %s: %v", existingItem.ID, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] SUCCESS: deleted media item '%s' (was at %s)", + existingItem.Title.String, existingItem.FilePath.String)) + } + } else if err != pgx.ErrNoRows { + errMsg := fmt.Sprintf("[WATCH-DELETE] ERROR: failed to look up media item for %s: %v", event.Name, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] No media item found in database for deleted file: %s", event.Name)) + } +} +``` + +**Usage:** Replace `fmt.Printf` with `s.logger.LogDelete()` and `s.logger.LogError()` for persistent file logging. + +**Note:** This requires Step 3.3-3.5 to be complete (GetMediaItemByFilePath accepts libraryID). + +### Step 4.3: Add deletion logic in Rescan + +**File:** `internal/services/media_scanner.go` + +**Find:** End of `ScanFolders` function (after line 242). + +**Current code (around line 242-248):** +```go +fmt.Printf("Scan completed: %d total files scanned, %d media files found, %d new items, %d errors\n", + processedFiles, mediaFiles, s.newItems, s.errors) + +if s.job != nil && s.totalFiles > 0 { + s.job.UpdateProgress(1.0, processedFiles, s.newItems, s.errors) +} + +return nil +``` + +**Add BEFORE `return nil`:** +```go +// Clean up: Find media items in DB that no longer exist on filesystem +for _, folder := range s.folders { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err != nil { + continue + } + libraryID := lib.LibraryID + + dbItems, err := s.db.ListMediaItemsByLibrary(ctx, libraryID) + if err != nil { + fmt.Printf("Warning: failed to get library items for cleanup: %v\n", err) + continue + } + + // Build set of scanned file paths for this folder + scannedPaths := make(map[string]bool) + filepath.WalkDir(folder, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() && s.isScannableFile(path) { + scannedPaths[path] = true + } + return nil + }) + + // Delete items whose files no longer exist + // ENHANCED SAFETY LOGGING: Log BEFORE each delete - use file logger for persistence + for _, item := range dbItems { + filePath := item.FilePath.String + if filePath != "" && !scannedPaths[filePath] { + msg := fmt.Sprintf("[RESCAN-CLEANUP] Orphaned media item found: ID=%s, Title=%s, Path=%s", + item.ID, item.Title.String, filePath) + s.logger.LogDelete(msg) + + delMsg := fmt.Sprintf("[RESCAN-CLEANUP] Deleting orphaned item '%s' (file no longer exists at %s)", + item.Title.String, filePath) + s.logger.LogDelete(delMsg) + + if err := s.db.DeleteMediaItem(ctx, item.ID); err != nil { + errMsg := fmt.Sprintf("[RESCAN-CLEANUP] ERROR: failed to delete orphaned item %s: %v", item.Title.String, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[RESCAN-CLEANUP] SUCCESS: deleted orphaned item '%s'", item.Title.String)) + } + } + } +} +``` + +**Log Files Location:** `/app/logs/` +- `scanner-deletes-YYYY-MM-DD.log` - All deletion events (watch mode + rescan) +- `scanner-errors-YYYY-MM-DD.log` - All error events +- Rotation: Daily, keeps 7 days of history +- Volume mount in docker-compose: `./logs:/app/logs` + +--- + +## Verification Steps After Implementation + +1. **Compile the code:** + ```bash + go build ./... + ``` + +2. **Run tests:** + ```bash + go test ./... -v + ``` + +3. **Test manually:** + - Add a new book to the uploads folder + - Verify it appears in the database with correct `created_at` (should be file's actual modification time) + - Delete a book from the filesystem + - Verify it's removed from the database (via watch mode OR rescan) + - Run force rescan on a library + - Verify `created_at` is preserved (not reset) + +4. **Check the logs** for expected output: + - Log files are at `/app/logs/scanner-deletes-YYYY-MM-DD.log` and `/app/logs/scanner-errors-YYYY-MM-DD.log` + - Look for `[WATCH-DELETE]` entries for watch mode deletions + - Look for `[RESCAN-CLEANUP]` entries for rescan cleanup deletions + - Check both files to verify all operations were logged + +--- + +## Safety Checks (IMPORTANT) + +### Prevent deleting ALL books: +- The cleanup logic MUST check `scannedPaths[filePath]` - this ensures we only delete items whose paths were NOT found during the filesystem scan +- The key condition is: `if filePath != "" && !scannedPaths[filePath]` - meaning "if this file was NOT found in our scan, delete it" +- This is correct because: + 1. We scan the filesystem → get all current file paths + 2. We query DB → get all stored file paths + 3. We compare → only delete if DB path is NOT in filesystem paths + +### Before running against production: +- Test with a small subset of books first +- Verify the delete queries target specific library_id (not all libraries) +- Check logs show only expected deletions + +--- + +## Files to Modify + +| Fix | File | Changes | +|-----|------|---------| +| 1a | `internal/database/queries/queries.sql` | Add created_at to INSERT columns | +| 1b | `internal/database/queries.sql.go` | Add CreatedAt to CreateMediaItemParams struct + query | +| 1c | `internal/services/media_scanner.go` | Get file.ModTime() + pass to CreateMediaItem | +| 2 | `internal/services/media_scanner.go` | Change force rescan from DELETE+INSERT to UPDATE | +| 3a | `internal/services/media_scanner.go` | Add SetLibraryID() method | +| 3b | `internal/handlers/scanner.go` | Call scanner.SetLibraryID(libraryID) | +| 3c | `internal/database/queries/queries.sql` | Add library_id to WHERE clause | +| 3d | `internal/database/queries.sql.go` | Update GetMediaItemByFilePath params | +| 3e | `internal/services/media_scanner.go` | Pass libraryID to getMediaItemByFilePath | +| 4.0 | `internal/services/scanner_logger.go` | **NEW FILE** - File logging infrastructure | +| 4a | `internal/services/media_scanner.go` | Add fsnotify.Remove handler in WatchChanges + use logger | +| 4b | `internal/services/media_scanner.go` | Add cleanup loop in ScanFolders + use logger | + +### Docker Compose Volume Mount + +Add to your `docker-compose.yml` to persist logs: + +```yaml +volumes: + - ./logs:/app/logs +``` + +### Important: SQL Code Generation + +**The file `internal/database/queries.sql.go` is AUTO-GENERATED from `internal/database/queries/queries.sql`** + +After editing `queries.sql`, you MUST regenerate the Go code: + +**Option A - Run SQL code generation (recommended):** +```bash +cd internal/database && go generate ./... +``` + +**Option B - Manual update (if Option A fails):** + +If `go generate` fails or is not available, manually update `queries.sql.go`: +1. Add `LibraryID pgtype.UUID` parameter to `GetMediaItemByFilePathParams` struct +2. Add parameter to the query function call + +For Fix 1, manually add `CreatedAt` to: +- `CreateMediaItemParams` struct +- The query VALUES + +--- + +## Implementation Order + +1. Fix 3a, 3b, 3c, 3d, 3e - Add library_id filtering (foundational for Fix 4) +2. Fix 1 - File mtime for created_at +3. Fix 2 - Force rescan UPDATE +4. Fix 4 - Watch mode + Rescan deletion handling + +--- + +## Testing Requirements + +### Existing Tests Analysis + +**Current scanner integration tests** (`cmd/server/tests/scanner_integration_test.go`): +- Uses `/app/uploads` as test folder +- Tests scan, progress tracking, watch mode start/stop +- **After Fix 3:** Tests can safely use `/app/uploads` because GetMediaItemByFilePath now filters by library_id - test library's entries are isolated from user's library + +**Current unit tests** (`internal/services/*_test.go`): +- `media_scanner_epub_cover_test.go` - Tests cover extraction +- `media_scanner_hash_test.go` - Tests hash calculation +- `media_scanner_library_type_test.go` - Tests library type detection +- `media_scanner_comic_test.go` - Tests comic handling + +These unit tests test specific functions but don't test the new features we're adding. + +### Required Test Changes + +#### Fix 1 & 2 Tests (File mtime + Force rescan UPDATE) + +**Unit Test:** Add test in `internal/services/media_scanner_test.go` (create if doesn't exist) + +```go +// TestProcessMediaFile_UsesFileMtime tests that created_at uses file modification time +func TestProcessMediaFile_UsesFileMtime(t *testing.T) { + // Create test file with specific modification time + testFile := createTestEpub(t, "test-book.epub") + defer os.Remove(testFile) + + // Set specific mtime + pastTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + os.Chtimes(testFile, pastTime, pastTime) + + // Process file + scanner := NewMediaScanner(db) + scanner.SetFolders([]string{filepath.Dir(testFile)}) + scanner.SetLibraryID(testLibraryID) + + wasNew, err := scanner.ProcessMediaFile(ctx, testFile) + require.NoError(t, err) + require.True(t, wasNew) + + // Verify created_at matches file mtime, not scan time + item, err := db.GetMediaItemByFilePath(ctx, testFile, testLibraryID) + require.NoError(t, err) + assert.WithinDuration(t, pastTime, item.CreatedAt.Time, time.Second) +} + +// TestForceRescan_PreservesCreatedAt tests that force rescan doesn't reset created_at +func TestForceRescan_PreservesCreatedAt(t *testing.T) { + // Create and process file + testFile := createTestEpub(t, "test-book.epub") + defer os.Remove(testFile) + + scanner := NewMediaScanner(db) + scanner.SetFolders([]string{filepath.Dir(testFile)}) + scanner.SetLibraryID(testLibraryID) + + _, _ = scanner.ProcessMediaFile(ctx, testFile) + + // Get original created_at + item, _ := db.GetMediaItemByFilePath(ctx, testFile, testLibraryID) + originalCreatedAt := item.CreatedAt.Time + + // Wait a moment to ensure time difference + time.Sleep(100 * time.Millisecond) + + // Force rescan + scanner.SetForce(true) + _, _ = scanner.ProcessMediaFile(ctx, testFile) + + // Verify created_at is preserved + item, _ = db.GetMediaItemByFilePath(ctx, testFile, testLibraryID) + assert.Equal(t, originalCreatedAt, item.CreatedAt.Time) +} +``` + +#### Fix 3 Tests (library_id filtering) + +**Unit Test:** Add to same test file + +```go +// TestGetMediaItemByFilePath_FiltersByLibrary tests that GetMediaItemByFilePath respects library_id +func TestGetMediaItemByFilePath_FiltersByLibrary(t *testing.T) { + // Same file path in two different libraries + testFile := createTestEpub(t, "shared-book.epub") + defer os.Remove(testFile) + + // Add to library A + scannerA := NewMediaScanner(db) + scannerA.SetLibraryID(libraryAID) + _, _ = scannerA.ProcessMediaFile(ctx, testFile) + + // Add same path to library B (simulating shared folder scenario) + scannerB := NewMediaScanner(db) + scannerB.SetLibraryID(libraryBID) + _, _ = scannerB.ProcessMediaFile(ctx, testFile) + + // Verify each library has its own entry + itemA, errA := db.GetMediaItemByFilePath(ctx, testFile, libraryAID) + itemB, errB := db.GetMediaItemByFilePath(ctx, testFile, libraryBID) + + require.NoError(t, errA) + require.NoError(t, errB) + assert.Equal(t, libraryAID, itemA.LibraryID) + assert.Equal(t, libraryBID, itemB.LibraryID) + assert.NotEqual(t, itemA.ID, itemB.ID) +} +``` + +#### Fix 4 Tests (Deletion handling) + +**Integration Test:** Modify existing scanner_integration_test.go + +**Note:** Tests can continue to use `/app/uploads` - Fix 3 ensures test library's entries are isolated from user's library. + +```go +// TestScan_DeletesOrphanedBooks tests that rescan removes books no longer on filesystem +func TestScan_DeletesOrphanedBooks(t *testing.T) { + // Create test library pointing to /app/uploads (same as existing tests) + testFolder := "/app/uploads" + + // Create test library with test folder + libraryID := createTestLibrary(t, s.setup.Token, "Orphan Test Library", testFolder) + + // Add a test file + testFile := createTestEpubFile(t, testFolder, "test-orphan-book.epub") + defer os.Remove(testFile) // Cleanup after test + + // Add a test file + testFile := createTestEpubFile(t, testFolder, "test-book.epub") + + // Initial scan + scanLibrary(t, s.setup.Server.URL, libraryID, s.setup.Token) + + // Verify book exists + items, _ := s.setup.DB.ListMediaItemsByLibrary(ctx, libraryID) + require.Len(t, items, 1) + + // Delete file from filesystem (simulating user deletion) + os.Remove(testFile) + + // Rescan + scanLibrary(t, s.setup.Server.URL, libraryID, s.setup.Token) + + // Verify book was deleted from DB + items, _ = s.setup.DB.ListMediaItemsByLibrary(ctx, libraryID) + assert.Len(t, items, 0, "Orphaned book should be removed from database") +} +``` + +**Integration Test for Watch Mode deletion:** + +**Note:** Tests can continue to use `/app/uploads` - Fix 3 ensures test library's entries are isolated. + +```go +// TestWatchMode_DeletesRemovedFile tests that watch mode detects and deletes removed files +func TestWatchMode_DeletesRemovedFile(t *testing.T) { + // Use /app/uploads - Fix 3 handles isolation + testFolder := "/app/uploads" + + // Create test library + libraryID := createTestLibrary(t, s.setup.Token, "Watch Delete Test", testFolder) + + // Add test file + testFile := createTestEpubFile(t, testFolder, "watch-test.epub") + + // Start watch mode + startWatchMode(t, s.setup.Server.URL, libraryID, s.setup.Token) + + // Wait for initial scan + time.Sleep(2 * time.Second) + + // Verify book exists + items, _ := s.setup.DB.ListMediaItemsByLibrary(ctx, libraryID) + require.Len(t, items, 1) + + // Delete file + os.Remove(testFile) + + // Wait for watch mode to detect + time.Sleep(2 * time.Second) + + // Verify book was deleted + items, _ = s.setup.DB.ListMediaItemsByLibrary(ctx, libraryID) + assert.Len(t, items, 0, "Book should be deleted when file removed from filesystem") + + // Stop watch mode + stopWatchMode(t, s.setup.Server.URL, libraryID, s.setup.Token) +} +``` + +### Test Infrastructure Changes Required + +1. **No folder changes needed:** The Fix 3 (library_id filtering) ENABLEs tests to safely use `/app/uploads` without affecting user's library +2. **Existing tests should work as-is** after Fix 3 is implemented +3. **Use test_helpers:** Ensure all tests use `setupTestServer()` from `test_helpers.go` + +### Documentation Updates + +If API behavior changes, update: +- `docs/developer/api/scanner.md` - For any endpoint changes +- `docs/user/` - If user-facing behavior changes + +### Running Tests + +After implementation, run: +```bash +# Unit tests +go test ./internal/services/... -v -run "TestProcessMediaFile|TestGetMediaItemByFilePath|TestForceRescan" + +# Integration tests +go test ./cmd/server/tests/... -v -run "Scanner" + +# All tests +go test ./... -v +``` diff --git a/SUMMARY.md b/SUMMARY.md new file mode 100644 index 0000000..070003d --- /dev/null +++ b/SUMMARY.md @@ -0,0 +1,117 @@ +# Bookhoard Scanner Issues - Summary and Fixes + +## Issues Identified + +### Issue 1: Scanner uses scan time instead of file modification time +**Problem:** When a new book is inserted into the database, `created_at` uses the database default (`NOW()`) which is the scan time, not when the file was actually added to the folder. + +**Example:** +- File `Pride and Prejudice.epub` was added to folder: Feb 2, 2026 +- Database shows `created_at`: Feb 26 17:51:19 (scan time) + +**Impact:** "Recently Added" section doesn't reflect actual file modification dates. + +--- + +### Issue 2: Force rescan creates new entries instead of updating +**Problem:** When force rescan is enabled, the scanner DELETES the existing media item and re-INSERTs it, which creates a new `created_at` timestamp. + +**Location:** `internal/services/media_scanner.go` lines 366-372 + +**Current code:** +```go +if s.forceRescan { + // Force update: delete existing and re-create + if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { + fmt.Printf("Warning: failed to delete existing media item: %v\n", err) + } + // Continue to create new entry below +} +``` + +**Impact:** Force rescan resets `created_at` to scan time. + +--- + +### Issue 3: Scanner checks file_path globally, not per-library +**Problem:** `GetMediaItemByFilePath` query doesn't filter by `library_id`: + +```sql +SELECT * FROM media_items WHERE file_path = $1; +``` + +**Impact:** When tests scan `/app/uploads` in their test library, they find and modify media items from YOUR manually created library because they share the same file path. This is why all your books show today's timestamps - tests constantly re-scan your uploads folder. + +--- + +### Issue 4: Watch mode doesn't handle file deletions +**Problem:** The watcher only handles `fsnotify.Create` and `fsnotify.Write` events, but not `fsnotify.Remove`. + +**Location:** `internal/services/media_scanner.go` - `WatchChanges()` function (lines 1427-1467) + +**Impact:** When a file is deleted from the OS, the database entry remains. + +--- + +### Issue 5: Rescan doesn't handle deleted books +**Problem:** When doing a full rescan, the scanner doesn't check for files that were deleted from the filesystem. + +**Impact:** Orphaned media items remain in the database for files that no longer exist. + +--- + +## Recommended Fixes + +### Fix 1: Use file modification time for created_at + +**File:** `internal/services/media_scanner.go` + +**Change:** Get file's actual modification time using `os.Stat()` and pass it to the INSERT query instead of relying on the database default. + +--- + +### Fix 2: Fix force rescan to use UPDATE instead of DELETE + INSERT + +**File:** `internal/services/media_scanner.go` - lines 366-372 + +**Change:** Instead of deleting and re-creating, use an UPDATE query that preserves `created_at` and only updates `updated_at`. + +--- + +### Fix 3: Fix GetMediaItemByFilePath to filter by library_id + +**File:** `internal/database/queries/queries.sql` - `GetMediaItemByFilePath` + +**Change:** Add `AND library_id = $2` to the query, and update the scanner to pass the library_id when checking for existing items. + +--- + +### Fix 4: Add file deletion handling to watch mode + +**File:** `internal/services/media_scanner.go` - `WatchChanges()` function + +**Change:** Add handler for `fsnotify.Remove` events to hard delete media items from the database when files are deleted from the OS. + +--- + +### Fix 5: Add deleted book handling to rescan + +**File:** `internal/services/media_scanner.go` - `ScanFolders()` function + +**Change:** After scanning the filesystem, compare the results against the database. Any media items whose files no longer exist should be hard deleted from the database. + +--- + +## Dashboard Carousel Ordering + +**Finding:** The carousel order is actually correct. The database shows Beowulf as the newest book (created_at: 2026-02-26 17:51:21.977928+00), so it correctly appears first. + +**If you want oldest books on the left instead:** Change `ORDER BY created_at DESC` to `ORDER BY created_at ASC` in `internal/database/queries/queries.sql` - `GetRecentlyAddedItems` query. + +--- + +## File Deletion Strategy + +**Decision:** Hard delete (completely remove from database) for deleted files. + +**Note on file logs:** Could be added later with a separate logging table if needed. Would require a mount point in Docker Compose. diff --git a/bruno/collections/Get Book Collections.yml b/bruno/collections/Get Book Collections.yml index c8fc6dd..57cbc62 100644 --- a/bruno/collections/Get Book Collections.yml +++ b/bruno/collections/Get Book Collections.yml @@ -2,7 +2,20 @@ info: name: Get Book Collections type: http seq: 12 + http: method: GET - url: '{{base_url}}/api/collections/books/{{book_id}}' + url: "{{base_url}}/api/collections/books" + body: + type: json + data: |- + { + "collection_id": "{{collection_id}}" + } auth: inherit + +settings: + encodeUrl: true + timeout: 0 + followRedirects: true + maxRedirects: 5 diff --git a/bruno/environments/Bookhoard.yml b/bruno/environments/Bookhoard.yml index 334d97f..d54b474 100644 --- a/bruno/environments/Bookhoard.yml +++ b/bruno/environments/Bookhoard.yml @@ -13,9 +13,9 @@ variables: - name: note_id value: 7710a602-g29b-61d4-c716-446655440002 - name: library_id - value: 551ac19c-896a-4406-b479-353fc489b295 + value: 849151fb-564e-4b24-89e3-d11360789576 - name: job_id - value: 450094c6-a8f1-4125-aea7-77df80223877 + value: 295654b3-eb4b-4e85-ac6c-9361c5821195 - name: rating value: "5" - name: is_visible @@ -33,4 +33,4 @@ variables: - secret: true name: other_device_id - name: collection_id - value: 11e8d915-ef95-4257-98b3-b8eeacfd59b9 + value: 412c03c3-0843-4bd4-b764-cc9457bb9df2 diff --git a/bruno/library/Get Scan Settings.yml b/bruno/library/Get Scan Settings.yml index 35d5a1f..baf3cda 100644 --- a/bruno/library/Get Scan Settings.yml +++ b/bruno/library/Get Scan Settings.yml @@ -2,15 +2,20 @@ info: name: Get Scan Settings type: http seq: 2 + http: method: GET - url: '{{base_url}}/api/library/scan-settings' - auth: inherit - body: - type: none + url: "{{base_url}}/api/libraries/scan-settings" headers: - - key: Content-Type - value: application/json + - name: "" + value: application/json + auth: inherit + +settings: + encodeUrl: true + timeout: 0 + followRedirects: true + maxRedirects: 5 docs: |- ## Get Scan Settings diff --git a/bruno/library/Update Scan Settings.yml b/bruno/library/Update Scan Settings.yml index f87a110..7410c06 100644 --- a/bruno/library/Update Scan Settings.yml +++ b/bruno/library/Update Scan Settings.yml @@ -2,17 +2,27 @@ info: name: Update Scan Settings type: http seq: 1 + http: method: PUT - url: '{{base_url}}/api/library/scan-settings' - auth: inherit + url: "{{base_url}}/api/libraries/scan-settings" + headers: + - name: "" + value: application/json body: type: json - jsonBody: "{\n \"scan_frequency_minutes\": 60,\n \"auto_scan_enabled\":\ - \ true" - headers: - - key: Content-Type - value: application/json + data: |- + { + "scan_frequency_minutes": 30, + "auto_scan_enabled": true + } + auth: inherit + +settings: + encodeUrl: true + timeout: 0 + followRedirects: true + maxRedirects: 5 docs: |- ## Update Scan Settings diff --git a/bruno/scanner/Scan Media Items.yml b/bruno/scanner/Scan Media Items.yml index 860e6fa..73b373f 100644 --- a/bruno/scanner/Scan Media Items.yml +++ b/bruno/scanner/Scan Media Items.yml @@ -21,6 +21,29 @@ http: } auth: inherit +runtime: + scripts: + - type: after-response + code: |- + function onResponse(res) { + try { + const responseBody = res.getBody(); + const job_id = responseBody.job_id; + + if (job_id) { + bru.setEnvVar("job_id", job_id, { persist: true }); + console.log("Access job_id saved:", job_id); + } + + if (!job_id) { + console.log("No job_ids found in response."); + } + } catch (error) { + console.error("Error in post-response script:", error.message); + } + } + onResponse(res); + settings: encodeUrl: true timeout: 0 diff --git a/internal/database/querier.go b/internal/database/querier.go index 44ba9c9..74f65e6 100644 --- a/internal/database/querier.go +++ b/internal/database/querier.go @@ -175,7 +175,8 @@ type Querier interface { GetMediaHighlight(ctx context.Context, id pgtype.UUID) (MediaHighlights, error) GetMediaHighlights(ctx context.Context, arg GetMediaHighlightsParams) ([]MediaHighlights, error) GetMediaItem(ctx context.Context, id pgtype.UUID) (MediaItems, error) - GetMediaItemByFilePath(ctx context.Context, filePath string) (MediaItems, error) + GetMediaItemByFilePath(ctx context.Context, arg GetMediaItemByFilePathParams) (MediaItems, error) + GetMediaItemByFilePathAnyLibrary(ctx context.Context, filePath string) (MediaItems, error) // ============================================ // KOREADER SYNC PROTOCOL // ============================================ diff --git a/internal/database/queries.sql.go b/internal/database/queries.sql.go index 02e79e1..2d47946 100644 --- a/internal/database/queries.sql.go +++ b/internal/database/queries.sql.go @@ -549,39 +549,40 @@ func (q *Queries) CreateMediaHighlight(ctx context.Context, arg CreateMediaHighl } const CreateMediaItem = `-- name: CreateMediaItem :one -INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27) +INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28) RETURNING id, library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, asin, date_published, publisher, contributors, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at, updated_at, format_group, format_mimetype, is_reflowable, has_fixed_layout, total_characters, chapter_count, entitlement_id, revision_number, kobo_content_id, kobo_metadata, tags_search, contributors_search, file_sha256, opf_identifier, opf_uuid, hash_confidence ` type CreateMediaItemParams struct { - LibraryID pgtype.UUID `db:"library_id" json:"library_id"` - Title string `db:"title" json:"title"` - Author pgtype.Text `db:"author" json:"author"` - Isbn pgtype.Text `db:"isbn" json:"isbn"` - Description pgtype.Text `db:"description" json:"description"` - FilePath string `db:"file_path" json:"file_path"` - FileSize pgtype.Int8 `db:"file_size" json:"file_size"` - MimeType pgtype.Text `db:"mime_type" json:"mime_type"` - CoverImagePath pgtype.Text `db:"cover_image_path" json:"cover_image_path"` - Series pgtype.Text `db:"series" json:"series"` - SeriesNumber pgtype.Int4 `db:"series_number" json:"series_number"` - Tags []string `db:"tags" json:"tags"` - TagsSearch []string `db:"tags_search" json:"tags_search"` - Asin pgtype.Text `db:"asin" json:"asin"` - DatePublished pgtype.Date `db:"date_published" json:"date_published"` - Publisher pgtype.Text `db:"publisher" json:"publisher"` - Contributors []string `db:"contributors" json:"contributors"` - ContributorsSearch []string `db:"contributors_search" json:"contributors_search"` - Language pgtype.Text `db:"language" json:"language"` - Edition pgtype.Text `db:"edition" json:"edition"` - PageCount pgtype.Int4 `db:"page_count" json:"page_count"` - Genre pgtype.Text `db:"genre" json:"genre"` - CopyrightYear pgtype.Int4 `db:"copyright_year" json:"copyright_year"` - GoodreadsID pgtype.Text `db:"goodreads_id" json:"goodreads_id"` - OpenlibraryID pgtype.Text `db:"openlibrary_id" json:"openlibrary_id"` - GoogleBooksID pgtype.Text `db:"google_books_id" json:"google_books_id"` - AddedByAdminID pgtype.UUID `db:"added_by_admin_id" json:"added_by_admin_id"` + LibraryID pgtype.UUID `db:"library_id" json:"library_id"` + Title string `db:"title" json:"title"` + Author pgtype.Text `db:"author" json:"author"` + Isbn pgtype.Text `db:"isbn" json:"isbn"` + Description pgtype.Text `db:"description" json:"description"` + FilePath string `db:"file_path" json:"file_path"` + FileSize pgtype.Int8 `db:"file_size" json:"file_size"` + MimeType pgtype.Text `db:"mime_type" json:"mime_type"` + CoverImagePath pgtype.Text `db:"cover_image_path" json:"cover_image_path"` + Series pgtype.Text `db:"series" json:"series"` + SeriesNumber pgtype.Int4 `db:"series_number" json:"series_number"` + Tags []string `db:"tags" json:"tags"` + TagsSearch []string `db:"tags_search" json:"tags_search"` + Asin pgtype.Text `db:"asin" json:"asin"` + DatePublished pgtype.Date `db:"date_published" json:"date_published"` + Publisher pgtype.Text `db:"publisher" json:"publisher"` + Contributors []string `db:"contributors" json:"contributors"` + ContributorsSearch []string `db:"contributors_search" json:"contributors_search"` + Language pgtype.Text `db:"language" json:"language"` + Edition pgtype.Text `db:"edition" json:"edition"` + PageCount pgtype.Int4 `db:"page_count" json:"page_count"` + Genre pgtype.Text `db:"genre" json:"genre"` + CopyrightYear pgtype.Int4 `db:"copyright_year" json:"copyright_year"` + GoodreadsID pgtype.Text `db:"goodreads_id" json:"goodreads_id"` + OpenlibraryID pgtype.Text `db:"openlibrary_id" json:"openlibrary_id"` + GoogleBooksID pgtype.Text `db:"google_books_id" json:"google_books_id"` + AddedByAdminID pgtype.UUID `db:"added_by_admin_id" json:"added_by_admin_id"` + CreatedAt pgtype.Timestamptz `db:"created_at" json:"created_at"` } // Media Items queries @@ -614,6 +615,7 @@ func (q *Queries) CreateMediaItem(ctx context.Context, arg CreateMediaItemParams arg.OpenlibraryID, arg.GoogleBooksID, arg.AddedByAdminID, + arg.CreatedAt, ) var i MediaItems err := row.Scan( @@ -3250,11 +3252,72 @@ func (q *Queries) GetMediaItem(ctx context.Context, id pgtype.UUID) (MediaItems, } const GetMediaItemByFilePath = `-- name: GetMediaItemByFilePath :one -SELECT id, library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, asin, date_published, publisher, contributors, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at, updated_at, format_group, format_mimetype, is_reflowable, has_fixed_layout, total_characters, chapter_count, entitlement_id, revision_number, kobo_content_id, kobo_metadata, tags_search, contributors_search, file_sha256, opf_identifier, opf_uuid, hash_confidence FROM media_items WHERE file_path = $1 +SELECT id, library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, asin, date_published, publisher, contributors, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at, updated_at, format_group, format_mimetype, is_reflowable, has_fixed_layout, total_characters, chapter_count, entitlement_id, revision_number, kobo_content_id, kobo_metadata, tags_search, contributors_search, file_sha256, opf_identifier, opf_uuid, hash_confidence FROM media_items WHERE file_path = $1 AND library_id = $2 ` -func (q *Queries) GetMediaItemByFilePath(ctx context.Context, filePath string) (MediaItems, error) { - row := q.db.QueryRow(ctx, GetMediaItemByFilePath, filePath) +type GetMediaItemByFilePathParams struct { + FilePath string `db:"file_path" json:"file_path"` + LibraryID pgtype.UUID `db:"library_id" json:"library_id"` +} + +func (q *Queries) GetMediaItemByFilePath(ctx context.Context, arg GetMediaItemByFilePathParams) (MediaItems, error) { + row := q.db.QueryRow(ctx, GetMediaItemByFilePath, arg.FilePath, arg.LibraryID) + var i MediaItems + err := row.Scan( + &i.ID, + &i.LibraryID, + &i.Title, + &i.Author, + &i.Isbn, + &i.Description, + &i.FilePath, + &i.FileSize, + &i.MimeType, + &i.CoverImagePath, + &i.Series, + &i.SeriesNumber, + &i.Tags, + &i.Asin, + &i.DatePublished, + &i.Publisher, + &i.Contributors, + &i.Language, + &i.Edition, + &i.PageCount, + &i.Genre, + &i.CopyrightYear, + &i.GoodreadsID, + &i.OpenlibraryID, + &i.GoogleBooksID, + &i.AddedByAdminID, + &i.CreatedAt, + &i.UpdatedAt, + &i.FormatGroup, + &i.FormatMimetype, + &i.IsReflowable, + &i.HasFixedLayout, + &i.TotalCharacters, + &i.ChapterCount, + &i.EntitlementID, + &i.RevisionNumber, + &i.KoboContentID, + &i.KoboMetadata, + &i.TagsSearch, + &i.ContributorsSearch, + &i.FileSha256, + &i.OpfIdentifier, + &i.OpfUuid, + &i.HashConfidence, + ) + return i, err +} + +const GetMediaItemByFilePathAnyLibrary = `-- name: GetMediaItemByFilePathAnyLibrary :one +SELECT id, library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, asin, date_published, publisher, contributors, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at, updated_at, format_group, format_mimetype, is_reflowable, has_fixed_layout, total_characters, chapter_count, entitlement_id, revision_number, kobo_content_id, kobo_metadata, tags_search, contributors_search, file_sha256, opf_identifier, opf_uuid, hash_confidence FROM media_items WHERE file_path = $1 LIMIT 1 +` + +func (q *Queries) GetMediaItemByFilePathAnyLibrary(ctx context.Context, filePath string) (MediaItems, error) { + row := q.db.QueryRow(ctx, GetMediaItemByFilePathAnyLibrary, filePath) var i MediaItems err := row.Scan( &i.ID, diff --git a/internal/database/queries/queries.sql b/internal/database/queries/queries.sql index ca5fe29..2a0a842 100644 --- a/internal/database/queries/queries.sql +++ b/internal/database/queries/queries.sql @@ -128,8 +128,8 @@ ORDER BY l.created_at DESC; -- Media Items queries -- name: CreateMediaItem :one -INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27) +INSERT INTO media_items (library_id, title, author, isbn, description, file_path, file_size, mime_type, cover_image_path, series, series_number, tags, tags_search, asin, date_published, publisher, contributors, contributors_search, language, edition, page_count, genre, copyright_year, goodreads_id, openlibrary_id, google_books_id, added_by_admin_id, created_at) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28) RETURNING *; -- name: GetMediaItem :one @@ -300,7 +300,10 @@ RETURNING *; DELETE FROM media_items WHERE id = $1; -- name: GetMediaItemByFilePath :one -SELECT * FROM media_items WHERE file_path = $1; +SELECT * FROM media_items WHERE file_path = $1 AND library_id = $2; + +-- name: GetMediaItemByFilePathAnyLibrary :one +SELECT * FROM media_items WHERE file_path = $1 LIMIT 1; -- name: GetReadingProgress :one SELECT * FROM reading_progress WHERE media_item_id = $1 AND user_id = $2; diff --git a/internal/handlers/koreader.go b/internal/handlers/koreader.go index 27add3c..1d8c539 100644 --- a/internal/handlers/koreader.go +++ b/internal/handlers/koreader.go @@ -264,8 +264,8 @@ func (h *KOReaderHandler) resolveBookToMediaItem(c echo.Context, deviceID pgtype return alias.MediaItemID, alias.ConfidenceScore.Float64 } - // Try to find by file path - mediaItem, err := h.db.GetMediaItemByFilePath(ctx, book.FilePath) + // Try to find by file path (search any library) + mediaItem, err := h.db.GetMediaItemByFilePathAnyLibrary(ctx, book.FilePath) if err == nil { // Create new alias confidence := 0.7 diff --git a/internal/handlers/scanner.go b/internal/handlers/scanner.go index c7031a2..c7f2c7b 100644 --- a/internal/handlers/scanner.go +++ b/internal/handlers/scanner.go @@ -266,6 +266,7 @@ func (h *Handler) StartWatchModeForLibrary(ctx context.Context, libraryID pgtype } scanner.SetAdminID(adminID) + scanner.SetLibraryID(libraryID) scanner.WatchChanges(h.watchModeCtx) h.watchingLibraries[libraryIDStr] = true @@ -382,7 +383,7 @@ func (h *Handler) StartWatchModeForAllLibraries(ctx context.Context) error { for _, library := range libraries { libraryIDStr := fmt.Sprintf("%x", library.ID.Bytes) - if err := h.StartWatchModeForLibrary(ctx, library.ID, library.ID); err != nil { + if err := h.StartWatchModeForLibrary(ctx, library.ID, library.CreatedByAdminID); err != nil { fmt.Printf("Warning: failed to start watch mode for library %s: %v\n", libraryIDStr, err) continue } diff --git a/internal/services/media_scanner.go b/internal/services/media_scanner.go index ba60c0d..432898c 100644 --- a/internal/services/media_scanner.go +++ b/internal/services/media_scanner.go @@ -77,6 +77,7 @@ type MediaScanner struct { defaultLibraryID pgtype.UUID libraryTypes map[string][]string forceRescan bool + logger *ScannerLogger totalFiles int newItems int @@ -98,6 +99,7 @@ func NewMediaScanner(db *database.Queries) *MediaScanner { adminID: pgtype.UUID{}, defaultLibraryID: pgtype.UUID{Valid: false}, libraryTypes: make(map[string][]string), + logger: NewScannerLogger(), } } @@ -105,6 +107,10 @@ func (s *MediaScanner) SetAdminID(adminID pgtype.UUID) { s.adminID = adminID } +func (s *MediaScanner) SetLibraryID(libraryID pgtype.UUID) { + s.defaultLibraryID = libraryID +} + func (s *MediaScanner) SetForce(force bool) { s.forceRescan = force } @@ -242,6 +248,55 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { fmt.Printf("Scan completed: %d total files scanned, %d media files found, %d new items, %d errors\n", processedFiles, mediaFiles, s.newItems, s.errors) + // Clean up: Find media items in DB that no longer exist on filesystem + for _, folder := range s.folders { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err != nil { + continue + } + libraryID := lib.LibraryID + + dbItems, err := s.db.ListMediaItemsByLibrary(ctx, libraryID) + if err != nil { + fmt.Printf("Warning: failed to get library items for cleanup: %v\n", err) + continue + } + + // Build set of scanned file paths for this folder + scannedPaths := make(map[string]bool) + filepath.WalkDir(folder, func(path string, d os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if !d.IsDir() && s.isScannableFile(path) { + scannedPaths[path] = true + } + return nil + }) + + // Delete items whose files no longer exist - with safety logging + for _, item := range dbItems { + filePath := item.FilePath + if filePath != "" && !scannedPaths[filePath] { + msg := fmt.Sprintf("[RESCAN-CLEANUP] Orphaned media item found: ID=%s, Title=%s, Path=%s", + item.ID, item.Title, filePath) + s.logger.LogDelete(msg) + + delMsg := fmt.Sprintf("[RESCAN-CLEANUP] Deleting orphaned item '%s' (file no longer exists at %s)", + item.Title, filePath) + s.logger.LogDelete(delMsg) + + if err := s.db.DeleteMediaItem(ctx, item.ID); err != nil { + errMsg := fmt.Sprintf("[RESCAN-CLEANUP] ERROR: failed to delete orphaned item %s: %v", item.Title, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[RESCAN-CLEANUP] SUCCESS: deleted orphaned item '%s'", item.Title)) + } + } + } + } + if s.job != nil && s.totalFiles > 0 { s.job.UpdateProgress(1.0, processedFiles, s.newItems, s.errors) } @@ -357,19 +412,39 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, fmt.Printf("File info for %s: size=%d\n", path, info.Size()) + // Get file modification time for created_at + fileModTime := info.ModTime() + + // Find library for this file's folder + var libraryID pgtype.UUID + for _, folder := range s.folders { + if strings.HasPrefix(path, folder) { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err != nil { + return false, fmt.Errorf("failed to find library for folder %s: %v", folder, err) + } + libraryID = lib.LibraryID + break + } + } + + if !libraryID.Valid { + return false, fmt.Errorf("no library found for file path: %s", path) + } + // Check if media item already exists in database - existingItem, err := s.getMediaItemByFilePath(ctx, path) + existingItem, err := s.getMediaItemByFilePath(ctx, path, libraryID) if err == nil { fmt.Printf("Media item already exists in database: %s (size: %d vs %d)\n", path, existingItem.FileSize.Int64, info.Size()) // If force rescan is enabled, always re-process if s.forceRescan { - fmt.Printf("Force rescan enabled, re-processing existing media item: %s\n", path) - // Force update: delete existing and re-create - if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { - fmt.Printf("Warning: failed to delete existing media item: %v\n", err) + fmt.Printf("Force rescan enabled, updating existing media item: %s\n", path) + // Use UPDATE instead of DELETE+INSERT to preserve created_at + if err := s.updateMediaItem(ctx, existingItem.ID, path, info); err != nil { + fmt.Printf("Warning: failed to update existing media item: %v\n", err) } - // Continue to create new entry below + return false, nil } else { // Normal behavior: check if file has changed (by size) if existingItem.FileSize.Int64 != info.Size() { @@ -483,22 +558,7 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, metadata.Author = "Unknown" } - // Find library for this folder - var libraryID pgtype.UUID - for _, folder := range s.folders { - if strings.HasPrefix(path, folder) { - lib, err := s.db.GetLibraryByFolder(ctx, folder) - if err != nil { - return false, fmt.Errorf("failed to find library for folder %s: %v", folder, err) - } - libraryID = lib.LibraryID - break - } - } - - if !libraryID.Valid { - return false, fmt.Errorf("no library found for file path: %s", path) - } + // libraryID already determined at start of function // Normalize metadata fields for display metadata.Contributors = utils.NormalizeContributors(metadata.Contributors) @@ -529,6 +589,7 @@ func (s *MediaScanner) processMediaFile(ctx context.Context, path string) (bool, Tags: metadata.Tags, TagsSearch: tagsSearch, AddedByAdminID: s.adminID, + CreatedAt: pgtype.Timestamptz{Time: fileModTime, Valid: true}, }) if err != nil { return false, fmt.Errorf("failed to create media item: %v", err) @@ -1387,13 +1448,46 @@ func isImageFile(filename string) bool { return ext == ".jpg" || ext == ".jpeg" || ext == ".png" || ext == ".gif" } -func (s *MediaScanner) updateMediaItem(ctx context.Context, mediaItemID pgtype.UUID, filePath string, info os.FileInfo) error { - // Update disabled - scanner creates items but doesn't update - return nil +func (s *MediaScanner) updateMediaItem(ctx context.Context, mediaItemID pgtype.UUID, path string, info os.FileInfo) error { + // Re-extract metadata for the update + metadata, err := s.extractMetadata(path) + if err != nil { + fmt.Printf("Warning: failed to extract metadata for force rescan %s: %v\n", path, err) + metadata = &MediaMetadata{} + } + + // Normalize metadata fields + metadata.Contributors = utils.NormalizeContributors(metadata.Contributors) + metadata.Tags = utils.NormalizeTags(metadata.Tags) + contributorsSearch := utils.NormalizeContributorsSearch(metadata.Contributors) + tagsSearch := utils.NormalizeTagsSearch(metadata.Tags) + + // Call the database update - only update fields available in MediaMetadata + _, err = s.db.UpdateMediaItem(ctx, database.UpdateMediaItemParams{ + ID: mediaItemID, + Title: metadata.Title, + Author: pgtype.Text{String: metadata.Author, Valid: metadata.Author != ""}, + Isbn: pgtype.Text{String: utils.NormalizeISBNSafe(metadata.ISBN), Valid: metadata.ISBN != ""}, + Description: pgtype.Text{String: metadata.Description, Valid: metadata.Description != ""}, + CoverImagePath: pgtype.Text{String: metadata.CoverPath, Valid: metadata.CoverPath != ""}, + Series: pgtype.Text{String: metadata.Series, Valid: metadata.Series != ""}, + SeriesNumber: pgtype.Int4{Int32: metadata.SeriesNumber, Valid: metadata.SeriesNumber > 0}, + Tags: metadata.Tags, + TagsSearch: tagsSearch, + Asin: pgtype.Text{String: metadata.ASIN, Valid: metadata.ASIN != ""}, + DatePublished: pgtype.Date{Time: metadata.PublishDate, Valid: !metadata.PublishDate.IsZero()}, + Publisher: pgtype.Text{String: metadata.Publisher, Valid: metadata.Publisher != ""}, + Contributors: metadata.Contributors, + ContributorsSearch: contributorsSearch, + }) + return err } -func (s *MediaScanner) getMediaItemByFilePath(ctx context.Context, filePath string) (database.MediaItems, error) { - return s.db.GetMediaItemByFilePath(ctx, filePath) +func (s *MediaScanner) getMediaItemByFilePath(ctx context.Context, filePath string, libraryID pgtype.UUID) (database.MediaItems, error) { + return s.db.GetMediaItemByFilePath(ctx, database.GetMediaItemByFilePathParams{ + FilePath: filePath, + LibraryID: libraryID, + }) } func (s *MediaScanner) getMimeType(path string) string { @@ -1454,6 +1548,55 @@ func (s *MediaScanner) WatchChanges(ctx context.Context) { } } + // Handle file deletions + if event.Has(fsnotify.Remove) && s.isScannableFile(event.Name) { + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] File removed from filesystem: %s", event.Name)) + + // Determine libraryID for this file + var libraryID pgtype.UUID + for _, folder := range s.folders { + if strings.HasPrefix(event.Name, folder) { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err == nil { + libraryID = lib.LibraryID + break + } + } + } + if !libraryID.Valid { + msg := fmt.Sprintf("[WATCH-DELETE] WARNING: could not determine library for deleted file: %s", event.Name) + s.logger.LogDelete(msg) + s.logger.LogError(msg) + return + } + + // Look up media item BEFORE deleting - log for safety + existingItem, err := s.db.GetMediaItemByFilePath(ctx, database.GetMediaItemByFilePathParams{ + FilePath: event.Name, + LibraryID: libraryID, + }) + if err == nil { + msg := fmt.Sprintf("[WATCH-DELETE] Found media item to delete: ID=%s, Title=%s, Path=%s", + existingItem.ID, existingItem.Title, existingItem.FilePath) + s.logger.LogDelete(msg) + + if err := s.db.DeleteMediaItem(ctx, existingItem.ID); err != nil { + errMsg := fmt.Sprintf("[WATCH-DELETE] ERROR: failed to delete media item %s: %v", existingItem.ID, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] SUCCESS: deleted media item '%s' (was at %s)", + existingItem.Title, existingItem.FilePath)) + } + } else if err != pgx.ErrNoRows { + errMsg := fmt.Sprintf("[WATCH-DELETE] ERROR: failed to look up media item for %s: %v", event.Name, err) + s.logger.LogDelete(errMsg) + s.logger.LogError(errMsg) + } else { + s.logger.LogDelete(fmt.Sprintf("[WATCH-DELETE] No media item found in database for deleted file: %s", event.Name)) + } + } + case err, ok := <-s.watcher.Errors: if !ok { return diff --git a/internal/services/scanner_logger.go b/internal/services/scanner_logger.go new file mode 100644 index 0000000..40c1667 --- /dev/null +++ b/internal/services/scanner_logger.go @@ -0,0 +1,107 @@ +package services + +import ( + "fmt" + "os" + "path/filepath" + "time" +) + +const ( + logDir = "/app/logs" + maxLogAgeDays = 7 +) + +type ScannerLogger struct { + deletesFile *os.File + errorsFile *os.File + currentDate string +} + +func NewScannerLogger() *ScannerLogger { + return &ScannerLogger{} +} + +func (l *ScannerLogger) ensureLogFiles() error { + today := time.Now().Format("2006-01-02") + + if l.currentDate == today && l.deletesFile != nil { + return nil + } + + if l.deletesFile != nil { + l.deletesFile.Close() + } + if l.errorsFile != nil { + l.errorsFile.Close() + } + + if err := os.MkdirAll(logDir, 0755); err != nil { + return fmt.Errorf("failed to create log directory: %v", err) + } + + deletesPath := filepath.Join(logDir, fmt.Sprintf("scanner-deletes-%s.log", today)) + errorsPath := filepath.Join(logDir, fmt.Sprintf("scanner-errors-%s.log", today)) + + deletesFile, err := os.OpenFile(deletesPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + return fmt.Errorf("failed to open deletes log file: %v", err) + } + + errorsFile, err := os.OpenFile(errorsPath, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + deletesFile.Close() + return fmt.Errorf("failed to open errors log file: %v", err) + } + + l.deletesFile = deletesFile + l.errorsFile = errorsFile + l.currentDate = today + + l.cleanupOldLogs() + + return nil +} + +func (l *ScannerLogger) cleanupOldLogs() { + cutoff := time.Now().AddDate(0, 0, -maxLogAgeDays) + + filepath.Walk(logDir, func(path string, info os.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr + } + if !info.IsDir() && info.ModTime().Before(cutoff) { + os.Remove(path) + } + return nil + }) +} + +func (l *ScannerLogger) LogDelete(message string) { + if err := l.ensureLogFiles(); err != nil { + fmt.Printf("ERROR: Failed to ensure log files: %v\n", err) + return + } + timestamp := time.Now().Format("2006-01-02 15:04:05") + logLine := fmt.Sprintf("[%s] %s\n", timestamp, message) + l.deletesFile.WriteString(logLine) +} + +func (l *ScannerLogger) LogError(message string) { + if err := l.ensureLogFiles(); err != nil { + fmt.Printf("ERROR: Failed to ensure log files: %v\n", err) + return + } + timestamp := time.Now().Format("2006-01-02 15:04:05") + logLine := fmt.Sprintf("[%s] %s\n", timestamp, message) + l.errorsFile.WriteString(logLine) +} + +func (l *ScannerLogger) Close() { + if l.deletesFile != nil { + l.deletesFile.Close() + } + if l.errorsFile != nil { + l.errorsFile.Close() + } +}