diff --git a/docs/development/calibre-opf-implementation.md b/docs/development/calibre-opf-implementation.md index cc10149..2010c1f 100644 --- a/docs/development/calibre-opf-implementation.md +++ b/docs/development/calibre-opf-implementation.md @@ -96,39 +96,46 @@ type MediaMetadata struct { ## Implementation Functions -### 1. extractMetadataFromCalibreSidecar() +### 1. extractCalibreSidecar() **Location**: `internal/services/media_scanner.go` **Signature**: ```go -func (s *MediaScanner) extractMetadataFromCalibreSidecar(mediaFilePath string) (*MediaMetadata, error) +func (s *MediaScanner) extractCalibreSidecar(path string) *MediaMetadata ``` **Purpose**: Checks for `metadata.opf` in the same directory as the book file. **Logic**: ```go -func (s *MediaScanner) extractMetadataFromCalibreSidecar(mediaFilePath string) (*MediaMetadata, error) { +func (s *MediaScanner) extractCalibreSidecar(path string) *MediaMetadata { // Get directory of media file - dir := filepath.Dir(mediaFilePath) - - // Check for metadata.opf + dir := filepath.Dir(path) opfPath := filepath.Join(dir, "metadata.opf") + + // Check if sidecar exists if _, err := os.Stat(opfPath); os.IsNotExist(err) { - // No sidecar file - not an error, just return nil - return nil, nil + return nil // No sidecar, not an error } - // Parse metadata.opf - return s.parseCalibreMetadataOPF(opfPath) + // Parse sidecar + metadata, err := s.parseCalibreMetadataOPF(opfPath) + if err != nil { + fmt.Printf("Warning: failed to parse Calibre metadata.opf: %v\n", err) + return nil // Parsing failed, fall back to embedded + } + + return metadata } ``` **Error Handling**: -- File not found → Return `nil, nil` (not an error) -- Permission denied → Log warning, return `nil, nil` -- Other errors → Log error, return `nil, err` +- File not found → Return `nil` (not an error) +- Permission denied → Log warning, return `nil` +- Parse errors → Log warning, return `nil` (graceful fallback) + +**Note**: Simplified from original plan - no wrapper function needed ### 2. parseCalibreMetadataOPF() @@ -260,40 +267,64 @@ func (s *MediaScanner) parseCalibreMetadataOPF(opfPath string) (*MediaMetadata, **XML Namespaces**: Handle Dublin Core (`dc:`) and Calibre (`calibre:`) namespaces properly. **Error Handling**: -- Malformed XML → Log warning, return `nil, nil` +- Malformed XML → Log warning, return `nil` - Missing required fields → Use available fields - Invalid date format → Log warning, skip field -### 3. extractMetadataWithSidecar() +### 3. Modified: extractMetadata() -**Location**: `internal/services/media_scanner.go` +**Location**: `internal/services/media_scanner.go` (line 712) -**Signature**: +**Signature**: (Unchanged - existing function) ```go -func (s *MediaScanner) extractMetadataWithSidecar(path string) (*MediaMetadata, error) +func (s *MediaScanner) extractMetadata(path string) (*MediaMetadata, error) ``` -**Purpose**: Implements sidecar-first metadata extraction. +**Purpose**: Extracts metadata from book files with sidecar-first approach. -**Logic**: +**Modified Logic**: ```go -func (s *MediaScanner) extractMetadataWithSidecar(path string) (*MediaMetadata, error) { - // Try Calibre sidecar first - if metadata, err := s.extractMetadataFromCalibreSidecar(path); metadata != nil { +func (s *MediaScanner) extractMetadata(path string) (*MediaMetadata, error) { + // NEW: Try Calibre sidecar first + if metadata := s.extractCalibreSidecar(path); metadata != nil { fmt.Printf("Using Calibre metadata.opf for %s\n", path) + + // Try to find cover image for sidecar metadata + coverPath := findSidecarCover(path) + if coverPath != "" { + metadata.CoverPath = s.getRelativePath(coverPath) + } + return metadata, nil - } else if err != nil { - // Log error but continue to embedded extraction - fmt.Printf("Warning: failed to read Calibre sidecar: %v\n", err) } - // Fallback to embedded metadata - return s.extractMetadata(path) + // EXISTING: Fallback to embedded metadata + ext := strings.ToLower(filepath.Ext(path)) + + switch ext { + case ".epub": + metadata, err := s.extractEPUBMetadata(path) + // ... existing EPUB logic + case ".pdf": + return s.extractPDFMetadata(path) + default: + return &MediaMetadata{ + Title: strings.TrimSuffix(filepath.Base(path), ext), + }, nil + } } ``` +**Changes**: +- Add sidecar check at very beginning (before switch statement) +- If sidecar found → return immediately with sidecar metadata +- Otherwise → continue to existing embedded extraction logic +- **No call site changes needed** - all existing code continues to work + **Priority**: Sidecar → Embedded → Folder → Filename +**Note**: This is a SIMPLIFIED approach based on user feedback. Original plan called for a wrapper function, but directly modifying `extractMetadata()` is cleaner and requires less code. + ## Database Schema Mapping All OPF fields map to existing `media_items` columns: @@ -569,6 +600,27 @@ type ScannerConfig struct { - [Calibre Manual](https://manual.calibre-ebook.com/) - [Go XML Encoding](https://pkg.go.dev/encoding/xml) +## Implementation Note: Simplified Approach + +**Original Plan**: Create a new wrapper function `extractMetadataWithSidecar()` that calls the existing `extractMetadata()`. + +**Revised Plan** (based on user feedback): Modify `extractMetadata()` directly to check for sidecar files at the beginning of the function. + +**Benefits of Simplified Approach**: +- **Less code**: ~150 lines vs. ~250 lines +- **No call site changes**: All existing code continues to work without modification +- **Clearer flow**: Single entry point for metadata extraction +- **Better testability**: Test `extractMetadata()` with/without sidecar in one place +- **Easier to maintain**: All metadata extraction logic in one function + +**What Changed**: +- No new wrapper function needed +- Only 2 new functions: `extractCalibreSidecar()` and `parseCalibreMetadataOPF()` +- Single modification to existing `extractMetadata()` function +- Simpler architecture, easier to understand + +This is a great example of how user feedback can improve implementation design! + ## See Also - [User Guide: Calibre Integration](../../user/calibre-integration.md)