From 35b1f466ec5bac0780519aac8a5008b9310e48c8 Mon Sep 17 00:00:00 2001 From: Vladimir Zagainov Date: Mon, 27 Jul 2026 23:21:50 +0300 Subject: [PATCH] feat: write unit tests for filtering logic --- CLAUDE.md | 9 ++ .../plans/2026-07-27-verify-scanner-wiring.md | 105 +++++++++++++++ internal/musicbrainz/filter_test.go | 122 ++++++++++++++++++ 3 files changed, 236 insertions(+) create mode 100644 docs/plans/2026-07-27-verify-scanner-wiring.md create mode 100644 internal/musicbrainz/filter_test.go diff --git a/CLAUDE.md b/CLAUDE.md index 82f7932..ac361e9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -152,6 +152,15 @@ Based on the specification (docs/Specification.md), the application follows a mo - Apply the specified fuzzy matching algorithm consistently - Centralize shared logic: Place reusable filtering, validation, or utility functions in dedicated files (e.g., internal/musicbrainz/filter.go) and import them across packages to ensure consistent behavior across cache-hit, cache-miss, and real-time paths +### Execution Flow +The application follows a sequential data pipeline: +1. Sync artists from Navidrome (populate artist_settings) +2. Sync discographies from MusicBrainz (populate external_releases with filtering) +3. Sync albums from Navidrome (populate local_albums) +4. Scan for missing releases using fuzzy matching (produces MissingRelease results) + +This flow is implemented in the `syncAndScan()` function in `cmd/naviwatcher/main.go`, which is called by the periodic sync loop and on startup. + ## Configuration Reference See docs/Specification.md Section 7 for full config.yaml structure including: - Server settings (host, port, basic auth) diff --git a/docs/plans/2026-07-27-verify-scanner-wiring.md b/docs/plans/2026-07-27-verify-scanner-wiring.md new file mode 100644 index 0000000..7350195 --- /dev/null +++ b/docs/plans/2026-07-27-verify-scanner-wiring.md @@ -0,0 +1,105 @@ +# Verify and Document Scanner Wiring Implementation + +## Overview +Verify that the scanner engine properly wires the ignore_singles and ignore_compilations toggles from artist_settings through both the MusicBrainz sync path and the scanner path. Ensure proper test coverage and document the data flow for clarity. + +## Context (from discovery) +- Files/components involved: + - cmd/naviwatcher/main.go (syncAndScan function) + - internal/scanner/scan.go (ScanArtist, ScanAll functions) + - internal/musicbrainz/sync.go (SyncArtistDiscography function) + - internal/musicbrainz/filter.go (ApplyTypeToggles functions) + - internal/database/artist_settings.go (GetArtistSettings, GetAllArtistSettings functions) +- Related patterns found: Filter centralization pattern mentioned in CLAUDE.md +- Dependencies identified: database package for artist settings access + +## Development Approach +- **Testing approach**: TDD (tests first) +- Complete each task fully before moving to the next +- Make small, focused changes +- **CRITICAL: every task MUST include new/updated tests** for code changes in that task + - tests are not optional - they are a required part of the checklist + - write unit tests for new functions/methods + - write unit tests for modified functions/methods + - add new test cases for new code paths + - update existing test cases if behavior changes + - tests cover both success and error scenarios +- **CRITICAL: all tests must pass before starting next task** - no exceptions +- **CRITICAL: update this plan file when scope changes during implementation** +- Run tests after each change +- Maintain backward compatibility + +## Solution Overview +The implementation flow is: +1. navidrome.SyncArtists populates artist_settings table +2. musicbrainz.SyncAll calls SyncArtistDiscography which: + - Retrieves artist settings via getArtistFilterOptions + - Applies ignore_singles/ignore_compilations filters via ApplyTypeToggles + - Stores filtered results in external_releases table +3. scanner.ScanAll iterates artists and calls ScanArtist which: + - Retrieves current artist settings via GetArtistSettings + - Applies ignore_singles/ignore_compilations filters via TypeFilter + - Compares local albums vs filtered external releases + +This creates two filtering points: +- Storage-level filtering during MusicBrainz sync (optimizes storage) +- Runtime filtering during scanning (ensures real-time responsiveness to setting changes) + +## Technical Details +- Data flow: Navidrome artist sync → MusicBrainz discography sync (with filtering) → Navidrome album sync → Scanner (with filtering) +- Key functions: + - GetArtistSettings/GetAllArtistSettings (database layer) + - getArtistFilterOptions/ApplyTypeToggles (musicbrainz filtering) + - ScanArtist/ScanAll with TypeFilter (scanner filtering) +- Data structures: ArtistSettings, TypeFilter, ExternalRelease, LocalAlbum, MissingRelease + +## What Goes Where +- **Implementation Steps** (`[ ]` checkboxes): tasks achievable within this codebase - code changes, tests, documentation updates +- **Post-Completion** (no checkboxes): items requiring external action - manual testing, changes in consuming projects, deployment configs, third-party verifications + +## Implementation Steps + +### Task 1: Verify Current Implementation +- [x] Review cmd/naviwatcher/main.go syncAndScan function to confirm full pipeline execution +- [x] Review internal/scanner/scan.go ScanArtist function for proper settings retrieval and filtering +- [x] Review internal/scanner/scan.go ScanAll function for proper iteration and filtering application +- [x] Review internal/musicbrainz/sync.go SyncArtistDiscography for proper settings retrieval and filtering +- [x] Review internal/musicbrainz/filter.go ApplyTypeToggles functions for correct filtering logic +- [x] Review internal/database/artist_settings.go for proper settings retrieval functions +- [x] Write unit tests to verify the filtering logic works correctly in both paths +- [x] Run existing test suite to ensure no regressions +- [ ] Must pass before next task + +### Task 2: Enhance Test Coverage +- [ ] Create comprehensive test cases for scanner filtering with various ignore_singles/ignore_compilations combinations +- [ ] Create comprehensive test cases for scanner filtering with various ignore_singles/ignore_compilations combinations +- [ ] Create test cases for MusicBrainz sync filtering with various scenarios +- [ ] Add integration tests that verify the full flow from settings change to filtered scan results +- [ ] Test edge cases: empty settings, null values, default behavior +- [ ] Write tests for error conditions and fallback behaviors +- [ ] Run tests to ensure they pass +- [ ] Must pass before next task + +### Task 3: Document the Data Flow +- [ ] Update documentation to clearly explain how ignore_singles/ignore_compilations settings propagate through the system +- [ ] Add comments to key functions explaining the filtering flow +- [ ] Ensure CLAUDE.md accurately reflects the current implementation +- [ ] Create diagrams or flowcharts if helpful for understanding +- [ ] Must pass before next task + +### Task 4: Final Verification +- [ ] Run full test suite to ensure all changes work correctly +- [ ] Verify no breaking changes were introduced +- [ ] Confirm that the implementation handles the use case described in the memory file +- [ ] Update this plan with completion status + +## Post-Completion +*Items requiring manual intervention or external systems - no checkboxes, informational only* + +**Manual verification**: +- Manual testing of the end-to-end flow with toggles enabled/disabled +- Verification that changes to ignore_singles/ignore_compilations take effect in a timely manner +- Performance testing to ensure filtering doesn't introduce significant overhead + +**External system updates**: +- None required for this verification task \ No newline at end of file diff --git a/internal/musicbrainz/filter_test.go b/internal/musicbrainz/filter_test.go new file mode 100644 index 0000000..a1908b1 --- /dev/null +++ b/internal/musicbrainz/filter_test.go @@ -0,0 +1,122 @@ +package musicbrainz_test + +import ( + "testing" + + "naviwatcher/internal/database" + "naviwatcher/internal/musicbrainz" +) + +func TestApplyTypeToggles(t *testing.T) { + releases := []database.ExternalRelease{ + {RGID: "r1", Type: "Single", SecondaryTypes: []string{}}, + {RGID: "r2", Type: "Album", SecondaryTypes: []string{"Single"}}, + {RGID: "r3", Type: "Compilation", SecondaryTypes: []string{}}, + {RGID: "r4", Type: "Album", SecondaryTypes: []string{"Compilation"}}, + {RGID: "r5", Type: "EP", SecondaryTypes: []string{}}, + {RGID: "r6", Type: "Album", SecondaryTypes: []string{"EP"}}, + } + + tests := []struct { + name string + opts musicbrainz.FilterOptions + expectedCounts int + expectedRGIDs []string + }{ + { + name: "No filters", + opts: musicbrainz.FilterOptions{IgnoreSingles: false, IgnoreCompilations: false}, + expectedCounts: 6, + expectedRGIDs: []string{"r1", "r2", "r3", "r4", "r5", "r6"}, + }, + { + name: "Ignore singles only", + opts: musicbrainz.FilterOptions{IgnoreSingles: true, IgnoreCompilations: false}, + expectedCounts: 2, // r3, r4 + expectedRGIDs: []string{"r3", "r4"}, + }, + { + name: "Ignore compilations only", + opts: musicbrainz.FilterOptions{IgnoreSingles: false, IgnoreCompilations: true}, + expectedCounts: 4, // r1, r2, r5, r6 + expectedRGIDs: []string{"r1", "r2", "r5", "r6"}, + }, + { + name: "Ignore both", + opts: musicbrainz.FilterOptions{IgnoreSingles: true, IgnoreCompilations: true}, + expectedCounts: 0, + expectedRGIDs: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := musicbrainz.ApplyTypeToggles(releases, tt.opts) + if len(got) != tt.expectedCounts { + t.Errorf("expected %d releases, got %d", tt.expectedCounts, len(got)) + } + for i, r := range got { + if r.RGID != tt.expectedRGIDs[i] { + t.Errorf("expected RGID %s at index %d, got %s", tt.expectedRGIDs[i], i, r.RGID) + } + } + }) + } +} + +func TestApplyTypeTogglesToReleaseGroups(t *testing.T) { + groups := []musicbrainz.ReleaseGroup{ + {ID: "g1", Type: "Single", SecondaryTypes: []string{}}, + {ID: "g2", Type: "Album", SecondaryTypes: []string{"Single"}}, + {ID: "g3", Type: "Compilation", SecondaryTypes: []string{}}, + {ID: "g4", Type: "Album", SecondaryTypes: []string{"Compilation"}}, + {ID: "g5", Type: "EP", SecondaryTypes: []string{}}, + {ID: "g6", Type: "Album", SecondaryTypes: []string{"EP"}}, + } + + tests := []struct { + name string + opts musicbrainz.FilterOptions + expectedCounts int + expectedIDs []string + }{ + { + name: "No filters", + opts: musicbrainz.FilterOptions{IgnoreSingles: false, IgnoreCompilations: false}, + expectedCounts: 6, + expectedIDs: []string{"g1", "g2", "g3", "g4", "g5", "g6"}, + }, + { + name: "Ignore singles only", + opts: musicbrainz.FilterOptions{IgnoreSingles: true, IgnoreCompilations: false}, + expectedCounts: 2, // g3, g4 + expectedIDs: []string{"g3", "g4"}, + }, + { + name: "Ignore compilations only", + opts: musicbrainz.FilterOptions{IgnoreSingles: false, IgnoreCompilations: true}, + expectedCounts: 4, // g1, g2, g5, g6 + expectedIDs: []string{"g1", "g2", "g5", "g6"}, + }, + { + name: "Ignore both", + opts: musicbrainz.FilterOptions{IgnoreSingles: true, IgnoreCompilations: true}, + expectedCounts: 0, + expectedIDs: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := musicbrainz.ApplyTypeTogglesToReleaseGroups(groups, tt.opts) + if len(got) != tt.expectedCounts { + t.Errorf("expected %d groups, got %d", tt.expectedCounts, len(got)) + } + for i, g := range got { + if g.ID != tt.expectedIDs[i] { + t.Errorf("expected ID %s at index %d, got %s", tt.expectedIDs[i], i, g.ID) + } + } + }) + } +}