diff --git a/CALIBRE_OPF_IMPLEMENTATION.md b/CALIBRE_OPF_IMPLEMENTATION.md index 7a4f0af..952431f 100644 --- a/CALIBRE_OPF_IMPLEMENTATION.md +++ b/CALIBRE_OPF_IMPLEMENTATION.md @@ -157,30 +157,37 @@ Scanner Pipeline (EXISTING) 5. Normalize and store ``` -**New Step 3a**: Check for Calibre sidecar **before** embedded extraction +**Simplified Approach**: Modify existing `extractMetadata()` to check for Calibre sidecar first ``` Scanner Pipeline (NEW) ↓ 1. Detect book file 2. Check if already in database -3. **NEW** Extract metadata from Calibre metadata.opf (if exists) - - If sidecar found → use sidecar metadata - - If no sidecar → use embedded metadata (existing) +3. Extract metadata (MODIFIED FUNCTION) + ├── Check for Calibre metadata.opf sidecar ← NEW + │ └─ If found → use sidecar metadata + └── Fallback to embedded metadata (existing) + ├── EPUB → extractEPUBMetadata() + ├── PDF → extractPDFMetadata() + └── Comic → extractComicMetadata() 4. Extract folder structure metadata (existing) 5. Normalize and store (existing) ``` ### Code Changes +**SIMPLIFIED APPROACH** (thanks to user feedback): Modify existing `extractMetadata()` function instead of creating wrapper. + #### File: `internal/services/media_scanner.go` -**New Functions:** +**New Functions (to add):** -1. `extractMetadataFromCalibreSidecar(path string) (*MediaMetadata, error)` +1. `extractCalibreSidecar(path string) *MediaMetadata` - Checks for `metadata.opf` in same directory as book file - Calls `parseCalibreMetadataOPF()` if found - Returns nil if no sidecar exists (not an error) + - Returns nil on parse errors (graceful fallback) 2. `parseCalibreMetadataOPF(opfPath string) (*MediaMetadata, error)` - Opens and parses XML file @@ -191,9 +198,11 @@ Scanner Pipeline (NEW) **Modified Function:** -1. `processMediaFile(ctx context.Context, path string) (bool, error)` - - Add sidecar extraction check at line ~544 (before embedded extraction) - - Flow: sidecar → embedded → folder → filename +1. `extractMetadata(path string) (*MediaMetadata, error)` at line 712 + - Add sidecar check at very beginning (before switch statement) + - If sidecar found → return sidecar metadata immediately + - Otherwise → continue to existing embedded extraction logic + - **No call site changes needed** - all existing code continues to work #### New Struct: `CalibreOPFMetadata` @@ -279,13 +288,18 @@ Create `testdata/calibre/` with sample files: ### Phase 1: Core Functionality (REQUIRED) -- [ ] **Step 1.1**: Add `CalibreOPFMetadata` struct -- [ ] **Step 1.2**: Implement `parseCalibreMetadataOPF()` function -- [ ] **Step 1.3**: Implement `extractMetadataFromCalibreSidecar()` function -- [ ] **Step 1.4**: Modify `processMediaFile()` to call sidecar extraction +- [ ] **Step 1.1**: Add `CalibreOPFMetadata` struct (intermediate parsing struct) +- [ ] **Step 1.2**: Implement `parseCalibreMetadataOPF()` function (~120 lines) +- [ ] **Step 1.3**: Implement `extractCalibreSidecar()` function (~20 lines) +- [ ] **Step 1.4**: Modify `extractMetadata()` function (~10 lines) + - Add sidecar check at beginning of function (line ~712) + - Return sidecar metadata if found + - Otherwise continue to existing switch statement - [ ] **Step 1.5**: Add unit tests for OPF parsing - [ ] **Step 1.6**: Test with real Calibre library +**Total new code: ~150 lines** (simplified from original ~250 lines thanks to direct modification of `extractMetadata()`) + ### Phase 2: Documentation (REQUIRED) - [ ] **Step 2.1**: Create user documentation (`docs/user/calibre-support.md`) @@ -314,9 +328,10 @@ Create `testdata/calibre/` with sample files: **No breaking changes.** Existing behavior is preserved: - Libraries without `metadata.opf` files → Existing behavior (embedded metadata) -- Scanning logic unchanged → Only adds sidecar check before embedded extraction +- Scanning logic unchanged → Single function modification, no call site changes - Database unchanged → All fields already exist - API unchanged → No new endpoints or response fields +- **No wrapper function needed** → Simpler, cleaner implementation ## Success Criteria @@ -379,3 +394,34 @@ Create `testdata/calibre/` with sample files: - Users may organize audiobooks/comics in Calibre - Consistent behavior across library types - No additional complexity + +### Decision 4: Simplified Implementation (REVISED) +**Date**: 2026-03-26 +**Decision**: Modify `extractMetadata()` directly instead of creating wrapper function +**Rationale**: +- **User feedback**: Suggested simpler approach with if statement check +- **Less code**: ~150 lines vs. ~250 lines +- **No call site changes**: All existing code continues to work +- **Clearer flow**: Single entry point for metadata extraction +- **Better testability**: Test `extractMetadata()` with/without sidecar +- **Easier to maintain**: All metadata logic in one place + +**Original plan**: +```go +// Wrapper function approach +extractMetadataWithSidecar() { + if sidecar exists → return sidecar + return extractMetadata() +} +``` + +**Revised plan (SIMPLER)**: +```go +// Direct modification +extractMetadata() { + if sidecar exists → return sidecar // ← NEW: Just add this at top + // existing switch statement continues... +} +``` + +This simplification reduces code complexity and makes the implementation cleaner and easier to understand.